[issues/568] Post CI summary comments on PR when jobs complete#570
Merged
Conversation
## Summary Adds PR comment posts after each CI job finishes, reporting elapsed duration, unit test counts, integration test passing counts, and the QA TC IDs exercised. Also hardens all test pipelines against exit-code swallowing and uploads test report artifacts for post-hoc debugging. Disambiguates two duplicate `Report:` lines in the test release runner. ## Changes - New composite action `post-ci-comment` builds a markdown table and posts it via `gh api` — always creates a new comment so each run appears chronologically - All test runner actions (`run-tests`, `run-integration-tests`, `run-integration-tests-with-extensions`) now: output test counts, use `set -o pipefail` + `tee` to prevent exit-code swallowing, and extract stats from captured output - Integration test actions now upload their captured output as 30-day artifacts - CI workflow uses `date +%s` for cross-step timing (not `$SECONDS`, which resets per shell), grants `pull-requests: write`, and wires the post-comment step with `continue-on-error: true` so API glitches don't fail the job - Renamed duplicate `Report:` lines in test-release-run.sh and validate-qa-coverage.sh to `Report output:` / `Report complete:` Documentation: CHANGELOG not needed (CI infrastructure, not user-facing); README not needed (no new commands/settings/features) ## Test Plan - [ ] All existing tests pass (1889/1889) - [ ] Formatting passes (prettier) - [ ] Manual testing: push branch and verify both CI jobs post PR comments with correct duration, test counts, and TC IDs ## Related - Closes #568
Contributor
WalkthroughThe PR implements automated PR comments that post CI job completion summaries by extending three test-running composite actions to capture and export test metrics (unit-passed, unit-total, int-passing), creating a new ChangesCI Summary Comments
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
CI / Integration Tests (with extensions) — run summary
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/actions/run-tests/action.yml (1)
37-51: 💤 Low valueConsider anchoring the "total" pattern to the "Tests:" prefix for robustness.
The pattern on line 44 (
'\d+(?= total)') will match any number before " total" in the entire file. Whiletail -1mitigates this by taking the last match (which should be Jest's final summary), the pattern could be more specific to avoid false positives if other output contains numbers and "total".♻️ More robust pattern
- UNIT_TOTAL=$(grep -oP '\d+(?= total)' /tmp/jest-out.txt | tail -1 || echo "0") + UNIT_TOTAL=$(grep -oP 'Tests:.*\K\d+(?= total)' /tmp/jest-out.txt | tail -1 || echo "0")This anchors the match to lines starting with "Tests:" to match the same summary line as the "passed" pattern.
🤖 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/actions/run-tests/action.yml around lines 37 - 51, The UNIT_TOTAL extraction is too broad and can match any number before " total"; update the grep used in the "Extract unit test stats" step (id: stats) to anchor the match to the "Tests:" summary line so it mirrors the UNIT_PASSED logic — specifically change the pattern used when setting UNIT_TOTAL (currently '\d+(?= total)') to a pattern that only matches numbers on the "Tests:" line (e.g., anchor to "Tests:" and then capture the number before "total") when reading /tmp/jest-out.txt.
🤖 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/actions/post-pr-ci-summary/action.yml:
- Line 43: The QA_YAML selection uses plain lexicographic sort which misorders
semantic versions; update the pipeline command that assigns QA_YAML (the find |
sort | tail -1 sequence) to use version-aware sorting by replacing sort with
sort -V so filenames like qa-test-cases-v1.10.0.yaml are ordered correctly and
the latest semantic version is chosen.
---
Nitpick comments:
In @.github/actions/run-tests/action.yml:
- Around line 37-51: The UNIT_TOTAL extraction is too broad and can match any
number before " total"; update the grep used in the "Extract unit test stats"
step (id: stats) to anchor the match to the "Tests:" summary line so it mirrors
the UNIT_PASSED logic — specifically change the pattern used when setting
UNIT_TOTAL (currently '\d+(?= total)') to a pattern that only matches numbers on
the "Tests:" line (e.g., anchor to "Tests:" and then capture the number before
"total") when reading /tmp/jest-out.txt.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 10454a43-585c-42b4-a9a0-11c2b1ee67e7
📒 Files selected for processing (7)
.github/actions/post-pr-ci-summary/action.yml.github/actions/run-integration-tests-with-extensions/action.yml.github/actions/run-integration-tests/action.yml.github/actions/run-tests/action.yml.github/workflows/ci.ymlpackages/rangelink-vscode-extension/scripts/test-release-run.shpackages/rangelink-vscode-extension/scripts/validate-qa-coverage.sh
CI / Test & Validate — run summary
|
Two fixes from CodeRabbit review on PR #570. - Replace plain `sort` with `sort -V` when selecting the latest QA YAML file so semantic versions (v1.10.0 > v1.9.0) order correctly - Anchor UNIT_TOTAL grep to the "Tests:" prefix (`Tests:.*\K\d+(?= total)`) to match the same summary line as the UNIT_PASSED pattern Benefits: - Prevents wrong QA YAML selection once versions reach 1.10+ - Grep patterns are now symmetric: both anchor to the "Tests:" summary line Ref: #570 (review)
CI / Integration Tests (with extensions) — run summary
|
CI / Test & Validate — run summary
|
…'t truncate digits The CodeRabbit-suggested pattern `Tests:.*\K\d+(?= total)` omitted a whitespace boundary before `\K`. Greedy `.*` consumes leading digits, leaving `\d+` with only the trailing digit — on `Tests: 1889 passed, 1889 total` it captured `9` instead of `1889`. Adding `\s` before `\K` (`Tests:.*\s\K\d+(?= total)`) forces backtracking to a space boundary, ensuring `\d+` captures the full number. Benefits: - UNIT_TOTAL now correctly captures the full number (1889, not 9) - Verified locally: pattern `Tests:.*\s\K\d+(?= total)` produces 12, 586, 1889 → tail -1 = 1889 Ref: #570 (comment)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/actions/run-tests/action.yml:
- Around line 43-44: The current pipelines that set UNIT_PASSED and UNIT_TOTAL
can return empty strings because the `|| echo "0"` applies to the last command
only (tail), not the whole pipeline; fix by applying the fallback to the entire
pipeline so an absent grep match yields "0" — for example, wrap the pipeline in
a command group so the `|| echo "0"` runs on pipeline failure (i.e., change the
assignments for UNIT_PASSED and UNIT_TOTAL that read /tmp/jest-out.txt to use a
grouped pipeline with `|| echo "0"`), or alternatively perform the command
substitution and then use shell parameter expansion to default empty values to
0; update both the UNIT_PASSED and UNIT_TOTAL assignments accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c47b2b5f-821c-471b-bece-43ee71ec4e81
📒 Files selected for processing (2)
.github/actions/post-pr-ci-summary/action.yml.github/actions/run-tests/action.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/post-pr-ci-summary/action.yml
Comment on lines
+43
to
+44
| UNIT_PASSED=$(grep -oP 'Tests:\s+\K\d+(?= passed)' /tmp/jest-out.txt | tail -1 || echo "0") | ||
| UNIT_TOTAL=$(grep -oP 'Tests:.*\s\K\d+(?= total)' /tmp/jest-out.txt | tail -1 || echo "0") |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current fallback behavior vs fixed behavior (no repo files needed).
set +e
CURRENT=$(grep -oP 'Tests:\s+\K\d+(?= passed)' /dev/null | tail -1 || echo "0")
echo "current='${CURRENT}' (expected often empty, not 0)"
FIXED=$(grep -oP 'Tests:\s+\K\d+(?= passed)' /dev/null | tail -1)
FIXED="${FIXED:-0}"
echo "fixed='${FIXED}' (expected 0)"Repository: couimet/rangeLink
Length of output: 125
🏁 Script executed:
cat -n .github/actions/run-tests/action.yml | sed -n '35,55p'Repository: couimet/rangeLink
Length of output: 853
Fallback-to-zero is ineffective when grep finds no match.
On Line 43 and Line 44, ... | tail -1 || echo "0" can yield empty output instead of 0 because the pipeline exits successfully via tail. When the Jest output file exists but lacks the expected pattern, these variables become empty strings, which propagate to the GITHUB_OUTPUT (lines 49–50) and can break downstream metrics.
Suggested fix
- UNIT_PASSED=$(grep -oP 'Tests:\s+\K\d+(?= passed)' /tmp/jest-out.txt | tail -1 || echo "0")
- UNIT_TOTAL=$(grep -oP 'Tests:.*\s\K\d+(?= total)' /tmp/jest-out.txt | tail -1 || echo "0")
+ UNIT_PASSED=$(grep -oP 'Tests:\s+\K\d+(?= passed)' /tmp/jest-out.txt | tail -1)
+ UNIT_TOTAL=$(grep -oP 'Tests:.*\s\K\d+(?= total)' /tmp/jest-out.txt | tail -1)
+ UNIT_PASSED="${UNIT_PASSED:-0}"
+ UNIT_TOTAL="${UNIT_TOTAL:-0}"📝 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.
Suggested change
| UNIT_PASSED=$(grep -oP 'Tests:\s+\K\d+(?= passed)' /tmp/jest-out.txt | tail -1 || echo "0") | |
| UNIT_TOTAL=$(grep -oP 'Tests:.*\s\K\d+(?= total)' /tmp/jest-out.txt | tail -1 || echo "0") | |
| UNIT_PASSED=$(grep -oP 'Tests:\s+\K\d+(?= passed)' /tmp/jest-out.txt | tail -1) | |
| UNIT_TOTAL=$(grep -oP 'Tests:.*\s\K\d+(?= total)' /tmp/jest-out.txt | tail -1) | |
| UNIT_PASSED="${UNIT_PASSED:-0}" | |
| UNIT_TOTAL="${UNIT_TOTAL:-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/actions/run-tests/action.yml around lines 43 - 44, The current
pipelines that set UNIT_PASSED and UNIT_TOTAL can return empty strings because
the `|| echo "0"` applies to the last command only (tail), not the whole
pipeline; fix by applying the fallback to the entire pipeline so an absent grep
match yields "0" — for example, wrap the pipeline in a command group so the `||
echo "0"` runs on pipeline failure (i.e., change the assignments for UNIT_PASSED
and UNIT_TOTAL that read /tmp/jest-out.txt to use a grouped pipeline with `||
echo "0"`), or alternatively perform the command substitution and then use shell
parameter expansion to default empty values to 0; update both the UNIT_PASSED
and UNIT_TOTAL assignments accordingly.
CI / Integration Tests (with extensions) — run summary
|
CI / Test & Validate — run summary
|
4 tasks
2 tasks
couimet
added a commit
that referenced
this pull request
May 19, 2026
* [issues/579] CI Summary Comment Improvements ## Summary Fixes six bugs in the PR CI summary action introduced in #570. Comments now show pass/fail at a glance, previous successful comments are minimized instead of piling up, missing test reports are surfaced explicitly, the extensions job explains its scope, and re-run blocks are suppressed when most tests fail. ## Changes - Pass/fail icon (S1): ✅ or ❌ prefix on the comment title, inferred from unit test pass rate and integration test failure count - Stale comment cleanup (S1): after a successful run posts, previous successful comments from the same job are minimized via GraphQL. Failed comments stay visible, preserving error history - Missing report detection (S2): when both integration test stats are 0/empty, the report file is checked for mocha output. If absent, a warning row is added - Extension job scope (S3): when label-filter is requires-extensions, a row explains that unit tests ran in the Test & Validate job - Re-run suppression (S4): when int-failed exceeds int-passing, the full re-run command block is replaced with a one-liner pointing to the suite command - Documentation: CHANGELOG not needed (CI infrastructure); README not needed (no user-facing changes) ## Test Plan - [ ] All existing tests pass - [ ] Manual testing: CI run on this PR will exercise all changes (comment posting, minimizing, icon, missing-report detection, scope note, re-run suppression) Closes #579 * [PR feedback] Defer pass/fail decision until after integration report validation; add --paginate The ICON (✅/❌) was computed before the missing-report check, so a crashed integration run with passing unit tests showed a green checkmark with only a suble warning row. The gh api call that fetches previous comments for minimizing also lacked --paginate, silently missing old comments on PRs with more than 30 comments. Fix A: detect missing/unparsable report file, set a flag, defer ICON computation until after that check. A missing report now flips PASS to false. Fix B: add --paginate to the gh api comment-listing call so all pages are retrieved. Ref: #591 (review)
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
1 participant
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.
Summary
Adds PR comment posts after each CI job finishes, reporting elapsed duration, unit test counts, integration test passing counts, and the QA TC IDs exercised. Also hardens all test pipelines against exit-code swallowing and uploads test report artifacts for post-hoc debugging. Disambiguates two duplicate
Report:lines in the test release runner.Changes
post-ci-commentbuilds a markdown table and posts it viagh api— always creates a new comment so each run appears chronologicallyrun-tests,run-integration-tests,run-integration-tests-with-extensions) now: output test counts, useset -o pipefail+teeto prevent exit-code swallowing, and extract stats from captured outputdate +%sfor cross-step timing (not$SECONDS, which resets per shell), grantspull-requests: write, and wires the post-comment step withcontinue-on-error: trueso API glitches don't fail the jobReport:lines in test-release-run.sh and validate-qa-coverage.sh toReport output:/Report complete:Documentation: CHANGELOG not needed (CI infrastructure, not user-facing); README not needed (no new commands/settings/features)
Test Plan
Related
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Chores