Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 58 additions & 6 deletions scripts/test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 dependency 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", "react", "package.json"))) return { kind: "present" };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate GUI dependencies against the lockfile

When a developer switches to a commit that adds or updates a GUI dependency, the existing React manifest remains, so this returns present without running the frozen install. The test runner then uses stale gui/node_modules; newly required packages can be absent and updated packages can remain at old versions, causing GUI-importing tests to fail or exercise dependencies different from gui/bun.lock. Compare an installation marker to the current lockfile or let Bun verify the install instead of treating one package as proof that the entire dependency tree is current.

AGENTS.md reference: scripts/AGENTS.md:L14-L15

Useful? React with 👍 / 👎.


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,
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<typeof inspectChangedRun> = 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<typeof inspectChangedRun> = 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(
Expand Down
85 changes: 85 additions & 0 deletions tests/test-runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { isAbsolute, join } from "node:path";
import {
changedSelectionFailure,
createIsolatedTestEnvironment,
ensureGuiDependencies,
inspectChangedRun,
resolveBunTestArgs,
resolveBunTestPlan,
Expand Down Expand Up @@ -459,3 +460,87 @@ 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[]) => {
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[] = [];
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 dependencies are missing or incomplete");
});

test("retries when node_modules exists without the required dependency", () => {
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: "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);
});

// 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" });
});
});
Loading