From 16de03595e21f7bf31818e717505b23c953b3b7d Mon Sep 17 00:00:00 2001
From: Keith Smiley <keithbsmiley@gmail.com>
Date: Tue, 15 Feb 2022 05:36:29 -0800
Subject: [PATCH] Fix bazel coverage false negative

Previously this short circuit meant the tests weren't actually run, and
they would always exit 0, in the case the test rule didn't set the lcov
related attributes.

More context: https://github.com/bazelbuild/bazel/issues/13978

Closes #14807.

RELNOTES: Fixed an issue where Bazel could erroneously report a test passes in coverage mode without actually running the test.
PiperOrigin-RevId: 428756211
---
 .../bazel/bazel_coverage_starlark_test.sh     | 27 +++++++++++++++++++
 tools/test/collect_coverage.sh                |  5 +++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/test/shell/bazel/bazel_coverage_starlark_test.sh b/src/test/shell/bazel/bazel_coverage_starlark_test.sh
index dd9e20b0ba517b..0ea7e032ef5dad 100755
--- a/src/test/shell/bazel/bazel_coverage_starlark_test.sh
+++ b/src/test/shell/bazel/bazel_coverage_starlark_test.sh
@@ -58,6 +58,33 @@ EOF
         || fail "Coverage run failed but should have succeeded."
 }
 
+function test_starlark_rule_without_lcov_merger_failing_test() {
+    cat <<EOF > rules.bzl
+def _impl(ctx):
+    executable = ctx.actions.declare_file(ctx.attr.name)
+    ctx.actions.write(executable, "exit 1", is_executable = True)
+    return [
+        DefaultInfo(
+            executable = executable,
+        )
+    ]
+custom_test = rule(
+    implementation = _impl,
+    test = True,
+)
+EOF
+
+    cat <<EOF > BUILD
+load(":rules.bzl", "custom_test")
+
+custom_test(name = "foo_test")
+EOF
+    if bazel coverage //:foo_test > $TEST_log; then
+      fail "Coverage run succeeded but should have failed."
+    fi
+}
+
+
 function test_starlark_rule_with_custom_lcov_merger() {
 
     cat <<EOF > lcov_merger.sh
diff --git a/tools/test/collect_coverage.sh b/tools/test/collect_coverage.sh
index febdb703a00dcd..3c475d9c9c3148 100755
--- a/tools/test/collect_coverage.sh
+++ b/tools/test/collect_coverage.sh
@@ -39,7 +39,10 @@ if [[ -z "$LCOV_MERGER" ]]; then
   # it.
   # TODO(cmita): Improve this situation so this early-exit isn't required.
   touch $COVERAGE_OUTPUT_FILE
-  exit 0
+  # Execute the test.
+  "$@"
+  TEST_STATUS=$?
+  exit "$TEST_STATUS"
 fi
 
 function resolve_links() {