Report coverage for binaries a rust_test runs as subprocesses - #4287
Draft
robot-head wants to merge 1 commit into
Draft
robot-head wants to merge 1 commit into
robot-head wants to merge 1 commit into
Conversation
A `rust_test` can list a `rust_binary` in `data` and spawn it through `CARGO_BIN_EXE_<name>`. Under `bazel coverage` that binary is instrumented, inherits `LLVM_PROFILE_FILE`, and writes its `.profraw` into the test's `COVERAGE_DIR`, where the collector merges it with the test's own. But `llvm-cov export` is given only the test binary, and it reads counters only for the objects it is given, so the subprocess's counters were merged and then discarded. Lines that only the subprocess reaches were reported as though no test ran them. - `rust_test` names each instrumented bin crate in `data` in `RUST_COVERAGE_OBJECTS`, and the collector passes each one to `llvm-cov export` as `-object`. - `data` joins `deps` and `crate` as a coverage dependency attribute, as it already is for `cc_*`, `sh_*` and `py_*` rules. That puts the binary's sources in the instrumented file set, and puts the binary in the coverage metadata that split coverage postprocessing stages. `//test/coverage_subprocess` is a binary reached only through a subprocess. Presubmit's coverage validation now requires its lines to be hit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
A
rust_testcan list arust_binaryindataand spawn it throughCARGO_BIN_EXE_<name>. This is the usual way to smoke-test a CLI. Underbazel coverage, lines that only that subprocess reaches are reported as not covered.Cause
The binary is instrumented, and because it inherits
LLVM_PROFILE_FILEit writes its.profrawinto the test'sCOVERAGE_DIR.collect_rust_coveragemerges that file with the test's own. It then runsllvm-cov exportwith only the test binary, andllvm-covreads counters only for the objects it is given. So the subprocess's counters are merged and then discarded.Change
rust.bzl:rust_testlists each instrumented bin crate indatainRUST_COVERAGE_OBJECTS. The paths are resolved the same way asRUST_LLVM_COV: exec paths withexperimental_use_coverage_metadata_files, runfiles paths without it.collect_rust_coverage.rs: passes each of those binaries tollvm-cov exportas-object.rustc.bzl:datajoinsdepsandcrateindependency_attributes, as it already is forcc_*,sh_*andpy_*rules. This puts the binary's sources in the instrumented file set. It also puts the binary itself in the coverage metadata that--experimental_split_coverage_postprocessingstages, which the collector needs in order to open it.RUST_COVERAGE_OBJECTSis only passed between the rule and the collector; it adds no attribute or setting.Test
//test/coverage_subprocesshas a binary whose lines only a subprocess reaches. Presubmit's coverage validation now requiresgreeter.rsto have lines hit.greeter.rsLH:0 LF:0LH:7 LF:7--noexperimental_split_coverage_postprocessing --//rust/settings:experimental_use_coverage_metadata_files=falseLH:7 LF:7bazel coverage --instrumentation_filter=^// --instrument_test_targets //test/...on linux-aarch64 (excluding the two multi-channel packages): 506 pass, 40 skipped.This came up in a downstream Cargo workspace that builds through rules_rs. There, it restored coverage for CLI source files exercised only by
CARGO_BIN_EXEsmoke tests (e.g. one file went from 461/608 to 583/608 lines).This change was written with AI assistance (Claude Code), per the AI tools policy in CONTRIBUTING.md. It stays a draft until it has been reviewed by hand.