From bbb744269fcbe0856a54f4ed86baea4544b7b15b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 17:17:42 -0400 Subject: [PATCH] test: make the local pnpm test gate green on macOS hosts (#100069) Fixes 21 macOS-only failing test cases across three classes: canonicalized fixture roots (macOS /var -> /private/var tmpdir symlink vs production realpathing), load-tolerant process-spawn tests (content-gated pid files, readiness-sequenced kill windows, bounded-window timer assertions), and cross-file worker leak guards (skip-channel env, gateway token env, imessage runtime singleton). Test-only; no production changes. Fixes #100025. --- .../run-attempt.turn-watches.test.ts | 21 ++++++- .../src/monitor.watch-subscribe-retry.test.ts | 5 ++ .../gateway-cli/run.option-collisions.test.ts | 7 +++ src/cli/logs-cli.test.ts | 6 +- src/cli/run-main.exit.test.ts | 5 ++ src/gateway/server-reload-handlers.test.ts | 11 +++- src/infra/exec-approvals-analysis.test.ts | 10 ++- src/secrets/resolve.test.ts | 9 ++- src/security/install-policy.test.ts | 9 ++- src/test-helpers/temp-dir.ts | 12 +++- src/test-utils/fixture-suite.ts | 5 +- src/test-utils/openclaw-test-state.ts | 5 +- src/test-utils/process-tree.ts | 5 ++ test/scripts/crabbox-wrapper.test.ts | 6 +- test/scripts/dev-tooling-safety.test.ts | 37 +++++++---- ...tension-package-boundary-artifacts.test.ts | 61 +++++++++++++------ .../secret-provider-integrations.test.ts | 30 ++++++--- 17 files changed, 188 insertions(+), 56 deletions(-) diff --git a/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts b/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts index 6fa64a3e48d0..1e9c58a75fd7 100644 --- a/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts +++ b/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts @@ -246,7 +246,18 @@ describe("runCodexAppServerAttempt turn watches", () => { const run = runCodexAppServerAttempt(params); await harness.waitForMethod("turn/start"); - expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), expectedTerminalIdleTimeoutMs); + // Real timers: the delay is timeout minus real ms elapsed since the last + // activity timestamp, so under suite load it lands slightly below the + // configured value. Accept a small window instead of exact equality. + const terminalIdleDelays = setTimeoutSpy.mock.calls + .map(([, delay]) => delay) + .filter((delay): delay is number => typeof delay === "number"); + expect( + terminalIdleDelays.some( + (delay) => + delay <= expectedTerminalIdleTimeoutMs && delay > expectedTerminalIdleTimeoutMs - 5_000, + ), + ).toBe(true); await harness.notify({ method: "turn/completed", params: { @@ -4986,7 +4997,11 @@ describe("runCodexAppServerAttempt turn watches", () => { path.join(tempDir, "session.jsonl"), path.join(tempDir, "workspace"), ); - params.timeoutMs = 200; + // Generous overall cap: promptness is proven by the 5ms completion-idle + // timeout producing the idle promptError below, not by this bound. A tight + // cap races attempt startup under parallel-suite load and turn/start never + // happens. + params.timeoutMs = 30_000; const run = runCodexAppServerAttempt(params, { turnCompletionIdleTimeoutMs: 5, @@ -4995,7 +5010,7 @@ describe("runCodexAppServerAttempt turn watches", () => { await vi.waitFor( () => expect(request).toHaveBeenCalledWith("turn/start", expect.anything(), expect.anything()), - { interval: 1 }, + { interval: 5, timeout: 10_000 }, ); await notify({ method: "item/started", diff --git a/extensions/imessage/src/monitor.watch-subscribe-retry.test.ts b/extensions/imessage/src/monitor.watch-subscribe-retry.test.ts index 7a1d093f956e..bd29d33f2cdc 100644 --- a/extensions/imessage/src/monitor.watch-subscribe-retry.test.ts +++ b/extensions/imessage/src/monitor.watch-subscribe-retry.test.ts @@ -8,6 +8,7 @@ import { describeIMessageInboundDropDiagnostic, shouldThrottleIMessageInboundDropDiagnostic, } from "./monitor/monitor-provider.js"; +import { clearIMessageRuntime } from "./runtime.js"; const waitForTransportReadyMock = vi.hoisted(() => vi.fn(async () => {}), @@ -67,6 +68,10 @@ function createRpcClient(overrides?: { describe("monitorIMessageProvider watch.subscribe startup retry", () => { beforeEach(() => { vi.useFakeTimers(); + // Sibling suites install the imessage runtime singleton without clearing + // it; a leaked runtime resurrects another file's recovery cursor and + // watch.subscribe then gains an unexpected since_rowid. + clearIMessageRuntime(); waitForTransportReadyMock.mockReset().mockResolvedValue(undefined); createIMessageRpcClientMock.mockReset(); attachIMessageMonitorAbortHandlerMock.mockReset().mockReturnValue(() => {}); diff --git a/src/cli/gateway-cli/run.option-collisions.test.ts b/src/cli/gateway-cli/run.option-collisions.test.ts index 1fa292607e72..5f55717719ca 100644 --- a/src/cli/gateway-cli/run.option-collisions.test.ts +++ b/src/cli/gateway-cli/run.option-collisions.test.ts @@ -101,10 +101,15 @@ const withoutGatewayAuthEnv = { }; const { runtimeErrors, defaultRuntime, resetRuntimeCapture } = createCliRuntimeCapture(); +// gateway run exports --token/--password into process.env as a side effect +// (see runGatewayCli auth wiring); snapshot and clear them so shared vitest +// workers do not leak credentials into later files' gateway connects. const serviceEnvSnapshot = captureEnv([ "OPENCLAW_SERVICE_MARKER", "OPENCLAW_SERVICE_KIND", GATEWAY_SERVICE_RUNTIME_PID_ENV, + "OPENCLAW_GATEWAY_TOKEN", + "OPENCLAW_GATEWAY_PASSWORD", ]); vi.mock("../../config/config.js", () => ({ @@ -304,6 +309,8 @@ describe("gateway run option collisions", () => { beforeEach(() => { delete process.env.OPENCLAW_SERVICE_MARKER; delete process.env.OPENCLAW_SERVICE_KIND; + delete process.env.OPENCLAW_GATEWAY_TOKEN; + delete process.env.OPENCLAW_GATEWAY_PASSWORD; deleteTestEnvValue(GATEWAY_SERVICE_RUNTIME_PID_ENV); resetRuntimeCapture(); configState.cfg = {}; diff --git a/src/cli/logs-cli.test.ts b/src/cli/logs-cli.test.ts index 13588342095a..ab3cbfeea895 100644 --- a/src/cli/logs-cli.test.ts +++ b/src/cli/logs-cli.test.ts @@ -535,7 +535,11 @@ describe("logs cli", () => { const stdoutWrites = captureStdoutWrites(); const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); - await runLogsCli(["logs", "--follow", "--plain", "--interval", "1", "--timeout", "250"]); + // Pin UTC: the recovered-line assertion below checks a rendered + // timestamp, which otherwise follows the host time zone. + await withTimeZone("UTC", () => + runLogsCli(["logs", "--follow", "--plain", "--interval", "1", "--timeout", "250"]), + ); expect(readConfiguredLogTail).not.toHaveBeenCalled(); expect(execFileUtf8Tail).toHaveBeenCalledTimes(2); diff --git a/src/cli/run-main.exit.test.ts b/src/cli/run-main.exit.test.ts index fdf484229c01..9a643d61746b 100644 --- a/src/cli/run-main.exit.test.ts +++ b/src/cli/run-main.exit.test.ts @@ -383,6 +383,11 @@ describe("runCli exit behavior", () => { beforeEach(() => { delete process.env.OPENCLAW_SERVICE_MARKER; delete process.env.OPENCLAW_SERVICE_KIND; + // Sibling CLI suites run `gateway run --token/--password`, which exports + // credentials into process.env; leaked values change gateway preflight + // auth in shared vitest workers. + delete process.env.OPENCLAW_GATEWAY_TOKEN; + delete process.env.OPENCLAW_GATEWAY_PASSWORD; delete process.env[GATEWAY_SERVICE_RUNTIME_PID_ENV]; vi.clearAllMocks(); readConfigFileSnapshotMock.mockResolvedValue({ diff --git a/src/gateway/server-reload-handlers.test.ts b/src/gateway/server-reload-handlers.test.ts index d27d4a5f76e9..71ae39d5dcd3 100644 --- a/src/gateway/server-reload-handlers.test.ts +++ b/src/gateway/server-reload-handlers.test.ts @@ -1,7 +1,7 @@ /** * Gateway config reload handler tests. */ -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { ConfigWriteNotification } from "../config/config.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { consumeGatewaySigusr1RestartIntent } from "../infra/restart.js"; @@ -215,6 +215,15 @@ function createReloadHandlersForTest( return { ...handlers, cron, heartbeatRunner, setState, stopExitWatchers }; } +// Other gateway test helpers (test-helpers.mocks.ts, test-helpers.server.ts) +// set OPENCLAW_SKIP_CHANNELS / OPENCLAW_SKIP_PROVIDERS at module load. When a +// shared vitest worker imports those helpers before this file runs, the leaked +// env routes reloads into the skip branch and channel restarts never fire. +beforeEach(() => { + delete process.env.OPENCLAW_SKIP_CHANNELS; + delete process.env.OPENCLAW_SKIP_PROVIDERS; +}); + afterEach(() => { vi.useRealTimers(); hoisted.startGmailWatcherWithLogs.mockClear(); diff --git a/src/infra/exec-approvals-analysis.test.ts b/src/infra/exec-approvals-analysis.test.ts index e020b3870b7b..75bdf37819a0 100644 --- a/src/infra/exec-approvals-analysis.test.ts +++ b/src/infra/exec-approvals-analysis.test.ts @@ -10,7 +10,7 @@ import { resolvePlannedSegmentArgv, windowsEscapeArg, } from "./exec-approvals-analysis.js"; -import { makePathEnv, makeTempDir } from "./exec-approvals-test-helpers.js"; +import { makeExecutable, makePathEnv, makeTempDir } from "./exec-approvals-test-helpers.js"; describe("exec argv analysis", () => { it("parses argv commands", () => { @@ -188,9 +188,15 @@ describe("Windows shell analysis", () => { describe("Windows enforced shell command rendering", () => { it("builds enforced command for simple Windows command", () => { + // Resolve from a fixture PATH: host python3 locations (e.g. Homebrew's + // python@3.x Cellar path) contain characters that trigger quoting and make + // the rendered command host-dependent. + const dir = makeTempDir(); + const python3 = makeExecutable(dir, "python3"); const analysis = analyzeWindowsShellCommand({ command: "python3 a.py", platform: "win32", + env: makePathEnv(dir), }); expect(analysis.ok).toBe(true); @@ -201,7 +207,7 @@ describe("Windows enforced shell command rendering", () => { }); expect(result.ok).toBe(true); - expect(result.command).toMatch(/^& .+python3(?:\.\d+)? a\.py$/); + expect(result.command).toBe(`& ${python3} a.py`); }); it("rejects Windows commands with unsafe tokens", () => { diff --git a/src/secrets/resolve.test.ts b/src/secrets/resolve.test.ts index 902176a34b7b..45369a479e2f 100644 --- a/src/secrets/resolve.test.ts +++ b/src/secrets/resolve.test.ts @@ -272,13 +272,16 @@ describe("secret ref resolver", () => { await expect( resolveExecSecret(scriptPath, { env: { NODE_BINARY: process.execPath, PID_FILE: pidPath }, - noOutputTimeoutMs: 150, - timeoutMs: 2000, + // The first no-output window must absorb shell spawn latency under + // parallel-suite load; the script's readiness byte then pins the + // killing silence window after the pid write. + noOutputTimeoutMs: 1000, + timeoutMs: 10_000, }), ).rejects.toThrow('Exec provider "execmain" produced no output'); childPid = await readPidFile(pidPath); - expect(await waitForPidToExit(childPid)).toBe(true); + expect(await waitForPidToExit(childPid, 5_000)).toBe(true); } finally { killPidIfAlive(childPid); } diff --git a/src/security/install-policy.test.ts b/src/security/install-policy.test.ts index 944d82b608c4..05ee935341e4 100644 --- a/src/security/install-policy.test.ts +++ b/src/security/install-policy.test.ts @@ -235,8 +235,11 @@ describe("runInstallPolicy", () => { command: forkScriptPath, env: { NODE_BINARY: process.execPath, PID_FILE: pidPath }, allowInsecurePath: true, - noOutputTimeoutMs: 150, - timeoutMs: 2000, + // The first no-output window must absorb shell spawn latency + // under parallel-suite load; the script's readiness byte then + // pins the killing silence window after the pid write. + noOutputTimeoutMs: 1000, + timeoutMs: 10_000, }, }, }, @@ -246,7 +249,7 @@ describe("runInstallPolicy", () => { expect(result?.blocked?.reason).toContain("policy command produced no output"); childPid = await readPidFile(pidPath); - expect(await waitForPidToExit(childPid)).toBe(true); + expect(await waitForPidToExit(childPid, 5_000)).toBe(true); } finally { killPidIfAlive(childPid); } diff --git a/src/test-helpers/temp-dir.ts b/src/test-helpers/temp-dir.ts index 17669c5b58d0..fdd11ea3fe94 100644 --- a/src/test-helpers/temp-dir.ts +++ b/src/test-helpers/temp-dir.ts @@ -6,6 +6,9 @@ import path from "node:path"; // Temp-dir helpers share one mkdtemp root per suite prefix and hand out numbered // case dirs. That reduces filesystem churn while preserving per-test cleanup. +// Roots are canonicalized (realpath) because macOS tmpdir sits behind a symlink +// (/var -> /private/var) while production code realpaths state/session paths; +// symlinked roots break tests that compare or intercept fs paths by equality. type PrefixRootState = { path: string; activeCount: number; @@ -41,6 +44,7 @@ async function acquireAsyncPrefixRoot(options: { } const create = fs .mkdtemp(path.join(options.parentDir ?? os.tmpdir(), options.prefix)) + .then((root) => fs.realpath(root)) .then((root) => ({ path: root, activeCount: 0 })); pendingAsyncPrefixRoots.set(key, create); try { @@ -60,7 +64,9 @@ function acquireSyncPrefixRoot(options: { prefix: string; parentDir?: string }): cached.activeCount += 1; return cached; } - const root = fsSync.mkdtempSync(path.join(options.parentDir ?? os.tmpdir(), options.prefix)); + const root = fsSync.realpathSync( + fsSync.mkdtempSync(path.join(options.parentDir ?? os.tmpdir(), options.prefix)), + ); const state = { path: root, activeCount: 1 }; syncPrefixRoots.set(key, state); return state; @@ -144,7 +150,9 @@ export function createSuiteTempRootTracker(options: { prefix: string; parentDir? return { async setup(): Promise { - root = await fs.mkdtemp(path.join(options.parentDir ?? os.tmpdir(), options.prefix)); + root = await fs.realpath( + await fs.mkdtemp(path.join(options.parentDir ?? os.tmpdir(), options.prefix)), + ); nextIndex = 0; return root; }, diff --git a/src/test-utils/fixture-suite.ts b/src/test-utils/fixture-suite.ts index fd46e168e287..6cf5544ba2c4 100644 --- a/src/test-utils/fixture-suite.ts +++ b/src/test-utils/fixture-suite.ts @@ -10,7 +10,10 @@ export function createFixtureSuite(rootPrefix: string) { return { async setup(): Promise { - fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), rootPrefix)); + // Canonicalize: macOS tmpdir sits behind a symlink (/var -> /private/var) + // and production realpaths state/session paths, so symlinked roots break + // path-equality assertions. + fixtureRoot = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), rootPrefix))); }, async cleanup(): Promise { if (!fixtureRoot) { diff --git a/src/test-utils/openclaw-test-state.ts b/src/test-utils/openclaw-test-state.ts index 2c0464ef0807..9899acd0d710 100644 --- a/src/test-utils/openclaw-test-state.ts +++ b/src/test-utils/openclaw-test-state.ts @@ -269,7 +269,10 @@ export async function createOpenClawTestState( ): Promise { const label = normalizeLabel(options.label ?? options.scenario); const prefix = options.prefix ?? `${DEFAULT_PREFIX}${label}-`; - const root = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + // Canonicalize: macOS tmpdir sits behind a symlink (/var -> /private/var) and + // production code realpaths state paths, so symlinked roots break tests that + // intercept or compare fs paths by equality. + const root = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), prefix))); const layout = options.layout ?? "home"; const paths = resolveLayout(root, layout); diff --git a/src/test-utils/process-tree.ts b/src/test-utils/process-tree.ts index cda36e3a4c2b..8330e90bd57f 100644 --- a/src/test-utils/process-tree.ts +++ b/src/test-utils/process-tree.ts @@ -3,12 +3,17 @@ import path from "node:path"; export async function writeForkingNoOutputScript(dir: string): Promise { const scriptPath = path.join(dir, "fork-no-output.sh"); + // The readiness byte on stderr re-arms the caller's rolling no-output timer, + // so the silence window that kills the tree starts only after the forked pid + // is on disk; without it, slow spawns under suite load race the first window + // and the test reads a missing/empty pid file. await fs.writeFile( scriptPath, [ "#!/bin/sh", '"$NODE_BINARY" -e "setInterval(() => {}, 1000)" &', 'printf "%s" "$!" > "$PID_FILE"', + "echo ready >&2", "sleep 30", ].join("\n"), "utf8", diff --git a/test/scripts/crabbox-wrapper.test.ts b/test/scripts/crabbox-wrapper.test.ts index 426aaaa0f066..1108a6d8da15 100644 --- a/test/scripts/crabbox-wrapper.test.ts +++ b/test/scripts/crabbox-wrapper.test.ts @@ -116,7 +116,11 @@ function writeFakeCrabbox(binDir: string, helpText: string): string { ' deleted_cwd="$PWD"', " cd / || exit 1", ' rm -rf "$deleted_cwd"', - " deadline=100", + // Fail-safe only: the wrapper normally kills this child mid-loop. The + // deadline just has to outlast wrapper timer starvation under + // parallel-suite load, or the child exits 66 before the wrapper reacts + // and the test asserts on the wrong stderr message. + " deadline=1000", ' while [ "$deadline" -gt 0 ] && [ ! -d "$deleted_cwd" ]; do', " deadline=$((deadline - 1))", " sleep 0.01", diff --git a/test/scripts/dev-tooling-safety.test.ts b/test/scripts/dev-tooling-safety.test.ts index 69b5ddec77a4..31c8657fcec7 100644 --- a/test/scripts/dev-tooling-safety.test.ts +++ b/test/scripts/dev-tooling-safety.test.ts @@ -1,7 +1,7 @@ // Dev Tooling Safety tests cover dev tooling safety script behavior. import { spawn, spawnSync } from "node:child_process"; import { EventEmitter } from "node:events"; -import { existsSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -42,6 +42,23 @@ async function waitForCondition(predicate: () => boolean, timeoutMs = 5_000): Pr throw new Error("timed out waiting for condition"); } +// writeFileSync is not atomic for concurrent readers: the pid file can exist +// before its payload is flushed, so wait for non-empty content or the parse +// races into NaN under parallel-suite load. Generous budget: probe children +// boot node + tsx before the descendant pid lands. +async function waitForPidFile(pidPath: string, timeoutMs = 15_000): Promise { + let content = ""; + await waitForCondition(() => { + try { + content = readFileSync(pidPath, "utf8").trim(); + } catch { + return false; + } + return content.length > 0; + }, timeoutMs); + return Number.parseInt(content, 10); +} + function isProcessAlive(pid: number): boolean { try { process.kill(pid, 0); @@ -191,9 +208,7 @@ describe("script-specific dev tooling hardening", () => { "--channel requires a value", ); for (const flag of ["--channel", "--token", "--timeout-ms", "--state-dir"]) { - expect(() => discordSmokeTesting.parseArgs([flag, "-h"])).toThrow( - `${flag} requires a value`, - ); + expect(() => discordSmokeTesting.parseArgs([flag, "-h"])).toThrow(`${flag} requires a value`); } }); @@ -767,8 +782,7 @@ describe("script-specific dev tooling hardening", () => { ); try { - await waitForCondition(() => existsSync(descendantPidPath)); - descendantPid = Number.parseInt(await fs.readFile(descendantPidPath, "utf8"), 10); + descendantPid = await waitForPidFile(descendantPidPath); expect(Number.isInteger(descendantPid)).toBe(true); expect(isProcessAlive(descendantPid)).toBe(true); @@ -810,8 +824,7 @@ describe("script-specific dev tooling hardening", () => { ); try { - await waitForCondition(() => existsSync(descendantPidPath)); - descendantPid = Number.parseInt(await fs.readFile(descendantPidPath, "utf8"), 10); + descendantPid = await waitForPidFile(descendantPidPath); expect(Number.isInteger(descendantPid)).toBe(true); expect(isProcessAlive(descendantPid)).toBe(true); @@ -927,8 +940,8 @@ describe("script-specific dev tooling hardening", () => { let closeCalls = 0; try { - await waitForCondition(() => isProcessAlive(child.pid!) && existsSync(descendantPidPath)); - descendantPid = Number.parseInt(await fs.readFile(descendantPidPath, "utf8"), 10); + await waitForCondition(() => isProcessAlive(child.pid!)); + descendantPid = await waitForPidFile(descendantPidPath); expect(Number.isInteger(descendantPid)).toBe(true); expect(isProcessAlive(descendantPid)).toBe(true); @@ -1007,8 +1020,8 @@ describe("script-specific dev tooling hardening", () => { }); try { - await waitForCondition(() => existsSync(readyPath) && existsSync(descendantPidPath)); - descendantPid = Number.parseInt(await fs.readFile(descendantPidPath, "utf8"), 10); + await waitForCondition(() => existsSync(readyPath)); + descendantPid = await waitForPidFile(descendantPidPath); expect(Number.isInteger(descendantPid)).toBe(true); expect(isProcessAlive(descendantPid)).toBe(true); diff --git a/test/scripts/prepare-extension-package-boundary-artifacts.test.ts b/test/scripts/prepare-extension-package-boundary-artifacts.test.ts index 145a2d7dde6f..012f0a120f6c 100644 --- a/test/scripts/prepare-extension-package-boundary-artifacts.test.ts +++ b/test/scripts/prepare-extension-package-boundary-artifacts.test.ts @@ -44,11 +44,19 @@ afterEach(() => { tempRoots.clear(); }); -async function waitForFile(filePath: string, timeoutMs: number) { +async function waitForFile(filePath: string, timeoutMs: number): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { - if (fs.existsSync(filePath)) { - return; + try { + // writeFileSync is not atomic for concurrent readers: the path can exist + // before the payload is flushed. Wait for non-empty content, or pid + // parsing races into NaN under parallel-suite load. + const content = fs.readFileSync(filePath, "utf8").trim(); + if (content) { + return content; + } + } catch { + // Not created yet. } await delay(25); } @@ -219,12 +227,21 @@ describe("prepare-extension-package-boundary-artifacts", () => { "setInterval(() => {}, 1000);", ].join("\n"); + // Fail the sibling only once the descendant reported its pid so the + // group abort cannot race the descendant's boot under suite load. + const failWhenDescendantReady = [ + "const fs = require('node:fs');", + "setInterval(() => {", + ` try { if (fs.readFileSync(${JSON.stringify(descendantPidPath)}, 'utf8').trim()) { process.exit(2); } } catch {}`, + "}, 25);", + ].join("\n"); + try { const command = runNodeStepsInParallel([ { label: "delayed-fail", - args: ["--eval", "setTimeout(() => process.exit(2), 150)"], - timeoutMs: 5_000, + args: ["--eval", failWhenDescendantReady], + timeoutMs: 30_000, }, { label: "abort-group-prep", @@ -235,8 +252,7 @@ describe("prepare-extension-package-boundary-artifacts", () => { const expectedFailure = expect(command).rejects.toThrow( "delayed-fail failed with exit code 2", ); - await waitForFile(descendantPidPath, 1_000); - descendantPid = Number.parseInt(fs.readFileSync(descendantPidPath, "utf8"), 10); + descendantPid = Number.parseInt(await waitForFile(descendantPidPath, 10_000), 10); await expectedFailure; await waitForDead(descendantPid, 2_000); @@ -272,11 +288,19 @@ describe("prepare-extension-package-boundary-artifacts", () => { "setInterval(() => {}, 1000);", ].join("\n"); + // Fail the sibling only once the descendant installed its SIGTERM trap + // (signalled via readyPath) so the group abort cannot race its boot. + const failWhenDescendantReady = [ + "const fs = require('node:fs');", + "setInterval(() => {", + ` try { if (fs.readFileSync(${JSON.stringify(readyPath)}, 'utf8').trim()) { process.exit(2); } } catch {}`, + "}, 25);", + ].join("\n"); const command = runNodeStepsInParallel([ { label: "delayed-fail", - args: ["--eval", "setTimeout(() => process.exit(2), 150)"], - timeoutMs: 5_000, + args: ["--eval", failWhenDescendantReady], + timeoutMs: 30_000, }, { label: "abort-group-drain", @@ -285,9 +309,9 @@ describe("prepare-extension-package-boundary-artifacts", () => { }, ]); - await waitForFile(readyPath, 1_000); + await waitForFile(readyPath, 10_000); await expect(command).rejects.toThrow("delayed-fail failed with exit code 2"); - expect(fs.readFileSync(drainedPath, "utf8")).toBe("drained"); + expect(await waitForFile(drainedPath, 10_000)).toBe("drained"); }, ); @@ -346,12 +370,14 @@ describe("prepare-extension-package-boundary-artifacts", () => { ].join("\n"); try { - const command = runNodeStep("hung-group-prep", ["--eval", parentScript], 750); + // The timeout clock starts at spawn, so it must leave room for two Node + // boots (parent + descendant) under parallel-suite load; otherwise the + // group is killed before the descendant ever writes its pid. + const command = runNodeStep("hung-group-prep", ["--eval", parentScript], 5_000); const expectedFailure = expect(command).rejects.toThrow( - "hung-group-prep timed out after 750ms", + "hung-group-prep timed out after 5000ms", ); - await waitForFile(descendantPidPath, 500); - descendantPid = Number.parseInt(fs.readFileSync(descendantPidPath, "utf8"), 10); + descendantPid = Number.parseInt(await waitForFile(descendantPidPath, 4_000), 10); await expectedFailure; await waitForDead(descendantPid, 2_000); @@ -395,9 +421,8 @@ describe("prepare-extension-package-boundary-artifacts", () => { runnerPid = runner.pid ?? 0; try { - await waitForFile(descendantPidPath, 2_000); - descendantPid = Number.parseInt(fs.readFileSync(descendantPidPath, "utf8"), 10); - const runnerExit = waitForProcessExit(runner, 2_000); + descendantPid = Number.parseInt(await waitForFile(descendantPidPath, 10_000), 10); + const runnerExit = waitForProcessExit(runner, 10_000); runner.kill("SIGTERM"); expect(await runnerExit).toEqual({ code: 143, signal: null }); diff --git a/test/scripts/secret-provider-integrations.test.ts b/test/scripts/secret-provider-integrations.test.ts index 23df7bed79ca..a3bf32bed0e0 100644 --- a/test/scripts/secret-provider-integrations.test.ts +++ b/test/scripts/secret-provider-integrations.test.ts @@ -34,6 +34,22 @@ async function waitFor(predicate: () => boolean, timeoutMs = 5_000): Promise { + let content = ""; + await waitFor(() => { + try { + content = fs.readFileSync(pidPath, "utf8").trim(); + } catch { + return false; + } + return content.length > 0; + }, timeoutMs); + return Number.parseInt(content, 10); +} + async function waitForChildClose(child: ReturnType, timeoutMs = 5_000) { return await new Promise<{ code: number | null; signal: NodeJS.Signals | null }>( (resolve, reject) => { @@ -481,8 +497,8 @@ describe("secret provider integration proof harness", () => { { timeoutKillGraceMs: 50, timeoutMs: 2_000 }, ); result.catch(() => {}); - await waitFor(() => fs.existsSync(readyPath) && fs.existsSync(descendantPidPath)); - descendantPid = Number.parseInt(fs.readFileSync(descendantPidPath, "utf8"), 10); + await waitFor(() => fs.existsSync(readyPath)); + descendantPid = await waitForPidFile(descendantPidPath); expect(Number.isInteger(descendantPid)).toBe(true); expect(isProcessAlive(descendantPid)).toBe(true); @@ -792,8 +808,7 @@ describe("secret provider integration proof harness", () => { timeoutMs: 150, }); - await waitFor(() => fs.existsSync(descendantPidPath)); - descendantPid = Number.parseInt(fs.readFileSync(descendantPidPath, "utf8"), 10); + descendantPid = await waitForPidFile(descendantPidPath); expect(Number.isInteger(descendantPid)).toBe(true); expect(isProcessAlive(descendantPid)).toBe(true); @@ -838,8 +853,7 @@ describe("secret provider integration proof harness", () => { }); try { - await waitFor(() => fs.existsSync(descendantPidPath)); - descendantPid = Number.parseInt(fs.readFileSync(descendantPidPath, "utf8"), 10); + descendantPid = await waitForPidFile(descendantPidPath); expect(Number.isInteger(descendantPid)).toBe(true); expect(isProcessAlive(descendantPid)).toBe(true); @@ -931,8 +945,8 @@ describe("secret provider integration proof harness", () => { cwd: process.cwd(), stdio: ["ignore", "ignore", "pipe"], }); - await waitFor(() => fs.existsSync(readyPath) && fs.existsSync(descendantPidPath)); - descendantPid = Number.parseInt(fs.readFileSync(descendantPidPath, "utf8"), 10); + await waitFor(() => fs.existsSync(readyPath)); + descendantPid = await waitForPidFile(descendantPidPath); expect(Number.isInteger(descendantPid)).toBe(true); expect(isProcessAlive(descendantPid)).toBe(true);