ci: enable coverage reporting with artifact upload and PR summary#487
Merged
LakshanSS merged 1 commit intoopenchoreo:mainfrom Apr 2, 2026
Merged
ci: enable coverage reporting with artifact upload and PR summary#487LakshanSS merged 1 commit intoopenchoreo:mainfrom
LakshanSS merged 1 commit intoopenchoreo:mainfrom
Conversation
kaviththiranga
requested review from
jhivandb and
sameerajayasoma
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe workflow updates GitHub Actions: grants Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
- Switch from `yarn test` to `yarn test:all` (with --coverage) - Upload coverage/ as downloadable artifact (14-day retention) - Add coverage summary to GitHub Actions step summary on PRs - Add pull-requests: write permission for PR interactions Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/build-and-test.yml (2)
17-19: Consider scopingpull-requests: writeto the test job only, or removing if not needed.The
$GITHUB_STEP_SUMMARYfeature doesn't requirepull-requests: writepermission—it works with default workflow permissions. This permission is typically needed for posting PR comments, adding labels, or creating reviews.If you plan to add PR commenting in the future, scope this permission to the
testjob rather than workflow-level to follow least-privilege principles:test: name: Tests permissions: pull-requests: writeIf no PR commenting is planned, consider removing this permission entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-test.yml around lines 17 - 19, The workflow currently grants workspace-wide pull-requests: write permission which is unnecessary for $GITHUB_STEP_SUMMARY and violates least-privilege; remove the top-level pull-requests: write entry or move it into the test job only by adding a permissions block under the test job (job name "test") with pull-requests: write if you actually need PR write actions later; otherwise delete the pull-requests: write line from the workflow-level permissions to keep default minimal permissions.
103-120: Improve robustness of lcov parsing when coverage file is missing.When
coverage/lcov.infodoesn't exist, thegrepcommands will output errors to stderr before falling back to0. This clutters the logs. Additionally, this step won't run if tests fail (unlike the artifact upload which usesif: always()).♻️ Suggested improvements
- name: Coverage summary on PR - if: github.event_name == 'pull_request' + if: github.event_name == 'pull_request' && always() run: | echo "## Test Coverage Report" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY + if [ ! -f coverage/lcov.info ]; then + echo "No coverage data found." >> $GITHUB_STEP_SUMMARY + exit 0 + fi # Parse lcov.info for summary - LINES_FOUND=$(grep -c "^DA:" coverage/lcov.info || echo 0) - LINES_HIT=$(grep "^DA:" coverage/lcov.info | grep -cv ",0$" || echo 0) + LINES_FOUND=$(grep -c "^DA:" coverage/lcov.info 2>/dev/null || echo 0) + LINES_HIT=$(grep "^DA:" coverage/lcov.info 2>/dev/null | grep -cv ",0$" || echo 0) if [ "$LINES_FOUND" -gt 0 ]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-test.yml around lines 103 - 120, The "Coverage summary on PR" step should avoid noisy grep errors and run even when tests fail: first guard access to coverage/lcov.info (test [ -f coverage/lcov.info ]) before running grep so LINES_FOUND/LINES_HIT are only computed when the file exists (otherwise set them to 0), and silence grep stderr (or skip grep entirely when file missing) to prevent error output; also change the step condition from limiting to github.event_name == 'pull_request' to use if: always() (or combine with pull request check) so artifacts/summary are attempted even on failing tests. Use the existing step name "Coverage summary on PR" and variables LINES_FOUND, LINES_HIT, COVERAGE and the path coverage/lcov.info to locate where to apply these changes.
🤖 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/build-and-test.yml:
- Around line 17-19: The workflow currently grants workspace-wide pull-requests:
write permission which is unnecessary for $GITHUB_STEP_SUMMARY and violates
least-privilege; remove the top-level pull-requests: write entry or move it into
the test job only by adding a permissions block under the test job (job name
"test") with pull-requests: write if you actually need PR write actions later;
otherwise delete the pull-requests: write line from the workflow-level
permissions to keep default minimal permissions.
- Around line 103-120: The "Coverage summary on PR" step should avoid noisy grep
errors and run even when tests fail: first guard access to coverage/lcov.info
(test [ -f coverage/lcov.info ]) before running grep so LINES_FOUND/LINES_HIT
are only computed when the file exists (otherwise set them to 0), and silence
grep stderr (or skip grep entirely when file missing) to prevent error output;
also change the step condition from limiting to github.event_name ==
'pull_request' to use if: always() (or combine with pull request check) so
artifacts/summary are attempted even on failing tests. Use the existing step
name "Coverage summary on PR" and variables LINES_FOUND, LINES_HIT, COVERAGE and
the path coverage/lcov.info to locate where to apply these changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8f6a53f-d865-4ef3-b8ef-445befae9b72
📒 Files selected for processing (1)
.github/workflows/build-and-test.yml
LakshanSS
approved these changes
Apr 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
yarn testtoyarn test:all(with --coverage)Current Coverage: 19% line coverage across the entire codebase (22,539 lines found, 4,313 hit). That's our
baseline to improve from.
Summary by CodeRabbit