From 4518004ba3a14a086237db875624e9788a863b22 Mon Sep 17 00:00:00 2001 From: olddonkey Date: Sat, 22 Aug 2026 20:43:29 -0700 Subject: [PATCH 1/2] fix(test): pass --parallel so the full suite finishes instead of reading as hung MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bun run test` spawned `bun test --isolate ./tests/`. With `--isolate` and no `--parallel`, Bun re-evaluates the module graph once per file on a single core. Past ~900 files that stops looking slow and starts looking hung. Measured on this tree (902 files): without --parallel 1 h 29 m, zero output, ~57 % CPU, 8.5 MB RSS, killed with --parallel ~110-190 s, 10x PARALLEL The failure mode is what makes this worth fixing rather than documenting: there is no progress output, one core is pinned, and RSS stays tiny, so it reads as a deadlock. A contributor's reasonable conclusion is that the suite is broken. The stale "normally runs in about 210s" warning is updated for the same reason — that number predates the file count that made the flag necessary. `resolveBunTestArgs` is exported and pinned by tests so the flag cannot be dropped again silently, including the two easy-to-regress cases: a caller supplying `--parallel=N` must not be overridden, and an option-only argv such as `--timeout=30000` must still count as a full-suite run and keep `./tests/`. Gate: 14436 pass / 2 fail; both also fail on untouched upstream/dev at this commit (baseline: 4 fail, a superset). Zero regressions. Note on scope: this is the smallest change that makes the suite runnable. Two adjacent changes are deliberately left out and will be proposed separately — narrowing the exclusive-run lock to full-suite runs (a behavior change that lets two focused runs share one sandboxed HOME), and a `test:changed` script with the contributing-guide updates that go with it. Separately and not addressed here: `tests/key-login-live-update.test.ts` fails standalone and serially on a clean tree, so every full run is red by at least one test regardless of this change. --- bunfig.toml | 1 + scripts/test.ts | 31 +++++++++++++++++++++++++++++-- tests/test-runner.test.ts | 31 ++++++++++++++++++++++++++++++- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/bunfig.toml b/bunfig.toml index 00cbc1231e..318845b44a 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -5,6 +5,7 @@ # so a bare `bun test` — or `bun test tests/` (a substring filter that also matches # devlog/opencode-cursor/tests/) — drags them in and reports hundreds of spurious failures. # `root` pins discovery to ./tests so every invocation stays on the real suite. +# File-level `--parallel` has no bunfig key; `scripts/test.ts` passes it for `bun run test`. # The npm script already uses `bun test ./tests/`; this makes a bare `bun test` behave the same. [test] root = "tests" diff --git a/scripts/test.ts b/scripts/test.ts index 5297a17722..7e055a700f 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -59,6 +59,33 @@ export function createIsolatedTestEnvironment( }; } +function hasCliFlag(requested: string[], name: string): boolean { + return requested.some(arg => arg === name || arg.startsWith(`${name}=`)); +} + +/** True for a filter-less `bun run test`. `--timeout` / `--dots` / `--parallel=N` still count. */ +function isFullSuiteRun(requested: string[]): boolean { + return !requested.some(arg => arg !== "-" && !arg.startsWith("-")); +} + +/** + * Default `bun test` argv for this repo. + * + * `--isolate` keeps a fresh global per file, and is the substring the exclusive-run pgrep + * matches. `--parallel` is what makes the suite finishable: with isolate alone Bun re-evaluates + * the module graph once per file on a single core, so past ~900 files the run stops looking slow + * and starts looking hung — measured here at 1 h 29 m with zero output, ~57 % CPU and 8.5 MB RSS, + * against ~110-190 s for the identical suite with `--parallel`. A caller-supplied `--parallel=N` + * is left alone. + */ +export function resolveBunTestArgs(requested: string[]): string[] { + const args = ["--isolate"]; + if (!hasCliFlag(requested, "--parallel")) args.push("--parallel"); + args.push(...requested); + if (isFullSuiteRun(requested)) args.push("./tests/"); + return args; +} + /** * Other `bun test` runners already on this machine. * @@ -141,7 +168,7 @@ if (import.meta.main) { await waitForExclusiveRun(process.pid); const startedAt = Date.now(); const child = Bun.spawnSync( - [process.execPath, "test", "--isolate", ...(requestedTests.length > 0 ? requestedTests : ["./tests/"])], + [process.execPath, "test", ...resolveBunTestArgs(requestedTests)], { env: isolated.env, stdin: "inherit", @@ -152,7 +179,7 @@ if (import.meta.main) { const elapsedSeconds = Math.round((Date.now() - startedAt) / 1000); if (requestedTests.length === 0 && elapsedSeconds > 600) { console.warn( - `[test] the suite took ${elapsedSeconds}s; it normally runs in about 210s on an idle machine. ` + `[test] the suite took ${elapsedSeconds}s; with --parallel it should finish in a few minutes on an idle machine. ` + "Check for another test runner, a busy CPU, or a test that started polling something real.", ); } diff --git a/tests/test-runner.test.ts b/tests/test-runner.test.ts index ef48654f15..253a9afb41 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import { existsSync } from "node:fs"; import { isAbsolute, join } from "node:path"; -import { createIsolatedTestEnvironment } from "../scripts/test"; +import { createIsolatedTestEnvironment, resolveBunTestArgs } from "../scripts/test"; import { decodeWindowsIdentityPowerShellOutputForTests, windowsIdentityPowerShellCommandForTests, @@ -68,3 +68,32 @@ describe("test runner isolation", () => { }, ); }); + +/** + * Without `--parallel`, `--isolate` re-evaluates the module graph once per file on a single + * core. Past ~900 files that stops reading as slow and starts reading as hung: measured at + * 1 h 29 m with zero output, ~57 % CPU and 8.5 MB RSS, against ~110-190 s for the identical + * suite with the flag. These pin the argv so the flag cannot be dropped again silently. + */ +describe("bun test argv", () => { + test("a filter-less run gets isolate, parallel and the suite path", () => { + expect(resolveBunTestArgs([])).toEqual(["--isolate", "--parallel", "./tests/"]); + }); + + test("a file filter keeps isolate and parallel but no suite path", () => { + expect(resolveBunTestArgs(["tests/foo.test.ts"])) + .toEqual(["--isolate", "--parallel", "tests/foo.test.ts"]); + }); + + test("a caller-supplied concurrency is left alone", () => { + expect(resolveBunTestArgs(["--parallel=2"])) + .toEqual(["--isolate", "--parallel=2", "./tests/"]); + expect(resolveBunTestArgs(["--parallel"])) + .toEqual(["--isolate", "--parallel", "./tests/"]); + }); + + test("option-only arguments still count as a full suite run", () => { + expect(resolveBunTestArgs(["--timeout=30000"])) + .toEqual(["--isolate", "--parallel", "--timeout=30000", "./tests/"]); + }); +}); From 8174ac738a59c1f70adb2b0ea1916de9c23b3f1a Mon Sep 17 00:00:00 2001 From: olddonkey Date: Sat, 22 Aug 2026 20:50:38 -0700 Subject: [PATCH 2/2] chore(test): queue only full-suite runs behind the exclusive-run lock `waitForExclusiveRun` made every invocation wait, including a single-file run. Behind a full suite that is a multi-minute wait for a check that takes seconds, which is the common case during implementation. What the lock actually guards is CPU contention, not shared state: each run gets its own `mkdtemp` sandbox, and the incident it was written for is two full suites crawling until the slowdown reads as a hang. The trade-off is deliberate and worth stating: with `--parallel` a full run already saturates the machine, so a focused run started alongside one does slow it. The judgement is that blocking every focused check behind a multi-minute suite costs more in practice than the contention it avoids. Full suites still queue behind each other, which is the case the lock was written for. Stacked on the `--parallel` fix, which introduces `isFullSuiteRun`. --- scripts/test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/test.ts b/scripts/test.ts index 7e055a700f..9a48648261 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -165,7 +165,15 @@ if (import.meta.main) { const isolated = createIsolatedTestEnvironment(); try { const requestedTests = process.argv.slice(2); - await waitForExclusiveRun(process.pid); + // Only full-suite runs queue. The lock guards CPU contention, not state — each run gets its + // own mkdtemp sandbox — and the case it was written for is two 900-file suites crawling into + // what reads as a hang. A focused file finishes in seconds, so making it wait behind someone + // else's multi-minute suite costs more than the contention it avoids. The trade-off is real + // though: with --parallel a full run already saturates the machine, so a focused run started + // alongside one does slow it. + if (isFullSuiteRun(requestedTests)) { + await waitForExclusiveRun(process.pid); + } const startedAt = Date.now(); const child = Bun.spawnSync( [process.execPath, "test", ...resolveBunTestArgs(requestedTests)],