ci(coverage): harden diff-cover step and handle non-Java PRs

[bladehan1]

Summary

Follow-up to PR #23 addressing two review comments from CodeRabbit and cubic.

Fix A — Drop the unreachable exit 0 branch in coverage-gate

The PR_XMLS empty check (removed here) was already unreachable: the earlier collect-xml step exit 1s when JaCoCo reports are missing. Worse, if it had ever fired, exit 0 would have bypassed the changed_line_coverage output, and the downstream enforcement step would have failed with a misleading "Failed to parse changed-line coverage" message instead of a skip.

Fix B — Handle PRs with no changed Java source lines

When a PR touches only non-Java files (workflow, docs, .proto, .md, config), diff-cover produces a JSON report with total_num_lines: 0 and total_percent_covered: null. The previous parse with // empty would then exit 1, wrongly failing legitimate non-Java PRs.

This PR:

• Reads total_num_lines from diff-cover.json. When 0, emits the sentinel changed_line_coverage=NA.
• Teaches the enforcement step to treat NA as SKIPPED (passes the changed-line gate).
• Renders NA without a trailing % in the step summary.

The overall-delta gate (MAX_DROP=-0.1%) is unchanged.

Test plan

• This PR itself is workflow-only → exercises the NA / SKIPPED branch in the fresh workflow once merged.
• Confirm Coverage Gate step summary shows Changed-line Coverage: NA and Changed-line Gate: SKIPPED (no changed Java source lines).
• Overall-delta gate continues to enforce > -0.1%.

---

Summary by cubic

Hardens the PR coverage gate to avoid false failures and clearer output. Skips changed-line enforcement when there are no changed Java lines, and removes a dead path that caused misleading errors.

• Bug Fixes
  • Removed unreachable PR_XMLS check that could unset changed_line_coverage and trigger a parse error.
  • Parse total_num_lines from diff-cover.json; when 0, set changed_line_coverage=NA and mark the gate as SKIPPED.
  • Show NA without a trailing % in logs and the step summary. Overall-delta gate remains unchanged.

^{Written for commit ece6e09. Summary will update on new commits.}

Summary by CodeRabbit

• Chores
  • Improved pull request build validation to handle code coverage reporting edge cases more gracefully. The workflow now properly identifies cases where no source changes are detected, appropriately marking coverage checks as skipped rather than failed, and provides clearer status reporting across all scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pr-build.yml workflow now handles cases where no Java source lines are changed. When diff-cover.json reports zero total lines, the workflow sets changed-line coverage to "NA", skips the threshold check, and marks the gate as passed-irrelevant. The diff-cover step no longer exits early on missing PR Jacoco reports.

Changes

Cohort / File(s)	Summary
Workflow Coverage Gating Logic .github/workflows/pr-build.yml	Modified changed-line coverage gating to detect and handle zero changed lines (diff-cover.json.total_num_lines = 0), setting coverage to "NA" and skipping/flexibly passing the gate instead of failing. Removed early exit on missing PR Jacoco XML; adjusted reporting to display NA or formatted percentage in logs and step summary.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• ci(coverage): disable jacoco PR comments, relax overall delta threshold to 0.4% #22: Modifies the same pr-build.yml changed-line coverage and diff-cover logic for computing/enforcing coverage gates.
• test: validate changed-line coverage gate #23: Updates changed-line coverage logic in pr-build.yml to introduce diff-cover computation and enforcement that this PR now builds upon.

Suggested reviewers

• 317787106
• halibobo1205

Poem

🐰 When zero lines hop through the gate,
We smile and mark it "NA"—not late!
No threshold to fear, no false alarm,
Just graceful handling, safe from harm.
A coverage tale, so clean and bright! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately summarizes the main changes: hardening the diff-cover step and handling non-Java PRs, which directly aligns with the two key fixes (removing unreachable exit branch and handling PRs with no changed Java source lines).
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 fix/ci_coverage_post_review

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

.github/workflows/pr-build.yml (1)

300-323: Optional: clearer diagnostic when diff-cover.json is absent.

set +e wraps the diff-cover invocation and DIFF_RC is captured but only logged later (line 335). If diff-cover fails catastrophically and never writes diff-cover.json, the subsequent jq call will fail with a generic file-not-found error instead of a message that points at the real cause. Consider asserting the file exists before parsing so the failure surfaces clearly.

♻️ Suggested refinement

           DIFF_RC=$?
           set -e
 
+          if [ ! -s diff-cover.json ]; then
+            echo "diff-cover did not produce diff-cover.json (exit code: ${DIFF_RC})."
+            exit 1
+          fi
+
           TOTAL_NUM_LINES=$(jq -r '.total_num_lines // 0' diff-cover.json)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/pr-build.yml around lines 300 - 323, Check for
diff-cover.json after running diff-cover and before jq usage: if DIFF_RC is
non-zero or diff-cover.json does not exist/has zero size, print a clear
diagnostic including DIFF_RC and the diff-cover stderr/stdout (or a note that
diff-cover failed) and fail fast; otherwise continue to parse TOTAL_NUM_LINES
and CHANGED_LINE_COVERAGE as before. Specifically update the section around the
diff-cover invocation and checks for DIFF_RC/diff-cover.json so the script
verifies file existence (e.g., test -s "diff-cover.json") and logs "diff-cover
failed (exit $DIFF_RC)" with context before running jq, referencing DIFF_RC,
diff-cover.json, TOTAL_NUM_LINES and CHANGED_LINE_COVERAGE in the message.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.github/workflows/pr-build.yml:
- Around line 300-323: Check for diff-cover.json after running diff-cover and
before jq usage: if DIFF_RC is non-zero or diff-cover.json does not exist/has
zero size, print a clear diagnostic including DIFF_RC and the diff-cover
stderr/stdout (or a note that diff-cover failed) and fail fast; otherwise
continue to parse TOTAL_NUM_LINES and CHANGED_LINE_COVERAGE as before.
Specifically update the section around the diff-cover invocation and checks for
DIFF_RC/diff-cover.json so the script verifies file existence (e.g., test -s
"diff-cover.json") and logs "diff-cover failed (exit $DIFF_RC)" with context
before running jq, referencing DIFF_RC, diff-cover.json, TOTAL_NUM_LINES and
CHANGED_LINE_COVERAGE in the message.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6d239511-590b-4046-9bd6-165275e3c9a0

📥 Commits

Reviewing files that changed from the base of the PR and between cbfc520 and ece6e09.

📒 Files selected for processing (1)

• .github/workflows/pr-build.yml

[cubic-dev-ai]

No issues found across 1 file