nodejs
deps: update libuv to 1.51.0
Failed to start CI
⚠ No approving reviews found ✘ Refusing to run CI on potentially unsafe PRhttps://github.com/nodejs/node/actions/runs/14799822670
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Failures in test-child-process-stdout-flush-exit.js are appearing now after libuv has unified the size of the child process stdio buffer in unices (See PR: https://github.com/libuv/libuv/pull/4694 and discussion in https://github.com/libuv/libuv/issues/4768). The test seems wrong as it assumes that all the writes to stdout will complete synchronously though, as stated in https://nodejs.org/api/process.html#processexitcode, it doesn't have to:
[...] The reason this is problematic is because writes to process.stdout in Node.js are sometimes asynchronous and may occur over multiple ticks of the Node.js event loop. Calling process.exit(), however, forces the process to exit before those additional writes to stdout can be performed.
@santigimeno How does a4218eea555fa23acd1c1ff60c8a6832a2e481cf sound to you? I've added a comment to explain the test (and hopefully through that why it's okay to change the test), I guess it does raise the question a bit whether we want this to be semver-major or not?
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 90.17%. Comparing base (
6710c00) to head (aece202). Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #58124 +/- ##
==========================================
- Coverage 90.18% 90.17% -0.01%
==========================================
Files 629 629
Lines 186631 186631
Branches 36656 36659 +3
==========================================
- Hits 168310 168303 -7
- Misses 11124 11126 +2
- Partials 7197 7202 +5
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
@santigimeno How does a4218ee sound to you? I've added a comment to explain the test (and hopefully through that why it's okay to change the test), I guess it does raise the question a bit whether we want this to be semver-major or not?
The change looks good to me though I also wonder about the "semverness" of the change: it may break people but on the other hand the documentation is clear that the writes are not guaranteed to be completed synchronously :man_shrugging: . If we don't want to risk it I might open a PR in libuv making the change only effective in macos as I proposed in the discussion, and see how it goes.
Let's see what CI is saying now. I assume we can also still just run CITGM on PRs like this?
Jenkins CI is passing, but GHA is not, failing consistently on sequential/test-watch-mode 🤔 let me try to rebase to see if that fixes anything
Failed to start CI
⚠ Commits were pushed since the last approving review: ⚠ - deps: update libuv to 1.51.0 ✘ Refusing to run CI on potentially unsafe PRhttps://github.com/nodejs/node/actions/runs/15046175159
