trevato/forge

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

More
Forks1g fMapg aRuns20g rEventsFlowsAPI

A file that is red in the full suite and green when run alone is filed as flaky under load, not as red #12302

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/12296-a-file-that-is-red-in-the-full-suite-and → main 5 commits fork

  • Merged. scout/12296-a-file-that-is-red-in-the-full-suite-and was merged into main as 4c3a9d3.

Review

Sign in to review this change.
5 files changed +387 −1
src/checks/rerun.ts added +135
@@ -0,0 +1,135 @@
1 /**
2 * A red full-suite verification runs each failing file once more on its own
3 * (#12296). A file green alone is flaky under load: a builder cannot fix it,
4 * so the report carries it as `flakyUnderLoad`, not as a plain red. A file red
5 * alone stays red. The re-runs share one time budget; a file the budget did
6 * not reach is not classified and stays a plain red, and a pass that ended
7 * past it is "over": not trusted as green, not called red either.
8 *
9 * Plain data in (the files, a runner, the budget), plain data out (a verdict
10 * per file and the lines for the verification's log).
11 */
12
13 /**
14 * How long all of one verification's re-runs may take together. They sit on
15 * the gate's path (#1242): one file alone takes about 1 s (a unit file) to
16 * 6 s (one that boots the checks service), so a minute re-runs the few files
17 * a flake under load leaves red; a suite that needs more is not load.
18 */
19 export const RERUN_BUDGET_MS = 60_000;
20 /** The most files re-run; more is a broken suite, not load. */
21 export const MAX_RERUN_FILES = 10;
22
23 const PATH = /^[\w@./+-]{1,200}\.test\.[cm]?[jt]sx?$/;
24
25 export type Alone = "green" | "red" | "over" | "skipped";
26
27 export interface Rerun {
28 readonly file: string;
29 readonly alone: Alone;
30 /** How long its re-run took; 0 when skipped. */
31 readonly ms: number;
32 }
33
34 /** Runs one test file alone within `timeoutMs`; true when it passed. */
35 export type FileRunner = (file: string, timeoutMs: number) => Promise<boolean>;
36
37 /** Whether a name from the suite's own output is a test file path safe to hand a shell. */
38 export const rerunnableFile = (file: string): boolean =>
39 PATH.test(file) && !file.split("/").includes("..");
40
41 /** `bun test` of the one file, as a path so no other file matches it. */
42 export const rerunFileCommand = (file: string): string =>
43 `bun test '${file.startsWith("./") ? file : `./${file}`}'`;
44
45 /** Each failing file once more, alone, in order, while the budget lasts. */
46 export async function rerunAlone(
47 files: readonly string[],
48 run: FileRunner,
49 options: { budgetMs?: number; now?: () => number } = {},
50 ): Promise<Rerun[]> {
51 const budget = options.budgetMs ?? RERUN_BUDGET_MS;
52 const now = options.now ?? Date.now;
53 const start = now();
54 const out: Rerun[] = [];
55 for (const [i, file] of files.entries()) {
56 const left = budget - (now() - start);
57 if (i >= MAX_RERUN_FILES || left <= 0 || !rerunnableFile(file)) {
58 out.push({ file, alone: "skipped", ms: 0 });
59 continue;
60 }
61 const began = now();
62 let green = false;
63 try {
64 green = await run(file, left);
65 } catch {
66 green = false;
67 }
68 const ms = now() - began;
69 // A pass that overran the budget is not trusted as green, nor is it red.
70 const alone: Alone = !green ? "red" : now() - start <= budget ? "green" : "over";
71 out.push({ file, alone, ms });
72 }
73 return out;
74 }
75
76 /** The files green alone: the report's `flakyUnderLoad`. */
77 export const flakyUnderLoad = (results: readonly Rerun[]): string[] =>
78 results.filter((r) => r.alone === "green").map((r) => r.file);
79
80 /** A red detail with the files green alone named on it; unchanged when there are none. */
81 export function withFlaky<T extends { readonly files?: readonly string[] }>(
82 detail: T | undefined,
83 flaky: readonly string[],
84 ): (T & { readonly flakyUnderLoad?: readonly string[] }) | undefined {
85 if (detail === undefined || flaky.length === 0) return detail;
86 const files = detail.files ?? [];
87 const named = flaky.filter((f) => files.includes(f));
88 return named.length === 0 ? detail : { ...detail, flakyUnderLoad: named };
89 }
90
91 /**
92 * One line per file for the verification's log,
93 * e.g. `rerun: test/x.test.ts green alone in 3.1 s`.
94 */
95 export const rerunLines = (results: readonly Rerun[]): string[] =>
96 results.map((r) => {
97 const took = `${(r.ms / 1000).toFixed(1)} s`;
98 if (r.alone === "skipped") return `rerun: ${r.file} not re-run (budget or limit reached, or not a test path)`;
99 if (r.alone === "over") return `rerun: ${r.file} passed alone in ${took}, past the budget: not counted`;
100 return `rerun: ${r.file} ${r.alone} alone in ${took}`;
101 });
102
103 /** What the red verification knows when it decides whether to re-run. */
104 export interface RedRun {
105 /** The failing files the suite's output named. */
106 readonly files: readonly string[];
107 /** Ran as a cluster Job: there is no checkout here to re-run in. */
108 readonly inJob: boolean;
109 /** The step was killed by a signal it did not send: no verdict to refine. */
110 readonly killed: boolean;
111 /** The run was cancelled. */
112 readonly aborted: boolean;
113 }
114
115 /**
116 * The red verification's re-runs end to end: nothing for a Job, a killed step
117 * or a cancelled run; otherwise each file alone, its line in the log, and the
118 * files green alone returned. A runner that throws is logged, never raised.
119 */
120 export async function rerunRed(
121 red: RedRun,
122 run: FileRunner,
123 write: (line: string) => void,
124 options: { budgetMs?: number; now?: () => number } = {},
125 ): Promise<string[]> {
126 if (red.files.length === 0 || red.inJob || red.killed || red.aborted) return [];
127 try {
128 const results = await rerunAlone(red.files, run, options);
129 rerunLines(results).forEach(write);
130 return flakyUnderLoad(results);
131 } catch (error) {
132 write(`rerun failed: ${String(error)}`);
133 return [];
134 }
135 }
src/checks/service.ts +19 −1
@@ -79,6 +79,7 @@ import { IMPACT_FILE, keepImpact, seedImpact } from "./impact-store.ts";
7979 import { KEPT_FILE, keepKept, seedKept } from "./kept-store.ts";
8080 import { afterVerify, closeOnGreen } from "./culprit.ts";
8181 import { redDetail, reportGreen, reportRed } from "./report.ts";
82 import { rerunFileCommand, rerunRed, withFlaky } from "./rerun.ts";
8283 import {
8384 bisectOrigin,
8485 failedFiles,
@@ -1596,6 +1597,23 @@ export function startChecks(app: App, options: ChecksOptions): ChecksHandle {
15961597 // A red verification keeps the map it measured all the same, and
15971598 // names the files it failed in for the bisection (#7786).
15981599 const files = x.tier === "verify" ? failedFiles(checkLines) : [];
1600 // Each failing file once more on its own (#12296): green alone is
1601 // flaky under load, which a builder cannot fix; red alone stays red.
1602 const flaky = await rerunRed(
1603 { files, inJob: job !== undefined, killed, aborted: signal.aborted },
1604 async (file, timeoutMs) => {
1605 const again = await runStep("rerun", rerunFileCommand(file), {
1606 cwd: dir,
1607 env,
1608 log: write,
1609 signal,
1610 timeoutMs: Math.min(timeoutMs, manifest.timeout * 1000),
1611 cell,
1612 });
1613 return again.ok;
1614 },
1615 write,
1616 );
15991617 if (x.tier === "verify" && x.keepImpact === true) {
16001618 try {
16011619 const kept = keepImpact(app.config.dataDir, repo.id, dir);
@@ -1609,7 +1627,7 @@ export function startChecks(app: App, options: ChecksOptions): ChecksHandle {
16091627 return {
16101628 ok: false,
16111629 error: killed ? ran.error : summary,
1612 detail: redDetail(detail, files, checkLines),
1630 detail: withFlaky(redDetail(detail, files, checkLines), flaky),
16131631 ...(wholeVerify ? { passed: passedFiles(checkLines) } : {}),
16141632 stage: checkStage(ran.error),
16151633 ...(killed ? { killed } : {}),
src/core/runs.ts +2
@@ -63,6 +63,8 @@ export interface CheckDetail {
6363 readonly shards?: readonly { readonly file: string; readonly shard: string; readonly slice: readonly string[] }[];
6464 /** How many files or shards the full suite ran side by side, as its banner said (#12295). */
6565 readonly concurrency?: string;
66 /** Of `files`, those that ran green when re-run alone: flaky under load, not red (#12296). */
67 readonly flakyUnderLoad?: readonly string[];
6668 /** The check machine's free disk share (0..1) when a red finished (#5453). */
6769 readonly diskFree?: number;
6870 /** `temp_full`: a shard ran out of temp space, the environment's fault, not the change's (#8679). */
test/check-verify-rerun.test.ts added +105
@@ -0,0 +1,105 @@
1 /**
2 * A red verification runs each failing file once more alone (#12296),
3 * through the checks service: green alone is `flakyUnderLoad` on the run's
4 * detail, red alone stays red, and each re-run has its `rerun:` line in the
5 * log. A step killed by a signal is not re-run.
6 */
7 import { afterAll, beforeAll, expect, test } from "bun:test";
8 import { mkdtemp, rm } from "node:fs/promises";
9 import { tmpdir } from "node:os";
10 import { join } from "node:path";
11 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
12 import { createApp, type App } from "../src/app.ts";
13 import { startChecks, type ChecksHandle } from "../src/checks/service.ts";
14 import { VERIFY_ENV } from "../src/checks/verify.ts";
15 import type { Repo } from "../src/core/repos.ts";
16 import { getRun, listRuns, type Run } from "../src/core/runs.ts";
17 import { createUser, type User } from "../src/core/users.ts";
18 import { readRunLog } from "../src/runs/log.ts";
19 import { quietLogs } from "./helpers/globals.ts";
20
21 quietLogs();
22
23 let dir: string;
24 let app: App;
25 let alice: User;
26 let repo: Repo;
27 let checks: ChecksHandle;
28
29 /** The full suite's output: both files failed beside each other. */
30 const bothRed = `printf 't/a.test.ts:\\n(fail) a > works\\nt/b.test.ts:\\n(fail) b > works\\n 0 pass\\n 2 fail\\n'`;
31
32 const testFile = (name: string, pass: boolean): string =>
33 `import { test, expect, describe } from "bun:test";\ndescribe("${name}", () => { test("works", () => { expect(${pass ? 1 : 2}).toBe(1); }); });\n`;
34
35 async function push(verify: string, note: string): Promise<string> {
36 const pushed = await commitToBranch(app, alice, repo, {
37 message: note,
38 files: [
39 { path: "forge.json", content: JSON.stringify({ check: { run: "true", verify, install: "" } }) },
40 { path: "t/a.test.ts", content: `// ${note}\n${testFile("a", true)}` },
41 { path: "t/b.test.ts", content: testFile("b", false) },
42 ],
43 });
44 if (!pushed.ok) throw new Error(JSON.stringify(pushed.error));
45 return pushed.value.commitOid;
46 }
47
48 async function until<T>(read: () => T | undefined, ms = 40_000): Promise<T> {
49 const deadline = Date.now() + ms;
50 for (;;) {
51 const got = read();
52 if (got !== undefined) return got;
53 if (Date.now() > deadline) throw new Error("timed out");
54 await Bun.sleep(100); // real-timer: polls a check that runs real bun test children
55 }
56 }
57
58 const finished = (env: string, oid: string): Run | undefined =>
59 listRuns(app.db, repo.id, { kind: "check", limit: 50 }).find(
60 (r) => r.env === env && r.commitOid === oid && (r.state === "ok" || r.state === "failed"),
61 );
62
63 async function verify(oid: string, at: number): Promise<{ run: Run; log: string }> {
64 await until(() => finished(repo.defaultBranch, oid));
65 expect(await checks.verifyDue(at)).toHaveLength(1);
66 const run = await until(() => finished(VERIFY_ENV, oid));
67 return { run, log: readRunLog(app.config.dataDir, run.id).lines.join("\n") };
68 }
69
70 beforeAll(async () => {
71 process.env["FORGE_CELLS"] = "off";
72 dir = await mkdtemp(join(tmpdir(), "check-verify-rerun-"));
73 app = createApp({ dataDir: dir, port: 0, host: "forge.test" });
74 const u = await createUser(app.db, { origin: "seed", trusted: true, username: "alice", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true });
75 if (!u.ok) throw new Error(u.error);
76 alice = u.value;
77 const made = await createRepo(app, alice, { name: "reran", visibility: "public" });
78 if (!made.ok) throw new Error("repo");
79 repo = made.value;
80 checks = startChecks(app, { forgeUrl: "http://forge.test", concurrency: 1 });
81 });
82 afterAll(async () => {
83 await checks.stop();
84 await rm(dir, { recursive: true, force: true });
85 });
86
87 test("a file green alone is flaky under load on the run; a file red alone is not", async () => {
88 const oid = await push(`${bothRed}; exit 1`, "red together");
89 const { run, log } = await verify(oid, Date.now() + 31 * 60_000);
90 expect(run.state).toBe("failed");
91 const detail = getRun(app.db, run.id)?.checkDetail;
92 expect(detail?.files).toEqual(["t/a.test.ts", "t/b.test.ts"]);
93 expect(detail?.flakyUnderLoad).toEqual(["t/a.test.ts"]);
94 expect(log).toMatch(/rerun: t\/a\.test\.ts green alone in \d+\.\d s/);
95 expect(log).toMatch(/rerun: t\/b\.test\.ts red alone in \d+\.\d s/);
96 }, 60_000);
97
98 test("a verification killed by a signal re-runs nothing", async () => {
99 const oid = await push(`${bothRed}; kill -KILL $$`, "killed");
100 const { run, log } = await verify(oid, Date.now() + 62 * 60_000);
101 expect(run.state).toBe("failed");
102 expect(log).toContain("was killed by SIGKILL");
103 expect(log).not.toContain("rerun:");
104 expect(getRun(app.db, run.id)?.checkDetail?.flakyUnderLoad).toBeUndefined();
105 }, 60_000);
test/checks-rerun.test.ts added +126
@@ -0,0 +1,126 @@
1 import { describe, expect, test } from "bun:test";
2 import {
3 flakyUnderLoad,
4 MAX_RERUN_FILES,
5 rerunAlone,
6 rerunFileCommand,
7 rerunLines,
8 RERUN_BUDGET_MS,
9 rerunnableFile,
10 rerunRed,
11 withFlaky,
12 } from "../src/checks/rerun.ts";
13
14 /** A clock the runner moves: each run takes `ms[file]`. */
15 function clocked(ms: Record<string, number>, green: Set<string>) {
16 let t = 0;
17 const ran: string[] = [];
18 return {
19 now: () => t,
20 ran,
21 run: async (file: string) => {
22 ran.push(file);
23 t += ms[file] ?? 0;
24 return green.has(file);
25 },
26 };
27 }
28
29 describe("rerunAlone (#12296)", () => {
30 test("a file green alone is flaky under load; a file red alone stays red", async () => {
31 const c = clocked({ "test/a.test.ts": 3100, "test/b.test.ts": 200 }, new Set(["test/a.test.ts"]));
32 const results = await rerunAlone(["test/a.test.ts", "test/b.test.ts"], c.run, { now: c.now });
33 expect(results).toEqual([
34 { file: "test/a.test.ts", alone: "green", ms: 3100 },
35 { file: "test/b.test.ts", alone: "red", ms: 200 },
36 ]);
37 expect(flakyUnderLoad(results)).toEqual(["test/a.test.ts"]);
38 expect(rerunLines(results)).toEqual([
39 "rerun: test/a.test.ts green alone in 3.1 s",
40 "rerun: test/b.test.ts red alone in 0.2 s",
41 ]);
42 });
43
44 test("the budget bounds the re-runs: files past it are not run and not flaky", async () => {
45 const files = ["test/a.test.ts", "test/b.test.ts", "test/c.test.ts"];
46 const c = clocked({ "test/a.test.ts": 600, "test/b.test.ts": 600 }, new Set(files));
47 const results = await rerunAlone(files, c.run, { now: c.now, budgetMs: 1000 });
48 expect(c.ran).toEqual(["test/a.test.ts", "test/b.test.ts"]);
49 // b passed but overran the budget: not trusted as green, and not red.
50 expect(results.map((r) => r.alone)).toEqual(["green", "over", "skipped"]);
51 expect(flakyUnderLoad(results)).toEqual(["test/a.test.ts"]);
52 expect(rerunLines(results)).toEqual([
53 "rerun: test/a.test.ts green alone in 0.6 s",
54 "rerun: test/b.test.ts passed alone in 0.6 s, past the budget: not counted",
55 "rerun: test/c.test.ts not re-run (budget or limit reached, or not a test path)",
56 ]);
57 });
58
59 test("the runner is given what is left of the budget", async () => {
60 const given: number[] = [];
61 let t = 0;
62 await rerunAlone(
63 ["test/a.test.ts", "test/b.test.ts"],
64 async (_f, left) => {
65 given.push(left);
66 t += 400;
67 return true;
68 },
69 { now: () => t, budgetMs: 1000 },
70 );
71 expect(given).toEqual([1000, 600]);
72 });
73
74 test("a runner that throws is red; unsafe paths and files past the limit are skipped", async () => {
75 const many = Array.from({ length: MAX_RERUN_FILES + 1 }, (_, i) => `test/f${i}.test.ts`);
76 const results = await rerunAlone(["test/x.test.ts; rm -rf /", "../up.test.ts", ...many], async (f) => {
77 if (f === "test/f0.test.ts") throw new Error("boom");
78 return true;
79 }, { now: () => 0 });
80 expect(results[0]!.alone).toBe("skipped");
81 expect(results[1]!.alone).toBe("skipped");
82 expect(results[2]!.alone).toBe("red");
83 expect(results.at(-1)!.alone).toBe("skipped");
84 });
85 });
86
87 describe("rerunRed (#12296)", () => {
88 const files = ["test/a.test.ts", "test/b.test.ts"];
89 const red = { files, inJob: false, killed: false, aborted: false };
90
91 test("re-runs each file, writes a line per file and returns those green alone", async () => {
92 const lines: string[] = [];
93 const flaky = await rerunRed(red, async (f) => f === "test/a.test.ts", (l) => lines.push(l), { now: () => 0 });
94 expect(flaky).toEqual(["test/a.test.ts"]);
95 expect(lines).toEqual(["rerun: test/a.test.ts green alone in 0.0 s", "rerun: test/b.test.ts red alone in 0.0 s"]);
96 });
97
98 test("a cluster Job, a killed step or a cancelled run re-runs nothing and writes nothing", async () => {
99 for (const not of [{ inJob: true }, { killed: true }, { aborted: true }, { files: [] }]) {
100 const lines: string[] = [];
101 let ran = 0;
102 const flaky = await rerunRed({ ...red, ...not }, async () => (ran++, true), (l) => lines.push(l));
103 expect([flaky, lines, ran]).toEqual([[], [], 0]);
104 }
105 });
106
107 test("the budget is a minute: it bounds what the re-runs add to the gate", () => {
108 expect(RERUN_BUDGET_MS).toBe(60_000);
109 });
110 });
111
112 describe("helpers", () => {
113 test("only plain test paths are rerunnable, and the command quotes the path", () => {
114 expect(rerunnableFile("test/app-home.test.ts")).toBe(true);
115 expect(rerunnableFile("test/a'b.test.ts")).toBe(false);
116 expect(rerunnableFile("src/x.ts")).toBe(false);
117 expect(rerunFileCommand("test/a.test.ts")).toBe("bun test './test/a.test.ts'");
118 });
119
120 test("withFlaky names only failing files, and leaves a detail with none untouched", () => {
121 const detail = { passed: 1, failed: 2, names: [], files: ["test/a.test.ts", "test/b.test.ts"] };
122 expect(withFlaky(detail, [])).toBe(detail);
123 expect(withFlaky(detail, ["test/a.test.ts", "test/z.test.ts"])?.flakyUnderLoad).toEqual(["test/a.test.ts"]);
124 expect(withFlaky(undefined, ["test/a.test.ts"])).toBeUndefined();
125 });
126 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs