diff --git a/scripts/test-paid-shards.ts b/scripts/test-paid-shards.ts index b27fa22d8..af9bcdfaf 100644 --- a/scripts/test-paid-shards.ts +++ b/scripts/test-paid-shards.ts @@ -393,6 +393,24 @@ export interface ShardOutcome { groupPid: number | null; /** Tests bun reported executing ("Ran N tests ..."), null when unknown. */ executedTests: number | null; + /** Tests bun reported skipping (" N skip" count line), null when unknown. + * "Ran N tests" COUNTS skips, so executedTests alone cannot distinguish a + * shard that verified work from one whose every test self-skipped — + * codex/gemini files green-by-skip on every CI runner (no binary) and the + * weekly census read them as covered. */ + skippedTests: number | null; +} + +/** + * True when a shard "passed" without verifying anything: every test bun ran + * was a skip. Legitimate for external-service files on hosts without the + * binary, but it must surface as a census warning, never read as coverage. + */ +export function isAllSkippedPass(outcome: Pick): boolean { + return outcome.status === 'passed' + && outcome.executedTests !== null + && outcome.executedTests > 0 + && outcome.skippedTests === outcome.executedTests; } export interface ShardCommand { @@ -562,7 +580,8 @@ export async function runPaidShard( const executedTests = summary.terminalTestCounts.length > 0 ? summary.terminalTestCounts.reduce((a, b) => a + b, 0) : null; - return { shard: shardNumber, files, status, exitCode, elapsedMs, groupPid, executedTests }; + const skippedTests = summary.terminalTestCounts.length > 0 ? summary.skippedTests : null; + return { shard: shardNumber, files, status, exitCode, elapsedMs, groupPid, executedTests, skippedTests }; } export interface RunSummary { @@ -636,6 +655,7 @@ export async function runPaidShards( elapsedMs: 0, groupPid: null, executedTests: null, + skippedTests: null, })); let next = 0; @@ -659,6 +679,7 @@ export async function runPaidShards( elapsedMs: 0, groupPid: null, executedTests: null, + skippedTests: null, }; console.error(`[test:paid] shard ${index + 1} could not run: ${error instanceof Error ? error.message : String(error)}`); } @@ -678,9 +699,14 @@ export function formatSummary(summary: RunSummary): string[] { + `${summary.skippedByDiff} skipped by diff`, ]; for (const outcome of summary.outcomes) { + // A pass whose every test skipped is labeled distinctly: it exited 0 but + // verified NOTHING (codex/gemini files on hosts without the binary). + // Status stays 'passed' — availability of an external service is not a + // repo regression — but the census must never read it as coverage. + const allSkipped = isAllSkippedPass(outcome) ? ` ⚠ all ${outcome.executedTests} tests SKIPPED — verified nothing` : ''; lines.push( ` ${outcome.status.padEnd(15)} ${String(Math.round(outcome.elapsedMs / 1000)).padStart(5)}s ` - + outcome.files.join(' '), + + outcome.files.join(' ') + allSkipped, ); } return lines; @@ -786,7 +812,7 @@ export interface SliceResult { tier: PaidTier; sliceIndex: number; sliceCount: number; - outcomes: Array>; + outcomes: Array>; } /** @@ -970,6 +996,18 @@ async function main(): Promise { for (const problem of verdict.problems) console.error(` ✗ ${problem}`); return 1; } + // Census honesty: a 'passed' shard whose every test skipped verified + // nothing (external-service binary absent on the runner). Not a failure — + // service availability is host state, not a repo regression — but the + // report must say so, or the weekly lane reads codex/gemini as covered + // on runners that never install them. + const allSkipped = results.flatMap((r) => r.outcomes.filter(isAllSkippedPass)); + if (allSkipped.length > 0) { + console.log(`[test:paid] report: ⚠ ${allSkipped.length} shard(s) passed with EVERY test skipped — they verified nothing:`); + for (const outcome of allSkipped) { + console.log(` ⚠ ${outcome.files.join(' ')} (${outcome.executedTests} skipped — external service missing or tier mismatch)`); + } + } console.log('[test:paid] report: every planned shard accounted and passed'); return 0; } @@ -1023,8 +1061,8 @@ async function main(): Promise { tier: manifest.tier, sliceIndex: options.sliceIndex, sliceCount: manifest.sliceCount, - outcomes: guarded.map(({ files, status, exitCode, elapsedMs, executedTests }) => - ({ files, status, exitCode, elapsedMs, executedTests })), + outcomes: guarded.map(({ files, status, exitCode, elapsedMs, executedTests, skippedTests }) => + ({ files, status, exitCode, elapsedMs, executedTests, skippedTests })), }; fs.mkdirSync(evalDirBase, { recursive: true }); const sliceResultPath = path.join(evalDirBase, `slice-${options.sliceIndex}.json`); @@ -1102,6 +1140,7 @@ async function main(): Promise { elapsedMs: 0, groupPid: null, executedTests: null, + skippedTests: null, })); const guardedOutcomes = applyHollowShardGuard(runSummary.outcomes, { evalsAll: process.env.EVALS_ALL === '1', diff --git a/scripts/test-strict-output.ts b/scripts/test-strict-output.ts index 2e91bd096..cfa41e63f 100644 --- a/scripts/test-strict-output.ts +++ b/scripts/test-strict-output.ts @@ -20,6 +20,15 @@ const ANSI_ESCAPE = /\u001B\[[0-?]*[ -/]*[@-~]/g; const BUN_FAIL_RESULT = /^\(fail\) .+ \[(?:\d+(?:\.\d+)?)(?:ns|us|µs|ms|s)\]$/; const BUN_BETWEEN_TESTS_ERROR = '# Unhandled error between tests'; const BUN_TERMINAL_SUMMARY = /^Ran (\d+) tests? across (\d+) files?\. \[(?:\d+(?:\.\d+)?)(?:ns|us|µs|ms|s)\]$/; +// The counts block bun prints just before the terminal summary (" 1 pass", +// " 2 skip", " 0 fail"). "Ran N tests" COUNTS skipped tests, so N alone +// cannot distinguish a shard that verified work from one whose every test +// self-skipped (external-service binary missing, tier mismatch) — the +// green-by-skip class. Anchored to whole-line matches; nested bun-test +// children can still contribute counts (same known limit as the terminal +// summary — see the last-summary-anchoring TODO in the audit). +const BUN_SKIP_COUNT = /^\s*(\d+) skip$/; +const BUN_PASS_COUNT = /^\s*(\d+) pass$/; export type BunTestOutputFinding = 'failed-test' | 'unhandled-between-tests'; @@ -29,6 +38,11 @@ export interface BunTestOutputSummary { terminalFileCounts: number[]; /** Test counts from the same terminal lines — feeds the hollow-shard guard. */ terminalTestCounts: number[]; + /** Sum of bun's " N skip" count lines. "Ran N tests" includes skips, so + * this is what separates verified work from green-by-skip. */ + skippedTests: number; + /** Sum of bun's " N pass" count lines. */ + passedTests: number; } export type ForwardedTerminationSignal = 'SIGINT' | 'SIGTERM'; @@ -229,6 +243,8 @@ export class BunTestOutputClassifier { private unhandledBetweenTests = 0; private terminalFileCounts: number[] = []; private terminalTestCounts: number[] = []; + private skippedTests = 0; + private passedTests = 0; write(chunk: Uint8Array | string, origin: ClassifierOrigin = 'stdout'): void { this.pending[origin] += typeof chunk === 'string' @@ -252,6 +268,8 @@ export class BunTestOutputClassifier { unhandledBetweenTests: this.unhandledBetweenTests, terminalFileCounts: [...this.terminalFileCounts], terminalTestCounts: [...this.terminalTestCounts], + skippedTests: this.skippedTests, + passedTests: this.passedTests, }; } @@ -268,6 +286,11 @@ export class BunTestOutputClassifier { const finding = classifyBunTestOutputLine(line); if (finding === 'failed-test') this.failedTests += 1; if (finding === 'unhandled-between-tests') this.unhandledBetweenTests += 1; + const stripped = stripAnsiLine(line); + const skip = BUN_SKIP_COUNT.exec(stripped); + if (skip !== null) this.skippedTests += Number.parseInt(skip[1], 10); + const pass = BUN_PASS_COUNT.exec(stripped); + if (pass !== null) this.passedTests += Number.parseInt(pass[1], 10); const terminal = parseBunTerminalSummary(line); if (terminal !== null) { this.terminalFileCounts.push(terminal.files); diff --git a/test/paid-shards.test.ts b/test/paid-shards.test.ts index e64cb8532..1c91da24f 100644 --- a/test/paid-shards.test.ts +++ b/test/paid-shards.test.ts @@ -22,6 +22,7 @@ import { computePaidDiffSelection, diffSkipDecisionForFile, formatSummary, + isAllSkippedPass, isPaidTestFile, knownTestNamesInSource, partitionShardsByDiffSelection, @@ -354,3 +355,39 @@ describe('parent-side diff shard skipping', () => { expect(summaryExitCode(withNeverStarted)).toBe(1); }); }); + +// Green-by-skip census: "Ran N tests" counts skips, so a codex/gemini file +// whose every test self-skipped (binary absent on the runner) exits 0 and +// used to read as coverage in the weekly report. The census label keeps the +// pass (service availability is host state, not a repo regression) but must +// say the shard verified nothing. +describe('all-skipped pass census', () => { + const base = { shard: 1, files: ['test/codex-e2e.test.ts'], exitCode: 0, elapsedMs: 1200, groupPid: 1 }; + + test('isAllSkippedPass: pass with every test skipped → true', () => { + expect(isAllSkippedPass({ ...base, status: 'passed', executedTests: 8, skippedTests: 8 } as ShardOutcome)).toBe(true); + }); + + test('isAllSkippedPass: real work, a failure, or no data → false', () => { + // one test actually ran + expect(isAllSkippedPass({ ...base, status: 'passed', executedTests: 8, skippedTests: 7 } as ShardOutcome)).toBe(false); + // zero tests: that's the hollow-shard guard's territory, not this label's + expect(isAllSkippedPass({ ...base, status: 'passed', executedTests: 0, skippedTests: 0 } as ShardOutcome)).toBe(false); + // non-pass statuses never get the label + expect(isAllSkippedPass({ ...base, status: 'failed', executedTests: 8, skippedTests: 8 } as ShardOutcome)).toBe(false); + // stream gave no counts (crash/timeout) — unknown, not all-skipped + expect(isAllSkippedPass({ ...base, status: 'passed', executedTests: null, skippedTests: null } as ShardOutcome)).toBe(false); + }); + + test('formatSummary labels an all-skipped pass and leaves real passes alone', () => { + const lines = formatSummary(summarize([ + { ...base, status: 'passed', executedTests: 8, skippedTests: 8 } as ShardOutcome, + { shard: 2, files: ['test/skill-e2e-review.test.ts'], status: 'passed', exitCode: 0, elapsedMs: 900, groupPid: 2, executedTests: 3, skippedTests: 0 } as ShardOutcome, + ])); + const codexLine = lines.find((l) => l.includes('codex-e2e')); + const reviewLine = lines.find((l) => l.includes('skill-e2e-review')); + expect(codexLine).toContain('all 8 tests SKIPPED'); + expect(codexLine).toContain('verified nothing'); + expect(reviewLine).not.toContain('SKIPPED'); + }); +}); diff --git a/test/strict-output.test.ts b/test/strict-output.test.ts index 832283081..a059dd9d5 100644 --- a/test/strict-output.test.ts +++ b/test/strict-output.test.ts @@ -89,6 +89,38 @@ describe('BunTestOutputClassifier', () => { expect(summary.terminalFileCounts).toEqual([2]); expect(strictTestExitCode(0, summary, 2)).toBe(0); }); + + // "Ran N tests" COUNTS skipped tests, so the skip/pass count lines are the + // only stream evidence separating verified work from green-by-skip (a + // codex/gemini file whose every test self-skips on a binary-less runner). + it('parses the skip and pass count lines from bun’s recap block', () => { + const c = new BunTestOutputClassifier(); + c.write(' 1 pass\n 2 skip\n 0 fail\nRan 3 tests across 1 file. [7.00ms]\n'); + const summary = c.end(); + expect(summary.passedTests).toBe(1); + expect(summary.skippedTests).toBe(2); + expect(summary.terminalTestCounts).toEqual([3]); + // all-skipped is still exit-0 at the classifier layer — the census + // labeling happens in the paid runner, not here + expect(strictTestExitCode(0, summary, 1)).toBe(0); + }); + + it('skip/pass counts survive ANSI color and chunk shears', () => { + const c = new BunTestOutputClassifier(); + c.write(' 4 pa'); + c.write('ss\n 9 skip\n'); + const summary = c.end(); + expect(summary.passedTests).toBe(4); + expect(summary.skippedTests).toBe(9); + }); + + it('prose mentioning skip counts does not pollute the tally', () => { + const c = new BunTestOutputClassifier(); + c.write('console.log said: 7 skip is what we expect later\n'); + c.write('(fail) 3 skip handling [1.00ms]\n'); + const summary = c.end(); + expect(summary.skippedTests).toBe(0); + }); }); describe('installChildSignalForwarding — cancellation terminates the RUN', () => {