From fc28e4b16970b8ef95341051c9adfa3826d6e325 Mon Sep 17 00:00:00 2001 From: olddonkey Date: Sat, 29 Aug 2026 18:38:54 -0700 Subject: [PATCH 1/2] fix(test): install gui dependencies the local runner already needs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gui` is not a workspace of the root package and declares React only in `gui/package.json`, so a root `bun install` never creates `gui/node_modules`. Twenty-five files under `tests/` import modules from `gui/src`, so on a fresh clone or worktree those tests fail with `Cannot find package 'react'` — reported as an "Unhandled error between tests" that names no test, which is why this has read as a flake rather than a missing setup step. `.github/workflows/ci.yml` already installs them explicitly, and its comment says why: "Several files under tests/ import JSX-bearing modules from gui/src ... and React is declared only in gui/package.json." The local runner had no equivalent, so `bun run test` and CI did not agree about what the suite needs. Verified as an A/B on a worktree with only the root install: the same gui-importing test goes 0 pass / 1 fail before and 4 pass / 0 fail after, with the directory installed on the way through. Installing rather than failing is the deliberate call — `gui/node_modules` is a gitignored build artifact, not source, and the tests genuinely require it. The bounds matter more than the convenience: it runs only when `gui/package.json` exists and `gui/node_modules` does not, so a published install tree with no `gui/` is untouched; it uses `--frozen-lockfile` to match CI; and a failed install aborts with the manual command instead of continuing into twenty-five unexplained React failures. This closes the deterministic half of the problem. A separate intermittent resolution failure still occurs at full-suite scale with the directory present, which does not reproduce at smaller scale and is not addressed here. Co-Authored-By: Claude Opus 5 --- scripts/test.ts | 64 +++++++++++++++++++++++++++++++++++---- tests/test-runner.test.ts | 63 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 6 deletions(-) diff --git a/scripts/test.ts b/scripts/test.ts index 832a537191..9064e5b854 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { existsSync, 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"; @@ -433,15 +433,67 @@ async function runTestLane(lane: BunTestLane, runId: string, capture = false): P } } +/** + * `gui` is not a workspace of the root package and declares React only in `gui/package.json`, so a + * root `bun install` never creates `gui/node_modules`. Twenty-five files under `tests/` import + * modules from `gui/src`, which makes those tests fail on a fresh clone or worktree with + * `Cannot find package 'react'` — reported as an "Unhandled error between tests" that names no + * test, so the cause is not obvious from the output. + * + * `.github/workflows/ci.yml` already installs them explicitly for exactly this reason; the local + * runner had no equivalent. Install on demand rather than fail, because the tests genuinely + * require the directory and `gui/node_modules` is a gitignored build artifact, not source. + */ +export function ensureGuiDependencies(io: { + cwd?: string; + exists?: (path: string) => boolean; + install?: (guiDir: string) => { ok: boolean; detail: string }; + log?: (message: string) => void; +} = {}): { kind: "present" | "installed" | "absent" | "failed"; detail?: string } { + const cwd = io.cwd ?? process.cwd(); + const exists = io.exists ?? existsSync; + const log = io.log ?? (message => console.warn(message)); + const guiDir = join(cwd, "gui"); + if (!exists(join(guiDir, "package.json"))) return { kind: "absent" }; + if (exists(join(guiDir, "node_modules"))) return { kind: "present" }; + + log("[test] gui/node_modules is missing; installing it so the tests that import gui/src can resolve React."); + const install = io.install ?? ((dir: string) => { + const result = Bun.spawnSync(["bun", "install", "--frozen-lockfile"], { + cwd: dir, + stdout: "pipe", + stderr: "pipe", + }); + return { + ok: result.exitCode === 0, + detail: decodeOutput(result.stderr) || decodeOutput(result.stdout), + }; + }); + const outcome = install(guiDir); + if (outcome.ok) return { kind: "installed" }; + return { kind: "failed", detail: outcome.detail }; +} + if (import.meta.main) { const requestedTests = process.argv.slice(2); - let changedRun: ReturnType = null; - try { - changedRun = inspectChangedRun(requestedTests); - } catch (error) { - console.error(error instanceof Error ? error.message : String(error)); + const guiDependencies = ensureGuiDependencies(); + if (guiDependencies.kind === "failed") { + console.error( + "[test] could not install gui/node_modules, which tests importing gui/src need to resolve React.\n" + + " Run it manually: cd gui && bun install --frozen-lockfile\n" + + (guiDependencies.detail ? ` ${guiDependencies.detail.trim().split("\n").slice(-3).join("\n ")}` : ""), + ); process.exitCode = 1; } + let changedRun: ReturnType = null; + if (process.exitCode !== 1) { + try { + changedRun = inspectChangedRun(requestedTests); + } catch (error) { + console.error(error instanceof Error ? error.message : String(error)); + process.exitCode = 1; + } + } if (process.exitCode !== 1) { if (changedRun) { console.warn( diff --git a/tests/test-runner.test.ts b/tests/test-runner.test.ts index 2d5423d628..6c22c491f5 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -5,6 +5,7 @@ import { isAbsolute, join } from "node:path"; import { changedSelectionFailure, createIsolatedTestEnvironment, + ensureGuiDependencies, inspectChangedRun, resolveBunTestArgs, resolveBunTestPlan, @@ -459,3 +460,65 @@ describe("bun test machine lock", () => { } }); }); + +describe("ensureGuiDependencies", () => { + // `gui` is not a workspace, so a root `bun install` leaves gui/node_modules absent and the + // twenty-five tests importing gui/src die on `Cannot find package 'react'` — an "Unhandled error + // between tests" that names no test. CI already installs them; this closes the local gap. + const paths = (present: string[]) => (path: string) => present.some(entry => path.endsWith(entry)); + + test("installs when gui/package.json exists but node_modules does not", () => { + const installed: string[] = []; + const logged: string[] = []; + const result = ensureGuiDependencies({ + cwd: "/repo", + exists: paths(["gui/package.json"]), + install: dir => { installed.push(dir); return { ok: true, detail: "" }; }, + log: message => logged.push(message), + }); + + expect(result).toEqual({ kind: "installed" }); + expect(installed).toEqual([join("/repo", "gui")]); + expect(logged[0]).toContain("gui/node_modules is missing"); + }); + + test("does nothing when node_modules is already there", () => { + let installs = 0; + const result = ensureGuiDependencies({ + cwd: "/repo", + exists: paths(["gui/package.json", "gui/node_modules"]), + install: () => { installs += 1; return { ok: true, detail: "" }; }, + log: () => {}, + }); + + expect(result).toEqual({ kind: "present" }); + expect(installs).toBe(0); + }); + + // A published install tree has no gui/ at all; the runner must not try to install there. + test("does nothing when there is no gui package", () => { + let installs = 0; + const result = ensureGuiDependencies({ + cwd: "/repo", + exists: () => false, + install: () => { installs += 1; return { ok: true, detail: "" }; }, + log: () => {}, + }); + + expect(result).toEqual({ kind: "absent" }); + expect(installs).toBe(0); + }); + + // Offline or a lockfile drift has to surface as its own message, not as twenty-five + // unexplained React failures once the lanes start. + test("reports the failure detail instead of continuing", () => { + const result = ensureGuiDependencies({ + cwd: "/repo", + exists: paths(["gui/package.json"]), + install: () => ({ ok: false, detail: "lockfile had changes" }), + log: () => {}, + }); + + expect(result).toEqual({ kind: "failed", detail: "lockfile had changes" }); + }); +}); From b0119e00300aae49406fcf5378bb41466412f4ea Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 30 Aug 2026 11:01:36 +0900 Subject: [PATCH 2/2] fix(test): detect incomplete gui dependency installs Treat React's package manifest as the install-completion marker so an interrupted gui install is retried instead of being accepted because node_modules exists. Normalize mocked path suffixes and exercise both separator styles so the focused runner tests remain valid on Windows. --- scripts/test.ts | 6 +++--- tests/test-runner.test.ts | 28 +++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/scripts/test.ts b/scripts/test.ts index 9064e5b854..39a73c8d5d 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -442,7 +442,7 @@ async function runTestLane(lane: BunTestLane, runId: string, capture = false): P * * `.github/workflows/ci.yml` already installs them explicitly for exactly this reason; the local * runner had no equivalent. Install on demand rather than fail, because the tests genuinely - * require the directory and `gui/node_modules` is a gitignored build artifact, not source. + * require the dependency and `gui/node_modules` is a gitignored build artifact, not source. */ export function ensureGuiDependencies(io: { cwd?: string; @@ -455,9 +455,9 @@ export function ensureGuiDependencies(io: { const log = io.log ?? (message => console.warn(message)); const guiDir = join(cwd, "gui"); if (!exists(join(guiDir, "package.json"))) return { kind: "absent" }; - if (exists(join(guiDir, "node_modules"))) return { kind: "present" }; + if (exists(join(guiDir, "node_modules", "react", "package.json"))) return { kind: "present" }; - log("[test] gui/node_modules is missing; installing it so the tests that import gui/src can resolve React."); + log("[test] gui dependencies are missing or incomplete; installing them so tests importing gui/src can resolve React."); const install = io.install ?? ((dir: string) => { const result = Bun.spawnSync(["bun", "install", "--frozen-lockfile"], { cwd: dir, diff --git a/tests/test-runner.test.ts b/tests/test-runner.test.ts index 6c22c491f5..28e747075b 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -465,7 +465,16 @@ describe("ensureGuiDependencies", () => { // `gui` is not a workspace, so a root `bun install` leaves gui/node_modules absent and the // twenty-five tests importing gui/src die on `Cannot find package 'react'` — an "Unhandled error // between tests" that names no test. CI already installs them; this closes the local gap. - const paths = (present: string[]) => (path: string) => present.some(entry => path.endsWith(entry)); + const paths = (present: string[]) => { + const normalized = present.map(path => path.replaceAll("\\", "/")); + return (path: string) => normalized.some(entry => path.replaceAll("\\", "/").endsWith(entry)); + }; + + test("mocked paths match POSIX and Windows separators", () => { + const exists = paths(["gui/package.json"]); + expect(exists("/repo/gui/package.json")).toBe(true); + expect(exists("C:\\repo\\gui\\package.json")).toBe(true); + }); test("installs when gui/package.json exists but node_modules does not", () => { const installed: string[] = []; @@ -479,10 +488,10 @@ describe("ensureGuiDependencies", () => { expect(result).toEqual({ kind: "installed" }); expect(installed).toEqual([join("/repo", "gui")]); - expect(logged[0]).toContain("gui/node_modules is missing"); + expect(logged[0]).toContain("gui dependencies are missing or incomplete"); }); - test("does nothing when node_modules is already there", () => { + test("retries when node_modules exists without the required dependency", () => { let installs = 0; const result = ensureGuiDependencies({ cwd: "/repo", @@ -491,6 +500,19 @@ describe("ensureGuiDependencies", () => { log: () => {}, }); + expect(result).toEqual({ kind: "installed" }); + expect(installs).toBe(1); + }); + + test("does nothing when the required dependency is already there", () => { + let installs = 0; + const result = ensureGuiDependencies({ + cwd: "/repo", + exists: paths(["gui/package.json", "gui/node_modules/react/package.json"]), + install: () => { installs += 1; return { ok: true, detail: "" }; }, + log: () => {}, + }); + expect(result).toEqual({ kind: "present" }); expect(installs).toBe(0); });