Uh oh!
There was an error while loading. Please reload this page.
test: fix flaky test-net-socket-timeout-unref - #6003
Conversation
Throw immediately on socket timeout rather than checking boolean in exit handler. Fixes: nodejs#5128
Trott
commented
Apr 1, 2016
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 |
Trott
commented
Apr 1, 2016
@nodejs/testing |
santigimeno
commented
Apr 1, 2016
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
MylesBorins
commented
Apr 1, 2016
LGTM with above nit assuming CI is green |
Throw immediately on socket timeout rather than checking boolean in exit handler. PR-URL: nodejs#6003Fixes: nodejs#5128 Reviewed-By: Myles Borins <myles.borins@gmail.com>
Trott
commented
Apr 4, 2016
Landed in c60faf6 |
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