From 189d70c6d503740057d67807db0c289413e786f7 Mon Sep 17 00:00:00 2001 From: olddonkey Date: Sat, 22 Aug 2026 20:43:29 -0700 Subject: [PATCH 1/7] 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 d62b8c2d57d4dbbbcfc30544da729ce62f930adb Mon Sep 17 00:00:00 2001 From: olddonkey Date: Sat, 22 Aug 2026 21:33:14 -0700 Subject: [PATCH 2/7] fix(test): parse the -- delimiter and pin the flag end to end Two review findings. hasCliFlag/isFullSuiteRun read the whole argv, so `test -- --parallel=2` suppressed the default --parallel even though everything after -- is passed through, and a bare - was classified as an option so `test -` was treated as a full-suite run. The tests also asserted only resolveBunTestArgs output: reverting the spawn call to a hardcoded argv left every assertion green. A spawn test now runs the wrapper against a non-matching filter and asserts bun reports PARALLEL. --- scripts/test.ts | 10 ++++++++-- tests/test-runner.test.ts | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/scripts/test.ts b/scripts/test.ts index 7e055a700f..956c81303d 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -60,12 +60,18 @@ export function createIsolatedTestEnvironment( } function hasCliFlag(requested: string[], name: string): boolean { - return requested.some(arg => arg === name || arg.startsWith(`${name}=`)); + const delimiterIndex = requested.indexOf("--"); + const wrapperArgs = delimiterIndex === -1 ? requested : requested.slice(0, delimiterIndex); + return wrapperArgs.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("-")); + const delimiterIndex = requested.indexOf("--"); + const wrapperArgs = delimiterIndex === -1 ? requested : requested.slice(0, delimiterIndex); + const passedThrough = delimiterIndex === -1 ? [] : requested.slice(delimiterIndex + 1); + return passedThrough.length === 0 + && !wrapperArgs.some(arg => arg === "-" || !arg.startsWith("-")); } /** diff --git a/tests/test-runner.test.ts b/tests/test-runner.test.ts index 253a9afb41..11bf5723f9 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -83,6 +83,8 @@ describe("bun test argv", () => { 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"]); + expect(resolveBunTestArgs(["-"])) + .toEqual(["--isolate", "--parallel", "-"]); }); test("a caller-supplied concurrency is left alone", () => { @@ -96,4 +98,27 @@ describe("bun test argv", () => { expect(resolveBunTestArgs(["--timeout=30000"])) .toEqual(["--isolate", "--parallel", "--timeout=30000", "./tests/"]); }); + + test("arguments after the delimiter are passed through instead of parsed as wrapper flags", () => { + expect(resolveBunTestArgs(["--", "--parallel=2"])) + .toEqual(["--isolate", "--parallel", "--", "--parallel=2"]); + }); + + test("the wrapper passes parallel execution through to bun", () => { + const result = Bun.spawnSync([ + process.execPath, + join(import.meta.dir, "../scripts/test.ts"), + "--pass-with-no-tests", + join(import.meta.dir, "__no_matching_test_file__.test.ts"), + ], { + cwd: join(import.meta.dir, ".."), + env: { ...process.env, OCX_TEST_NO_QUEUE: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + + const output = new TextDecoder().decode(result.stdout) + + new TextDecoder().decode(result.stderr); + expect(output).toContain("PARALLEL"); + }); }); From 8e05c39762ae19f17ba08c7514e66f87ebdc86a7 Mon Sep 17 00:00:00 2001 From: olddonkey Date: Sat, 22 Aug 2026 22:56:19 -0700 Subject: [PATCH 3/7] fix(test): only required-value options consume the next argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CodeRabbit findings, plus a regression the first attempt introduced. isFullSuiteRun read a space-separated option value as a file filter, so `bun run test --timeout 30000` silently stopped being a full-suite run and dropped ./tests/. Bun 1.4.0 accepts that form. The first fix over-corrected: treating every value-taking option as consuming the next argument swallowed the filter in ["--parallel", "tests/foo.test.ts"], so a focused run became a full-suite run — worse than the original bug, and silent. --parallel, --changed, --timings and --coverage take OPTIONAL values, which Bun expects attached with =. Now only required-value options consume the next argument, and all six boundary shapes are pinned as tests. The spawn test also asserted only that the output contained PARALLEL, so it could pass after a nonzero wrapper exit; it now asserts exitCode 0 first, against a real fixture file so a successful run is meaningful. --- scripts/test.ts | 82 ++++++++++++++++++++++++++++++++++++++- tests/test-runner.test.ts | 53 ++++++++++++++++++------- 2 files changed, 118 insertions(+), 17 deletions(-) diff --git a/scripts/test.ts b/scripts/test.ts index 956c81303d..3fcaf7b594 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -65,13 +65,91 @@ function hasCliFlag(requested: string[], name: string): boolean { return wrapperArgs.some(arg => arg === name || arg.startsWith(`${name}=`)); } +// Bun 1.4.0 builds `bun test` options from its test, runtime, transpiler, and base tables. +// Only required values consume the next argument. Optional values such as `--parallel=2` +// must stay attached so a bare option cannot hide the positional filter that follows it. +const BUN_TEST_OPTIONS_REQUIRING_VALUES = new Set([ + // Test options. + "--timeout", + "--rerun-each", + "--retry", + "--seed", + "--coverage-reporter", + "--coverage-dir", + "-t", + "--test-name-pattern", + "--grep", + "--reporter", + "--reporter-outfile", + "--max-concurrency", + "--path-ignore-patterns", + "--parallel-delay", + "--shard", + // Runtime options accepted by `bun test`. + "--watch-kill-signal", + "-r", + "--preload", + "--require", + "--import", + "--cpu-prof-name", + "--cpu-prof-dir", + "--cpu-prof-interval", + "--heap-prof-name", + "--heap-prof-dir", + "--heap-prof-interval", + "--install", + "-e", + "--eval", + "-p", + "--print", + "--port", + "--origin", + "--conditions", + "--fetch-preconnect", + "--max-http-header-size", + "--dns-result-order", + "--redirect-warnings", + "--disable-warning", + "--title", + "--unhandled-rejections", + "--console-depth", + "--user-agent", + "--cron-title", + "--cron-period", + "--trace-event-categories", + "--trace-event-file-pattern", + "--stack-trace-limit", + // Transpiler and base options accepted by `bun test`. + "--main-fields", + "--extension-order", + "--tsconfig-override", + "-d", + "--define", + "--drop", + "--feature", + "-l", + "--loader", + "--jsx-factory", + "--jsx-fragment", + "--jsx-import-source", + "--jsx-runtime", + "--env-file", + "--cwd", +]); + /** True for a filter-less `bun run test`. `--timeout` / `--dots` / `--parallel=N` still count. */ function isFullSuiteRun(requested: string[]): boolean { const delimiterIndex = requested.indexOf("--"); const wrapperArgs = delimiterIndex === -1 ? requested : requested.slice(0, delimiterIndex); const passedThrough = delimiterIndex === -1 ? [] : requested.slice(delimiterIndex + 1); - return passedThrough.length === 0 - && !wrapperArgs.some(arg => arg === "-" || !arg.startsWith("-")); + if (passedThrough.length > 0) return false; + + for (let index = 0; index < wrapperArgs.length; index++) { + const arg = wrapperArgs[index]; + if (arg === "-" || !arg.startsWith("-")) return false; + if (!arg.includes("=") && BUN_TEST_OPTIONS_REQUIRING_VALUES.has(arg)) index++; + } + return true; } /** diff --git a/tests/test-runner.test.ts b/tests/test-runner.test.ts index 11bf5723f9..810acd33c8 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; -import { existsSync } from "node:fs"; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { isAbsolute, join } from "node:path"; import { createIsolatedTestEnvironment, resolveBunTestArgs } from "../scripts/test"; import { @@ -92,11 +93,26 @@ describe("bun test argv", () => { .toEqual(["--isolate", "--parallel=2", "./tests/"]); expect(resolveBunTestArgs(["--parallel"])) .toEqual(["--isolate", "--parallel", "./tests/"]); + expect(resolveBunTestArgs(["--parallel", "tests/foo.test.ts"])) + .toEqual(["--isolate", "--parallel", "tests/foo.test.ts"]); + expect(resolveBunTestArgs(["--parallel=2", "tests/foo.test.ts"])) + .toEqual(["--isolate", "--parallel=2", "tests/foo.test.ts"]); }); test("option-only arguments still count as a full suite run", () => { expect(resolveBunTestArgs(["--timeout=30000"])) .toEqual(["--isolate", "--parallel", "--timeout=30000", "./tests/"]); + expect(resolveBunTestArgs(["--timeout", "30000"])) + .toEqual(["--isolate", "--parallel", "--timeout", "30000", "./tests/"]); + expect(resolveBunTestArgs(["--timeout", "30000", "tests/foo.test.ts"])) + .toEqual(["--isolate", "--parallel", "--timeout", "30000", "tests/foo.test.ts"]); + expect(resolveBunTestArgs(["-t", "serial test"])).toEqual([ + "--isolate", + "--parallel", + "-t", + "serial test", + "./tests/", + ]); }); test("arguments after the delimiter are passed through instead of parsed as wrapper flags", () => { @@ -105,20 +121,27 @@ describe("bun test argv", () => { }); test("the wrapper passes parallel execution through to bun", () => { - const result = Bun.spawnSync([ - process.execPath, - join(import.meta.dir, "../scripts/test.ts"), - "--pass-with-no-tests", - join(import.meta.dir, "__no_matching_test_file__.test.ts"), - ], { - cwd: join(import.meta.dir, ".."), - env: { ...process.env, OCX_TEST_NO_QUEUE: "1" }, - stdout: "pipe", - stderr: "pipe", - }); + const fixtureRoot = mkdtempSync(join(tmpdir(), "opencodex-test-runner-")); + const fixturePath = join(fixtureRoot, "parallel-smoke.test.ts"); + writeFileSync(fixturePath, 'import { test } from "bun:test"; test("smoke", () => {});\n'); + try { + const result = Bun.spawnSync([ + process.execPath, + join(import.meta.dir, "../scripts/test.ts"), + fixturePath, + ], { + cwd: join(import.meta.dir, ".."), + env: { ...process.env, OCX_TEST_NO_QUEUE: "1" }, + stdout: "pipe", + stderr: "pipe", + }); - const output = new TextDecoder().decode(result.stdout) - + new TextDecoder().decode(result.stderr); - expect(output).toContain("PARALLEL"); + const output = new TextDecoder().decode(result.stdout) + + new TextDecoder().decode(result.stderr); + expect(result.exitCode).toBe(0); + expect(output).toContain("PARALLEL"); + } finally { + rmSync(fixtureRoot, { recursive: true, force: true }); + } }); }); From 5cab7ccc84a10d7e21de31461f7caa09d42d836a Mon Sep 17 00:00:00 2001 From: olddonkey Date: Sun, 23 Aug 2026 02:03:24 -0700 Subject: [PATCH 4/7] fix(test): parse separated timings paths --- scripts/test.ts | 1 + tests/test-runner.test.ts | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/scripts/test.ts b/scripts/test.ts index 3fcaf7b594..7d1d73cdbe 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -85,6 +85,7 @@ const BUN_TEST_OPTIONS_REQUIRING_VALUES = new Set([ "--path-ignore-patterns", "--parallel-delay", "--shard", + "--timings", // Runtime options accepted by `bun test`. "--watch-kill-signal", "-r", diff --git a/tests/test-runner.test.ts b/tests/test-runner.test.ts index 810acd33c8..320d8402c7 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -106,6 +106,14 @@ describe("bun test argv", () => { .toEqual(["--isolate", "--parallel", "--timeout", "30000", "./tests/"]); expect(resolveBunTestArgs(["--timeout", "30000", "tests/foo.test.ts"])) .toEqual(["--isolate", "--parallel", "--timeout", "30000", "tests/foo.test.ts"]); + expect(resolveBunTestArgs(["--timings", ".bun-test-timings/current.json"])) + .toEqual([ + "--isolate", + "--parallel", + "--timings", + ".bun-test-timings/current.json", + "./tests/", + ]); expect(resolveBunTestArgs(["-t", "serial test"])).toEqual([ "--isolate", "--parallel", From e6765d180271741bf35e3f392183846cc1a5b3a2 Mon Sep 17 00:00:00 2001 From: olddonkey Date: Sun, 23 Aug 2026 02:15:26 -0700 Subject: [PATCH 5/7] test: cover config values and fixture execution --- scripts/test.ts | 2 ++ tests/test-runner.test.ts | 11 ++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/scripts/test.ts b/scripts/test.ts index 7d1d73cdbe..41d141b7fd 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -136,6 +136,8 @@ const BUN_TEST_OPTIONS_REQUIRING_VALUES = new Set([ "--jsx-runtime", "--env-file", "--cwd", + "-c", + "--config", ]); /** True for a filter-less `bun run test`. `--timeout` / `--dots` / `--parallel=N` still count. */ diff --git a/tests/test-runner.test.ts b/tests/test-runner.test.ts index 320d8402c7..408d5dcf89 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -114,6 +114,10 @@ describe("bun test argv", () => { ".bun-test-timings/current.json", "./tests/", ]); + for (const configFlag of ["-c", "--config"]) { + expect(resolveBunTestArgs([configFlag, "ci.bunfig.toml"])) + .toEqual(["--isolate", "--parallel", configFlag, "ci.bunfig.toml", "./tests/"]); + } expect(resolveBunTestArgs(["-t", "serial test"])).toEqual([ "--isolate", "--parallel", @@ -131,7 +135,11 @@ describe("bun test argv", () => { test("the wrapper passes parallel execution through to bun", () => { const fixtureRoot = mkdtempSync(join(tmpdir(), "opencodex-test-runner-")); const fixturePath = join(fixtureRoot, "parallel-smoke.test.ts"); - writeFileSync(fixturePath, 'import { test } from "bun:test"; test("smoke", () => {});\n'); + const markerPath = join(fixtureRoot, "executed.marker"); + writeFileSync( + fixturePath, + `import { test } from "bun:test"; import { writeFileSync } from "node:fs"; test("smoke", () => writeFileSync(${JSON.stringify(markerPath)}, "executed"));\n`, + ); try { const result = Bun.spawnSync([ process.execPath, @@ -148,6 +156,7 @@ describe("bun test argv", () => { + new TextDecoder().decode(result.stderr); expect(result.exitCode).toBe(0); expect(output).toContain("PARALLEL"); + expect(existsSync(markerPath)).toBe(true); } finally { rmSync(fixtureRoot, { recursive: true, force: true }); } From b15b2088381ae62419e4716491e5765f68935995 Mon Sep 17 00:00:00 2001 From: olddonkey Date: Mon, 24 Aug 2026 15:46:58 -0700 Subject: [PATCH 6/7] fix(test): bound default suite parallelism --- scripts/test.ts | 15 ++++++++++----- tests/test-runner.test.ts | 29 +++++++++++++++-------------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/scripts/test.ts b/scripts/test.ts index 41d141b7fd..0ee72c3c3e 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -65,6 +65,8 @@ function hasCliFlag(requested: string[], name: string): boolean { return wrapperArgs.some(arg => arg === name || arg.startsWith(`${name}=`)); } +const DEFAULT_TEST_PARALLELISM = 4; + // Bun 1.4.0 builds `bun test` options from its test, runtime, transpiler, and base tables. // Only required values consume the next argument. Optional values such as `--parallel=2` // must stay attached so a bare option cannot hide the positional filter that follows it. @@ -159,15 +161,18 @@ function isFullSuiteRun(requested: string[]): boolean { * 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 + * matches. Bounded parallelism 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. + * against a few minutes for the identical suite with four workers. Leaving Bun to select all ten + * workers made deadline-sensitive tests fail under load, so the repository default is deterministic. + * A caller-supplied `--parallel` or `--parallel=N` is left alone. */ export function resolveBunTestArgs(requested: string[]): string[] { const args = ["--isolate"]; - if (!hasCliFlag(requested, "--parallel")) args.push("--parallel"); + if (!hasCliFlag(requested, "--parallel")) { + args.push(`--parallel=${DEFAULT_TEST_PARALLELISM}`); + } args.push(...requested); if (isFullSuiteRun(requested)) args.push("./tests/"); return args; @@ -266,7 +271,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; with --parallel it should finish in a few minutes on an idle machine. ` + `[test] the suite took ${elapsedSeconds}s; with --parallel=${DEFAULT_TEST_PARALLELISM} 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 408d5dcf89..bbd91ffddc 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -73,19 +73,20 @@ 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. + * 1 h 29 m with zero output, ~57 % CPU and 8.5 MB RSS. Four workers keep the suite inside a + * few minutes without the deadline-sensitive failures observed when Bun selected all ten cores. + * These pin the argv so the bound 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 filter-less run gets isolate, bounded parallelism and the suite path", () => { + expect(resolveBunTestArgs([])).toEqual(["--isolate", "--parallel=4", "./tests/"]); }); - test("a file filter keeps isolate and parallel but no suite path", () => { + test("a file filter keeps isolate and bounded parallelism but no suite path", () => { expect(resolveBunTestArgs(["tests/foo.test.ts"])) - .toEqual(["--isolate", "--parallel", "tests/foo.test.ts"]); + .toEqual(["--isolate", "--parallel=4", "tests/foo.test.ts"]); expect(resolveBunTestArgs(["-"])) - .toEqual(["--isolate", "--parallel", "-"]); + .toEqual(["--isolate", "--parallel=4", "-"]); }); test("a caller-supplied concurrency is left alone", () => { @@ -101,26 +102,26 @@ describe("bun test argv", () => { test("option-only arguments still count as a full suite run", () => { expect(resolveBunTestArgs(["--timeout=30000"])) - .toEqual(["--isolate", "--parallel", "--timeout=30000", "./tests/"]); + .toEqual(["--isolate", "--parallel=4", "--timeout=30000", "./tests/"]); expect(resolveBunTestArgs(["--timeout", "30000"])) - .toEqual(["--isolate", "--parallel", "--timeout", "30000", "./tests/"]); + .toEqual(["--isolate", "--parallel=4", "--timeout", "30000", "./tests/"]); expect(resolveBunTestArgs(["--timeout", "30000", "tests/foo.test.ts"])) - .toEqual(["--isolate", "--parallel", "--timeout", "30000", "tests/foo.test.ts"]); + .toEqual(["--isolate", "--parallel=4", "--timeout", "30000", "tests/foo.test.ts"]); expect(resolveBunTestArgs(["--timings", ".bun-test-timings/current.json"])) .toEqual([ "--isolate", - "--parallel", + "--parallel=4", "--timings", ".bun-test-timings/current.json", "./tests/", ]); for (const configFlag of ["-c", "--config"]) { expect(resolveBunTestArgs([configFlag, "ci.bunfig.toml"])) - .toEqual(["--isolate", "--parallel", configFlag, "ci.bunfig.toml", "./tests/"]); + .toEqual(["--isolate", "--parallel=4", configFlag, "ci.bunfig.toml", "./tests/"]); } expect(resolveBunTestArgs(["-t", "serial test"])).toEqual([ "--isolate", - "--parallel", + "--parallel=4", "-t", "serial test", "./tests/", @@ -129,7 +130,7 @@ describe("bun test argv", () => { test("arguments after the delimiter are passed through instead of parsed as wrapper flags", () => { expect(resolveBunTestArgs(["--", "--parallel=2"])) - .toEqual(["--isolate", "--parallel", "--", "--parallel=2"]); + .toEqual(["--isolate", "--parallel=4", "--", "--parallel=2"]); }); test("the wrapper passes parallel execution through to bun", () => { From 9f8fc2bf9557b25ad9cafab26964e50e9ea03c52 Mon Sep 17 00:00:00 2001 From: olddonkey Date: Mon, 24 Aug 2026 20:21:39 -0700 Subject: [PATCH 7/7] fix(test): isolate load-sensitive full-suite lanes --- scripts/test-run-lock.ts | 227 ++++++++++++++++++++++++++++++ scripts/test.ts | 210 +++++++++++++++++----------- tests/preload.ts | 23 +++ tests/release-helper.test.ts | 261 ++++++++++++++++++++--------------- tests/server-auth.test.ts | 2 +- tests/test-runner.test.ts | 145 ++++++++++++++++++- 6 files changed, 669 insertions(+), 199 deletions(-) create mode 100644 scripts/test-run-lock.ts diff --git a/scripts/test-run-lock.ts b/scripts/test-run-lock.ts new file mode 100644 index 0000000000..d1c65487d5 --- /dev/null +++ b/scripts/test-run-lock.ts @@ -0,0 +1,227 @@ +import { randomUUID } from "node:crypto"; +import { + mkdirSync, + readFileSync, + readdirSync, + renameSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +export const TEST_RUN_ID_ENV = "OCX_TEST_RUN_ID"; +export const TEST_RUN_NO_QUEUE_ENV = "OCX_TEST_NO_QUEUE"; +const DEFAULT_LOCK_PATH = join(tmpdir(), "opencodex-bun-test.lock"); +const OWNER_FILE = "owner.json"; +const MEMBERS_DIR = "members"; +const INCOMPLETE_OWNER_GRACE_MS = 10_000; + +export interface TestRunLockOwner { + version: 1; + runId: string; + token: string; + pid: number; + acquiredAt: string; +} + +export interface TestRunLock { + acquired: boolean; + owner: TestRunLockOwner | null; + release(): void; +} + +export interface AcquireTestRunLockOptions { + runId: string; + ownerPid?: number; + lockPath?: string; + pollMs?: number; + maxWaitMs?: number; + env?: NodeJS.ProcessEnv; + onWait?: (owner: TestRunLockOwner | null) => void; + onAcquiredAfterWait?: (elapsedMs: number) => void; +} + +export interface BareTestRunIdentity { + ownerPid: number; + runId: string; +} + +/** + * Give one bare Bun invocation a stable identity without conflating sibling commands. + * + * Without `--parallel`, the preload runs in the test-runner process itself and its + * parent may be a long-lived shell or agent host shared by many unrelated commands. + * Bun parallel workers expose `BUN_TEST_WORKER_ID` and share one short-lived parent + * controller, so only that case may safely rendezvous on the parent PID. + */ +export function resolveBareTestRunIdentity(options: { + pid: number; + ppid: number; + workerId?: string; +}): BareTestRunIdentity { + const coordinatorPid = options.workerId ? options.ppid : options.pid; + return { ownerPid: options.pid, runId: `bare-${coordinatorPid}` }; +} + +function ownerPath(lockPath: string): string { + return join(lockPath, OWNER_FILE); +} + +function memberPath(lockPath: string, owner: TestRunLockOwner, pid: number): string { + return join(lockPath, MEMBERS_DIR, `${pid}-${owner.token}`); +} + +function readOwner(lockPath: string): TestRunLockOwner | null { + try { + const parsed = JSON.parse(readFileSync(ownerPath(lockPath), "utf8")) as Partial; + if (parsed.version !== 1 || typeof parsed.runId !== "string" || typeof parsed.token !== "string" + || !Number.isInteger(parsed.pid) || (parsed.pid ?? 0) <= 0 || typeof parsed.acquiredAt !== "string") { + return null; + } + return parsed as TestRunLockOwner; + } catch { + return null; + } +} + +function processIsAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === "EPERM"; + } +} + +function liveMemberExists(lockPath: string, owner: TestRunLockOwner): boolean { + try { + return readdirSync(join(lockPath, MEMBERS_DIR)).some(file => { + const suffix = `-${owner.token}`; + if (!file.endsWith(suffix)) return false; + const pid = Number.parseInt(file.slice(0, -suffix.length), 10); + return Number.isInteger(pid) && pid > 0 && processIsAlive(pid); + }); + } catch { + return false; + } +} + +function lockIsLive(lockPath: string, owner: TestRunLockOwner): boolean { + return processIsAlive(owner.pid) || liveMemberExists(lockPath, owner); +} + +function registerMember(lockPath: string, owner: TestRunLockOwner, pid: number): boolean { + const membersPath = join(lockPath, MEMBERS_DIR); + try { + mkdirSync(membersPath, { recursive: true, mode: 0o700 }); + writeFileSync(memberPath(lockPath, owner, pid), "", { flag: "a", mode: 0o600 }); + } catch { + return false; + } + if (ownsLock(lockPath, owner)) return true; + rmSync(memberPath(lockPath, owner, pid), { force: true }); + return false; +} + +function incompleteOwnerIsRecent(lockPath: string): boolean { + try { + return Date.now() - statSync(lockPath).mtimeMs < INCOMPLETE_OWNER_GRACE_MS; + } catch { + return false; + } +} + +function reclaimStaleLock(lockPath: string): boolean { + const stalePath = `${lockPath}.stale-${process.pid}-${randomUUID()}`; + try { + renameSync(lockPath, stalePath); + } catch (error) { + if (["ENOENT", "EACCES", "EPERM"].includes((error as NodeJS.ErrnoException).code ?? "")) return false; + throw error; + } + rmSync(stalePath, { recursive: true, force: true }); + return true; +} + +function ownsLock(lockPath: string, owner: TestRunLockOwner): boolean { + const current = readOwner(lockPath); + return current?.runId === owner.runId && current.token === owner.token && current.pid === owner.pid; +} + +/** + * Acquire the machine-wide OpenCodex Bun-test lock. + * + * `mkdir` is the cross-platform atomic primitive. The owner PID makes a lock left by + * SIGKILL recoverable, while the run ID lets every worker belonging to one bare + * `bun test --parallel` invocation join the same lock without blocking its siblings. + * Joiners register their own PIDs so a worker-owned lock remains live if its first + * worker exits before the rest of the pool. + */ +export async function acquireTestRunLock(options: AcquireTestRunLockOptions): Promise { + const env = options.env ?? process.env; + if (env[TEST_RUN_NO_QUEUE_ENV] === "1") { + return { acquired: false, owner: null, release() {} }; + } + + const lockPath = options.lockPath ?? DEFAULT_LOCK_PATH; + const ownerPid = options.ownerPid ?? process.pid; + const pollMs = Math.max(1, options.pollMs ?? 5_000); + const maxWaitMs = Math.max(pollMs, options.maxWaitMs ?? 45 * 60 * 1000); + const startedAt = Date.now(); + let announced = false; + + for (;;) { + const owner: TestRunLockOwner = { + version: 1, + runId: options.runId, + token: randomUUID(), + pid: ownerPid, + acquiredAt: new Date().toISOString(), + }; + try { + mkdirSync(lockPath, { mode: 0o700 }); + try { + writeFileSync(ownerPath(lockPath), `${JSON.stringify(owner)}\n`, { encoding: "utf8", mode: 0o600, flag: "wx" }); + } catch (error) { + rmSync(lockPath, { recursive: true, force: true }); + throw error; + } + if (announced) options.onAcquiredAfterWait?.(Date.now() - startedAt); + return { + acquired: true, + owner, + release() { + if (!ownsLock(lockPath, owner)) return; + reclaimStaleLock(lockPath); + }, + }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; + } + + const current = readOwner(lockPath); + if (current?.runId === options.runId && lockIsLive(lockPath, current)) { + if (registerMember(lockPath, current, process.pid)) { + return { acquired: false, owner: current, release() {} }; + } + continue; + } + const ownerIsLive = current ? lockIsLive(lockPath, current) : incompleteOwnerIsRecent(lockPath); + if (!ownerIsLive && reclaimStaleLock(lockPath)) continue; + + if (!announced) { + announced = true; + options.onWait?.(current); + } + if (Date.now() - startedAt >= maxWaitMs) { + const holder = current ? `pid ${current.pid} (run ${current.runId})` : "an initializing runner"; + throw new Error( + `timed out after ${Math.round(maxWaitMs / 1000)}s waiting for ${holder} to release ${lockPath}; ` + + `set ${TEST_RUN_NO_QUEUE_ENV}=1 only when overlapping test runners are intentional`, + ); + } + await Bun.sleep(pollMs); + } +} diff --git a/scripts/test.ts b/scripts/test.ts index 0ee72c3c3e..29357a681a 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -1,6 +1,8 @@ +import { randomUUID } from "node:crypto"; import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; +import { acquireTestRunLock, TEST_RUN_ID_ENV } from "./test-run-lock"; export interface IsolatedTestEnvironment { root: string; @@ -160,8 +162,8 @@ function isFullSuiteRun(requested: string[]): boolean { /** * Default `bun test` argv for this repo. * - * `--isolate` keeps a fresh global per file, and is the substring the exclusive-run pgrep - * matches. Bounded parallelism is what makes the suite finishable: with isolate alone Bun re-evaluates + * `--isolate` keeps a fresh global per file. Bounded parallelism 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 a few minutes for the identical suite with four workers. Leaving Bun to select all ten @@ -178,105 +180,145 @@ export function resolveBunTestArgs(requested: string[]): string[] { return args; } -/** - * Other `bun test` runners already on this machine. - * - * Two full suites sharing one CPU do not fail — they crawl. A run that normally - * finishes in about 210s took 26 minutes against a runner an earlier session had - * left behind, and neither process said anything, so the slowdown read as a hang - * in this suite. Bun's own timeouts cannot see the contention, so name it here. - * - * `pgrep` is absent on Windows and may exit non-zero for "no matches"; both cases - * mean "nothing to warn about" rather than an error worth failing a test run over. - */ -function findCompetingTestRunners(selfPid: number): number[] { - try { - const found = Bun.spawnSync(["pgrep", "-f", "bun.*test --isolate"], { - stdout: "pipe", - stderr: "ignore", - }); - if (!found.success) return []; - return new TextDecoder().decode(found.stdout) - .split("\n") - .map(line => Number.parseInt(line.trim(), 10)) - .filter(pid => Number.isInteger(pid) && pid > 0 && pid !== selfPid); - } catch { - return []; +export const SERIAL_FULL_SUITE_FILES = [ + "codex-shim.test.ts", + "cursor-native-exec-shell.test.ts", + "issue-452-empty-503.test.ts", + "openai-provider-option-e2e.test.ts", + "release-helper.test.ts", + "update-stop-first.test.ts", +] as const; + +const SERIAL_LANE_TIMEOUT_MS: Partial> = { + // This file intentionally exercises 33 complete release-script subprocess trees. + // It is ~90s on an idle machine and measured at ~170s under unrelated host load. + "release-helper.test.ts": 5 * 60 * 1000, +}; + +export interface BunTestLane { + label: string; + args: string[]; + timeoutMs: number; +} + +function withoutParallelOverride(requested: string[]): string[] { + return requested.filter(arg => arg !== "--parallel" && !arg.startsWith("--parallel=")); +} + +function canUseSerialLanes(requested: string[]): boolean { + if (!isFullSuiteRun(requested)) return false; + return !["--changed", "--shard", "--reporter-outfile", "--update-timings"].some(flag => hasCliFlag(requested, flag)); +} + +/** Build the default full-suite plan: one bounded main lane plus isolated risky files. */ +export function resolveBunTestPlan(requested: string[]): BunTestLane[] { + if (!canUseSerialLanes(requested)) { + return [{ label: "suite", args: resolveBunTestArgs(requested), timeoutMs: 15 * 60 * 1000 }]; } + + const mainArgs = resolveBunTestArgs(requested); + const rootIndex = mainArgs.lastIndexOf("./tests/"); + const ignores = SERIAL_FULL_SUITE_FILES.flatMap(file => ["--path-ignore-patterns", `**/${file}`]); + mainArgs.splice(rootIndex === -1 ? mainArgs.length : rootIndex, 0, ...ignores); + const serialRequested = withoutParallelOverride(requested); + return [ + { label: "parallel suite", args: mainArgs, timeoutMs: 15 * 60 * 1000 }, + ...SERIAL_FULL_SUITE_FILES.map(file => ({ + label: file, + args: resolveBunTestArgs(["--parallel=1", ...serialRequested, `./tests/${file}`]), + timeoutMs: SERIAL_LANE_TIMEOUT_MS[file] ?? 3 * 60 * 1000, + })), + ]; } -/** - * Wait until this machine has no other full-suite runner, then proceed. - * - * Warning about contention was not enough: the warning scrolls past, the run still - * starts, and four concurrent suites drove load average to 10 and turned a ~210s - * suite into a 13-minute one that read as a hang. Agents in parallel worktrees each - * think they are the only runner, so the serialization has to live here rather than - * in anyone's discipline. - * - * Queue rather than refuse: a failed `bun run test` invites `bun test` directly, - * which bypasses this file entirely. Waiting is the behavior that survives being - * worked around. `OCX_TEST_NO_QUEUE=1` opts out for anyone who really wants overlap. - */ -async function waitForExclusiveRun(selfPid: number): Promise { - if (process.env.OCX_TEST_NO_QUEUE === "1") return; - const pollMs = 5_000; - // Long enough for a full suite plus slack; past this, assume the holder is wedged - // rather than working and let this run start anyway. - const maxWaitMs = 45 * 60 * 1000; +function waitWithTimeout(promise: Promise, timeoutMs: number): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => resolve(null), timeoutMs); + promise.then( + value => { + clearTimeout(timer); + resolve(value); + }, + error => { + clearTimeout(timer); + reject(error); + }, + ); + }); +} + +async function runTestLane(lane: BunTestLane, runId: string): Promise { + const isolated = createIsolatedTestEnvironment({ ...process.env, [TEST_RUN_ID_ENV]: runId }); const startedAt = Date.now(); - let announced = false; - for (;;) { - const competing = findCompetingTestRunners(selfPid); - if (competing.length === 0) { - if (announced) { - console.warn(`[test] the other runner(s) finished after ${Math.round((Date.now() - startedAt) / 1000)}s; starting.`); + let interrupted: NodeJS.Signals | null = null; + const child = Bun.spawn([process.execPath, "test", ...lane.args], { + env: isolated.env, + stdin: "inherit", + stdout: "inherit", + stderr: "inherit", + }); + const forward = (signal: NodeJS.Signals) => { + interrupted = signal; + try { child.kill(signal); } catch { /* child already exited */ } + }; + const onInterrupt = () => forward("SIGINT"); + const onTerminate = () => forward("SIGTERM"); + process.once("SIGINT", onInterrupt); + process.once("SIGTERM", onTerminate); + + const exited = child.exited; + try { + const exitCode = await waitWithTimeout(exited, lane.timeoutMs); + if (exitCode === null) { + console.error(`[test] ${lane.label} exceeded ${Math.round(lane.timeoutMs / 1000)}s; terminating pid ${child.pid}.`); + try { child.kill("SIGTERM"); } catch { /* child already exited */ } + const graceful = await waitWithTimeout(exited, 5_000); + if (graceful === null) { + try { child.kill("SIGKILL"); } catch { /* child already exited */ } + await waitWithTimeout(exited, 2_000); } - return; + return 124; } - if (Date.now() - startedAt > maxWaitMs) { - console.warn( - `[test] still waiting on pid ${competing.join(", ")} after ${Math.round(maxWaitMs / 60000)} minutes. ` - + "Assuming they are stuck and starting anyway; expect a slow run.", - ); - return; - } - if (!announced) { - announced = true; - console.warn( - `[test] ${competing.length} other bun test runner(s) already running (pid ${competing.join(", ")}). ` - + "Waiting for them to finish so the suites do not fight over the CPU. " - + "Set OCX_TEST_NO_QUEUE=1 to run concurrently anyway.", - ); - } - await Bun.sleep(pollMs); + if (interrupted === "SIGINT") return 130; + if (interrupted === "SIGTERM") return 143; + const seconds = ((Date.now() - startedAt) / 1000).toFixed(1); + console.warn(`[test] ${lane.label} finished in ${seconds}s (exit ${exitCode}).`); + return exitCode; + } finally { + process.off("SIGINT", onInterrupt); + process.off("SIGTERM", onTerminate); + isolated.cleanup(); } } if (import.meta.main) { - const isolated = createIsolatedTestEnvironment(); + const requestedTests = process.argv.slice(2); + const runId = randomUUID(); + const lock = await acquireTestRunLock({ + runId, + onWait: owner => console.warn( + `[test] another Bun test run${owner ? ` (pid ${owner.pid})` : ""} holds the machine lock; waiting. ` + + "Set OCX_TEST_NO_QUEUE=1 only for intentional overlap.", + ), + onAcquiredAfterWait: elapsedMs => console.warn(`[test] acquired the machine lock after ${Math.round(elapsedMs / 1000)}s.`), + }); + const startedAt = Date.now(); try { - const requestedTests = process.argv.slice(2); - await waitForExclusiveRun(process.pid); - const startedAt = Date.now(); - const child = Bun.spawnSync( - [process.execPath, "test", ...resolveBunTestArgs(requestedTests)], - { - env: isolated.env, - stdin: "inherit", - stdout: "inherit", - stderr: "inherit", - }, - ); + let exitCode = 0; + for (const lane of resolveBunTestPlan(requestedTests)) { + const laneExitCode = await runTestLane(lane, runId); + if (laneExitCode !== 0 && exitCode === 0) exitCode = laneExitCode; + if ([124, 130, 143].includes(laneExitCode)) break; + } const elapsedSeconds = Math.round((Date.now() - startedAt) / 1000); - if (requestedTests.length === 0 && elapsedSeconds > 600) { + if (isFullSuiteRun(requestedTests) && elapsedSeconds > 600) { console.warn( `[test] the suite took ${elapsedSeconds}s; with --parallel=${DEFAULT_TEST_PARALLELISM} 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.", ); } - process.exitCode = child.exitCode ?? 1; + process.exitCode = exitCode; } finally { - isolated.cleanup(); + lock.release(); } } diff --git a/tests/preload.ts b/tests/preload.ts index b728565f42..37b2233df0 100644 --- a/tests/preload.ts +++ b/tests/preload.ts @@ -13,8 +13,31 @@ */ import { isTestHomeGuardArmed, protectedHomeForTests } from "../src/lib/test-home-guard"; import { createIsolatedTestEnvironment } from "../scripts/test"; +import { acquireTestRunLock, resolveBareTestRunIdentity, TEST_RUN_ID_ENV } from "../scripts/test-run-lock"; import { rmSync } from "node:fs"; +// `scripts/test.ts` owns the lock for wrapped runs. A bare `bun test` has no wrapper, +// so a single-process runner uses its own PID while true parallel workers rendezvous +// on their short-lived controller PID. The first worker acquires the lock and siblings +// join it. The bare-run lock is deliberately left for the next invocation to reclaim +// after every registered worker exits — releasing it from an early-finishing worker +// would let another suite overlap the remaining workers. +const wrappedRunId = process.env[TEST_RUN_ID_ENV]?.trim(); +const bareIdentity = resolveBareTestRunIdentity({ + pid: process.pid, + ppid: process.ppid, + workerId: process.env.BUN_TEST_WORKER_ID, +}); +const runId = wrappedRunId || bareIdentity.runId; +process.env[TEST_RUN_ID_ENV] = runId; +await acquireTestRunLock({ + runId, + ownerPid: bareIdentity.ownerPid, + onWait: owner => console.warn( + `[test] bare Bun worker ${process.pid} is waiting for test run${owner ? ` pid ${owner.pid}` : ""} to release the machine lock.`, + ), +}); + // Under `bun run test` the wrapper already handed us a sandbox (and OCX_REAL_HOME so the // guard could still see the true home). Isolating again is harmless and deliberate: the // alternative — inferring "already isolated" from path shapes — would trust exactly the diff --git a/tests/release-helper.test.ts b/tests/release-helper.test.ts index d5ce3fa614..90e2f55c5b 100644 --- a/tests/release-helper.test.ts +++ b/tests/release-helper.test.ts @@ -1,6 +1,5 @@ import { describe, expect, setDefaultTimeout, test } from "bun:test"; import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { spawnSync } from "node:child_process"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -41,6 +40,39 @@ interface SshInvocation { args: string[]; } +interface CapturedProcessResult { + status: number | null; + stderr: string; + stdout: string; +} + +async function runCaptured( + command: string, + args: string[], + options: { cwd: string; env: Record; timeoutMs?: number }, +): Promise { + const child = Bun.spawn([command, ...args], { + cwd: options.cwd, + env: options.env, + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }); + const stdout = new Response(child.stdout).text(); + const stderr = new Response(child.stderr).text(); + let timedOut = false; + const timer = setTimeout(() => { + timedOut = true; + try { child.kill("SIGKILL"); } catch { /* child already exited */ } + }, options.timeoutMs ?? 20_000); + try { + const [status, capturedStdout, capturedStderr] = await Promise.all([child.exited, stdout, stderr]); + return { status: timedOut ? null : status, stdout: capturedStdout, stderr: capturedStderr }; + } finally { + clearTimeout(timer); + } +} + function writeExecutable(path: string, contents: string): void { writeFileSync(path, contents, "utf8"); chmodSync(path, 0o755); @@ -215,7 +247,7 @@ function findCallIndex(calls: LoggedCall[], name: string, matcher: (call: Logged return calls.findIndex(call => call.name === name && matcher(call)); } -function runRelease(version: string, scenario: ReleaseScenario = {}) { +async function runRelease(version: string, scenario: ReleaseScenario = {}) { const shimDir = mkdtempSync(join(tmpdir(), "ocx-release-helper-")); const logPath = join(shimDir, "release-log.jsonl"); writeFileSync(logPath, "", "utf8"); @@ -241,31 +273,32 @@ function runRelease(version: string, scenario: ReleaseScenario = {}) { const pathKey = process.platform === "win32" ? "Path" : "PATH"; const pathValue = `${shimDir}${process.platform === "win32" ? ";" : ":"}${process.env.PATH ?? process.env.Path ?? ""}`; - const result = spawnSync(process.execPath, [releaseScriptPath, version], { - cwd: repoRoot, - env: { - ...inheritedEnv, - [pathKey]: pathValue, - FAKE_RELEASE_LOG: logPath, - FAKE_GIT_BRANCH: scenario.branch ?? "main", - FAKE_GIT_HEAD_SHA: scenario.headSha ?? "abc123def456", - ...(scenario.remoteHeadSha ? { FAKE_GIT_REMOTE_HEAD_SHA: scenario.remoteHeadSha } : {}), - FAKE_BUN_TSC_EXIT_CODE: String(scenario.typecheckExitCode ?? 0), - FAKE_BUN_TEST_EXIT_CODE: String(scenario.testExitCode ?? 0), - FAKE_BUN_PRIVACY_EXIT_CODE: String(scenario.privacyExitCode ?? 0), - ...(scenario.npmLatest ? { FAKE_NPM_LATEST: scenario.npmLatest } : {}), - ...(scenario.npmPreview ? { FAKE_NPM_PREVIEW: scenario.npmPreview } : {}), - ...(scenario.releaseSshKey ? { OCX_RELEASE_SSH_KEY: scenario.releaseSshKey } : {}), - ...(scenario.releaseSshRepo ? { OCX_RELEASE_SSH_REPO: scenario.releaseSshRepo } : {}), - ...(scenario.pendingBump ? { FAKE_GIT_PENDING_BUMP: " M package.json" } : {}), - ...(scenario.originUrl ? { FAKE_GIT_ORIGIN_URL: scenario.originUrl } : {}), - }, - encoding: "utf8", - }); - - const calls = readLoggedCalls(logPath); - rmSync(shimDir, { recursive: true, force: true }); - return { calls, result }; + const env = { + ...inheritedEnv, + [pathKey]: pathValue, + FAKE_RELEASE_LOG: logPath, + FAKE_GIT_BRANCH: scenario.branch ?? "main", + FAKE_GIT_HEAD_SHA: scenario.headSha ?? "abc123def456", + ...(scenario.remoteHeadSha ? { FAKE_GIT_REMOTE_HEAD_SHA: scenario.remoteHeadSha } : {}), + FAKE_BUN_TSC_EXIT_CODE: String(scenario.typecheckExitCode ?? 0), + FAKE_BUN_TEST_EXIT_CODE: String(scenario.testExitCode ?? 0), + FAKE_BUN_PRIVACY_EXIT_CODE: String(scenario.privacyExitCode ?? 0), + ...(scenario.npmLatest ? { FAKE_NPM_LATEST: scenario.npmLatest } : {}), + ...(scenario.npmPreview ? { FAKE_NPM_PREVIEW: scenario.npmPreview } : {}), + ...(scenario.releaseSshKey ? { OCX_RELEASE_SSH_KEY: scenario.releaseSshKey } : {}), + ...(scenario.releaseSshRepo ? { OCX_RELEASE_SSH_REPO: scenario.releaseSshRepo } : {}), + ...(scenario.pendingBump ? { FAKE_GIT_PENDING_BUMP: " M package.json" } : {}), + ...(scenario.originUrl ? { FAKE_GIT_ORIGIN_URL: scenario.originUrl } : {}), + }; + try { + const result = await runCaptured(process.execPath, [releaseScriptPath, version], { + cwd: repoRoot, + env, + }); + return { calls: readLoggedCalls(logPath), result }; + } finally { + rmSync(shimDir, { recursive: true, force: true }); + } } /** @@ -275,7 +308,7 @@ function runRelease(version: string, scenario: ReleaseScenario = {}) { * contract for `GIT_SSH_COMMAND`. Exercising a real Git process here catches quoting that looks * correct in text yet splits, substitutes, or reinterprets the private-key path before SSH sees it. */ -function executeGitSshCommand(gitSshCommand: string): { calls: SshInvocation[]; result: ReturnType } { +async function executeGitSshCommand(gitSshCommand: string): Promise<{ calls: SshInvocation[]; result: CapturedProcessResult }> { const shimDir = mkdtempSync(join(tmpdir(), "ocx-release-ssh-")); const logPath = join(shimDir, "ssh-log.jsonl"); const jsPath = join(shimDir, "ssh.js"); @@ -296,26 +329,29 @@ process.exit(0); const inheritedEnv = Object.fromEntries( Object.entries(process.env).filter(([key]) => key !== "GIT_SSH" && key !== "GIT_SSH_COMMAND"), ); - const result = spawnSync("git", ["ls-remote", "ssh://example.invalid/owner/repository.git"], { - cwd: repoRoot, - env: { - ...inheritedEnv, - FAKE_SSH_LOG: logPath, - GIT_SSH_COMMAND: nativeFakeCommand, - }, - encoding: "utf8", - }); - const raw = readFileSync(logPath, "utf8").trim(); - const calls = raw - ? raw.split(/\r?\n/).filter(Boolean).map(line => JSON.parse(line) as SshInvocation) - : []; - rmSync(shimDir, { recursive: true, force: true }); - return { calls, result }; + const env = { + ...inheritedEnv, + FAKE_SSH_LOG: logPath, + GIT_SSH_COMMAND: nativeFakeCommand, + }; + try { + const result = await runCaptured("git", ["ls-remote", "ssh://example.invalid/owner/repository.git"], { + cwd: repoRoot, + env, + }); + const raw = readFileSync(logPath, "utf8").trim(); + const calls = raw + ? raw.split(/\r?\n/).filter(Boolean).map(line => JSON.parse(line) as SshInvocation) + : []; + return { calls, result }; + } finally { + rmSync(shimDir, { recursive: true, force: true }); + } } describe("release helper", () => { - test("preflight runs the shared audit, typecheck, test suite, and privacy scan before version bump", () => { - const { calls, result } = runRelease("9.9.9"); + test("preflight runs the shared audit, typecheck, test suite, and privacy scan before version bump", async () => { + const { calls, result } = await runRelease("9.9.9"); // Report what the script actually said. A bare status assertion turned a // Windows-only spawn failure into "Expected: 0 Received: 1" with no cause, @@ -343,8 +379,8 @@ describe("release helper", () => { expect(dispatchIndex).toBeGreaterThan(versionIndex); }); - test("an obsolete version that would move latest backwards aborts before the bump", () => { - const { calls, result } = runRelease("9.9.8", { npmLatest: "9.9.9" }); + test("an obsolete version that would move latest backwards aborts before the bump", async () => { + const { calls, result } = await runRelease("9.9.8", { npmLatest: "9.9.9" }); expect(result.status).not.toBe(0); expect(result.stderr ?? "").toContain("does not move the 'latest' channel forward"); @@ -352,21 +388,21 @@ describe("release helper", () => { expect(findCallIndex(calls, "git", call => call.args[0] === "commit")).toBe(-1); }); - test("a version newer than the channel tip passes the forward guard", () => { - const { calls, result } = runRelease("9.9.10", { npmLatest: "9.9.9" }); + test("a version newer than the channel tip passes the forward guard", async () => { + const { calls, result } = await runRelease("9.9.10", { npmLatest: "9.9.9" }); expect(`${result.status}\n${result.stderr ?? ""}`.trim()).toBe("0"); expect(findCallIndex(calls, "npm", call => call.args.join(" ") === "version 9.9.10 --no-git-tag-version")).toBeGreaterThanOrEqual(0); }); - test("preview releases compare against the preview channel, not latest", () => { - const { result } = runRelease("9.9.9-preview.2", { branch: "preview", npmLatest: "10.0.0", npmPreview: "9.9.9-preview.1" }); + test("preview releases compare against the preview channel, not latest", async () => { + const { result } = await runRelease("9.9.9-preview.2", { branch: "preview", npmLatest: "10.0.0", npmPreview: "9.9.9-preview.1" }); expect(`${result.status}\n${result.stderr ?? ""}`.trim()).toBe("0"); }); - test("failed privacy scan aborts before version bump, commit, and push", () => { - const { calls, result } = runRelease("9.9.9", { privacyExitCode: 1 }); + test("failed privacy scan aborts before version bump, commit, and push", async () => { + const { calls, result } = await runRelease("9.9.9", { privacyExitCode: 1 }); expect(result.status).not.toBe(0); expect(findCallIndex(calls, "bun", call => call.args.join(" ") === "run privacy:scan")).toBeGreaterThanOrEqual(0); @@ -375,8 +411,8 @@ describe("release helper", () => { expect(findCallIndex(calls, "git", call => call.args[0] === "push")).toBe(-1); }); - test("preview branch still defaults to preview tag and dry-run dispatch", () => { - const { calls, result } = runRelease("9.9.9-preview.1", { branch: "preview" }); + test("preview branch still defaults to preview tag and dry-run dispatch", async () => { + const { calls, result } = await runRelease("9.9.9-preview.1", { branch: "preview" }); expect(result.status).toBe(0); expect(findCallIndex(calls, "gh", call => @@ -388,8 +424,8 @@ describe("release helper", () => { )).toBeGreaterThanOrEqual(0); }); - test("dispatch pins the audited release SHA via expected-sha", () => { - const { calls, result } = runRelease("9.9.9", { headSha: "deadbeefcafe1234" }); + test("dispatch pins the audited release SHA via expected-sha", async () => { + const { calls, result } = await runRelease("9.9.9", { headSha: "deadbeefcafe1234" }); expect(result.status).toBe(0); expect(findCallIndex(calls, "gh", call => @@ -410,8 +446,8 @@ describe("release helper", () => { * rejected by the ruleset again), and the default path must stay byte-identical so a contributor * or CI clone without the variable is unaffected. */ - test("the protected push uses the release deploy key only when one is configured", () => { - const { calls, result } = runRelease("9.9.9", { + test("the protected push uses the release deploy key only when one is configured", async () => { + const { calls, result } = await runRelease("9.9.9", { releaseSshKey: "/tmp/ocx-release-key", releaseSshRepo: sshTarget, pendingBump: true, @@ -430,8 +466,8 @@ describe("release helper", () => { * (`C:\Users\Jun Kim\.ssh\...`) is exactly that shape, and ssh would read the tail as its next * flag. Assert the whole command string, not a substring: `toContain` passes on the broken form. */ - test("a key path with spaces and backslashes stays a single ssh argument", () => { - const { calls } = runRelease("9.9.9", { + test("a key path with spaces and backslashes stays a single ssh argument", async () => { + const { calls } = await runRelease("9.9.9", { releaseSshKey: "C:\\Users\\Jun Kim\\.ssh\\ocx release key", pendingBump: true, }); @@ -440,9 +476,9 @@ describe("release helper", () => { expect(push?.gitSshCommand).toBe('ssh -i "C:\\\\Users\\\\Jun Kim\\\\.ssh\\\\ocx release key" -o IdentitiesOnly=yes'); }); - test("Git passes the emitted deploy-key path to SSH as one literal argument", () => { + test("Git passes the emitted deploy-key path to SSH as one literal argument", async () => { const keyPath = 'C:\\Users\\Jun Kim\\.ssh\\ocx "quoted" $HOME $(not-run) `not-run`; key'; - const { calls: releaseCalls } = runRelease("9.9.9", { + const { calls: releaseCalls } = await runRelease("9.9.9", { releaseSshKey: keyPath, releaseSshRepo: sshTarget, pendingBump: true, @@ -450,7 +486,7 @@ describe("release helper", () => { const push = releaseCalls.find(call => call.name === "git" && call.args[0] === "push"); expect(push?.gitSshCommand).toBeDefined(); - const { calls } = executeGitSshCommand(push?.gitSshCommand ?? ""); + const { calls } = await executeGitSshCommand(push?.gitSshCommand ?? ""); expect(calls.length).toBeGreaterThan(0); for (const call of calls) { const identityIndex = call.args.indexOf("-i"); @@ -463,8 +499,8 @@ describe("release helper", () => { * The SSH target is derived from `origin` rather than hardcoded, so a fork's release pushes to * the fork instead of silently targeting upstream. */ - test("the ssh push target follows the configured origin remote", () => { - const { calls } = runRelease("9.9.9", { + test("the ssh push target follows the configured origin remote", async () => { + const { calls } = await runRelease("9.9.9", { releaseSshKey: "/tmp/k", originUrl: "https://github.com/someone-else/opencodex.git", pendingBump: true, @@ -479,8 +515,8 @@ describe("release helper", () => { * failing command, so a folded `user:token@` would put the token on the terminal and in the * release log. Refuse instead of building a target. */ - test("an origin carrying credentials is refused rather than transplanted", () => { - const { calls, result } = runRelease("9.9.9", { + test("an origin carrying credentials is refused rather than transplanted", async () => { + const { calls, result } = await runRelease("9.9.9", { releaseSshKey: "/tmp/k", originUrl: `https://x-access-token:SECRET@${"github.com"}/lidge-jun/opencodex.git`, pendingBump: true, @@ -492,8 +528,8 @@ describe("release helper", () => { expect(calls.find(call => call.name === "git" && call.args[0] === "push")).toBeUndefined(); }); - test("a malformed OCX_RELEASE_SSH_REPO override is refused instead of pushed to", () => { - const { calls, result } = runRelease("9.9.9", { + test("a malformed OCX_RELEASE_SSH_REPO override is refused instead of pushed to", async () => { + const { calls, result } = await runRelease("9.9.9", { releaseSshKey: "/tmp/k", releaseSshRepo: "not-a-remote", pendingBump: true, @@ -504,18 +540,19 @@ describe("release helper", () => { expect(calls.find(call => call.name === "git" && call.args[0] === "push")).toBeUndefined(); }); - test("credential-bearing SSH targets are rejected without logging the credential", () => { - for (const scenario of [ - { releaseSshRepo: "ssh://git:SECRET@example.test/owner/repository.git" }, - { releaseSshRepo: "ssh://SECRET@example.test/owner/repository.git" }, - { releaseSshRepo: "ssh://git%3ASECRET@example.test/owner/repository.git" }, - { releaseSshRepo: "git@SECRET@example.test:owner/repository.git" }, - { releaseSshRepo: "ssh://git:@example.test/owner/repository.git" }, - { releaseSshRepo: "git@example.test:owner/repository.git?token=SECRET" }, - { originUrl: "ssh://git:SECRET@example.test/owner/repository.git" }, - { originUrl: "git:SECRET@example.test:owner/repository.git" }, - ]) { - const { calls, result } = runRelease("9.9.9", { + test.each([ + { releaseSshRepo: "ssh://git:SECRET@example.test/owner/repository.git" }, + { releaseSshRepo: "ssh://SECRET@example.test/owner/repository.git" }, + { releaseSshRepo: "ssh://git%3ASECRET@example.test/owner/repository.git" }, + { releaseSshRepo: "git@SECRET@example.test:owner/repository.git" }, + { releaseSshRepo: "ssh://git:@example.test/owner/repository.git" }, + { releaseSshRepo: "git@example.test:owner/repository.git?token=SECRET" }, + { originUrl: "ssh://git:SECRET@example.test/owner/repository.git" }, + { originUrl: "git:SECRET@example.test:owner/repository.git" }, + ] satisfies ReleaseScenario[])( + "credential-bearing SSH target is rejected without logging the credential", + async scenario => { + const { calls, result } = await runRelease("9.9.9", { releaseSshKey: "/tmp/k", pendingBump: true, ...scenario, @@ -524,28 +561,26 @@ describe("release helper", () => { expect(result.status).not.toBe(0); expect(output).not.toContain("SECRET"); expect(calls.find(call => call.name === "git" && call.args[0] === "push")).toBeUndefined(); - } - }); + }, + ); - test("credential-free ssh URL and scp-like release targets remain accepted", () => { - for (const releaseSshRepo of [ - "ssh://git@example.test/owner/repository.git", - "ssh://example.test/owner/repository.git", - "git@example.test:owner/repository.git", - ]) { - const { calls, result } = runRelease("9.9.9", { - releaseSshKey: "/tmp/k", - releaseSshRepo, - pendingBump: true, - }); - expect(result.status).toBe(0); - expect(calls.find(call => call.name === "git" && call.args[0] === "push")?.args[1]) - .toBe(releaseSshRepo); - } + test.each([ + "ssh://git@example.test/owner/repository.git", + "ssh://example.test/owner/repository.git", + "git@example.test:owner/repository.git", + ])("credential-free ssh URL or scp-like release target remains accepted", async releaseSshRepo => { + const { calls, result } = await runRelease("9.9.9", { + releaseSshKey: "/tmp/k", + releaseSshRepo, + pendingBump: true, + }); + expect(result.status).toBe(0); + expect(calls.find(call => call.name === "git" && call.args[0] === "push")?.args[1]) + .toBe(releaseSshRepo); }); - test("an ssh origin is reused verbatim rather than rewritten", () => { - const { calls } = runRelease("9.9.9", { + test("an ssh origin is reused verbatim rather than rewritten", async () => { + const { calls } = await runRelease("9.9.9", { releaseSshKey: "/tmp/k", originUrl: `${"git"}@${"github.com"}:lidge-jun/opencodex.git`, pendingBump: true, @@ -555,8 +590,8 @@ describe("release helper", () => { expect(push?.args[1]).toBe(`${"git"}@${"github.com"}:lidge-jun/opencodex.git`); }); - test("an origin that yields no ssh target aborts instead of guessing one", () => { - const { calls, result } = runRelease("9.9.9", { + test("an origin that yields no ssh target aborts instead of guessing one", async () => { + const { calls, result } = await runRelease("9.9.9", { releaseSshKey: "/tmp/k", originUrl: "/srv/git/opencodex.git", pendingBump: true, @@ -567,8 +602,8 @@ describe("release helper", () => { expect(calls.find(call => call.name === "git" && call.args[0] === "push")).toBeUndefined(); }); - test("without a configured key the push is unchanged and carries no ssh override", () => { - const { calls, result } = runRelease("9.9.9", { pendingBump: true }); + test("without a configured key the push is unchanged and carries no ssh override", async () => { + const { calls, result } = await runRelease("9.9.9", { pendingBump: true }); expect(result.status).toBe(0); const push = calls.find(call => call.name === "git" && call.args[0] === "push"); @@ -576,8 +611,8 @@ describe("release helper", () => { expect(push?.gitSshCommand).toBeUndefined(); }); - test("aborts before dispatch when the remote branch moved during the CI wait", () => { - const { calls, result } = runRelease("9.9.9", { + test("aborts before dispatch when the remote branch moved during the CI wait", async () => { + const { calls, result } = await runRelease("9.9.9", { headSha: "abc123def456", remoteHeadSha: "9999999999999999999999999999999999999999", }); @@ -652,19 +687,19 @@ describe("release helper", () => { // #1753 review follow-up: build metadata on the channel tip is valid semver // and compares by precedence only; an unparseable tip must fail CLOSED // (Number() on a garbage core used to yield NaN and pass any candidate). - test("channel tip with build metadata compares by precedence, not NaN", () => { - const { result } = runRelease("2.19.4", { npmLatest: "2.19.3+build.1" }); + test("channel tip with build metadata compares by precedence, not NaN", async () => { + const { result } = await runRelease("2.19.4", { npmLatest: "2.19.3+build.1" }); expect(`${result.status}\n${result.stderr ?? ""}`.trim()).toBe("0"); }); - test("channel tip equal after stripping build metadata does not move forward", () => { - const { result } = runRelease("2.19.3", { npmLatest: "2.19.3+build.1" }); + test("channel tip equal after stripping build metadata does not move forward", async () => { + const { result } = await runRelease("2.19.3", { npmLatest: "2.19.3+build.1" }); expect(result.status).toBe(1); expect(result.stderr ?? "").toContain("does not move"); }); - test("unparseable channel tip fails closed", () => { - const { result } = runRelease("2.19.4", { npmLatest: "not-a-version" }); + test("unparseable channel tip fails closed", async () => { + const { result } = await runRelease("2.19.4", { npmLatest: "not-a-version" }); expect(result.status).toBe(1); expect(result.stderr ?? "").toContain("cannot compare release versions"); }); diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index de2de9fc42..e2d6f30066 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -3415,7 +3415,7 @@ describe("server local API auth", () => { await server.stop(true); await upstream.stop(true); } - }); + }, { timeout: SERVER_BUDGET_MS }); test("passthrough SSE cyber terminal is logged as 400 cyber_policy", async () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); diff --git a/tests/test-runner.test.ts b/tests/test-runner.test.ts index bbd91ffddc..9589bc59d8 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -2,7 +2,17 @@ import { describe, expect, test } from "bun:test"; import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { isAbsolute, join } from "node:path"; -import { createIsolatedTestEnvironment, resolveBunTestArgs } from "../scripts/test"; +import { + createIsolatedTestEnvironment, + resolveBunTestArgs, + resolveBunTestPlan, + SERIAL_FULL_SUITE_FILES, +} from "../scripts/test"; +import { + acquireTestRunLock, + resolveBareTestRunIdentity, + TEST_RUN_NO_QUEUE_ENV, +} from "../scripts/test-run-lock"; import { decodeWindowsIdentityPowerShellOutputForTests, windowsIdentityPowerShellCommandForTests, @@ -82,6 +92,40 @@ describe("bun test argv", () => { expect(resolveBunTestArgs([])).toEqual(["--isolate", "--parallel=4", "./tests/"]); }); + test("the default full suite quarantines load-sensitive files into one-worker lanes", () => { + const plan = resolveBunTestPlan([]); + expect(plan).toHaveLength(SERIAL_FULL_SUITE_FILES.length + 1); + expect(plan[0]?.label).toBe("parallel suite"); + expect(plan[0]?.args).toContain("--parallel=4"); + expect(plan[0]?.args).toContain("./tests/"); + for (const file of SERIAL_FULL_SUITE_FILES) { + expect(plan[0]?.args).toContain(`**/${file}`); + expect(plan.find(lane => lane.label === file)?.args).toEqual([ + "--isolate", + "--parallel=1", + `./tests/${file}`, + ]); + } + expect(plan.find(lane => lane.label === "release-helper.test.ts")?.timeoutMs).toBe(5 * 60 * 1000); + expect(plan.find(lane => lane.label === "codex-shim.test.ts")?.timeoutMs).toBe(3 * 60 * 1000); + }); + + test("serial lanes override caller parallelism without changing the main lane", () => { + const plan = resolveBunTestPlan(["--parallel=2", "--only-failures"]); + expect(plan[0]?.args).toContain("--parallel=2"); + for (const lane of plan.slice(1)) { + expect(lane.args).toContain("--parallel=1"); + expect(lane.args).not.toContain("--parallel=2"); + expect(lane.args).toContain("--only-failures"); + } + }); + + test("sharded and reporter-file runs stay a single caller-controlled lane", () => { + expect(resolveBunTestPlan(["--shard=1/3"])).toHaveLength(1); + expect(resolveBunTestPlan(["--reporter=junit", "--reporter-outfile", "results.xml"])) + .toHaveLength(1); + }); + test("a file filter keeps isolate and bounded parallelism but no suite path", () => { expect(resolveBunTestArgs(["tests/foo.test.ts"])) .toEqual(["--isolate", "--parallel=4", "tests/foo.test.ts"]); @@ -163,3 +207,102 @@ describe("bun test argv", () => { } }); }); + +describe("bun test machine lock", () => { + test("independent bare runners do not inherit a shared long-lived parent identity", () => { + expect(resolveBareTestRunIdentity({ pid: 101, ppid: 50 })).toEqual({ + ownerPid: 101, + runId: "bare-101", + }); + expect(resolveBareTestRunIdentity({ pid: 102, ppid: 50 })).toEqual({ + ownerPid: 102, + runId: "bare-102", + }); + }); + + test("parallel Bun workers rendezvous on their short-lived controller PID", () => { + expect(resolveBareTestRunIdentity({ pid: 101, ppid: 90, workerId: "1" })).toEqual({ + ownerPid: 101, + runId: "bare-90", + }); + expect(resolveBareTestRunIdentity({ pid: 102, ppid: 90, workerId: "2" })).toEqual({ + ownerPid: 102, + runId: "bare-90", + }); + }); + + test("one run owns the lock while sibling workers with its run ID join", async () => { + const root = mkdtempSync(join(tmpdir(), "opencodex-test-lock-")); + const lockPath = join(root, "suite.lock"); + try { + const owner = await acquireTestRunLock({ runId: "suite-a", lockPath, pollMs: 5, maxWaitMs: 50 }); + const sibling = await acquireTestRunLock({ runId: "suite-a", lockPath, pollMs: 5, maxWaitMs: 50 }); + expect(owner.acquired).toBe(true); + expect(sibling.acquired).toBe(false); + sibling.release(); + expect(existsSync(lockPath)).toBe(true); + owner.release(); + expect(existsSync(lockPath)).toBe(false); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + test("a dead owner is reclaimed even when the next bare invocation derives the same run ID", async () => { + const root = mkdtempSync(join(tmpdir(), "opencodex-test-lock-")); + const lockPath = join(root, "suite.lock"); + try { + const stale = await acquireTestRunLock({ + runId: "stale", + ownerPid: 2_147_483_647, + lockPath, + pollMs: 5, + maxWaitMs: 50, + }); + const replacement = await acquireTestRunLock({ runId: "stale", lockPath, pollMs: 5, maxWaitMs: 50 }); + expect(replacement.acquired).toBe(true); + stale.release(); + expect(existsSync(lockPath)).toBe(true); + replacement.release(); + expect(existsSync(lockPath)).toBe(false); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + test("a live competing run fails closed after the bounded wait", async () => { + const root = mkdtempSync(join(tmpdir(), "opencodex-test-lock-")); + const lockPath = join(root, "suite.lock"); + try { + const owner = await acquireTestRunLock({ runId: "live", lockPath, pollMs: 5, maxWaitMs: 50 }); + let waits = 0; + await expect(acquireTestRunLock({ + runId: "blocked", + lockPath, + pollMs: 5, + maxWaitMs: 20, + onWait: () => { waits += 1; }, + })).rejects.toThrow("timed out"); + expect(waits).toBe(1); + owner.release(); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + test("the explicit no-queue escape hatch does not create a lock", async () => { + const root = mkdtempSync(join(tmpdir(), "opencodex-test-lock-")); + const lockPath = join(root, "suite.lock"); + try { + const lock = await acquireTestRunLock({ + runId: "opt-out", + lockPath, + env: { [TEST_RUN_NO_QUEUE_ENV]: "1" }, + }); + expect(lock.acquired).toBe(false); + expect(existsSync(lockPath)).toBe(false); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); +});