test: cover crash-path reaping and the relocation guard - #17
Merged
Conversation
The Electron migration deleted a 136-test Rust supervisor suite, including the kernel-level orphan-prevention test and 7 relocation cases, and replaced them with coverage that missed both behaviours entirely. These were the two gaps the migration audit called out as unverified. Crash path (electron/tests/crash-recovery.test.mjs, + two fixtures): The existing reap test drives watchdog.close(), which is the GRACEFUL path — the child reaps from its stdin closing. That is a normal quit, not the reason the crash guard exists. Three tests now cover the abrupt case: - the liveness poll alone reaps a tracked child after its guarded parent is SIGKILLed, with the watchdog's stdin deliberately held open so the reap cannot come from the close-driven path; - a SIGKILLed main process does not orphan its sidecar grandchild, end to end, using the sidecar manager's own spawn options; - the watchdog refuses to kill a tracked pid the OS reports as parented elsewhere, which is the PID-reuse/ownership guard. Two findings from building these, both recorded in the tests: - The fixture child must be spawned DETACHED. With the sidecar manager's own non-detached options on win32, libuv's job object reaps it first, so the test passed with the liveness poll disabled — it was passing for the wrong reason. Detaching puts the child outside that job so only the watchdog can kill it. - The end-to-end test still passes with the poll disabled, and says so: on Windows the guarantee users actually depend on comes from libuv's job object, an undocumented side effect of `detached: false` that this test now pins. Relocation guard (electron/tests/runtime.test.mjs): The existing test only exercised process.platform, and the electron CI job runs windows-latest only, so the macOS and Linux branches were entirely unverified — a wrong path there strands every Mac and Linux user's question bank silently. Added resolution coverage for win32/darwin/linux including the XDG fallback, plus the two Rust cases with no JS equivalent: an unresolvable environment must not throw during boot, and the legacy and current directories must never collide (the same-path branch is unreachable today; the test fails first if a leaf constant is renamed into a collision). Every test was mutation-verified rather than assumed: disabling the liveness poll fails the crash test, and pointing the darwin branch at the Linux layout fails the platform test.
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.
Closes the two verification gaps the Electron migration audit identified. The migration deleted a 136-test Rust supervisor suite — including the kernel-level orphan-prevention test and 7 relocation cases — and the replacement coverage missed both behaviours.
Crash path
The existing reap test drives
watchdog.close(), which is the graceful path: the child reaps because its stdin closed. That's a normal quit, not the reason the crash guard exists. Three new tests cover the abrupt case:Two findings from writing them
The fixture child must be spawned detached. With the sidecar manager's own non-detached options on win32, libuv's job object reaps it first — so the test passed with the liveness poll disabled. It was passing for the wrong reason. Detaching puts the child outside that job so only the watchdog can kill it.
The end-to-end test still passes with the poll disabled, and now says so. On Windows the guarantee users actually depend on comes from libuv's
KILL_ON_JOB_CLOSEjob — an undocumented side effect ofdetached: false. That's worth pinning explicitly rather than letting the test read as watchdog coverage it isn't.Relocation guard
The existing test only exercised
process.platform, and theelectron-runtimeCI job runswindows-latestonly — so the macOS and Linux branches were entirely unverified. A wrong path there silently strands every Mac and Linux user's question bank. Added:XDG_DATA_HOMEfallback and macOS keeping its own layoutAPPDATA/HOME/XDG_DATA_HOME) must not throw — this runs at boot before any window exists, so throwing is an unrecoverable startup crashVerification
Every test was mutation-verified, not assumed:
if (false && !parentIsAlive())) → crash test failsSuite: 60 Electron tests passing (was 55),
npx eslint electron/clean, source files confirmed unmodified after mutation runs.🤖 Generated with Claude Code