feat: Add code coverage delta to PR's by MaximilianSoerenPollak · Pull Request #927 · eclipse-score/docs-as-code

MaximilianSoerenPollak · 2026-10-07T23:48:21Z

📌 Description

This brings in the ability to measure complete code coverage in DaC and display the code coverage as a comment in the PR's themselves.

WIP:

Documentation still missing and final approval of how the comments etc look like.
Also I think some defensive programming & error handling is missing (what if there is no coverage on main etc.)
Also to think about, somehow pull this out into a reusable workflow and put it into cicd? Or is this too specific to DaC?

Currently this is how they look like:

Can be seen here

🚨 Impact Analysis

This change does not violate any tool requirements and is covered by existing tool requirements
This change does not violate any design decisions
Otherwise I have created a ticket for new tool qualification

✅ Checklist

Added/updated documentation for new or changed features
Added/updated tests to cover the changes
Followed project coding standards and guidelines

WIP: Add code coverage to DaC

Signed-off-by: Maximilian Sören Pollak <maximilian.pollak@qorix.com>

WIP:testing

AlexanderLanin · 2026-10-08T13:04:45Z

+          REPO: ${{ github.repository }}
+          HEAD_SHA: ${{ github.event.workflow_run.head_sha }}
+        run: |
+          pr=$(cat coverage-report/pr_number)


this should not rely on what a PR has said what it is. See docs-publish workflow

Okay that is a good catch.

Will address that once I take a look at trying to fix the code.

Copilot

🟡 Changes recommended

Missing-artifact handling can prevent reports, and malformed reference data can still crash rendering.

2 open findings

Create coverage output directory before writing report · New
Validate reference column types and coverage invariants · New

What changed in this PR

Adds merged Python coverage reporting and PR comments, including deltas against main.

Changes:

Adds LCOV merging, summaries, comparison logic, and tests.
Instruments Bazel and docs scenarios for coverage.
Adds workflows to collect, publish, and comment coverage results.

File	Description
`.github/workflows/_test.yml`	Collects and merges coverage artifacts.
`.github/workflows/coverage_comment.yml`	Posts coverage summaries on PRs.
`.gitignore`	Ignores generated coverage output.
`bzl/needs_rules.bzl`	Forwards coverage settings to Sphinx actions.
`pyproject.toml`	Configures coverage.py.
`src/docs_cli/BUILD`	Defines coverage build settings.
`src/docs_cli/cli.py`	Starts coverage before application imports.
`src/requirements.in`	Adds coverage.py dependency.
`src/requirements.txt`	Locks coverage.py for Python 3.12.
`src/requirements_py314.txt`	Locks coverage.py for Python 3.14.
`src/tests/docs_bzl/README.md`	Documents scenario coverage usage.
`src/tests/docs_bzl/helpers.py`	Propagates coverage configuration through Bazel.
`tools/BUILD`	Defines the coverage-report binary.
`tools/coverage_report.py`	Implements merging, comparison, and Markdown output.
`tools/tests/BUILD`	Registers coverage-report tests.
`tools/tests/coverage_report_test.py`	Tests coverage processing and reporting.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        run: >-
+          bazel run --lockfile_mode=error //tools:coverage_report --
+          "Unit tests=coverage-data/unit_tests/unit_tests.lcov"
+          "docs.bzl scenarios=coverage-data/docs_bzl"
+          --output coverage-data/coverage.lcov
+          --markdown coverage-data/summary.md
+          --json coverage-data/coverage.json
+          ${{ github.event_name == 'pull_request' && '--compare-with coverage-data/main/coverage.json' || '' }}


MaximilianSoerenPollak · 2026-10-08T19:50:46Z

+        lines_hit, lines_total = data["lines"]
+        branches_hit, branches_total = data["branches"]
+        return cls(lines_hit, lines_total, branches_hit, branches_total)


Can this actually ever occur?
I guess adding some defenses here is not a bad idea.
I will add this.

github-actions · 2026-10-08T19:28:51Z

Documentation preview for this pull request is available at:
pr-927: https://eclipse-score.github.io/docs-as-code/pr-927/

AlexanderLanin · 2026-10-08T19:24:44Z

+        "_coverage_file": attr.label(default = Label("//src/docs_cli:coverage_file")),
+        "_coverage_process_start": attr.label(
+            default = Label("//src/docs_cli:coverage_process_start"),
+        ),


use internal_target for these?

You mean the __internal__ prefix?

AlexanderLanin · 2026-10-08T19:27:21Z

+        # An absolute path in the checkout is readable from every sandbox and
+        # runfiles tree the Sphinx runs start in.


not with remote execution

That is true, but I think once we have remote execution a lot of things will fail.

Though I will adapt the comments

AlexanderLanin · 2026-10-08T19:28:18Z

    """Run Bazel, optionally overriding the environment of the subprocess."""
    start_time = time.time()
-    cmd = ["bazel", *args]
+    coverage_env = _coverage_env()


not sure, but something like: env |= _coverage_env()

Oh, you mean env = _coverage_env() | env?
Though this would then not add them but instead choose one or the other which is not quiet what we want.

Hmm but I think I kind of get what you mean, let me see if I can integrate this.

AlexanderLanin · 2026-10-08T19:29:48Z

+
+# Measure what the docs.bzl scenario tests exercise inside Sphinx, and merge
+# coverage reports in //tools:coverage_report.
+coverage


latest after this PR we need to discuss splitting requirements.in for internal and public usage

I think we for sure need to do this.

AlexanderLanin · 2026-10-08T19:32:33Z

+#
+# SPDX-License-Identifier: Apache-2.0
+# *******************************************************************************
+"""Merge Python coverage reports and add every tracked file no test imports.


extract to tools? actually this is exact same thing as coverage_tool does... but generalizing coverage_tool is 10x effort.

Yes, I saw that tools has a similar script-ish already, and I think it might makes sense in the future to bring them together.
But I think for that coverage_tool needs to get into a better state first so we can see if maybe that script can be generalized and used here.

MaximilianSoerenPollak

Questions answered, and new ones raised.

MaximilianSoerenPollak · 2026-10-08T19:15:08Z

@@ -0,0 +1,980 @@
+# *******************************************************************************


This file is quiet large due to the execesive comments that explain inputs & outputs as well as other things.

I'm unsure if this is good or bad.

MaximilianSoerenPollak · 2026-10-08T19:39:39Z

+        "_coverage_file": attr.label(default = Label("//src/docs_cli:coverage_file")),
+        "_coverage_process_start": attr.label(
+            default = Label("//src/docs_cli:coverage_process_start"),
+        ),


You mean the __internal__ prefix?

MaximilianSoerenPollak · 2026-10-08T19:40:27Z

+        # An absolute path in the checkout is readable from every sandbox and
+        # runfiles tree the Sphinx runs start in.


That is true, but I think once we have remote execution a lot of things will fail.

Though I will adapt the comments

MaximilianSoerenPollak · 2026-10-08T19:47:50Z

    """Run Bazel, optionally overriding the environment of the subprocess."""
    start_time = time.time()
-    cmd = ["bazel", *args]
+    coverage_env = _coverage_env()


Oh, you mean env = _coverage_env() | env?
Though this would then not add them but instead choose one or the other which is not quiet what we want.

Hmm but I think I kind of get what you mean, let me see if I can integrate this.

MaximilianSoerenPollak · 2026-10-08T19:48:05Z

+
+# Measure what the docs.bzl scenario tests exercise inside Sphinx, and merge
+# coverage reports in //tools:coverage_report.
+coverage


I think we for sure need to do this.

MaximilianSoerenPollak · 2026-10-08T19:49:40Z

+#
+# SPDX-License-Identifier: Apache-2.0
+# *******************************************************************************
+"""Merge Python coverage reports and add every tracked file no test imports.


Yes, I saw that tools has a similar script-ish already, and I think it might makes sense in the future to bring them together.
But I think for that coverage_tool needs to get into a better state first so we can see if maybe that script can be generalized and used here.

MaximilianSoerenPollak · 2026-10-08T19:50:46Z

+        lines_hit, lines_total = data["lines"]
+        branches_hit, branches_total = data["branches"]
+        return cls(lines_hit, lines_total, branches_hit, branches_total)


Can this actually ever occur?
I guess adding some defenses here is not a bad idea.
I will add this.

- Make new targets internal - Add more defensive measures

MaximilianSoerenPollak added 6 commits October 7, 2026 15:44

WIP: Add code coverage to DaC

5d22222

Merge pull request #16 from MaximilianSoerenPollak/MSP_add_coverage

c9432cb

WIP: Add code coverage to DaC

Update Python version from 3.14 to 3.12 in workflow

c23b658

Signed-off-by: Maximilian Sören Pollak <maximilian.pollak@qorix.com>

WIP:testing

87022e6

Merge branch 'main' into MSP_add_cov_2

7d671b4

Signed-off-by: Maximilian Sören Pollak <maximilian.pollak@qorix.com>

Merge pull request #18 from MaximilianSoerenPollak/MSP_add_cov_2

56043c4

WIP:testing

github-project-automation Bot added this to INF - Infrastructure Community Oct 7, 2026

MaximilianSoerenPollak mentioned this pull request Oct 7, 2026

WIP: Add code coverage to DaC #924

Closed

6 tasks

AlexanderLanin reviewed Oct 8, 2026

View reviewed changes

MaximilianSoerenPollak added 5 commits October 8, 2026 16:22

WIP: Add some defensive programming

9576fa4

feat: Allow for multiple forks of the same owner

9aae7b7

fix: Remove regex

cd57f9c

chore: Add tests

da17af2

feat: Adapt dataclasses & delete non needed functions

f020f00

MaximilianSoerenPollak requested a balanced review from Copilot October 8, 2026 19:16

Copilot started reviewing on behalf of MaximilianSoerenPollak October 8, 2026 19:16 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

AlexanderLanin reviewed Oct 8, 2026

View reviewed changes

MaximilianSoerenPollak commented Oct 8, 2026

View reviewed changes

fix: Incooperate Review Feedback

064d260

- Make new targets internal - Add more defensive measures

MaximilianSoerenPollak marked this pull request as ready for review October 9, 2026 09:00

MaximilianSoerenPollak requested review from a-zw, dcalavrezo-qorix and nradakovic as code owners October 9, 2026 09:00

		# An absolute path in the checkout is readable from every sandbox and
		# runfiles tree the Sphinx runs start in.

		@@ -0,0 +1,980 @@
		# *******************************************************************************

Conversation

MaximilianSoerenPollak commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 Description

🚨 Impact Analysis

✅ Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 8, 2026

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!

MaximilianSoerenPollak 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MaximilianSoerenPollak commented Oct 7, 2026 •

edited

Loading