Fix bazel coverage false negative (#14836)

brentleyjones · keith · web-flow · commit 344e8f8e97db · 2022-02-17T03:03:01.000+11:00
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: #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 16de035) Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
diff --git 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
@@ -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() {
