Skip to content

Commit 840feaa

Browse files
fix(test): align the shared tree-kill helper with @vxrn/test/setup
process-tree.ts now uses the same tree-kill strategy as killProcessTree in packages/test/src/setup.ts: execSync taskkill /F /T on Windows (deterministic, was fire-and-forget spawn) and an async group SIGTERM -> 200ms grace -> group SIGKILL on POSIX (was a detached 1000ms setTimeout). The helper is async so the dev-server suites await deterministic cleanup, and the 'mirrors setup.ts' doc comment is now accurate. Validated on Windows 11 and WSL (Linux node 24.3): tree-termination tests pass on both; typecheck, oxlint, oxfmt clean.
1 parent 60e8a13 commit 840feaa

4 files changed

Lines changed: 27 additions & 24 deletions

File tree

tests/test/tests/metro-compiler.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ const oneRunEntry = join(
3232
describe('Metro React compiler', { retry: 1 }, () => {
3333
let devServer: ChildProcess | null = null
3434

35-
afterEach(() => {
35+
afterEach(async () => {
3636
if (devServer) {
37-
killProcessTree(devServer.pid)
37+
await killProcessTree(devServer.pid)
3838
devServer = null
3939
}
4040
})

tests/test/tests/metro-startup-order.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ describe('Metro startup order', () => {
9191
})
9292

9393
// Kill the server (tree-kill: taskkill /T on Windows, process-group on POSIX)
94-
killProcessTree(devServer.pid)
94+
await killProcessTree(devServer.pid)
9595
await new Promise((r) => setTimeout(r, 500))
9696

9797
// Verify the order - Vite should be ready BEFORE Metro

tests/test/tests/process-cleanup.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ describe('process cleanup', () => {
3939
let devServer: ChildProcess | null = null
4040
let wrapper: ChildProcess | null = null
4141

42-
afterEach(() => {
42+
afterEach(async () => {
4343
// Clean up any leftover processes (tree-kill so dev-server workers don't orphan)
4444
if (devServer?.pid && isRunning(devServer.pid)) {
45-
killProcessTree(devServer.pid)
45+
await killProcessTree(devServer.pid)
4646
}
4747
if (wrapper?.pid && isRunning(wrapper.pid)) {
48-
killProcessTree(wrapper.pid)
48+
await killProcessTree(wrapper.pid)
4949
}
5050
})
5151

@@ -132,7 +132,7 @@ describe('process cleanup', () => {
132132
expect(isRunning(devServer.pid!)).toBe(true)
133133

134134
// Clean up (tree-kill so dev-server workers don't orphan)
135-
killProcessTree(devServer.pid)
135+
await killProcessTree(devServer.pid)
136136
}, 15000)
137137
})
138138

@@ -221,7 +221,7 @@ describe('process tree termination', () => {
221221
expect(isRunning(server.pid!)).toBe(true)
222222
expect(isRunning(workerPid)).toBe(true)
223223

224-
killProcessTree(server.pid)
224+
await killProcessTree(server.pid)
225225
await new Promise((r) => setTimeout(r, 2000))
226226

227227
expect(isRunning(server.pid!)).toBe(false)
@@ -256,7 +256,7 @@ describe('process tree termination', () => {
256256
expect(isRunning(server.pid!)).toBe(true)
257257
expect(isRunning(workerPid)).toBe(true)
258258

259-
killProcessTree(server.pid)
259+
await killProcessTree(server.pid)
260260
await new Promise((r) => setTimeout(r, 2000))
261261

262262
expect(isRunning(server.pid!)).toBe(false)

tests/test/tests/process-tree.ts

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import { spawn } from 'node:child_process'
1+
import { execSync } from 'node:child_process'
22

33
/**
44
* Tree-kill a spawned dev server together with the worker processes it forked,
55
* cross-platform. Shared by the spawn-based tests in this directory so they all
6-
* clean up the same (correct) way — mirrors `killProcessTree` in
7-
* packages/test/src/setup.ts.
6+
* clean up the same way, using the same tree-kill strategy as `killProcessTree`
7+
* in packages/test/src/setup.ts: on POSIX a graceful group SIGTERM, a brief
8+
* grace period, then a group SIGKILL; on Windows `taskkill /F /T`.
89
*
910
* POSIX: signal the whole process group via the negative PID. This requires the
1011
* server to have been spawned with `detached: true` so it leads its own group;
@@ -18,20 +19,21 @@ import { spawn } from 'node:child_process'
1819
* `taskkill /F /T` walks the PID tree and ends those workers too. (Node has no
1920
* built-in tree-kill, so this one shell-out is unavoidable for a robust result.)
2021
*/
21-
export function killProcessTree(pid: number | undefined): void {
22+
export async function killProcessTree(pid: number | undefined): Promise<void> {
2223
if (!pid) return
2324

2425
if (process.platform === 'win32') {
2526
try {
26-
spawn('taskkill', ['/F', '/T', '/PID', String(pid)], { stdio: 'ignore' })
27+
execSync(`taskkill /F /T /PID ${pid}`, { stdio: 'ignore' })
2728
} catch {
2829
// process already gone — fine
2930
}
3031
return
3132
}
3233

33-
// POSIX: graceful group SIGTERM, then a group SIGKILL fallback. The inner
34-
// fallbacks signal the single process when the pid is not a group leader.
34+
// POSIX: graceful group SIGTERM, a brief grace period, then a group SIGKILL
35+
// fallback. The inner fallbacks signal the single process when the pid is not
36+
// a group leader.
3537
try {
3638
process.kill(-pid, 'SIGTERM')
3739
} catch {
@@ -41,15 +43,16 @@ export function killProcessTree(pid: number | undefined): void {
4143
// already gone
4244
}
4345
}
44-
setTimeout(() => {
46+
47+
await new Promise((resolve) => setTimeout(resolve, 200))
48+
49+
try {
50+
process.kill(-pid, 'SIGKILL')
51+
} catch {
4552
try {
46-
process.kill(-pid, 'SIGKILL')
53+
process.kill(pid, 'SIGKILL')
4754
} catch {
48-
try {
49-
process.kill(pid, 'SIGKILL')
50-
} catch {
51-
// already gone
52-
}
55+
// already gone
5356
}
54-
}, 1000)
57+
}
5558
}

0 commit comments

Comments
 (0)