Public: anyone can read this; the crew can change it.
Shards: a failing file is run once more alone, and one that passes alone does not fail the check #2465
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/2445-shards-a-failing-file-is-run-once-more-a → main 3 commits fork 1 thread · 1 unresolved
8 files
changed +212
−19
README.md +11
| @@ -1051,6 +1051,17 @@ shard's output is written whole as it finishes; the log ends with every | ||
| 1051 | 1051 | failing test from every shard, or on green with one line — how many tests, |
| 1052 | 1052 | across how many files, in how long, then the slowest and fastest shard. |
| 1053 | 1053 | |
| 1054 | A shard of several files that goes red gets a second chance for its failing | |
| 1055 | files, first three only: each is run once alone, same flags and env. A file | |
| 1056 | that passes alone was red only beside others and no longer fails the check; | |
| 1057 | the shard's log (and the tail) carries `test-shards: fails only beside | |
| 1058 | others: <file> (shard-mates: …)`. Only failed tests clear: if the report | |
| 1059 | holds an error (a file that threw while loading, an unhandled error, a | |
| 1060 | crash before the summary) nothing in that shard is retried, and a file that | |
| 1061 | fails alone stays red. `--record`, `--impact` and one-file-per-process runs | |
| 1062 | never retry. The cleared file is reported in the log only; filing it as an | |
| 1063 | issue is not built yet (#2480). | |
| 1064 | ||
| 1054 | 1065 | Two things keep a test's setup in milliseconds (#2038). `bunfig.toml` |
| 1055 | 1066 | preloads `test/helpers/preload.ts`, which hashes passwords at argon2id's |
| 1056 | 1067 | floor (8 KiB, one pass) instead of Bun's default 64 MiB, two passes: |
scripts/shards.ts +34
| @@ -495,3 +495,37 @@ export function besideOthers( | ||
| 495 | 495 | const line = `fails only beside others: ${cleared.join(", ")} (shard-mates: ${mates})`; |
| 496 | 496 | return { red: stillRed, besideOthers: cleared, line }; |
| 497 | 497 | } |
| 498 | ||
| 499 | const HEADER = /^(\S+\.test\.[cm]?[jt]sx?):$/; | |
| 500 | const FAILED = /^\(fail\) /; | |
| 501 | ||
| 502 | /** | |
| 503 | * The files whose tests failed in one `bun test` report, in the order the | |
| 504 | * report reached them: bun prints `path:` before a file's tests and | |
| 505 | * `(fail) name` for each one that failed. A report that names no failing | |
| 506 | * test (a crash, a file that would not load) yields none. | |
| 507 | */ | |
| 508 | export function failingFiles(output: string): string[] { | |
| 509 | const found: string[] = []; | |
| 510 | let current: string | undefined; | |
| 511 | for (const line of output.split("\n")) { | |
| 512 | const header = HEADER.exec(line); | |
| 513 | if (header) current = header[1]!; | |
| 514 | else if (FAILED.test(line) && current !== undefined && !found.includes(current)) | |
| 515 | found.push(current); | |
| 516 | } | |
| 517 | return found; | |
| 518 | } | |
| 519 | ||
| 520 | /** | |
| 521 | * Whether every red in one `bun test` report is a failed test: the report | |
| 522 | * reached its summary, counts no errors, and has no unhandled error between | |
| 523 | * tests. A file that threw while loading, an error outside any test, or a | |
| 524 | * crash before the summary is red for a reason a second run alone does not | |
| 525 | * explain away, so nothing in such a report may be cleared. | |
| 526 | */ | |
| 527 | export function onlyTestFailures(output: string): boolean { | |
| 528 | if (!/^Ran \d+ tests? across \d+ files?/m.test(output)) return false; | |
| 529 | if (/^# Unhandled error/m.test(output)) return false; | |
| 530 | return !/^\s*[1-9][0-9]* errors?\s*$/m.test(output); | |
| 531 | } |
scripts/test-shards.ts +59 −18
| @@ -58,6 +58,9 @@ import { | ||
| 58 | 58 | splitCapped, |
| 59 | 59 | tally, |
| 60 | 60 | weigher, |
| 61 | besideOthers, | |
| 62 | failingFiles, | |
| 63 | onlyTestFailures, | |
| 61 | 64 | type ShardResult, |
| 62 | 65 | } from "./shards.ts"; |
| 63 | 66 | import { gateFiles, gateRefusal, presentTests } from "./gate.ts"; |
| @@ -425,6 +428,12 @@ async function drain( | ||
| 425 | 428 | const measured = new Map<string, string[]>(); |
| 426 | 429 | const unmeasured = new Set<string>(); |
| 427 | 430 | |
| 431 | // Failing files a red shard re-runs alone. | |
| 432 | const RETRY_CAP = 3; | |
| 433 | // Set once any shard has gone red for good: no retries then. | |
| 434 | let halted = false; | |
| 435 | const stopping = (): boolean => halted; | |
| 436 | ||
| 428 | 437 | type Timed = ShardResult & { seconds: number }; |
| 429 | 438 | |
| 430 | 439 | /** |
| @@ -463,29 +472,60 @@ async function runShard( | ||
| 463 | 472 | const cover = covDir |
| 464 | 473 | ? ["--coverage", "--coverage-reporter=lcov", `--coverage-dir=${covDir}`] |
| 465 | 474 | : []; |
| 466 | const child = Bun.spawn(["bun", "test", ...flags, ...cover, ...paths], { | |
| 467 | stdin: "ignore", | |
| 468 | stdout: "pipe", | |
| 469 | stderr: "pipe", | |
| 470 | // Bun keeps shell-inherited vars after `delete process.env.X`; pass a | |
| 471 | // cleaned env so capacityDefault does not see the gate's shard count. | |
| 472 | env: childEnv, | |
| 473 | }); | |
| 474 | children.add(child); | |
| 475 | // One buffer, in arrival order: `bun test` splits its report across | |
| 476 | // stdout and stderr. | |
| 477 | const output: string[] = []; | |
| 475 | const run = async ( | |
| 476 | targets: string[], | |
| 477 | extra: string[], | |
| 478 | through?: (text: string) => void, | |
| 479 | ): Promise<{ code: number; output: string[] }> => { | |
| 480 | const child = Bun.spawn(["bun", "test", ...flags, ...extra, ...targets], { | |
| 481 | stdin: "ignore", | |
| 482 | stdout: "pipe", | |
| 483 | stderr: "pipe", | |
| 484 | // Bun keeps shell-inherited vars after `delete process.env.X`; pass a | |
| 485 | // cleaned env so capacityDefault does not see the gate's shard count. | |
| 486 | env: childEnv, | |
| 487 | }); | |
| 488 | children.add(child); | |
| 489 | // One buffer, in arrival order: `bun test` splits its report across | |
| 490 | // stdout and stderr. | |
| 491 | const output: string[] = []; | |
| 492 | await Promise.all([ | |
| 493 | drain(child.stdout, output, through), | |
| 494 | drain(child.stderr, output, through), | |
| 495 | ]); | |
| 496 | const code = await child.exited; | |
| 497 | children.delete(child); | |
| 498 | return { code, output }; | |
| 499 | }; | |
| 478 | 500 | const through = live |
| 479 | 501 | ? (text: string): void => { |
| 480 | 502 | process.stdout.write(text); |
| 481 | 503 | } |
| 482 | 504 | : undefined; |
| 483 | await Promise.all([ | |
| 484 | drain(child.stdout, output, through), | |
| 485 | drain(child.stderr, output, through), | |
| 486 | ]); | |
| 487 | const code = await child.exited; | |
| 488 | children.delete(child); | |
| 505 | const first = await run(paths, cover, through); | |
| 506 | let code = first.code; | |
| 507 | const output = first.output; | |
| 508 | // A sliced shard that went red: each failing file (the first few) is run | |
| 509 | // once alone, same env and flags. One that passes alone was red only | |
| 510 | // beside others, and does not fail the check; the log names it. | |
| 511 | if (code !== 0 && slice.length > 1 && !covDir && !stopping()) { | |
| 512 | const report = output.join(""); | |
| 513 | // A file that errored (not a failed test) keeps the whole shard red. | |
| 514 | const red = onlyTestFailures(report) ? failingFiles(report) : []; | |
| 515 | const alone = new Map<string, boolean>(); | |
| 516 | for (const file of red.slice(0, RETRY_CAP)) { | |
| 517 | if (stopping()) break; | |
| 518 | const again = await run([file.startsWith("/") || file.startsWith("./") ? file : `./${file}`], []); | |
| 519 | alone.set(file, again.code === 0); | |
| 520 | } | |
| 521 | const verdict = besideOthers(red, alone, RETRY_CAP); | |
| 522 | if (verdict.line !== null) { | |
| 523 | const note = `${output.join("").endsWith("\n") ? "" : "\n"}test-shards: ${verdict.line}\n`; | |
| 524 | output.push(note); | |
| 525 | through?.(note); | |
| 526 | if (verdict.red.length === 0) code = 0; | |
| 527 | } | |
| 528 | } | |
| 489 | 529 | const text = output.join(""); |
| 490 | 530 | const seconds = ((performance.now() - started) / 1000).toFixed(1); |
| 491 | 531 | process.stdout.write( |
| @@ -524,6 +564,7 @@ async function pool(width: number): Promise<Timed[]> { | ||
| 524 | 564 | results[index] = await runShard(slices[index]!, index, width === 1); |
| 525 | 565 | if (results[index]!.code !== 0) { |
| 526 | 566 | stop = true; |
| 567 | halted = true; | |
| 527 | 568 | for (const child of children) child.kill("SIGTERM"); |
| 528 | 569 | } |
| 529 | 570 | } |
test/fixtures/shard-broken.test.ts added +6
| @@ -0,0 +1,6 @@ | ||
| 1 | // Fixture for test/shards.test.ts, kept out of the gate: fails alone too. | |
| 2 | import { expect, test } from "bun:test"; | |
| 3 | ||
| 4 | test("broken", () => { | |
| 5 | expect(1).toBe(2); | |
| 6 | }); |
test/fixtures/shard-errors.test.ts added +2
| @@ -0,0 +1,2 @@ | ||
| 1 | // Fixture for test/shards.test.ts, kept out of the gate: errors while loading. | |
| 2 | throw new Error("boom at load"); |
test/fixtures/shard-poison.test.ts added +6
| @@ -0,0 +1,6 @@ | ||
| 1 | // Fixture for test/shards.test.ts, kept out of the gate: sets a global that | |
| 2 | // shard-victim.test.ts cannot stand beside. Both pass alone. | |
| 3 | import { test } from "bun:test"; | |
| 4 | ||
| 5 | (globalThis as Record<string, unknown>)["__shardPoison"] = true; | |
| 6 | test("poison passes", () => {}); |
test/fixtures/shard-victim.test.ts added +7
| @@ -0,0 +1,7 @@ | ||
| 1 | // Fixture for test/shards.test.ts, kept out of the gate: fails only in a | |
| 2 | // process where shard-poison.test.ts has already run. | |
| 3 | import { expect, test } from "bun:test"; | |
| 4 | ||
| 5 | test("victim", () => { | |
| 6 | expect((globalThis as Record<string, unknown>)["__shardPoison"]).toBeUndefined(); | |
| 7 | }); |
test/shards.test.ts +87 −1
| @@ -3,7 +3,7 @@ | ||
| 3 | 3 | * the count follows the host and the override, and one red shard is a |
| 4 | 4 | * red run whose tail names every failing test from every shard. |
| 5 | 5 | */ |
| 6 | import { describe, expect, test } from "bun:test"; | |
| 6 | import { afterAll, describe, expect, test } from "bun:test"; | |
| 7 | 7 | import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; |
| 8 | 8 | import { tmpdir } from "node:os"; |
| 9 | 9 | import { join } from "node:path"; |
| @@ -11,6 +11,8 @@ import { | ||
| 11 | 11 | SECONDS_PER_BYTE_DEFAULT, |
| 12 | 12 | SHARDS_MAX, |
| 13 | 13 | besideOthers, |
| 14 | failingFiles, | |
| 15 | onlyTestFailures, | |
| 14 | 16 | WORKER_MEMORY_BYTES, |
| 15 | 17 | CELL_CPU_SHARE, |
| 16 | 18 | FILES_PER_PROCESS, |
| @@ -754,3 +756,87 @@ describe("besideOthers", () => { | ||
| 754 | 756 | }); |
| 755 | 757 | }); |
| 756 | 758 | }); |
| 759 | ||
| 760 | describe("failingFiles", () => { | |
| 761 | test("names each file with a failing test once, in report order", () => { | |
| 762 | const report = [ | |
| 763 | "bun test v1", | |
| 764 | "test/a.test.ts:", | |
| 765 | "(pass) fine", | |
| 766 | "test/b.test.ts:", | |
| 767 | "(fail) one [0.1ms]", | |
| 768 | "(fail) two [0.1ms]", | |
| 769 | "test/c.test.ts:", | |
| 770 | "(fail) three [0.1ms]", | |
| 771 | ].join("\n"); | |
| 772 | expect(failingFiles(report)).toEqual(["test/b.test.ts", "test/c.test.ts"]); | |
| 773 | }); | |
| 774 | ||
| 775 | test("a report that names no failing test names no file", () => { | |
| 776 | expect(failingFiles("test/a.test.ts:\nerror: cannot load\n")).toEqual([]); | |
| 777 | expect(failingFiles("(fail) before any header")).toEqual([]); | |
| 778 | }); | |
| 779 | }); | |
| 780 | ||
| 781 | describe("onlyTestFailures", () => { | |
| 782 | const ran = "Ran 3 tests across 2 files. [9ms]"; | |
| 783 | test("failed tests and a summary: true", () => { | |
| 784 | expect(onlyTestFailures(`(fail) a\n 0 pass\n 1 fail\n${ran}`)).toBe(true); | |
| 785 | }); | |
| 786 | test("an error count, an unhandled error or no summary: false", () => { | |
| 787 | expect(onlyTestFailures(`(fail) a\n 1 fail\n 1 error\n${ran}`)).toBe(false); | |
| 788 | expect(onlyTestFailures(`# Unhandled error between tests\n(fail) a\n${ran}`)).toBe(false); | |
| 789 | expect(onlyTestFailures("(fail) a\n 1 fail\n")).toBe(false); | |
| 790 | }); | |
| 791 | }); | |
| 792 | ||
| 793 | describe("a failing file is run once more alone", () => { | |
| 794 | const script = new URL("../scripts/test-shards.ts", import.meta.url).pathname; | |
| 795 | const dir = "test/fixtures"; | |
| 796 | // Heavy by bytes, so the splitter gives it a shard of its own and the | |
| 797 | // other fixtures share the second one. Written here, not committed. | |
| 798 | const scratch = mkdtempSync(join(tmpdir(), "shard-filler-")); | |
| 799 | const filler = join(scratch, "shard-filler.test.ts"); | |
| 800 | writeFileSync(filler, `import { test } from "bun:test";\ntest("filler", () => {});\n// ${"padding ".repeat(4000)}\n`); | |
| 801 | afterAll(() => rmSync(scratch, { recursive: true, force: true })); | |
| 802 | const run = async (names: string[]) => { | |
| 803 | const child = Bun.spawn( | |
| 804 | ["bun", script, ...names.map((name) => (name === "filler" ? filler : `${dir}/shard-${name}.test.ts`))], | |
| 805 | { | |
| 806 | stdin: "ignore", | |
| 807 | stdout: "pipe", | |
| 808 | stderr: "pipe", | |
| 809 | env: { ...process.env, FORGE_CHECK_SHARDS: "2" }, | |
| 810 | }, | |
| 811 | ); | |
| 812 | const [out] = await Promise.all([new Response(child.stdout).text(), new Response(child.stderr).text()]); | |
| 813 | return { code: await child.exited, out }; | |
| 814 | }; | |
| 815 | ||
| 816 | test("red only beside another file: the shard is green and the tail names it", async () => { | |
| 817 | // filler is heavy by bytes, so poison and victim share the other shard. | |
| 818 | const got = await run(["filler", "poison", "victim"]); | |
| 819 | expect(got.out).toContain("fails only beside others: test/fixtures/shard-victim.test.ts (shard-mates: none)"); | |
| 820 | expect(got.code).toBe(0); | |
| 821 | }, 30_000); | |
| 822 | ||
| 823 | test("red alone too: it stays red", async () => { | |
| 824 | const got = await run(["filler", "poison", "victim", "broken"]); | |
| 825 | expect(got.out).toContain("fails only beside others: test/fixtures/shard-victim.test.ts (shard-mates: test/fixtures/shard-broken.test.ts)"); | |
| 826 | expect(got.code).toBe(1); | |
| 827 | expect(got.out).toContain("(fail) broken"); | |
| 828 | }, 30_000); | |
| 829 | ||
| 830 | test("one file fails beside others and another errors: the shard stays red", async () => { | |
| 831 | const got = await run(["filler", "poison", "victim", "errors"]); | |
| 832 | expect(got.code).toBe(1); | |
| 833 | expect(got.out).not.toContain("fails only beside others"); | |
| 834 | expect(got.out).toContain("boom at load"); | |
| 835 | }, 30_000); | |
| 836 | ||
| 837 | test("a lone file that fails stays red with no verdict line", async () => { | |
| 838 | const got = await run(["broken"]); | |
| 839 | expect(got.code).toBe(1); | |
| 840 | expect(got.out).not.toContain("fails only beside others"); | |
| 841 | }, 30_000); | |
| 842 | }); |
new above
offline