test: fix `net-socket-timeout-unref` flakiness by santigimeno · Pull Request #4772 · nodejs/node · GitHub

From time to time this test is failing in OS X because at least one of
the connections takes quite a long time (around 5 seconds) causing some
of the timers may fire before the test exited. To solve this, wait for
all the connections to be established before setting the timeouts and
unrefing the sockets.

The error I'm getting is

=== release test-net-socket-timeout-unref ===                                  
Path: parallel/test-net-socket-timeout-unref
[]
[]
[]
[]
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: Socket timeout should not hold loop open
    at process.<anonymous> (/Users/sgimeno/node/node/test/parallel/test-net-socket-timeout-unref.js:26:10)
    at emitOne (events.js:96:20)
    at process.emit (events.js:183:7)

I don't know if it's a legit issue or just a problem in my OS X machine as it seems strange that some connection takes that long, but I have seen this error appear relatively frequently when running the test suite and I can reproduce it more easily by running multiple copies of this test in parallel.

/cc @Trott @nodejs/testing

@santigimeno saw it also on my OS X machine, but it's really really rare.

CI: https://ci.nodejs.org/job/node-test-pull-request/1458/

Just had this test fail on me in CI for the first time I can remember:

https://ci.nodejs.org/job/node-test-binary-arm/926/RUN_SUBSET=0,nodes=pi1-raspbian-wheezy/console

LGTM and CI is green. Any thoughts or concerns @jasnell @romankl @nodejs/testing ? I'd like at least one more LGTM besides my own.

Can you remove the whitespace around the braces.

Enable `space-unary-ops` in `.eslintrc`. This prohibits things like: i ++ // use `i++` instead typeof(foo) // use `typeof foo` or `typeof (foo)` instead Ref: nodejs#4772 (comment)

PR update. Thanks for the review.

Can you remove the hard coded 6. Maybe use the array length.

One comment. LGTM if the CI is happy.

From time to time this test is failing in OS X because at least one of the connections takes quite a long time (around 5 seconds) causing some of the timers may fire before the test exited. To solve this, wait for all the connections to be established before setting the timeouts and unrefing the sockets.

Sorry, I forgot about that. Done

CI: https://ci.nodejs.org/job/node-test-pull-request/1537/

LGTM

From time to time this test is failing in OS X because at least one of the connections takes quite a long time (around 5 seconds) causing some of the timers may fire before the test exited. To solve this, wait for all the connections to be established before setting the timeouts and unrefing the sockets. PR-URL: #4772 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Landed in 538813c

Enable `space-unary-ops` in `.eslintrc`. This prohibits things like: i ++ // use `i++` instead typeof(foo) // use `typeof foo` or `typeof (foo)` instead Ref: #4772 (comment) PR-URL: #5063 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com>