Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 2 additions & 15 deletions .github/scripts/ci-summary-report-publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,25 +106,12 @@ function sanitizeSnapshots(raw) {
* Derive metrics conclusion and display text from the parsed ci-summary artifact.
* Uses === true for boolean fields to avoid misinterpreting JSON strings.
* @param {object} s - Parsed ci-summary.json
* @returns {{ hasInfraErrors: boolean, totalChanges: number|null, snapshots: Array|null, skipped: boolean, conclusion: string, text: string }}
* @returns {{ hasInfraErrors: boolean, totalChanges: number|null, snapshots: Array|null, conclusion: string, text: string }}
*/
function computeMetrics(s) {
const hasInfraErrors = s.metrics_has_infra_errors === true;
const totalChanges = safeNum(s.metrics_total_changes);
const snapshots = sanitizeSnapshots(s.metrics_snapshots);
const skipped = s.metrics_conclusion === 'skipped';

if (skipped) {
return {
hasInfraErrors,
totalChanges,
snapshots,
skipped,
conclusion: 'success',
text: '⏭️ Metrics comparison skipped for this PR.',
};
}

// Derive conclusion from the same conditions that drive text so they are always consistent.
const conclusion = (hasInfraErrors || totalChanges === null || totalChanges > 0) ? 'failure' : 'success';

Expand All @@ -139,7 +126,7 @@ function computeMetrics(s) {
text = '✅ No significant metric changes';
}

return { hasInfraErrors, totalChanges, snapshots, skipped, conclusion, text };
return { hasInfraErrors, totalChanges, snapshots, conclusion, text };
}

/**
Expand Down
13 changes: 0 additions & 13 deletions .github/scripts/ci-summary-report-publish.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,19 +208,6 @@ describe('computeMetrics', () => {
expect(r.conclusion).toBe('failure');
expect(r.text).toBe('❌ Could not read metrics_total_changes from summary');
expect(r.totalChanges).toBeNull();
expect(r.skipped).toBe(false);
});

test('success when metrics comparison is explicitly skipped', () => {
const r = computeMetrics({
metrics_conclusion: 'skipped',
metrics_has_infra_errors: false,
metrics_total_changes: null,
});
expect(r.conclusion).toBe('success');
expect(r.text).toBe('⏭️ Metrics comparison skipped for this PR.');
expect(r.totalChanges).toBeNull();
expect(r.skipped).toBe(true);
});

test('infra errors take precedence over missing total_changes', () => {
Expand Down
15 changes: 2 additions & 13 deletions .github/workflows/ci-summary-report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ jobs:
run: python3 -m pip install prometheus-client

- name: Compare metrics and generate summary
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.user.login != 'dependabot[bot]' }}
id: compare-metrics
shell: bash
run: bash ./scripts/e2e/metrics_summary.sh
Expand Down Expand Up @@ -159,7 +158,6 @@ jobs:
env:
PR_NUMBER: ${{ github.event.pull_request.number }}
METRICS_CONCLUSION: ${{ steps.compare-metrics.outputs.CONCLUSION }}
METRICS_OUTCOME: ${{ steps.compare-metrics.outcome }}
METRICS_TOTAL: ${{ steps.compare-metrics.outputs.TOTAL_CHANGES }}
METRICS_INFRA_ERRORS: ${{ steps.compare-metrics.outputs.INFRA_ERRORS }}
COVERAGE_MERGE_OUTCOME: ${{ steps.merge-coverage.outcome }}
Expand All @@ -171,18 +169,10 @@ jobs:
python3 - <<'PYEOF'
import json, os

metrics_conclusion = os.environ.get('METRICS_CONCLUSION') or ''
if not metrics_conclusion:
# Step failed/cancelled before writing CONCLUSION output.
outcome = os.environ.get('METRICS_OUTCOME', '')
metrics_conclusion = 'failure' if outcome in ('failure', 'cancelled') else 'skipped'
metrics_conclusion = os.environ.get('METRICS_CONCLUSION') or 'failure'
metrics_total_env = os.environ.get('METRICS_TOTAL')
if metrics_total_env not in (None, ''):
metrics_total = int(metrics_total_env)
elif metrics_conclusion == 'skipped':
# Keep publisher check green for intentionally skipped metrics runs
# (for example, Dependabot PRs where compare-metrics is disabled).
metrics_total = 0
else:
metrics_total = None
has_infra_errors = os.environ.get('METRICS_INFRA_ERRORS') == 'true'
Expand Down Expand Up @@ -275,8 +265,7 @@ jobs:
# ci-summary-report-publish.yml can still post the PR comment and check runs.
- name: Fail if coverage or metrics gate failed
if: |
((github.event_name != 'pull_request' || github.event.pull_request.user.login != 'dependabot[bot]') &&
steps.compare-metrics.outputs.CONCLUSION == 'failure') ||
steps.compare-metrics.outputs.CONCLUSION == 'failure' ||
steps.coverage-gate.outputs.conclusion == 'failure'
run: |
echo "Metrics: ${{ steps.compare-metrics.outputs.CONCLUSION }}"
Expand Down
Loading