diff --git a/.changeset/hot-bikes-beg.md b/.changeset/hot-bikes-beg.md new file mode 100644 index 000000000..a845151cc --- /dev/null +++ b/.changeset/hot-bikes-beg.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.github/workflows/pr-ci.yml b/.github/workflows/pr-ci.yml index 3d1fc27f3..9b53ad6f2 100644 --- a/.github/workflows/pr-ci.yml +++ b/.github/workflows/pr-ci.yml @@ -95,7 +95,7 @@ jobs: run: bun run test:theme-contrast - name: Test suite - run: bun test ./src ./packages ./scripts ./test/cli ./test/session + run: bun run test compiled-headless-portability: name: Compiled headless portability (${{ matrix.os }}) diff --git a/package.json b/package.json index 5b729e50d..274dfc6f3 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,7 @@ "changeset:status": "bunx @changesets/cli@2.31.0 status", "release:version": "bunx @changesets/cli@2.31.0 version", "prepare": "simple-git-hooks", - "test": "\"${npm_execpath:-bun}\" test ./src ./packages ./scripts ./test/cli ./test/session", + "test": "bun run ./scripts/run-test-suite.ts", "test:theme-contrast": "bun test src/ui/themes.test.ts --test-name-pattern contrast", "test:integration": "\"${npm_execpath:-bun}\" test ./test/pty", "test:tty-smoke": "HUNK_RUN_TTY_SMOKE=1 \"${npm_execpath:-bun}\" test ./test/smoke", diff --git a/scripts/run-test-suite.test.ts b/scripts/run-test-suite.test.ts new file mode 100644 index 000000000..16b2a9731 --- /dev/null +++ b/scripts/run-test-suite.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, test } from "bun:test"; +import { + buildTestShardCommand, + DEFAULT_TEST_PATTERNS, + resolveTestShardCount, + terminateTestShardProcesses, +} from "./run-test-suite"; + +describe("test suite sharding", () => { + test("uses the available CPUs up to the automatic Linux cap", () => { + expect(resolveTestShardCount(1, undefined, "linux")).toBe(1); + expect(resolveTestShardCount(2, undefined, "linux")).toBe(2); + expect(resolveTestShardCount(32, undefined, "linux")).toBe(2); + }); + + test("accepts an explicit positive shard count on Linux", () => { + expect(resolveTestShardCount(32, "1", "linux")).toBe(1); + expect(resolveTestShardCount(2, "16", "linux")).toBe(16); + }); + + test("keeps non-Linux suites serial", () => { + expect(resolveTestShardCount(32, undefined, "win32")).toBe(1); + expect(resolveTestShardCount(32, "16", "darwin")).toBe(1); + }); + + test("rejects malformed or excessive Linux shard overrides", () => { + expect(() => resolveTestShardCount(8, "0", "linux")).toThrow( + "HUNK_TEST_SHARDS must be a positive safe integer", + ); + expect(() => resolveTestShardCount(8, "2.5", "linux")).toThrow( + "HUNK_TEST_SHARDS must be a positive safe integer", + ); + expect(() => resolveTestShardCount(8, "999999999999999999999999", "linux")).toThrow( + "HUNK_TEST_SHARDS must be a positive safe integer", + ); + expect(() => resolveTestShardCount(8, "65", "linux")).toThrow( + "HUNK_TEST_SHARDS cannot exceed 64", + ); + }); + + test("builds serial and sharded Bun commands", () => { + expect(buildTestShardCommand("/opt/bun", 1, 1, [], "linux")).toEqual([ + "/opt/bun", + "test", + "--no-orphans", + ...DEFAULT_TEST_PATTERNS, + ]); + expect(buildTestShardCommand("/opt/bun", 2, 4, ["--rerun-each=2"], "linux")).toEqual([ + "/opt/bun", + "test", + "--no-orphans", + "--shard=2/4", + ...DEFAULT_TEST_PATTERNS, + "--rerun-each=2", + ]); + expect(buildTestShardCommand("C:\\bun.exe", 1, 1, [], "win32")).toEqual([ + "C:\\bun.exe", + "test", + ...DEFAULT_TEST_PATTERNS, + ]); + }); + + test("forwards termination while tolerating an already stopped shard", () => { + const signals: Array = []; + terminateTestShardProcesses( + [ + { kill: (signal) => signals.push(signal as NodeJS.Signals) }, + { + kill: () => { + throw new Error("already stopped"); + }, + }, + ], + "SIGTERM", + ); + + expect(signals).toEqual(["SIGTERM"]); + }); +}); diff --git a/scripts/run-test-suite.ts b/scripts/run-test-suite.ts new file mode 100644 index 000000000..e34602d9d --- /dev/null +++ b/scripts/run-test-suite.ts @@ -0,0 +1,166 @@ +#!/usr/bin/env bun + +/** + * Runs Hunk's default tests concurrently without Bun's isolated parallel worker mode. + * + * Bun 1.3.14's `--parallel` implies `--isolate`, which makes OpenTUI's native FFI + * renderer fail to initialize with "Cannot access 'default' before initialization." + * Independent `--shard=N/M` processes avoid that failure, but Bun runs only the one + * requested shard, so this module launches and supervises every shard. Sharding stays + * Linux-only because the complete multi-process suite is validated and benchmarked there. + */ + +import { availableParallelism } from "node:os"; + +export const DEFAULT_TEST_PATTERNS = [ + "./src", + "./packages", + "./scripts", + "./test/cli", + "./test/session", +] as const; + +const MAX_AUTOMATIC_TEST_SHARDS = 2; +const MAX_EXPLICIT_TEST_SHARDS = 64; +const SHARD_TERMINATION_GRACE_MS = 1_000; + +type KillableProcess = { + kill(signal?: number | NodeJS.Signals): void; +}; + +/** Resolve a Linux shard override or choose a bounded count from the available CPUs. */ +export function resolveTestShardCount( + cpuCount: number, + override?: string, + platform: NodeJS.Platform = process.platform, +) { + if (platform !== "linux") return 1; + + if (override !== undefined) { + const count = Number(override); + if (!/^\d+$/.test(override) || !Number.isSafeInteger(count) || count < 1) { + throw new Error("HUNK_TEST_SHARDS must be a positive safe integer"); + } + if (count > MAX_EXPLICIT_TEST_SHARDS) { + throw new Error(`HUNK_TEST_SHARDS cannot exceed ${MAX_EXPLICIT_TEST_SHARDS}`); + } + return count; + } + + return Math.min(MAX_AUTOMATIC_TEST_SHARDS, Math.max(1, Math.floor(cpuCount))); +} + +/** Build one Bun test command for an independent file shard. */ +export function buildTestShardCommand( + bunExecutable: string, + shard: number, + shardCount: number, + forwardedArgs: string[] = [], + platform: NodeJS.Platform = process.platform, +) { + return [ + bunExecutable, + "test", + ...(platform === "win32" ? [] : ["--no-orphans"]), + ...(shardCount > 1 ? [`--shard=${shard}/${shardCount}`] : []), + ...DEFAULT_TEST_PATTERNS, + ...forwardedArgs, + ]; +} + +/** Forward a termination signal to every live shard, tolerating shards that already exited. */ +export function terminateTestShardProcesses(processes: KillableProcess[], signal: NodeJS.Signals) { + for (const proc of processes) { + try { + proc.kill(signal); + } catch { + // Another shard or the terminal process group may already have stopped it. + } + } +} + +/** Run the default suite in independent Bun processes without enabling Bun's isolate mode. */ +export async function main(args = Bun.argv.slice(2)) { + const shardCount = resolveTestShardCount(availableParallelism(), process.env.HUNK_TEST_SHARDS); + const bunExecutable = process.execPath; + + console.error(`Running the test suite in ${shardCount} shard${shardCount === 1 ? "" : "s"}...`); + + const shards: Array<{ proc: ReturnType; shard: number }> = []; + try { + for (let index = 0; index < shardCount; index += 1) { + const shard = index + 1; + const proc = Bun.spawn( + buildTestShardCommand(bunExecutable, shard, shardCount, args, process.platform), + { + cwd: process.cwd(), + env: { ...process.env, npm_execpath: bunExecutable }, + stdin: "inherit", + stdout: "inherit", + stderr: "inherit", + }, + ); + shards.push({ proc, shard }); + } + } catch (error) { + const spawnedProcesses = shards.map(({ proc }) => proc); + terminateTestShardProcesses(spawnedProcesses, "SIGTERM"); + const forceKillTimer = setTimeout(() => { + terminateTestShardProcesses(spawnedProcesses, "SIGKILL"); + }, SHARD_TERMINATION_GRACE_MS); + forceKillTimer.unref(); + try { + await Promise.allSettled(shards.map(({ proc }) => proc.exited)); + } finally { + clearTimeout(forceKillTimer); + } + throw error; + } + + const processes = shards.map(({ proc }) => proc); + let interruptedExitCode: number | null = null; + let forceKillTimer: ReturnType | null = null; + const handleSignal = (signal: NodeJS.Signals, exitCode: number) => { + if (interruptedExitCode !== null) return; + interruptedExitCode = exitCode; + terminateTestShardProcesses(processes, signal); + forceKillTimer = setTimeout(() => { + terminateTestShardProcesses(processes, "SIGKILL"); + }, SHARD_TERMINATION_GRACE_MS); + forceKillTimer.unref(); + }; + const handleSigint = () => handleSignal("SIGINT", 130); + const handleSigterm = () => handleSignal("SIGTERM", 143); + process.once("SIGINT", handleSigint); + process.once("SIGTERM", handleSigterm); + + let results: Array<{ exitCode: number; shard: number }>; + try { + results = await Promise.all( + shards.map(async ({ proc, shard }) => ({ exitCode: await proc.exited, shard })), + ); + } finally { + process.off("SIGINT", handleSigint); + process.off("SIGTERM", handleSigterm); + if (forceKillTimer) clearTimeout(forceKillTimer); + } + + if (interruptedExitCode !== null) return interruptedExitCode; + const failedShards = results.filter(({ exitCode }) => exitCode !== 0); + + if (failedShards.length > 0) { + console.error( + `Test shard failure: ${failedShards + .map(({ exitCode, shard }) => `${shard}/${shardCount} (exit ${exitCode})`) + .join(", ")}`, + ); + return 1; + } + + console.error(`All ${shardCount} test shard${shardCount === 1 ? "" : "s"} passed.`); + return 0; +} + +if (import.meta.main) { + process.exitCode = await main(); +} diff --git a/scripts/verify-pr-release-notes.test.ts b/scripts/verify-pr-release-notes.test.ts index 38f89ef39..dca493e69 100644 --- a/scripts/verify-pr-release-notes.test.ts +++ b/scripts/verify-pr-release-notes.test.ts @@ -2,6 +2,7 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, test } from "bun:test"; +import { removeTestDirectory } from "../test/helpers/filesystem"; import { isGeneratedPrereleasePreparation, isGeneratedReleasePath, @@ -88,10 +89,8 @@ function writeGeneratedPrerelease(root: string, initialVersion = "0.17.7") { runGit(root, ["commit", "--quiet", "-m", "prepare prerelease"]); } -afterEach(() => { - for (const root of tempRoots.splice(0)) { - rmSync(root, { recursive: true, force: true }); - } +afterEach(async () => { + await Promise.all(tempRoots.splice(0).map((root) => removeTestDirectory(root))); }); describe("isGeneratedReleasePath", () => { diff --git a/src/extensions/discovery.test.ts b/src/extensions/discovery.test.ts index f20bfd943..479ef52c8 100644 --- a/src/extensions/discovery.test.ts +++ b/src/extensions/discovery.test.ts @@ -485,6 +485,7 @@ describe("manifest api version requirements", () => { repoRoot: undefined, globalExtensionsDir: undefined, flagPaths: [folder], + env: {}, }); expect(candidates).toEqual([ @@ -503,6 +504,7 @@ describe("manifest api version requirements", () => { repoRoot: undefined, globalExtensionsDir: undefined, flagPaths: [folder], + env: {}, }); expect(candidates).toEqual([ @@ -524,6 +526,7 @@ describe("manifest api version requirements", () => { repoRoot: undefined, globalExtensionsDir: undefined, flagPaths: [folder], + env: {}, }); expect(candidates).toEqual([{ id: "bad-api-ext", path: entry, origin: "flag" }]); diff --git a/src/ui/components/scrollbar/VerticalScrollbar.test.tsx b/src/ui/components/scrollbar/VerticalScrollbar.test.tsx index 702b535d4..6522d52e1 100644 --- a/src/ui/components/scrollbar/VerticalScrollbar.test.tsx +++ b/src/ui/components/scrollbar/VerticalScrollbar.test.tsx @@ -198,7 +198,7 @@ describe("Vertical scrollbar", () => { contentHeight={40} theme={theme} height={10} - hideDelayMs={5} + hideDelayMs={120} />, { width: 2, height: 10 }, ); @@ -213,8 +213,9 @@ describe("Vertical scrollbar", () => { await flush(setup); expect(frameHasBackground(setup, theme.accentMuted)).toBe(true); + // Keep a wide margin beyond the deadline for Windows CI timer granularity. await act(async () => { - await Bun.sleep(10); + await Bun.sleep(180); }); await flush(setup); expect(frameHasBackground(setup, theme.accentMuted)).toBe(false); diff --git a/test/session/cli.test.ts b/test/session/cli.test.ts index 2133d1199..c506e277b 100644 --- a/test/session/cli.test.ts +++ b/test/session/cli.test.ts @@ -1,15 +1,19 @@ import { afterAll, afterEach, describe, expect, test } from "bun:test"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { createServer } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { cleanupTestConfigHomes, createTestConfigHome } from "../helpers/config-home"; +import { removeTestDirectory } from "../helpers/filesystem"; const repoRoot = process.cwd(); const sourceEntrypoint = join(repoRoot, "src/main.tsx"); // Spawned hunk processes must assert built-in defaults, not the developer's ambient user config. const testConfigHome = createTestConfigHome(); +const testRuntimeDir = mkdtempSync(join(tmpdir(), "hunk-session-cli-runtime-")); afterAll(cleanupTestConfigHomes); +afterAll(() => removeTestDirectory(testRuntimeDir)); const tempDirs: string[] = []; /** Check for the util-linux `script` interface these Unix-only terminal tests require. */ function supportsControllableScript() { @@ -28,6 +32,20 @@ function supportsControllableScript() { const ttyToolsAvailable = supportsControllableScript(); +/** Reserve a currently unused loopback port for one isolated daemon test. */ +async function reserveLoopbackPort() { + const listener = createServer(() => undefined); + await new Promise((resolve, reject) => { + listener.once("error", reject); + listener.listen(0, "127.0.0.1", () => resolve()); + }); + + const address = listener.address(); + const port = typeof address === "object" && address ? address.port : 0; + await new Promise((resolve) => listener.close(() => resolve())); + return port; +} + interface SessionListJson { sessions: Array<{ sessionId: string; @@ -102,6 +120,7 @@ function spawnHunkSession(fixture: ReturnType, port: env: { ...process.env, XDG_CONFIG_HOME: testConfigHome, + XDG_RUNTIME_DIR: testRuntimeDir, TERM: "xterm-256color", COLUMNS: "120", LINES: "24", @@ -193,19 +212,15 @@ async function quitHunkSession( } } +const ownedDaemonPids = new Map(); + /** Poll daemon health directly before exercising the CLI boundary once. */ async function waitForRegisteredSessions(port: number) { await waitUntil("registered live session", async () => { - try { - const response = await fetch(`http://127.0.0.1:${port}/health`); - if (!response.ok) { - return null; - } - const health = (await response.json()) as { sessions?: number }; - return (health.sessions ?? 0) > 0 ? true : null; - } catch { - return null; - } + const health = await readDaemonHealth(port); + if (!health || (health.sessions ?? 0) === 0) return null; + ownedDaemonPids.set(port, health.pid); + return true; }); const { proc, stdout, stderr } = runSessionCli(["list", "--json"], port); @@ -215,6 +230,88 @@ async function waitForRegisteredSessions(port: number) { return (JSON.parse(stdout) as SessionListJson).sessions; } +/** Read one test daemon's health without leaking connection failures into teardown. */ +async function readDaemonHealth(port: number) { + try { + const response = await fetch(`http://127.0.0.1:${port}/health`); + if (!response.ok) return null; + return (await response.json()) as { pid: number; sessions?: number }; + } catch { + return null; + } +} + +/** Report whether an owned daemon PID still exists. */ +function isProcessRunning(pid: number) { + try { + process.kill(pid, 0); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ESRCH") return false; + throw error; + } +} + +/** Signal an owned daemon while tolerating a concurrent clean exit. */ +function signalProcess(pid: number, signal: NodeJS.Signals) { + try { + process.kill(pid, signal); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; + } +} + +/** Wait until an owned daemon process exits and releases its loopback port. */ +async function waitForDaemonExit(port: number, pid: number, label: string) { + await waitUntil( + label, + async () => { + const health = await readDaemonHealth(port); + if (health && health.pid !== pid) { + throw new Error(`Refusing to manage unexpected daemon ${health.pid} on port ${port}.`); + } + return !isProcessRunning(pid) && health === null ? true : null; + }, + 1_500, + 25, + ); +} + +/** Stop the detached daemon that an integration session auto-started. */ +async function stopTestDaemon(port: number) { + const pid = ownedDaemonPids.get(port); + ownedDaemonPids.delete(port); + if (pid === undefined) return; + + const health = await readDaemonHealth(port); + if (health && health.pid !== pid) { + throw new Error(`Refusing to stop unexpected daemon ${health.pid} on port ${port}.`); + } + + signalProcess(pid, "SIGTERM"); + try { + await waitForDaemonExit(port, pid, "session daemon exit"); + } catch (error) { + const remaining = await readDaemonHealth(port); + if (remaining && remaining.pid !== pid) throw error; + signalProcess(pid, "SIGKILL"); + await waitForDaemonExit(port, pid, "killed session daemon exit"); + } +} + +/** Quit a test session and always stop the detached daemon it owns. */ +async function cleanupHunkSession( + proc: HunkSessionProcess, + fixture: ReturnType, + port: number, +) { + try { + await quitHunkSession(proc, fixture); + } finally { + await stopTestDaemon(port); + } +} + function runSessionCli(args: string[], port: number, stdinText?: string) { const proc = Bun.spawnSync(["bun", "run", "src/main.tsx", "session", ...args], { cwd: repoRoot, @@ -224,6 +321,7 @@ function runSessionCli(args: string[], port: number, stdinText?: string) { env: { ...process.env, XDG_CONFIG_HOME: testConfigHome, + XDG_RUNTIME_DIR: testRuntimeDir, HUNK_MCP_PORT: `${port}`, }, }); @@ -241,7 +339,7 @@ const sessionDescribe = ttyToolsAvailable ? describe : describe.skip; sessionDescribe("session CLI integration", () => { test("list/get/context expose live Hunk sessions through the daemon", async () => { - const port = 48961; + const port = await reserveLoopbackPort(); const fixture = createFixtureFiles( "inspect", ["export const value = 1;", "console.log(value);"], @@ -282,12 +380,12 @@ sessionDescribe("session CLI integration", () => { }, }); } finally { - await quitHunkSession(session, fixture); + await cleanupHunkSession(session, fixture, port); } }); test("reload replaces what a live session is showing", async () => { - const port = 48963; + const port = await reserveLoopbackPort(); const fixture = createFixtureFiles( "reload-alpha", ["export const alpha = 1;"], @@ -341,12 +439,12 @@ sessionDescribe("session CLI integration", () => { }, }); } finally { - await quitHunkSession(session, fixture); + await cleanupHunkSession(session, fixture, port); } }, 20_000); test("reload refuses to read files outside the live session root", async () => { - const port = 48966; + const port = await reserveLoopbackPort(); const fixture = createFixtureFiles( "reload-denied", ["export const visible = 1;"], @@ -389,12 +487,12 @@ sessionDescribe("session CLI integration", () => { }, }); } finally { - await quitHunkSession(session, fixture); + await cleanupHunkSession(session, fixture, port); } }, 20_000); test("navigate works, and comment add only focuses the session when --focus is passed", async () => { - const port = 48962; + const port = await reserveLoopbackPort(); const fixture = createFixtureFiles( "mutate", [ @@ -587,12 +685,12 @@ sessionDescribe("session CLI integration", () => { : null; }); } finally { - await quitHunkSession(session, fixture); + await cleanupHunkSession(session, fixture, port); } }, 20_000); test("comment apply adds a batch from stdin without moving focus by default", async () => { - const port = 48964; + const port = await reserveLoopbackPort(); const fixture = createFixtureFiles( "apply-batch", [ @@ -691,12 +789,12 @@ sessionDescribe("session CLI integration", () => { comments: [{ summary: "First hunk note" }, { summary: "Second hunk note" }], }); } finally { - await quitHunkSession(session, fixture); + await cleanupHunkSession(session, fixture, port); } }, 20_000); test("comment apply with --focus jumps to the first applied comment", async () => { - const port = 48965; + const port = await reserveLoopbackPort(); const fixture = createFixtureFiles( "apply-batch-focus", [ @@ -780,7 +878,7 @@ sessionDescribe("session CLI integration", () => { : null; }); } finally { - await quitHunkSession(session, fixture); + await cleanupHunkSession(session, fixture, port); } }, 20_000); }); diff --git a/test/session/daemon.test.ts b/test/session/daemon.test.ts index f559d30de..c068dc7a2 100644 --- a/test/session/daemon.test.ts +++ b/test/session/daemon.test.ts @@ -89,14 +89,15 @@ describe("session daemon lifecycle", () => { spawned.push(proc); const health = await waitUntil("daemon health", () => readHealth(port), 3_000, 50); - expect(health).toMatchObject({ ok: true, pid: proc.pid }); + expect(health).toMatchObject({ ok: true }); let exited = false; void proc.exited.then(() => { exited = true; }); - process.kill(proc.pid, "SIGTERM"); + // Windows may keep the `bun run` launcher separate from the child serving the daemon. + process.kill(health.pid, "SIGTERM"); await waitUntil("daemon serve process exit", () => (exited ? true : null), 1_500, 25); await waitUntil("daemon port close", async () =>