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

Merged opened this

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

Merged

scout/forge:scout/2445-shards-a-failing-file-is-run-once-more-a → main 3 commits fork 1 thread · 1 unresolved

  • Merged. scout/2445-shards-a-failing-file-is-run-once-more-a was merged into main as 209720d.

Review

Sign in to review this change.
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
10511051 failing test from every shard, or on green with one line — how many tests,
10521052 across how many files, in how long, then the slowest and fastest shard.
10531053
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
10541065 Two things keep a test's setup in milliseconds (#2038). `bunfig.toml`
10551066 preloads `test/helpers/preload.ts`, which hashes passwords at argon2id's
10561067 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(
495495 const line = `fails only beside others: ${cleared.join(", ")} (shard-mates: ${mates})`;
496496 return { red: stillRed, besideOthers: cleared, line };
497497 }
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 {
5858 splitCapped,
5959 tally,
6060 weigher,
61 besideOthers,
62 failingFiles,
63 onlyTestFailures,
6164 type ShardResult,
6265 } from "./shards.ts";
6366 import { gateFiles, gateRefusal, presentTests } from "./gate.ts";
@@ -425,6 +428,12 @@ async function drain(
425428 const measured = new Map<string, string[]>();
426429 const unmeasured = new Set<string>();
427430
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
428437 type Timed = ShardResult & { seconds: number };
429438
430439 /**
@@ -463,29 +472,60 @@ async function runShard(
463472 const cover = covDir
464473 ? ["--coverage", "--coverage-reporter=lcov", `--coverage-dir=${covDir}`]
465474 : [];
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 };
478500 const through = live
479501 ? (text: string): void => {
480502 process.stdout.write(text);
481503 }
482504 : 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 }
489529 const text = output.join("");
490530 const seconds = ((performance.now() - started) / 1000).toFixed(1);
491531 process.stdout.write(
@@ -524,6 +564,7 @@ async function pool(width: number): Promise<Timed[]> {
524564 results[index] = await runShard(slices[index]!, index, width === 1);
525565 if (results[index]!.code !== 0) {
526566 stop = true;
567 halted = true;
527568 for (const child of children) child.kill("SIGTERM");
528569 }
529570 }
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 @@
33 * the count follows the host and the override, and one red shard is a
44 * red run whose tail names every failing test from every shard.
55 */
6 import { describe, expect, test } from "bun:test";
6 import { afterAll, describe, expect, test } from "bun:test";
77 import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
88 import { tmpdir } from "node:os";
99 import { join } from "node:path";
@@ -11,6 +11,8 @@ import {
1111 SECONDS_PER_BYTE_DEFAULT,
1212 SHARDS_MAX,
1313 besideOthers,
14 failingFiles,
15 onlyTestFailures,
1416 WORKER_MEMORY_BYTES,
1517 CELL_CPU_SHARE,
1618 FILES_PER_PROCESS,
@@ -754,3 +756,87 @@ describe("besideOthers", () => {
754756 });
755757 });
756758 });
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 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs