Public: anyone can read this; the crew can change it.

The gate answers in a minute, not four: the check runs its test files in parallel shards #306

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

mason/forge:mason/277-the-gate-answers-in-a-minute-not-four-th → main 2 commits fork

  • Merged. mason/277-the-gate-answers-in-a-minute-not-four-th was merged into main as 0e6fc81.

Review

Sign in to review this change.
7 files changed +389 −5
README.md +9 −1
@@ -302,7 +302,15 @@ bun run check # exactly what the forge runs against every push
302302 ```
303303
304304 `bun run check` is the gate: the same command `forge.json` declares, so what
305 it says here is what the deploy will say. Two things it will not do:
305 it says here is what the deploy will say. It type-checks once, then runs the
306 test files named in `check.run` as parallel `bun test` shards
307 (`scripts/test-shards.ts`): one per CPU the process may use, at most eight,
308 and `FORGE_CHECK_SHARDS=2` turns that down on a small or busy host. Each
309 shard's output is written whole as it finishes; the log ends with every
310 failing test from every shard, or on green with one line — how many tests,
311 across how many files, in how long. Every file is independent (its own forge,
312 its own database, port 0), so a test that fails only beside another is a bug
313 in that test. Two things it will not do:
306314
307315 - **It never borrows your git configuration.** A test drives a real `git`
308316 against a forge it booted a second ago, and builds that child's
forge.json +1 −1
@@ -6,7 +6,7 @@
66 "ready": 90
77 },
88 "check": {
9 "run": "bun x tsc --noEmit && bun test --timeout 60000 test/crew.test.ts test/crew-wrapup.test.ts test/crew-limits.test.ts test/crew-sandbox.test.ts test/steer.test.ts test/landing.test.ts test/decide.test.ts test/ledger.test.ts test/checks.test.ts test/dependencies.test.ts test/autonomy.test.ts test/studio.test.ts test/studio-words.test.ts test/forks.test.ts test/fork-flow.test.ts test/cross-repo-pull.test.ts test/pulls.test.ts test/repos.test.ts test/apps.test.ts test/apps-adversarial.test.ts test/web.test.ts test/git-trust.test.ts test/helpers.test.ts",
9 "run": "bun x tsc --noEmit && bun scripts/test-shards.ts --timeout 60000 test/crew.test.ts test/crew-wrapup.test.ts test/crew-limits.test.ts test/crew-sandbox.test.ts test/steer.test.ts test/landing.test.ts test/decide.test.ts test/ledger.test.ts test/checks.test.ts test/dependencies.test.ts test/autonomy.test.ts test/studio.test.ts test/studio-words.test.ts test/forks.test.ts test/fork-flow.test.ts test/cross-repo-pull.test.ts test/pulls.test.ts test/apps.test.ts test/apps-adversarial.test.ts test/web.test.ts test/git-trust.test.ts test/helpers.test.ts test/shards.test.ts",
1010 "timeout": 1800
1111 }
1212 }
scripts/shards.ts added +136
@@ -0,0 +1,136 @@
1 /**
2 * The gate's test files, split into shards that run side by side.
3 *
4 * Plain data in, plain data out: how many shards, which files each gets,
5 * and what one log says when they are done. `scripts/test-shards.ts` does
6 * the spawning; this module only decides, so a test can prove the split
7 * is disjoint and covering, and that one red shard makes the run red.
8 */
9
10 /** No more than this many `bun test` processes at once, whatever the host. */
11 export const SHARDS_MAX = 8;
12
13 /**
14 * How many shards: one per CPU the process may use, capped at
15 * {@link SHARDS_MAX} and at the number of files, at least one. One per CPU,
16 * not fewer: most of a forge test's time is spent waiting on git children
17 * and timers, not computing. `FORGE_CHECK_SHARDS` overrides it (still
18 * capped at the file count) so a small or busy host can turn it down.
19 */
20 export function shardCount(
21 cpus: number,
22 files: number,
23 override?: string,
24 ): { ok: true; value: number } | { ok: false; error: string } {
25 let n = Math.min(SHARDS_MAX, Math.floor(cpus));
26 if (override !== undefined && override.trim() !== "") {
27 if (!/^[1-9][0-9]{0,2}$/.test(override.trim()))
28 return {
29 ok: false,
30 error: `FORGE_CHECK_SHARDS must be a whole number from 1, not ${JSON.stringify(override)}`,
31 };
32 n = Number(override.trim());
33 }
34 return { ok: true, value: Math.max(1, Math.min(n, files)) };
35 }
36
37 /**
38 * Splits `files` into at most `n` disjoint, non-empty slices that together
39 * hold every file once. Heaviest file first onto the lightest slice, so a
40 * big file does not share a shard with other big ones; ties break on the
41 * file's place in the list, so the same input always gives the same split.
42 * Each slice keeps the list's own order.
43 */
44 export function splitFiles(
45 files: readonly string[],
46 n: number,
47 weight: (file: string) => number = () => 1,
48 ): string[][] {
49 const count = Math.max(1, Math.min(Math.floor(n), files.length));
50 const order = files
51 .map((file, index) => ({ file, index, weight: weight(file) }))
52 .sort((a, b) => b.weight - a.weight || a.index - b.index);
53 const slices = Array.from({ length: count }, () => ({
54 load: 0,
55 items: [] as { file: string; index: number }[],
56 }));
57 for (const item of order) {
58 let lightest = slices[0]!;
59 // Fewest files breaks a tie, so weightless files still spread out.
60 for (const slice of slices)
61 if (
62 slice.load < lightest.load ||
63 (slice.load === lightest.load &&
64 slice.items.length < lightest.items.length)
65 )
66 lightest = slice;
67 lightest.load += item.weight;
68 lightest.items.push(item);
69 }
70 return slices
71 .filter((slice) => slice.items.length > 0)
72 .map((slice) =>
73 slice.items.sort((a, b) => a.index - b.index).map((item) => item.file),
74 );
75 }
76
77 /** One shard, finished: what it ran, how it exited, everything it printed. */
78 export interface ShardResult {
79 readonly files: readonly string[];
80 readonly code: number;
81 readonly output: string;
82 }
83
84 /** What the log ends with, and the exit code of the whole run. */
85 export interface Tally {
86 readonly code: number;
87 readonly lines: string[];
88 }
89
90 const RAN = /^Ran (\d+) tests? across (\d+) files?\./m;
91 const FAIL = /^\(fail\) .*$/gm;
92
93 /**
94 * Reads every shard's `bun test` output into one ending. Green: one line,
95 * how many tests across how many files and how long. Red: every failing
96 * test from every shard, and any shard that went red without naming one
97 * (a file that would not load, a crash between tests) by its files.
98 */
99 export function tally(results: readonly ShardResult[], seconds: number): Tally {
100 let tests = 0;
101 let files = 0;
102 const failing: string[] = [];
103 const unnamed: string[] = [];
104 results.forEach((result, i) => {
105 const ran = RAN.exec(result.output);
106 if (ran) {
107 tests += Number(ran[1]);
108 files += Number(ran[2]);
109 }
110 const named = result.output.match(FAIL) ?? [];
111 for (const line of new Set(named)) failing.push(`${line} [shard ${i + 1}]`);
112 if (result.code !== 0 && named.length === 0)
113 unnamed.push(
114 `shard ${i + 1} exited ${result.code} without naming a test: ${result.files.join(" ")}`,
115 );
116 });
117 const took = `${seconds.toFixed(1)}s`;
118 const shards = `${results.length} shard${results.length === 1 ? "" : "s"}`;
119 const red = results.filter((result) => result.code !== 0).length;
120 if (red === 0)
121 return {
122 code: 0,
123 lines: [
124 `Ran ${tests} tests across ${files} files in ${took} (${shards}).`,
125 ],
126 };
127 return {
128 code: 1,
129 lines: [
130 `${red} of ${shards} failed:`,
131 ...failing,
132 ...unnamed,
133 `Ran ${tests} tests across ${files} files in ${took}; ${failing.length} failed.`,
134 ],
135 };
136 }
scripts/test-shards.ts added +108
@@ -0,0 +1,108 @@
1 /**
2 * `bun test`, over the named files, in parallel shards.
3 *
4 * bun scripts/test-shards.ts [bun test flags…] test/a.test.ts test/b.test.ts …
5 *
6 * The files come from the command line, so `forge.json`'s `check.run`
7 * stays the one place that names them. Each shard is its own `bun test`
8 * over a disjoint slice (`scripts/shards.ts` decides the slices); a
9 * shard's output is written through whole when it finishes, so the log
10 * reads one shard at a time, and the tail names every failing test from
11 * every shard. Exits non-zero if any shard does.
12 */
13
14 import { availableParallelism } from "node:os";
15 import { statSync } from "node:fs";
16 import { shardCount, splitFiles, tally, type ShardResult } from "./shards.ts";
17
18 const TEST_FILE = /\.test\.[cm]?[jt]sx?$/;
19
20 const args = process.argv.slice(2);
21 const files = args.filter((arg) => TEST_FILE.test(arg));
22 const flags = args.filter((arg) => !TEST_FILE.test(arg));
23
24 if (files.length === 0) {
25 console.error("test-shards: name the test files to run");
26 process.exit(2);
27 }
28 // A file that is not there would be a filter matching nothing to `bun
29 // test`: a gate that quietly judges less than it says.
30 const size = new Map<string, number>();
31 for (const file of files) {
32 try {
33 size.set(file, statSync(file).size);
34 } catch {
35 console.error(`test-shards: ${file} does not exist`);
36 process.exit(2);
37 }
38 }
39 if (new Set(files).size !== files.length) {
40 console.error("test-shards: a file is named twice");
41 process.exit(2);
42 }
43
44 const count = shardCount(
45 availableParallelism(),
46 files.length,
47 process.env["FORGE_CHECK_SHARDS"],
48 );
49 if (!count.ok) {
50 console.error(`test-shards: ${count.error}`);
51 process.exit(2);
52 }
53 // Bytes stand in for how long a file takes: no timings are kept between runs.
54 const slices = splitFiles(files, count.value, (file) => size.get(file) ?? 0);
55
56 console.log(
57 `test-shards: ${files.length} files in ${slices.length} shard${slices.length === 1 ? "" : "s"}`,
58 );
59
60 const children = new Set<Bun.Subprocess>();
61 for (const signal of ["SIGINT", "SIGTERM"] as const)
62 process.on(signal, () => {
63 for (const child of children) child.kill(signal);
64 process.exit(130);
65 });
66
67 async function drain(
68 stream: ReadableStream<Uint8Array>,
69 into: string[],
70 ): Promise<void> {
71 const decoder = new TextDecoder();
72 for await (const chunk of stream)
73 into.push(decoder.decode(chunk, { stream: true }));
74 into.push(decoder.decode());
75 }
76
77 async function runShard(slice: string[], index: number): Promise<ShardResult> {
78 const started = performance.now();
79 // An exact path, never a filter: `test/web.test.ts` alone would also
80 // match any file whose path contains it, in some other shard.
81 const paths = slice.map((file) =>
82 file.startsWith("/") || file.startsWith("./") ? file : `./${file}`,
83 );
84 const child = Bun.spawn(["bun", "test", ...flags, ...paths], {
85 stdin: "ignore",
86 stdout: "pipe",
87 stderr: "pipe",
88 });
89 children.add(child);
90 // One buffer, in arrival order: `bun test` splits its report across
91 // stdout and stderr.
92 const output: string[] = [];
93 await Promise.all([drain(child.stdout, output), drain(child.stderr, output)]);
94 const code = await child.exited;
95 children.delete(child);
96 const text = output.join("");
97 const seconds = ((performance.now() - started) / 1000).toFixed(1);
98 process.stdout.write(
99 `\n── shard ${index + 1}/${slices.length} (exit ${code}, ${seconds}s): ${slice.join(" ")}\n${text}${text.endsWith("\n") ? "" : "\n"}`,
100 );
101 return { files: slice, code, output: text };
102 }
103
104 const started = performance.now();
105 const results = await Promise.all(slices.map(runShard));
106 const ending = tally(results, (performance.now() - started) / 1000);
107 console.log(`\n${ending.lines.join("\n")}`);
108 process.exit(ending.code);
test/shards.test.ts added +119
@@ -0,0 +1,119 @@
1 /**
2 * The gate's shards (`scripts/shards.ts`): every file runs exactly once,
3 * the count follows the host and the override, and one red shard is a
4 * red run whose tail names every failing test from every shard.
5 */
6 import { describe, expect, test } from "bun:test";
7 import {
8 SHARDS_MAX,
9 shardCount,
10 splitFiles,
11 tally,
12 type ShardResult,
13 } from "../scripts/shards.ts";
14
15 const files = Array.from({ length: 23 }, (_, i) => `test/f${i}.test.ts`);
16
17 describe("splitFiles", () => {
18 for (const n of [1, 2, 3, 5, 8, 23, 40]) {
19 test(`${n} shards: disjoint, covering, none empty`, () => {
20 const weight = (file: string) =>
21 (Number(file.match(/\d+/)![0]) * 37) % 11;
22 const slices = splitFiles(files, n, weight);
23 expect(slices.length).toBe(Math.min(n, files.length));
24 for (const slice of slices) expect(slice.length).toBeGreaterThan(0);
25 const all = slices.flat();
26 expect(all.length).toBe(files.length);
27 expect(new Set(all).size).toBe(files.length);
28 expect([...all].sort()).toEqual([...files].sort());
29 });
30 }
31
32 test("keeps heavy files apart and is the same every time", () => {
33 const weight = (file: string) => (file === "a" || file === "b" ? 100 : 1);
34 const list = ["a", "x", "b", "y", "z"];
35 const slices = splitFiles(list, 2, weight);
36 expect(slices.some((s) => s.includes("a") && s.includes("b"))).toBe(false);
37 expect(splitFiles(list, 2, weight)).toEqual(slices);
38 // Each slice keeps the list's order.
39 for (const slice of slices)
40 expect(slice).toEqual(list.filter((f) => slice.includes(f)));
41 });
42
43 test("never makes more shards than files, nor fewer than one", () => {
44 expect(splitFiles(["only"], 8)).toEqual([["only"]]);
45 expect(splitFiles(["a", "b"], 0)).toEqual([["a", "b"]]);
46 });
47 });
48
49 describe("shardCount", () => {
50 test("one per CPU, capped, at least one, never more than files", () => {
51 expect(shardCount(80, 22)).toEqual({ ok: true, value: SHARDS_MAX });
52 expect(shardCount(4, 22)).toEqual({ ok: true, value: 4 });
53 expect(shardCount(1, 22)).toEqual({ ok: true, value: 1 });
54 expect(shardCount(80, 3)).toEqual({ ok: true, value: 3 });
55 });
56
57 test("FORGE_CHECK_SHARDS turns it up or down; nonsense is refused", () => {
58 expect(shardCount(80, 22, "2")).toEqual({ ok: true, value: 2 });
59 expect(shardCount(4, 22, "12")).toEqual({ ok: true, value: 12 });
60 expect(shardCount(80, 22, "1")).toEqual({ ok: true, value: 1 });
61 expect(shardCount(4, 22, "")).toEqual({ ok: true, value: 4 });
62 for (const bad of ["0", "-1", "two", "1.5", "1e3", "9999"])
63 expect(shardCount(4, 22, bad).ok).toBe(false);
64 });
65 });
66
67 const green = (files: string[], tests: number): ShardResult => ({
68 files,
69 code: 0,
70 output: `(pass) fine [1ms]\n\n ${tests} pass\n 0 fail\nRan ${tests} tests across ${files.length} files. [1.00s]\n`,
71 });
72
73 describe("tally", () => {
74 test("all green ends in one line: tests, files, time", () => {
75 const ending = tally([green(["a", "b"], 10), green(["c"], 1)], 61.23);
76 expect(ending.code).toBe(0);
77 expect(ending.lines).toEqual([
78 "Ran 11 tests across 3 files in 61.2s (2 shards).",
79 ]);
80 });
81
82 test("one red shard makes the run red, and every failure is named", () => {
83 const red = (files: string[], names: string[]): ShardResult => ({
84 files,
85 code: 1,
86 output: `${names.map((n) => `(fail) ${n} [2ms]`).join("\n")}\n\n 3 pass\n ${names.length} fail\nRan ${3 + names.length} tests across ${files.length} file. [1.00s]\n\n${names.map((n) => `(fail) ${n} [2ms]`).join("\n")}\n`,
87 });
88 const ending = tally(
89 [
90 red(["a"], ["x > one"]),
91 green(["b"], 4),
92 red(["c"], ["y > two", "y > three"]),
93 ],
94 5,
95 );
96 expect(ending.code).toBe(1);
97 expect(ending.lines).toEqual([
98 "2 of 3 shards failed:",
99 "(fail) x > one [2ms] [shard 1]",
100 "(fail) y > two [2ms] [shard 3]",
101 "(fail) y > three [2ms] [shard 3]",
102 "Ran 13 tests across 3 files in 5.0s; 3 failed.",
103 ]);
104 });
105
106 test("a shard that crashes without naming a test still fails, by its files", () => {
107 const ending = tally(
108 [
109 green(["a"], 2),
110 { files: ["b", "c"], code: 1, output: "error: boom\n" },
111 ],
112 1,
113 );
114 expect(ending.code).toBe(1);
115 expect(ending.lines).toContain(
116 "shard 2 exited 1 without naming a test: b c",
117 );
118 });
119 });
test/steer.test.ts +15 −2
@@ -164,7 +164,20 @@ fi
164164 throw new Error("the run never started");
165165 };
166166 const run = await running();
167 await Bun.sleep(300); // past the session's first line
167 // Past the session's first line: `running` is set before the agent
168 // is spawned, so on a busy machine a fixed sleep can speak too early
169 // and queue a second run. The session line proves it can hear us.
170 const logPath = join(dataDir, "runs", `${run.id}.log`);
171 const opened = Date.now() + 20_000;
172 while (
173 Date.now() < opened &&
174 !(
175 await Bun.file(logPath)
176 .text()
177 .catch(() => "")
178 ).includes("session s2")
179 )
180 await Bun.sleep(50);
168181 expect(
169182 addComment(app, alice, repo, issue.value, "also the tests").ok,
170183 ).toBe(true);
@@ -180,7 +193,7 @@ fi
180193 expect(runs.length).toBe(1);
181194 expect(runs[0]!.state).toBe("ok");
182195 expect(runs[0]!.turns).toBe(2);
183 const log = readFileSync(join(dataDir, "runs", `${run.id}.log`), "utf8");
196 const log = readFileSync(logPath, "utf8");
184197 expect(log).toContain("you: alice on #1: also the tests");
185198 expect(log).toContain("heard: alice on #1: also the tests");
186199 } finally {
tsconfig.json +1 −1
@@ -16,5 +16,5 @@
1616 "skipLibCheck": true,
1717 "noEmit": true
1818 },
19 "include": ["src", "test"]
19 "include": ["src", "test", "scripts"]
2020 }

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs