diff --git a/packages/test/src/setup.ts b/packages/test/src/setup.ts index ed70b1166..45390e67f 100644 --- a/packages/test/src/setup.ts +++ b/packages/test/src/setup.ts @@ -32,6 +32,20 @@ export async function setup(project: TestProject) { // Kill a process group (for detached processes) or process tree async function killProcessTree(pid: number, name: string): Promise { + if (process.platform === 'win32') { + // negative-pid process groups don't exist on Windows — the POSIX path below + // would degrade to killing only the direct child and orphan its workers. + // taskkill /T terminates the whole tree (mirrors setupTest.ts). + try { + const { execSync } = await import('node:child_process') + execSync(`taskkill /F /T /PID ${pid}`, { stdio: 'ignore' }) + console.info(`${name} process (PID: ${pid}) killed successfully.`) + } catch { + // process may already be gone, which is fine + } + return + } + try { // for detached processes, kill the entire process group using negative pid try { diff --git a/tests/test/tests/metro-compiler.test.ts b/tests/test/tests/metro-compiler.test.ts index f7a75af49..e1b09d0c4 100644 --- a/tests/test/tests/metro-compiler.test.ts +++ b/tests/test/tests/metro-compiler.test.ts @@ -1,6 +1,18 @@ import { spawn, type ChildProcess } from 'node:child_process' +import { createRequire } from 'node:module' +import { dirname, join } from 'node:path' import getPort from 'get-port' import { describe, expect, it, afterEach } from 'vitest' +import { killProcessTree } from './process-tree' + +// Resolve one's JS entry (run.mjs) instead of node_modules/.bin/one: on Windows +// the .bin shim is a .cmd/.ps1/.exe wrapper that `node ` can't load +// (MODULE_NOT_FOUND), so the dev server never starts. Resolving via package.json +// gives the real JS file on every platform (mirrors packages/test/src/setupTest.ts). +const oneRunEntry = join( + dirname(createRequire(import.meta.url).resolve('one/package.json')), + 'run.mjs' +) /** * Test that the React compiler works through the Metro bundling path. @@ -20,10 +32,9 @@ import { describe, expect, it, afterEach } from 'vitest' describe('Metro React compiler', { retry: 1 }, () => { let devServer: ChildProcess | null = null - afterEach(() => { + afterEach(async () => { if (devServer) { - devServer.kill('SIGTERM') - setTimeout(() => devServer?.kill('SIGKILL'), 1000) + await killProcessTree(devServer.pid) devServer = null } }) @@ -35,18 +46,16 @@ describe('Metro React compiler', { retry: 1 }, () => { let metroReady = false let processExited = false - devServer = spawn( - 'node', - ['../../node_modules/.bin/one', 'dev', '--port', port.toString()], - { - cwd: process.cwd(), - env: { - ...process.env, - DEBUG: 'vxrn:*,vite-plugin-metro:*', - }, - stdio: ['ignore', 'pipe', 'pipe'], - } - ) + devServer = spawn('node', [oneRunEntry, 'dev', '--port', port.toString()], { + cwd: process.cwd(), + env: { + ...process.env, + DEBUG: 'vxrn:*,vite-plugin-metro:*', + }, + stdio: ['ignore', 'pipe', 'pipe'], + // group leader on POSIX so killProcessTree can signal the whole group + detached: process.platform !== 'win32', + }) devServer.on('exit', (code) => { processExited = true diff --git a/tests/test/tests/metro-startup-order.test.ts b/tests/test/tests/metro-startup-order.test.ts index 4c2bbf797..336db23ad 100644 --- a/tests/test/tests/metro-startup-order.test.ts +++ b/tests/test/tests/metro-startup-order.test.ts @@ -1,6 +1,18 @@ import { spawn } from 'node:child_process' +import { createRequire } from 'node:module' +import { dirname, join } from 'node:path' import getPort from 'get-port' import { describe, expect, it } from 'vitest' +import { killProcessTree } from './process-tree' + +// Resolve one's JS entry (run.mjs) instead of node_modules/.bin/one: on Windows +// the .bin shim is a .cmd/.ps1/.exe wrapper that `node ` can't load +// (MODULE_NOT_FOUND), so the dev server never starts. Resolving via package.json +// gives the real JS file on every platform (mirrors packages/test/src/setupTest.ts). +const oneRunEntry = join( + dirname(createRequire(import.meta.url).resolve('one/package.json')), + 'run.mjs' +) /** * Test that Metro initialization happens AFTER Vite server is fully started. @@ -23,21 +35,19 @@ describe('Metro startup order', () => { let viteReadyPos = -1 let metroReadyPos = -1 - const devServer = spawn( - 'node', - ['../../node_modules/.bin/one', 'dev', '--port', port.toString()], - { - cwd: process.cwd(), - env: { - ...process.env, - // Enable Metro mode - ONE_METRO_MODE: '1', - // Enable debug logging to see Metro ready message - DEBUG: 'vxrn:*', - }, - stdio: ['ignore', 'pipe', 'pipe'], - } - ) + const devServer = spawn('node', [oneRunEntry, 'dev', '--port', port.toString()], { + cwd: process.cwd(), + env: { + ...process.env, + // Enable Metro mode + ONE_METRO_MODE: '1', + // Enable debug logging to see Metro ready message + DEBUG: 'vxrn:*', + }, + stdio: ['ignore', 'pipe', 'pipe'], + // group leader on POSIX so killProcessTree can signal the whole group + detached: process.platform !== 'win32', + }) const collectLogs = (data: Buffer) => { const text = data.toString() @@ -80,10 +90,9 @@ describe('Metro startup order', () => { }, 100) }) - // Kill the server - devServer.kill('SIGTERM') + // Kill the server (tree-kill: taskkill /T on Windows, process-group on POSIX) + await killProcessTree(devServer.pid) await new Promise((r) => setTimeout(r, 500)) - devServer.kill('SIGKILL') // Verify the order - Vite should be ready BEFORE Metro console.info('Output captured:\n', allOutput) diff --git a/tests/test/tests/process-cleanup.test.ts b/tests/test/tests/process-cleanup.test.ts index 7742c5929..a3bd5ac80 100644 --- a/tests/test/tests/process-cleanup.test.ts +++ b/tests/test/tests/process-cleanup.test.ts @@ -1,6 +1,18 @@ import { spawn, type ChildProcess } from 'node:child_process' +import { createRequire } from 'node:module' +import { dirname, join } from 'node:path' import { afterEach, describe, expect, test } from 'vitest' import getPort from 'get-port' +import { killProcessTree } from './process-tree' + +// Resolve one's JS entry (run.mjs) instead of node_modules/.bin/one: on Windows +// the .bin shim is a .cmd/.ps1/.exe wrapper that `node ` can't load +// (MODULE_NOT_FOUND), so the dev server never starts. Resolving via package.json +// gives the real JS file on every platform (mirrors packages/test/src/setupTest.ts). +const oneRunEntry = join( + dirname(createRequire(import.meta.url).resolve('one/package.json')), + 'run.mjs' +) // Helper to wait for a process to exit function waitForExit(proc: ChildProcess, timeout = 5000): Promise { @@ -27,81 +39,86 @@ describe('process cleanup', () => { let devServer: ChildProcess | null = null let wrapper: ChildProcess | null = null - afterEach(() => { - // Clean up any leftover processes + afterEach(async () => { + // Clean up any leftover processes (tree-kill so dev-server workers don't orphan) if (devServer?.pid && isRunning(devServer.pid)) { - process.kill(devServer.pid, 'SIGKILL') + await killProcessTree(devServer.pid) } if (wrapper?.pid && isRunning(wrapper.pid)) { - process.kill(wrapper.pid, 'SIGKILL') + await killProcessTree(wrapper.pid) } }) - test('dev server exits when parent is killed (orphan detection)', async () => { - const port = await getPort() + // orphan detection is POSIX-only by design (setupOrphanDetection in + // packages/vxrn/src/exports/dev.ts returns early on win32), and the test + // discovers the grandchild via pgrep — skip on Windows rather than fail + test.skipIf(process.platform === 'win32')( + 'dev server exits when parent is killed (orphan detection)', + async () => { + const port = await getPort() - // Spawn a wrapper process that spawns the dev server - // When we kill the wrapper, the dev server should detect it's orphaned and exit - wrapper = spawn( - 'node', - [ - '-e', - ` + // Spawn a wrapper process that spawns the dev server + // When we kill the wrapper, the dev server should detect it's orphaned and exit + wrapper = spawn( + 'node', + [ + '-e', + ` const { spawn } = require('child_process'); - const dev = spawn('node', ['../../node_modules/.bin/one', 'dev', '--port', '${port}'], { + const dev = spawn('node', [${JSON.stringify(oneRunEntry)}, 'dev', '--port', '${port}'], { stdio: 'inherit' }); dev.unref(); // Keep wrapper alive setInterval(() => {}, 1000); `, - ], - { - cwd: process.cwd(), - stdio: 'pipe', - } - ) + ], + { + cwd: process.cwd(), + stdio: 'pipe', + } + ) - // Wait for dev server to start - await new Promise((r) => setTimeout(r, 8000)) - - // Find the dev server process - const devPid = await new Promise((resolve) => { - const pgrep = spawn('pgrep', ['-f', `one.*dev.*port.*${port}`]) - let output = '' - pgrep.stdout?.on('data', (d) => (output += d.toString())) - pgrep.on('close', () => { - const pid = parseInt(output.trim().split('\n')[0], 10) - resolve(isNaN(pid) ? null : pid) + // Wait for dev server to start + await new Promise((r) => setTimeout(r, 8000)) + + // Find the dev server process + const devPid = await new Promise((resolve) => { + const pgrep = spawn('pgrep', ['-f', `one.*dev.*port.*${port}`]) + let output = '' + pgrep.stdout?.on('data', (d) => (output += d.toString())) + pgrep.on('close', () => { + const pid = parseInt(output.trim().split('\n')[0], 10) + resolve(isNaN(pid) ? null : pid) + }) }) - }) - expect(devPid).toBeTruthy() + expect(devPid).toBeTruthy() - // Kill the wrapper (simulating vitest being killed) - if (wrapper.pid) { - process.kill(wrapper.pid, 'SIGKILL') - } + // Kill the wrapper (simulating vitest being killed) + if (wrapper.pid) { + process.kill(wrapper.pid, 'SIGKILL') + } - // Wait for dev server to detect orphan and exit (should happen within 2 seconds) - await new Promise((r) => setTimeout(r, 2000)) + // Wait for dev server to detect orphan and exit (should happen within 2 seconds) + await new Promise((r) => setTimeout(r, 2000)) - // Dev server should have exited - const stillRunning = devPid ? isRunning(devPid) : false - expect(stillRunning).toBe(false) - }, 20000) + // Dev server should have exited + const stillRunning = devPid ? isRunning(devPid) : false + expect(stillRunning).toBe(false) + }, + 20000 + ) test('dev server does not exit prematurely when parent is alive', async () => { const port = await getPort() - devServer = spawn( - 'node', - ['../../node_modules/.bin/one', 'dev', '--port', port.toString()], - { - cwd: process.cwd(), - stdio: 'pipe', - } - ) + devServer = spawn('node', [oneRunEntry, 'dev', '--port', port.toString()], { + cwd: process.cwd(), + stdio: 'pipe', + // group leader on POSIX so killProcessTree can signal the whole group + detached: process.platform !== 'win32', + }) // Wait for dev server to start await new Promise((r) => setTimeout(r, 6000)) @@ -114,7 +131,154 @@ describe('process cleanup', () => { await new Promise((r) => setTimeout(r, 2000)) expect(isRunning(devServer.pid!)).toBe(true) - // Clean up - devServer.kill('SIGTERM') + // Clean up (tree-kill so dev-server workers don't orphan) + await killProcessTree(devServer.pid) }, 15000) }) + +// Pins the cross-platform tree-kill contract that killProcessTree (./process-tree, +// and packages/test/src/setup.ts) relies on. The mechanisms differ by platform, so +// each gets a positive (the whole tree is reaped) and a negative (a bare single-PID +// kill leaves a worker orphaned — i.e. why a plain process.kill is not enough): +// - Windows: process.kill(pid) is TerminateProcess and does not walk the tree; a +// worker that broke away from libuv's job object (detached) survives it, while +// taskkill /F /T walks the PID tree and reaps it. +// - POSIX: process.kill(pid) signals only the leader; killProcessTree signals the +// whole process group (negative pid), so the workers it forked go too. +describe('process tree termination', () => { + const spawnedPids: number[] = [] + + afterEach(() => { + // safety net: force-kill anything a test spawned, even if it failed mid-way + for (const pid of spawnedPids) { + try { + if (process.platform === 'win32') { + spawn('taskkill', ['/F', '/T', '/PID', String(pid)], { stdio: 'ignore' }) + } else { + try { + process.kill(-pid, 'SIGKILL') + } catch { + // not a group leader + } + try { + process.kill(pid, 'SIGKILL') + } catch { + // already gone + } + } + } catch { + // already gone + } + } + spawnedPids.length = 0 + }) + + // Spawn a server -> worker tree, resolving once the worker PID is known. The + // server is a group leader on POSIX (detached) so killProcessTree's group-kill + // reaches the worker; on Windows detached is irrelevant to taskkill /T. The + // worker is spawned detached only when we want it to escape the parent's job + // object / process group (the case a single-process kill cannot clean up). + function spawnTree( + detachedWorker: boolean + ): Promise<{ server: ChildProcess; workerPid: number }> { + const server = spawn( + 'node', + [ + '-e', + ` + const { spawn } = require('child_process') + const worker = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + detached: ${detachedWorker}, + stdio: 'ignore', + }) + ${detachedWorker ? 'worker.unref()' : ''} + process.stdout.write('WORKER_PID=' + worker.pid + '\\n') + setInterval(() => {}, 1000) + `, + ], + { stdio: ['ignore', 'pipe', 'ignore'], detached: process.platform !== 'win32' } + ) + return new Promise((resolve) => { + let out = '' + server.stdout?.on('data', (d) => { + out += d.toString() + const match = out.match(/WORKER_PID=(\d+)\r?\n/) + if (match) { + const workerPid = Number(match[1]) + if (server.pid) spawnedPids.push(server.pid) + spawnedPids.push(workerPid) + resolve({ server, workerPid }) + } + }) + }) + } + + test.skipIf(process.platform !== 'win32')( + 'killProcessTree reaps a detached worker that escaped the job object (Windows)', + async () => { + const { server, workerPid } = await spawnTree(true) + expect(server.pid).toBeTruthy() + expect(isRunning(server.pid!)).toBe(true) + expect(isRunning(workerPid)).toBe(true) + + await killProcessTree(server.pid) + await new Promise((r) => setTimeout(r, 2000)) + + expect(isRunning(server.pid!)).toBe(false) + expect(isRunning(workerPid)).toBe(false) + }, + 15000 + ) + + test.skipIf(process.platform !== 'win32')( + 'a bare process.kill orphans a detached worker on Windows (why tree-kill is needed)', + async () => { + const { server, workerPid } = await spawnTree(true) + expect(isRunning(workerPid)).toBe(true) + + // TerminateProcess does not walk the tree, and the detached worker broke away + // from the job object — so it survives as an orphan + process.kill(server.pid!, 'SIGKILL') + await new Promise((r) => setTimeout(r, 2000)) + + expect(isRunning(server.pid!)).toBe(false) + expect(isRunning(workerPid)).toBe(true) + // (afterEach reaps the orphaned worker) + }, + 15000 + ) + + test.skipIf(process.platform === 'win32')( + 'killProcessTree reaps an in-group worker (POSIX group-kill)', + async () => { + const { server, workerPid } = await spawnTree(false) + expect(server.pid).toBeTruthy() + expect(isRunning(server.pid!)).toBe(true) + expect(isRunning(workerPid)).toBe(true) + + await killProcessTree(server.pid) + await new Promise((r) => setTimeout(r, 2000)) + + expect(isRunning(server.pid!)).toBe(false) + expect(isRunning(workerPid)).toBe(false) + }, + 15000 + ) + + test.skipIf(process.platform === 'win32')( + 'a bare single-process kill orphans an in-group worker (POSIX, why tree-kill is needed)', + async () => { + const { server, workerPid } = await spawnTree(false) + expect(isRunning(workerPid)).toBe(true) + + // signalling only the leader's PID leaves the rest of the group running + process.kill(server.pid!, 'SIGKILL') + await new Promise((r) => setTimeout(r, 2000)) + + expect(isRunning(server.pid!)).toBe(false) + expect(isRunning(workerPid)).toBe(true) + // (afterEach reaps the orphaned worker) + }, + 15000 + ) +}) diff --git a/tests/test/tests/process-tree.ts b/tests/test/tests/process-tree.ts new file mode 100644 index 000000000..db96d9737 --- /dev/null +++ b/tests/test/tests/process-tree.ts @@ -0,0 +1,58 @@ +import { execSync } from 'node:child_process' + +/** + * Tree-kill a spawned dev server together with the worker processes it forked, + * cross-platform. Shared by the spawn-based tests in this directory so they all + * clean up the same way, using the same tree-kill strategy as `killProcessTree` + * in packages/test/src/setup.ts: on POSIX a graceful group SIGTERM, a brief + * grace period, then a group SIGKILL; on Windows `taskkill /F /T`. + * + * POSIX: signal the whole process group via the negative PID. This requires the + * server to have been spawned with `detached: true` so it leads its own group; + * the workers it forks inherit that group and are signalled too. If the pid is + * not a group leader the negative-pid call throws and we fall back to signalling + * the single process. Pure Node — no shell-out. + * + * Windows: there is no process-group kill (`process.kill(-pid)` throws with + * ESRCH), and `process.kill(pid)` maps to TerminateProcess, which ends only the + * root — a worker that broke away from libuv's job object would be orphaned. + * `taskkill /F /T` walks the PID tree and ends those workers too. (Node has no + * built-in tree-kill, so this one shell-out is unavoidable for a robust result.) + */ +export async function killProcessTree(pid: number | undefined): Promise { + if (!pid) return + + if (process.platform === 'win32') { + try { + execSync(`taskkill /F /T /PID ${pid}`, { stdio: 'ignore' }) + } catch { + // process already gone — fine + } + return + } + + // POSIX: graceful group SIGTERM, a brief grace period, then a group SIGKILL + // fallback. The inner fallbacks signal the single process when the pid is not + // a group leader. + try { + process.kill(-pid, 'SIGTERM') + } catch { + try { + process.kill(pid, 'SIGTERM') + } catch { + // already gone + } + } + + await new Promise((resolve) => setTimeout(resolve, 200)) + + try { + process.kill(-pid, 'SIGKILL') + } catch { + try { + process.kill(pid, 'SIGKILL') + } catch { + // already gone + } + } +}