feat: 汎用カバレッジレポート PR コメント投稿の reusable workflow 追加 - #496
Conversation
jacoco (Android/JVM) と jest (JS/TS) 形式をサポートする workflow_call ベースの reusable workflow を新規作成。 BATS テスト 11 件を追加。 Closes #495 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
coverage-report.yml の説明、使用例（jest/jacoco）、 トラブルシューティングを追加。Coverage Format セクションを reusable workflow への移行案内に更新。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
coverage-report ジョブの 80 行のハードコード（Node setup + npm ci + npm test + github-script）を coverage-report.yml reusable workflow の 10 行の呼び出しに簡素化。quality-checks に artifact upload を追加。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughA reusable GitHub Actions workflow is introduced to post coverage reports to PRs, supporting JaCoCo and Jest formats. The unified-ci template is refactored to invoke this external workflow instead of handling coverage reporting inline, with comprehensive documentation and integration tests added. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as unified-ci.yml
participant WF as coverage-report.yml
participant Artifact as Artifact Storage
participant Action as JaCoCo/GitHub Script
participant PR as PR Comments
CI->>CI: Quality Checks Job<br/>(generate coverage report)
CI->>Artifact: Upload coverage artifact
CI->>WF: Invoke reusable workflow<br/>(format, report-path)
alt format == 'jacoco'
WF->>Artifact: Download coverage artifact
WF->>Action: Call madrapps/jacoco-report
Action->>PR: Post JaCoCo report
else format == 'jest'
WF->>Artifact: Download coverage artifact
WF->>Action: Parse coverage-summary.json<br/>via GitHub Script
Action->>PR: Find existing comment by title
alt Comment exists
Action->>PR: Update existing comment
else Comment not found
Action->>PR: Create new comment
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
🔍 AI Code Review (Local Hook)Models: Codex (default) / Gemini (default) 🤖 Codex Review指摘事項
総合判定
|
PR レビュー全体として、80行のハードコードを reusable workflow に抽出するリファクタリングの方向性は正しく、DRY原則に沿っています。BATS テストの追加も適切です。いくつか改善点を挙げます。 🐛 バグ / 設計上の問題1.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/coverage-report.yml (1)
52-81: No feedback when an unsupportedformatvalue is provided.If a caller passes
format: cobertura(or a typo), both jobs are skipped via theirifconditions and the workflow completes successfully with no output or warning. This silent failure makes misconfiguration hard to diagnose.Consider adding a validation job that fails explicitly on unsupported format values.
Example: add a validation job
validate-inputs: name: Validate Inputs runs-on: ubuntu-latest steps: - name: Validate format if: inputs.format != 'jacoco' && inputs.format != 'jest' run: | echo "::error::Unsupported coverage format '${{ inputs.format }}'. Supported formats: jacoco, jest" exit 1Then add
needs: validate-inputsto both report jobs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/coverage-report.yml around lines 52 - 81, Add a new validation job named validate-inputs that runs before report-jacoco and report-jest and fails with an explicit error when inputs.format is not 'jacoco' or 'jest' (use a step with an if: inputs.format != 'jacoco' && inputs.format != 'jest' to echo an error and exit 1); then add needs: validate-inputs to both report-jacoco and report-jest so they won’t run when validation fails, ensuring unsupported or misspelled format values produce a clear failing message..github/workflows/templates/README.md (1)
56-101: Documentation accurately reflects the workflow design.The usage examples, supported formats table, and feature list align well with the actual
coverage-report.ymlimplementation. One minor note: the documentation doesn't mention thatmin-coverage-overallandmin-coverage-changed-filesthresholds are currently only enforced for thejacocoformat (notjest). Consider adding a note about this limitation to avoid user confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/templates/README.md around lines 56 - 101, Update the README entry for the reusable workflow to explicitly state that the input parameters min-coverage-overall and min-coverage-changed-files are only enforced for the jacoco format and are not applied when format: jest; mention this limitation near the "Supported Formats" table or the "Features" list and reference the coverage-report.yml inputs (min-coverage-overall, min-coverage-changed-files) and the format names (jacoco, jest) so users know the thresholds currently only work with jacoco.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/coverage-report.yml:
- Around line 116-128: The loop over coverage entries assumes
metrics.statements.pct, metrics.branches.pct, metrics.functions.pct, and
metrics.lines.pct always exist; modify the block that builds commentBody (where
packageName and metrics are used) to defensively handle missing shapes by either
skipping entries that don't have the expected metric objects or using safe
access with defaults (e.g., optional chaining metrics?.statements?.pct ?? 'N/A'
or a numeric 0) before interpolating; ensure you check metrics itself and each
sub-property (statements, branches, functions, lines) and update the table rows
accordingly so the code never throws when a property is missing.
- Around line 32-41: The workflow defines inputs min-coverage-overall and
min-coverage-changed-files but the Jest job never reads/enforces them; either
document this limitation or implement enforcement by adding a Jest
coverage-check step that reads those inputs and fails the job when thresholds
are not met. Concretely, update the Jest job to pass the inputs
(min-coverage-overall, min-coverage-changed-files) into the run context and add
a step (e.g., a new step named check-jest-coverage or a script in package.json
like scripts/check-jest-coverage) that parses Jest's coverage-summary
(coverage/coverage-summary.json), compares overall and changed-files coverage
against the input thresholds, and exits non-zero if any threshold is violated;
ensure the step name and input variable names match min-coverage-overall and
min-coverage-changed-files so users get expected enforcement.
- Line 111: Wrap the JSON.parse(fs.readFileSync(coveragePath, 'utf8')) call in a
try-catch to handle malformed JSON: read the file into a variable (using
fs.readFileSync(coveragePath, 'utf8')), then try to parse it into coverage with
JSON.parse; on error catch the exception and throw or log a clear, actionable
message that includes coveragePath and the original error so the step fails with
an informative message instead of an unhandled stack trace (referencing the
symbols coverage, coveragePath, fs.readFileSync, and JSON.parse).
- Around line 131-140: Replace the single-page call to
github.rest.issues.listComments with a paginated listing (use
github.paginate(github.rest.issues.listComments, {...}) or call with per_page
and iterate) so you reliably retrieve all comments before searching for an
existing bot comment; when locating the comment, stop using
comment.body.includes(title) and instead search for a unique hidden marker
(e.g., "<!-- coverage-report-marker -->") embedded in the comment body (update
where you construct the coverage comment to include that marker) and use that
marker to set botComment. Ensure references to github.rest.issues.listComments,
github.paginate, botComment, and title are updated consistently.
---
Nitpick comments:
In @.github/workflows/coverage-report.yml:
- Around line 52-81: Add a new validation job named validate-inputs that runs
before report-jacoco and report-jest and fails with an explicit error when
inputs.format is not 'jacoco' or 'jest' (use a step with an if: inputs.format !=
'jacoco' && inputs.format != 'jest' to echo an error and exit 1); then add
needs: validate-inputs to both report-jacoco and report-jest so they won’t run
when validation fails, ensuring unsupported or misspelled format values produce
a clear failing message.
In @.github/workflows/templates/README.md:
- Around line 56-101: Update the README entry for the reusable workflow to
explicitly state that the input parameters min-coverage-overall and
min-coverage-changed-files are only enforced for the jacoco format and are not
applied when format: jest; mention this limitation near the "Supported Formats"
table or the "Features" list and reference the coverage-report.yml inputs
(min-coverage-overall, min-coverage-changed-files) and the format names (jacoco,
jest) so users know the thresholds currently only work with jacoco.
| min-coverage-overall: | ||
| description: 'Minimum overall coverage threshold (%)' | ||
| required: false | ||
| type: number | ||
| default: 0 | ||
| min-coverage-changed-files: | ||
| description: 'Minimum changed files coverage threshold (%)' | ||
| required: false | ||
| type: number | ||
| default: 0 |
There was a problem hiding this comment.
Jest format silently ignores min-coverage-overall and min-coverage-changed-files inputs.
The threshold inputs are defined in the workflow interface (Lines 32-41) and wired into the JaCoCo job, but the Jest job never reads or enforces them. Users setting these thresholds for Jest will get no feedback that the thresholds are being ignored.
At minimum, document this limitation. Ideally, implement threshold checking in the Jest script to fail the step when coverage falls below the configured minimum.
Also applies to: 78-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/coverage-report.yml around lines 32 - 41, The workflow
defines inputs min-coverage-overall and min-coverage-changed-files but the Jest
job never reads/enforces them; either document this limitation or implement
enforcement by adding a Jest coverage-check step that reads those inputs and
fails the job when thresholds are not met. Concretely, update the Jest job to
pass the inputs (min-coverage-overall, min-coverage-changed-files) into the run
context and add a step (e.g., a new step named check-jest-coverage or a script
in package.json like scripts/check-jest-coverage) that parses Jest's
coverage-summary (coverage/coverage-summary.json), compares overall and
changed-files coverage against the input thresholds, and exits non-zero if any
threshold is violated; ensure the step name and input variable names match
min-coverage-overall and min-coverage-changed-files so users get expected
enforcement.
| return; | ||
| } | ||
|
|
||
| const coverage = JSON.parse(fs.readFileSync(coveragePath, 'utf8')); |
There was a problem hiding this comment.
Missing error handling for malformed JSON.
If coverage-summary.json exists but contains invalid JSON, JSON.parse will throw an unhandled error and the step will fail with an unhelpful stack trace. Wrap in a try-catch with a clear error message.
Proposed fix
- const coverage = JSON.parse(fs.readFileSync(coveragePath, 'utf8'));
+ let coverage;
+ try {
+ coverage = JSON.parse(fs.readFileSync(coveragePath, 'utf8'));
+ } catch (e) {
+ core.setFailed(`Failed to parse coverage file at ${coveragePath}: ${e.message}`);
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const coverage = JSON.parse(fs.readFileSync(coveragePath, 'utf8')); | |
| let coverage; | |
| try { | |
| coverage = JSON.parse(fs.readFileSync(coveragePath, 'utf8')); | |
| } catch (e) { | |
| core.setFailed(`Failed to parse coverage file at ${coveragePath}: ${e.message}`); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/coverage-report.yml at line 111, Wrap the
JSON.parse(fs.readFileSync(coveragePath, 'utf8')) call in a try-catch to handle
malformed JSON: read the file into a variable (using
fs.readFileSync(coveragePath, 'utf8')), then try to parse it into coverage with
JSON.parse; on error catch the exception and throw or log a clear, actionable
message that includes coveragePath and the original error so the step fails with
an informative message instead of an unhandled stack trace (referencing the
symbols coverage, coveragePath, fs.readFileSync, and JSON.parse).
| for (const [packageName, metrics] of Object.entries(coverage)) { | ||
| if (packageName === 'total') { | ||
| commentBody += '### Overall Coverage\n\n'; | ||
| } else { | ||
| commentBody += `### ${packageName}\n\n`; | ||
| } | ||
|
|
||
| commentBody += '| Metric | Coverage |\n'; | ||
| commentBody += '|--------|----------|\n'; | ||
| commentBody += `| Statements | ${metrics.statements.pct}% |\n`; | ||
| commentBody += `| Branches | ${metrics.branches.pct}% |\n`; | ||
| commentBody += `| Functions | ${metrics.functions.pct}% |\n`; | ||
| commentBody += `| Lines | ${metrics.lines.pct}% |\n\n`; |
There was a problem hiding this comment.
No null-safety when accessing metric properties.
The code assumes every entry in the coverage JSON has statements.pct, branches.pct, functions.pct, and lines.pct. If a package entry has a different shape (e.g., missing branches), this will throw at runtime. Consider adding defensive access or skipping entries that don't match the expected schema.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/coverage-report.yml around lines 116 - 128, The loop over
coverage entries assumes metrics.statements.pct, metrics.branches.pct,
metrics.functions.pct, and metrics.lines.pct always exist; modify the block that
builds commentBody (where packageName and metrics are used) to defensively
handle missing shapes by either skipping entries that don't have the expected
metric objects or using safe access with defaults (e.g., optional chaining
metrics?.statements?.pct ?? 'N/A' or a numeric 0) before interpolating; ensure
you check metrics itself and each sub-property (statements, branches, functions,
lines) and update the table rows accordingly so the code never throws when a
property is missing.
| // Find existing coverage comment by title | ||
| const { data: comments } = await github.rest.issues.listComments({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.issue.number, | ||
| }); | ||
|
|
||
| const botComment = comments.find(comment => | ||
| comment.user.type === 'Bot' && comment.body.includes(title) | ||
| ); |
There was a problem hiding this comment.
Bug: listComments is paginated — existing bot comment may not be found.
github.rest.issues.listComments returns at most 30 results per page by default. On PRs with more than 30 comments, the bot comment may not be found, resulting in duplicate coverage comments instead of updating the existing one.
Use github.paginate or set per_page: 100 and paginate manually.
Also, matching by comment.body.includes(title) is fragile — a user-supplied title like "Coverage" could match unrelated comments. The standard pattern is to embed a hidden HTML marker (e.g., <!-- coverage-report-marker -->) in the comment body and search for that.
Proposed fix
+ const marker = `<!-- coverage-report: ${title} -->`;
+ commentBody = marker + '\n' + commentBody;
+
// Find existing coverage comment by title
- const { data: comments } = await github.rest.issues.listComments({
- owner: context.repo.owner,
- repo: context.repo.repo,
- issue_number: context.issue.number,
- });
-
- const botComment = comments.find(comment =>
- comment.user.type === 'Bot' && comment.body.includes(title)
- );
+ const comments = await github.paginate(github.rest.issues.listComments, {
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: context.issue.number,
+ });
+
+ const botComment = comments.find(comment =>
+ comment.user.type === 'Bot' && comment.body.includes(marker)
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/coverage-report.yml around lines 131 - 140, Replace the
single-page call to github.rest.issues.listComments with a paginated listing
(use github.paginate(github.rest.issues.listComments, {...}) or call with
per_page and iterate) so you reliably retrieve all comments before searching for
an existing bot comment; when locating the comment, stop using
comment.body.includes(title) and instead search for a unique hidden marker
(e.g., "<!-- coverage-report-marker -->") embedded in the comment body (update
where you construct the coverage comment to include that marker) and use that
marker to set botComment. Ensure references to github.rest.issues.listComments,
github.paginate, botComment, and title are updated consistently.
Phase 2 として cobertura (.NET/Python/Go) と lcov (Istanbul/nyc/c8) の 2形式を coverage-report reusable workflow に追加。 - irongut/CodeCoverageSummary@v1.3.0 + marocchino/sticky-pull-request-comment@v2.9.4 (cobertura) - romeovs/lcov-reporter-action@v0.4.0 (lcov) - BATS テスト 4件追加（計15件） - テンプレート README に使用例を追記 Closes #495 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Claude Code レビュー全体的な設計（reusable workflow への抽象化、artifact ベースのレポート転送）は良好です。ただし、いくつか対処すべき問題を見つけました。 🔴 Major1. Jestジョブでのカバレッジ閾値が機能しない（CodeRabbit指摘と同様）
修正案（ // 閾値チェック
const minOverall = parseFloat(process.env.MIN_COVERAGE_OVERALL || '0');
if (minOverall > 0 && coverage.total) {
const linesPct = coverage.total.lines.pct;
if (linesPct < minOverall) {
core.setFailed(`Line coverage ${linesPct}% is below minimum ${minOverall}%`);
return;
}
}環境変数への追加も必要: env:
REPORT_PATH: ${{ inputs.report-path }}
COMMENT_TITLE: ${{ inputs.title }}
MIN_COVERAGE_OVERALL: ${{ inputs.min-coverage-overall }}
MIN_COVERAGE_CHANGED_FILES: ${{ inputs.min-coverage-changed-files }}2. コメント検索でページネーション未対応（CodeRabbit指摘と同様）
// 現状（問題あり）
const { data: comments } = await github.rest.issues.listComments({ ... });
// 修正案
const comments = await github.paginate(github.rest.issues.listComments, {
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
});また、タイトルによるマッチング（ const marker = `<!-- coverage-report:${title} -->`;
commentBody = `${marker}\n${commentBody}`;
// 検索時: comment.body.includes(marker)🟡 Minor3.
|
|
🎉 This PR is included in version 1.81.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
coverage-report.yml) を新規追加jacoco,jest,cobertura,lcovunified-ci.ymlテンプレートのハードコードされたカバレッジジョブを reusable workflow 呼び出しに置換Supported Formats
jacocomadrapps/jacoco-report@v1.7.2jestactions/github-script@v8coberturairongut/CodeCoverageSummary@v1.3.0lcovromeovs/lcov-reporter-action@v0.4.0Test plan
Closes #495
🤖 Generated with Claude Code