mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-10 23:19:09 +02:00
evals: parent-side shard skipping — a one-test diff runs 3 of 44 shards
The sharded runner spawned every shard regardless of diff; only the
child self-skipped, so a typical single-skill change still paid 44 Bun
boots + container-equivalent setup for shards with zero selected tests.
The parent now computes selection once (mirroring e2e-helpers exactly:
EVALS_ALL -> run-all, empty union -> run-all, git errors propagate the
fail-closed throw) and drops shards where no selected test name maps in.
Mapping = quoted E2E map keys in the file's source UNION keys whose dep
list registers the file (constructed-name families need the second
direction). FAIL-OPEN everywhere it matters: run-all, non-skill-e2e
files, unreadable source, zero mapped names all keep the shard — the
child filter stays authoritative, so a parent bug can only run extra.
New taxonomy status skipped-by-diff (never conflated with
never-started); selection banner prints once; --list is selection-aware.
C6 lands in the same commit: a HARD tier-alignment test — every paid
skill-e2e file must be parent-mappable or provably fail-open-safe.
Note: this change-set's 14 dep-list registrations in touchfiles-data.ts
rode along in f945c841 (concurrent-agent staging); they belong to this
change logically.
Demo: selection of one test -> 'running 3 of 44 shards, 41
skipped-by-diff'. 13 new $0 tests via injected seams.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
0337aaf05d
commit
9b9623e30d
@@ -20,6 +20,8 @@ import { describe, test, expect } from 'bun:test';
|
||||
import { readdirSync, readFileSync } from 'fs';
|
||||
import * as path from 'path';
|
||||
import { E2E_TOUCHFILES, E2E_TIERS, LLM_JUDGE_TOUCHFILES } from './helpers/touchfiles';
|
||||
import { isPaidTestFile } from './helpers/paid-test-set';
|
||||
import { knownTestNamesInSource, PARENT_MAPPER_TEST_NAMES } from '../scripts/test-paid-shards';
|
||||
|
||||
const TEST_DIR = import.meta.dir;
|
||||
// Both quote styles — a mechanical refactor to double quotes must not
|
||||
@@ -88,4 +90,47 @@ describe('E2E tier alignment (touchfiles declaration vs test self-gate)', () =>
|
||||
|
||||
expect(misaligned).toEqual([]);
|
||||
});
|
||||
|
||||
// HARD invariant (C6): the paid sharded runner skips a skill-e2e shard when
|
||||
// none of the file's MAPPED test names (E2E map keys quoted in its source,
|
||||
// union E2E map keys whose dep list registers the file) are diff-selected.
|
||||
// A skill-e2e file the mapper cannot see at all is only safe if it provably
|
||||
// opts out of name-based selection: it must not touch the e2e-helpers
|
||||
// selection surface (describeIfSelected / runSkillTest / selectedTests) AND
|
||||
// it must carry an explicit whole-file EVALS_TIER self-gate (the child-side
|
||||
// gate that makes the parent's fail-open keep semantically correct).
|
||||
//
|
||||
// Anything else is an invisible-test-names hole: the parent could drop a
|
||||
// shard whose child would have run real work. Fix by either quoting the
|
||||
// test's E2E map key as a string literal in the file, or adding the file's
|
||||
// path to its key's dep list in test/helpers/touchfiles-data.ts.
|
||||
test('every paid skill-e2e file is visible to the parent diff mapper (or provably fail-open-safe)', () => {
|
||||
const invisible: string[] = [];
|
||||
|
||||
for (const file of testFiles) {
|
||||
const repoPath = `test/${file}`;
|
||||
if (!isPaidTestFile(repoPath)) continue;
|
||||
const content = readFileSync(path.join(TEST_DIR, file), 'utf-8');
|
||||
|
||||
const quoted = knownTestNamesInSource(content, PARENT_MAPPER_TEST_NAMES);
|
||||
const registered = Object.keys(E2E_TOUCHFILES).filter((k) => E2E_TOUCHFILES[k].includes(repoPath));
|
||||
if (quoted.length + registered.length > 0) continue; // parent-mappable
|
||||
|
||||
const usesNameSelection = /\b(describeIfSelected|runSkillTest|selectedTests)\b/.test(content);
|
||||
const selfGated = /EVALS_TIER\s*===\s*['"](gate|periodic)['"]/.test(content);
|
||||
if (!usesNameSelection && selfGated) continue; // fail-open-safe standalone
|
||||
|
||||
invisible.push(
|
||||
`${repoPath}: invisible to the parent diff mapper — no E2E map key quoted in the file, `
|
||||
+ 'not registered in any E2E_TOUCHFILES dep list, and it '
|
||||
+ (usesNameSelection
|
||||
? 'uses name-based selection (describeIfSelected/runSkillTest/selectedTests)'
|
||||
: 'has no whole-file EVALS_TIER self-gate')
|
||||
+ '. Quote the test\'s E2E map key as a string literal, or add this file path to its '
|
||||
+ 'key\'s dep list in test/helpers/touchfiles-data.ts.',
|
||||
);
|
||||
}
|
||||
|
||||
expect(invisible).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,10 +14,16 @@ import {
|
||||
PAID_TEST_GLOBS,
|
||||
classifyPaidTestFile,
|
||||
collectPaidTestFiles,
|
||||
computePaidDiffSelection,
|
||||
diffSkipDecisionForFile,
|
||||
formatSummary,
|
||||
isPaidTestFile,
|
||||
knownTestNamesInSource,
|
||||
partitionShardsByDiffSelection,
|
||||
planPaidShards,
|
||||
runPaidShards,
|
||||
summarize,
|
||||
summaryExitCode,
|
||||
type ShardOutcome,
|
||||
} from '../scripts/test-paid-shards';
|
||||
|
||||
@@ -115,3 +121,140 @@ describe('shard execution', () => {
|
||||
expect(summary).toMatchObject({ total: 2, executed: 1, passed: 1, neverStarted: 1 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('parent-side diff shard skipping', () => {
|
||||
const ALL_NAMES = ['alpha-test', 'beta-test', 'gamma-registered'];
|
||||
const TOUCHFILES: Record<string, string[]> = {
|
||||
'alpha-test': ['a/**'],
|
||||
'beta-test': ['b/**'],
|
||||
'gamma-registered': ['g/**', 'test/skill-e2e-gamma.test.ts'],
|
||||
};
|
||||
const SOURCES: Record<string, string> = {
|
||||
'test/skill-e2e-alpha.test.ts': "runSkillTest('alpha-test', async () => {});",
|
||||
'test/skill-e2e-beta.test.ts': 'describeIfSelected("beta", ["beta-test"], () => {});',
|
||||
// Constructed testName — invisible by quotes, mapped only via registration.
|
||||
'test/skill-e2e-gamma.test.ts': 'const name = buildName(); test(name, async () => {});',
|
||||
// No recognizable names, no registration — the fail-open class.
|
||||
'test/skill-e2e-opaque.test.ts': "const shouldRun = process.env.EVALS_TIER === 'periodic';",
|
||||
'test/codex-e2e.test.ts': 'codex tests keyed off CODEX_E2E_TOUCHFILES',
|
||||
};
|
||||
const opts = {
|
||||
readSource: (file: string) => {
|
||||
if (!(file in SOURCES)) throw new Error(`unreadable: ${file}`);
|
||||
return SOURCES[file];
|
||||
},
|
||||
allNames: ALL_NAMES,
|
||||
e2eTouchfiles: TOUCHFILES,
|
||||
};
|
||||
|
||||
test('knownTestNamesInSource matches only exact quoted strings', () => {
|
||||
expect(knownTestNamesInSource("x 'alpha-test' y", ['alpha-test', 'beta-test'])).toEqual(['alpha-test']);
|
||||
expect(knownTestNamesInSource('x "beta-test" y', ['alpha-test', 'beta-test'])).toEqual(['beta-test']);
|
||||
expect(knownTestNamesInSource('`alpha-test`', ['alpha-test'])).toEqual(['alpha-test']);
|
||||
// Substring inside a longer quoted string is not a hit.
|
||||
expect(knownTestNamesInSource("'alpha-test-extended'", ['alpha-test'])).toEqual([]);
|
||||
});
|
||||
|
||||
test('selected name in file → shard kept', () => {
|
||||
const d = diffSkipDecisionForFile('test/skill-e2e-alpha.test.ts', new Set(['alpha-test']), opts);
|
||||
expect(d.kept).toBe(true);
|
||||
expect(d.reason).toContain('alpha-test');
|
||||
});
|
||||
|
||||
test('no selected names in file → skipped-by-diff', () => {
|
||||
const d = diffSkipDecisionForFile('test/skill-e2e-beta.test.ts', new Set(['alpha-test']), opts);
|
||||
expect(d.kept).toBe(false);
|
||||
expect(d.reason).toContain('mapped test(s)');
|
||||
});
|
||||
|
||||
test('dep-list registration maps files with constructed test names', () => {
|
||||
const selected = diffSkipDecisionForFile('test/skill-e2e-gamma.test.ts', new Set(['gamma-registered']), opts);
|
||||
expect(selected.kept).toBe(true);
|
||||
const unselected = diffSkipDecisionForFile('test/skill-e2e-gamma.test.ts', new Set(['alpha-test']), opts);
|
||||
expect(unselected.kept).toBe(false);
|
||||
});
|
||||
|
||||
test('FAIL-OPEN: unmapped file kept, child self-skip authoritative', () => {
|
||||
const d = diffSkipDecisionForFile('test/skill-e2e-opaque.test.ts', new Set(['alpha-test']), opts);
|
||||
expect(d.kept).toBe(true);
|
||||
expect(d.reason).toContain('fail-open');
|
||||
});
|
||||
|
||||
test('FAIL-OPEN: unreadable source kept', () => {
|
||||
const d = diffSkipDecisionForFile('test/skill-e2e-missing.test.ts', new Set(['alpha-test']), opts);
|
||||
expect(d.kept).toBe(true);
|
||||
expect(d.reason).toContain('fail-open');
|
||||
});
|
||||
|
||||
test('FAIL-OPEN: non-skill-e2e paid files always kept', () => {
|
||||
const d = diffSkipDecisionForFile('test/codex-e2e.test.ts', new Set(['alpha-test']), opts);
|
||||
expect(d.kept).toBe(true);
|
||||
expect(d.reason).toContain('non-skill-e2e');
|
||||
});
|
||||
|
||||
test('run-all selection (null) bypasses skipping entirely', () => {
|
||||
const shards = [['test/skill-e2e-alpha.test.ts'], ['test/skill-e2e-beta.test.ts']];
|
||||
const { runnable, skipped } = partitionShardsByDiffSelection(shards, null, opts);
|
||||
expect(runnable).toEqual(shards);
|
||||
expect(skipped).toEqual([]);
|
||||
});
|
||||
|
||||
test('EVALS_ALL=1 yields run-all selection (no git consulted)', () => {
|
||||
const selection = computePaidDiffSelection({ EVALS_ALL: '1' } as NodeJS.ProcessEnv);
|
||||
expect(selection.selectedNames).toBeNull();
|
||||
expect(selection.reason).toContain('EVALS_ALL=1');
|
||||
expect(selection.totalTests).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test('partition drops only all-skippable shards', () => {
|
||||
const shards = [
|
||||
['test/skill-e2e-alpha.test.ts'],
|
||||
['test/skill-e2e-beta.test.ts'],
|
||||
['test/skill-e2e-opaque.test.ts'],
|
||||
['test/codex-e2e.test.ts'],
|
||||
];
|
||||
const { runnable, skipped } = partitionShardsByDiffSelection(shards, new Set(['alpha-test']), opts);
|
||||
expect(runnable).toEqual([
|
||||
['test/skill-e2e-alpha.test.ts'],
|
||||
['test/skill-e2e-opaque.test.ts'],
|
||||
['test/codex-e2e.test.ts'],
|
||||
]);
|
||||
expect(skipped.length).toBe(1);
|
||||
expect(skipped[0].files).toEqual(['test/skill-e2e-beta.test.ts']);
|
||||
});
|
||||
|
||||
test('taxonomy: skipped-by-diff counted separately, never conflated with never-started', () => {
|
||||
const summary = summarize([
|
||||
{ shard: 1, files: ['a'], status: 'passed', exitCode: 0, elapsedMs: 1, groupPid: 1 },
|
||||
{ shard: 2, files: ['b'], status: 'skipped-by-diff', exitCode: null, elapsedMs: 0, groupPid: null },
|
||||
{ shard: 3, files: ['c'], status: 'never-started', exitCode: null, elapsedMs: 0, groupPid: null },
|
||||
]);
|
||||
expect(summary).toMatchObject({
|
||||
total: 3, executed: 1, passed: 1, skippedByDiff: 1, neverStarted: 1,
|
||||
});
|
||||
const lines = formatSummary(summary);
|
||||
expect(lines[1]).toContain('1 skipped by diff');
|
||||
expect(lines[1]).toContain('1 never started');
|
||||
expect(lines.some((l) => l.includes('skipped-by-diff') && l.includes('b'))).toBe(true);
|
||||
});
|
||||
|
||||
test('exit code ignores skipped-by-diff shards (they are successes)', () => {
|
||||
const allGood = summarize([
|
||||
{ shard: 1, files: ['a'], status: 'passed', exitCode: 0, elapsedMs: 1, groupPid: 1 },
|
||||
{ shard: 2, files: ['b'], status: 'skipped-by-diff', exitCode: null, elapsedMs: 0, groupPid: null },
|
||||
]);
|
||||
expect(summaryExitCode(allGood)).toBe(0);
|
||||
|
||||
const withFailure = summarize([
|
||||
{ shard: 1, files: ['a'], status: 'failed', exitCode: 1, elapsedMs: 1, groupPid: 1 },
|
||||
{ shard: 2, files: ['b'], status: 'skipped-by-diff', exitCode: null, elapsedMs: 0, groupPid: null },
|
||||
]);
|
||||
expect(summaryExitCode(withFailure)).toBe(1);
|
||||
|
||||
const withNeverStarted = summarize([
|
||||
{ shard: 1, files: ['a'], status: 'never-started', exitCode: null, elapsedMs: 0, groupPid: null },
|
||||
{ shard: 2, files: ['b'], status: 'skipped-by-diff', exitCode: null, elapsedMs: 0, groupPid: null },
|
||||
]);
|
||||
expect(summaryExitCode(withNeverStarted)).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user