Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: fix race condition in test-http-client-onerror #4346

Closed
wants to merge 1 commit into from

Conversation

dnakamura
Copy link
Contributor

I've noticed an error pop up very rarely in test-http-client-onerror, where the test fails whith a refused connection. Tracking it down I believe the issue is that the test is written such that connections could be attempted before the socket is listening

getall();
function runTest() {
for (var i = 0; i < 10; i++)
getall();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you restore the two space indent?

@bnoordhuis
Copy link
Member

LGTM with a style nit. Can you add your description of the issue to the commit log? Thanks.

@dnakamura dnakamura force-pushed the onerror-fix branch 2 times, most recently from fb92064 to 95aa340 Compare December 18, 2015 15:50
Occasionally test-http-client-onerror will fail with a refused connection.
This patch fixes the possibility that connections will be attempted before
server is listening.
@dnakamura
Copy link
Contributor Author

Fixed.

@bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis added the test Issues and PRs related to the tests. label Dec 18, 2015
@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Dec 18, 2015
@mscdex
Copy link
Contributor

mscdex commented Dec 18, 2015

LGTM

@jbergstroem
Copy link
Member

LGTM

@jasnell
Copy link
Member

jasnell commented Dec 30, 2015

LGTM

jasnell pushed a commit that referenced this pull request Dec 30, 2015
Occasionally test-http-client-onerror will fail with a refused connection.
This patch fixes the possibility that connections will be attempted before
server is listening.

PR-URL: #4346
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Dec 30, 2015

Landed in 2413a4e

@jasnell jasnell closed this Dec 30, 2015
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
Occasionally test-http-client-onerror will fail with a refused connection.
This patch fixes the possibility that connections will be attempted before
server is listening.

PR-URL: nodejs#4346
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 7, 2016
Occasionally test-http-client-onerror will fail with a refused connection.
This patch fixes the possibility that connections will be attempted before
server is listening.

PR-URL: #4346
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
Occasionally test-http-client-onerror will fail with a refused connection.
This patch fixes the possibility that connections will be attempted before
server is listening.

PR-URL: #4346
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Occasionally test-http-client-onerror will fail with a refused connection.
This patch fixes the possibility that connections will be attempted before
server is listening.

PR-URL: nodejs#4346
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants