fix(test): spawn one via run.mjs in standalone tests so they run on Windows#722
Open
YevheniiKotyrlo wants to merge 3 commits into
Open
fix(test): spawn one via run.mjs in standalone tests so they run on Windows#722YevheniiKotyrlo wants to merge 3 commits into
YevheniiKotyrlo wants to merge 3 commits into
Conversation
4 tasks
YevheniiKotyrlo
force-pushed
the
fix/tests-bin-one-sweep
branch
from
96b73a4 to
21a38e2
Compare
…indows metro-compiler, metro-startup-order, and process-cleanup spawned `node node_modules/.bin/one` - on Windows the .bin shim is a .cmd/.exe wrapper that node cannot load (MODULE_NOT_FOUND), so the dev server never started and all three suites failed before testing anything. Resolve one's real JS entry via createRequire (the same pattern as packages/test/src/setupTest.ts and the test-warm-routes fix) and kill the spawned tree with taskkill /T on Windows so dev-server workers don't orphan. POSIX kill behavior is unchanged in the metro tests (SIGTERM then delayed SIGKILL, as before); process-cleanup's final cleanup now force-kills on POSIX too, matching what its own afterEach already did. The orphan-detection test is now skipIf(win32): setupOrphanDetection in packages/vxrn/src/exports/dev.ts is POSIX-only by design (returns early on win32) and the test discovers the grandchild via pgrep, which doesn't exist on Windows. Its spawn site is still fixed so the test stays correct where it runs. Also give packages/test/src/setup.ts's killProcessTree a Windows branch - negative-pid process groups don't exist there, so the previous fallback killed only the direct child and orphaned its workers. Validated on Windows 11 (Bun 1.3.13, monorepo at v1.18.0): tests/test goes from 4 failed suites (MODULE_NOT_FOUND) to 30 test files passed / 1 skipped, 145 tests passed / 10 skipped, 0 failures - Metro bundler boots (600ms), the React-compiler bundle assertion and the startup-order assertion both pass. oxlint 0/0, oxfmt clean, tsc --noEmit clean.
… suites The three spawn-based suites (process-cleanup, metro-compiler, metro-startup-order) each carried their own killTree copy, and all three single-killed on POSIX — so a dev server's worker processes were orphaned on teardown (only Windows' taskkill /T walked the tree). Extract one killProcessTree into tests/test/tests/process-tree.ts: POSIX signals the whole process group (negative pid), Windows uses taskkill /F /T. Spawn the dev servers detached on POSIX so the group-kill reaches their workers — mirrors packages/test/src/setupTest.ts. Add cross-platform positive/negative tests that pin the contract: the tree-kill reaps the worker, while a bare single-pid kill leaves it orphaned (the reason the tree-kill exists). Windows uses a detached worker that escapes the job object; POSIX uses an in-group worker. Validated on Windows 11 (process-cleanup 3 passed, metro 2 passed, oxlint/tsc clean) and the POSIX group-kill mechanism on Linux (WSL).
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.
YevheniiKotyrlo
force-pushed
the
fix/tests-bin-one-sweep
branch
from
840feaa to
51526bf
Compare
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
1 participant
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.
Summary
Follow-up to the
test-warm-routesfix (#720): three more standalone test files intests/testspawn the dev server viaspawn('node', ['../../node_modules/.bin/one', …]). On Windows the.binshim is a.cmd/.exewrapper thatnodecan't load (MODULE_NOT_FOUND), so the dev server never starts and all three suites fail before testing anything:metro-compiler.test.tsmetro-startup-order.test.tsprocess-cleanup.test.ts(the spawn lives inside the generated-ewrapper script)Changes
createRequire(import.meta.url).resolve('one/package.json')→run.mjs(same pattern aspackages/test/src/setupTest.ts), correct on every platform.taskkill /F /Ton Windows so dev-server workers don't orphan. POSIX kill behavior is unchanged in the metro tests (SIGTERM → delayed SIGKILL as before);process-cleanup's final cleanup now force-kills on POSIX too, matching what its ownafterEachalready did.process-cleanup's orphan-detection test is nowskipIf(win32):setupOrphanDetection(packages/vxrn/src/exports/dev.ts) is POSIX-only by design (returns early on win32), and the test discovers the grandchild viapgrep, which doesn't exist on Windows. Its spawn site is still fixed so the test stays correct where it runs.packages/test/src/setup.ts'skillProcessTreegets a Windows branch — negative-pid process groups don't exist there, so the previous fallback killed only the direct child and orphaned its workers.Test plan
Windows 11 (Bun 1.3.13, monorepo at v1.18.0):
tests/test— from 4 failed suites (MODULE_NOT_FOUND) to 30 test files passed / 1 skipped, 145 tests passed / 10 skipped, 0 failures. Metro boots (Metro bundler ready (600ms)), the React-compiler bundle assertions and the startup-order assertion pass.oxlint0/0,oxfmt --checkclean,tsc --noEmitclean.run.mjsthe.binsymlink points to; metro-test kill paths unchanged on non-Windows.With this and the warm-routes fix, the integration suite's spawn-class Windows failures are gone — one step closer to a viable
windows-latestCI job.