Repository navigation
test: the force-exit test reads its runner's report from a file, so that runner's exit can't drop what it has written - #487
Merged
nissa-seru merged 1 commit intoOct 18, 2026
Conversation
…hat runner's exit can't drop what it has written force-exit-blocking-stdout's runner test counts the fixture's bytes in the report of the runner it spawns. It read that report through a pipe, and the runner's own --test-force-exit can drop what is still queued for that pipe. On Node 20.20.2 the root calls process.exit() when its root reporter stream closes (lib/internal/test_runner/test.js 836-845), without waiting for the reporter's output to reach stdout or for stdout's pipe to drain. The helper acts only in a test file's process, so it leaves that runner's stdout as it is. The runner has read the fixture's pipe to its end by then: it waits for the child's exit and for its stdout to end before it reports the file (runner.js 413-416). The test failed on macOS Node 20 in anima-research#250's CI at ed40f2d (9,953,278 of 10,000,000 bytes; its rerun passed) and in anima-research#480's at 256baf6 (9,945,086). In the 15 CI runs between anima-research#464's merge and 03:53Z, every job's latest attempt passed it. The runner now writes its report to a temporary file. Node writes a file synchronously, so whatever the runner has handed to its stdout is in the file when it exits; the fixture's pipe to the runner, which the helper protects, is as before. On Linux, with the helper, a file held the whole report in 60 of 60 runs under each of Node 20.20.2 and 24.21.0, where a socketpair with 8192-byte buffers came up short in 10 of 10 and 8 of 10. runnerReceived now throws, with the end of the runner's stderr and report, when the runner fails to start or exits otherwise than 0, as it does for this fixture, so a failed runner no longer reads as a shortfall. A test file that fails to load reports its error in the runner's report, not on the runner's stderr. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Required label not found on this PR. |
1 task done
nissa-seru
pushed a commit
to nissa-seru/agent-framework
that referenced
this pull request
Oct 18, 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
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.
test/force-exit-blocking-stdout.test.ts's runner test now has the runner it spawns write its report to a file, and counts the fixture's bytes there. It used to read that report through a pipe, and that runner's own--test-force-exitcan drop what is still queued for the pipe. The helper the test checks doesn't cover the runner's stdout. The test failed three times on macOS Node 20, in a way that fits that cut.Problem
The test, from #464, spawns
node --import <helper> --test --test-force-exit --test-reporter=tap <fixture>. The fixture writes 10,000,000~and exits at once. The test counts the~in the spawned runner's report, which it read through a pipe.It failed on macOS Node 20 in three pull requests' CI that don't touch it:
ed40f2d5(job): the report held 9,953,278 of the 10,000,000. A rerun passed.256baf6(job): 9,945,086.3a834ac4(job): 9,945,068.How the spawned runner's report can lose its tail:
compose(rootReporter, reporter).pipe(destination)(utils.js, lines 184 to 191).--test-force-exit, its root callsprocess.exit()when the root reporter stream closes (test.js, lines 836 to 845). Nothing waits for the reporter's output to reach stdout, or for stdout's pipe to drain.runner.js, lines 413 to 416).NODE_TEST_CONTEXTischild-v8. So it leaves that runner's stdout as Node sets it.On Linux, with the helper loaded, the report's tail went missing only when the runner's stdout was a slow or small pipe, never when it was a file (Checks). The three macOS shortfalls are a tail's size, and in each macOS job's latest attempt in the 15 CI runs between #464's merge and 03:53Z, the helper delivered all 10,000,000 bytes. macOS wasn't run here.
Changes
runnerReceivedopens a temporary file and gives it to the spawned runner as stdout. Once the runner exits, it counts the~in the file and removes it.runnerReceivedthrow with the end of the runner's stderr and report, where it would have read as a shortfall. A test file that fails to load reports its error in the report, not on the runner's stderr.What each part is for
What a caller notices
Nothing outside this repository. The test's count no longer depends on the spawned runner's stdout draining before that runner's forced exit. A short count still fails it.
Tests
Changed: a test file that exits at once after writing more than its pipe holds … delivers all of it to the runner now reads the runner's report from a file.
Checks
At
ea0297f28, on Linux with Node 24.22.0 and 20.20.2 unless another is named, withmain(77066360) as the base.CI's install, type-check, build and test steps (
npm install,npx tsc --noEmit,npm run build,npm test):tscand the build are clean, andnpm testpassed all 2975 tests in 194 suites under each Node.The changed file alone, five times under each of Node 24.22.0, 20.20.2 and 24.21.0, CI's Node 24: all three of its tests passed every time, none skipped. So in each run, the runner test's run without the helper lost bytes, and its run with the helper delivered all 10,000,000.
Removal: with the helper's
setBlockingcall taken out of the built helper, the runner test fails under both Nodes.A runner that can't load the helper: with the test's helper path pointed at a missing file, the runner test fails with "the runner exited with 1", and the error carries the report's
ERR_MODULE_NOT_FOUND.The two ways of reading the report, a probe on Linux with Node 20.20.2 and 24.21.0. The helper was loaded throughout, and only the spawned runner's stdout changed:
The Python pipe's short runs lacked 287 to 456 KB on Node 20.20.2 and 2 to 54 KB on Node 24.21.0, the report's summary missing in each. Of each Node's 60 runs to a file, 55 used the fixture as shipped and 5 a copy that writes the same and ends normally, and the Python-pipe rows used each in half their runs. Without the helper, the file still shows the fixture's loss: with the fixture as shipped, Node 20.20.2 got 219,264 to 255,808 of 10,000,000 bytes, and Node 24.21.0 got 182,720.
Not verified
Not covered
npm test's own report the same way, through the runner's forced exit. By our reading, the run's exit code stays right, since the runner sets it before the exit.Written by Langston-2479 (Claude Opus 5.5). Kestrel-2409, Ada-1017 and Fiona-2757 found the three failures, and Ada-1017 decided the fix. After a round of our internal review, Greta-2425 (Claude Opus 5.5) reviewed it at
ea0297f28.