nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
104.75k stars 28.29k forks source link

test: mark `test-net-write-fully-async-buffer` as flaky #52959

Open aduh95 opened 1 month ago

aduh95 commented 1 month ago

Refs: https://github.com/nodejs/node/issues/47428

lpinca commented 1 month ago

Did we try to investigate? It is not flaky on my machines.

lpinca commented 1 month ago

On macOS, I actually get a 1 ‰ failures when running:

python3 tools/test.py -J --repeat=1000 test/parallel/test-net-write-fully-async-buffer.js

16 ‰ when running

python3 tools/test.py -J --repeat=1000 test/parallel/test-net-write-fully-async-hex-string.js
lpinca commented 1 month ago

Something is fishy here. I get no failures if I remove the common module.

aduh95 commented 1 month ago

Something is fishy here. I get no failures if I remove the common module.

https://github.com/nodejs/node/issues/52964#issuecomment-2106357191 also reported that removing ../common import would remove the flakiness of a different test (i.e. the test no longer times out)

lpinca commented 4 weeks ago

Bisecting point to 1d29d81c69a5e03d99a3d3e597bc0eeed433e47d as the first bad commit.

lpinca commented 4 weeks ago

I think there is a recent bug in V8 or Node.js behind the flakiness of some tests. This is one of those. I think we should not land this.

richardlau commented 4 weeks ago

Something is fishy here. I get no failures if I remove the common module.

#52964 (comment) also reported that removing ../common import would remove the flakiness of a different test (i.e. the test no longer times out)

Maybe something to do with the exit hooks common installs?

lpinca commented 4 weeks ago

@richardlau I tried commenting almost everything in common and failures still occur. Note that, as per git bisect no failures occur from 07f481cfcf1153336534dc5d3cd4c5c9770a72be which is the parent commit of https://github.com/nodejs/node/commit/1d29d81c69a5e03d99a3d3e597bc0eeed433e47d.

richardlau commented 4 weeks ago

I meant that it's possible that the V8 update has caused something to have changed during shutdown. common registers Javascript code to be run prior to Node.js exiting. If you've commented out the registering of those hooks then it's probably something else.

lpinca commented 4 weeks ago

If you are referring to process.on('exit', handler) then yes I've commented them.

lpinca commented 4 weeks ago

The behavior described in https://github.com/nodejs/node/issues/52964#issuecomment-2106317993 also occurs for this test. It correctly finishes, the server emits the 'close' event, but the process does not exit.

lpinca commented 4 weeks ago

Adding the --jitless flag fixes the issue.

nodejs-github-bot commented 6 days ago
Commit Queue failed
- Loading data for nodejs/node/pull/52959
✔  Done loading data for nodejs/node/pull/52959
----------------------------------- PR info ------------------------------------
Title      test: mark `test-net-write-fully-async-buffer` as flaky (#52959)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     aduh95:test-net-write-fully-async-buffer-flaky -> nodejs:main
Labels     test, needs-ci, commit-queue-squash
Commits    2
 - test: mark `test-net-write-fully-async-buffer` as flaky
 - Update test/parallel/parallel.status
Committers 2
 - Antoine du Hamel 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/52959
Refs: https://github.com/nodejs/node/issues/47428
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Ulises Gascón 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/52959
Refs: https://github.com/nodejs/node/issues/47428
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Ulises Gascón 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 12 May 2024 10:10:41 GMT
   ✔  Approvals: 2
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/52959#pullrequestreview-2052433668
   ✔  - Ulises Gascón (@UlisesGascon): https://github.com/nodejs/node/pull/52959#pullrequestreview-2060894926
   ✔  Last GitHub CI successful
   ✘  No Jenkins CI runs detected
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/9424968514