Surface untested source files at 0% coverage in reports

[olivembo]

Summary

llvm-cov only reports files whose object files are linked into at least one test.

Source files that exist in the workspace but no cc_test pulls in silently disappear from the coverage report.

This PR adds a Bazel aspect that walks the dependency graph to collect all C/C++ sources, compares them against what llvm-cov actually reported, and augments the LCOV, HTML, and text outputs with synthetic 0%-coverage entries for the missing files.

The coverage pipeline (merger, reporter, justify, effective_coverage) was originally developed by Jan Schlosser in eclipse-score/communication#549.
This PR centralizes it in score_cpp_policies, adds the baseline-coverage aspect, and improves the reporter.

Approach

Why an aspect + manifest instead of patching --instrumentation_filter?

instrumentation_filter controls which targets get built with coverage instrumentation, but llvm-cov still only reports files whose .o is linked into a test binary. There is no Bazel-native way to surface the gap. The aspect walks deps/srcs/implementation_deps transitively and writes a manifest of all reachable .cpp/.cc/.cxx/.c files. The reporter then diffs manifest vs. LCOV output.

Why heuristic line counts instead of exact numbers?

Without running llvm-cov against actual instrumented object files (which don't exist for untested sources), we can't get exact instrumentable line counts. The heuristic (_count_instrumentable_lines) filters blank lines, comments, preprocessor directives, lone braces, and namespace declarations. All outputs explicitly label these as estimates
(~N, "estimated via heuristic") to avoid false precision. The TOTALS line in summary.txt is intentionally left untouched. Only a WARNING banner is appended.

Known limitations (tracked for follow-up)

• Baseline coverage / rules_cc gap: Proper baseline generation requires upstream support in rules_cc first. Current heuristic line counts are estimates.
• QNX: Coverage on QNX may pull in Rust coverage data; how to separate these is still an open question.
• constexpr functions: These appear uncovered due to how llvm-cov handles them.
• toolchains_llvm: Once the above are resolved, contributing upstream there is the intended next step.

What changed

coverage/defs.bzl: _collect_sources_aspect, score_instrumented_sources_manifest rule, instrumented_sources_manifest parameter on score_coverage_reporter macro
coverage/reporter.py: LCOV augmentation, HTML augmentation (top-banner + per-file pages + detail table), text summary banner, workspace-bounds check, HTML escaping including quotes
coverage/BUILD.bazel: reporter_lib py_library for testability
tests/coverage/: uncovered.cpp/.h fixture (library with no test), reporter_test.py with unit tests for all augmentation helpers including path-traversal rejection

Consumer usage

load("@score_cpp_policies//coverage:defs.bzl",
     "score_coverage_reporter", "score_instrumented_sources_manifest")

score_instrumented_sources_manifest(
    name = "instrumented_sources",
    targets = ["//src:mylib"],
)

score_coverage_reporter(
    name = "reporter_wrapper",
    llvm_cov = "@llvm_toolchain//:llvm-cov",
    llvm_profdata = "@llvm_toolchain//:llvm-profdata",
    instrumented_sources_manifest = ":instrumented_sources",
)

[nradakovic]

To be honest. I will need a lot of time (and coffee) to do proper review. Hopefully by Monday I will have feedback.

[olivembo]

To be honest. I will need a lot of time (and coffee) to do proper review. Hopefully by Monday I will have feedback.

Thanks for the quick feedback Nikola.

In fact, I took most of the code from this PR in the communication repository. Additionally, I fixed that files without unittests are shown in the report so not a higher coverage is show that is actually the case.

Maybe for the review you seek help in the communication team.

Happy review and hopefully good ☕😉

[nradakovic]

To be honest. I will need a lot of time (and coffee) to do proper review. Hopefully by Monday I will have feedback.

Thanks for the quick feedback Nikola.

In fact, I took most of the code from this PR in the communication repository. Additionally, I fixed that files without unittests are shown in the report so not a higher coverage is show that is actually the case.

Maybe for the review you seek help in the communication team.

Happy review and hopefully good ☕😉

And this is also another problem. I'll for sure have some questions why certain implementation blocks are done in such way, but since communication PR is already merged (and in use?) not sure if I want to dive into large discussion and diverge from original PR. Anyway, let's see once I have concrete feedback. If something is on critical path, I will request that communication change their implementation.

[nradakovic]

So something that I see by just looking at. This implementation relies on a generated Bash wrapper to reconstruct Bazel runfiles and workspace paths before invoking the actual reporter. This feels a bit non-Bazel-native. Can we instead model this as an executable Starlark rule, or move runfiles/path resolution into the reporter binary using Bazel runfiles libraries?

[nradakovic]

For start I left 2 comments. It's enough to discuss Bazel implementation blocks (I still haven't check Python scripts) but I think they look OK. Have to do better look.

I see two paths for this PR to get merged:

1. I will approve as is but only if we have plan to improve current implementation (tickets to track it with clear plan how to improve it). What communication guys did is on them to support it. This will be untilized by every single module in S-CORE so we do not want to find nasty surprises 2 days before release of S-CORE.
2. We start now changing blocks but this will require some time and probably we will need to include more people who have more exp. with Bazel and code coverage.

@RSingh1511 Please provide your thoughts on this as well.

[nradakovic]

As I already said in my previous comment, this simply looks wrong from Bazel native perspective. This is one of the biggest anti-patterns in Bazel and people very often fall into this trap which later brings nothing more than pain in the head. Genrules are for simple content transformation (cat, grep, sed), not what we have here.

The solution here would be to encapsulate all this into custom rules with executable python binaries + Python's Runfiles API as dependency.

Then we would benefint in multiple fronts:

1. Escaping horrible complexity with genrule (\\$$, $$, $)
2. Increasing reproducibility since shell has differences per OS.
3. Increasing debug capabilites.

[olivembo]

Thanks for the feedback. Bazel is a b...east 🤪 ... I still try to tame it 😉

Addressed with d762cc5

[nradakovic]

And here is where things become fun. What if reference integration wants to execute code coverage which is in the end overall plan. How reference integration will set reporter for different modules? For start, everything that needs to be set in module .bazelrc file needs to be implemented over module extention (if possible).

[RSingh1511]

Hello @olivembo , Thanks for this. Since you referenced communication, could you create an adoption PR in communication or logging showing how we can consume it? A real data example would help with verification and allow everyone to understand the use case.

[Copilot]

Pull request overview

This PR extends the repo’s Bazel/llvm-cov coverage tooling so that C/C++ source files reachable from configured coverage roots—but not linked into any executed test binary—are still surfaced in coverage outputs as synthetic 0%-coverage entries (LCOV, HTML, and text summary). It does so by generating a manifest of reachable sources via a Bazel aspect and then augmenting the reporter outputs based on a manifest-vs-LCOV diff.

Changes:

• Adds a Bazel aspect + score_instrumented_sources_manifest rule to enumerate transitive C/C++ sources and plumbs an optional instrumented_sources_manifest into score_coverage_reporter.
• Implements report augmentation in coverage/reporter.py (LCOV + HTML index/pages + text summary banner) and adds unit tests for the augmentation helpers.
• Adds coverage adoption docs/config (coverage/README.md, coverage.bazelrc, filter regex baseline) and a consumer smoke-test workspace under tests/coverage.

Reviewed changes

Copilot reviewed 5 out of 27 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/MODULE.bazel	Adds rules_python + Python toolchain setup for the test workspace.
tests/coverage/uncovered.h	Adds an intentionally untested header fixture for smoke-testing “0% untested file” surfacing.
tests/coverage/uncovered.cpp	Adds an intentionally untested source fixture for smoke-testing “0% untested file” surfacing.
tests/coverage/reporter_test.py	Adds unit tests for coverage reporter augmentation helpers (LCOV/HTML/text/escaping/path checks).
tests/coverage/coverage_justifications.yaml	Adds a sample justification DB used by the coverage smoke test.
tests/coverage/coverage_filter_regexes.txt	Adds smoke-test consumer-side extra ignore regexes.
tests/coverage/coverable.h	Adds a small API exercised by the coverage smoke test (with an intentionally uncovered branch).
tests/coverage/coverable.cpp	Adds the implementation for the smoke-test library.
tests/coverage/coverable_test.cpp	Adds a test that intentionally leaves one branch uncovered for report validation.
tests/coverage/BUILD.bazel	Wires the smoke test targets, manifest generation, and reporter wrapper.
tests/BUILD.bazel	Exports MODULE.bazel so the reporter wrapper can resolve workspace root at runtime.
tests/.bazelrc	Imports coverage bazelrc and adds coverage smoke-test settings for the tests workspace.
README.md	Updates top-level README to document coverage tooling availability and link to coverage guide.
MODULE.bazel	Adds rules_python, rules_shell, Python toolchain, and pip hub wiring for coverage tooling.
coverage/requirements.in	Adds pyyaml as the direct dependency for justification processing.
coverage/requirements_lock.txt	Adds a locked pyyaml pin with hashes for Bazel pip integration.
coverage/reporter.py	Implements final report generation + augmentation with synthetic 0%-coverage entries for untested sources.
coverage/README.md	Adds an adoption guide for consumers (setup, configs, optional untested-files manifest).
coverage/merger.py	Adds per-test merger that packages profdata + object file metadata for final reporting.
coverage/justify.py	Adds YAML + in-source marker processing to generate a justification manifest.
coverage/generate_coverage_html.sh	Adds a consumer-facing driver to extract reports, run justification/effective-coverage, and optionally archive artifacts.
coverage/filter_regexes.txt	Adds baseline ignore regexes (tests/mocks/fakes/external/benchmarks).
coverage/effective_coverage.py	Adds HTML post-processing + effective coverage computation incorporating justifications.
coverage/defs.bzl	Adds the manifest aspect/rule and updates score_coverage_reporter to pass the manifest into the reporter.
coverage/coverage.bazelrc	Adds generic Bazel coverage flags/config to be imported by consumers.
coverage/BUILD.bazel	Adds Bazel targets for merger/reporter/justify/effective_coverage and the driver script.
.gitignore	Ignores generated cpp_coverage/ output directories.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[olivembo]

Hello @olivembo , Thanks for this. Since you referenced communication, could you create an adoption PR in communication or logging showing how we can consume it? A real data example would help with verification and allow everyone to understand the use case.

--> eclipse-score/communication#604

[FScholPer]

Gentleman still wait or proceed with merge?

[castler]

Two cents from the guy where this was copied over from.

As I wrote already in the Slack, the proper generation of baseline coverage requires a deeper look. Especially, as rules_cc is missing this support today - and I believe for a proper generation of baseline coverage, we need to look there first. But, this will require quite some time.

Second, I am still in the workings to make the coverage also correct for QNX. First experiments also showed that the coverage for QNX will bring coverage results for rust included. Thus, I am also not sure yet how to best handle this and if this will be at the end an "cpp_policies" only thing, or might not include Rust also.

Third, we still have a problem with constexpr functions, as they appear not covered.

Fourth, my original plan would have been, once the three mentioned points are solved, to contribute this solution to toolchains_llvm, as obviously this can be reused by a way broader community.

Anyhow, it can be decided that this is "good enough" for now, and these problems are tackled in an incremental way.
Also just raising awareness for them.

---

On a personal note - while the licenses and copyrights that we utilize in this project fully allow a copying of the code to different places without touching the license + copyright, I still think it follows best practices to give at least some credit / blame for the original author. Feels quite bad if a topic where quite some time was invested is just copied over. I would have loved to see at least a clean commit with the original state and author, and then the improvements, refactorings on top of it. (I do not require to change this, but I would like to raise awareness).

[olivembo]

Gentleman still wait or proceed with merge?

It's not approved yet

[olivembo]

Two cents from the guy where this was copied over from.

As I wrote already in the Slack, the proper generation of baseline coverage requires a deeper look. Especially, as rules_cc is missing this support today - and I believe for a proper generation of baseline coverage, we need to look there first. But, this will require quite some time.

Second, I am still in the workings to make the coverage also correct for QNX. First experiments also showed that the coverage for QNX will bring coverage results for rust included. Thus, I am also not sure yet how to best handle this and if this will be at the end an "cpp_policies" only thing, or might not include Rust also.

Third, we still have a problem with constexpr functions, as they appear not covered.

Fourth, my original plan would have been, once the three mentioned points are solved, to contribute this solution to toolchains_llvm, as obviously this can be reused by a way broader community.

Anyhow, it can be decided that this is "good enough" for now, and these problems are tackled in an incremental way. Also just raising awareness for them.

On a personal note - while the licenses and copyrights that we utilize in this project fully allow a copying of the code to different places without touching the license + copyright, I still think it follows best practices to give at least some credit / blame for the original author. Feels quite bad if a topic where quite some time was invested is just copied over. I would have loved to see at least a clean commit with the original state and author, and then the improvements, refactorings on top of it. (I do not require to change this, but I would like to raise awareness).

Hi Jan, thanks for the detailed write-up. This is genuinely useful context.

On the technical points:

• Baseline coverage / rules_cc: Agreed. The heuristic line counts in this PR are explicitly labeled as estimates precisely because we know proper baseline generation needs upstream work in rules_cc first. That's not something we tried to paper over.
• QNX + Rust interleaving: That's not on our radar yet. Good to know it's an active problem. If the solution ends up language-agnostic, landing it there rather than in cpp_policies alone makes sense.
• constexpr: Known and annoying. Nothing to add here.

On attribution: you're right to call this out, and I'll be direct: It should have been handled better from the start. A commit that establishes the original state with proper authorship, followed by the changes on top, would have been the clean way to do it. That ship has sailed for the commit history, but I've updated the PR description to explicitly credit your PR #549 as the origin of the pipeline. Not a substitute for getting the history right, but at least the record is there.

The plan: Merge this as "good enough for now", and track the three open items as follow-ups. Does that work for you?