Revamp code coverage tooling#30181
Conversation
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR replaces legacy coverage utilities with a unified tools/run-cov tool that can generate terminal/HTML/LLM-friendly coverage reports, including coverage focused on changed lines in a diff, and adds Bazel-backed tests and fixtures to prevent regressions.
Changes:
- Add
tools/run-cov(Python) to run/reuse Bazel C++ coverage and emit terminal, HTML, and LLM-optimized reports (including diff coverage). - Add Bazel
sh_testcoverage-tool regression tests plus LCOV/diff fixtures undertools/tests/. - Remove legacy coverage scripts and update repo config (
.bazelrccoverage settings,.gitignoreforcoverage-out/).
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tools/run-cov |
New unified coverage runner/report generator (terminal/html/llm + diff coverage). |
tools/tests/run_cov_test.sh |
End-to-end Bazel shell test for tools/run-cov outputs and failure modes. |
tools/tests/BUILD |
Registers the run_cov_test Bazel test and its runfiles. |
tools/tests/testdata/sample.diff |
Fixture unified diff used to validate diff-coverage classification. |
tools/tests/testdata/coverage_fixture.dat |
Fixture LCOV data used by the regression test. |
tools/tests/testdata/BUILD |
Exposes test fixtures as Bazel data files. |
tools/BUILD |
Exports run-cov for Bazel runfiles usage. |
.bazelrc |
Updates coverage config flags/environment. |
.gitignore |
Ignores coverage-out/ (default output dir). |
.claude/skills/improve-coverage/SKILL.md |
Adds a Claude skill/playbook for using the new coverage tooling. |
tools/single_test_cov.sh |
Removed legacy single-test coverage script. |
tools/gen_coverage.py |
Removed legacy coverage generator. |
tools/coverage_dash.py |
Removed legacy coverage dashboard generator. |
| info.changed_lines[current_file].add(current_line) | ||
| current_line += 1 | ||
| elif line.startswith("-"): | ||
| pass # deleted line — don't advance new-file counter |
There was a problem hiding this comment.
parse_unified_diff() treats any non "+"/"-" line inside a hunk as a context line and increments current_line. This will miscount when the diff contains metadata lines like \ No newline at end of file (which should not advance either side’s line counters), causing off-by-one changed-line mapping for the remainder of the hunk. Consider explicitly skipping lines starting with \\ (and potentially other non-hunk metadata) when current_file is set.
| pass # deleted line — don't advance new-file counter | |
| pass # deleted line — don't advance new-file counter | |
| elif line.startswith("\\"): | |
| pass # hunk metadata (for example: '\ No newline at end of file') |
CI test resultstest results on build#83411
|
| return None | ||
|
|
||
| bin_dir = os.path.join( | ||
| output_base, "external", "current_llvm_toolchain_llvm", "bin" |
There was a problem hiding this comment.
Did you try this on arm. I remember something breaking there when I did similar for PGO.
There was a problem hiding this comment.
I didn't. I don't think any developers work on arm. I guess if we wanted to use this tooling in CI we'd need to address that for sure.
| workspace_root: str | None, | ||
| output, | ||
| ): | ||
| """Generate an LLM-optimized diff coverage report in markdown.""" |
There was a problem hiding this comment.
What does "LLM-optimized" mean?
There was a problem hiding this comment.
basically it means no formatting.
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def parse_lcov(path: str) -> CoverageReport: |
There was a problem hiding this comment.
Bit confused by all the things going on in this script:
- Parsing lcov
- Demangling
- Output generation
Isn't there existing tooling for all of this? Gcov? Never really been much into coverage reports.
There was a problem hiding this comment.
I'm unaware of any tooling that would provide reporting on the terminal. But maybe? If there is then we should use that.
| build:coverage --action_env=BAZEL_USE_LLVM_NATIVE_COVERAGE=1 | ||
| build:coverage --action_env=GCOV=llvm-profdata | ||
| build:coverage --copt=-DNDEBUG | ||
| build:coverage --define=dynamic_link_tests=true | ||
| build:coverage --repo_env=BAZEL_USE_LLVM_NATIVE_COVERAGE=1 | ||
| build:coverage --combined_report=lcov | ||
| build:coverage --experimental_use_llvm_covmap | ||
| build:coverage --experimental_generate_llvm_lcov | ||
| build:coverage --experimental_split_coverage_postprocessing | ||
| build:coverage --experimental_fetch_all_coverage_outputs | ||
| build:coverage --collect_code_coverage |
There was a problem hiding this comment.
i copied these settings from another c++ project two years ago. i don't know how much of that is old bazel versions vs secret magic settings. in any case, with these simplified settings coverage passes the spot testing i've been doing.
|
|
||
|
|
||
| @dataclass | ||
| class LineCoverage: |
There was a problem hiding this comment.
The tool is large because it does parsing of coverage data and commits in order to be able to build custom output formats for UIs like the terminal.
| workspace_root: str | None, | ||
| output, | ||
| ): | ||
| """Generate an LLM-optimized diff coverage report in markdown.""" |
There was a problem hiding this comment.
basically it means no formatting.
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def parse_lcov(path: str) -> CoverageReport: |
There was a problem hiding this comment.
I'm unaware of any tooling that would provide reporting on the terminal. But maybe? If there is then we should use that.
| return None | ||
|
|
||
| bin_dir = os.path.join( | ||
| output_base, "external", "current_llvm_toolchain_llvm", "bin" |
There was a problem hiding this comment.
I didn't. I don't think any developers work on arm. I guess if we wanted to use this tooling in CI we'd need to address that for sure.
| @@ -1,289 +0,0 @@ | |||
| import argparse | |||
There was a problem hiding this comment.
We should update the bazel wiki's coverage subheader to point to this new tool once this merges
Revamp code coverage tooling
Backports Required
Release Notes