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. With that Feature Request #6 should be resolved.

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.

[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.

[nradakovic]

OK, so it took me a while to understand implementation design here (still haven't cover everything), to follow up on every discussion made (and probably I missed ton of them) and finally to review the PR. To be fair, the drive of the topic is in mess. Several people across different working community groups, across different modules are involved, everyone has it's own 5 cents to present and when you put all of this into mix, no wonder why no one wants to be maintainer of it. Anyway, let's at least try to fix this.

1. @olivembo @castler is there a single place where this topic is documented? I found issues and PRs in communication, reference integration and on CI workflows. This has to be consolidated and put in one place. I don't mind doing this work, but I need to know if there is something more than what I found so far. If not, all good, I will create overlord Epic here and we can swing it from there.
2. As it seems, there are plans to move this beyond usage of S-CORE. I would like to have plan how to move forward with this and who will take this on him to finalize implementation.
   I will soon provide PR review for implementation.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 27 changed files in this pull request and generated no new comments.

[olivembo]

OK, so it took me a while to understand implementation design here (still haven't cover everything), to follow up on every discussion made (and probably I missed ton of them) and finally to review the PR. To be fair, the drive of the topic is in mess. Several people across different working community groups, across different modules are involved, everyone has it's own 5 cents to present and when you put all of this into mix, no wonder why no one wants to be maintainer of it. Anyway, let's at least try to fix this.

1. @olivembo @castler is there a single place where this topic is documented? I found issues and PRs in communication, reference integration and on CI workflows. This has to be consolidated and put in one place. I don't mind doing this work, but I need to know if there is something more than what I found so far. If not, all good, I will create overlord Epic here and we can swing it from there.

2. As it seems, there are plans to move this beyond usage of S-CORE. I would like to have plan how to move forward with this  and who will take this on him to finalize implementation.
   I will soon provide PR review for implementation.

Thanks for taking the time @nradakovic

I just got to know this PR: eclipse-score/baselibs#295

As mentioned: I thinks this PR at the moment is the most centralized one, but I also wrote with @castler and we agreed that a final solution should even be at a higher place (e.g. in rules_cpp), but that this is at least on step into the direction that we can provide a first coverage report to the modules do not fly blind and have no clue about there current coverage.

[nradakovic]

This could be an issue if the consumer doesn't sets it. I see reporter already has label for it, so I wonder why we don't do it for merger as well. We create a small executable script on the fly that has default label for it and we wire it to the command coverage --coverage_output_generator=//tools/coverage:merger_wrapper. Make sense?

[olivembo]

Makes sense. Addressed with #064db8173f96391cd4f1d68faa6f1eeb940d6a2

[4og]

The eclipse-score/baselibs#295 is just to try things out before centralizing it.

The findings are captured in eclipse-score/baselibs#289 Unfortunately, llvm-cov doesn't perform well when it comes to computing line coverage per file. If we are to adopt it in S-CORE we should find a solution for it, ideally fixing it upstream.

I would also be interesting to see how this works together with lcov.

llvm-cov export to JSON --> llvm2lcov --> genhtml

Latest versions of genhtml have a differential view mode with git integration.

[olivembo]

Hi @nradakovic anything left to do from your side to get an approval?

[olivembo]

Hi @nradakovic anything left to do from your side to get an approval?

ping @nradakovic , @RSingh1511 : Can you review?

[dcalavrezo-qorix]

Leaving the TOTALS untouched and only appending a warning banner means a repo with a 100%-coverage gate still passes while entire files are untested — which is the failure mode this PR sets out to fix. With the --empty-profile baseline the untested files enter the real metric (concrete example from the persistency port: one untested 311-line CLI crate pulls effective coverage from ~95% to 86.95% — a number a threshold can act on).