Stop two families of flaky test - #142
Merged
Merged
Conversation
Twelve tests slept for a fixed period and then asserted what an AsyncStream subscriber had collected. The sleep was sizing how long delivery was expected to take, which is a guess in both directions: too short and a loaded runner fails a test that is merely late, too long and every run pays for the worst case. Several asserted exact contents, so late delivery failed as wrongness. The two coalescing tests were a different shape: the sleep sized an overlap window, holding that the first command was still in flight when the duplicate arrived. TestGate holds the runner open until the test lets it go. The recording centre logs the duplicate submission before it coalesces, so that half is observable too; the plain centre offers nothing equivalent, and its remaining window is between the second task signalling and its call reaching the actor, with the first command held. The two cleanup tests assert an absence, which polling cannot express. They await the cancelled task so termination is done rather than assumed, and a live witness subscriber observes the broadcast. Verified by mutation: dropping the output broadcast fails these tests rather than passing vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Darwin discards whatever is still queued on a pty when the last replica descriptor closes. Writing three bytes into the replica and closing it leaves the primary with nothing to read, so this is the device, not anything about the child or the spawn. The previous fix moved that last close into this process: the parent now holds the replica for the whole run and closes it once the child has exited. That put the destructive close immediately after a loop that can exit with bytes still queued. A poll times out, the child writes and exits before isRunning is read, the loop breaks without polling again, and closeReplica destroys the three bytes. An empty transcript, an exit status of 0 and a clean end of input follow, which is the failure CI reported twice. Reading the terminal dry once the child is reaped closes the window. Nothing further can arrive after waitpid returns, so what is queued at that point is all there will ever be. Stalling the gap between the poll timeout and the isRunning check reproduces the failure on demand: 2ms loses output on a quarter of runs and 60ms on all of them, both with the reported signature. With the drain in place every stall passes. The runner is unaffected. Its drain is parked on the primary from before the spawn, so it is woken while the child still holds the replica open. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015NMRuMKDdQZg32DHwxdMDP
graeme
changed the title
MikeMcQuaid
approved these changes
Sep 2, 2026
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
2 participants
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.
PR: Stop two families of flaky test
Summary
Two unrelated flakes, both in test timing. Twelve command centre tests slept and then asserted what a stream subscriber had collected. The pty suite lost the child's output to a kernel behaviour the previous fix walked into.
Changes
Waiting instead of sleeping. The sleep was sizing how long delivery should take, a guess in both directions: too short and a loaded runner fails a test that is merely late, too long and every run pays the worst case. Sixteen sleeps across two files now poll the condition through
waitUntil. The two coalescing tests sized an overlap window rather than a delay, soTestGateholds the runner open until the test lets it go. The two cleanup tests assert an absence, so they await the cancelled task and add a live witness subscriber to wait on.Reading the terminal dry. Darwin discards whatever is still queued on a pty when the last replica descriptor closes; writing three bytes and closing leaves the primary with nothing to read. The earlier fix moved that close into the test process, right after a loop that can exit with bytes still queued: a poll times out, the child writes and exits before
isRunningis read, the loop breaks without polling again, and the close destroys the output. The suite now reads the terminal dry once the child is reaped, before dropping the replica.Testing
scripts/test: full package suite green.isRunningcheck: 2ms loses output on a quarter of runs, 60ms on all of them, both matching the signature CI reported. Every stall passes with the drain in place.PR checklist
waitUntilsignature was specified by hand.