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

Fast checks: compare `bun test --changed` with the impact map on the last 50 pulls, keep whichever never missed a red #2066

Merged opened this

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

Merged

scout/forge:scout/2056-fast-checks-compare-bun-test-changed-wit → main 1 commit fork

  • Merged. scout/2056-fast-checks-compare-bun-test-changed-wit was merged into main as bbbbf1f.

Review

Sign in to review this change.
4 files changed +312 −0
scripts/compare-changed.ts added +167
@@ -0,0 +1,167 @@
1 /**
2 * Replays the last merged pulls and compares what the fast tier picked with
3 * what `bun test --changed=<parent>` picks (#2056).
4 *
5 * FORGE_URL=… FORGE_TOKEN=… bun scripts/compare-changed.ts [--pulls 50] [--dir <scratch>] [--map <impact map>]
6 *
7 * For each merged pull: its squash commit against the parent; the fast tier's
8 * pick (`fastSet`, with the impact map given by --map: the day's map is not
9 * kept, so this is today's map on that day's diff); the `--changed` pick at
10 * that commit, from a detached worktree; and the test files red in the
11 * pull's check logs.
12 * Prints a markdown table and the verdict. Logs are cached in --dir.
13 */
14
15 import { existsSync, mkdirSync, readFileSync, writeFileSync, symlinkSync, rmSync } from "node:fs";
16 import { join } from "node:path";
17 import { tmpdir } from "node:os";
18 import { junitFiles, redFiles, runsAll, verdict, missed, type Row } from "../src/checks/changed_compare.ts";
19 import { GATE_FILE, gateListAdds, parseImpact } from "../src/checks/impact.ts";
20 import { fastSet, importsOf } from "../src/checks/tiers.ts";
21
22 const argv = process.argv.slice(2);
23 const flag = (name: string, fallback: string): string => {
24 const at = argv.indexOf(name);
25 return at >= 0 && argv[at + 1] !== undefined ? argv[at + 1]! : fallback;
26 };
27 const count = Number(flag("--pulls", "50"));
28 const dir = flag("--dir", join(process.env.TMPDIR ?? tmpdir(), "compare-changed"));
29 const base = `${process.env.FORGE_URL}/api/v1/repos/trevato/forge`;
30 const headers = { authorization: `Bearer ${process.env.FORGE_TOKEN}` };
31 const root = process.cwd();
32 mkdirSync(join(dir, "logs"), { recursive: true });
33
34 async function get(path: string): Promise<any> {
35 return (await fetch(`${base}${path}`, { headers })).json();
36 }
37
38 async function page(path: string, keep: (item: any) => boolean, want: number): Promise<any[]> {
39 const out: any[] = [];
40 let cursor = "";
41 for (let i = 0; i < 60 && out.length < want; i++) {
42 const j = await get(`${path}&limit=100${cursor ? `&cursor=${cursor}` : ""}`);
43 out.push(...j.items.filter(keep));
44 if (!j.next_cursor) break;
45 cursor = j.next_cursor;
46 }
47 return out.slice(0, want);
48 }
49
50 const git = (...args: string[]): string => {
51 const r = Bun.spawnSync(["git", ...args], { cwd: root, stderr: "pipe" });
52 if (r.exitCode !== 0) throw new Error(`git ${args.join(" ")}: ${r.stderr.toString()}`);
53 return r.stdout.toString();
54 };
55
56 async function logOf(id: string): Promise<string> {
57 const file = join(dir, "logs", `${id}.txt`);
58 if (existsSync(file)) return readFileSync(file, "utf8");
59 const text = await (await fetch(`${base}/runs/${id}/log`, { headers })).text();
60 writeFileSync(file, text);
61 return text;
62 }
63
64 const pulls = await page("/pulls?state=closed", (p) => p.mergedAt && p.mergedOid, count);
65 const oldest = pulls.at(-1)!.createdAt as string;
66 const runs: any[] = [];
67 for (let cursor = ""; ; ) {
68 const j = await get(`/runs?kind=check&limit=100${cursor ? `&cursor=${cursor}` : ""}`);
69 runs.push(...j.items);
70 if (!j.next_cursor || j.items.at(-1).createdAt < oldest) break;
71 cursor = j.next_cursor;
72 }
73
74 const tree = join(dir, "tree");
75 // The map is handed by the forge, not kept in the repository: without --map the fast tier guesses by imports.
76 const mapFile = flag("--map", "");
77 const impact = mapFile === "" ? undefined : parseImpact(readFileSync(mapFile, "utf8"));
78 if (impact !== undefined && !impact.ok) throw new Error(`--map: ${impact.error}`);
79 const needle = "NO-TEST-IS-NAMED-THIS";
80 const rows: Row[] = [];
81 const notes: string[] = [];
82 for (const pull of pulls) {
83 const oid: string = pull.mergedOid;
84 let parent: string;
85 try {
86 parent = git("rev-parse", `${oid}^`).trim();
87 } catch {
88 notes.push(`#${pull.number}: ${oid.slice(0, 8)} is not in this clone`);
89 continue;
90 }
91 const changed = git("diff", "--name-only", "--no-renames", parent, oid).split("\n").filter(Boolean);
92 // This pull's checks, newest first: on its branch, and the land gate on its squash commit.
93 const mine = runs
94 .filter(
95 (r) =>
96 (r.env === `pull-${pull.number}` || (r.env === "main" && r.commitOid === oid)) &&
97 (r.state === "ok" || r.state === "failed"),
98 )
99 .sort((a, b) => (a.createdAt < b.createdAt ? 1 : -1));
100 const reds = new Set<string>();
101 for (const r of mine.filter((r) => r.state === "failed")) for (const f of redFiles(await logOf(r.id))) reds.add(f);
102 if (changed.length === 0) {
103 notes.push(`#${pull.number}: ${oid.slice(0, 8)} changes nothing against its parent (the work landed in an earlier commit); left out`);
104 continue;
105 }
106 if (!existsSync(tree)) git("worktree", "add", "--detach", "-f", tree, oid);
107 else Bun.spawnSync(["git", "-C", tree, "checkout", "--detach", "--force", oid]);
108 if (!existsSync(join(tree, "node_modules"))) symlinkSync(join(root, "node_modules"), join(tree, "node_modules"));
109 const gateModule = (await import(`${join(tree, "scripts/gate.ts")}?${oid}`)) as { GATE: string[] };
110 const gate = new Set<string>(gateModule.GATE);
111 const cache = new Map<string, string[]>();
112 const imports = (file: string): string[] => {
113 let found = cache.get(file);
114 if (found === undefined) {
115 try {
116 found = importsOf(readFileSync(join(tree, file), "utf8"), file).flatMap((path) =>
117 /\.[cm]?[jt]sx?$/.test(path) ? [path] : [path, `${path}.ts`],
118 );
119 } catch {
120 found = [];
121 }
122 cache.set(file, found);
123 }
124 return found;
125 };
126 let gateAdds: string[] | undefined;
127 if (changed.includes(GATE_FILE)) gateAdds = gateListAdds(git("diff", "-U0", "--no-renames", parent, oid, "--", GATE_FILE));
128 let times = new Map<string, number>();
129 try {
130 times = new Map(Object.entries(JSON.parse(readFileSync(join(tree, "scripts/test-times.json"), "utf8")) as Record<string, number>));
131 } catch {
132 // no times: a test is not known to be quick
133 }
134 const plan = fastSet({
135 changed,
136 tests: [...gate],
137 imports,
138 times,
139 ...(gateAdds === undefined ? {} : { gateAdds }),
140 ...(impact?.ok ? { impact: impact.value } : {}),
141 });
142 const fast = plan.files;
143 const report = join(dir, "junit.xml");
144 rmSync(report, { force: true });
145 Bun.spawnSync(
146 ["bun", "test", `--changed=${parent}`, "-t", needle, "--reporter=junit", `--reporter-outfile=${report}`],
147 { cwd: tree, stdout: "ignore", stderr: "ignore", env: { ...process.env, FORGE_TEST_SOCKET: "port" } },
148 );
149 const imported = existsSync(report) ? junitFiles(readFileSync(report, "utf8")).filter((f) => gate.has(f)) : [];
150 // Non-import triggers (fixtures, forge.json, scripts/, bun.lock) run all.
151 const bun = changed.some(runsAll) ? [...gate] : imported;
152 rows.push({ pull: pull.number, changed: changed.length, fast, bun, reds: [...reds].sort() });
153 }
154 Bun.spawnSync(["git", "worktree", "remove", "--force", tree], { cwd: root });
155
156 console.log("| pull | changed | fast set | --changed set | reds | fast missed | --changed missed |");
157 console.log("|---|---|---|---|---|---|---|");
158 for (const r of rows) {
159 const f = r.fast === undefined ? undefined : missed(r.reds, new Set(r.fast));
160 const b = missed(r.reds, new Set(r.bun));
161 console.log(
162 `| #${r.pull} | ${r.changed} | ${r.fast?.length ?? "n/a"} | ${r.bun.length} | ${r.reds.length} | ${f === undefined ? "n/a" : f.length ? f.join(", ") : 0} | ${b.length ? b.join(", ") : 0} |`,
163 );
164 }
165 const v = verdict(rows);
166 console.log(`\n${JSON.stringify(v)}`);
167 for (const n of notes) console.log(n);
scripts/gate.ts +1
@@ -319,6 +319,7 @@ export const GATE: readonly string[] = [
319319 "test/check-list.test.ts",
320320 "test/check-land-main.test.ts",
321321 "test/check-tiers.test.ts",
322 "test/changed-compare.test.ts",
322323 "test/pull-check-again.test.ts",
323324 "test/holds.test.ts",
324325 "test/hollow.test.ts",
src/checks/changed_compare.ts added +95
@@ -0,0 +1,95 @@
1 /**
2 * Reading a check's log and a bun report for the comparison of the fast
3 * tier with `bun test --changed` (#2056): which test files a run found
4 * red, and which of two picks missed a red.
5 *
6 * Plain text in, plain data out. `scripts/compare-changed.ts` fetches the
7 * logs and runs bun; this module only reads and counts.
8 */
9
10 const TEST_FILE = /^test\/[A-Za-z0-9_.-]+\.test\.[cm]?[jt]sx?$/;
11
12 /**
13 * A change `bun test --changed` cannot judge, because no test imports it:
14 * a file that is not source (fixtures, forge.json, bun.lock, anything a test
15 * reads at run time) or a script. Prose (`*.md`) and the two tables the gate
16 * writes about itself are not.
17 */
18 export function runsAll(file: string): boolean {
19 if (file.endsWith(".md") || file.endsWith("test-times.json") || file.endsWith("test-impact.json"))
20 return false;
21 return file.startsWith("scripts/") || !/\.[cm]?[jt]sx?$/.test(file);
22 }
23
24 /**
25 * The test files with a failed test in a log: `(fail)` lines belong to the
26 * `test/<name>.test.ts:` header printed before them.
27 */
28 export function redFiles(log: string): string[] {
29 const red = new Set<string>();
30 let current: string | undefined;
31 for (const line of log.split("\n")) {
32 const header = /^(test\/\S+\.test\.[cm]?[jt]sx?):$/.exec(line);
33 if (header !== null) current = header[1]!;
34 else if (line.startsWith("(fail) ") && current !== undefined) red.add(current);
35 }
36 return [...red].sort();
37 }
38
39 /**
40 * The test files in a bun JUnit report: the suites named like a test file.
41 * (`--changed` has no list-only mode; a run that matches no test name
42 * still opens every file it picked.)
43 */
44 export function junitFiles(xml: string): string[] {
45 const found = new Set<string>();
46 for (const m of xml.matchAll(/<testsuite name="([^"]*)"/g))
47 if (TEST_FILE.test(m[1]!)) found.add(m[1]!);
48 return [...found].sort();
49 }
50
51 /** The reds a pick left out. */
52 export const missed = (reds: readonly string[], pick: ReadonlySet<string>): string[] =>
53 reds.filter((r) => !pick.has(r));
54
55 export interface Row {
56 readonly pull: number;
57 readonly changed: number;
58 /** `undefined`: no fast-tier log for the pull. */
59 readonly fast?: readonly string[];
60 readonly bun: readonly string[];
61 readonly reds: readonly string[];
62 }
63
64 export interface Verdict {
65 readonly fastMissed: number;
66 readonly bunMissed: number;
67 readonly fastTotal: number;
68 readonly bunTotal: number;
69 /** The pulls that have both picks, where the two can be compared. */
70 readonly compared: number;
71 readonly use: "fast" | "bun" | "neither";
72 }
73
74 /**
75 * Which pick `check.fast` should use: one that never missed a red; of two
76 * that did not, the one with the smaller total. Only pulls with both picks
77 * count.
78 */
79 export function verdict(rows: readonly Row[]): Verdict {
80 const both = rows.filter((r) => r.fast !== undefined);
81 let fastMissed = 0, bunMissed = 0, fastTotal = 0, bunTotal = 0;
82 for (const r of both) {
83 fastMissed += missed(r.reds, new Set(r.fast)).length;
84 bunMissed += missed(r.reds, new Set(r.bun)).length;
85 fastTotal += r.fast!.length;
86 bunTotal += r.bun.length;
87 }
88 const use =
89 fastMissed === 0 && (bunMissed > 0 || fastTotal <= bunTotal)
90 ? "fast"
91 : bunMissed === 0
92 ? "bun"
93 : "neither";
94 return { fastMissed, bunMissed, fastTotal, bunTotal, compared: both.length, use };
95 }
test/changed-compare.test.ts added +49
@@ -0,0 +1,49 @@
1 import { expect, test } from "bun:test";
2 import { junitFiles, missed, redFiles, runsAll, verdict } from "../src/checks/changed_compare.ts";
3
4 test("a failed test belongs to the file header printed before it", () => {
5 const log = [
6 "test/a.test.ts:",
7 "(pass) one",
8 "(fail) two [1ms]",
9 "test/b.test.ts:",
10 "(pass) three",
11 "(fail) four",
12 "(fail) five",
13 "(fail) with no header above is not a file",
14 ].join("\n");
15 expect(redFiles(log)).toEqual(["test/a.test.ts", "test/b.test.ts"]);
16 expect(redFiles("(fail) orphan\nnothing")).toEqual([]);
17 });
18
19 test("a bun report names the files it opened, and only test files", () => {
20 const xml = `<testsuite name="test/b.test.ts" x="1"><testsuite name="a describe"/><testsuite name="test/a.test.ts"/><testsuite name="../etc/x.test.ts"/>`;
21 expect(junitFiles(xml)).toEqual(["test/a.test.ts", "test/b.test.ts"]);
22 });
23
24 test("what no import can show runs everything", () => {
25 for (const f of ["forge.json", "bun.lock", "package.json", "test/fixtures/a.json", "scripts/gate.ts", "src/web/style.css"])
26 expect(runsAll(f)).toBe(true);
27 for (const f of ["src/a.ts", "test/a.test.ts", "README.md", "docs/X.md", "scripts/test-times.json"])
28 expect(runsAll(f)).toBe(false);
29 });
30
31 test("a pick misses the reds it left out", () => {
32 expect(missed(["a", "b"], new Set(["a"]))).toEqual(["b"]);
33 });
34
35 test("the verdict keeps a pick that missed nothing, the smaller of two such", () => {
36 const row = (fast: string[] | undefined, bun: string[], reds: string[]) => ({
37 pull: 1,
38 changed: 1,
39 ...(fast === undefined ? {} : { fast }),
40 bun,
41 reds,
42 });
43 expect(verdict([row(["a", "b"], ["a"], [])]).use).toBe("bun");
44 expect(verdict([row(["a"], ["a", "b"], [])]).use).toBe("fast");
45 expect(verdict([row(["a", "b"], ["a"], ["b"])]).use).toBe("fast");
46 expect(verdict([row(["a"], ["a", "b"], ["b"])]).use).toBe("bun");
47 expect(verdict([row(["a"], ["b"], ["a", "b"])]).use).toBe("neither");
48 expect(verdict([row(undefined, ["a"], ["b"])]).compared).toBe(0);
49 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs