test: remove 1 second delay from test #4616
Conversation
Trott
added
cluster
|
Booooo! Blowing up on SmartOS. Still think this is workable. Will revise. Closing for now. Might re-open. |
|
OK, pushed a modification, and this is looking much better. https://ci.nodejs.org/job/node-test-commit/1684/ |
|
As a side note, this test is flaky, at least on FreeBSD.
(One of the motivations for this is that one additional second per test run starts to add up when you're doing 10K test runs.) I have a subsequent modification to tack on to this one that will hopefully fix the flakiness. Stress test currently running at https://ci.nodejs.org/job/node-stress-single-test/333/nodes=freebsd102-64/console. Assuming it works out, I can lump it in with this PR or keep it as a separate PR. Doesn't really matter either way to me. |
|
Stress test for the additional change is looking great. Going to cherry-pick it into this PR... |
|
Cherry-picked. New CI: https://ci.nodejs.org/job/node-test-pull-request/1190/ |
|
R=@jbergstroem (This is one of the tests that failed in one of the CI runs for the make-it-all-parallel PR you have open.) |
|
Cool. Looks green indeed. Lets wait for the last arm bot to finish (just got it back online) - but LGTM. |
|
LGTM if CI is green |
|
Hmmm...the stress test failed, albeit with a bit less frequency than the previous version of the test. So, it's better, but it's still not there yet. More work to be done, alas. |
|
OK, revised to get rid of flakiness (again).
|
|
Afaik there are no known infra issues. That's too many fails to just have one bot act up as well. |
|
@jbergstroem Yup. Looks like it's a result of #4575. Looks like it got merged even though there were test failures. At least, that's what I'm seeing. Maybe I'm missing something. (I've certainly been known to be wrong on my what-went-wrong diagnosis in these situations.) |
|
Stress test came out green. Since I made some changes, can I get a reaffirmation of the (Yes, I'll make sure to squash the four commits down to one before landing.) |
|
LGTM |
|
Still LGTM |
In test-cluster-worker-wait-server-close, remove unneeded 1-second delay and refactor to eliminate flakiness on FreeBSD. PR-URL: nodejs#4616 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in e98bcfa |
In test-cluster-worker-wait-server-close, remove unneeded 1-second delay and refactor to eliminate flakiness on FreeBSD. PR-URL: #4616 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com>
In test-cluster-worker-wait-server-close, remove unneeded 1-second delay and refactor to eliminate flakiness on FreeBSD. PR-URL: #4616 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com>
In test-cluster-worker-wait-server-close, remove unneeded 1-second delay and refactor to eliminate flakiness on FreeBSD. PR-URL: #4616 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com>
In test-cluster-worker-wait-server-close, remove unneeded 1-second delay and refactor to eliminate flakiness on FreeBSD. PR-URL: nodejs#4616 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com>
In test-cluster-worker-wait-server-close, remove unneeded 1-second delay and refactor to eliminate flakiness on FreeBSD. PR-URL: nodejs#4616 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com>
In test-cluster-worker-wait-server-close, remove unneeded 1-second delay and refactor to eliminate flakiness on FreeBSD. PR-URL: nodejs#4616 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com>
Remove unnecessary 1-second delay from test. Confirmed that test (with changes to eliminate ES6-isms in common.js) still fails in v1.8.1 (which has the bug this test was written for) and passes in v2.3.4 (which contains the fix).
R=@sam-github