From aa93af5753828f329bcd55924b5bc76de87ded48 Mon Sep 17 00:00:00 2001 From: m4air Date: Fri, 2 Oct 2026 12:44:06 -0700 Subject: [PATCH] test(orcad): wait for killed terminal daemons to exit before removing their temp profiles --- ...untime-daemon-adoption.integration.test.ts | 36 +++----- .../orcad/orcad-daemon-teardown-fixture.ts | 92 +++++++++++++++++++ .../orcad-serve-parity.integration.test.ts | 48 +++------- ...-stop-request-shutdown.integration.test.ts | 29 +++--- 4 files changed, 130 insertions(+), 75 deletions(-) create mode 100644 src/main/orcad/orcad-daemon-teardown-fixture.ts diff --git a/src/main/orcad/orcad-cross-runtime-daemon-adoption.integration.test.ts b/src/main/orcad/orcad-cross-runtime-daemon-adoption.integration.test.ts index 09cc44ed3b02..a80a6c9b9ec9 100644 --- a/src/main/orcad/orcad-cross-runtime-daemon-adoption.integration.test.ts +++ b/src/main/orcad/orcad-cross-runtime-daemon-adoption.integration.test.ts @@ -28,8 +28,11 @@ import { orcadBunRuntimeFilename, orcadNodeRuntimeRelativePath } from '../../shared/orcad-artifacts' -import { removeTreeSync } from '../../shared/windows-transient-lock-removal' -import { readDaemonPidRecord } from '../daemon/daemon-endpoint-incarnation' +import { + killAndAwaitExit, + killProfileDaemons, + removeTestRoot +} from './orcad-daemon-teardown-fixture' import { PROTOCOL_VERSION } from '../daemon/types' import type { ServeReadiness } from '../server/serve-readiness' import { @@ -256,13 +259,10 @@ async function backUpProfile(slot: Slot, runtime: string, userDataDir: string): expect(JSON.parse(result.stdout.trim().split('\n').at(-1) ?? '')).toEqual({ ok: true }) } -function killLaunched(): void { - for (const pid of launched) { - try { - process.kill(pid, 'SIGKILL') - } catch {} - } +async function killLaunched(): Promise { + const pids = [...launched] launched.clear() + await killAndAwaitExit(pids) } /** The protocol the Bun slot's own daemon reports, read from a throwaway launch. */ @@ -276,17 +276,9 @@ async function probeBunDaemonProtocol(): Promise { return daemon.protocolVersion! } finally { await stop(slot).catch(() => {}) - killLaunched() + await killLaunched() // Even after a failed launch: the daemon outlives orcad, and only its pid file names it. - const daemonDir = join(userDataDir, 'daemon') - for (const name of existsSync(daemonDir) ? readdirSync(daemonDir) : []) { - const pid = /^daemon-v\d+\.pid$/.test(name) - ? readDaemonPidRecord(join(daemonDir, name))?.pid - : undefined - if (pid && isAlive(pid)) { - process.kill(pid, 'SIGKILL') - } - } + await killProfileDaemons(userDataDir) } } @@ -362,8 +354,8 @@ worker.on('error', (error) => { console.error(error); process.exitCode = 1 }) afterEach(killLaunched) - afterAll(() => { - removeTreeSync(root) + afterAll(async () => { + await removeTestRoot(root) }) it.for([ @@ -459,8 +451,8 @@ worker.on('error', (error) => { console.error(error); process.exitCode = 1 }) await daemonClient('kill', userDataDir, sessionId, '') await vi.waitFor(() => expect(isAlive(created.pid)).toBe(false), { timeout: 10_000 }) } finally { - if (daemonPid && isAlive(daemonPid)) { - process.kill(daemonPid, 'SIGKILL') + if (daemonPid) { + await killAndAwaitExit([daemonPid]) } } } diff --git a/src/main/orcad/orcad-daemon-teardown-fixture.ts b/src/main/orcad/orcad-daemon-teardown-fixture.ts new file mode 100644 index 000000000000..41798ac2ac92 --- /dev/null +++ b/src/main/orcad/orcad-daemon-teardown-fixture.ts @@ -0,0 +1,92 @@ +/** + * Teardown for integration tests whose orcad started a real terminal daemon in a temp profile. + * + * The daemon outlives orcad by design and only its pid record names it. Killing it is not + * enough: until it has exited it can still remove its socket, pid and token files and append + * to its log, so a recursive delete racing it fails with ENOTEMPTY (seen on macOS). + */ +import { existsSync, readdirSync } from 'node:fs' +import { join } from 'node:path' +import { removeTreeSync } from '../../shared/windows-transient-lock-removal' +import { readDaemonPidRecord } from '../daemon/daemon-endpoint-incarnation' +import type { spawnProcess } from '../../shared/child-process/run-process' + +const EXIT_WAIT_MS = 10_000 +const POLL_MS = 50 +const REMOVE_ATTEMPTS = 8 +const REMOVE_RETRY_MS = 150 + +const sleep = (ms: number): Promise => new Promise((settle) => setTimeout(settle, ms)) + +function errorCode(error: unknown): string | undefined { + return typeof error === 'object' && + error !== null && + 'code' in error && + typeof error.code === 'string' + ? error.code + : undefined +} + +function hasExited(pid: number): boolean { + try { + process.kill(pid, 0) + return false + } catch (error) { + // EPERM still proves the process exists. + return errorCode(error) === 'ESRCH' + } +} + +/** SIGKILLs each pid and waits, bounded, until the OS no longer knows it. */ +export async function killAndAwaitExit(pids: Iterable): Promise { + const pending = [...pids] + for (const pid of pending) { + try { + process.kill(pid, 'SIGKILL') + } catch {} + } + const deadline = Date.now() + EXIT_WAIT_MS + while (pending.some((pid) => !hasExited(pid)) && Date.now() < deadline) { + await sleep(POLL_MS) + } +} + +/** Kills a spawned child and waits for its exit event, so nothing it spawned races teardown. */ +export async function killChildAndWait(child: ReturnType): Promise { + if (child.exitCode !== null || child.signalCode !== null) { + return + } + const exited = new Promise((settle) => child.once('exit', settle)) + child.kill('SIGKILL') + await exited +} + +/** Kills every terminal daemon `/daemon` names and waits until each has exited. */ +export async function killProfileDaemons(userData: string): Promise { + const daemonDir = join(userData, 'daemon') + const pids = (existsSync(daemonDir) ? readdirSync(daemonDir) : []).flatMap((name) => { + const pid = /^daemon-v\d+\.pid$/u.test(name) + ? readDaemonPidRecord(join(daemonDir, name))?.pid + : undefined + return pid ? [pid] : [] + }) + await killAndAwaitExit(pids) +} + +/** Removes a test root, retrying a late writer's ENOTEMPTY/EBUSY on every platform. */ +export async function removeTestRoot(root: string): Promise { + for (let attempt = 1; ; attempt += 1) { + try { + removeTreeSync(root) + return + } catch (error) { + if ( + attempt >= REMOVE_ATTEMPTS || + !['ENOTEMPTY', 'EBUSY', 'EPERM'].includes(errorCode(error) ?? '') + ) { + throw error + } + await sleep(REMOVE_RETRY_MS) + } + } +} diff --git a/src/main/orcad/orcad-serve-parity.integration.test.ts b/src/main/orcad/orcad-serve-parity.integration.test.ts index 98e640fcb5b1..418c3dd63c64 100644 --- a/src/main/orcad/orcad-serve-parity.integration.test.ts +++ b/src/main/orcad/orcad-serve-parity.integration.test.ts @@ -3,13 +3,11 @@ * first, desktop-serve flag parity on stdout (the readiness contract), and the shared-profile * refusal while the desktop app holds the profile. */ -import { existsSync, mkdtempSync, readdirSync, realpathSync, writeFileSync } from 'node:fs' +import { existsSync, mkdtempSync, realpathSync, writeFileSync } from 'node:fs' import { tmpdir, userInfo } from 'node:os' import { join, resolve } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { runProcess, spawnProcess } from '../../shared/child-process/run-process' -import { removeTreeSync } from '../../shared/windows-transient-lock-removal' -import { readDaemonPidRecord } from '../daemon/daemon-endpoint-incarnation' import { ORCAD_NODE_RUNTIME_MARKER_FILENAME } from '../../shared/orcad-artifacts' import { ORCAD_NATIVE_PREFLIGHT_FLAG, @@ -19,6 +17,12 @@ import { ORCAD_LOCK_FILE_NAME } from './orcad-instance-lock' import { resolveBundledOrcadRuntime } from './orcad-bundled-runtime' import { skipForMissingInputs } from './orcad-node-slot-fixture' import { buildDaemonSessionClient } from './orcad-daemon-session-client-fixture' +import { + killAndAwaitExit, + killChildAndWait, + killProfileDaemons, + removeTestRoot +} from './orcad-daemon-teardown-fixture' const slotDir = resolve('out/orcad') const runtime = existsSync(join(slotDir, ORCAD_NODE_RUNTIME_MARKER_FILENAME)) @@ -27,38 +31,14 @@ const runtime = existsSync(join(slotDir, ORCAD_NODE_RUNTIME_MARKER_FILENAME)) const skip = skipForMissingInputs('artifact', runtime ? [] : ['a Node orcad slot in out/orcad']) const roots: string[] = [] -afterEach(() => { +afterEach(async () => { for (const root of roots.splice(0)) { - // The terminal daemon outlives orcad by design; on Windows its open files pin the profile. - stopProfileDaemons(root) - removeTreeSync(root) + // The terminal daemon outlives orcad by design; it must be gone before its profile is. + await killProfileDaemons(root) + await removeTestRoot(root) } }) -function stopProfileDaemons(userData: string): void { - const daemonDir = join(userData, 'daemon') - for (const name of existsSync(daemonDir) ? readdirSync(daemonDir) : []) { - const pid = /^daemon-v\d+\.pid$/u.test(name) - ? readDaemonPidRecord(join(daemonDir, name))?.pid - : undefined - try { - if (pid) { - process.kill(pid, 'SIGKILL') - } - } catch {} - } -} - -/** Kills a serve child and waits until it has really exited, so nothing holds its files. */ -async function killAndWait(child: ReturnType): Promise { - if (child.exitCode !== null || child.signalCode !== null) { - return - } - const exited = new Promise((settle) => child.once('exit', settle)) - child.kill('SIGKILL') - await exited -} - /** Without Vitest's markers: daemon-entry.js does not start its server under VITEST. */ function serveEnv(userData: string): NodeJS.ProcessEnv { return { @@ -126,7 +106,7 @@ describe.skipIf(skip)('orca serve on orcad', () => { // Anything after the recipe line would break a reader of this contract. await new Promise((settle) => setTimeout(settle, 1_000)) } finally { - await killAndWait(child) + await killChildAndWait(child) } const lines = stdout.split('\n').filter(Boolean) expect(lines, stderr).toHaveLength(1) @@ -196,9 +176,7 @@ describe.skipIf(skip)('orca serve on orcad', () => { } finally { await stopServe(userData) if (daemonPid) { - try { - process.kill(daemonPid, 'SIGKILL') - } catch {} + await killAndAwaitExit([daemonPid]) daemonPid = null } } diff --git a/src/main/orcad/orcad-stop-request-shutdown.integration.test.ts b/src/main/orcad/orcad-stop-request-shutdown.integration.test.ts index 76b9abe83d41..bf0aed4f78fe 100644 --- a/src/main/orcad/orcad-stop-request-shutdown.integration.test.ts +++ b/src/main/orcad/orcad-stop-request-shutdown.integration.test.ts @@ -6,15 +6,18 @@ * server notices the file, shuts down cleanly (exit 0, lock released) before the 15 s * deadline, and published an instance lock carrying a real process start time. */ -import { existsSync, mkdtempSync, readdirSync, writeFileSync } from 'node:fs' +import { existsSync, mkdtempSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' import { spawnProcess } from '../../shared/child-process/run-process' import { ORCAD_NODE_RUNTIME_MARKER_FILENAME } from '../../shared/orcad-artifacts' import { ORCAD_STOP_REQUEST_FILENAME } from '../../shared/orcad-stop-request' -import { removeTreeSync } from '../../shared/windows-transient-lock-removal' -import { readDaemonPidRecord } from '../daemon/daemon-endpoint-incarnation' +import { + killChildAndWait, + killProfileDaemons, + removeTestRoot +} from './orcad-daemon-teardown-fixture' import { parseOrcadReadinessOutput } from '../ssh/orcad-remote-launch' import { ORCAD_LOCK_FILE_NAME, readOrcadInstanceLockRecord } from './orcad-instance-lock' import { ORCAD_SHUTDOWN_DEADLINE_MS } from './orcad-lifecycle' @@ -48,24 +51,14 @@ function hostEnv(userData: string): NodeJS.ProcessEnv { } } -afterEach(() => { +afterEach(async () => { for (const child of children.splice(0)) { - child.kill('SIGKILL') - } - // The terminal daemon deliberately outlives orcad; only its pid file names it. - const daemonDir = join(root, 'data', 'daemon') - for (const name of existsSync(daemonDir) ? readdirSync(daemonDir) : []) { - const pid = /^daemon-v\d+\.pid$/u.test(name) - ? readDaemonPidRecord(join(daemonDir, name))?.pid - : undefined - try { - if (pid) { - process.kill(pid, 'SIGKILL') - } - } catch {} + await killChildAndWait(child) } if (root) { - removeTreeSync(root) + // The terminal daemon deliberately outlives orcad; it must be gone before its profile is. + await killProfileDaemons(join(root, 'data')) + await removeTestRoot(root) } })