From 41160a14ed9cf58ca0f2b00b545e5869fe4ffab0 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 15 Aug 2026 08:22:44 -0700 Subject: [PATCH] =?UTF-8?q?test:=20free=20runner=20=E2=80=94=20strict=20ou?= =?UTF-8?q?tput,=20parallel=20execution,=20stable=20shard=20indices?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three coupled changes to scripts/test-free-shards.ts: 1. STRICT OUTPUT: runFreeShard streams through the paid runner's BunTestOutputClassifier — exit 0 without bun's 'Ran N tests across M files' summary, with (fail) lines, or with a wrong file count is a FAILURE (anti-truncation backstop at the runner layer), plus an external wall-clock timeout that SIGKILLs the process group (timed-out distinct from failed; exit 124 vs 1). Also fixes a latent shard-bleed: file selectors now use exactTestFileSelectors (relative paths were substring filters that matched sibling roots). 2. PARALLEL: full-suite mode is one 'bun test --parallel' invocation (Bun 1.3.13). Measured semantics recorded in the header: per-file worker isolation, standard summary, and mid-suite process.exit surfaces as a crashed-worker FAIL with exit 1 — strictly safer than serial, where the same exit truncates silently. No static weight lists; --shards M --shard i keeps deterministic hash partitioning for CI matrices (native --shard rejected: round-robin renumbers when files land). Spawned shards get throwaway GSTACK_HOME/TMPDIR so parallel shards can't contend on real state. Per-shard epilogue prints files/seconds/status every run. 3. Stable indices: assignFilesToShards no longer drops empty shards, so a shard's index depends only on the file hash and requested count — an empty CI matrix slot is a fast no-op success, not a renumbering. package.json 'test' now delegates to the runner (TEST_ROOTS becomes the single source of truth for roots; slop:diff tail preserved; the runner inherits the 30s per-test timeout the old glob passed inline). Co-Authored-By: Claude Fable 5 --- package.json | 2 +- scripts/test-free-shards.ts | 363 +++++++++++++++++++++++++++++----- scripts/test-strict-output.ts | 29 +++ test/test-free-shards.test.ts | 165 +++++++++++++++- 4 files changed, 501 insertions(+), 58 deletions(-) diff --git a/package.json b/package.json index 8d613f978..5b125c7eb 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "gen:skill-docs:user": "bun run scripts/gen-skill-docs.ts --respect-detection", "dev": "bun run browse/src/cli.ts", "server": "bun run browse/src/server.ts", - "test": "bun test browse/test/ test/ make-pdf/test/ design/test/ --timeout 30000 --ignore 'test/skill-e2e-*.test.ts' --ignore test/skill-llm-eval.test.ts --ignore test/skill-routing-e2e.test.ts --ignore test/codex-e2e.test.ts --ignore test/gemini-e2e.test.ts && (bun run slop:diff 2>/dev/null || true)", + "test": "bun run scripts/test-free-shards.ts && (bun run slop:diff 2>/dev/null || true)", "test:free": "bun run scripts/test-free-shards.ts", "test:windows": "bun run scripts/test-free-shards.ts --windows-only", "test:evals": "EVALS=1 bun test --retry 1 --concurrent --max-concurrency ${EVALS_CONCURRENCY:-15} test/skill-llm-eval.test.ts test/skill-e2e-*.test.ts test/skill-routing-e2e.test.ts test/codex-e2e.test.ts test/gemini-e2e.test.ts", diff --git a/scripts/test-free-shards.ts b/scripts/test-free-shards.ts index 156594ab1..87026dfc4 100755 --- a/scripts/test-free-shards.ts +++ b/scripts/test-free-shards.ts @@ -1,8 +1,8 @@ #!/usr/bin/env bun /** - * test-free-shards — enumerate, shard, and curate the free test suite. + * test-free-shards — enumerate, shard, curate, and run the free test suite. * - * Three jobs: + * Four jobs: * 1. Enumeration. Walk `browse/test/`, `test/`, `make-pdf/test/` and return * every `*.test.{ts,tsx,js,jsx,mjs,cjs}` that isn't a paid-eval test. * 2. Sharding. Stable-hash assign each test to one of N shards. Used by CI @@ -11,28 +11,70 @@ * patterns (`/bin/bash`, `sh -c`, raw `/tmp/`, `chmod`, `xargs`). Files * that match are excluded from the Windows-safe subset — they would fail * on `windows-latest` no matter how the runner shards them. + * 4. Execution. Spawn `bun test` children and refuse to trust their exit + * code alone: every byte of output is classified through + * scripts/test-strict-output.ts, so a child that exits 0 without bun's + * terminal summary (a mid-suite process.exit truncation), with `(fail)` + * result lines, or with fewer files run than planned is a FAILURE. An + * external wall-clock timeout SIGKILLs the child's process group and + * reports the shard as timed-out — distinct from failed. + * + * Execution strategy (decision ledger V3/D6 — evaluate the Bun built-in + * first; probed 2026-08 on Bun 1.3.13): + * - Full-suite runs (`bun test` via package.json, `bun run test:free`) use + * ONE child invocation with `--parallel`. Probes on real test files + * showed --parallel (a) prints the standard `Ran N tests across M files` + * terminal summary, (b) exits non-zero when any file fails, (c) runs each + * file in its own worker process (distinct pids, no shared globals), and + * (d) converts a mid-suite process.exit(0) — which silently truncates a + * serial run at exit 0 — into a per-file `(crashed: exited)` failure with + * a complete summary and exit 1. Strictly SAFER than the serial path and + * ~2x faster on a 6-file probe (0.22s -> 0.11s wall, 280% CPU); the win + * grows with suite size since the serial suite measured 454s. + * - CI-matrix runs (`--shards M --shard i`) keep the hash-partitioned + * one-child-per-shard path. Cross-runner partitioning must be + * deterministic and per-file stable, so bun's own `--shard=M/N` + * (round-robin over sorted paths — every assignment shifts when a file + * lands) is not used, and there are no static per-file weight lists. + * Shard indices are STABLE: assignFilesToShards never renumbers on + * occupancy, and an empty shard is a fast no-op success. * * Adapted from the McGluut/gstack fork's test-free-shards.ts (190 LOC). The * Windows-safe filter is upstream-original — codex flagged that sharding alone * doesn't fix POSIX-bound tests, so we curate the subset that actually runs * on the windows-latest CI job. * + * Exit codes: 0 pass, 1 fail, 124 wall-clock timeout. + * * Usage: - * bun run scripts/test-free-shards.ts --list # show all - * bun run scripts/test-free-shards.ts --windows-only --list # show curated - * bun run scripts/test-free-shards.ts --windows-only # run curated - * bun run scripts/test-free-shards.ts --shards 4 --shard 1 # one shard + * bun run scripts/test-free-shards.ts # full suite, one --parallel child + * bun run scripts/test-free-shards.ts --list # show all + * bun run scripts/test-free-shards.ts --windows-only --list # show curated + * bun run scripts/test-free-shards.ts --windows-only # run curated + * bun run scripts/test-free-shards.ts --shards 4 --shard 1 # one shard (CI matrix) + * bun run scripts/test-free-shards.ts --wall-timeout 600 # override the kill deadline */ import * as fs from 'fs'; +import * as os from 'os'; import * as path from 'path'; -import { spawnSync } from 'child_process'; +import { spawn } from 'child_process'; import { isPaidTestFile } from '../test/helpers/paid-test-set'; +import { + BunTestOutputClassifier, + exactTestFileSelectors, + forwardAndClassify, + installChildSignalForwarding, + killProcessGroup, + strictTestExitCode, +} from './test-strict-output'; const ROOT = path.resolve(import.meta.dir, '..'); // design/test was silently absent from BOTH the package.json test script and // this list — design tests (including a teardown bomb) never ran in any CI -// or local free run. Keep the two lists in sync. +// or local free run. Keep the two lists in sync. This list is the single +// source of truth for free-suite roots: package.json's `test` script routes +// through this runner rather than passing its own directory globs. const TEST_ROOTS = ['browse/test', 'test', 'make-pdf/test', 'design/test'] as const; const TEST_FILE_REGEX = /\.test\.(?:[cm]?[jt]s|tsx|jsx)$/; @@ -128,7 +170,15 @@ const KNOWN_WINDOWS_SAFE: Array<{ file: string; reason: string }> = [ ]; export const DEFAULT_SHARD_COUNT = 20; -export const FREE_TEST_TIMEOUT_MS = 10_000; +// Per-test timeout passed to `bun test --timeout`. 30s matches what +// package.json's `test` script used before it was repointed at this runner — +// the runner is now the single owner of that semantic. +export const FREE_TEST_TIMEOUT_MS = 30_000; +// External wall-clock deadline per spawned child (whole shard or the single +// full-suite --parallel invocation). A wedged child — a spinning main thread +// no in-process --timeout timer can interrupt — is SIGKILLed at the group +// level and reported 'timed-out', distinct from 'failed'. +export const DEFAULT_WALL_TIMEOUT_MS = 15 * 60_000; export function normalizeRelativePath(filePath: string): string { return filePath.replace(/\\/g, '/'); @@ -227,6 +277,15 @@ export function stableHash(input: string): number { return hash >>> 0; } +/** + * Hash-partition files across EXACTLY shardCount shards. Empty shards are + * preserved: a file's shard index is a pure function of its own path and the + * shard count, never of which other files happen to exist. A CI matrix keys + * runners off the index, so filtering empty shards (the old behavior) would + * renumber every later shard whenever occupancy shifted — runner 3 silently + * running shard 4's files. An empty shard is instead a fast no-op success at + * run time. + */ export function assignFilesToShards(files: string[], shardCount: number): string[][] { if (!Number.isInteger(shardCount) || shardCount <= 0) { throw new Error(`Shard count must be a positive integer. Received: ${shardCount}`); @@ -238,13 +297,24 @@ export function assignFilesToShards(files: string[], shardCount: number): string shards[shardIndex].push(file); } - return shards - .map(filesInShard => filesInShard.sort()) - .filter(filesInShard => filesInShard.length > 0); + return shards.map(filesInShard => filesInShard.sort()); } -export function buildShardArgs(files: string[]): string[] { - return ['test', ...files, '--max-concurrency=1', `--timeout=${FREE_TEST_TIMEOUT_MS}`]; +export interface BuildShardArgsOptions { + /** Run test files in parallel worker processes (bun 1.3.13+, implies --isolate). */ + parallel?: boolean; + rootDir?: string; +} + +export function buildShardArgs(files: string[], options: BuildShardArgsOptions = {}): string[] { + // Exact absolute selectors: bun treats positional test paths as substring + // filters, so a relative `test/x.test.ts` would ALSO select + // `browse/test/x.test.ts` — shard bleed that double-runs files. + const selectors = exactTestFileSelectors(files, options.rootDir ?? ROOT); + const args = ['test', ...selectors, `--timeout=${FREE_TEST_TIMEOUT_MS}`]; + if (options.parallel) args.push('--parallel'); + else args.push('--max-concurrency=1'); + return args; } type CliOptions = { @@ -253,6 +323,7 @@ type CliOptions = { windowsOnly: boolean; shardCount: number; shardIndex: number | null; + wallTimeoutMs: number; }; function parseCliOptions(argv: string[]): CliOptions { @@ -261,6 +332,7 @@ function parseCliOptions(argv: string[]): CliOptions { let windowsOnly = false; let shardCount = DEFAULT_SHARD_COUNT; let shardIndex: number | null = null; + let wallTimeoutMs = DEFAULT_WALL_TIMEOUT_MS; for (let index = 0; index < argv.length; index += 1) { const arg = argv[index]; @@ -281,10 +353,17 @@ function parseCliOptions(argv: string[]): CliOptions { index += 1; continue; } + if (arg === '--wall-timeout') { + const value = Number.parseInt(argv[index + 1] ?? '', 10); + if (!Number.isInteger(value) || value <= 0) throw new Error('--wall-timeout needs a positive integer (seconds)'); + wallTimeoutMs = value * 1000; + index += 1; + continue; + } throw new Error(`Unknown argument: ${arg}`); } - return { dryRun, listOnly, windowsOnly, shardCount, shardIndex }; + return { dryRun, listOnly, windowsOnly, shardCount, shardIndex, wallTimeoutMs }; } function formatShardSummary(shards: string[][]): string[] { @@ -301,40 +380,200 @@ function formatShardSummary(shards: string[][]): string[] { * summary AND hands back whatever code the caller passed — historically 0, * which made a truncated shard indistinguishable from a green one. Exit code * alone is therefore not evidence of completion; the summary line is. - * (Fault-injection coverage: test/exit-propagation.test.ts.) + * + * The runner itself now enforces this (and more) through + * scripts/test-strict-output.ts inside runFreeShard; this predicate remains + * the minimal documented primitive that test/exit-propagation.test.ts drives + * with genuine truncated and genuine complete bun runs. */ export function shardRunLooksTruncated(status: number | null, output: string): boolean { if (status !== 0) return false; // already failing — not the silent case return !/Ran \d+ tests? across \d+ files?/.test(output); } -function runShard(files: string[], shardNumber: number, totalShards: number): number { - const header = `[test:free] shard ${shardNumber}/${totalShards} (${files.length} files)`; - console.log(header); - const result = spawnSync(process.execPath, buildShardArgs(files), { - cwd: ROOT, - stdio: ['ignore', 'pipe', 'pipe'], - encoding: 'utf8', - env: process.env, - }); - // Preserve the inherit-style UX: replay the shard's output. - if (result.stdout) process.stdout.write(result.stdout); - if (result.stderr) process.stderr.write(result.stderr); - const combined = `${result.stdout ?? ''}${result.stderr ?? ''}`; - if (shardRunLooksTruncated(result.status, combined)) { - console.error( - `${header} exited 0 WITHOUT bun's final summary — the run was truncated ` + - '(a process.exit fired mid-suite). Treating as FAILED.', - ); - return 1; - } - if (result.status !== 0) { - console.error(`${header} failed with exit code ${result.status ?? 1}`); - } - return result.status ?? 1; +export type FreeShardStatus = 'passed' | 'failed' | 'timed-out'; + +export interface FreeShardOutcome { + shard: number; + files: string[]; + status: FreeShardStatus; + exitCode: number | null; + elapsedMs: number; + groupPid: number | null; } -function main(): number { +export interface ShardCommand { + command: string; + args: string[]; +} + +export interface RunFreeShardOptions { + /** External wall-clock deadline; on expiry the child's process GROUP is SIGKILLed. */ + wallTimeoutMs?: number; + rootDir?: string; + env?: NodeJS.ProcessEnv; + /** Full-suite mode: single bun invocation with --parallel (per-file worker isolation). */ + parallel?: boolean; + /** Override the spawned command. Tests inject fake pass/fail/slow commands. */ + commandFor?: (files: string[]) => ShardCommand; + /** Suppress forwarding child output to parent stdio (tests). Classification still sees every byte. */ + quiet?: boolean; + log?: (line: string) => void; +} + +const EPILOGUE_WORD: Record = { + passed: 'pass', + failed: 'fail', + 'timed-out': 'timed-out', +}; + +/** One line per shard, printed after the run: `[test:free] shard i/N: M files, XXs, pass|fail|timed-out`. */ +function shardEpilogue(outcome: FreeShardOutcome, totalShards: number): string { + return `[test:free] shard ${outcome.shard}/${totalShards}: ${outcome.files.length} files, ` + + `${Math.round(outcome.elapsedMs / 1000)}s, ${EPILOGUE_WORD[outcome.status]}`; +} + +/** + * Run one shard (or the whole suite, in --parallel full-suite mode) in its own + * bun process and classify the result strictly. + * + * Verdict integrity: the child's exit code is never trusted alone. Output is + * fed through BunTestOutputClassifier, and strictTestExitCode requires bun's + * terminal summary to report EXACTLY the planned file count — a shard that + * exits 0 without the summary (mid-suite process.exit truncation), with + * `(fail)` result lines, or having run fewer files than planned is a FAILURE. + * This is enforced for injected fake commands too (unlike the paid runner), + * so tests can pin the summary-missing => failure backstop; fake passing + * commands must print a synthetic `Ran N tests across M files. [Xms]` line. + * + * Per-shard state isolation: each spawned child gets its own throwaway + * GSTACK_HOME and TMPDIR (TEMP/TMP on Windows) so shards — and the bun + * --parallel workers inside the full-suite invocation — can't contend on the + * operator's real ~/.gstack or trip over each other's temp files. Tests that + * mkdtemp their own state dirs are unaffected: this only moves the DEFAULT + * location. The throwaway dirs are removed when the shard finishes. + */ +export async function runFreeShard( + files: string[], + shardNumber: number, + totalShards: number, + options: RunFreeShardOptions = {}, +): Promise { + const log = options.log ?? ((line: string) => console.log(line)); + const label = `[test:free] shard ${shardNumber}/${totalShards}`; + + // Empty shard = fast no-op SUCCESS. Indices are stable for the CI matrix, + // so an unoccupied index must not fail or shift work to a different runner. + if (files.length === 0) { + const outcome: FreeShardOutcome = { + shard: shardNumber, files: [], status: 'passed', exitCode: 0, elapsedMs: 0, groupPid: null, + }; + log(shardEpilogue(outcome, totalShards)); + return outcome; + } + + const rootDir = options.rootDir ?? ROOT; + const wallTimeoutMs = options.wallTimeoutMs ?? DEFAULT_WALL_TIMEOUT_MS; + log(`${label} (${files.length} files${options.parallel ? ', bun --parallel' : ''})`); + + const { command, args } = options.commandFor + ? options.commandFor(files) + : { command: process.execPath, args: buildShardArgs(files, { parallel: options.parallel, rootDir }) }; + + const env = { ...(options.env ?? process.env) }; + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-free-shard-')); + const gstackHome = path.join(stateDir, 'gstack-home'); + const childTmp = path.join(stateDir, 'tmp'); + fs.mkdirSync(gstackHome); + fs.mkdirSync(childTmp); + env.GSTACK_HOME = gstackHome; + env.TMPDIR = childTmp; + env.TEMP = childTmp; + env.TMP = childTmp; + + const startedAt = Date.now(); + const child = spawn(command, args, { + cwd: rootDir, + env, + stdio: ['ignore', 'pipe', 'pipe'], + detached: process.platform !== 'win32', + windowsHide: true, + }); + const groupPid = child.pid ?? null; + // Group-kill on parent SIGINT/SIGTERM too, not just on timeout. + const forwarding = installChildSignalForwarding({ + kill: (signal?: NodeJS.Signals | number) => { + killProcessGroup(child, (signal as NodeJS.Signals) ?? 'SIGTERM'); + return true; + }, + }); + + const classifier = new BunTestOutputClassifier(); + const devNull = { write: () => true } as unknown as NodeJS.WriteStream; + + let timedOut = false; + const killTimer = setTimeout(() => { + timedOut = true; + killProcessGroup(child, 'SIGKILL'); + }, wallTimeoutMs); + + let exitCode: number | null = null; + try { + const streams: Array> = []; + if (child.stdout) streams.push(forwardAndClassify(child.stdout, options.quiet ? devNull : process.stdout, classifier)); + if (child.stderr) streams.push(forwardAndClassify(child.stderr, options.quiet ? devNull : process.stderr, classifier)); + exitCode = await new Promise((resolve, reject) => { + child.once('error', reject); + child.once('close', (code) => resolve(code)); + }); + await Promise.all(streams); + } finally { + clearTimeout(killTimer); + forwarding.dispose(); + // Reap survivors of this shard even on the clean path. + killProcessGroup(child, 'SIGKILL'); + try { + fs.rmSync(stateDir, { recursive: true, force: true }); + } catch { + // Best-effort cleanup of a throwaway temp dir — a locked file on + // Windows must not turn a real verdict into an exception. + } + } + + const summary = classifier.end(); + const status: FreeShardStatus = timedOut + ? 'timed-out' + : strictTestExitCode(exitCode ?? 1, summary, files.length) === 0 ? 'passed' : 'failed'; + + if (status === 'timed-out') { + console.error( + `${label} exceeded the ${Math.round(wallTimeoutMs / 1000)}s wall-clock deadline — ` + + 'killed the process group. Reporting as TIMED-OUT (distinct from failed).', + ); + } else if (status === 'failed' && (exitCode ?? 1) === 0) { + const reason = summary.failedTests > 0 || summary.unhandledBetweenTests > 0 + ? `printed ${summary.failedTests} failing result(s) and ${summary.unhandledBetweenTests} unhandled error(s) between tests` + : summary.terminalFileCounts.length === 0 + ? "never printed bun's terminal summary — the run was truncated (a process.exit fired mid-suite)" + : `bun's summary reported ${summary.terminalFileCounts.join(', ')} file(s), expected ${files.length}`; + console.error(`${label} exited 0 but ${reason}. Treating as FAILED.`); + } else if (status === 'failed') { + console.error(`${label} failed with exit code ${exitCode ?? 'signal'}`); + } + + const outcome: FreeShardOutcome = { + shard: shardNumber, files, status, exitCode, elapsedMs: Date.now() - startedAt, groupPid, + }; + log(shardEpilogue(outcome, totalShards)); + return outcome; +} + +function exitCodeFor(status: FreeShardStatus): number { + if (status === 'passed') return 0; + return status === 'timed-out' ? 124 : 1; +} + +async function main(): Promise { const options = parseCliOptions(process.argv.slice(2)); const allFiles = collectFreeTestFiles(); if (allFiles.length === 0) { @@ -361,28 +600,46 @@ function main(): number { return 0; } - const shards = assignFilesToShards(files, options.shardCount); if (options.dryRun) { - console.log(`\nWould run ${files.length} files across ${shards.length} shards.`); + const shards = assignFilesToShards(files, options.shardCount); + const occupied = shards.filter((s) => s.length > 0).length; + console.log( + `\nWould run ${files.length} files across ${shards.length} shards (${occupied} occupied). ` + + 'Without --shard, the full suite runs as ONE bun --parallel invocation instead.', + ); for (const line of formatShardSummary(shards)) console.log(line); return 0; } if (options.shardIndex !== null) { - if (!Number.isInteger(options.shardIndex) || options.shardIndex < 1 || options.shardIndex > shards.length) { - throw new Error(`--shard must be between 1 and ${shards.length}. Received: ${options.shardIndex}`); + // Bounds-check against the REQUESTED shard count, not post-assignment + // occupancy — indices must be stable for a CI matrix, and an empty shard + // is a valid fast no-op. + if (!Number.isInteger(options.shardIndex) || options.shardIndex < 1 || options.shardIndex > options.shardCount) { + throw new Error(`--shard must be between 1 and ${options.shardCount}. Received: ${options.shardIndex}`); } - return runShard(shards[options.shardIndex - 1], options.shardIndex, shards.length); + const shards = assignFilesToShards(files, options.shardCount); + const outcome = await runFreeShard(shards[options.shardIndex - 1], options.shardIndex, options.shardCount, { + wallTimeoutMs: options.wallTimeoutMs, + }); + return exitCodeFor(outcome.status); } - for (let index = 0; index < shards.length; index += 1) { - const exitCode = runShard(shards[index], index + 1, shards.length); - if (exitCode !== 0) return exitCode; - } - - return 0; + // Full-suite mode: one bun invocation, files parallelized across per-file + // worker processes. See the header for the probe results that picked this + // over N spawned shard processes. + const outcome = await runFreeShard(files, 1, 1, { + parallel: true, + wallTimeoutMs: options.wallTimeoutMs, + }); + return exitCodeFor(outcome.status); } if (import.meta.main) { - process.exitCode = main(); + try { + process.exitCode = await main(); + } catch (error) { + console.error(`[test:free] ${error instanceof Error ? error.message : String(error)}`); + process.exitCode = 1; + } } diff --git a/scripts/test-strict-output.ts b/scripts/test-strict-output.ts index 16c073c0a..cfdd94514 100644 --- a/scripts/test-strict-output.ts +++ b/scripts/test-strict-output.ts @@ -103,6 +103,35 @@ export function installChildSignalForwarding( }; } +/** + * SIGKILL the shard's whole process group. Orphaned grandchildren (browsers, + * claude sessions) are how a stalled run once burned a core for 15.7 hours. + */ +export function killProcessGroup(child: ChildProcess, signal: NodeJS.Signals): void { + if (process.platform === 'win32' || typeof child.pid !== 'number') { + child.kill(signal); + return; + } + try { + process.kill(-child.pid, signal); + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ESRCH') return; // group already gone + if (code !== 'EPERM') throw err; + // Observed on macOS after a SIGKILLed group is reaped: signalling the + // now-empty group id returns EPERM, not ESRCH. Throwing here loses the + // shard's real outcome (a timeout gets recorded as a failure) and, from + // the timeout timer, leaves the shard promise unsettled — a hang, which + // is the exact failure class this runner exists to kill. Fall back to the + // direct pid so a genuinely-live child is still signalled. + try { + child.kill(signal); + } catch { + // Best-effort reap: nothing actionable is left if this fails too. + } + } +} + export function classifyBunTestOutputLine(rawLine: string): BunTestOutputFinding | null { const line = rawLine.replace(ANSI_ESCAPE, '').replace(/\r$/, ''); if (BUN_FAIL_RESULT.test(line)) return 'failed-test'; diff --git a/test/test-free-shards.test.ts b/test/test-free-shards.test.ts index 5e1cbd6ae..541ff4fe7 100644 --- a/test/test-free-shards.test.ts +++ b/test/test-free-shards.test.ts @@ -9,7 +9,10 @@ import { curateWindowsSafe, stableHash, assignFilesToShards, + buildShardArgs, normalizeRelativePath, + runFreeShard, + FREE_TEST_TIMEOUT_MS, } from '../scripts/test-free-shards'; const ROOT = path.resolve(import.meta.dir, '..'); @@ -106,12 +109,33 @@ describe('test-free-shards: sharding', () => { expect(stableHash('foo.test.ts')).not.toBe(stableHash('bar.test.ts')); }); - test('assignFilesToShards distributes files into N non-empty shards', () => { + test('assignFilesToShards partitions every file across exactly shardCount shards', () => { const files = ['a.test.ts', 'b.test.ts', 'c.test.ts', 'd.test.ts', 'e.test.ts']; const shards = assignFilesToShards(files, 3); - const flattened = shards.flat(); - expect(flattened.sort()).toEqual([...files].sort()); - expect(shards.every((s) => s.length > 0)).toBe(true); + expect(shards.length).toBe(3); + expect(shards.flat().sort()).toEqual([...files].sort()); + }); + + test('empty shards are preserved so indices stay stable for a CI matrix', () => { + // 2 files can never occupy 10 shards — the rest MUST be present and empty, + // not filtered out (filtering renumbered every later shard by occupancy). + const files = ['a.test.ts', 'b.test.ts']; + const shards = assignFilesToShards(files, 10); + expect(shards.length).toBe(10); + expect(shards.flat().sort()).toEqual([...files].sort()); + expect(shards.some((s) => s.length === 0)).toBe(true); + }); + + test("a file's shard index depends only on its own path — other files never renumber it", () => { + const target = 'test/target.test.ts'; + const expected = stableHash(target) % 7; + const alone = assignFilesToShards([target], 7); + const crowded = assignFilesToShards( + [target, 'test/a.test.ts', 'test/b.test.ts', 'test/c.test.ts', 'test/d.test.ts', 'browse/test/e.test.ts'], + 7, + ); + expect(alone.findIndex((s) => s.includes(target))).toBe(expected); + expect(crowded.findIndex((s) => s.includes(target))).toBe(expected); }); test('assignFilesToShards rejects invalid shard counts', () => { @@ -126,3 +150,136 @@ describe('test-free-shards: sharding', () => { expect(a).toEqual(b); }); }); + +describe('test-free-shards: shard args', () => { + test('resolves exact absolute selectors (no substring shard bleed) and pins the per-test timeout', () => { + const args = buildShardArgs(['test/foo.test.ts'], { rootDir: ROOT }); + expect(args[0]).toBe('test'); + expect(args[1]).toBe(path.resolve(ROOT, 'test/foo.test.ts')); + expect(args).toContain(`--timeout=${FREE_TEST_TIMEOUT_MS}`); + expect(args).toContain('--max-concurrency=1'); + expect(args).not.toContain('--parallel'); + }); + + test('parallel mode swaps serial max-concurrency for --parallel', () => { + const args = buildShardArgs(['test/foo.test.ts'], { rootDir: ROOT, parallel: true }); + expect(args).toContain('--parallel'); + expect(args).not.toContain('--max-concurrency=1'); + }); + + test('per-test timeout matches the 30s the package.json test script used before the repoint', () => { + expect(FREE_TEST_TIMEOUT_MS).toBe(30_000); + }); +}); + +describe('test-free-shards: strict shard execution', () => { + // Fake command seam, same pattern as test/paid-shards.test.ts: each "file" + // label selects a child command. Unlike the paid runner, runFreeShard + // enforces the terminal-summary file count on injected commands too, so + // fake PASSING commands must print a synthetic bun summary line. + const SUMMARY_1 = 'Ran 3 tests across 1 files. [12.00ms]'; + const BUSY_LOOP = 'const end = Date.now() + 600000; while (Date.now() < end) {}'; + const FAIL_LINE = '(fa' + 'il) planted failure [0.10ms]'; // split so this source file never contains a raw bun fail line + + const commandFor = (files: string[]) => { + const mode = files[0]; + if (mode === 'spin') return { command: process.execPath, args: ['-e', BUSY_LOOP] }; + if (mode === 'no-summary') return { command: process.execPath, args: ['-e', 'console.log("ok")'] }; + if (mode === 'fail-exit') { + return { command: process.execPath, args: ['-e', `console.log(${JSON.stringify(SUMMARY_1)}); process.exit(3)`] }; + } + if (mode === 'fail-line-exit-zero') { + return { command: process.execPath, args: ['-e', `console.log(${JSON.stringify(FAIL_LINE)}); console.log(${JSON.stringify(SUMMARY_1)})`] }; + } + if (mode === 'wrong-file-count') { + return { command: process.execPath, args: ['-e', 'console.log("Ran 3 tests across 4 files. [12.00ms]")'] }; + } + return { command: process.execPath, args: ['-e', `console.log(${JSON.stringify(SUMMARY_1)})`] }; + }; + + test('exit 0 WITHOUT bun\'s terminal summary is a FAILURE (anti-truncation backstop)', async () => { + const outcome = await runFreeShard(['no-summary'], 1, 1, { commandFor, quiet: true, log: () => {} }); + expect(outcome.status).toBe('failed'); + expect(outcome.exitCode).toBe(0); + }); + + test('exit 0 WITH the terminal summary passes, and the per-shard epilogue line is printed', async () => { + const lines: string[] = []; + const outcome = await runFreeShard(['pass'], 1, 1, { commandFor, quiet: true, log: (l) => lines.push(l) }); + expect(outcome.status).toBe('passed'); + expect(lines.some((l) => /^\[test:free\] shard 1\/1: 1 files, \d+s, pass$/.test(l))).toBe(true); + }); + + test('a non-zero exit stays a failure even when the summary is present', async () => { + const outcome = await runFreeShard(['fail-exit'], 1, 1, { commandFor, quiet: true, log: () => {} }); + expect(outcome.status).toBe('failed'); + expect(outcome.exitCode).toBe(3); + }); + + test('a printed (fail) result line is a failure even on exit 0 (bun exit-code bug class)', async () => { + const outcome = await runFreeShard(['fail-line-exit-zero'], 1, 1, { commandFor, quiet: true, log: () => {} }); + expect(outcome.status).toBe('failed'); + expect(outcome.exitCode).toBe(0); + }); + + test('a summary reporting the wrong file count is a failure (partial execution)', async () => { + const outcome = await runFreeShard(['wrong-file-count'], 1, 1, { commandFor, quiet: true, log: () => {} }); + expect(outcome.status).toBe('failed'); + }); + + test('a spinning shard is killed at the wall-clock deadline and reported timed-out, distinct from failed', async () => { + const lines: string[] = []; + const outcome = await runFreeShard(['spin'], 1, 1, { + commandFor, quiet: true, wallTimeoutMs: 1_200, log: (l) => lines.push(l), + }); + expect(outcome.status).toBe('timed-out'); + expect(outcome.status).not.toBe('failed'); + // Killed at the deadline, not left to burn the full 600s busy loop. + expect(outcome.elapsedMs).toBeLessThan(30_000); + expect(outcome.groupPid).toBeGreaterThan(0); + if (process.platform !== 'win32') { + expect(() => process.kill(outcome.groupPid as number, 0)).toThrow(); + } + expect(lines.some((l) => /^\[test:free\] shard 1\/1: 1 files, \d+s, timed-out$/.test(l))).toBe(true); + }, 30_000); + + test('an empty shard is a fast no-op success and never spawns (stable CI-matrix indices)', async () => { + const lines: string[] = []; + const outcome = await runFreeShard([], 7, 20, { + commandFor: () => { throw new Error('an empty shard must not spawn a child'); }, + log: (l) => lines.push(l), + }); + expect(outcome.status).toBe('passed'); + expect(lines.some((l) => /^\[test:free\] shard 7\/20: 0 files, 0s, pass$/.test(l))).toBe(true); + }); + + test('each spawned shard gets its own throwaway GSTACK_HOME and TMPDIR, removed after the run', async () => { + const captureDir = fs.mkdtempSync(path.join(os.tmpdir(), 'free-shard-env-')); + const dump = path.join(captureDir, 'env.json'); + try { + const script = + `const fs = require("fs");` + + `fs.writeFileSync(${JSON.stringify(dump)}, JSON.stringify({` + + ` home: process.env.GSTACK_HOME, tmp: process.env.TMPDIR,` + + ` homeExists: fs.existsSync(process.env.GSTACK_HOME || ""),` + + ` tmpExists: fs.existsSync(process.env.TMPDIR || "") }));` + + `console.log(${JSON.stringify(SUMMARY_1)});`; + const outcome = await runFreeShard(['env-dump'], 1, 1, { + commandFor: () => ({ command: process.execPath, args: ['-e', script] }), + quiet: true, + log: () => {}, + }); + expect(outcome.status).toBe('passed'); + const seen = JSON.parse(fs.readFileSync(dump, 'utf8')); + expect(seen.home).toContain('gstack-free-shard-'); + expect(seen.homeExists).toBe(true); + expect(seen.tmpExists).toBe(true); + expect(seen.home).not.toBe(process.env.GSTACK_HOME ?? ''); + expect(seen.tmp).not.toBe(process.env.TMPDIR ?? ''); + // The throwaway state dir is cleaned up once the shard finishes. + expect(fs.existsSync(seen.home)).toBe(false); + } finally { + fs.rmSync(captureDir, { recursive: true, force: true }); + } + }); +});