From e8952c615790cd4596a01271fcb236c6672143b3 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Tue, 8 Sep 2026 10:11:10 -0400 Subject: [PATCH] perf(test): make suite sharding configurable --- .changeset/fast-checks-rest.md | 2 + package.json | 2 +- scripts/test/run-test-suite.test.ts | 53 +++++++++++++--- scripts/test/run-test-suite.ts | 95 +++++++++++++++++++++++------ 4 files changed, 125 insertions(+), 27 deletions(-) create mode 100644 .changeset/fast-checks-rest.md diff --git a/.changeset/fast-checks-rest.md b/.changeset/fast-checks-rest.md new file mode 100644 index 000000000..a845151cc --- /dev/null +++ b/.changeset/fast-checks-rest.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/package.json b/package.json index 3be751641..3e596a316 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "prepare": "simple-git-hooks", "test": "bun run ./scripts/test/run-test-suite.ts", "test:theme-contrast": "bun test packages/hunk/src/ui/themes.test.ts --test-name-pattern contrast", - "test:integration": "\"${npm_execpath:-bun}\" test ./test/pty", + "test:integration": "bun run ./scripts/test/run-test-suite.ts --group=integration", "test:session-broker-node": "bun run ./scripts/test/test-session-broker-node.ts", "test:tty-smoke": "HUNK_RUN_TTY_SMOKE=1 \"${npm_execpath:-bun}\" test ./test/smoke", "test:install-vm": "bun run ./test/cli/install-vm/runner.ts", diff --git a/scripts/test/run-test-suite.test.ts b/scripts/test/run-test-suite.test.ts index 16b2a9731..fba05335f 100644 --- a/scripts/test/run-test-suite.test.ts +++ b/scripts/test/run-test-suite.test.ts @@ -2,7 +2,11 @@ import { describe, expect, test } from "bun:test"; import { buildTestShardCommand, DEFAULT_TEST_PATTERNS, + requiresSerialTestExecution, + resolveTestGroupShardCount, + resolveTestInvocation, resolveTestShardCount, + TEST_PATTERN_GROUPS, terminateTestShardProcesses, } from "./run-test-suite"; @@ -18,12 +22,19 @@ describe("test suite sharding", () => { expect(resolveTestShardCount(2, "16", "linux")).toBe(16); }); - test("keeps non-Linux suites serial", () => { + test("keeps automatic non-Linux suites serial but accepts explicit CI sharding", () => { expect(resolveTestShardCount(32, undefined, "win32")).toBe(1); - expect(resolveTestShardCount(32, "16", "darwin")).toBe(1); + expect(resolveTestShardCount(32, "2", "win32")).toBe(2); + expect(resolveTestShardCount(32, "16", "darwin")).toBe(16); }); - test("rejects malformed or excessive Linux shard overrides", () => { + test("keeps PTY integration serial by default while allowing measured overrides", () => { + expect(resolveTestGroupShardCount(32, undefined, "linux", "integration")).toBe(1); + expect(resolveTestGroupShardCount(32, "2", "linux", "integration")).toBe(2); + expect(resolveTestGroupShardCount(32, undefined, "linux", "default")).toBe(2); + }); + + test("rejects malformed or excessive shard overrides", () => { expect(() => resolveTestShardCount(8, "0", "linux")).toThrow( "HUNK_TEST_SHARDS must be a positive safe integer", ); @@ -38,6 +49,34 @@ describe("test suite sharding", () => { ); }); + test("resolves named test groups without forwarding the selector", () => { + expect(resolveTestInvocation(["--group=integration", "--rerun-each=2"])).toEqual({ + forwardedArgs: ["--rerun-each=2"], + group: "integration", + patterns: TEST_PATTERN_GROUPS.integration, + }); + expect(resolveTestInvocation([]).group).toBe("default"); + expect(() => resolveTestInvocation(["--group=missing"])).toThrow("Unknown test group: missing"); + expect(() => resolveTestInvocation(["--group=toString"])).toThrow( + "Unknown test group: toString", + ); + expect(() => resolveTestInvocation(["--group=default", "--group=integration"])).toThrow( + "Only one --group argument may be provided", + ); + }); + + test("keeps filtered and file-output invocations serial", () => { + expect(requiresSerialTestExecution([])).toBe(false); + expect(requiresSerialTestExecution(["--rerun-each=2"])).toBe(false); + expect(requiresSerialTestExecution(["-t", "one test"])).toBe(true); + expect(requiresSerialTestExecution(["--only"])).toBe(true); + expect(requiresSerialTestExecution(["--test-name-pattern=one test"])).toBe(true); + expect(requiresSerialTestExecution(["--coverage"])).toBe(true); + expect(requiresSerialTestExecution(["--coverage-dir=coverage/custom"])).toBe(true); + expect(requiresSerialTestExecution(["--reporter-outfile=reports/junit.xml"])).toBe(true); + expect(requiresSerialTestExecution(["--reporter-outfile", "reports/junit.xml"])).toBe(true); + }); + test("builds serial and sharded Bun commands", () => { expect(buildTestShardCommand("/opt/bun", 1, 1, [], "linux")).toEqual([ "/opt/bun", @@ -53,11 +92,9 @@ describe("test suite sharding", () => { ...DEFAULT_TEST_PATTERNS, "--rerun-each=2", ]); - expect(buildTestShardCommand("C:\\bun.exe", 1, 1, [], "win32")).toEqual([ - "C:\\bun.exe", - "test", - ...DEFAULT_TEST_PATTERNS, - ]); + expect( + buildTestShardCommand("C:\\bun.exe", 1, 1, [], "win32", TEST_PATTERN_GROUPS.integration), + ).toEqual(["C:\\bun.exe", "test", ...TEST_PATTERN_GROUPS.integration]); }); test("forwards termination while tolerating an already stopped shard", () => { diff --git a/scripts/test/run-test-suite.ts b/scripts/test/run-test-suite.ts index fce869e50..c010716f3 100644 --- a/scripts/test/run-test-suite.ts +++ b/scripts/test/run-test-suite.ts @@ -1,24 +1,25 @@ #!/usr/bin/env bun /** - * Runs Hunk's default tests concurrently without Bun's isolated parallel worker mode. + * Runs Hunk's test groups 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. + * requested shard, so this module launches and supervises every shard. The default suite + * shards automatically on Linux; resource-intensive PTY tests and non-Linux platforms + * stay serial unless CI or a developer explicitly chooses a validated shard count. */ import { availableParallelism } from "node:os"; -export const DEFAULT_TEST_PATTERNS = [ - "./packages", - "./scripts", - "./examples", - "./test/cli", - "./test/session", -] as const; +export const TEST_PATTERN_GROUPS = { + default: ["./packages", "./scripts", "./examples", "./test/cli", "./test/session"], + integration: ["./test/pty"], +} as const; + +export const DEFAULT_TEST_PATTERNS = TEST_PATTERN_GROUPS.default; +export type TestPatternGroup = keyof typeof TEST_PATTERN_GROUPS; const MAX_AUTOMATIC_TEST_SHARDS = 2; const MAX_EXPLICIT_TEST_SHARDS = 64; @@ -28,14 +29,12 @@ type KillableProcess = { kill(signal?: number | NodeJS.Signals): void; }; -/** Resolve a Linux shard override or choose a bounded count from the available CPUs. */ +/** Resolve an explicit shard override or choose a bounded Linux count from 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) { @@ -47,9 +46,52 @@ export function resolveTestShardCount( return count; } + if (platform !== "linux") return 1; return Math.min(MAX_AUTOMATIC_TEST_SHARDS, Math.max(1, Math.floor(cpuCount))); } +/** Keep resource-intensive PTY tests serial unless the caller explicitly chooses a shard count. */ +export function resolveTestGroupShardCount( + cpuCount: number, + override: string | undefined, + platform: NodeJS.Platform, + group: TestPatternGroup, +) { + if (group === "integration" && override === undefined) return 1; + return resolveTestShardCount(cpuCount, override, platform); +} + +/** Resolve the selected test group while preserving arguments meant for Bun's test runner. */ +export function resolveTestInvocation(args: string[]) { + const groupArguments = args.filter((arg) => arg.startsWith("--group=")); + if (groupArguments.length > 1) { + throw new Error("Only one --group argument may be provided"); + } + + const group = (groupArguments[0]?.slice("--group=".length) ?? "default") as TestPatternGroup; + if (!Object.hasOwn(TEST_PATTERN_GROUPS, group)) { + throw new Error(`Unknown test group: ${group}`); + } + + return { + forwardedArgs: args.filter((arg) => !arg.startsWith("--group=")), + group, + patterns: TEST_PATTERN_GROUPS[group], + }; +} + +/** Keep filtered runs and shared file-backed output on one process. */ +export function requiresSerialTestExecution(args: string[]) { + return args.some( + (arg) => + arg === "-t" || + arg === "--only" || + arg.startsWith("--test-name-pattern") || + arg.startsWith("--coverage") || + arg.startsWith("--reporter-outfile"), + ); +} + /** Build one Bun test command for an independent file shard. */ export function buildTestShardCommand( bunExecutable: string, @@ -57,13 +99,14 @@ export function buildTestShardCommand( shardCount: number, forwardedArgs: string[] = [], platform: NodeJS.Platform = process.platform, + patterns: readonly string[] = DEFAULT_TEST_PATTERNS, ) { return [ bunExecutable, "test", ...(platform === "win32" ? [] : ["--no-orphans"]), ...(shardCount > 1 ? [`--shard=${shard}/${shardCount}`] : []), - ...DEFAULT_TEST_PATTERNS, + ...patterns, ...forwardedArgs, ]; } @@ -79,19 +122,35 @@ export function terminateTestShardProcesses(processes: KillableProcess[], signal } } -/** Run the default suite in independent Bun processes without enabling Bun's isolate mode. */ +/** Run one test group 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 { forwardedArgs, group, patterns } = resolveTestInvocation(args); + const resolvedShardCount = resolveTestGroupShardCount( + availableParallelism(), + process.env.HUNK_TEST_SHARDS, + process.platform, + group, + ); + const shardCount = requiresSerialTestExecution(forwardedArgs) ? 1 : resolvedShardCount; const bunExecutable = process.execPath; - console.error(`Running the test suite in ${shardCount} shard${shardCount === 1 ? "" : "s"}...`); + console.error( + `Running the ${group} test group 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), + buildTestShardCommand( + bunExecutable, + shard, + shardCount, + forwardedArgs, + process.platform, + patterns, + ), { cwd: process.cwd(), env: { ...process.env, npm_execpath: bunExecutable },