test: move net bytes-per-chunk test to parallel #21322
Conversation
This was added in 3217e8e as a regression test for a security patch. We moved it to `sequential` to lower the risk of creating a flaky test, because an earlier version of it was failing one some platforms. There is no known reason why te test should be flaky in this form, though, and moving it to parallel would be good because it does take around 3 seconds that would otherwise fully count towards the test run time.
There was a problem hiding this comment.
This test is unusually resource-intensive and times out for me locally with tools/test.py -j 40 --repeat 80 test/parallel/test-net-bytes-per-incoming-chunk-overhead.js . (I usually try to get -j up to 96 and --repeat to 192 before I move something to parallel but I have no basis for that other than my own experience. Empirical? Anecdotal? Arbitrary? All of the above?)
That said, my local machine is not CI and running 40 copies of the test is not what CI will do. So I'm fine with moving this to parallel. It can always be fast-track moved back to sequential if we start seeing it time out.
addaleax
added
the
author ready
|
Landed in d362b07 |
This was added in 3217e8e as a regression test for a security patch. We moved it to `sequential` to lower the risk of creating a flaky test, because an earlier version of it was failing one some platforms. There is no known reason why te test should be flaky in this form, though, and moving it to parallel would be good because it does take around 3 seconds that would otherwise fully count towards the test run time. PR-URL: #21322 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
|
@addaleax Unfortunately, this test timed out on FreeBSD today. https://ci.nodejs.org/job/node-test-commit-freebsd/18625/nodes=freebsd10-64/console 15:52:56 not ok 1288 parallel/test-net-bytes-per-incoming-chunk-overhead
15:52:56 ---
15:52:56 duration_ms: 120.210
15:52:56 severity: fail
15:52:56 exitcode: -15
15:52:56 stack: |-
15:52:56 timeout
15:52:56 ... |
|
Even when it passes, it is dangerously close to the 120 second limit on FreeBSD. https://ci.nodejs.org/job/node-test-commit-freebsd/18623/nodes=freebsd10-64/console https://ci.nodejs.org/job/node-test-commit-freebsd/18619/nodes=freebsd10-64/console For comparison, when it was in sequential instead of parallel: |
|
Hmmm...seems like something worth looking at on FreeBSD that it takes 52 seconds even in |
The test is timing out on FreeBSD 10 in CI. It takes less than half as long to run when it is in sequential on that platform instead of parallel. Refs: nodejs#21322 (comment)
This was added in 3217e8e as a regression test for a security patch. We moved it to `sequential` to lower the risk of creating a flaky test, because an earlier version of it was failing one some platforms. There is no known reason why te test should be flaky in this form, though, and moving it to parallel would be good because it does take around 3 seconds that would otherwise fully count towards the test run time. PR-URL: #21322 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
The test is timing out on FreeBSD 10 in CI. It takes less than half as long to run when it is in sequential on that platform instead of parallel. Refs: #21322 (comment) PR-URL: #21457 Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
The test is timing out on FreeBSD 10 in CI. It takes less than half as long to run when it is in sequential on that platform instead of parallel. Refs: #21322 (comment) PR-URL: #21457 Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
This was added in 3217e8e as
a regression test for a security patch. We moved it to
sequentialto lower the risk of creating a flaky test,because an earlier version of it was failing one some platforms.
There is no known reason why te test should be flaky in this form,
though, and moving it to parallel would be good because it does take
around 3 seconds that would otherwise fully count towards the
test run time.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes