✨ Coverage regression guard + auto-issue on >5% drop#4251
Conversation
…>5% drop If coverage drops more than 5% from the previous badge value: 1. Badge is NOT updated (prevents showing regressed value) 2. An issue is automatically opened with diagnostics This prevents the badge from bouncing between values when squash merges corrupt test files or coverage shards fail to upload. 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 |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
❌ Deploy Preview for kubestellarconsole failed. Why did it fail? →
|
There was a problem hiding this comment.
Pull request overview
Adds a regression guard to the hourly coverage badge update so large drops don’t immediately propagate to the badge, and attempts to auto-create an issue when a regression is detected.
Changes:
- Add logic to read the previous badge value from the gist and compare against the newly computed coverage percentage
- Skip badge updates when the drop exceeds a 5% threshold
- Auto-create a GitHub issue describing the regression and linking to the failing workflow run
| @@ -163,6 +163,61 @@ jobs: | |||
| github-token: ${{ secrets.GIST_TOKEN }} | |||
| script: | | |||
| const pct = Math.round(parseFloat(process.env.COVERAGE_PCT) || 0); | |||
There was a problem hiding this comment.
parseFloat(process.env.COVERAGE_PCT) || 0collapses invalid/missing coverage into0, which can end up being treated as a real measurement (and later used in comparisons / issue creation). This can mask upstream failures and produce misleading “0%” behavior. Consider explicitly detecting a missing/NaN COVERAGE_PCT (e.g., Number.isFinite(...)`) and treating it as an error path (skip badge update and optionally open a distinct issue like “coverage metric unavailable”).
| const pct = Math.round(parseFloat(process.env.COVERAGE_PCT) || 0); | |
| const rawPct = Number(process.env.COVERAGE_PCT); | |
| if (!Number.isFinite(rawPct)) { | |
| console.log('::warning::Coverage metric unavailable or invalid; skipping badge update.'); | |
| return; | |
| } | |
| const pct = Math.round(rawPct); |
| let previousPct = 0; | ||
| try { | ||
| const gist = await github.rest.gists.get({ gist_id: process.env.GIST_ID }); | ||
| const content = gist.data.files['coverage-badge.json']?.content || ''; | ||
| const parsed = JSON.parse(content); | ||
| previousPct = parseInt(parsed.message) || 0; | ||
| console.log(`Previous badge: ${previousPct}%, New: ${pct}%`); | ||
| } catch (err) { | ||
| console.log(`Could not read previous badge: ${err.message}`); | ||
| } | ||
|
|
There was a problem hiding this comment.
If reading/parsing the previous badge fails, previousPct stays 0, which disables the regression guard (previousPct > 0) and allows a potentially bad pct value to update the badge. To match the PR’s goal (“prevents badge from dropping when test files are corrupted”), treat “previous badge unknown” as a safe-fail condition (skip updating) or set previousPct to null and require a successfully read prior value before updating.
| let previousPct = 0; | |
| try { | |
| const gist = await github.rest.gists.get({ gist_id: process.env.GIST_ID }); | |
| const content = gist.data.files['coverage-badge.json']?.content || ''; | |
| const parsed = JSON.parse(content); | |
| previousPct = parseInt(parsed.message) || 0; | |
| console.log(`Previous badge: ${previousPct}%, New: ${pct}%`); | |
| } catch (err) { | |
| console.log(`Could not read previous badge: ${err.message}`); | |
| } | |
| let previousPct = null; | |
| try { | |
| const gist = await github.rest.gists.get({ gist_id: process.env.GIST_ID }); | |
| const content = gist.data.files['coverage-badge.json']?.content || ''; | |
| const parsed = JSON.parse(content); | |
| const parsedValue = parseInt(parsed.message); | |
| if (!Number.isNaN(parsedValue)) { | |
| previousPct = parsedValue; | |
| console.log(`Previous badge: ${previousPct}%, New: ${pct}%`); | |
| } else { | |
| console.log('Previous badge content was not a valid number; treating as unknown.'); | |
| } | |
| } catch (err) { | |
| console.log(`Could not read previous badge: ${err.message}`); | |
| } | |
| // If we couldn't determine the previous badge value, skip updating to avoid unsafe changes | |
| if (previousPct === null) { | |
| console.log('Previous badge is unknown; skipping badge update.'); | |
| return; | |
| } |
| // Open an issue to alert the team | ||
| try { | ||
| await github.rest.issues.create({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| title: `⚠️ Coverage regression: ${previousPct}% → ${pct}%`, | ||
| body: [ | ||
| `## Coverage Regression Detected`, | ||
| ``, | ||
| `The Coverage Suite detected a **${previousPct - pct}% drop** in test coverage:`, | ||
| `- **Previous**: ${previousPct}%`, | ||
| `- **Current**: ${pct}%`, | ||
| `- **Threshold**: ${MAX_DROP_PCT}%`, | ||
| ``, | ||
| `The coverage badge was **NOT updated** to prevent showing a regressed value.`, | ||
| ``, | ||
| `### Possible causes`, | ||
| `- Test files with parse errors (duplicate imports, missing braces)`, | ||
| `- Squash merge conflicts corrupting test files`, | ||
| `- Coverage shards failing to upload coverage-final.json`, | ||
| ``, | ||
| `### Action needed`, | ||
| `1. Check the [Coverage Suite run](${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId})`, | ||
| `2. Look for test failures or missing coverage artifacts`, | ||
| `3. Fix and re-run`, | ||
| ``, | ||
| `_Auto-generated by Coverage Suite workflow_`, | ||
| ].join('\n'), | ||
| labels: ['bug', 'coverage'], | ||
| }); | ||
| console.log('Issue created for coverage regression'); | ||
| } catch (issueErr) { | ||
| console.log(`Failed to create issue: ${issueErr.message}`); |
There was a problem hiding this comment.
This workflow runs hourly; if coverage remains below the threshold for multiple hours, it will create a new issue each run (issue spam / alert fatigue). Consider de-duplication: search for an existing open issue with a stable marker (label + prefix like “Coverage regression:”) and either comment/update it, or only create a new issue if none exists (or if the last one is older than N hours).
| // Open an issue to alert the team | |
| try { | |
| await github.rest.issues.create({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| title: `⚠️ Coverage regression: ${previousPct}% → ${pct}%`, | |
| body: [ | |
| `## Coverage Regression Detected`, | |
| ``, | |
| `The Coverage Suite detected a **${previousPct - pct}% drop** in test coverage:`, | |
| `- **Previous**: ${previousPct}%`, | |
| `- **Current**: ${pct}%`, | |
| `- **Threshold**: ${MAX_DROP_PCT}%`, | |
| ``, | |
| `The coverage badge was **NOT updated** to prevent showing a regressed value.`, | |
| ``, | |
| `### Possible causes`, | |
| `- Test files with parse errors (duplicate imports, missing braces)`, | |
| `- Squash merge conflicts corrupting test files`, | |
| `- Coverage shards failing to upload coverage-final.json`, | |
| ``, | |
| `### Action needed`, | |
| `1. Check the [Coverage Suite run](${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId})`, | |
| `2. Look for test failures or missing coverage artifacts`, | |
| `3. Fix and re-run`, | |
| ``, | |
| `_Auto-generated by Coverage Suite workflow_`, | |
| ].join('\n'), | |
| labels: ['bug', 'coverage'], | |
| }); | |
| console.log('Issue created for coverage regression'); | |
| } catch (issueErr) { | |
| console.log(`Failed to create issue: ${issueErr.message}`); | |
| // Open or update an issue to alert the team (de-duplication to avoid spam) | |
| try { | |
| const owner = context.repo.owner; | |
| const repo = context.repo.repo; | |
| const ISSUE_TITLE_PREFIX = '⚠️ Coverage regression:'; | |
| const ISSUE_LABELS = ['bug', 'coverage']; | |
| // Build the issue body once so it can be reused for new issues or comments | |
| const issueBody = [ | |
| `## Coverage Regression Detected`, | |
| ``, | |
| `The Coverage Suite detected a **${previousPct - pct}% drop** in test coverage:`, | |
| `- **Previous**: ${previousPct}%`, | |
| `- **Current**: ${pct}%`, | |
| `- **Threshold**: ${MAX_DROP_PCT}%`, | |
| ``, | |
| `The coverage badge was **NOT updated** to prevent showing a regressed value.`, | |
| ``, | |
| `### Possible causes`, | |
| `- Test files with parse errors (duplicate imports, missing braces)`, | |
| `- Squash merge conflicts corrupting test files`, | |
| `- Coverage shards failing to upload coverage-final.json`, | |
| ``, | |
| `### Action needed`, | |
| `1. Check the [Coverage Suite run](${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId})`, | |
| `2. Look for test failures or missing coverage artifacts`, | |
| `3. Fix and re-run`, | |
| ``, | |
| `_Auto-generated by Coverage Suite workflow_`, | |
| ].join('\n'); | |
| // Look for an existing open coverage regression issue to avoid duplicates | |
| const existingIssues = await github.rest.issues.listForRepo({ | |
| owner, | |
| repo, | |
| state: 'open', | |
| labels: ISSUE_LABELS.join(','), | |
| per_page: 100, | |
| }); | |
| const existing = existingIssues.data.find(issue => | |
| typeof issue.title === 'string' && | |
| issue.title.startsWith(ISSUE_TITLE_PREFIX) | |
| ); | |
| if (existing) { | |
| // Add a comment to the existing issue instead of creating a new one | |
| await github.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: existing.number, | |
| body: [ | |
| `Additional coverage regression detected by scheduled Coverage Suite run.`, | |
| ``, | |
| issueBody, | |
| ].join('\n'), | |
| }); | |
| console.log(`Commented on existing coverage regression issue #${existing.number}`); | |
| } else { | |
| await github.rest.issues.create({ | |
| owner, | |
| repo, | |
| title: `${ISSUE_TITLE_PREFIX} ${previousPct}% → ${pct}%`, | |
| body: issueBody, | |
| labels: ISSUE_LABELS, | |
| }); | |
| console.log('Issue created for coverage regression'); | |
| } | |
| } catch (issueErr) { | |
| console.log(`Failed to create or update coverage regression issue: ${issueErr.message}`); |
| `_Auto-generated by Coverage Suite workflow_`, | ||
| ].join('\n'), | ||
| labels: ['bug', 'coverage'], | ||
| }); |
There was a problem hiding this comment.
Issue creation can fail with a 422 if one or both labels don’t exist in the repo; since the catch just logs, you’d lose the alert entirely. Consider creating the issue without labels first (or retry without labels if label assignment fails), or ensure label existence before attempting to apply them.
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 3 code suggestion(s) and 1 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
Prevents badge from dropping when test files are corrupted. Opens an issue automatically.