Skip to content

Conversation

@santigimeno
Copy link
Member

This test was sometimes timing out in OS X. Remove the timeout and
clean up the code.

This test has failed in my OS X box with this error:

=== release test-dgram-udp4 === Path: parallel/test-dgram-udp4 server is listening on 0.0.0.0:12446 /Users/sgimeno/node/node/test/parallel/test-dgram-udp4.js:59 throw new Error('Timeout'); ^ Error: Timeout at null._onTimeout (/Users/sgimeno/node/node/test/parallel/test-dgram-udp4.js:59:9) at Timer.listOnTimeout (timers.js:92:15) Command: out/Release/node /Users/sgimeno/node/node/test/parallel/test-dgram-udp4.js 

This test was sometimes timing out in `OS X`. Remove the timeout and clean up the code.
@r-52r-52 added dgram Issues and PRs related to the dgram subsystem / UDP. test Issues and PRs related to the tests. labels Feb 20, 2016
@jasnell
Copy link
Member

LGTM

@r-52
Copy link
Contributor

r-52 commented Feb 22, 2016

@mcollina
Copy link
Member

I'm not sure removing the timeout is a great call, otherwise the test will get stuck instead of failing (as far as I understand it).
However, we should be consistent within all dgram tests.

@santigimeno
Copy link
MemberAuthor

@mcollina the idea was that the test runner timeout handled it. Sometimes the use of timeouts has been a source of test flakiness. Do you see it as a problem?

@mcollina
Copy link
Member

No, I'm happy with this.
There are more that needs the timeout removal.

@mcollina
Copy link
Member

LGTM

@santigimeno
Copy link
MemberAuthor

There are more that needs the timeout removal.

Agreed. See: nodejs/testing#19

@mcollina
Copy link
Member

are you a contributor here, or should I land this?

@santigimeno
Copy link
MemberAuthor

I am not, please land. Thanks

mcollina pushed a commit to mcollina/node that referenced this pull request Feb 26, 2016
This test was sometimes timing out in `OS X`. Remove the timeout and clean up the code. Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> PR-URL: nodejs#5339
@mcollina
Copy link
Member

Landed in dff01d1

rvagg pushed a commit that referenced this pull request Feb 27, 2016
This test was sometimes timing out in `OS X`. Remove the timeout and clean up the code. Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> PR-URL: #5339
rvagg pushed a commit that referenced this pull request Feb 27, 2016
This test was sometimes timing out in `OS X`. Remove the timeout and clean up the code. Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> PR-URL: #5339
@Fishrock123Fishrock123 mentioned this pull request Mar 1, 2016
5 tasks
MylesBorins pushed a commit that referenced this pull request Mar 17, 2016
This test was sometimes timing out in `OS X`. Remove the timeout and clean up the code. Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> PR-URL: #5339
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
This test was sometimes timing out in `OS X`. Remove the timeout and clean up the code. Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> PR-URL: #5339
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dgramIssues and PRs related to the dgram subsystem / UDP.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@santigimeno@jasnell@r-52@mcollina@MylesBorins@Trott