From 344e8f8e97db2e2aa9b2fce7d68083a7549e4bc6 Mon Sep 17 00:00:00 2001
From: Brentley Jones <github@brentleyjones.com>
Date: Wed, 16 Feb 2022 10:03:01 -0600
Subject: [PATCH] Fix bazel coverage false negative (#14836)

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
(cherry picked from commit 16de03595e21f7bf31818e717505b23c953b3b7d)

Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
---
 .../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() {