use llvm cov for coverage and justification by castler · Pull Request #549 · eclipse-score/communication

castler · 2026-06-15T11:50:20Z

No description provided.

- Move quality/coverage.bazelrc to quality/coverage/coverage.bazelrc for better organization alongside the coverage tooling. - Enable user.bazelrc support via try-import in .bazelrc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

LittleHuba

Really nice work!

LittleHuba · 2026-06-15T11:58:05Z

+# at the report level.
+
+# Exclude mock files.
+.*mock.*\.(h|hpp|cpp)$


Better make it .*_mock.*\.(h|hpp|cpp)$. I'd prefer to start very strict and relax it when we find files that should be excluded.

LittleHuba · 2026-06-15T11:59:03Z

+.*mock.*\.(h|hpp|cpp)$
+
+# Exclude external dependencies (anything under external/).
+.*/external/.*


Not a big fan of the starting .*. What is the base path? Could we live with external/.*?

LittleHuba · 2026-06-15T12:10:05Z

+    return object_files
+
+
+def find_llvm_bin() -> Path:


Uff... not really sure whether this is a good idea.
This could silently break without us being aware.

Two problems I see:

This might become unhermetic if we ever end up with a llvm-toolchain that does not provide llvm-cov for some reason

globbing through all of "external" invites usage of an llvm-cov that ends up there for a different reason (e.g. sysroot for a different tool)

maybe try to hard code it - ,aybe look at it next.

LittleHuba · 2026-06-15T12:11:29Z

+            return llvm_cov.resolve().parent
+
+    # Search from ROOT.
+    root = Path(os.environ.get("ROOT", "."))


Again, not sure whether . is a good fallback here. The fallback also makes no sense since it results in the same logic as ll254.

LittleHuba · 2026-06-15T12:25:35Z

+    # Also remove any other symlinks pointing into sandbox paths.
+    for entry in directory.iterdir():
+        if entry.is_symlink():
+            target = os.readlink(entry)
+            if "sandbox" in target:
+                entry.unlink()


Should we at least issue a warning for such symlinks?
Better to know which ones point in and then add them here explicitly than just removing everything that is there blindly?

maybe log into undeclared test outputs (maybe do later)

LittleHuba · 2026-06-16T15:45:08Z

+┌───────────────────────────┐
+│ generate_coverage_html.sh │  ◄── bazel run //quality/coverage:generate_coverage_html
+└────┬──────────────────────┘
+     │  Extracts html_report/ → cpp_coverage/
+     ▼
+┌─────────────┐     coverage_justifications.yaml
+│  justify.py  │ ◄── + source files (COV_JUSTIFIED markers)
+└────┬────────┘
+     │  manifest.json: {file → {line → justification}}
+     ▼
+┌───────────────────────┐
+│ effective_coverage.py  │ ◄── manifest.json + html_report/
+└────┬──────────────────┘
+     │  • Modifies HTML in-place (restyled justified lines/branches)
+     │  • report.json + summary.txt
+     ▼
+  Console output: effective coverage summary


I think it would help to argue why we do not call this from reporter.py.
If it is because we want to retain the original data without the justifications applied, we could just output the modified report next to the original report.

LittleHuba · 2026-06-16T15:49:04Z

+
+## Requirements
+
+The coverage pipeline was built to satisfy the following requirements:


Should we put those directly into TRLC? Nit-pick can be done in a follow-up.

We can do in an other iteration

LittleHuba · 2026-06-16T15:52:02Z

+Code coverage is generated using LLVM's source-based coverage instrumentation. The instrumentation filter is configured in [`quality/coverage/coverage.bazelrc`](coverage/coverage.bazelrc) to cover `//score/message_passing` and `//score/mw/com` while excluding test and benchmark code.

-### Running Coverage
+HTML reports are generated directly by `llvm-cov show` — no intermediate LCOV conversion or `genhtml` is needed.


Please just drop everything after the dash. It just confuses.

LittleHuba · 2026-06-16T15:53:45Z

+#### Adding a Justification
+
+1. Add an entry to `coverage_justifications.yaml`:
+
+```yaml
+justifications:
+  - id: my-justification-id
+    category: defensive_programming  # or: tool_false_positive, platform_specific, other
+    reason: >
+      Explanation of why these lines cannot be covered.
+    locations:
+      - file: score/mw/com/impl/some_file.cpp
+        line_start: 42
+        line_end: 45
+```
+
+2. Alternatively, reference the justification from code (no `locations` needed in YAML):
+
+```cpp
+unreachable_code();  // COV_JUSTIFIED my-justification-id
+
+// COV_JUSTIFIED_START my-justification-id
+defensive_block();
+more_defensive_code();
+// COV_JUSTIFIED_STOP
+```
+
+#### Effective Coverage
+
+The coverage pipeline calculates:
+- **Raw line coverage**: actual lines hit / total instrumented lines
+- **Effective line coverage**: (lines hit + justified lines) / total instrumented lines
+- **Raw branch coverage**: actual branches hit / total branches
+- **Effective branch coverage**: (branches hit + justified branches) / total branches
+
+The index page shows a banner with overall effective coverage and updates per-file percentages. The `generate_coverage_html` script prints the full summary. Set `COVERAGE_THRESHOLD` to enforce a minimum (default: 100%):
+
+```bash
+COVERAGE_THRESHOLD=95 bazel run //quality/coverage:generate_coverage_html
+```


Not really a huge fan of this duplication. Can we not just link to the actual readme for coverage?

LittleHuba · 2026-06-18T13:34:41Z

-# on PATH inside the run environment.
-LCOV_DAT="${BUILD_WORKSPACE_DIRECTORY}/bazel-out/_coverage/_coverage_report.dat"
+# Resolve OUTPUT_DIR to absolute path (relative to workspace root).
+OUTPUT_DIR="${BUILD_WORKSPACE_DIRECTORY}/${OUTPUT_DIR}"


What if OUTPUT_DIR is an absolute path?

LittleHuba · 2026-06-18T13:42:04Z

+        "--show-region-summary=0",
+    ]
+
+    cxxfilt = llvm_bin_path.parent / "llvm-cxxfilt"


Suggested change

cxxfilt = llvm_bin_path.parent / "llvm-cxxfilt"

cxxfilt = r.Rlocation("llvm_toolchain/llvm-cxxfilt")

LittleHuba · 2026-06-18T13:44:09Z

+
+    # Get llvm tools via runfiles.
+    r = Runfiles.Create()
+    llvm_bin_path = Path(r.Rlocation("llvm_toolchain/llvm-cov"))


It feels hacky that in some places you use r.Rlocation to determine the path. And in others you do path traversal based on this path.

LittleHuba · 2026-06-18T13:52:16Z

+if [[ -z "\\$${RUNFILES_DIR:-}" ]]; then
+  if [[ -d "\\$$0.runfiles" ]]; then
+    export RUNFILES_DIR="\\$$0.runfiles"
+  fi


Should we have an else condition as a safeguard here?

…reports Replace the genhtml/lcov-based coverage report pipeline with a native llvm-cov implementation that produces HTML reports directly from profraw data. Key changes: - Add merger.py as --coverage_output_generator (per-test profraw→profdata+HTML) - Add reporter.py as --coverage_report_generator (aggregate all tests) - Add filter_regexes.txt config file for configurable source filtering - Update coverage.bazelrc with llvm-cov flags (--experimental_use_llvm_covmap, --dynamic_mode=off, continuous profiling mode) - Update generate_coverage_html.sh to extract from the new zip format The source filtering is configurable via filter_regexes.txt (loaded from runfiles) and can be overridden via --filter_sources CLI argument. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add a justification system that allows developers to argue lines that cannot be covered by tests (defensive programming, tool false positives, platform-specific code). Components: - justify.py: YAML loader + source scanner → resolved manifest JSON - effective_coverage.py: HTML post-processor + effective coverage calculator - coverage_justifications.yaml: initial justification database - Integration into generate_coverage_html.sh (runs after report generation) Justifications can be applied via: - In-code markers: // COV_JUSTIFIED <id> - YAML locations: file + line range (no code change needed) Justified lines appear yellow in the HTML report, and effective coverage is calculated as (covered + justified) / instrumented. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Add quality/coverage/README.md with pipeline architecture overview - Add quality/coverage/llvm_cov/README.md with tool-level documentation - Update quality/quality.md to reflect new llvm-cov pipeline and link to the detailed documentation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

LittleHuba

Approving as remaining issues are nitpicks that can be resolved in a follow-up.

olivembo · 2026-06-23T11:09:35Z

@castler Moved and adapted your implementation to the cpp_policies repo.

PR is: eclipse-score/score_cpp_policies#9

Can you check?

castler requested review from LittleHuba, bemerybmw, crimson11, hoe-jo and limdor as code owners June 15, 2026 11:50

castler enabled auto-merge June 15, 2026 12:23

LittleHuba reviewed Jun 16, 2026

View reviewed changes

anmittag assigned castler Jun 17, 2026

anmittag added this to COM - Communication FT Jun 17, 2026

github-project-automation Bot moved this to Backlog in COM - Communication FT Jun 17, 2026

PiotrKorkus mentioned this pull request Jun 18, 2026

Coverage calculation for C++ and Rust eclipse-score/reference_integration#259

Open

castler force-pushed the js_use_llvm_cov_for_coverage_and_justification branch from ac55cc5 to 0b875d2 Compare June 18, 2026 13:01

LittleHuba reviewed Jun 18, 2026

View reviewed changes

castler and others added 3 commits June 18, 2026 16:01

LittleHuba approved these changes Jun 18, 2026

View reviewed changes

castler added this pull request to the merge queue Jun 18, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 18, 2026

castler added this pull request to the merge queue Jun 18, 2026

Merged via the queue into eclipse-score:main with commit b66f303 Jun 18, 2026
9 checks passed

castler deleted the js_use_llvm_cov_for_coverage_and_justification branch June 18, 2026 15:09

github-project-automation Bot moved this from Backlog to Done in COM - Communication FT Jun 18, 2026

4og mentioned this pull request Jun 19, 2026

Evaluate llvm-cov coverage solution in baselibs eclipse-score/baselibs#289

Open

4 tasks

olivembo mentioned this pull request Jun 25, 2026

Surface untested source files at 0% coverage in reports eclipse-score/score_cpp_policies#9

Open


		## Requirements

		The coverage pipeline was built to satisfy the following requirements:

	cxxfilt = llvm_bin_path.parent / "llvm-cxxfilt"
	cxxfilt = r.Rlocation("llvm_toolchain/llvm-cxxfilt")

Uh oh!

Conversation

castler commented Jun 15, 2026

Uh oh!

LittleHuba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LittleHuba left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

olivembo commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants