test(subprocess): assert ordering, not wall-clock, in the starved-pipe test - #352
Open
mtschoen wants to merge 3 commits into
Open
test(subprocess): assert ordering, not wall-clock, in the starved-pipe test#352mtschoen wants to merge 3 commits into
mtschoen wants to merge 3 commits into
Conversation
…e test The starved-pipe test compared clock readings with a fixed cushion: the child had to finish writing its 1MiB output within the first 500ms of a 2000ms busy-spin. That cushion describes how fast a process spawns on an idle machine, not a property of the code under test. Under CI load (concurrent duplicate runs sharing the runner, in this case) the margin evaporates and the test reddens while the subject stays healthy. The test's real invariant is ordering, not duration: the child must finish its whole write while the parent is still starved and draining nothing. Replace the fixed-duration spin and timestamp comparison with a poll that spins until the marker file exists (existsSync, generous 30s deadline) and assert on that boolean. This is load-tolerant, still fails on a real regression (a genuinely pipe-blocked child never writes the marker), and is typically faster since it exits as soon as the marker lands instead of always burning the fixed spin duration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
aislop skipped this PRThis workspace does not have an active paid scanaislop plan. Choose a plan to resume hosted scans and PR gates: This comment appears once per PR. Further pushes will be skipped silently until the workspace has paid access. |
… timeout The internal poll deadline was 30000ms, exactly vitest's global testTimeout, so a genuine regression (marker never written) would be killed by the framework as an opaque timeout instead of failing the ordering assertion. 20000ms leaves real margin for the assertion to fire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
aislop scan: 100/100 (Healthy) +Clean run — no errors, no warnings, no auto-fixable findings. |
mtschoen
marked this pull request as ready for review
August 16, 2026 00:33
Keep the three facts the code cannot show (ordering invariant drives spin-until-marker, existsSync keeps the loop starved, deadline must stay under vitest's testTimeout); drop the change-justification prose that already lives in the PR description. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mtschoen
added a commit
to mtschoen/aislop
that referenced
this pull request
Aug 16, 2026
…ion) into schoen/main # Conflicts: # tests/subprocess.test.ts
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.
Replaces a wall-clock assertion in the starved-pipe backpressure test with an ordering-based one. The old test required the child to finish writing its full 1MiB output within the first 500ms of a fixed 2000ms busy-spin; under CI load that cushion evaporates and the test reddens even though the subject (capturing subprocess output via temp files instead of pipes) is healthy.
The test's actual invariant is that the child finishes its write while the parent is still starved, not that it finishes within some fixed duration. The fix polls existsSync(markerFile) inside the busy loop up to a generous 30s deadline and asserts on that boolean instead of comparing timestamps: a genuinely pipe-blocked child never writes the marker at all, so real regressions still fail, and the loop exits the moment the marker lands so the healthy path stays fast.
Verified: pnpm typecheck clean, pnpm vitest run tests/subprocess.test.ts 14/14 pass.
🤖 Generated with Claude Code