From 730c7269efe1d446230cdb254ea22b2b96915ebd Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 20 Jun 2026 15:35:28 +0200 Subject: [PATCH] fix(test): clean Vitest runner child groups on signal --- scripts/lib/vitest-batch-runner.mjs | 14 +++++++---- scripts/run-vitest.mjs | 37 +++++++++++++++++++++++++---- scripts/test-projects.mjs | 9 ++++++- scripts/vitest-process-group.mjs | 11 +++++++++ test/scripts/run-vitest.test.ts | 20 +++++++++++++++- test/scripts/test-extension.test.ts | 18 ++++++++++++++ 6 files changed, 97 insertions(+), 12 deletions(-) diff --git a/scripts/lib/vitest-batch-runner.mjs b/scripts/lib/vitest-batch-runner.mjs index 1091b14565b..8d1ade67d46 100644 --- a/scripts/lib/vitest-batch-runner.mjs +++ b/scripts/lib/vitest-batch-runner.mjs @@ -3,6 +3,7 @@ import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { spawnPnpmRunner } from "../pnpm-runner.mjs"; import { + forceKillVitestProcessGroup, installVitestProcessGroupCleanup, shouldUseDetachedVitestProcessGroup, } from "../vitest-process-group.mjs"; @@ -26,8 +27,10 @@ export async function runVitestBatch(params) { }); const teardownChildCleanup = installVitestProcessGroupCleanup({ child, + forceSignal: "SIGKILL", + forceSignalDelayMs: 100, onSignal(signal) { - forwardedSignal = signal; + forwardedSignal ??= signal; }, }); @@ -37,12 +40,13 @@ export async function runVitestBatch(params) { }); child.on("exit", (code, signal) => { teardownChildCleanup(); - if (signal) { - process.kill(process.pid, signal); + if (forwardedSignal) { + forceKillVitestProcessGroup(child); + process.kill(process.pid, forwardedSignal); return; } - if (forwardedSignal) { - process.kill(process.pid, forwardedSignal); + if (signal) { + process.kill(process.pid, signal); return; } resolve(code ?? 1); diff --git a/scripts/run-vitest.mjs b/scripts/run-vitest.mjs index b70ce7f3cc7..3868f837894 100644 --- a/scripts/run-vitest.mjs +++ b/scripts/run-vitest.mjs @@ -10,7 +10,7 @@ import { boundaryTestFiles } from "../test/vitest/vitest.unit-paths.mjs"; import { resolveLocalVitestEnv } from "./lib/vitest-local-scheduling.mjs"; import { spawnPnpmRunner } from "./pnpm-runner.mjs"; import { - forwardSignalToVitestProcessGroup, + forceKillVitestProcessGroup, installVitestProcessGroupCleanup, shouldUseDetachedVitestProcessGroup, } from "./vitest-process-group.mjs"; @@ -994,11 +994,19 @@ export function spawnWatchedVitestProcess({ label, onNoOutputTimeout, }) { + let forwardedSignal = null; const child = spawnVitestProcess({ pnpmArgs, spawnParams, }); - const teardownChildCleanup = installVitestProcessGroupCleanup({ child }); + const teardownChildCleanup = installVitestProcessGroupCleanup({ + child, + forceSignal: "SIGKILL", + forceSignalDelayMs: 100, + onSignal: (signal) => { + forwardedSignal ??= signal; + }, + }); const teardownNoOutputWatchdog = installVitestNoOutputWatchdog({ streams: [child.stdout, child.stderr], timeoutMs: resolveVitestNoOutputTimeoutMs(env), @@ -1028,6 +1036,7 @@ export function spawnWatchedVitestProcess({ return { child, + getForwardedSignal: () => forwardedSignal, teardown: () => { teardownChildCleanup(); teardownNoOutputWatchdog(); @@ -1054,13 +1063,19 @@ export function resolveTestProjectsRunnerSpawnParams(env, platform = process.pla } function spawnTestProjectsRunner(argv, env) { + let forwardedSignal = null; const child = spawn(process.execPath, [testProjectsRunnerPath, ...argv], { ...resolveTestProjectsRunnerSpawnParams(env), }); const teardown = installVitestProcessGroupCleanup({ child, + forceSignal: "SIGKILL", + forceSignalDelayMs: 100, + onSignal: (signal) => { + forwardedSignal ??= signal; + }, }); - return { child, teardown }; + return { child, getForwardedSignal: () => forwardedSignal, teardown }; } function main(argv = process.argv.slice(2), env = process.env) { @@ -1082,9 +1097,15 @@ function main(argv = process.argv.slice(2), env = process.env) { const delegatedArgs = resolveTestProjectsDelegationArgs(argv); if (delegatedArgs) { - const { child, teardown } = spawnTestProjectsRunner(delegatedArgs, env); + const { child, getForwardedSignal, teardown } = spawnTestProjectsRunner(delegatedArgs, env); child.on("exit", (code, signal) => { teardown(); + const forwardedSignal = getForwardedSignal(); + if (forwardedSignal) { + forceKillVitestProcessGroup(child); + process.kill(process.pid, forwardedSignal); + return; + } if (signal) { process.kill(process.pid, signal); return; @@ -1113,7 +1134,7 @@ function main(argv = process.argv.slice(2), env = process.env) { throw error; } - const { child, teardown } = spawnWatchedVitestProcess({ + const { child, getForwardedSignal, teardown } = spawnWatchedVitestProcess({ pnpmArgs: ["exec", "node", ...resolveVitestNodeArgs(env), vitestCliEntry, ...guardedVitestArgs], spawnParams: resolveVitestSpawnParams(spawnEnv), env: spawnEnv, @@ -1122,6 +1143,12 @@ function main(argv = process.argv.slice(2), env = process.env) { child.on("exit", (code, signal) => { teardown(); + const forwardedSignal = getForwardedSignal(); + if (forwardedSignal) { + forceKillVitestProcessGroup(child); + process.kill(process.pid, forwardedSignal); + return; + } if (signal) { process.kill(process.pid, signal); return; diff --git a/scripts/test-projects.mjs b/scripts/test-projects.mjs index fb66fb000d5..7dd7d6da80f 100644 --- a/scripts/test-projects.mjs +++ b/scripts/test-projects.mjs @@ -38,6 +38,7 @@ import { shouldRetryVitestNoOutputTimeout, writeVitestIncludeFile, } from "./test-projects.test-support.mjs"; +import { forceKillVitestProcessGroup } from "./vitest-process-group.mjs"; // Keep this shim so `pnpm test -- src/foo.test.ts` still forwards filters // cleanly instead of leaking pnpm's passthrough sentinel to Vitest. @@ -88,7 +89,7 @@ function runVitestSpec(spec) { } let noOutputTimedOut = false; return new Promise((resolve, reject) => { - const { child, teardown } = spawnWatchedVitestProcess({ + const { child, getForwardedSignal, teardown } = spawnWatchedVitestProcess({ pnpmArgs: spec.pnpmArgs, env: spec.env, label: spec.config, @@ -104,6 +105,12 @@ function runVitestSpec(spec) { child.on("exit", (code, signal) => { teardown(); cleanupVitestRunSpec(spec); + const forwardedSignal = getForwardedSignal(); + if (forwardedSignal) { + forceKillVitestProcessGroup(child); + resolve({ code: 143, noOutputTimedOut, signal: forwardedSignal }); + return; + } resolve({ code: code ?? (signal ? 143 : 1), noOutputTimedOut, signal }); }); diff --git a/scripts/vitest-process-group.mjs b/scripts/vitest-process-group.mjs index 1a8d2faa95d..57744f83170 100644 --- a/scripts/vitest-process-group.mjs +++ b/scripts/vitest-process-group.mjs @@ -41,6 +41,17 @@ export function forwardSignalToVitestProcessGroup(params) { } } +/** + * Force-cleans any remaining processes in a Vitest child process group. + */ +export function forceKillVitestProcessGroup(child, kill = process.kill.bind(process)) { + return forwardSignalToVitestProcessGroup({ + child, + kill, + signal: "SIGKILL", + }); +} + function ensureProcessListenerCapacity(processObject, eventName, additionalListeners = 1) { if ( typeof processObject.getMaxListeners !== "function" || diff --git a/test/scripts/run-vitest.test.ts b/test/scripts/run-vitest.test.ts index e154766911c..ebdf09e6d99 100644 --- a/test/scripts/run-vitest.test.ts +++ b/test/scripts/run-vitest.test.ts @@ -716,14 +716,21 @@ describe("scripts/run-vitest", () => { os.tmpdir(), `openclaw-run-vitest-delegated-child-${process.pid}-${Date.now()}.pid`, ); + const descendantPidPath = nodePath.join( + os.tmpdir(), + `openclaw-run-vitest-delegated-descendant-${process.pid}-${Date.now()}.pid`, + ); fs.writeFileSync( fixturePath, [ + 'import { spawn } from "node:child_process";', 'import fs from "node:fs";', 'import { it } from "vitest";', 'it("waits for wrapper termination", async () => {', + ' const child = spawn(process.execPath, ["-e", "process.on(\\\'SIGTERM\\\', () => {}); setInterval(() => {}, 1000);"], { stdio: "ignore" });', " fs.writeFileSync(process.env.OPENCLAW_DELEGATED_SIGNAL_CHILD_PID!, String(process.pid));", + " fs.writeFileSync(process.env.OPENCLAW_DELEGATED_SIGNAL_DESCENDANT_PID!, String(child.pid));", " await new Promise(() => {});", "});", "", @@ -737,24 +744,31 @@ describe("scripts/run-vitest", () => { env: { ...process.env, OPENCLAW_DELEGATED_SIGNAL_CHILD_PID: childPidPath, + OPENCLAW_DELEGATED_SIGNAL_DESCENDANT_PID: descendantPidPath, }, stdio: "ignore", }, ); let childPid = 0; + let descendantPid = 0; try { await waitFor(() => fs.existsSync(childPidPath), 10_000); + await waitFor(() => fs.existsSync(descendantPidPath), 10_000); childPid = Number(fs.readFileSync(childPidPath, "utf8")); + descendantPid = Number(fs.readFileSync(descendantPidPath, "utf8")); expect(Number.isInteger(childPid)).toBe(true); + expect(Number.isInteger(descendantPid)).toBe(true); expect(isProcessAlive(childPid)).toBe(true); + expect(isProcessAlive(descendantPid)).toBe(true); expect(runner.pid).toBeGreaterThan(0); process.kill(runner.pid!, "SIGTERM"); const result = await waitForClose(runner); - expect(result).toEqual({ code: 143, signal: null }); + expect(result).toEqual({ code: null, signal: "SIGTERM" }); await waitFor(() => !isProcessAlive(childPid), 5_000); + await waitFor(() => !isProcessAlive(descendantPid), 5_000); } finally { if (runner.pid && isProcessAlive(runner.pid)) { process.kill(runner.pid, "SIGKILL"); @@ -762,8 +776,12 @@ describe("scripts/run-vitest", () => { if (childPid && isProcessAlive(childPid)) { process.kill(childPid, "SIGKILL"); } + if (descendantPid && isProcessAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } fs.rmSync(fixturePath, { force: true }); fs.rmSync(childPidPath, { force: true }); + fs.rmSync(descendantPidPath, { force: true }); } }); diff --git a/test/scripts/test-extension.test.ts b/test/scripts/test-extension.test.ts index 30f389975cb..c0225b96883 100644 --- a/test/scripts/test-extension.test.ts +++ b/test/scripts/test-extension.test.ts @@ -748,6 +748,7 @@ describe("scripts/test-extension.mjs", () => { const root = mkdtempSync(path.join(tmpdir(), "openclaw-test-extension-signal-")); const fakePnpmPath = path.join(root, "pnpm"); const childPidPath = path.join(root, "child.pid"); + const descendantPidPath = path.join(root, "descendant.pid"); const signaledPath = path.join(root, "signaled"); writeFakePnpm(fakePnpmPath); @@ -755,6 +756,7 @@ describe("scripts/test-extension.mjs", () => { cwd: process.cwd(), env: { ...process.env, + OPENCLAW_FAKE_PNPM_DESCENDANT_PID_PATH: descendantPidPath, OPENCLAW_FAKE_PNPM_PID_PATH: childPidPath, OPENCLAW_FAKE_PNPM_SIGNALED_PATH: signaledPath, npm_execpath: fakePnpmPath, @@ -762,11 +764,15 @@ describe("scripts/test-extension.mjs", () => { stdio: "ignore", }); let childPid = 0; + let descendantPid = 0; try { await waitFor(() => fileExists(childPidPath), 5_000); + await waitFor(() => fileExists(descendantPidPath), 5_000); childPid = Number(readFileSync(childPidPath, "utf8")); + descendantPid = Number(readFileSync(descendantPidPath, "utf8")); expect(Number.isInteger(childPid)).toBe(true); + expect(Number.isInteger(descendantPid)).toBe(true); expect(runner.pid).toBeGreaterThan(0); process.kill(runner.pid!, "SIGTERM"); @@ -776,6 +782,7 @@ describe("scripts/test-extension.mjs", () => { await waitFor(() => fileExists(signaledPath), 5_000); expect(readFileSync(signaledPath, "utf8")).toBe("SIGTERM"); await waitFor(() => !isProcessAlive(childPid), 5_000); + await waitFor(() => !isProcessAlive(descendantPid), 5_000); } finally { if (runner.pid && isProcessAlive(runner.pid)) { process.kill(runner.pid, "SIGKILL"); @@ -783,6 +790,9 @@ describe("scripts/test-extension.mjs", () => { if (childPid && isProcessAlive(childPid)) { process.kill(childPid, "SIGKILL"); } + if (descendantPid && isProcessAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } rmSync(root, { force: true, recursive: true }); } }, @@ -901,11 +911,19 @@ function writeFakePnpm(filePath: string): void { filePath, [ "#!/usr/bin/env node", + 'const { spawn } = require("node:child_process");', 'const fs = require("node:fs");', "if (process.env.OPENCLAW_FAKE_PNPM_ARGS_PATH) {", " fs.writeFileSync(process.env.OPENCLAW_FAKE_PNPM_ARGS_PATH, JSON.stringify(process.argv.slice(2)));", " process.exit(0);", "}", + "if (process.env.OPENCLAW_FAKE_PNPM_DESCENDANT_PID_PATH) {", + " const child = spawn(process.execPath, [", + ' "-e",', + " \"process.on('SIGTERM', () => {}); setInterval(() => {}, 1000);\",", + " ], { stdio: 'ignore' });", + " fs.writeFileSync(process.env.OPENCLAW_FAKE_PNPM_DESCENDANT_PID_PATH, String(child.pid));", + "}", "fs.writeFileSync(process.env.OPENCLAW_FAKE_PNPM_PID_PATH, String(process.pid));", 'process.on("SIGTERM", () => {', ' fs.writeFileSync(process.env.OPENCLAW_FAKE_PNPM_SIGNALED_PATH, "SIGTERM");',