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: bazelbuild#13978

Closes bazelbuild#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)
  • Loading branch information
keith authored and brentleyjones committed Feb 16, 2022
1 parent af34c45 commit d53c0a1
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 d53c0a1

Please sign in to comment.