test: eval mode net.listen should emit listening #1581
Conversation
mscdex
added
test
| ]; | ||
|
|
||
| // should exit cleanly almost immediately | ||
| cp.execFile(process.execPath, args, {timeout: 500}, assert.ifError); |
There was a problem hiding this comment.
Not to be that guy but was the 500 timeout randomly picked? We want each test to exit as fast as possible so if this can be reduced, that'd be great - if not, thats okay too, thoughts?
There was a problem hiding this comment.
It was more or less arbitrary. I know some CI machines can be slow at things like forking, but I wasn't sure how slow. On the upside, the test is not CPU intensive and will run in parallel, so its impact on the suite overall should be extremely low.
There was a problem hiding this comment.
okay, sweet, it shouldn't be an issue
There was a problem hiding this comment.
500 is definitely too low for ARMv6, where a simple startup can take upwards of 3 seconds. I suggest using common.platformTimeout(1000), which right now applies factor 7 (7 seconds) for ARMv6.
There was a problem hiding this comment.
@silverwind thanks the tip!
|
Confirmed -- would you mind opening an issue for this too so we can track it better? |
|
Opened issue #1600. |
Server does not emit a listen event when no address is specified and is called in eval mode. Regression introduced in 5b636fe.
|
Thanks for telling us about the bug! A fix is pending in #1793 - I'm closing this PR as it was decided to use the other test case. Thanks. |
I found this when trying to confirm the format of the address object. The following hangs on most releases of node higher than 0.10.38:
It seems that Server does not emit a listen event when no address is specified and is called in eval mode.
I don't fully understand how the regression was introduced, but
git bisectseemed pretty confident that it was in 5b636fe./to @trevnorris for the indicated commit
/cc @piscisaureus @sam-github