Repository navigation
Drive the quit sweep over all four child registries - #163
Merged
Merged
Conversation
The 'quitting sweeps running children through kill-tree' test only started an npm:install, so runningScripts, playgroundServers and playgroundWebServer were empty when it quit — their spread lines in the before-quit sweep swept nothing and deleting any of them kept the suite green. Start a script, a per-site Playground server and the shared web server as well as an install, quit once, and assert kill-tree receives every child exactly once and child.kill() none of them. Verified the test fails for the right reason against each of the four mutations the issue names: deleting the runningScripts, playgroundServers or playgroundWebServer spread, and swapping killChildTree(child) for child.kill(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
juanmaguitar
force-pushed
the
juanmaguitar/issue-160-quit-sweep
branch
from
August 7, 2026 06:21
a4211c6 to
b7799cd
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
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 #160.
The quit sweep ends children from four registries and routes each through the tree kill (#83). Its test only started an install, so the other three registries were empty and the lines sweeping them contributed nothing to the assertion — the test could not tell the difference between that code being present and being deleted.
It now starts an install, a script, a per-site Playground server and the shared web server, quits once, and asserts the tree kill received every child exactly once and that the direct kill received none. The assertion is order-independent, so it does not fossilise the order of the registries in the sweep.
Test-only; the sweep itself was already correct.
Mutations killed
Verified by applying each mutation to the sweep and confirming the test goes red, and that it is green on trunk:
Worth naming the asymmetry this closes: stopping a server from the UI was wired through the tested modules and covered in #146, but quitting is a different path, so a contributor who closes the window rather than pressing stop was in the gap.
Review
.github/instructions/code-review.instructions.mdrun against the branch: 1 finding, fixed here — the first version asserted the children in a fixed order, which would have failed for a harmless reordering of the sweep.npm testandnpm run test:electronboth 219/219, lint clean.