Title: test_runner: flush report stream before force exit by Navaneethp007 · Pull Request #64875 · nodejs/node · GitHub
Open Graph Title: test_runner: flush report stream before force exit by Navaneethp007 · Pull Request #64875 · nodejs/node
X Title: test_runner: flush report stream before force exit by Navaneethp007 · Pull Request #64875 · nodejs/node
Description: Summary Fixes: #64833 When --test-force-exit is used together with process isolation, each test file runs in a child process that streams its results back to the test runner's parent process over its stdout, which is a pipe. Writes to a pipe are asynchronous, so the process.exit() issued on force exit could quit the process while report frames were still buffered in the pipe. The trailing test:pass / test:fail events were silently dropped, yet the process still exited with code 0 — so verdicts disappeared with no error and no failing exit status. The flush loop added in #55099 only drains reporter destinations that expose a close() method (for example file destinations). A pipe has no close(), so that branch resolved immediately without flushing anything, leaving the async pipe writes to be truncated by the exit. This is why the loss only reproduces where the pipe is non-blocking (Linux); on Windows named pipes, TTYs, and file destinations the writes are effectively blocking, so the output looked clean. Fix In the force-exit path, for a destination that has no close() (the child-process pipe case), switch the underlying handle to blocking with _handle.setBlocking(true) — the same idiom Node already uses before exit in lib/net.js and lib/tty.js — then end() the stream and await it, so any queued bytes are flushed to the OS before the process exits. Existing file destinations continue to use the close() path unchanged. Additionally, yield once via setImmediate after tearing down the harness and before process.exit(), so report events that have already arrived on the parent's readable side get dispatched to the reporters before exit. This part follows the approach proposed in #64857 by @NeverBetterEnough; on its own a single tick is probabilistic under concurrency and did not fully eliminate the loss in my testing, but combined with the blocking flush the result is deterministic. Test Adds test/parallel/test-runner-force-exit-no-verdict-loss.js, which generates 12 files of 1000 tests each and runs them with --test-force-exit at --test-concurrency=16, then asserts that all 12000 verdicts are present in the reporter output and the process exits 0. The test fails on main (e.g. 11532 !== 12000) and passes with this change. Existing test-runner-force-exit* tests, including the one from #55099, continue to pass.
Open Graph Description: Summary Fixes: #64833 When --test-force-exit is used together with process isolation, each test file runs in a child process that streams its results back to the test runner's parent process ov...
X Description: Summary Fixes: #64833 When --test-force-exit is used together with process isolation, each test file runs in a child process that streams its results back to the test runner's parent proces...
Opengraph URL: https://github.com/nodejs/node/pull/64875
X: @github
Domain: github.com
| react-profiling | 0 |
| react-import-map | react,react-dom,react-dom/client,react-dom/profiling,react-is,react-reconciler,react/compiler-runtime,react/jsx-dev-runtime,react/jsx-runtime,scheduler@d5750093facf |
| route-pattern | /:user_id/:repository/pull/:id/commits/:range(.:format) |
| route-controller | pull_requests |
| route-action | commits |
| fetch-nonce | v2:d0406d0b-1ab0-d688-7d25-399d8f324e0a |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | C212:28029D:192087:22D209:6AEDAB7E |
| html-safe-nonce | db2b1162b8cbea9e19aaa599472e947f376c085089d0f49867b0b562571ae7eb |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJDMjEyOjI4MDI5RDoxOTIwODc6MjJEMjA5OjZBRURBQjdFIiwidmlzaXRvcl9pZCI6Ijc3OTM4MjA2MjQwMzg1MDUzNDIiLCJyZWdpb25fZWRnZSI6ImlhZCIsInJlZ2lvbl9yZW5kZXIiOiJpYWQifQ== |
| visitor-hmac | 8110d0cc4ccf23d8afb6778756b59a249e59bd0f207321f4a7c06c094bf63bab |
| hovercard-subject-tag | pull_request:4178561715 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/nodejs/node/pull/64875/commits/6a7d3c42fe032ebd0d8450eb0eb228cccaacfa87 |
| twitter:image | https://avatars.githubusercontent.com/u/76899241?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/76899241?s=400&v=4 |
| og:image:alt | Summary Fixes: #64833 When --test-force-exit is used together with process isolation, each test file runs in a child process that streams its results back to the test runner's parent process ov... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 9f51e3b6c7a190e426e641ae15acbc58dd1ac2003e96b58879bbc298e213dbf3 |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/nodejs/node git https://github.com/nodejs/node.git |
| octolytics-dimension-user_id | 9950313 |
| octolytics-dimension-user_login | nodejs |
| octolytics-dimension-repository_id | 27193779 |
| octolytics-dimension-repository_nwo | nodejs/node |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 27193779 |
| octolytics-dimension-repository_network_root_nwo | nodejs/node |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 7060660810b587033ea861b8aa67beb72c8e6542 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width