Skip to content

Commit aa76fca

Browse files
committed
test(ci): block retries on non-test failures and classify timeouts structurally
1 parent 539622b commit aa76fca

8 files changed

Lines changed: 446 additions & 112 deletions

‎docs/agents/testing.md‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -306,10 +306,18 @@ the retry policy cannot drift apart.
306306
The CI Coverage job runs the suite through `pnpm test:coverage:ci`
307307
(`scripts/lib/contention-retry-run.ts`), which applies one rule:
308308

309-
- **Timeouts only.** A rerun happens only when *every* failure in the run is timeout-shaped **and**
310-
lands in a listed file. One assertion failure — in a listed file or not — fails the job on the
311-
first run, so a real regression can never be papered over by a retry.
312-
- **One retry, of the failed files only.** Not the suite, and never twice.
309+
- **Timeouts only, classified structurally.** A rerun happens only when *every* failure in the run is
310+
a timeout raised by the runner itself **and** lands in a listed file. The lane's own reporter
311+
(`scripts/lib/contention-retry-reporter.ts`) classifies the live error object — Vitest's exact
312+
timeout template, a plain `Error`, no `expected`/`actual` — so an assertion whose *message*
313+
mentions a timeout cannot opt itself into a retry. One assertion failure — in a listed file or not
314+
— fails the job on the first run, so a real regression can never be papered over by a retry.
315+
- **Anything a rerun cannot re-check blocks the retry.** Unhandled errors, module load/setup errors,
316+
a coverage-threshold miss, or a nonzero exit no failed test explains are recorded as blockers
317+
(`scripts/lib/contention-retry-blockers.ts`) and fail the job, so a green retry can never erase a
318+
second, unrelated failure from the same run.
319+
- **One retry, of the failed files only.** Not the suite, and never twice. Two timed-out tests in one
320+
file are one retry, and count as one.
313321
- **Retries stay visible.** Every retried file is named in the job summary with its tracking issue
314322
and review date, and the run writes the shared scheduled-lane envelope
315323
(`scripts/lib/lane-envelope.ts`, #1430) with the retry count, so a permanently flaky file shows up
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Process-level blockers for the contention single-retry policy (#1419).
2+
//
3+
// The reporter can only see what happened inside the run. Two failure classes
4+
// live outside it and would otherwise be erased by a green retry: a coverage
5+
// verdict (computed after the tests, and a whole-suite property a file-scoped
6+
// rerun cannot re-establish), and any nonzero exit no failed test explains
7+
// (worker crash, config error, report never written). Both are recorded as
8+
// blockers, which forbid the retry outright.
9+
10+
import type { RunBlocker } from './contention-retry.ts';
11+
12+
/** Vitest's own coverage verdict lines (`checkCoverages`, vitest 4). */
13+
const COVERAGE_FAILURE = [
14+
/ERROR: Coverage for [a-z]+ \([\d.]+%\) does not meet (?:global )?threshold/i,
15+
/ERROR: Coverage for [a-z]+ \([\d.]+%\) does not meet .*minimum threshold/i,
16+
];
17+
18+
export type ProcessOutcome = {
19+
ok: boolean;
20+
/** Combined stdout+stderr of the run. */
21+
output: string;
22+
failureCount: number;
23+
};
24+
25+
export function processBlockers(outcome: ProcessOutcome): RunBlocker[] {
26+
if (outcome.ok) return [];
27+
const blockers: RunBlocker[] = [];
28+
const coverage = outcome.output
29+
.split('\n')
30+
.filter((line) => COVERAGE_FAILURE.some((pattern) => pattern.test(line)));
31+
for (const line of coverage) {
32+
blockers.push({ kind: 'coverage threshold', detail: line.trim() });
33+
}
34+
if (outcome.failureCount === 0 && blockers.length === 0) {
35+
blockers.push({
36+
kind: 'unexplained failure',
37+
detail: 'the run exited nonzero with no failed test recorded — see the job log',
38+
});
39+
}
40+
return blockers;
41+
}

‎scripts/lib/contention-retry-lane.ts‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,21 @@ import {
1313
planContentionRetry,
1414
type RetryOutcome,
1515
type RetryPlan,
16+
type RunBlocker,
1617
type TestFailure,
1718
} from './contention-retry.ts';
1819

1920
export type TestRun = {
2021
ok: boolean;
2122
failures: readonly TestFailure[];
23+
/** Non-test failures; any of them forbids a retry. */
24+
blockers?: readonly RunBlocker[];
2225
};
2326

2427
/** Telemetry payload for scheduled-lane health (#1430). */
2528
export type ContentionRetryTelemetry = {
26-
/** Files rerun in this job, with the test that timed out. */
27-
retried: ReadonlyArray<{ file: string; testName: string; trackingIssue: string }>;
29+
/** Files rerun in this job (one entry per file, not per failed test). */
30+
retried: ReadonlyArray<{ file: string; testNames: readonly string[]; trackingIssue: string }>;
2831
retryCount: number;
2932
retryOutcome: RetryOutcome | null;
3033
listSize: number;
@@ -81,7 +84,7 @@ export async function runWithContentionRetry(
8184
return finish(options, { ok: true, summary: '', plan: undefined, outcome: null });
8285
}
8386

84-
const plan = planContentionRetry(first.failures);
87+
const plan = planContentionRetry(first.failures, first.blockers ?? []);
8588
if (!plan.retry) {
8689
return finish(options, {
8790
ok: false,
@@ -111,16 +114,17 @@ function finish(
111114
},
112115
): ContentionRetryResult {
113116
const plan = state.plan;
117+
// Keyed by file, matching what actually reran: two timed-out tests in one file
118+
// are one retry, not two.
114119
const retried = plan?.retry
115-
? plan.failures.map((failure) => {
116-
const file = normalizeTestFile(failure.file);
117-
return {
118-
file,
119-
testName: failure.testName,
120-
trackingIssue:
121-
CONTENTION_RETRY_FILES.find((entry) => entry.file === file)?.trackingIssue ?? '',
122-
};
123-
})
120+
? plan.files.map((file) => ({
121+
file,
122+
testNames: plan.failures
123+
.filter((failure) => normalizeTestFile(failure.file) === file)
124+
.map((failure) => failure.testName),
125+
trackingIssue:
126+
CONTENTION_RETRY_FILES.find((entry) => entry.file === file)?.trackingIssue ?? '',
127+
}))
124128
: [];
125129
return {
126130
ok: state.ok,

0 commit comments

Comments
 (0)