test(the-framework): make the re-home failure say why#1166
Merged
Conversation
The CI-only failure of this test is a bare `1 !== 2`, which says only that the daemon did not spawn the continued run. The reason is already on disk: a failed re-home is recorded as an event in the scratch log, and the scratch is retained rather than removed. Read both when the second spawn is missing, so the next occurrence names the cause instead of costing another round of guessing. The passing case takes ~370ms, so a run that burns the whole poll budget has not been slow. Part of #1165
suleimansh
added
bug
suleimansh
marked this pull request as ready for review
July 25, 2026 14:19
This was referenced Jul 25, 2026
suleimansh
added a commit
that referenced
this pull request
Jul 25, 2026
The #1122 re-home test seeded the bind the instant `onStart` returned, so the re-home it triggers could terminate a child that had not finished booting. On a loaded runner that SIGTERM landed before the stub recorded its argv, and the run looked like it never spawned: a bare `1 !== 2` after the whole poll budget. A real bind cannot arrive that early. The run writes it into its own event log (`recordBind`, cli.ts), so a bind the daemon sees always comes from a process that is already running. The test now waits for that start before binding. Also: the stub installs its SIGTERM handler before its first write, so a signal in between cannot take the default disposition and kill it silently; the #1166 diagnostic prints the recorded starts, since "which spawn is missing" was the first question it could not answer; and the poll ceiling drops to 10s, which is a backstop rather than a slowness allowance -- 6s, 30s and 15s all failed the same way, because the re-home either lands in under a second or has gone wrong. Closes #1165
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.
Part of #1165.
The CI-only failure of the #1122 re-home test is a bare
1 !== 2. The reason is already on disk — a failed re-home recordscould not re-home this run: <reason>in the scratch events log and retains the scratch — so the test now reads both when the second spawn is missing.Why this rather than another budget bump: the passing case takes 367ms and every failure burns the entire poll budget (6s, then 15s, then 30s at which point the file hit the 60s per-test cap). It is binary — the re-home happens at once or not at all — so no budget will ever fix it, and my earlier #1153/#1154 framing of it as a slow runner was wrong.
1286 tests green locally. Test-only, no changeset.