Skip to content

feat: opt-in coverage comment in the CI pipeline#140

Merged
yonatankarp merged 3 commits into
mainfrom
feat/jvm-coverage-comment
Jun 20, 2026
Merged

feat: opt-in coverage comment in the CI pipeline#140
yonatankarp merged 3 commits into
mainfrom
feat/jvm-coverage-comment

Conversation

@yonatankarp

@yonatankarp yonatankarp commented Jun 20, 2026

Copy link
Copy Markdown
Owner

Why

The reusable ci.yml build workflow publishes JUnit results and uploads coverage HTML as an artifact, but never surfaces coverage on the pull request itself. Repos that hand-roll their pipeline (e.g. agent-desk) post a coverage comment themselves; repos that use this wrapper get nothing.

What

  • New composite action jvm-coverage: parses JaCoCo-format coverage XML, builds a per-module + total Markdown table, and posts/updates one idempotent PR comment (matched by a hidden marker) via actions/github-script. JaCoCo and Kotlinx Kover both emit JaCoCo-format XML, so the same action serves either tool.
  • Wired into ci.yml behind a new coverage input (default false — existing consumers are unaffected) plus a coverage-xml glob input. The step is gated to same-repo pull requests, since fork PRs receive a read-only token.

Enabling it (consumer side)

uses: yonatankarp/github-actions/.github/workflows/ci.yml@v2
with:
  coverage: true
  # coverage-xml defaults to JaCoCo's path; override for Kover, e.g.:
  # coverage-xml: '**/build/reports/kover/report.xml'

The consumer's build must produce the XML (e.g. JaCoCo xml.required = true + a report task wired into the build).

Verified

The summary script was run against a real multi-module JaCoCo set (beat-the-machine: domain/application/adapters) and produced the expected per-module + total table. YAML validated.

Release

After merge, move the v2 tag (sh ./bin/replace-tags.sh v2) so consumers pick it up. beat-the-machine will be the first consumer (coverage: true).

Summary by CodeRabbit

  • New Features
    • Added automatic coverage summary comments to pull requests
    • Coverage reports are now configurable through workflow inputs to specify report file paths
    • Coverage comment functionality can be enabled or disabled per workflow execution

The reusable build workflow published JUnit results and uploaded coverage
HTML as an artifact, but never surfaced coverage on the pull request. Add a
`jvm-coverage` composite action that summarises JaCoCo-format XML reports
(emitted by both JaCoCo and Kotlinx Kover) into one Markdown table and
posts/updates a single idempotent PR comment via github-script.

Wire it into ci.yml behind a new `coverage` input (default false, so existing
consumers are unaffected) and a `coverage-xml` glob input. The comment is
gated to same-repo pull requests, since fork PRs receive a read-only token.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds two workflow_call inputs (coverage boolean and coverage-xml glob string) and two conditional CI steps to .github/workflows/ci.yml. The steps parse matching JaCoCo/Kover XML files into a Markdown coverage summary and post or update a marker-tagged PR comment.

Changes

JVM Coverage PR Comment Feature

Layer / File(s) Summary
Workflow inputs for coverage toggle and XML glob
.github/workflows/ci.yml
Adds coverage (boolean, default true) and coverage-xml (string glob, default **/build/reports/jacoco/test/jacocoTestReport.xml) under on.workflow_call.inputs.
Coverage report generation and PR comment posting
.github/workflows/ci.yml
Adds a step that globs and parses coverage XML files to produce jvm-coverage-summary.md with per-module and total line/branch percentages, and a subsequent step that searches existing bot-authored PR comments for the <!-- jvm-coverage-summary --> marker and creates or updates the comment. Both steps are gated on source-code changes, PR context from the same repository, and inputs.coverage being enabled.

Sequence Diagram(s)

sequenceDiagram
  participant Caller as Calling Workflow
  participant CI as CI Runner
  participant FS as File System (Artifacts)
  participant GH as GitHub API

  Caller->>CI: trigger with coverage=true, coverage-xml glob
  CI->>FS: glob matching XML paths
  FS-->>CI: JaCoCo/Kover XML files
  CI->>CI: parse <counter> elements per module
  CI->>FS: write jvm-coverage-summary.md
  CI->>GH: GET /repos/.../issues/.../comments
  GH-->>CI: list of PR comments
  alt existing bot comment with marker found
    CI->>GH: PATCH comment (update)
  else no existing comment
    CI->>GH: POST new comment
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A bunny checked the branches today,
Counted lines covered along the way.
XML parsed, a report in .md,
Posted to PR for all eyes to see.
Green percentages hop down the trail,
No missed line shall make coverage fail! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description covers purpose, implementation details, and usage instructions but does not follow the repository's required template structure with sections for Type of changes and Checklist. Restructure the description to include the required template sections: Purpose, Types of changes (with checkboxes), and Checklist. The detailed implementation information can be preserved within or alongside these sections.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: adding an opt-in coverage comment feature to the CI pipeline, though the description notes coverage is now enabled by default.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/jvm-coverage-comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Default `coverage` to true and make the action a no-op when no JaCoCo-format
XML matches the glob: it logs a notice and skips the comment instead of posting
an empty one. This makes coverage zero-config for repos that emit JaCoCo/Kover
XML and invisible for those that don't, so it is safe on by default. Repos can
still set `coverage: false` to opt out entirely.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yonatankarp

Copy link
Copy Markdown
Owner Author

Update (commit de433e5): the coverage input now defaults to true. The action is a no-op when no JaCoCo-format XML matches (logs a notice, posts nothing), so it's safe on by default — zero-config for repos that emit JaCoCo/Kover XML, invisible for those that don't. Set coverage: false to opt out. (The PR description above still says default false; couldn't edit it — token lacks read:org.)

A reusable workflow can only reference actions by a published ref, and a new
composite action does not exist on the v2 tag until release, so referencing
jvm-coverage@v2 from ci.yml broke the workflow (and its own self-test) with
"Can't find action.yml". Inline the coverage summary (python) and comment
(github-script) steps directly into the build job; inline run steps and the
published github-script action resolve for every consumer with no new tag.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Test Results (spring-skeleton)

1 tests   1 ✅  2s ⏱️
1 suites  0 💤
1 files    0 ❌

Results for commit 8c31767.

@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown

Test Results (spring-skeleton)

2 tests   2 ✅  2s ⏱️
2 suites  0 💤
2 files    0 ❌

Results for commit 8c31767.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown

Test Results (KTOR-SKELETON)

2 tests   2 ✅  3s ⏱️
2 suites  0 💤
2 files    0 ❌

Results for commit 8c31767.

♻️ This comment has been updated with latest results.

@yonatankarp yonatankarp marked this pull request as ready for review June 20, 2026 13:30

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 133-136: The ET.parse(path) call in the coverage XML parsing
section lacks error handling and will raise an exception if the XML is
malformed, causing the entire workflow to fail. Wrap the ET.parse(path)
statement and subsequent operations (including the attrib.get, counters call,
and line retrieval) in a try-except block that catches XML parsing errors. When
an exception occurs, log a warning message that includes the file path and the
actual error details, then skip processing that file by continuing to the next
iteration. This allows the workflow to gracefully degrade by warning about bad
files while continuing to process valid coverage files.
- Line 171: The actions/github-script action is using a mutable version tag
(`@v9`) which weakens supply-chain security. Replace the version tag `@v9` with the
full commit SHA 3a2844b7e9c422d3c10d287c895573f7108da1b3 in the uses field, and
add a comment on the same line (# v9.0.0) for readability to indicate which
version this commit corresponds to.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2f55a8a0-2272-48a6-a249-2ac98466b81c

📥 Commits

Reviewing files that changed from the base of the PR and between b3c54c1 and 8c31767.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml

Comment thread .github/workflows/ci.yml
Comment on lines +133 to +136
root = ET.parse(path).getroot()
name = root.attrib.get("name") or path
module = counters(root)
line = module.get("LINE", (0, 0))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle malformed coverage XML without failing the whole workflow.

A single malformed/misdetected XML causes ET.parse(path) to raise and fail the job. Since this feature is enabled by default and intended to be no-op-safe, this should degrade gracefully (warn + skip bad file), not hard-fail CI.

Suggested patch
           rows = []
           totals = {}
+          parsed_any = False
           for path in files:
-              root = ET.parse(path).getroot()
+              try:
+                  root = ET.parse(path).getroot()
+              except ET.ParseError:
+                  print(f"::warning file={path}::Invalid coverage XML; skipping.")
+                  continue
               name = root.attrib.get("name") or path
               module = counters(root)
               line = module.get("LINE", (0, 0))
               branch = module.get("BRANCH", (0, 0))
               rows.append(f"| {name} | {percentage(*line)} | {percentage(*branch)} |")
+              parsed_any = True
               for counter_type, (covered, missed) in module.items():
                   acc_covered, acc_missed = totals.get(counter_type, (0, 0))
                   totals[counter_type] = (acc_covered + covered, acc_missed + missed)
+
+          if not parsed_any:
+              print("::notice::No valid coverage XML parsed; skipping the coverage comment.")
+              raise SystemExit(0)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 133 - 136, The ET.parse(path) call in
the coverage XML parsing section lacks error handling and will raise an
exception if the XML is malformed, causing the entire workflow to fail. Wrap the
ET.parse(path) statement and subsequent operations (including the attrib.get,
counters call, and line retrieval) in a try-except block that catches XML
parsing errors. When an exception occurs, log a warning message that includes
the file path and the actual error details, then skip processing that file by
continuing to the next iteration. This allows the workflow to gracefully degrade
by warning about bad files while continuing to process valid coverage files.

Comment thread .github/workflows/ci.yml

- name: Comment coverage on the pull request
if: ${{ steps.coverage.outputs.has_reports == 'true' }}
uses: actions/github-script@v9

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify unpinned action refs in workflow files
rg -nP '^\s*uses:\s*[^@]+@[A-Za-z0-9._-]+$' .github/workflows

Repository: yonatankarp/github-actions

Length of output: 2086


🏁 Script executed:

sed -n '165,180p' .github/workflows/ci.yml

Repository: yonatankarp/github-actions

Length of output: 674


🏁 Script executed:

rg 'actions/github-script' .github/workflows/ci.yml -n

Repository: yonatankarp/github-actions

Length of output: 113


🌐 Web query:

GitHub Actions pinning commit SHA security best practice actions/github-script v9

💡 Result:

Pinning GitHub Actions to a full-length commit SHA is a security best practice that protects against supply chain attacks, such as malicious code being injected into an existing tag or branch [1][2][3]. Because tags and branches are mutable references that can be moved or force-pushed, a compromised action repository could silently inject malicious changes into your workflows [4][3]. A full commit SHA is immutable, ensuring that your workflow runs exactly the code you intend [5][2][6]. For actions/github-script v9, which was released in April 2026 [7][8], the recommended security approach is to pin to its specific full commit SHA [5][2]. The current SHA for version 9.0.0 is 3a2844b7e9c422d3c10d287c895573f7108da1b3 [7][8]. When pinning to a SHA, it is a standard practice to include the version tag as a comment for readability and to facilitate automated updates by tools like Dependabot or Renovate [2][9][3]: - uses: actions/github-script@3a2844b # v9.0.0 Key considerations for this practice include: 1. Enforcement: Many organizations now enforce SHA pinning via policy to prevent the use of mutable tags [10][5][2]. If your organization enables this, workflows using tags will fail [5][2]. 2. Maintenance: Using Dependabot or Renovate with appropriate presets (e.g., helpers:pinGitHubActionDigests) allows you to automate the process of updating these pinned SHAs, balancing security with ease of maintenance [2][4][11]. 3. Dependency Trees: Security policies that enforce SHA pinning may check the entire dependency tree of your actions, including internal sub-actions referenced by the action you are using [5]. 4. V9 Specifics: Note that actions/github-script v9 introduced breaking changes, including an upgrade to @actions/github v9 (which is ESM-only) and the requirement to use an injected getOctokit function instead of requiring @actions/github [12][7][13]. When updating to v9, ensure your scripts are compatible with these changes [12][13].

Citations:


🌐 Web query:

actions/github-script v9 commit SHA release

💡 Result:

The actions/github-script v9.0.0 release is associated with the commit SHA 3a2844b7e9c422d3c10d287c895573f7108da1b3 [1][2]. This release, published on April 9, 2026, upgraded the action to use @actions/github v9 [3][1]. Notable changes in v9 include: - The addition of a getOctokit factory function available directly in the script context [1][4]. - An upgrade to an ESM-only version of @actions/github, which means require('@actions/github') no longer works in scripts [1][5]. - The requirement to use the injected getOctokit function instead of importing it from @actions/github [1][6]. - The automatic addition of an orchestration ID to the user-agent string for request tracing [4][5].

Citations:


Pin actions/github-script to a full commit SHA.

Using @v9 is mutable and weakens supply-chain guarantees. Pin to the exact commit SHA 3a2844b7e9c422d3c10d287c895573f7108da1b3 (with # v9.0.0 comment for readability) to protect against tag manipulation and supply-chain attacks.

🧰 Tools
🪛 zizmor (1.25.2)

[error] 171-171: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml at line 171, The actions/github-script action is
using a mutable version tag (`@v9`) which weakens supply-chain security. Replace
the version tag `@v9` with the full commit SHA
3a2844b7e9c422d3c10d287c895573f7108da1b3 in the uses field, and add a comment on
the same line (# v9.0.0) for readability to indicate which version this commit
corresponds to.

Source: Linters/SAST tools

@yonatankarp yonatankarp merged commit c765288 into main Jun 20, 2026
19 checks passed
@yonatankarp yonatankarp deleted the feat/jvm-coverage-comment branch June 20, 2026 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant