test: fix fork-worker process-listener leaks behind the windows CI flake (#159)#184
Merged
Merged
Conversation
…159) A vitest fork worker on windows-latest occasionally died before flushing its results, failing a fully green suite with "[vitest-pool]: Worker exited unexpectedly". Attribution via summary counts: the dying file was tests/unit/proxy/dap-proxy-core.test.ts, whose setupGlobalErrorHandlers describe attached four REAL exit-bearing listeners to the worker process (call-through process.on spy) and never removed them. - dap-proxy-core.test.ts: capture handlers via mockImplementation instead of attaching; file-wide process.exit mock net (start() arms a real 10s exit(1) timeout); drop inline exitSpy.mockRestore() calls so the net survives until restoreAllMocks. - dap-proxy-dependencies.test.ts: settle the async shutdownFn().finally(() => process.exit(1)) chain before restoring the exit spy (its intercepted exit(1) appears in the failed run's log); assert the exit. - stdio-command.test.ts: capture process.on without attaching; drain the 60s keepAlive interval via the captured SIGTERM handler; fake stdin everywhere. - vitest.setup.ts: per-fork process-listener leak guard - baselines rawListeners for 8 critical events, removes + reports leaks after each test with test/file attribution; LEAK_GUARD_STRICT=1 makes it fail. - CI attribution: json reporter on CI (survives fork death) and test-results.json in the windows-test-diagnostics artifact. - Delete dead tests/unit/proxy/proxy-manager-test-setup.ts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #159.
Root cause
The failed run (28960586768 attempt 1) reported
157 passed | 2 skipped (160)files and2462 passed | 5 skipped (2505)tests — exactly one file with exactly 38 tests never reported. The only 38-test file in the suite istests/unit/proxy/dap-proxy-core.test.ts: its fork worker died before flushing results (unit project =pool: 'forks',isolate: true).Why the fork died: the "registers handlers…" test spied
process.onwithoutmockImplementation, so the spy called through andsetupGlobalErrorHandlers()attached 4 real listeners (uncaughtException,unhandledRejection,SIGTERM,SIGINT) to the vitest fork's process.afterEachonly restored the spy — the listeners were never removed. The leakeduncaughtExceptionhandler runserrorShutdown().finally(() => process.exit(1)); aftervi.resetAllMocks(), the capturedshutdownFnreturnsundefined, so the handler either calls the realprocess.exit(1)or throws (.finallyofundefined) — and a throw inside anuncaughtExceptionhandler is a fatal Node exit. Either way:[vitest-pool]: Worker exited unexpectedly, results lost, red CI over a green suite.The same bug class was caught red-handed in the failed run's log for a second file —
tests/unit/proxy/dap-proxy-dependencies.test.tsinvokes theunhandledRejectionhandler (which schedulesshutdownFn().finally(() => process.exit(1))on the microtask queue) and restored the exit spy synchronously before it fired:An audit found a third offender:
tests/unit/cli/stdio-command.test.tsleaks realSIGTERM/SIGINT/exitlisteners plus a 60skeepAliveinterval per successful-start test.Fix (no retries, no masking)
tests/vitest.setup.ts— process-listener leak guard: baselinesrawListenersfor 8 critical events per fork, and after each test removes + loudly reports ([process-listener-leak], with test name + file) anything not in the baseline.LEAK_GUARD_STRICT=1upgrades to a hard failure. Diagnostics handlers now include the test file path.tests/unit/proxy/dap-proxy-core.test.ts— capture handlers viamockImplementation(never attach); file-wideprocess.exitmock net (ProxyRunner.start()arms a real 10ssetTimeout(() => process.exit(1))); inlineexitSpy.mockRestore()calls removed so the net survives torestoreAllMocks.tests/unit/proxy/dap-proxy-dependencies.test.ts— settle the async exit chain before restoring the spy; assertshutdownFn+exit(1)(previously unasserted).tests/unit/cli/stdio-command.test.ts— captureprocess.onwithout attaching; drain the keepAlive interval via the captured SIGTERM handler; fake stdin everywhere (no moreresume()/'end'listener on the fork's real stdin).tests/unit/proxy/proxy-manager-test-setup.ts— deleted (dead code; would have nuked vitest's ownunhandledRejectionhandlers if ever imported).jsonreporter enabled on CI (written by the main process, so it survives fork death and records exactly which file never reported);test-results.jsonadded to thewindows-test-diagnosticsartifact.Failing-first proof
Guard landed before the test fixes flags exactly the killer (and the third offender):
After the fixes, the same commands run clean under
LEAK_GUARD_STRICT=1.Verification
LEAK_GUARD_STRICT=1 vitest run --project uniton all three fixed files — green, zero leak messagespnpm run test:ci-coverage(same command as the failing CI job) — green; full-output sweep: 0[process-listener-leak]messages, 0process.exit unexpectedly calledmessages (previously present), and the summary counts add up exactly (161+2 = 163files,2513+5 = 2518tests — no lost workers)LEAK_GUARD_STRICT=1— 30/30 greennode scripts/flake-hunt.mjs 3(within-file shuffle, fresh seeds) — 3/3 greenProxyRunner/setupGlobalErrorHandlers/stdio-command🤖 Generated with Claude Code