mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 06:28:59 +02:00
feat: green-by-skip census — skip counts in the classifier, all-skipped labeling in the paid runner
bun's 'Ran N tests' line COUNTS skipped tests, so a codex/gemini shard whose every test self-skipped (binary absent on the runner — true of every CI runner today) exits 0, dodges the hollow-shard guard, and reads as coverage in the weekly census. The classifier now parses bun's ' N skip' / ' N pass' recap lines; ShardOutcome carries skippedTests; formatSummary and the fail-closed slices report label an all-skipped pass explicitly: 'all N tests SKIPPED — verified nothing'. Status stays 'passed' (external service availability is host state, not a repo regression) but the census can no longer mistake absence for coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
d96027c098
commit
7788f7197e
@@ -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<ShardOutcome, 'status' | 'executedTests' | 'skippedTests'>): 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<Pick<ShardOutcome, 'files' | 'status' | 'exitCode' | 'elapsedMs' | 'executedTests'>>;
|
||||
outcomes: Array<Pick<ShardOutcome, 'files' | 'status' | 'exitCode' | 'elapsedMs' | 'executedTests' | 'skippedTests'>>;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -970,6 +996,18 @@ async function main(): Promise<number> {
|
||||
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<number> {
|
||||
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<number> {
|
||||
elapsedMs: 0,
|
||||
groupPid: null,
|
||||
executedTests: null,
|
||||
skippedTests: null,
|
||||
}));
|
||||
const guardedOutcomes = applyHollowShardGuard(runSummary.outcomes, {
|
||||
evalsAll: process.env.EVALS_ALL === '1',
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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('[32m 4 pa');
|
||||
c.write('ss[0m\n[33m 9 skip[0m\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', () => {
|
||||
|
||||
Reference in New Issue
Block a user