From 7e1061c6dd3b4d8dbaf80924d525dd42945746cc Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 29 Aug 2026 05:42:57 +0000 Subject: [PATCH] fix(test-runner): flaky-retry vetoes on ANY unattributable failure evidence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate equated 'some failure attributed' with 'all failures attributed': a shard with one attributed failure plus a headerless failure, an unhandled error between tests, or a truncated run (no terminal summary) qualified for retry — re-running only failingFiles and masking the rest as FLAKY-PASS, re-opening the silent-truncation hole the strict classifier closes. FreeShardOutcome now carries unattributedFailures; nonzero vetoes the retry. Pins: mixed shard, truncated-with-attributed shard, empty-shard field values. Co-Authored-By: Claude Fable 5 --- scripts/test-free-shards.ts | 24 ++++++++++++++--- test/test-free-shards-sandbox-knobs.test.ts | 30 +++++++++++++++++++++ test/test-free-shards.test.ts | 4 +++ 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/scripts/test-free-shards.ts b/scripts/test-free-shards.ts index 2615cec21..11187876c 100755 --- a/scripts/test-free-shards.ts +++ b/scripts/test-free-shards.ts @@ -945,6 +945,15 @@ export interface FreeShardOutcome { * meaningless without knowing what to re-run). */ failingFiles: string[]; + /** + * Count of failure evidence the retry pass CANNOT re-run by file: fail + * lines seen before any file-chunk header, unhandled errors between tests, + * and a truncated run (no terminal summary). Nonzero vetoes the flaky + * retry for the whole run — retrying only failingFiles would re-run a + * subset and mask the rest as a FLAKY-PASS, re-opening the silent-truncation + * hole the strict classifier exists to close. + */ + unattributedFailures: number; } export interface ShardCommand { @@ -1023,7 +1032,7 @@ export async function runFreeShard( // so an unoccupied index must not fail or shift work to a different runner. if (files.length === 0) { const outcome: FreeShardOutcome = { - shard: shardNumber, files: [], status: 'passed', exitCode: 0, elapsedMs: 0, groupPid: null, failingFiles: [], + shard: shardNumber, files: [], status: 'passed', exitCode: 0, elapsedMs: 0, groupPid: null, failingFiles: [], unattributedFailures: 0, }; log(shardEpilogue(outcome, totalShards)); return outcome; @@ -1160,8 +1169,12 @@ export async function runFreeShard( ...report.failures.map((f) => f.file).filter((f): f is string => !!f), ...report.crashedFiles, ])]; + const unattributedFailures = status === 'passed' ? 0 + : report.failures.filter((f) => !f.file).length + + report.unhandledErrors.length + + (report.sawTerminalSummary ? 0 : 1); const outcome: FreeShardOutcome = { - shard: shardNumber, files, status, exitCode, elapsedMs: Date.now() - startedAt, groupPid, failingFiles, + shard: shardNumber, files, status, exitCode, elapsedMs: Date.now() - startedAt, groupPid, failingFiles, unattributedFailures, }; log(shardEpilogue(outcome, totalShards)); for (const line of buildRunEpilogue(status, report, outcome.elapsedMs, logPath)) log(line); @@ -1305,7 +1318,12 @@ async function main(): Promise { ) { const flakyFiles = [...new Set(outcomes.flatMap((o) => o.failingFiles))] .filter((f): f is string => typeof f === 'string' && f.length > 0); - const allAttributed = outcomes.every((o) => o.status === 'passed' || o.failingFiles.length > 0); + // "Fully attributed" is per-failure, not per-shard: a shard with one + // attributed failure PLUS a headerless failure / unhandled error / + // truncated run must veto the retry — re-running only failingFiles would + // mask the unattributable evidence as a FLAKY-PASS. + const allAttributed = outcomes.every((o) => o.status === 'passed' + || (o.failingFiles.length > 0 && o.unattributedFailures === 0)); if (allAttributed && flakyFiles.length > 0 && flakyFiles.length <= RETRY_CAP) { console.log(`[test:free] flaky-retry: re-running ${flakyFiles.length} failing file(s) once, serially: ${flakyFiles.join(', ')}`); const retryOutcome = await runFreeShard(flakyFiles, totalShards + 1, totalShards + 1, { diff --git a/test/test-free-shards-sandbox-knobs.test.ts b/test/test-free-shards-sandbox-knobs.test.ts index 1ab10bcfc..1de5b81b7 100644 --- a/test/test-free-shards-sandbox-knobs.test.ts +++ b/test/test-free-shards-sandbox-knobs.test.ts @@ -94,6 +94,23 @@ describe('test-free-shards: FreeShardOutcome.failingFiles (flaky-retry feed)', ( const outcome = await runFreeShard(['planted'], 1, 1, { commandFor, quiet: true, log: () => {} }); expect(outcome.status).toBe('failed'); expect(outcome.failingFiles).toEqual(['test/planted.test.ts']); + expect(outcome.unattributedFailures).toBe(0); // fully attributed — retry-eligible + }); + + test('a MIXED shard (one attributed + one headerless failure) is flagged unattributable — retry must not mask the headerless one', async () => { + // The retry gate must not equate "some failure attributed" with "all + // failures attributed": re-running only test/planted.test.ts and passing + // would report the suite green over the headerless failure. + const commandFor = commandPrinting([ + failLine('headerless failure before any file chunk'), + 'test/planted.test.ts:', + failLine('planted failure'), + summary(2, 1), + ]); + const outcome = await runFreeShard(['mixed'], 1, 1, { commandFor, quiet: true, log: () => {} }); + expect(outcome.status).toBe('failed'); + expect(outcome.failingFiles).toEqual(['test/planted.test.ts']); + expect(outcome.unattributedFailures).toBeGreaterThan(0); }); test('failures across two files attribute both; a crashed worker file joins the set deduped', async () => { @@ -128,5 +145,18 @@ describe('test-free-shards: FreeShardOutcome.failingFiles (flaky-retry feed)', ( const outcome = await runFreeShard(['truncated'], 1, 1, { commandFor, quiet: true, log: () => {} }); expect(outcome.status).toBe('failed'); expect(outcome.failingFiles).toEqual([]); + expect(outcome.unattributedFailures).toBeGreaterThan(0); // truncation counts as unattributable evidence + }); + + test('a truncated run WITH an attributed failure is still unattributable — tests after the cut never ran', async () => { + const commandFor = commandPrinting([ + 'test/planted.test.ts:', + failLine('planted failure'), + // no terminal summary: the child died mid-suite + ]); + const outcome = await runFreeShard(['planted', 'neverran'], 1, 1, { commandFor, quiet: true, log: () => {} }); + expect(outcome.status).toBe('failed'); + expect(outcome.failingFiles).toEqual(['test/planted.test.ts']); + expect(outcome.unattributedFailures).toBeGreaterThan(0); }); }); diff --git a/test/test-free-shards.test.ts b/test/test-free-shards.test.ts index 5bfe009e2..e20bd924f 100644 --- a/test/test-free-shards.test.ts +++ b/test/test-free-shards.test.ts @@ -260,6 +260,10 @@ describe('test-free-shards: strict shard execution', () => { log: (l) => lines.push(l), }); expect(outcome.status).toBe('passed'); + // Mutation-caught gap: bun strips types at runtime, so a missing + // failingFiles here feeds undefined into the flaky-retry flatMap. + expect(outcome.failingFiles).toEqual([]); + expect(outcome.unattributedFailures).toBe(0); expect(lines.some((l) => /^\[test:free\] shard 7\/20: 0 files, 0s, pass$/.test(l))).toBe(true); });