[8.5.0] Add basic support for coverage on Windows #26927
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The core idea of this change is to make `collect_coverage` an `sh_binary` instead of a single `.sh` file as the former can be executed on Windows via the native launcher it creates. However, this requires a bunch of work in the test framework: * Ensure that the `collect_coverage` script has its runfiles included in `TestActionBuilder`. Along the way and to ensure consistency, clean up unnecessary cases for the runfiles of the LCOV merger tool. * Add support for launching the coverage wrapper to `tw`, which didn't have any logic for this case. * Hide the runfiles env variables meant for the test from the coverage wrapper. The wrapper has its own runfiles tree and would otherwise be mislead to reuse the test's runfiles tree, which doesn't work. Instead, wrap the environment before invoking the wrapper and then unwrap them in the wrapper. * Modify the coverage output generator to emit `\n` line endings on all platforms for consistency (and to get existing tests to pass). With these changes, `bazel_coverage_starlark_test` now passes on Windows after adding batch test variants of shell tests. Support for Java and C++ will be added in future PRs. Work towards bazelbuild#6374 Work towards bazelbuild#18839 Closes bazelbuild#26603. PiperOrigin-RevId: 799018475 Change-Id: Icdb1e6e4698b2a749fbb17f7e1a9aaa34e895d70
0713563 to
0eb1f35
Compare
dd-mergequeue bot
pushed a commit
to DataDog/datadog-agent
that referenced
this pull request
Jan 15, 2026
### What does this PR do? Bump `bazel` version from [8.4.2](https://github.com/bazelbuild/bazel/releases/tag/8.4.2) to [8.5.1](https://github.com/bazelbuild/bazel/releases/tag/8.5.1). To make that happen, we also need to: - specify which `bash` to use on Windows when evaluating **repository** rules, for compatibility with bazelbuild/bazel#26927: ``` ERROR: /path/to/external/bazel_tools/tools/test/BUILD:23:10: in sh_binary rule @@bazel_tools//tools/test:collect_coverage: Error in fail: No suitable shell toolchain found: * if you are running Bazel on Windows, set the BAZEL_SH environment variable to the path of bash.exe ``` (actual job output: https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/1349246422#L77) - bump `rules_go` from (implicit) [0.57.0](https://github.com/bazel-contrib/rules_go/releases/tag/v0.57.0) to (explicit) [0.59.0](https://github.com/bazel-contrib/rules_go/releases/tag/v0.59.0) for bazel-contrib/rules_go/pull/4493 to be compatible with bazelbuild/bazel/pull/27296: ``` Error: 'Facts' value has no field or method 'clear' ``` ### Motivation 1. this upgrade alone brings several improvements and bug fixes, among which: - bazelbuild/bazel#27117 - bazelbuild/bazel#27296 - bazelbuild/bazel#27417, covers: - bazelbuild/bazel#25834 - bazelbuild/bazel#25863 - bazelbuild/bazel#25864 - bazelbuild/bazel#25870 - bazelbuild/bazel#26698 - bazelbuild/bazel#27531 - bazelbuild/bazel#27560 - bazelbuild/bazel#27564 - bazelbuild/bazel#27705 - bazelbuild/bazel#27995 - bazelbuild/bazel#27996 2. `bazel` 9.0 is due soon, so better off favoring incremental bumps. ### Additional Notes Leveraging [the latter](bazelbuild/bazel#27996) might allow us to later reconsider whether we'd like to go back to the `--remote_cache` flag (instead of the `--remote_executor` flag that we had to switch to in #44962). Co-authored-by: regis.desgroppes <regis.desgroppes@datadoghq.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The core idea of this change is to make
collect_coverageansh_binaryinstead of a single.shfile as the former can be executed on Windows via the native launcher it creates. However, this requires a bunch of work in the test framework:collect_coveragescript has its runfiles included inTestActionBuilder. Along the way and to ensure consistency, clean up unnecessary cases for the runfiles of the LCOV merger tool.tw, which didn't have any logic for this case.\nline endings on all platforms for consistency (and to get existing tests to pass).With these changes,
bazel_coverage_starlark_testnow passes on Windows after adding batch test variants of shell tests. Support for Java and C++ will be added in future PRs.Work towards #6374 Work towards #18839
Closes #26603.
PiperOrigin-RevId: 799018475
Change-Id: Icdb1e6e4698b2a749fbb17f7e1a9aaa34e895d70
Commit: 74285e2