Coverage is broken on Bazel 9

[UebelAndre]

As revealed on #612

Macos: https://buildkite.com/bazel/rules-cc/builds/4764/steps/canvas?sid=019ca055-a33b-4f69-86b4-152db7af6ad6

external/bazel_tools/tools/test/collect_cc_coverage.sh: line 99: : command not found
error: coverage collection script failed

Windows: https://buildkite.com/bazel/rules-cc/builds/4764/steps/canvas?sid=019ca055-a33c-4746-aace-efe7a6b88e02

GCov does not exist at the given path: 'external/+cc_configure_extension+local_config_cc/wrapper/bin/msvc_nop.bat'
error: coverage collection script failed

[UebelAndre]

Maybe it would be worth-while to move @bazel_tools//tools/test/collect_cc_coverage.sh into rules_cc?

[fmeum]

As far as I remember Windows C++ coverage never worked and macOS is pretty fiddly to set up correctly. I think that they only fail in 9 since failures previously weren't fatal.

[UebelAndre]

As far as I remember Windows C++ coverage never worked and macOS is pretty fiddly to set up correctly. I think that they only fail in 9 since failures previously weren't fatal.

I'm obviously a fan of coverage failing the action if there was an error (bazelbuild/bazel#18650) but my expectation was that those errors would be fixed before allowing coverage to fail actions. Would it be possible to add an incompatibility flag for this (defaulted to off) that allows silent failures and then allow users to flip it on (to expose errors) if they either only build on linux or a fix is rolled out?

[UebelAndre]

Maybe it would be worth-while to move @bazel_tools//tools/test/collect_cc_coverage.sh into rules_cc?

For example, rules_rust has it's own coverage collection tool @rules_rust//util/collect_coverage/collect_coverage.rs

[fmeum]

@c-mita @keith

[keith]

--test_env=IGNORE_COVERAGE_COLLECTION_FAILURES=1 would sidestep that behavior if coverage was silently failing before, the assumption being that maybe instead you just shouldn't run bazel coverage and should swap to bazel test

[keith]

Is coverage fully broken or is it just that the buildkite env isn't correctly setup for it? I think:

external/bazel_tools/tools/test/collect_cc_coverage.sh: line 99: : command not found

is a very bad error for a missing env var?

[UebelAndre]

Is coverage fully broken or is it just that the buildkite env isn't correctly setup for it? I think:

external/bazel_tools/tools/test/collect_cc_coverage.sh: line 99: : command not found

is a very bad error for a missing env var?

I opened this because I noticed the issue in rules_rust which I was upgrading to run Bazel 9 (bazelbuild/rules_rust#3867) and saw a cc_test fail with this issue. Seeing rules_cc CI fail with the same issue motivated me to start a discussion here.

[keith]

ah yea that's an interesting case, sounds like coverage was working fine with some target types, but not cc_test, and so now they all have to work? either way that test_env is a good short term workaround

[UebelAndre]

I added --test_env=IGNORE_COVERAGE_COLLECTION_FAILURES=1 to #612 and interestingly there are still some issues though for Bazel 7:

https://buildkite.com/bazel/rules-cc/builds/4778/steps/canvas?sid=019cb066-921f-4077-9bae-c562cd5e3afd

(21:14:42) INFO: From Testing //tests/runfiles:runfiles_test:
==================== Test output for //tests/runfiles:runfiles_test:
ERROR(tools/test/windows/tw.cc:1302) ERROR: src/main/native/windows/process.cc(202): CreateProcessW("C:\b\i3qnbkwl\execroot\_main\bazel-out\x64_windows-fastbuild\bin\tests\runfiles\runfiles_test.exe.runfiles\_main\external\bazel_tools\tools\test\collect_coverage.sh"  tests/runfiles/runfiles_test.exe): The system cannot find the file specified.
 (error: 2)
ERROR(tools/test/windows/tw.cc:1479) Failed to start test process (arg: C:\b\i3qnbkwl\execroot\_main\bazel-out\x64_windows-fastbuild\bin\tests\runfiles\runfiles_test.exe.runfiles\_main\external\bazel_tools\tools\test\collect_coverage.sh)
================================================================================
(21:14:42) ERROR: C:/b/bk-windows-c1ns/bazel/rules-cc/tests/runfiles/BUILD:5:8: output 'tests/runfiles/runfiles_test/coverage.dat' was not created
(21:14:42) ERROR: C:/b/bk-windows-c1ns/bazel/rules-cc/tests/runfiles/BUILD:5:8: Testing //tests/runfiles:runfiles_test failed: not all outputs were created or valid

Separately, it looks like HEAD is broken for some tests. Should these tests be flagged as soft_fail: yes?

[UebelAndre]

Maybe it would be worth-while to move @bazel_tools//tools/test/collect_cc_coverage.sh into rules_cc?

For example, rules_rust has it's own coverage collection tool @rules_rust//util/collect_coverage/collect_coverage.rs

@keith curious what your thoughts are on this. Seems like it could be a path to providing a more consistent behavior around coverage for rules_cc?

[keith]

I think it would be fine to move, maybe a bit weird since it's so intertwined with the other bazel logic tho https://github.com/bazelbuild/bazel/blob/3bdf554c2ff9bbe90dabfdd4d38d8d132e7e61a1/tools/test/collect_coverage.sh#L195

[UebelAndre]

I think it would be fine to move, maybe a bit weird since it's so intertwined with the other bazel logic tho https://github.com/bazelbuild/bazel/blob/3bdf554c2ff9bbe90dabfdd4d38d8d132e7e61a1/tools/test/collect_coverage.sh#L195

I'm not suggestion removing this from bazel. The script I referenced was collect_cc_coverage.sh which seemed both cc specific and like a fitting candidate to fork into rules_cc so changes can be made at a faster pace than the release of new Bazel versions.