✨ Restructure CI: slim PR gate, hourly coverage suite, weekly review#4128
✨ Restructure CI: slim PR gate, hourly coverage suite, weekly review#4128clubanderson merged 1 commit intomainfrom
Conversation
- coverage-gate.yml: PR-only, fast smoke check on modified files (<2 min) - coverage-hourly.yml: NEW — runs full test suite every hour, updates badge gist - coverage-weekly-review.yml: NEW — Monday review of past week's runs, opens issues for flaky tests - Badge now updates hourly instead of on push (which kept getting cancelled) Signed-off-by: Andrew Anderson <andy@clubanderson.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole canceled.
|
There was a problem hiding this comment.
Pull request overview
Restructures CI coverage workflows by moving full-suite coverage off the PR path and adding scheduled automation for continuous coverage health monitoring.
Changes:
- Slims PR coverage gate to a faster, PR-only workflow that targets modified files and posts a PR comment.
- Adds an hourly full test+coverage workflow that uploads artifacts and updates a coverage badge gist.
- Adds a weekly scheduled workflow that reviews the last week’s hourly runs and opens issues when failure rate exceeds a threshold.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| .github/workflows/coverage-gate.yml | Makes PR gate PR-only and attempts to scope coverage checks to modified files with a PR comment report. |
| .github/workflows/coverage-hourly.yml | Adds an hourly full coverage run, uploads coverage artifacts, and updates a badge gist. |
| .github/workflows/coverage-weekly-review.yml | Adds a weekly analysis of hourly workflow outcomes and auto-opens issues when failure rate is high. |
| run: npx vitest run --coverage | ||
| run: | | ||
| # Get modified source files | ||
| MODIFIED=$(git diff --name-only origin/main...HEAD -- 'web/src/**/*.ts' 'web/src/**/*.tsx' \ |
There was a problem hiding this comment.
Using git diff origin/main...HEAD is likely to fail in GitHub Actions because actions/checkout@v4 typically does not fetch origin/main with the default fetch depth. Set fetch-depth: 0 (or explicitly fetch the base branch) or diff against ${{ github.event.pull_request.base.sha }} to ensure the modified-file detection works reliably.
| MODIFIED=$(git diff --name-only origin/main...HEAD -- 'web/src/**/*.ts' 'web/src/**/*.tsx' \ | |
| MODIFIED=$(git diff --name-only ${{ github.event.pull_request.base.sha }}...HEAD -- 'web/src/**/*.ts' 'web/src/**/*.tsx' \ |
| fi | ||
|
|
||
| # Run only tests related to modified files (fast) | ||
| npx vitest run --coverage --reporter=verbose 2>&1 | tail -20 || true |
There was a problem hiding this comment.
This step claims to run only tests related to modified files, but MODIFIED is never used to scope the Vitest run, so it still runs the full suite. Additionally, piping to tail (and || true) masks the Vitest exit code, which can hide real failures and still produce a 'successful' workflow run. Consider (1) actually passing the modified file list to Vitest using an appropriate related/changed option, and (2) preserving Vitest’s exit status while still limiting logs (e.g., avoid the pipe or explicitly propagate the first command’s exit code).
| npx vitest run --coverage --reporter=verbose 2>&1 | tail -20 || true | |
| LOG_FILE="$(mktemp)" | |
| npx vitest related $MODIFIED --coverage --reporter=verbose >"$LOG_FILE" 2>&1 | |
| VITEST_STATUS=$? | |
| echo "Last 20 lines of Vitest output:" | |
| tail -20 "$LOG_FILE" | |
| exit $VITEST_STATUS |
| if [ ! -f "$SUMMARY_FILE" ]; then | ||
| echo "::warning::No coverage summary found — tests may have failed" | ||
| echo "::warning::No coverage summary found" | ||
| echo "status=skip" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
If tests fail (or coverage generation breaks), this reports status=skip and exits 0, which can silently pass the PR gate and skip PR feedback. If the intent is a non-blocking gate, consider setting status=warn and still posting a PR comment; if the intent is to ensure smoke-test health, consider failing the job when the summary is missing so genuine regressions are visible.
| echo "status=skip" >> "$GITHUB_OUTPUT" | |
| echo "status=warn" >> "$GITHUB_OUTPUT" |
| continue-on-error: true | ||
|
|
||
| - name: Extract coverage percentage |
There was a problem hiding this comment.
With continue-on-error: true, the workflow will usually finish with a successful conclusion even when tests fail. That breaks the weekly review’s failure-rate calculation (it relies on conclusion === 'failure') and can also update the badge based on missing/partial coverage. Prefer letting the test step fail (and use if: always() on artifact/badge steps if you still want post-processing), or explicitly fail the job at the end based on the test step outcome.
| continue-on-error: true | |
| - name: Extract coverage percentage | |
| - name: Extract coverage percentage | |
| if: always() |
| GIST_ID: ${{ env.GIST_ID }} | ||
| COVERAGE_PCT: ${{ steps.cov.outputs.pct }} | ||
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
Updating a gist typically requires a token with gist scope; GITHUB_TOKEN generally cannot write to user gists. As written, the badge update will likely fail every run (and the try/catch only logs it), undermining the PR’s stated goal of reliable badge updates. Use a dedicated secret (e.g., secrets.GIST_TOKEN) with appropriate permissions and pass that as github-token.
| github-token: ${{ secrets.GITHUB_TOKEN }} | |
| github-token: ${{ secrets.GIST_TOKEN }} |
| // Get hourly coverage workflow runs from past week | ||
| const { data: runs } = await github.rest.actions.listWorkflowRuns({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| workflow_id: 'coverage-hourly.yml', | ||
| status: 'completed', | ||
| per_page: 100, | ||
| created: `>=${since.split('T')[0]}`, | ||
| }); | ||
|
|
||
| const totalRuns = runs.workflow_runs.length; | ||
| const successful = runs.workflow_runs.filter(r => r.conclusion === 'success').length; | ||
| const failed = runs.workflow_runs.filter(r => r.conclusion === 'failure').length; | ||
| const cancelled = runs.workflow_runs.filter(r => r.conclusion === 'cancelled').length; |
There was a problem hiding this comment.
This only fetches the first 100 workflow runs, but a 7-day hourly schedule produces ~168 runs. The failure rate will be computed from an incomplete dataset and can be significantly wrong. Paginate through results (or iterate page until empty) to cover the full review period.
| // Get hourly coverage workflow runs from past week | |
| const { data: runs } = await github.rest.actions.listWorkflowRuns({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| workflow_id: 'coverage-hourly.yml', | |
| status: 'completed', | |
| per_page: 100, | |
| created: `>=${since.split('T')[0]}`, | |
| }); | |
| const totalRuns = runs.workflow_runs.length; | |
| const successful = runs.workflow_runs.filter(r => r.conclusion === 'success').length; | |
| const failed = runs.workflow_runs.filter(r => r.conclusion === 'failure').length; | |
| const cancelled = runs.workflow_runs.filter(r => r.conclusion === 'cancelled').length; | |
| // Get hourly coverage workflow runs from past week (paginate to cover full period) | |
| const allRuns = []; | |
| let page = 1; | |
| while (true) { | |
| const { data } = await github.rest.actions.listWorkflowRuns({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| workflow_id: 'coverage-hourly.yml', | |
| status: 'completed', | |
| per_page: 100, | |
| page, | |
| created: `>=${since.split('T')[0]}`, | |
| }); | |
| const pageRuns = data.workflow_runs || []; | |
| if (pageRuns.length === 0) { | |
| break; | |
| } | |
| allRuns.push(...pageRuns); | |
| if (pageRuns.length < 100) { | |
| break; | |
| } | |
| page += 1; | |
| } | |
| const totalRuns = allRuns.length; | |
| const successful = allRuns.filter(r => r.conclusion === 'success').length; | |
| const failed = allRuns.filter(r => r.conclusion === 'failure').length; | |
| const cancelled = allRuns.filter(r => r.conclusion === 'cancelled').length; |
| }); | ||
|
|
||
| const titlePrefix = issue.title.split(' in past')[0]; | ||
| const duplicate = existing.find(e => e.title.includes('[Coverage]')); |
There was a problem hiding this comment.
The duplicate detection ignores titlePrefix and treats any open issue containing [Coverage] as a duplicate, which can prevent creating a new issue even when the titles/problems differ. Use titlePrefix (or an exact match / startsWith check) so only the same weekly alert is deduped.
| const duplicate = existing.find(e => e.title.includes('[Coverage]')); | |
| const duplicate = existing.find(e => e.title.startsWith(titlePrefix)); |
| } | ||
|
|
||
| if (issues.length === 0) { | ||
| console.log(`All clear — ${successRate}% success rate is above ${FLAKY_THRESHOLD}% threshold`); |
There was a problem hiding this comment.
This log message compares successRate against FLAKY_THRESHOLD, but the threshold is defined as a failure-rate percent. The message is misleading; it should state that the failure rate is below the threshold (or compare success rate against 100 - FLAKY_THRESHOLD).
| console.log(`All clear — ${successRate}% success rate is above ${FLAKY_THRESHOLD}% threshold`); | |
| console.log(`All clear — ${failureRate}% failure rate is at or below ${FLAKY_THRESHOLD}% threshold`); |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 8 code suggestion(s) and 0 general comment(s). @copilot Please apply all of the following code review suggestions:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
Summary
Why
The push-to-main coverage runs kept getting cancelled by concurrency groups. Moving to hourly schedule eliminates this — the badge will update reliably.
Test plan