test: move test-worker-eventlooputil to sequential - #35996
Conversation
Sorry, something went wrong.
The test is not nearly as unreliable as it used to be but we're still seeing failures around the timing checks that will definitely be affected by other tests running in other processes. So move it to sequential. Refs: nodejs#35961 (comment)
|
Here's what the very occasional failure looks like today: 22:31:18 not ok 2552 parallel/test-worker-eventlooputil # TODO : Fix flaky test
22:31:18 ---
22:31:18 duration_ms: 0.990
22:31:18 severity: flaky
22:31:18 exitcode: 1
22:31:18 stack: |-
22:31:18 node:assert:385
22:31:18 throw err;
22:31:18 ^
22:31:18
22:31:18 AssertionError [ERR_ASSERTION]: 0 < 25
22:31:18 at Timeout.<anonymous> (/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/test/parallel/test-worker-eventlooputil.js:90:12)
22:31:18 at Timeout._onTimeout (/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/test/common/index.js:377:15)
22:31:18 at listOnTimeout (node:internal/timers:555:17)
22:31:18 at processTimers (node:internal/timers:498:7) {
22:31:18 generatedMessage: false,
22:31:18 code: 'ERR_ASSERTION',
22:31:18 actual: false,
22:31:18 expected: true,
22:31:18 operator: '=='
22:31:18 }
22:31:18 ...
|
Sorry, something went wrong.
This comment has been minimized.
|
I think the test could be fixed by using a larger timeout, or maybe by adding some more logic to ensure that worker already reached the waitForNext event place in libuv after posting the return message. Do you think it's worth to invest some time in this or is it fine to keep the test in sequential forever? |
Sorry, something went wrong.
Using a larger timeout? I'd rather just move to sequential because the magic numbers in parallel come back to bite us in surprising ways sooner or later. Adding some more logic to ensure that worker already reached the waitForNext event place in libuv after posting the return message? That sounds more robust/correct to me. Whether or not it's worth it depends on how satisfied you will feel to have done that. 😄 |
Sorry, something went wrong.
|
FWIW, here's what I had to do to see the test fail more than once or twice in a stress test: https://ci.nodejs.org/job/node-stress-single-test/200/ 12 failures in 1000 runs. The key was -j 8 --repeat=100 test/parallel/test-worker-eventlooputil.js. EDIT: This was on an osx1015 host in CI. |
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong.
The test is not nearly as unreliable as it used to be but we're still seeing failures around the timing checks that will definitely be affected by other tests running in other processes. So move it to sequential. Refs: #35961 (comment) PR-URL: #35996 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
The test is not nearly as unreliable as it used to be but we're still seeing failures around the timing checks that will definitely be affected by other tests running in other processes. So move it to sequential. Refs: #35961 (comment) PR-URL: #35996 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
The test is not nearly as unreliable as it used to be but we're still seeing failures around the timing checks that will definitely be affected by other tests running in other processes. So move it to sequential. Refs: nodejs/node#35961 (comment) PR-URL: nodejs/node#35996 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
The test is not nearly as unreliable as it used to be but we're still
seeing failures around the timing checks that will definitely be
affected by other tests running in other processes. So move it to
sequential.
Checklist