test: fix flaky test-net-socket-timeout-unref#6003
test: fix flaky test-net-socket-timeout-unref#6003Trott wants to merge 1 commit intonodejs:masterfrom
Conversation
Throw immediately on socket timeout rather than checking boolean in exit handler. Fixes: nodejs#5128
|
Stress test confirming flakiness of current version on master: https://ci.nodejs.org/job/node-stress-single-test/584/nodes=pi2-raspbian-wheezy/console Stress test showing this version is not flaky: https://ci.nodejs.org/job/node-stress-single-test/583/nodes=pi2-raspbian-wheezy/console |
|
@nodejs/testing |
|
LGTM |
| process.on('exit', function() { | ||
| assert.strictEqual(timedout, false, | ||
| 'Socket timeout should not hold loop open'); | ||
| sockets.push({socket: socket, timeout: T * 1000}); |
There was a problem hiding this comment.
should this timout be platform specific?
There was a problem hiding this comment.
This is a socket timeout and not a timer, so I'm inclined to not make it platform dependent if it doesn't need to be.
There was a problem hiding this comment.
ahhh... /me leaves and reads more about socket time outs
|
LGTM with above nit assuming CI is green |
Throw immediately on socket timeout rather than checking boolean in exit handler. PR-URL: nodejs#6003 Fixes: nodejs#5128 Reviewed-By: Myles Borins <[email protected]>
|
Landed in c60faf6 |
Throw immediately on socket timeout rather than checking boolean in exit handler. PR-URL: #6003 Fixes: #5128 Reviewed-By: Myles Borins <[email protected]>
Throw immediately on socket timeout rather than checking boolean in exit handler. PR-URL: #6003 Fixes: #5128 Reviewed-By: Myles Borins <[email protected]>
Pull Request check-list
Please make sure to review and check all of these items:
make -j8 test(UNIX) orvcbuild test nosign(Windows) pass withthis change (including linting)?
Affected core subsystem(s)
test, net
Description of change
Throw immediately on socket timeout rather than checking boolean in exit
handler.
Fixes: #5128