Skip to content

Commit b12a3e3

Browse files
authored
test: pin test files over 1,000 lines at their exact length so they can only shrink (#1843)
* test: pin test files over 1,000 lines at their exact length so they can only shrink AGENTS.md has said for a while that past 1,000 lines is architecture debt and tests are not exempt; nothing enforced it, and the second-largest test file gained 55 lines in the PR before this one. This is the slow-test ratchet's shape for a reader's context instead of wall clock: the 26 test files over the tripwire are pinned at their exact length (R9-style equality pin, #1781 A6); growth fails, shrink fails until the pin is lowered in the same PR, a file that drops under the line leaves the list, and a new file may not cross it. One directory walk per unit run, ~250ms; the pin list emptying deletes it. * test(ratchet): hold giant test files to their merge-base length so pin edits cannot admit growth Review (P1): the equality pin compared measured lengths only against the pin map in the same checkout, so growing a file and raising its pin, or adding a new >1,000-line file with a pin, stayed green. The gate is now history-backed: every test file over the tripwire may be no longer than at the merge-base with origin/main (renames followed; new files may not cross the line), and no pin may exceed its file's base length — one git cat-file --batch spawn, parsed by bytes because the sizes are bytes. Both bypasses planted red against real git on a pinned file and on a fresh 1,001-line file with a pin added. * test(ratchet): a pin on a file at or under the tripwire is itself a finding Review: a new pin for an unchanged sub-tripwire file (900 pinned at 900) passed equality and history and grew the map. Pins now exist only for files over the tripwire — any other pin is red with 'remove it' — which also subsumes the old shrink-under-the-line message. Planted red in-file and against real git (a 186-line test pinned at 186). The android snapshot test pin bootstraps 1636→1660: main grew that file in #1846 before this gate exists, and history agrees (1660 at the merge-base).
1 parent 3f0f706 commit b12a3e3

3 files changed

Lines changed: 347 additions & 1 deletion

File tree

AGENTS.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,9 @@ Module size is about agent context safety, and the unit is questions, not lines:
134134
one question so `rg` → read-whole-file stays one cheap bounded read.
135135

136136
- tripwires: target ≤300 LOC per implementation file; past 500, extract before adding behavior; past
137-
1,000 is architecture debt unless it is generated data or a fixture snapshot. Tests are not exempt.
137+
1,000 is architecture debt unless it is generated data or a fixture snapshot. Tests are not exempt:
138+
test files over 1,000 lines are pinned at their exact length and may only shrink
139+
(`src/__tests__/test-file-size-ratchet.test.ts`).
138140
- name files by the domain concept they answer (`runner-cache.ts`, `interaction-touch-response.ts`),
139141
not by layer leftovers (`utils2.ts`, `common.ts` accretion).
140142
- colocate machine-readable claims with the code they describe — coverage manifests beside contract

docs/agents/testing.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,17 @@ Measured on the full unit suite (340 files, 3,210 tests, 48s wall at ~7x paralle
555555
2.5s, integration 15s, failure at 2x budget (the band between reports without failing — host
556556
load legitimately stretches borderline tests, and a flaky gate trains people to ignore it).
557557
The pin list only shrinks, or grows in the same PR with a justification.
558+
- **The test-file size ratchet** (`src/__tests__/test-file-size-ratchet.test.ts`) is the same
559+
shape for the other resource a giant test file consumes — a reader's context. Every test file
560+
over the 1,000-line tripwire is pinned at its exact length, R9-style: growth fails ("split it
561+
along the source module it mirrors"), shrinking fails until the pin is lowered in the same PR,
562+
a file that drops under the line leaves the list, and a new file may not cross it. The map is
563+
not the authority — git is: every file over the line is also held to its length at the
564+
merge-base with `origin/main` (renames followed), and no pin may exceed its file's base length,
565+
so growing a file and raising its pin, or adding a giant file with a pin, are red against
566+
history; a pin on a file at or under the line is red on its own, so the map cannot grow by
567+
pinning small files at their own length. Adding a test to a pinned file means moving that family out first — the failure names
568+
the file and the fix; never raise a pin. Needs `origin/main` fetched (CI's Coverage job does).
558569
- **Isolation stays ON; pool stays forks — both measured.** `--no-isolate`: 205s wall vs 48s
559570
(module state — timers, memos, singletons — thrashes across files sharing a worker).
560571
`--pool=threads`: no change (50.4s). The ~100s aggregate import overhead is the price of
Lines changed: 333 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,333 @@
1+
import fs from 'node:fs';
2+
import path from 'node:path';
3+
import { expect, test } from 'vitest';
4+
import { walkFiles } from '../../scripts/lib/walk-files.ts';
5+
import { runCmdSync } from '../utils/exec.ts';
6+
7+
/**
8+
* Test-file size ratchet (AGENTS.md "Scope & shape": past 1,000 lines is architecture debt,
9+
* and tests are not exempt; the topology rule says a test file mirrors its source module and
10+
* splits when the source does).
11+
*
12+
* The slow-test ratchet keeps the unit suite's wall clock honest; this one keeps its files
13+
* readable in one bounded read. Every test file over the tripwire is pinned at its exact
14+
* length, R9-style (#1781 A6): growing a pinned file fails ("split it, don't add to it"), and
15+
* shrinking one fails until the pin is lowered, so the list only ever ratchets down. A file
16+
* that drops under the tripwire leaves the list; a new file may not cross it.
17+
*
18+
* The pin map alone could be edited alongside the file (raise a pin and grow into it; add a
19+
* pin with a new giant file), so the gate is history-backed as well: every test file over the
20+
* tripwire may be no longer than it was at the merge-base with origin/main (or no longer than
21+
* the tripwire if it did not exist there), and no pin may exceed its file's base length. Both
22+
* pin-edit bypasses go red against git, not against the map.
23+
*
24+
* Catches: a >1,000-line test file growing (with or without a matching pin edit), or a new one
25+
* appearing (with or without a pin).
26+
* Evidence: 26 test files were over the line when this landed (2026-08-18); the largest,
27+
* `snapshot-handler.test.ts`, gained 55 lines in the PR before, under a rule with no gate.
28+
* Cost: one directory walk and a line count per test file — well under a second.
29+
* Kill criterion: the pin list is empty. Delete this file with the last pin.
30+
*/
31+
32+
const TRIPWIRE_LINES = 1_000;
33+
34+
// Exact current lengths. Lower a pin when its file shrinks; never raise one — extract instead.
35+
const PINNED_TEST_FILE_LINES: Readonly<Record<string, number>> = Object.freeze({
36+
'src/__tests__/remote-connection.test.ts': 2973,
37+
'src/daemon/handlers/__tests__/snapshot-handler.test.ts': 2652,
38+
'src/commands/interaction/runtime/settle.test.ts': 2361,
39+
'src/platforms/apple/core/__tests__/runner-session.test.ts': 2083,
40+
'src/daemon/handlers/__tests__/session-replay-runtime-maestro.test.ts': 2031,
41+
'src/utils/__tests__/daemon-client.test.ts': 1910,
42+
'src/utils/__tests__/output.test.ts': 1861,
43+
'src/platforms/android/__tests__/snapshot.test.ts': 1660,
44+
'src/platforms/apple/core/__tests__/runner-client.test.ts': 1615,
45+
'src/__tests__/client.test.ts': 1598,
46+
'test/integration/provider-scenarios/android-lifecycle.test.ts': 1597,
47+
'src/utils/__tests__/daemon-client-lifecycle.test.ts': 1414,
48+
'src/platforms/apple/core/__tests__/runner-command-retry.test.ts': 1327,
49+
'src/__tests__/cli-client-commands.test.ts': 1317,
50+
'src/__tests__/cli-config.test.ts': 1282,
51+
'src/daemon/handlers/__tests__/find.test.ts': 1237,
52+
'src/platforms/apple/core/__tests__/perf.test.ts': 1222,
53+
'src/mcp/__tests__/command-tools.test.ts': 1218,
54+
'src/daemon/handlers/__tests__/session-replay-divergence.test.ts': 1215,
55+
'src/platforms/apple/core/__tests__/apps.test.ts': 1210,
56+
'src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts': 1208,
57+
'src/daemon/snapshot-presentation/ios/presentation.test.ts': 1201,
58+
'src/daemon/handlers/__tests__/session-replay-target-verification-runtime.test.ts': 1183,
59+
'src/__tests__/client-metro.test.ts': 1105,
60+
'src/__tests__/cli-network.test.ts': 1092,
61+
'src/platforms/android/__tests__/snapshot-helper.test.ts': 1002,
62+
});
63+
64+
const REPO_ROOT = path.resolve(import.meta.dirname, '..', '..');
65+
const TEST_ROOTS = ['src', 'packages', 'test', 'scripts'];
66+
const SKIPPED_DIRECTORIES = new Set(['node_modules', 'dist', 'dist-types', '.tmp']);
67+
const TEST_FILE = /\.test\.(?:ts|tsx|mjs)$/;
68+
69+
function isTestFile(file: string): boolean {
70+
if (!TEST_FILE.test(file)) return false;
71+
return !path
72+
.relative(REPO_ROOT, file)
73+
.split(path.sep)
74+
.some((part) => SKIPPED_DIRECTORIES.has(part));
75+
}
76+
77+
/** Line count as `wc -l` reports it: newline characters. */
78+
function countLines(file: string): number {
79+
let lines = 0;
80+
for (const char of fs.readFileSync(file, 'utf8')) if (char === '\n') lines += 1;
81+
return lines;
82+
}
83+
84+
/** The ratchet decision, separated from the filesystem so the tests below can plant each red. */
85+
function ratchetFindings(
86+
measured: ReadonlyMap<string, number>,
87+
pinned: Readonly<Record<string, number>>,
88+
tripwire: number,
89+
): string[] {
90+
const findings: string[] = [];
91+
for (const [file, lines] of [...measured].sort()) {
92+
const pin = pinned[file];
93+
if (pin === undefined) {
94+
if (lines > tripwire) {
95+
findings.push(
96+
`${file} is ${lines} lines, over the ${tripwire}-line tripwire and not pinned: split it ` +
97+
`along the source module it mirrors (docs/agents/testing.md) rather than pinning it.`,
98+
);
99+
}
100+
continue;
101+
}
102+
if (lines <= tripwire) {
103+
// Pins exist only for files over the tripwire: one on a smaller file grows the map for
104+
// nothing (900 pinned at 900 would satisfy equality and history alike) and defeats the
105+
// only-shrink kill criterion.
106+
findings.push(
107+
`${file} is ${lines} lines, at or under the ${tripwire}-line tripwire, but has a pin (${pin}): remove it — pins are only for files over the tripwire.`,
108+
);
109+
continue;
110+
}
111+
if (lines > pin) {
112+
findings.push(
113+
`${file} grew to ${lines} lines (pinned ${pin}): extract instead of adding to a file over the tripwire.`,
114+
);
115+
} else if (lines < pin) {
116+
findings.push(
117+
`${file} shrank to ${lines} lines (pinned ${pin}): lower its pin in this PR so the ratchet keeps the gain.`,
118+
);
119+
}
120+
}
121+
for (const file of Object.keys(pinned)) {
122+
if (!measured.has(file)) {
123+
findings.push(`${file} is pinned but does not exist: remove its pin.`);
124+
}
125+
}
126+
return findings;
127+
}
128+
129+
/**
130+
* Line counts of the given repo paths at the merge-base with origin/main, following renames, in
131+
* one `git cat-file --batch` spawn. `undefined` = the file did not exist there.
132+
*/
133+
function baseLineCounts(paths: readonly string[]): ReadonlyMap<string, number | undefined> {
134+
const mergeBase = runCmdSync('git', ['merge-base', 'origin/main', 'HEAD'], {
135+
cwd: REPO_ROOT,
136+
allowFailure: true,
137+
});
138+
if (mergeBase.exitCode !== 0) {
139+
throw new Error(
140+
'test-file size ratchet needs origin/main to read base lengths (git merge-base origin/main HEAD failed): ' +
141+
`${mergeBase.stderr.trim()}. Fetch origin/main; the gate does not skip.`,
142+
);
143+
}
144+
const base = mergeBase.stdout.trim();
145+
const renamedFrom = new Map<string, string>();
146+
const renames = runCmdSync(
147+
'git',
148+
['diff', '--name-status', '--find-renames', '--diff-filter=R', base, 'HEAD', '--', '*.test.*'],
149+
{ cwd: REPO_ROOT },
150+
);
151+
for (const line of renames.stdout.split('\n')) {
152+
const [, from, to] = line.split('\t');
153+
if (from && to) renamedFrom.set(to, from);
154+
}
155+
const requests = paths.map((file) => `${base}:${renamedFrom.get(file) ?? file}`);
156+
const batch = runCmdSync('git', ['cat-file', '--batch'], {
157+
cwd: REPO_ROOT,
158+
stdin: `${requests.join('\n')}\n`,
159+
binaryStdout: true,
160+
maxBuffer: 256 * 1024 * 1024,
161+
});
162+
return parseCatFileBatch(batch.stdoutBuffer ?? Buffer.alloc(0), paths);
163+
}
164+
165+
/**
166+
* `<sha> blob <size>\n<size bytes>\n` per hit, `<request> missing\n` per miss, in request order.
167+
* Sizes are bytes, so this walks the raw buffer: a string offset drifts after the first file with
168+
* a multi-byte character (every test file with an em dash).
169+
*/
170+
function parseCatFileBatch(
171+
output: Buffer,
172+
paths: readonly string[],
173+
): ReadonlyMap<string, number | undefined> {
174+
const counts = new Map<string, number | undefined>();
175+
let offset = 0;
176+
for (const file of paths) {
177+
const headerEnd = output.indexOf(0x0a, offset);
178+
const header = output.subarray(offset, headerEnd).toString('utf8');
179+
offset = headerEnd + 1;
180+
const blob = /^\S+ blob (\d+)$/.exec(header);
181+
if (!blob) {
182+
counts.set(file, undefined); // "<request> missing"
183+
continue;
184+
}
185+
const size = Number(blob[1]);
186+
let lines = 0;
187+
for (let index = offset; index < offset + size; index += 1) {
188+
if (output[index] === 0x0a) lines += 1;
189+
}
190+
offset += size + 1;
191+
counts.set(file, lines);
192+
}
193+
return counts;
194+
}
195+
196+
/**
197+
* The history-backed half: measured against the merge-base, not against the pin map, so
198+
* editing the map alongside the file cannot admit growth.
199+
*/
200+
function historyFindings(
201+
measured: ReadonlyMap<string, number>,
202+
pinned: Readonly<Record<string, number>>,
203+
baseLines: ReadonlyMap<string, number | undefined>,
204+
tripwire: number,
205+
): string[] {
206+
const findings: string[] = [];
207+
for (const [file, lines] of [...measured].sort()) {
208+
if (lines <= tripwire) continue;
209+
const base = baseLines.get(file);
210+
if (base === undefined) {
211+
findings.push(
212+
`${file} is ${lines} lines and did not exist at the merge-base: a new test file may not cross the ${tripwire}-line tripwire, pinned or not.`,
213+
);
214+
} else if (lines > Math.max(base, tripwire)) {
215+
findings.push(
216+
`${file} is ${lines} lines, ${base} at the merge-base: a test file over the tripwire may not grow, whatever its pin says.`,
217+
);
218+
}
219+
}
220+
for (const [file, pin] of Object.entries(pinned).sort()) {
221+
const base = baseLines.get(file);
222+
if (base !== undefined && pin > base) {
223+
findings.push(
224+
`${file} is pinned at ${pin} but was ${base} lines at the merge-base: a pin may not be raised above its file's base length.`,
225+
);
226+
}
227+
}
228+
return findings;
229+
}
230+
231+
test('no test file over the tripwire grows, and every pin matches its file exactly', () => {
232+
const measured = new Map<string, number>();
233+
for (const root of TEST_ROOTS) {
234+
for (const file of walkFiles(path.join(REPO_ROOT, root), isTestFile)) {
235+
measured.set(path.relative(REPO_ROOT, file).split(path.sep).join('/'), countLines(file));
236+
}
237+
}
238+
expect(measured.size).toBeGreaterThan(500);
239+
expect(ratchetFindings(measured, PINNED_TEST_FILE_LINES, TRIPWIRE_LINES)).toEqual([]);
240+
241+
const ofInterest = [
242+
...new Set([
243+
...[...measured].filter(([, lines]) => lines > TRIPWIRE_LINES).map(([file]) => file),
244+
...Object.keys(PINNED_TEST_FILE_LINES),
245+
]),
246+
];
247+
const baseLines = baseLineCounts(ofInterest);
248+
expect(historyFindings(measured, PINNED_TEST_FILE_LINES, baseLines, TRIPWIRE_LINES)).toEqual([]);
249+
});
250+
251+
test('planted reds: growth, shrink, unpinned crossing, and a stale pin each name their fix', () => {
252+
const pinned = { 'a.test.ts': 1200, 'b.test.ts': 1500, 'e.test.ts': 900, 'gone.test.ts': 1100 };
253+
const measured = new Map([
254+
['a.test.ts', 1201], // grew
255+
['b.test.ts', 900], // shrank under the tripwire: the pin must go
256+
['e.test.ts', 900], // unchanged sub-tripwire file that someone pinned at its own length
257+
['c.test.ts', 1001], // new offender
258+
['d.test.ts', 1000], // at the line, fine
259+
]);
260+
expect(ratchetFindings(measured, pinned, 1000)).toEqual([
261+
'a.test.ts grew to 1201 lines (pinned 1200): extract instead of adding to a file over the tripwire.',
262+
'b.test.ts is 900 lines, at or under the 1000-line tripwire, but has a pin (1500): remove it — pins are only for files over the tripwire.',
263+
'c.test.ts is 1001 lines, over the 1000-line tripwire and not pinned: split it along the source module it mirrors (docs/agents/testing.md) rather than pinning it.',
264+
// The arbitrary-new-pin bypass: equality (900 == 900) and history (900 <= base) both pass,
265+
// so this rule is the one that rejects it.
266+
'e.test.ts is 900 lines, at or under the 1000-line tripwire, but has a pin (900): remove it — pins are only for files over the tripwire.',
267+
'gone.test.ts is pinned but does not exist: remove its pin.',
268+
]);
269+
expect(ratchetFindings(new Map([['a.test.ts', 1200]]), { 'a.test.ts': 1200 }, 1000)).toEqual([]);
270+
});
271+
272+
test('planted reds against history: raising a pin, growing into it, and pinning a new giant file are all red', () => {
273+
const baseLines = new Map<string, number | undefined>([
274+
['a.test.ts', 1200], // existed, 1200 at base
275+
['b.test.ts', 1500],
276+
['fresh.test.ts', undefined], // did not exist at base
277+
['small.test.ts', 900], // existed, under the tripwire at base
278+
]);
279+
// Bypass 1: grow a pinned file and raise its pin so the equality pin stays green.
280+
expect(
281+
historyFindings(new Map([['a.test.ts', 1230]]), { 'a.test.ts': 1230 }, baseLines, 1000),
282+
).toEqual([
283+
'a.test.ts is 1230 lines, 1200 at the merge-base: a test file over the tripwire may not grow, whatever its pin says.',
284+
"a.test.ts is pinned at 1230 but was 1200 lines at the merge-base: a pin may not be raised above its file's base length.",
285+
]);
286+
// Raising the pin alone (before growing into it) is already red.
287+
expect(
288+
historyFindings(new Map([['a.test.ts', 1200]]), { 'a.test.ts': 1230 }, baseLines, 1000),
289+
).toEqual([
290+
"a.test.ts is pinned at 1230 but was 1200 lines at the merge-base: a pin may not be raised above its file's base length.",
291+
]);
292+
// Bypass 2: add a new >1,000-line file together with a pin for it.
293+
expect(
294+
historyFindings(new Map([['fresh.test.ts', 1400]]), { 'fresh.test.ts': 1400 }, baseLines, 1000),
295+
).toEqual([
296+
'fresh.test.ts is 1400 lines and did not exist at the merge-base: a new test file may not cross the 1000-line tripwire, pinned or not.',
297+
]);
298+
// Same for a file that existed but was under the tripwire at base.
299+
expect(
300+
historyFindings(new Map([['small.test.ts', 1001]]), { 'small.test.ts': 1001 }, baseLines, 1000),
301+
).toEqual([
302+
'small.test.ts is 1001 lines, 900 at the merge-base: a test file over the tripwire may not grow, whatever its pin says.',
303+
"small.test.ts is pinned at 1001 but was 900 lines at the merge-base: a pin may not be raised above its file's base length.",
304+
]);
305+
// Allowed: shrink with a lowered pin, a pin that disappears, an unchanged file.
306+
expect(
307+
historyFindings(
308+
new Map([
309+
['a.test.ts', 1100],
310+
['b.test.ts', 1500],
311+
]),
312+
{ 'a.test.ts': 1100, 'b.test.ts': 1500 },
313+
baseLines,
314+
1000,
315+
),
316+
).toEqual([]);
317+
expect(historyFindings(new Map([['a.test.ts', 950]]), {}, baseLines, 1000)).toEqual([]);
318+
});
319+
320+
test('cat-file --batch output is parsed per request, in order, with misses as undefined', () => {
321+
// The dash is 3 bytes in UTF-8: the parser must count by bytes, not characters.
322+
const dashed = Buffer.from('a — b\nc\n', 'utf8');
323+
const output = Buffer.concat([
324+
Buffer.from(`abc blob ${dashed.length}\n`),
325+
dashed,
326+
Buffer.from('\nHEAD:missing.ts missing\ndef blob 6\nx\ny\nz\n\n'),
327+
]);
328+
expect([...parseCatFileBatch(output, ['dashed.ts', 'missing.ts', 'xyz.ts'])]).toEqual([
329+
['dashed.ts', 2],
330+
['missing.ts', undefined],
331+
['xyz.ts', 3],
332+
]);
333+
});

0 commit comments

Comments
 (0)