From 7c2302ec5b9ddd7711add2f94ee2d28c116c15a7 Mon Sep 17 00:00:00 2001 From: Oliver Mee <102673257+oliver-mee@users.noreply.github.com> Date: Wed, 21 Oct 2026 00:15:00 +0800 Subject: [PATCH] test: bound shell integration scenarios independently Split CI aggregate matrix cases and SSH discovery scenarios into bounded asynchronous child invocations. Preserve gate-policy and argv assertions while allowing real candidate probes to finish and validating process cleanup. Co-Authored-By: Claude Opus 5.5 --- tests/ci-workflows/ci-review-lanes.test.ts | 92 ++++++++++--------- tests/clients/link-ssh-argv-discovery.test.ts | 70 ++++++++------ 2 files changed, 91 insertions(+), 71 deletions(-) diff --git a/tests/ci-workflows/ci-review-lanes.test.ts b/tests/ci-workflows/ci-review-lanes.test.ts index 158ff0c7de6..99d0aaf7164 100644 --- a/tests/ci-workflows/ci-review-lanes.test.ts +++ b/tests/ci-workflows/ci-review-lanes.test.ts @@ -4,6 +4,8 @@ import { mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSyn import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { repoPath } from "../helpers/repo-root"; +import { spawnTestChild } from "../helpers/spawn-test-child"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; // Review-lane contracts for ci.yml and enforce-pr-target.yml. They live beside // ci-workflows.test.ts because that file sits at its file-size cap. @@ -245,8 +247,8 @@ describe("CI review lanes", () => { }); // The aggregate step runs under bash with jq, as ci-scope-reduction.test.ts guards. - test.skipIf(process.platform === "win32" || !Bun.which("jq"))("release-gates aggregate accepts diagnostic skips but rejects producer failures", async () => { - const ci = Bun.YAML.parse(await readText(".github/workflows/ci.yml")) as { + describe.skipIf(process.platform === "win32" || !Bun.which("jq"))("release-gates aggregate", () => { + const ci = Bun.YAML.parse(readFileSync(repoPath(".github/workflows/ci.yml"), "utf8")) as { jobs: Record; run?: string }[]; @@ -254,10 +256,12 @@ describe("CI review lanes", () => { }; const aggregate = ci.jobs.ci!; const step = aggregate.steps?.find(step => step.name === "Assert every job this event requested succeeded"); - expect(step?.shell).toBe("bash"); - expect(step?.env?.RESULTS).toBe("${{ toJSON(needs) }}"); - expect(step?.run).toBeDefined(); - expect(step?.run).not.toContain("${{"); + test("runs the checked-in Bash gate", () => { + expect(step?.shell).toBe("bash"); + expect(step?.env?.RESULTS).toBe("${{ toJSON(needs) }}"); + expect(step?.run).toBeDefined(); + expect(step?.run).not.toContain("${{"); + }); const results: Record = Object.fromEntries( (aggregate.needs ?? []).map(job => [job, { result: "success" }]), ); @@ -268,48 +272,52 @@ describe("CI review lanes", () => { results["privacy-gate"] = { result: "skipped" }; results["setup-action"] = { result: "skipped" }; results["remote-helper"] = { result: "skipped" }; - const run = (value: typeof results, packaging = "true") => spawnSync("bash", ["-c", step!.run!], { - encoding: "utf8", - env: { - ...process.env, RESULTS: JSON.stringify(value), - EVENT_NAME: "workflow_dispatch", LANE: "release-gates", - CHANGES_CI: "true", CHANGES_NATIVE: "true", CHANGES_PACKAGING: packaging, - CHANGES_DOCS: "false", CHANGES_STRUCTURE: "false", - CHANGES_SETUP_ACTION: "false", CHANGES_REMOTE_HELPER: "false", - }, - timeout: 5_000, - }); - for (const packaging of ["success", "skipped"]) { - const result = run({ ...results, "npm-global-smoke": { result: packaging } }, packaging === "success" ? "true" : "false"); - expect(result.error).toBeUndefined(); - expect(result.status).toBe(0); - } + const run = async (value: typeof results, packaging = "true") => { + const result = await spawnTestChild(["bash", "-c", step!.run!], { + cwd: repoPath(), + env: { + ...process.env, RESULTS: JSON.stringify(value), + EVENT_NAME: "workflow_dispatch", LANE: "release-gates", + CHANGES_CI: "true", CHANGES_NATIVE: "true", CHANGES_PACKAGING: packaging, + CHANGES_DOCS: "false", CHANGES_STRUCTURE: "false", + CHANGES_SETUP_ACTION: "false", CHANGES_REMOTE_HELPER: "false", + }, + deadlineMs: 5_000, + }); + expect(result.timedOut).toBe(false); + expect(result.reaped).toBe(true); + expect(result.pipesClosed).toBe(true); + expect(result.signal).toBeNull(); + return result; + }; + // Each shell invocation owns its deadline instead of sharing one matrix-wide budget. + test.each(["success", "skipped"])("accepts packaging %s with diagnostic skips", async packaging => { + const result = await run({ ...results, "npm-global-smoke": { result: packaging } }, packaging === "success" ? "true" : "false"); + expect(result.exitCode).toBe(0); + }, SPAWN_BUDGET_MS); // Execute the checked-in Bash/jq gate, not a duplicate JS allowlist. The // selector must remain visible even though its Windows consumer is skipped. - for (const producer of Object.keys(results)) { - for (const status of ["failure", "cancelled"]) { - const result = run({ ...results, [producer]: { result: status } }); - expect(result.error).toBeUndefined(); - expect(result.status).toBe(1); - expect(result.stdout).toContain(producer); - expect(result.stdout).toContain(status); - } - } + const failures = Object.keys(results).flatMap(producer => + ["failure", "cancelled"].map(status => [producer, status] as const)); + test.each(failures)("rejects %s result %s", async (producer, status) => { + const result = await run({ ...results, [producer]: { result: status } }); + expect(result.exitCode).toBe(1); + expect(result.stdout).toContain(producer); + expect(result.stdout).toContain(status); + }, SPAWN_BUDGET_MS); // A requested producer cannot disappear behind the intentional diagnostic skips. - for (const producer of Object.keys(results).filter(job => results[job]!.result === "success")) { - const result = run({ ...results, [producer]: { result: "skipped" } }); - expect(result.error).toBeUndefined(); - expect(result.status).toBe(1); + test.each(Object.keys(results).filter(job => results[job]!.result === "success"))("rejects skipped requested producer %s", async producer => { + const result = await run({ ...results, [producer]: { result: "skipped" } }); + expect(result.exitCode).toBe(1); expect(result.stdout).toContain(producer); - } - for (const status of ["timed_out", "unexpected-status"]) { - const result = run({ ...results, "select-windows-runner": { result: status } }); - expect(result.error).toBeUndefined(); - expect(result.status).toBe(1); + }, SPAWN_BUDGET_MS); + test.each(["timed_out", "unexpected-status"])("rejects selector result %s", async status => { + const result = await run({ ...results, "select-windows-runner": { result: status } }); + expect(result.exitCode).toBe(1); expect(result.stdout).toContain("select-windows-runner"); expect(result.stdout).toContain(status); - } - }, 30_000); + }, SPAWN_BUDGET_MS); + }); }); describe("Windows and nightly CI contracts", () => { diff --git a/tests/clients/link-ssh-argv-discovery.test.ts b/tests/clients/link-ssh-argv-discovery.test.ts index 86f97f65dba..767663cc497 100644 --- a/tests/clients/link-ssh-argv-discovery.test.ts +++ b/tests/clients/link-ssh-argv-discovery.test.ts @@ -4,9 +4,15 @@ import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { quoteRemote, REMOTE_OCX_SCRIPT, remoteOcxArgv } from "../../src/link/ssh-argv"; import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { spawnTestChild } from "../helpers/spawn-test-child"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; const roots: string[] = []; -afterEach(() => { for (const root of roots.splice(0)) removeTreeWithRetry(root); }); +// Keep any fixture whose child or inherited output pipes have not settled. +const activeRoots = new Set(); +afterEach(() => { + for (const root of roots.splice(0)) if (!activeRoots.has(root)) removeTreeWithRetry(root); +}); function executable(path: string, body: string): void { mkdirSync(dirname(path), { recursive: true }); @@ -36,12 +42,22 @@ function fixture(platform = "Darwin") { .replaceAll("/usr/local/bin", join(home, "local/bin")); return { home, paths, - run(args = ["link", "port"]) { + async run(args = ["link", "port"]) { const argv = remoteOcxArgv(args); argv[2] = script; - return Bun.spawnSync(["/bin/sh", "-c", quoteRemote(argv)], { + // Real candidate probes need a live parent loop and a separate reap budget. + activeRoots.add(home); + const result = await spawnTestChild(["/bin/sh", "-c", quoteRemote(argv)], { + cwd: home, env: { HOME: home, PATH: `${home}/remote-bin:${home}/second-bin` }, + deadlineMs: 15_000, }); + if (result.reaped && result.pipesClosed) activeRoots.delete(home); + expect(result.timedOut).toBe(false); + expect(result.reaped).toBe(true); + expect(result.pipesClosed).toBe(true); + expect(result.signal).toBeNull(); + return result; }, install(index: number, fails = false) { executable(paths[index]!, fails @@ -51,44 +67,40 @@ function fixture(platform = "Darwin") { }; } -test.skipIf(process.platform === "win32").each([0, 1, 2, 3, 4, 5, 6, 7, 8])("remote sh chooses candidate %i in Desktop/system/PATH order, skipping failed versions", winner => { +test.skipIf(process.platform === "win32").each([0, 1, 2, 3, 4, 5, 6, 7, 8])("remote sh chooses candidate %i in Desktop/system/PATH order, skipping failed versions", async winner => { const f = fixture(winner % 2 ? "Linux" : "Darwin"); for (let index = 0; index < 9; index++) f.install(index, index < winner); const args = ["link", "issue", "--alias", "it's x", "two\nlines", "火🔥", "x;$(echo no)", "", "--json"]; - const result = f.run(args); + const result = await f.run(args); expect(result.exitCode).toBe(0); - expect(result.stdout).toEqual(new TextEncoder().encode(`${winner}\n${args.join("\0")}\0`)); - expect(result.stderr.toString()).toBe(""); -}); + expect(result.stdout).toBe(`${winner}\n${args.join("\0")}\0`); + expect(result.stderr).toBe(""); +}, SPAWN_BUDGET_MS); -test.skipIf(process.platform === "win32")("remote sh skips absent and non-executable Desktop candidates", () => { +test.skipIf(process.platform === "win32")("remote sh skips absent and non-executable Desktop candidates", async () => { const f = fixture(); mkdirSync(dirname(f.paths[0]!), { recursive: true }); writeFileSync(f.paths[0]!, "unusable", { mode: 0o644 }); f.install(2); - const result = f.run(); + const result = await f.run(); expect(result.exitCode).toBe(0); - expect(result.stdout.toString()).toBe("2\nlink\0port\0"); -}); + expect(result.stdout).toBe("2\nlink\0port\0"); +}, SPAWN_BUDGET_MS); -test.skipIf(process.platform === "win32")("remote sh reports missing when all candidates are absent or fail --version", () => { - for (const broken of [false, true]) { - const f = fixture(); - if (broken) for (let index = 0; index < 9; index++) f.install(index, true); - const result = f.run(); - expect(result.exitCode).toBe(127); - expect(result.stdout.toString()).toBe(""); - expect(result.stderr.toString()).toBe(""); - } -}); +test.skipIf(process.platform === "win32").each([["absent", false], ["broken", true]] as const)("remote sh reports missing when candidates are %s", async (_label, broken) => { + const f = fixture(); + if (broken) for (let index = 0; index < 9; index++) f.install(index, true); + const result = await f.run(); + expect(result.exitCode).toBe(127); + expect(result.stdout).toBe(""); + expect(result.stderr).toBe(""); +}, SPAWN_BUDGET_MS); -test.skipIf(process.platform === "win32")("remote sh refuses Windows-like platforms before invoking any ocx candidate", () => { - for (const platform of ["Windows_NT", "MSYS_NT-10.0-22631", "MINGW64_NT-10.0", "MINGW32_NT-6.1", "CYGWIN_NT-10.0"]) { +test.skipIf(process.platform === "win32").each(["Windows_NT", "MSYS_NT-10.0-22631", "MINGW64_NT-10.0", "MINGW32_NT-6.1", "CYGWIN_NT-10.0"])("remote sh refuses %s before invoking any ocx candidate", async platform => { const f = fixture(platform); for (const path of f.paths) executable(path, "printf 'ocx must not run\\n' >&2; exit 1"); - const result = f.run(["--version"]); + const result = await f.run(["--version"]); expect(result.exitCode).toBe(125); - expect(result.stdout.toString()).toBe(""); - expect(result.stderr.toString()).toBe(""); - } -}); + expect(result.stdout).toBe(""); + expect(result.stderr).toBe(""); +}, SPAWN_BUDGET_MS);