From c11ff358410186ef3a0e46459a6e14594fac8ccc Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 1 Jun 2026 08:37:46 +0200 Subject: [PATCH] fix(e2e): bound Parallels update logs --- scripts/e2e/parallels/npm-update-smoke.ts | 58 ++++++++++++++++--- scripts/e2e/parallels/update-job-timeout.ts | 8 ++- .../parallels-npm-update-smoke.test.ts | 12 ++++ .../parallels-update-job-timeout.test.ts | 32 ++++++++++ 4 files changed, 98 insertions(+), 12 deletions(-) diff --git a/scripts/e2e/parallels/npm-update-smoke.ts b/scripts/e2e/parallels/npm-update-smoke.ts index c79398d23d3e..e63a6821a95c 100755 --- a/scripts/e2e/parallels/npm-update-smoke.ts +++ b/scripts/e2e/parallels/npm-update-smoke.ts @@ -1,6 +1,6 @@ #!/usr/bin/env -S pnpm tsx import { spawn } from "node:child_process"; -import { readFileSync } from "node:fs"; +import { appendFileSync, readFileSync, writeFileSync } from "node:fs"; import { readFile, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import { pathToFileURL } from "node:url"; @@ -64,6 +64,7 @@ interface Job { interface UpdateJobContext { append(chunk: string | Uint8Array): void; logPath: string; + signal: AbortSignal; } interface NpmUpdateSummary { @@ -575,19 +576,19 @@ class NpmUpdateSmoke { startedAt, }; job.promise = (async () => { - let log = ""; + writeFileSync(logPath, "", "utf8"); const append = (chunk: string | Uint8Array): void => { const text = typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"); - log += text; + appendFileSync(logPath, text, "utf8"); this.noteJobOutput(job, text); }; return await runTimedUpdateJob({ append, label, - run: () => fn({ append, logPath }), + run: ({ signal }) => fn({ append, logPath, signal }), timeoutDescription: `${updateTimeoutSeconds}s plus cleanup backstop`, timeoutMs: updateTimeoutSeconds * 1000 + updateCleanupBackstopMs, - writeLog: () => writeFile(logPath, log, "utf8"), + writeLog: async () => undefined, }); })().finally(() => { job.durationMs = Date.now() - job.startedAt; @@ -898,6 +899,7 @@ class NpmUpdateSmoke { return await new Promise((resolve, reject) => { const child = spawn(command, args, { cwd: repoRoot, + detached: process.platform !== "win32", env: process.env, stdio: ["ignore", "pipe", "pipe"], }); @@ -906,16 +908,54 @@ class NpmUpdateSmoke { child.stderr.on("data", (chunk: Buffer) => ctx.append(chunk)); let timedOut = false; - const timer = setTimeout(() => { + let killTimer: NodeJS.Timeout | undefined; + const signalChild = (signal: NodeJS.Signals): void => { + if (!child.pid) { + return; + } + try { + if (process.platform === "win32") { + child.kill(signal); + } else { + process.kill(-child.pid, signal); + } + } catch { + child.kill(signal); + } + }; + const abort = (): void => { + if (timedOut) { + return; + } timedOut = true; - child.kill("SIGTERM"); - setTimeout(() => child.kill("SIGKILL"), 2_000).unref(); + signalChild("SIGTERM"); + killTimer = setTimeout(() => signalChild("SIGKILL"), 2_000); + killTimer.unref(); + }; + if (ctx.signal.aborted) { + abort(); + } else { + ctx.signal.addEventListener("abort", abort, { once: true }); + } + const timer = setTimeout(() => { + abort(); }, timeoutMs); - child.on("error", reject); + child.on("error", (error) => { + ctx.signal.removeEventListener("abort", abort); + if (killTimer) { + clearTimeout(killTimer); + } + reject(error); + }); child.on("close", (code, signal) => { + ctx.signal.removeEventListener("abort", abort); clearTimeout(timer); + if (killTimer) { + clearTimeout(killTimer); + } if (timedOut) { + signalChild("SIGKILL"); resolve(124); return; } diff --git a/scripts/e2e/parallels/update-job-timeout.ts b/scripts/e2e/parallels/update-job-timeout.ts index a963adeacb0f..69c000eae557 100644 --- a/scripts/e2e/parallels/update-job-timeout.ts +++ b/scripts/e2e/parallels/update-job-timeout.ts @@ -1,7 +1,7 @@ interface TimedUpdateJobOptions { append(this: void, chunk: string): void; label: string; - run(this: void): Promise | void; + run(this: void, context: { signal: AbortSignal }): Promise | void; timeoutDescription: string; timeoutMs: number; writeLog(this: void): Promise; @@ -16,20 +16,22 @@ export async function runTimedUpdateJob({ writeLog, }: TimedUpdateJobOptions): Promise { let timedOut = false; + const controller = new AbortController(); const timeoutMessage = `${label} update timed out after ${timeoutDescription}`; let timeout: NodeJS.Timeout | undefined; const timeoutPromise = new Promise((_, reject) => { timeout = setTimeout(() => { timedOut = true; append(`${timeoutMessage}\n`); + controller.abort(new Error(timeoutMessage)); reject(new Error(timeoutMessage)); }, timeoutMs); }); try { - await Promise.race([Promise.resolve(run()), timeoutPromise]); + await Promise.race([Promise.resolve(run({ signal: controller.signal })), timeoutPromise]); await writeLog(); - return 0; + return timedOut ? 1 : 0; } catch (error) { if (!timedOut) { append(`${error instanceof Error ? error.message : String(error)}\n`); diff --git a/test/scripts/parallels-npm-update-smoke.test.ts b/test/scripts/parallels-npm-update-smoke.test.ts index 38cfdbae2154..9ee06012156d 100644 --- a/test/scripts/parallels-npm-update-smoke.test.ts +++ b/test/scripts/parallels-npm-update-smoke.test.ts @@ -65,6 +65,18 @@ describe("parallels npm update smoke", () => { expect(script).toContain("Parallels NPM Update Smoke"); }); + it("streams aggregate update logs instead of retaining them in memory", () => { + const script = readFileSync(SCRIPT_PATH, "utf8"); + const updateBlock = script.slice( + script.indexOf(" private spawnUpdate"), + script.indexOf(" private async runMacosUpdate"), + ); + + expect(updateBlock).toContain("appendFileSync(logPath, text"); + expect(updateBlock).toContain("run: ({ signal }) => fn({ append, logPath, signal })"); + expect(updateBlock).not.toContain("log += text"); + }); + it("runs Windows updates through a detached done-file runner", () => { const script = readFileSync(SCRIPT_PATH, "utf8"); const transports = readFileSync(GUEST_TRANSPORTS_PATH, "utf8"); diff --git a/test/scripts/parallels-update-job-timeout.test.ts b/test/scripts/parallels-update-job-timeout.test.ts index 81744011e0bc..506b06a90680 100644 --- a/test/scripts/parallels-update-job-timeout.test.ts +++ b/test/scripts/parallels-update-job-timeout.test.ts @@ -88,4 +88,36 @@ describe("Parallels update job timeout", () => { expect(chunks).toEqual(["Windows update timed out after 1s\n"]); expect(writeLog).toHaveBeenCalledTimes(1); }); + + it("aborts the update body when the timeout fires", async () => { + vi.useFakeTimers(); + const chunks: string[] = []; + const writeLog = vi.fn(async () => undefined); + let aborted = false; + + const result = runTimedUpdateJob({ + append: (chunk) => chunks.push(chunk), + label: "Linux", + run: ({ signal }) => + new Promise((resolve) => { + signal.addEventListener( + "abort", + () => { + aborted = true; + resolve(); + }, + { once: true }, + ); + }), + timeoutDescription: "1s plus cleanup backstop", + timeoutMs: 1000, + writeLog, + }); + + await vi.advanceTimersByTimeAsync(1000); + await expect(result).resolves.toBe(1); + expect(aborted).toBe(true); + expect(chunks).toEqual(["Linux update timed out after 1s plus cleanup backstop\n"]); + expect(writeLog).toHaveBeenCalledTimes(1); + }); });