From 061638b667a5f4c9cb76ac6df8c8f14a613c04b0 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Fri, 23 Oct 2026 23:15:20 -0400 Subject: [PATCH] test: restore leaked listeners at file boundaries and pin guard-only leak fixes The global-state guard now puts the guarded process listeners back at each file boundary after reporting the change, so a finished file's leftover handler is no longer run or consumed by a later test's process.emit and charged to that test (HACK-1416). Only the difference is edited, so a kept once listener stays a once listener. The Loki follow also gets a test that removes its SIGINT listener through a real process.emit("SIGINT") inside a fixture that owns every SIGINT listener. The routing refused-teardown case restores only the spies its fixture installed instead of the global mock.restore(), and runs through the file's fixture gate (HACK-1388). The gate and the unknown-lifetime teardown are now shared by the gated runner and that case, which pins the runner's HACK_HOME release. File generation's PATH restore under an unknown lifetime and the protected-files settlement veto's process.kill restore get deterministic assertions. Refs HACK-1416, HACK-1388 Co-Authored-By: Claude Opus 5.5 --- .ai/skills/hack-repo-verify/SKILL.md | 8 +- tests/global-state-guard.test.ts | 81 +++++++- tests/helpers/global-state-guard-preload.ts | 16 +- tests/helpers/global-state-guard.ts | 76 +++++++- tests/helpers/owned-process-listeners.ts | 32 +++ tests/helpers/retained-routing-adoption.ts | 17 +- tests/loki-short-lived-logs.test.ts | 23 +++ ...e-compose-adoption-file-generation.test.ts | 33 +++- ...ompose-adoption-routing-generation.test.ts | 184 +++++++++++------- ...ive-config-protected-files-fixture.test.ts | 2 + 10 files changed, 387 insertions(+), 85 deletions(-) create mode 100644 tests/helpers/owned-process-listeners.ts diff --git a/.ai/skills/hack-repo-verify/SKILL.md b/.ai/skills/hack-repo-verify/SKILL.md index 2c83dba6..86a4ee8c 100644 --- a/.ai/skills/hack-repo-verify/SKILL.md +++ b/.ai/skills/hack-repo-verify/SKILL.md @@ -69,8 +69,12 @@ every `bun test` run: a test or file that leaves the environment, cwd, umask, exit code, a guarded global function or a process listener changed fails with a `[global-state-guard]` message naming the file. The guard restores the environment, cwd, umask and exit code so later files run clean; it keeps -replaced functions and listeners, which may still protect unsettled work. Fix -the named file's restore (`try`/`finally` or `afterEach`), not the later failures. +replaced functions, which may still protect unsettled work. It keeps a test's +leftover listeners until its file ends, then puts the file's listeners back, so +a later `process.emit` is not blamed for them. Fix the named file's restore +(`try`/`finally` or `afterEach`), not the later failures. To emit a real signal +in a test, use `withOwnedProcessListeners` from +`tests/helpers/owned-process-listeners.ts`. Restore environment keys with `restoreEnv` from `tests/helpers/env.ts`: Bun keeps a deleted `HTTP_PROXY`-style key for `fetch` and `Bun.spawn` without `env`, and the guard reports that as `native env changed`. diff --git a/tests/global-state-guard.test.ts b/tests/global-state-guard.test.ts index e3ff41fc..fbe24937 100644 --- a/tests/global-state-guard.test.ts +++ b/tests/global-state-guard.test.ts @@ -16,7 +16,9 @@ import { diffGlobalState, diffNativeEnv, restoreGlobalState, + restoreProcessListeners, } from "./helpers/global-state-guard.ts"; +import { withOwnedProcessListeners } from "./helpers/owned-process-listeners.ts"; const PRELOAD = resolve( import.meta.dir, @@ -133,6 +135,52 @@ test("a replaced function or listener is reported but left for its owner", () => expect(diffGlobalState(before)).toEqual([]); }); +test("the file-boundary listener restore removes leftovers, adds back missing listeners and keeps a kept once listener", async () => { + await withOwnedProcessListeners("SIGUSR2", () => { + let keptOnceCalls = 0; + const keptOnce = () => { + keptOnceCalls += 1; + }; + const removed = () => undefined; + const leftover = () => undefined; + const secondLeftover = () => undefined; + process.once("SIGUSR2", keptOnce); + process.on("SIGUSR2", removed); + try { + const before = captureGlobalState(); + process.removeListener("SIGUSR2", removed); + process.on("SIGUSR2", leftover); + process.once("SIGUSR2", secondLeftover); + expect(diffGlobalState(before)).toContain( + "process 'SIGUSR2' listeners changed (2 -> 3)" + ); + + expect(restoreProcessListeners(before)).toEqual(["SIGUSR2"]); + + expect(sameItems(process.listeners("SIGUSR2"), [keptOnce, removed])).toBe( + true + ); + expect(diffGlobalState(before)).toEqual([]); + expect(restoreProcessListeners(before)).toEqual([]); + // The kept listener was never re-added, so it is still a `once`. + process.emit("SIGUSR2"); + expect(keptOnceCalls).toBe(1); + expect(sameItems(process.listeners("SIGUSR2"), [removed])).toBe(true); + } finally { + for (const listener of [keptOnce, removed, leftover, secondLeftover]) { + process.removeListener("SIGUSR2", listener); + } + } + }); +}); + +function sameItems(left: readonly unknown[], right: readonly unknown[]) { + return ( + left.length === right.length && + left.every((item, index) => item === right[index]) + ); +} + const LEAK_IN_TEST = `import { test } from "bun:test"; test("sets PATH and fails before restoring", () => { const saved = process.env.PATH; @@ -159,7 +207,24 @@ test("sets a proxy and deletes it, which Bun's fetch never sees", () => { }); `; +// A once listener that this file never removes. Without the file-boundary +// restore, it stays in the process and a later test that emits SIGUSR2 +// consumes it and is blamed for the change (HACK-1416). +const LEAK_LISTENER = `import { test } from "bun:test"; +process.once("SIGUSR2", () => undefined); +test("passes but leaves a SIGUSR2 listener installed", () => {}); +`; + const LATER = `import { expect, test } from "bun:test"; +test("a later test that emits SIGUSR2 is not charged with an earlier listener", () => { + const own = () => undefined; + process.on("SIGUSR2", own); + try { + process.emit("SIGUSR2"); + } finally { + process.off("SIGUSR2", own); + } +}); test("a later file still finds executables on PATH", () => { const child = Bun.spawnSync(["hack-guard-probe"], { env: { ...process.env } }); expect(child.exitCode).toBe(0); @@ -206,6 +271,10 @@ async function runNestedSuite(opts: { join(suite, "c-leak-native-proxy.test.ts"), LEAK_NATIVE_PROXY ); + await writeFile( + join(suite, "c2-leak-signal-listener.test.ts"), + LEAK_LISTENER + ); await writeFile(join(suite, "d-later.test.ts"), LATER); const child = Bun.spawn( [ @@ -214,6 +283,7 @@ async function runNestedSuite(opts: { "./a-leak-in-test.test.ts", "./b-leak-in-hooks.test.ts", "./c-leak-native-proxy.test.ts", + "./c2-leak-signal-listener.test.ts", "./d-later.test.ts", ], { @@ -256,7 +326,16 @@ test("the guard names each leaking file, restores state, and later files pass", expect(run.output).toContain( "c-leak-native-proxy.test.ts left Bun's native environment changed after the file finished: native env HTTP_PROXY changed." ); - expect(run.output).toContain("2 test file(s) leaked process state"); + expect(run.output).toContain( + "c2-leak-signal-listener.test.ts left process state changed after the file finished: process 'SIGUSR2' listeners changed (0 -> 1). Restored the environment, cwd and process listeners" + ); + // The leftover listener was removed at the file boundary, so the later + // test that emits SIGUSR2 is not blamed for it. + expect(run.output).not.toContain("a test in d-later.test.ts"); + expect(run.output).toContain( + "(pass) a later test that emits SIGUSR2 is not charged with an earlier listener" + ); + expect(run.output).toContain("3 test file(s) leaked process state"); expect(run.output).not.toContain("127.0.0.1:9"); expect(run.output).not.toContain("Executable not found"); expect(run.output).toContain("LATER-FILE-PASSED"); diff --git a/tests/helpers/global-state-guard-preload.ts b/tests/helpers/global-state-guard-preload.ts index a7041435..b7b88791 100644 --- a/tests/helpers/global-state-guard-preload.ts +++ b/tests/helpers/global-state-guard-preload.ts @@ -2,8 +2,9 @@ * `bun test` preload (see `bunfig.toml`): fails loudly when a test or a test * file leaves process-global state changed, names the file, and restores the * environment, working directory, umask and exit code so later files do not - * fail for an unrelated reason. Replaced functions and process listeners are - * reported but kept; `restoreGlobalState` explains why. + * fail for an unrelated reason. Replaced functions are reported but kept; + * `restoreGlobalState` explains why. Process listeners are reported and kept + * per test, and put back at the file boundary (`restoreProcessListeners`). * * Two checks run: * - Per test: the state before the global `beforeEach` must match the state @@ -14,8 +15,11 @@ * - Per file: when Bun loads the next test file, the state must match the * state before the previous file loaded. This catches module-scope and * `beforeAll`/`afterAll` leaks, including an `afterAll` that never ran its - * restore. A mismatch is reported at once, the pre-file environment is restored, - * and the run fails from the preload `afterAll` with every leaking file. + * restore. A mismatch is reported at once, the pre-file environment and + * process listeners are restored, and the run fails from the preload + * `afterAll` with every leaking file. Restoring the listeners keeps a + * finished file's leftover handler from being run or consumed by a later + * test's `process.emit`, which would blame that later test (HACK-1416). * This check also compares Bun's native environment (one short `/usr/bin/env` * child per file), which keeps a proxy key that a test set and then deleted * although `process.env` no longer shows it. @@ -37,6 +41,7 @@ import { type GlobalStateSnapshot, type NativeEnv, restoreGlobalState, + restoreProcessListeners, } from "./global-state-guard.ts"; /** @@ -92,9 +97,10 @@ function checkFileBoundary(): void { if (changes.length > 0) { reportFileLeak( `${displayPath(currentFile)} left process state changed after the file finished: ${changes.join("; ")}`, - "Restored the environment and cwd from before the file so later files run clean." + "Restored the environment, cwd and process listeners from before the file so later files run clean." ); restoreGlobalState(fileBaseline); + restoreProcessListeners(fileBaseline); } checkNativeEnv(); fileBaseline = captureGlobalState(); diff --git a/tests/helpers/global-state-guard.ts b/tests/helpers/global-state-guard.ts index 948de7af..e7a4eda2 100644 --- a/tests/helpers/global-state-guard.ts +++ b/tests/helpers/global-state-guard.ts @@ -15,7 +15,11 @@ * because fixtures may hold secret-shaped values. */ -/** Process events whose listener lists are compared (reported, not restored). */ +/** + * Process events whose listener lists are compared. A change is reported per + * test and per file, and restored only at the file boundary (see + * {@link restoreProcessListeners}). + */ export const GUARDED_PROCESS_EVENTS = [ "SIGINT", "SIGTERM", @@ -278,11 +282,12 @@ function restoreEnvObject(snapshot: GlobalStateSnapshot): void { * * Replaced functions (`Bun.spawn`, `process.kill`, `fetch`, timers), process * listeners and `process.platform` are reported by {@link diffGlobalState} - * but deliberately not restored. A leak there usually comes from a test that - * timed out while its work is still running, and that fake is what keeps the - * leftover work away from real processes, signals, the network and the + * but deliberately not restored here. A leak there usually comes from a test + * that timed out while its work is still running, and that fake is what keeps + * the leftover work away from real processes, signals, the network and the * Docker socket. Restoring the real function under that work could let it - * reach the host. + * reach the host. Listeners are the exception at a file boundary; see + * {@link restoreProcessListeners}. */ export function restoreGlobalState(snapshot: GlobalStateSnapshot): void { restoreEnvObject(snapshot); @@ -295,6 +300,67 @@ export function restoreGlobalState(snapshot: GlobalStateSnapshot): void { process.exitCode = snapshot.exitCode; } +function countListeners(listeners: readonly Listener[]): Map { + const counts = new Map(); + for (const listener of listeners) { + counts.set(listener, (counts.get(listener) ?? 0) + 1); + } + return counts; +} + +/** + * Puts the guarded process listeners back to `snapshot` and returns the events + * it changed. The preload calls this at a file boundary after it has reported + * the change, so a listener a finished file left behind cannot run (or be + * consumed by) a later file's `process.emit` and get that later test blamed + * (HACK-1416). + * + * Only the difference is edited: a listener added since the snapshot is + * removed and a snapshot listener that went missing is added back with + * `process.on`. Listeners present in both stay untouched, so a `once` + * listener there keeps its `once` behavior; Bun does not expose `once`, so a + * missing listener always comes back as a persistent one, at the end of the + * list. + * + * Unlike replaced functions, removing a leftover listener cannot let unsettled + * work reach the host: it only stops that work from reacting to a signal or + * exit event of the shared test process. + */ +export function restoreProcessListeners( + snapshot: GlobalStateSnapshot +): GuardedProcessEvent[] { + const restored: GuardedProcessEvent[] = []; + for (const event of GUARDED_PROCESS_EVENTS) { + const was = snapshot.listeners.get(event) ?? []; + const now = process.listeners(event) as Listener[]; + if (sameListeners(was, now)) { + continue; + } + restored.push(event); + const wanted = countListeners(was); + const present = countListeners(now); + for (const [listener, count] of present) { + for ( + let extra = count - (wanted.get(listener) ?? 0); + extra > 0; + extra-- + ) { + process.removeListener(event, listener); + } + } + const kept = countListeners(process.listeners(event) as Listener[]); + for (const listener of was) { + const left = kept.get(listener) ?? 0; + if (left > 0) { + kept.set(listener, left - 1); + } else { + process.on(event, listener); + } + } + } + return restored; +} + /** * The environment Bun itself uses for `fetch` proxies and for `Bun.spawn` * without an `env` option, as key to value. diff --git a/tests/helpers/owned-process-listeners.ts b/tests/helpers/owned-process-listeners.ts new file mode 100644 index 00000000..5b310766 --- /dev/null +++ b/tests/helpers/owned-process-listeners.ts @@ -0,0 +1,32 @@ +/** + * Lets a test emit a real process event without reaching listeners it does not + * own. + * + * Every test file shares one Bun process, so `process.emit("SIGINT")` in a + * test runs every SIGINT listener in that process, including a leftover from + * another file. {@link withOwnedProcessListeners} removes the listeners that + * are present, runs the callback, and adds them back afterwards, so the + * callback sees only the listeners it installs itself. + * + * Listeners the callback leaves behind are kept, so the test's own assertions + * and the global-state guard still see them. Bun does not expose `once`, so a + * saved `once` listener comes back as a persistent one. Between tests the + * guarded events normally have no listeners (the guard fails a file that + * leaves one), so the saved list is normally empty. + */ +export async function withOwnedProcessListeners( + event: NodeJS.Signals | "exit", + run: () => Promise | T +): Promise { + const saved = process.listeners(event); + for (const listener of saved) { + process.removeListener(event, listener); + } + try { + return await run(); + } finally { + for (const listener of saved) { + process.on(event, listener); + } + } +} diff --git a/tests/helpers/retained-routing-adoption.ts b/tests/helpers/retained-routing-adoption.ts index c6d1f17a..f70aedb3 100644 --- a/tests/helpers/retained-routing-adoption.ts +++ b/tests/helpers/retained-routing-adoption.ts @@ -552,6 +552,11 @@ else { return originalKill.call(process, pid, signal); }), ]; + const restoreDoubles = () => { + for (const spy of spies) { + spy.mockRestore(); + } + }; const receiptPath = join( root, ".hack/.internal/legacy-compose-adoption-v1/receipt.json" @@ -682,14 +687,20 @@ else { releaseEnvironment: () => { restoreEnv("HACK_HOME", previousHome); }, + /** Whether this fixture's own global doubles are still installed. */ + doublesInstalled: () => process.kill !== originalKill, + /** + * Restores this fixture's own spies, and no other spy in the shared Bun + * process, whatever the lifetime says. Only for a case that has proven no + * work it started is still running; {@link cleanup} is the gated path. + */ + restoreDoubles, cleanup: async () => { restoreEnv("HACK_HOME", previousHome); if (!lifetime.canRestore()) { return refuse(); } - for (const spy of spies) { - spy.mockRestore(); - } + restoreDoubles(); await rm(outer, { recursive: true, force: true }); }, }; diff --git a/tests/loki-short-lived-logs.test.ts b/tests/loki-short-lived-logs.test.ts index f5a5eb43..864205fa 100644 --- a/tests/loki-short-lived-logs.test.ts +++ b/tests/loki-short-lived-logs.test.ts @@ -20,6 +20,7 @@ import { lokiLogs, lokiSnapshot, } from "../src/ui/loki-logs.ts"; +import { withOwnedProcessListeners } from "./helpers/owned-process-listeners.ts"; // HACK-1277: a one-shot Compose service that exits quickly must not make // Loki-backed `hack logs` look empty while `docker compose logs` has output. @@ -573,3 +574,25 @@ test("Loki follow stops on SIGINT and leaves no listener behind", async () => { } ); }); + +test("Loki follow drops its SIGINT listener through a real SIGINT emit", async () => { + // The fixture owns every SIGINT listener, so the emit reaches only the + // follow's handler and not another file's or test's. + await withOwnedProcessListeners("SIGINT", () => + withFakeLokiTail( + () => undefined, + async ({ baseUrl, sockets }) => { + captureStdout(); + const exit = lokiLogs({ baseUrl, ...FOLLOW_PARAMS }); + await waitFor(() => sockets.length === 1, "the tail connection"); + expect(process.listenerCount("SIGINT")).toBe(1); + process.emit("SIGINT"); + // The signal path itself removes the `once` listener, before the + // tail closes and the follow's `finally` runs. + expect(process.listenerCount("SIGINT")).toBe(0); + expect(await exit).toBe(0); + expect(process.listenerCount("SIGINT")).toBe(0); + } + ) + ); +}); diff --git a/tests/native-compose-adoption-file-generation.test.ts b/tests/native-compose-adoption-file-generation.test.ts index cbb04dae..f1fee675 100644 --- a/tests/native-compose-adoption-file-generation.test.ts +++ b/tests/native-compose-adoption-file-generation.test.ts @@ -172,10 +172,13 @@ else {const source=JSON.parse(await Bun.stdin.text());const {schema_version,...p pathReplaced = true; process.env.PATH = root; }); -afterEach(async () => { - // PATH is restored even when the lifetime is unknown: a PATH left pointing - // at this fixture root hides git and bun from every later file in the Bun - // process (HACK-1327). The root and spawn spy stay retained. +/** + * The per-case teardown, run from `afterEach`. PATH is restored even when the + * lifetime is unknown: a PATH left pointing at this fixture root hides git and + * bun from every later file in the Bun process (HACK-1327). The root and spawn + * spy stay retained, and later cases in this file refuse. + */ +async function releaseRetainedFileFixture() { if (pathReplaced) { pathReplaced = false; restoreEnv("PATH", priorPath); @@ -189,6 +192,28 @@ afterEach(async () => { spawnSpy?.mockRestore(); spawnSpy = undefined; await rm(root, { recursive: true, force: true }); +} +afterEach(releaseRetainedFileFixture); +test("an unknown lifetime still restores PATH before the teardown refuses", async () => { + expect(process.env.PATH).toBe(root); + const owned = lifetime; + const refused = retainedRoutingFixtureLifetime(Date.now() + 5000); + refused.retain(); + lifetime = refused; + try { + await expect(releaseRetainedFileFixture()).rejects.toThrow( + "Retained-file fixture callback or child is unsettled" + ); + expect(uncertain).toBe(true); + expect(process.env.PATH).toBe(priorPath); + // The refusal kept the spawn spy for the unsettled work. + expect(Bun.spawn).not.toBe(originalSpawn); + } finally { + // Only this case's synthetic lifetime was unknown; its own lifetime ends + // normally in afterEach, which restores the spy and removes the root. + uncertain = false; + lifetime = owned; + } }); async function save() { await writeFile(join(root, "fixture.json"), JSON.stringify(fixture)); diff --git a/tests/native-compose-adoption-routing-generation.test.ts b/tests/native-compose-adoption-routing-generation.test.ts index 0906d705..47977224 100644 --- a/tests/native-compose-adoption-routing-generation.test.ts +++ b/tests/native-compose-adoption-routing-generation.test.ts @@ -1,4 +1,4 @@ -import { test as boundedTest, expect, mock, spyOn } from "bun:test"; +import { test as boundedTest, expect, spyOn } from "bun:test"; import * as fs from "node:fs/promises"; import { readFile, rm } from "node:fs/promises"; import { join } from "node:path"; @@ -37,9 +37,86 @@ const SINGLE_OPERATION_TEST_BUDGET_MS = 180_000; * but well inside the product's default caller deadline: a slow but unchanged * operation. */ const SLOW_OPERATION_ADVANCE_MS = 16_000; -let h: Awaited>; +type RoutingFixture = Awaited>; +type RoutingFixtureLifetime = ReturnType; +let h: RoutingFixture; let fixtureActive = false; let fixtureUncertain = false; +/** + * The file's fixture gate. A case refuses while an earlier case's fixture is + * still active or its lifetime is unknown, because that fixture's global + * doubles may still be installed. `run` calls `restored()` only once every + * global double and root it created is restored; otherwise the gate stays + * closed and every later gated case in this file refuses. + */ +const gatedTest = ( + name: string, + run: (restored: () => void) => Promise, + timeoutMs: number +) => + boundedTest( + name, + async () => { + if (fixtureActive || fixtureUncertain) { + fixtureUncertain = true; + throw new Error( + "Prior retained routing fixture lifetime is unknown; values omitted." + ); + } + fixtureActive = true; + let isRestored = false; + try { + await run(() => { + isRestored = true; + }); + } finally { + if (isRestored) { + fixtureActive = false; + } else { + fixtureUncertain = true; + } + } + }, + timeoutMs + ); +/** + * Ends a gated case's fixture. With a known lifetime it runs the full + * teardown. With an unknown lifetime it keeps the spies and root for the + * unsettled work but still restores HACK_HOME, so later files never inherit + * the fixture home (HACK-1327). `failure` is set whenever not everything was + * restored. + */ +async function endRoutingFixture( + lifetime: RoutingFixtureLifetime, + fixture: RoutingFixture | undefined +): Promise<{ readonly restored: boolean; readonly failure?: unknown }> { + if (!lifetime.canRestore()) { + fixture?.releaseEnvironment(); + return { + restored: false, + failure: new Error( + "Retained routing fixture lifetime is unknown; values omitted." + ), + }; + } + if (fixture === undefined) { + // Setup did not issue a complete fixture; do not reuse its global context. + lifetime.retain(); + return { + restored: false, + failure: new Error( + "Retained routing fixture setup is incomplete; values omitted." + ), + }; + } + try { + await fixture.cleanup(); + return { restored: true }; + } catch (error: unknown) { + lifetime.retain(); + return { restored: false, failure: error }; + } +} const test = ( name: string, run: () => Promise, @@ -52,68 +129,31 @@ const test = ( } = {} ) => { const timeoutMs = opts.timeoutMs ?? 30_000; - return boundedTest( + return gatedTest( name, - async () => { - if (fixtureActive || fixtureUncertain) { - fixtureUncertain = true; - throw new Error( - "Prior retained routing fixture lifetime is unknown; values omitted." - ); - } - fixtureActive = true; + async (restored) => { const lifetime = retainedRoutingFixtureLifetime(Date.now() + timeoutMs); - let issued = false; + let fixture: RoutingFixture | undefined; let failed = false; let failure: unknown; try { h = await retainedRoutingFixture(lifetime); - issued = true; + fixture = h; await run(); } catch (error: unknown) { failed = true; failure = error; - } finally { - if (!lifetime.canRestore()) { - fixtureUncertain = true; - // Keep the spies and root for the unsettled work, but never leave - // HACK_HOME pointing at this fixture for later files (HACK-1327). - if (issued) { - h.releaseEnvironment(); - } - if (!failed) { - failed = true; - failure = new Error( - "Retained routing fixture lifetime is unknown; values omitted." - ); - } - } else if (issued) { - try { - await h.cleanup(); - fixtureActive = false; - } catch (error: unknown) { - lifetime.retain(); - fixtureUncertain = true; - if (!failed) { - failed = true; - failure = error; - } - } - } else { - // Setup did not issue a complete fixture; do not reuse its global context. - lifetime.retain(); - fixtureUncertain = true; - if (!failed) { - failed = true; - failure = new Error( - "Retained routing fixture setup is incomplete; values omitted." - ); - } - } + } + const end = await endRoutingFixture(lifetime, fixture); + if (end.restored) { + restored(); } if (failed) { throw failure; } + if (!end.restored) { + throw end.failure; + } }, timeoutMs ); @@ -864,25 +904,39 @@ boundedTest( }, 1000 ); -boundedTest( +gatedTest( "a refused teardown still restores HACK_HOME so later files never inherit the fixture home", - async () => { + async (restored) => { const before = process.env.HACK_HOME; - const lifetime = retainedRoutingFixtureLifetime(Date.now() + 5000); - const fixture = await retainedRoutingFixture(lifetime); + // Stands in for another fixture's spy that is kept on purpose while its + // settlement is unknown: this case must restore only its own spies. + const foreign = spyOn(privateState, "readPrivate"); try { - expect(process.env.HACK_HOME).toBe(fixture.home); - fixture.releaseEnvironment(); - expect(process.env.HACK_HOME).toBe(before); - process.env.HACK_HOME = fixture.home; - lifetime.retain(); - await expect(fixture.cleanup()).rejects.toThrow(); - expect(process.env.HACK_HOME).toBe(before); + const lifetime = retainedRoutingFixtureLifetime(Date.now() + 5000); + const fixture = await retainedRoutingFixture(lifetime); + try { + expect(process.env.HACK_HOME).toBe(fixture.home); + lifetime.retain(); + // The gated runner's own teardown path for an unknown lifetime. + const end = await endRoutingFixture(lifetime, fixture); + expect(end.restored).toBe(false); + expect(end.failure).toBeInstanceOf(Error); + expect(process.env.HACK_HOME).toBe(before); + expect(fixture.doublesInstalled()).toBe(true); + process.env.HACK_HOME = fixture.home; + await expect(fixture.cleanup()).rejects.toThrow(); + expect(process.env.HACK_HOME).toBe(before); + expect(fixture.doublesInstalled()).toBe(true); + } finally { + restoreEnv("HACK_HOME", before); + fixture.restoreDoubles(); + await rm(fixture.outer, { recursive: true, force: true }); + restored(); + } + expect(fixture.doublesInstalled()).toBe(false); + expect(privateState.readPrivate).toBe(foreign); } finally { - restoreEnv("HACK_HOME", before); - // The refused teardown kept the fixture spies; nothing else is mocked here. - mock.restore(); - await rm(fixture.outer, { recursive: true, force: true }); + foreign.mockRestore(); } }, 5000 diff --git a/tests/native-config-protected-files-fixture.test.ts b/tests/native-config-protected-files-fixture.test.ts index fde295a4..b4900ba3 100644 --- a/tests/native-config-protected-files-fixture.test.ts +++ b/tests/native-config-protected-files-fixture.test.ts @@ -780,6 +780,7 @@ test("closed-pipe survivor refuses completion and permanently vetoes restoration `await Bun.write(${JSON.stringify(pidPath)},String(process.pid));await Bun.sleep(1500);` ); const settlement = createCompletedJobFixtureSettlement(); + const unmockedKill = process.kill; const original = process.kill.bind(process), groups: number[] = []; const signal = spyOn(process, "kill").mockImplementation((pid, kind) => { @@ -851,4 +852,5 @@ test("closed-pipe survivor refuses completion and permanently vetoes restoration // Restore it here: the vetoed teardown would otherwise leave process.kill // replaced for every later file in the Bun process (HACK-1327). signal.mockRestore(); + expect(process.kill).toBe(unmockedKill); });