Skip to content

Commit

Permalink
Fix bazel coverage false negative
Browse files Browse the repository at this point in the history
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
  • Loading branch information
keith authored and copybara-github committed Feb 15, 2022
1 parent 464f5d5 commit 16de035
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
27 changes: 27 additions & 0 deletions src/test/shell/bazel/bazel_coverage_starlark_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion tools/test/collect_coverage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 16de035

Please sign in to comment.