chore: coverage scripts collection simplifications#5724
Open
chore: coverage scripts collection simplifications#5724
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExtracted inline CI steps into reusable scripts (coverage downloaders, tmpfs/RAM-disk mount, summary writers), added coverage config, refactored compare script I/O to here-strings, updated workflows to checkout and invoke scripts, and tightened Slack notification gating and summary/report behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow as "GitHub Workflow"
participant Downloader as "download-previous(.sh)"
participant Releases as "gh API / Releases"
participant Artifacts as "GitHub Artifacts"
participant Runner as "Runner FS"
participant Comparer as "compare-coverage.sh"
participant Summarizer as "write-summary(.sh)"
participant GHA_SUM as "GITHUB_STEP_SUMMARY"
Workflow->>Downloader: run with REPO, BRANCH, CURRENT_RUN_ID / TAG_NAME
Downloader->>Releases: query runs/releases via gh api / gh release list
Releases-->>Downloader: run/release metadata
Downloader->>Artifacts: request/download artifact ZIP / release asset
Artifacts-->>Downloader: artifact ZIP / file
Downloader->>Runner: unzip/save to output dir (previous/)
Downloader-->>Workflow: print has_previous=true|false
Workflow->>Comparer: run compare-coverage.sh (uses coverage.conf)
Comparer->>Runner: read current & previous summaries
Comparer-->>Runner: write comparison-report.json (includes threshold, significant_change)
Workflow->>Summarizer: run write-summary(.sh) with REPORT_MODE
Summarizer->>Runner: read comparison-report.json
Summarizer->>GHA_SUM: append markdown tables (baseline/diff, top drops/gains)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/scripts/coverage/download-previous.sh (1)
35-36: Hardcoded/tmppath could cause issues in parallel executions.If multiple instances of this script run concurrently, they would overwrite the same
/tmp/prev-coverage.zipfile. Consider usingmktempfor a unique temporary file.💡 Suggested improvement
# Download and extract -gh api "repos/${REPO}/actions/artifacts/${ARTIFACT_ID}/zip" >/tmp/prev-coverage.zip -unzip -o /tmp/prev-coverage.zip -d "$OUTPUT_DIR/" +TMPZIP=$(mktemp) +gh api "repos/${REPO}/actions/artifacts/${ARTIFACT_ID}/zip" >"$TMPZIP" +unzip -o "$TMPZIP" -d "$OUTPUT_DIR/" +rm -f "$TMPZIP"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/coverage/download-previous.sh around lines 35 - 36, The script currently writes to a hardcoded /tmp/prev-coverage.zip which can collide when run in parallel; change it to create a unique temp file (e.g., TMPFILE=$(mktemp) or mktemp -u pattern) and use that variable in the gh api download and unzip commands (referencing ARTIFACT_ID, REPO, and OUTPUT_DIR in the same lines), and add a trap to remove the temp file on exit to avoid leaving junk..github/scripts/coverage/write-summary-compare.sh (1)
19-21: Consider handlingnullvalues from jq.If the JSON report has missing or null fields,
jq -rwill output the literal string"null", which would render asnull%in the summary table. Consider adding validation or fallback values.💡 Suggested improvement
-CURR=$(jq -r '.current_total' "$REPORT") -PREV=$(jq -r '.previous_total' "$REPORT") -DELTA=$(jq -r '.total_delta' "$REPORT") +CURR=$(jq -r '.current_total // "N/A"' "$REPORT") +PREV=$(jq -r '.previous_total // "N/A"' "$REPORT") +DELTA=$(jq -r '.total_delta // "N/A"' "$REPORT")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/coverage/write-summary-compare.sh around lines 19 - 21, The script assigns CURR, PREV, and DELTA directly from jq which can return the literal "null"; update the jq invocations used to populate CURR, PREV, and DELTA to provide safe fallbacks (e.g. use jq's // operator like '.current_total // 0', '.previous_total // 0', '.total_delta // 0') or validate and coerce "null" to 0 after reading, and ensure downstream formatting uses numeric values rather than the string "null" so the summary shows 0% instead of null%..github/scripts/coverage/write-summary.sh (1)
24-26: Same null handling consideration aswrite-summary-compare.sh.The jq extractions could return literal
"null"strings if the JSON fields are missing. Consider using jq's alternative operator (//) for fallback values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/coverage/write-summary.sh around lines 24 - 26, The jq extracts assigning CURR, PREV, and DELTA can yield the string "null" when fields are missing; update the three assignments (CURR, PREV, DELTA) to use jq's alternative operator (//) to provide safe fallback values (e.g., 0) so they never become the literal "null" string and remain numeric for later math/formatting. Locate the lines setting CURR, PREV, and DELTA in write-summary.sh and adjust the jq expressions to include a default via // (and keep -r) to ensure robust null handling.
🤖 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/scripts/coverage/download-previous.sh:
- Around line 35-36: The script currently writes to a hardcoded
/tmp/prev-coverage.zip which can collide when run in parallel; change it to
create a unique temp file (e.g., TMPFILE=$(mktemp) or mktemp -u pattern) and use
that variable in the gh api download and unzip commands (referencing
ARTIFACT_ID, REPO, and OUTPUT_DIR in the same lines), and add a trap to remove
the temp file on exit to avoid leaving junk.
In @.github/scripts/coverage/write-summary-compare.sh:
- Around line 19-21: The script assigns CURR, PREV, and DELTA directly from jq
which can return the literal "null"; update the jq invocations used to populate
CURR, PREV, and DELTA to provide safe fallbacks (e.g. use jq's // operator like
'.current_total // 0', '.previous_total // 0', '.total_delta // 0') or validate
and coerce "null" to 0 after reading, and ensure downstream formatting uses
numeric values rather than the string "null" so the summary shows 0% instead of
null%.
In @.github/scripts/coverage/write-summary.sh:
- Around line 24-26: The jq extracts assigning CURR, PREV, and DELTA can yield
the string "null" when fields are missing; update the three assignments (CURR,
PREV, DELTA) to use jq's alternative operator (//) to provide safe fallback
values (e.g., 0) so they never become the literal "null" string and remain
numeric for later math/formatting. Locate the lines setting CURR, PREV, and
DELTA in write-summary.sh and adjust the jq expressions to include a default via
// (and keep -r) to ensure robust null handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 12c3c202-23b1-422c-93d0-922adf9ef86e
📒 Files selected for processing (7)
.github/scripts/coverage/compare-coverage.sh.github/scripts/coverage/download-previous.sh.github/scripts/coverage/write-summary-compare.sh.github/scripts/coverage/write-summary.sh.github/workflows/coverage-compare-main.yml.github/workflows/coverage-compare.yml.github/workflows/coverage-release.yml
yhakbar
reviewed
Mar 24, 2026
| fi | ||
|
|
||
| # Sanitize user inputs for markdown injection | ||
| BASE_REF=$(echo "$BASE_REF" | tr -d '<>|`\n\r') |
yhakbar
previously approved these changes
Mar 24, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/scripts/coverage/download-previous-release.sh:
- Around line 10-11: The PREV_TAG extraction uses `gh release list -L 50` which
limits the lookup window and can misreport a previous tag as missing for older
TAG_NAME values; update the logic around PREV_TAG to paginate or fetch all
non-draft/non-pre-release releases (or explicitly validate TAG_NAME is within
the fetched window and fail) so the predecessor lookup is correct—specifically
change the `gh release list` usage (where PREV_TAG is set) to iterate/paginate
through all releases (or increase/fetch until TAG_NAME is found) and keep the jq
logic that finds the index of $tagName and returns the element at index+1 only
after ensuring the full list contains TAG_NAME.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4d4d7418-007f-469f-bb80-60c7ab5131b8
📒 Files selected for processing (5)
.github/scripts/coverage/compare-coverage.sh.github/scripts/coverage/download-previous-release.sh.github/scripts/coverage/write-summary-compare.sh.github/workflows/coverage-compare-main.yml.github/workflows/coverage-release.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/scripts/coverage/write-summary-compare.sh
- .github/workflows/coverage-compare-main.yml
- .github/scripts/coverage/compare-coverage.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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-release.yml:
- Around line 22-27: The checkout for the coverage helpers currently uses
actions/checkout but will resolve against the dispatched release tag; change the
"Fetch coverage scripts" checkout step (actions/checkout) to explicitly fetch
the helpers from the repository default branch (e.g., ref: main or a
configurable branch input) into path: _coverage-scripts so the workflow always
uses the current helper files rather than the release tag’s .github/scripts;
apply the same change to the other identical checkout block around lines 85-100.
In @.github/workflows/integration-test.yml:
- Around line 158-160: The "Mount tmpfs / RAM disk" step calls
./.github/scripts/setup/mount-tmpfs.sh which fails on Windows; add a conditional
to skip this step when the job runs on Windows (e.g., use a GitHub Actions
conditional referencing runner.os or the matrix OS such as if: runner.os !=
'Windows' or if: matrix.os != 'windows') so the step only executes on
Linux/Darwin; update the step named "Mount tmpfs / RAM disk" to include that
condition and keep the existing shell and run settings unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dffce446-0162-46cf-a018-58a33d59de7d
📒 Files selected for processing (8)
.github/scripts/setup/mount-tmpfs.sh.github/workflows/base-test.yml.github/workflows/build-no-proxy.yml.github/workflows/coverage-compare-main.yml.github/workflows/coverage-compare.yml.github/workflows/coverage-release.yml.github/workflows/integration-test.yml.github/workflows/lint.yml
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/coverage-compare-main.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/coverage-compare.yml
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/scripts/coverage/download-previous-release.sh (1)
28-35: Consider preserving stderr for unexpected errors.The
2>/dev/nullsuppresses all error output, which keeps logs clean for expected "asset not found" cases but also hides unexpected failures (network issues, authentication problems, rate limiting). This could make debugging harder in production.♻️ Optional: Capture and conditionally log stderr
if [[ -n "$PREV_TAG" ]]; then echo "Previous release: $PREV_TAG" - if gh release download "$PREV_TAG" -p "coverage-summary.json" -D "$OUTPUT_DIR/" 2>/dev/null; then + if err=$(gh release download "$PREV_TAG" -p "coverage-summary.json" -D "$OUTPUT_DIR/" 2>&1); then echo "has_previous=true" else + # Log error for debugging if it's not a simple "not found" + [[ -n "$err" ]] && echo "Debug: $err" >&2 echo "No coverage data in previous release $PREV_TAG" echo "has_previous=false" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/coverage/download-previous-release.sh around lines 28 - 35, The gh release download call currently discards all stderr via 2>/dev/null; change it to capture stderr (e.g., into a variable or temp file) when running gh release download "$PREV_TAG" -p "coverage-summary.json" -D "$OUTPUT_DIR/" so you can inspect the error message on failure; then, in the failure branch for the gh call, only suppress or treat as non-fatal when the captured stderr clearly indicates a missing asset (asset not found), but otherwise log the captured stderr (or print it via echo/processLogger) so unexpected errors like network, auth, or rate-limiting are visible. Ensure you reference the same PREV_TAG, gh release download invocation, and OUTPUT_DIR variables when implementing the capture-and-conditional-log behavior.
🤖 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/scripts/coverage/download-previous-release.sh:
- Around line 28-35: The gh release download call currently discards all stderr
via 2>/dev/null; change it to capture stderr (e.g., into a variable or temp
file) when running gh release download "$PREV_TAG" -p "coverage-summary.json" -D
"$OUTPUT_DIR/" so you can inspect the error message on failure; then, in the
failure branch for the gh call, only suppress or treat as non-fatal when the
captured stderr clearly indicates a missing asset (asset not found), but
otherwise log the captured stderr (or print it via echo/processLogger) so
unexpected errors like network, auth, or rate-limiting are visible. Ensure you
reference the same PREV_TAG, gh release download invocation, and OUTPUT_DIR
variables when implementing the capture-and-conditional-log behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bb6574f0-ab3b-47ca-8eec-e52b14f178a8
📒 Files selected for processing (1)
.github/scripts/coverage/download-previous-release.sh
* docs: Updating docs dependencies * docs: Adding changelog in docs * docs: Fixing some styling * docs: Adding prior releases section * docs: Fixing links
* docs: Updating docs dependencies * docs: Adding changelog in docs * docs: Fixing some styling * docs: Adding prior releases section * docs: Fixing links * docs: Adding changelog copy button * fix: Adding support for 'Unreleased'
yhakbar
approved these changes
Apr 1, 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.
Description
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added / Removed / Updated [X].
Migration Guide
Summary by CodeRabbit