From 88afe3d3c2a96bed05e661d4ec248e24cb2cd608 Mon Sep 17 00:00:00 2001 From: dengbushi <131344412+dengbushi@users.noreply.github.com> Date: Thu, 16 Jul 2026 18:42:54 +0800 Subject: [PATCH] fix(cli): clean test runner temp environments (#12232) * fix(cli): clean test runner temp environments * fix(cli): clean test environments on runner shutdown --------- Co-authored-by: marius-kilocode --- packages/opencode/script/test-runner.ts | 64 +++++++-- .../test/kilocode/test-runner-cleanup.test.ts | 121 ++++++++++++++++++ 2 files changed, 175 insertions(+), 10 deletions(-) create mode 100644 packages/opencode/test/kilocode/test-runner-cleanup.test.ts diff --git a/packages/opencode/script/test-runner.ts b/packages/opencode/script/test-runner.ts index 795bf056604..45740bf19fe 100644 --- a/packages/opencode/script/test-runner.ts +++ b/packages/opencode/script/test-runner.ts @@ -9,6 +9,7 @@ import path from "path" import fs from "fs/promises" import { TestProfile } from "./kilocode/test-profile" import { TestShard } from "./kilocode/test-shard" +import { remove } from "../test/kilocode/cleanup" const root = path.resolve(import.meta.dir, "..") const argv = process.argv.slice(2) @@ -187,6 +188,10 @@ if (ci) await fs.mkdir(xmldir, { recursive: true }) const counter = { done: 0 } const pad = String(files.length).length const progress = { width: 80 } +const active = new Map>() +const pending = new Map>() +const stopping = { promise: undefined as Promise | undefined } +const stopped = { value: false } const marks = { pass: ".", retry: "R", @@ -217,32 +222,65 @@ async function run(file: string): Promise { stderr: "pipe", windowsHide: true, }) + active.set(proc.pid, proc) const timer = setTimeout(() => { killed.value = true proc.kill() }, deadline) - const [stdout, stderr, code] = await Promise.all([ - new Response(proc.stdout).text(), - new Response(proc.stderr).text(), - proc.exited, - ]) - - clearTimeout(timer) + const stdout = new Response(proc.stdout).text() + const stderr = new Response(proc.stderr).text() + const code = await proc.exited.finally(async () => { + clearTimeout(timer) + await finish(proc) + }) + const output = await Promise.all([stdout, stderr]) return { file, passed: code === 0, code, - stdout, - stderr, + stdout: output[0], + stderr: output[1], duration: performance.now() - start, timedout: killed.value, attempts: 1, } } +function finish(proc: ReturnType) { + const found = pending.get(proc.pid) + if (found) return found + + const promise = (async () => { + await proc.exited + await cleanup(proc.pid) + })().finally(() => { + active.delete(proc.pid) + pending.delete(proc.pid) + }) + pending.set(proc.pid, promise) + return promise +} + +function shutdown(code: number) { + if (stopping.promise) return stopping.promise + stopping.promise = (async () => { + stopped.value = true + const children = [...active.values()] + for (const proc of children) { + if (proc.exitCode === null) proc.kill("SIGKILL") + } + await Promise.all(children.map(finish)) + process.exit(code) + })() + return stopping.promise +} + +process.once("SIGINT", () => void shutdown(130)) +process.once("SIGTERM", () => void shutdown(143)) + // --------------------------------------------------------------------------- // Report a single result // --------------------------------------------------------------------------- @@ -302,7 +340,6 @@ console.log() const start = performance.now() const results: Result[] = [] const queue = TestShard.order(files, weight) -const stopped = { value: false } const workers = Array.from({ length: Math.min(concurrency, files.length) }, async () => { while (queue.length > 0 && !stopped.value) { @@ -481,6 +518,13 @@ async function merge() { await Bun.write(path.join(dir, "junit.xml"), body) } +async function cleanup(pid: number) { + const dir = path.join(os.tmpdir(), `opencode-test-data-${pid}`) + await remove(dir).catch((err) => { + console.error(`cleanup failed for ${dir}:`, err) + }) +} + // Grab everything between the outer and of a // per-file JUnit XML. Preserves nested blocks verbatim — the // previous hand-rolled walker matched the first it found, which diff --git a/packages/opencode/test/kilocode/test-runner-cleanup.test.ts b/packages/opencode/test/kilocode/test-runner-cleanup.test.ts new file mode 100644 index 00000000000..9f5dbdb6397 --- /dev/null +++ b/packages/opencode/test/kilocode/test-runner-cleanup.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, test } from "bun:test" +import fs from "fs/promises" +import os from "os" +import path from "path" +import { tmpdir } from "../fixture/fixture" +import { remove } from "./cleanup" + +const root = path.resolve(import.meta.dir, "../..") + +function env(marker: string) { + const vars: NodeJS.ProcessEnv = { ...process.env, KILO_TEST_RUNNER_PID_FILE: marker } + delete vars.KILO_TEST_PROFILE + delete vars.KILO_TEST_SHARD + return vars +} + +function spawn(name: string, marker: string) { + return Bun.spawn( + ["bun", "run", "script/test-runner.ts", "--concurrency", "1", "--retries", "-1", `kilocode/${name}`], + { + cwd: root, + env: env(marker), + stdout: "pipe", + stderr: "pipe", + windowsHide: true, + }, + ) +} + +async function deadline(promise: Promise, timeout: number) { + const expired = Symbol("expired") + const result = await Promise.race([promise, Bun.sleep(timeout).then(() => expired)]) + if (result === expired) throw new Error(`Timed out after ${timeout}ms`) + return result +} + +describe("test runner cleanup", () => { + test("removes the temp environment after an abrupt child exit", async () => { + await using tmp = await tmpdir() + const name = `runner-abrupt-${process.pid}-${Date.now()}.test.ts` + const file = path.join(import.meta.dir, name) + const marker = path.join(tmp.path, "pid") + const state = { pid: 0 } + const src = [ + "const marker = process.env.KILO_TEST_RUNNER_PID_FILE", + 'if (!marker) throw new Error("KILO_TEST_RUNNER_PID_FILE is required")', + "await Bun.write(marker, String(process.pid))", + "process.exit(1)", + "", + ].join("\n") + + await fs.writeFile(file, src) + const proc = spawn(name, marker) + const stdout = new Response(proc.stdout).text() + const stderr = new Response(proc.stderr).text() + + try { + const code = await deadline(proc.exited, 15_000) + const output = await Promise.all([stdout, stderr]) + + if (!(await Bun.file(marker).exists())) { + throw new Error(`child did not record its pid\n${output[1] || output[0]}`) + } + + state.pid = Number(await fs.readFile(marker, "utf8")) + expect(code).not.toBe(0) + expect(await Bun.file(path.join(os.tmpdir(), `opencode-test-data-${state.pid}`)).exists()).toBe(false) + } finally { + if (proc.exitCode === null) proc.kill("SIGKILL") + await proc.exited + await fs.rm(file, { force: true }) + if (state.pid) await remove(path.join(os.tmpdir(), `opencode-test-data-${state.pid}`)) + } + }) + + test.skipIf(process.platform === "win32")( + "removes active temp environments when the runner is terminated", + async () => { + await using tmp = await tmpdir() + const name = `runner-signal-${process.pid}-${Date.now()}.test.ts` + const file = path.join(import.meta.dir, name) + const marker = path.join(tmp.path, "pid") + const state = { pid: 0 } + const src = [ + "const marker = process.env.KILO_TEST_RUNNER_PID_FILE", + 'if (!marker) throw new Error("KILO_TEST_RUNNER_PID_FILE is required")', + "await Bun.write(marker, String(process.pid))", + "const parent = process.ppid", + "setInterval(() => process.ppid === parent || process.exit(1), 50)", + "await Bun.sleep(60_000)", + "", + ].join("\n") + + await fs.writeFile(file, src) + const proc = spawn(name, marker) + const stdout = new Response(proc.stdout).text() + const stderr = new Response(proc.stderr).text() + + try { + await deadline( + (async () => { + while (!(await Bun.file(marker).exists())) await Bun.sleep(10) + })(), + 10_000, + ) + state.pid = Number(await fs.readFile(marker, "utf8")) + proc.kill("SIGTERM") + + expect(await deadline(proc.exited, 10_000)).toBe(143) + await Promise.all([stdout, stderr]) + expect(await Bun.file(path.join(os.tmpdir(), `opencode-test-data-${state.pid}`)).exists()).toBe(false) + } finally { + if (proc.exitCode === null) proc.kill("SIGKILL") + await proc.exited + await fs.rm(file, { force: true }) + if (state.pid) await remove(path.join(os.tmpdir(), `opencode-test-data-${state.pid}`)) + } + }, + 30_000, + ) +})