From bd5a5a0cfc9158b2fd84a9aafef4e3316429c13a Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 20 Jun 2026 10:38:46 +0200 Subject: [PATCH] fix(test): preserve lifecycle probe timeout failures --- .../plugins/plugin-lifecycle-probe-runtime.ts | 41 +++++++++++++++---- .../plugin-lifecycle-probe.e2e.test.ts | 41 ++++++++++++++++++- 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/test/e2e/qa-lab/plugins/plugin-lifecycle-probe-runtime.ts b/test/e2e/qa-lab/plugins/plugin-lifecycle-probe-runtime.ts index cab80c18ac53..7adb3cbc4979 100644 --- a/test/e2e/qa-lab/plugins/plugin-lifecycle-probe-runtime.ts +++ b/test/e2e/qa-lab/plugins/plugin-lifecycle-probe-runtime.ts @@ -16,6 +16,8 @@ type MatrixEnv = NodeJS.ProcessEnv & ProbeEnv; interface CommandOptions { env?: NodeJS.ProcessEnv; outputFile?: string; + spawnImpl?: typeof spawn; + timeoutKillGraceMs?: number; timeoutMs?: number; } @@ -231,28 +233,45 @@ async function runCommand(command: string, args: readonly string[], options: Com options.outputFile === undefined ? undefined : fs.openSync(options.outputFile, "a"); try { await new Promise((resolve, reject) => { - const child = spawn(command, args, { + const spawnImpl = options.spawnImpl ?? spawn; + const child = spawnImpl(command, args, { cwd: process.cwd(), env: options.env ?? process.env, stdio: outputFd === undefined ? "inherit" : (["ignore", outputFd, outputFd] as const), }); let settled = false; - const timer = + let forceKillTimer: NodeJS.Timeout | undefined; + let timeoutTimer: NodeJS.Timeout | undefined; + let timeoutError: Error | undefined; + const clearTimers = () => { + if (timeoutTimer) { + clearTimeout(timeoutTimer); + } + if (forceKillTimer) { + clearTimeout(forceKillTimer); + } + }; + timeoutTimer = options.timeoutMs === undefined ? undefined : setTimeout(() => { + timeoutError = new Error( + `${command} ${args.join(" ")} timed out after ${options.timeoutMs}ms`, + ); child.kill("SIGTERM"); - setTimeout(() => child.kill("SIGKILL"), 2_000).unref(); + forceKillTimer = setTimeout( + () => child.kill("SIGKILL"), + options.timeoutKillGraceMs ?? 2_000, + ); + forceKillTimer.unref(); }, options.timeoutMs); - timer?.unref(); + timeoutTimer?.unref(); child.once("error", (error) => { if (settled) { return; } settled = true; - if (timer) { - clearTimeout(timer); - } + clearTimers(); reject(error); }); child.once("exit", (code, signal) => { @@ -260,8 +279,10 @@ async function runCommand(command: string, args: readonly string[], options: Com return; } settled = true; - if (timer) { - clearTimeout(timer); + clearTimers(); + if (timeoutError) { + reject(timeoutError); + return; } if (code === 0 && !signal) { resolve(); @@ -532,6 +553,8 @@ export async function runPluginLifecycleMatrix() { } } +export const testing = { runCommand }; + const isLifecycleMatrixCli = process.argv[2] === "--lifecycle-matrix"; if (isLifecycleMatrixCli) { diff --git a/test/e2e/qa-lab/plugins/plugin-lifecycle-probe.e2e.test.ts b/test/e2e/qa-lab/plugins/plugin-lifecycle-probe.e2e.test.ts index d4f309d3e3da..a353ccaa96b6 100644 --- a/test/e2e/qa-lab/plugins/plugin-lifecycle-probe.e2e.test.ts +++ b/test/e2e/qa-lab/plugins/plugin-lifecycle-probe.e2e.test.ts @@ -1,12 +1,14 @@ // Plugin Lifecycle Probe tests cover QA Lab plugin lifecycle evidence. +import { EventEmitter } from "node:events"; import { mkdirSync, writeFileSync } from "node:fs"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createTempDirTracker } from "../../../helpers/temp-dir.js"; import { assertInspectLoaded, assertUninstalled, parseDurationMs, + testing as probeTesting, } from "./plugin-lifecycle-probe-runtime.js"; const tempDirs = createTempDirTracker(); @@ -15,6 +17,18 @@ function makeTempDir(): string { return tempDirs.make("openclaw-plugin-lifecycle-probe-"); } +class FakeCommandChild extends EventEmitter { + readonly signals: string[] = []; + + kill(signal?: NodeJS.Signals | number): boolean { + this.signals.push(String(signal)); + if (signal === "SIGTERM") { + queueMicrotask(() => this.emit("exit", 0, null)); + } + return true; + } +} + afterEach(tempDirs.cleanup); describe("plugin lifecycle matrix probe", () => { @@ -70,4 +84,29 @@ describe("plugin lifecycle matrix probe", () => { it("preserves disabled npm install timeout semantics", () => { expect(parseDurationMs("0", "600s")).toBeUndefined(); }); + + it("rejects timed commands that exit cleanly during kill grace", async () => { + vi.useFakeTimers(); + try { + const child = new FakeCommandChild(); + const runPromise = probeTesting.runCommand("fake-command", ["install"], { + spawnImpl: (() => child) as unknown as typeof import("node:child_process").spawn, + timeoutKillGraceMs: 100, + timeoutMs: 10, + }); + const runError = runPromise.catch((error: unknown) => error); + + await vi.advanceTimersByTimeAsync(10); + + const error = await runError; + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("fake-command install timed out after 10ms"); + expect(child.signals).toEqual(["SIGTERM"]); + + await vi.advanceTimersByTimeAsync(100); + expect(child.signals).toEqual(["SIGTERM"]); + } finally { + vi.useRealTimers(); + } + }); });