Skip to content

test: improve cluster-disconnect-handles test#4084

Closed
mscdex wants to merge 1 commit intonodejs:masterfrom
mscdex:fix-test-cluster-disconnect-handles
Closed

test: improve cluster-disconnect-handles test#4084
mscdex wants to merge 1 commit intonodejs:masterfrom
mscdex:fix-test-cluster-disconnect-handles

Conversation

@mscdex
Copy link
Contributor

@mscdex mscdex commented Dec 1, 2015

This commit fixes two issues in test-cluster-disconnect-handles:

  1. If the master's TCP connection to the worker fails, the worker process stays alive and causes many other tests that use the same common port number to also fail (with EADDRINUSE).
  2. One particular problem that can cause the master's TCP connection to fail is attempting an IPv6 connection to the worker when no IPv6 network interfaces are available (this was the case on my local Linux machine).

@mscdex mscdex added cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests. labels Dec 1, 2015
@mscdex
Copy link
Contributor Author

mscdex commented Dec 1, 2015

CI is all green except unrelated errors on Windows.

cjihrig
cjihrig reviewed Dec 1, 2015
View reviewed changes
test/parallel/test-cluster-disconnect-handles.js
Copy link
Contributor

@cjihrig cjihrig Dec 1, 2015

Choose a reason for hiding this comment

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

Hmm... is this the only test that suffers from this problem?

Copy link
Contributor Author

@mscdex mscdex Dec 1, 2015

Choose a reason for hiding this comment

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

I don't know, but this was the only one I personally saw that was causing this problem.

@cjihrig
Copy link
Contributor

cjihrig commented Dec 1, 2015

LGTM with a question and comment.

@mscdex
test: improve cluster-disconnect-handles test
e22c756 
This commit fixes two issues in test-cluster-disconnect-handles:

1. If the master's TCP connection to the worker fails, the worker
process stays alive and causes many other tests that use the same
common port number to also fail (with EADDRINUSE).

2. One particular problem that can cause the master's TCP connection
to fail is attempting an IPv6 connection to the worker when no IPv6
network interfaces are available.
@mscdex mscdex force-pushed the fix-test-cluster-disconnect-handles branch from 73d2dd4 to e22c756 Compare December 1, 2015 18:34
@cjihrig
Copy link
Contributor

cjihrig commented Dec 1, 2015

Does this close #3907? There is also #4009 for that test.

@mscdex
Copy link
Contributor Author

mscdex commented Dec 1, 2015

@cjihrig I don't know, that might be a separate issue.

This is what I get when I don't have the IPv6 check in place:

Debugger listening on port 12946
Debugger listening on port 12346
/home/mscdex/git/node/test/parallel/test-cluster-disconnect-handles.js:72
    throw ex;
    ^

Error: connect EADDRNOTAVAIL :::12346 - Local (:::0)
    at Object.exports._errnoException (util.js:873:11)
    at exports._exceptionWithHostPort (util.js:896:20)
    at connect (net.js:844:14)
    at net.js:942:9
    at doNTCallback0 (node.js:430:9)
    at process._tickDomainCallback (node.js:400:13)

@cjihrig
Copy link
Contributor

cjihrig commented Dec 1, 2015

Does listening on 'localhost' work for you?

@mscdex
Copy link
Contributor Author

mscdex commented Dec 1, 2015

It would probably work for me because I only have localhost resolving to 127.0.0.1, but by default most modern Linux distros have localhost resolve to both IPv4 and IPv6 addresses (even if there are no IPv6 interfaces).

@cjihrig
Copy link
Contributor

cjihrig commented Dec 1, 2015

OK, I don't want to hold up this PR, I was just looking for a cleaner alternative.

mscdex added a commit that referenced this pull request Dec 2, 2015
@mscdex
test: improve cluster-disconnect-handles test
49b5445 
This commit fixes two issues in test-cluster-disconnect-handles:

1. If the master's TCP connection to the worker fails, the worker
process stays alive and causes many other tests that use the same
common port number to also fail (with EADDRINUSE).

2. One particular problem that can cause the master's TCP connection
to fail is attempting an IPv6 connection to the worker when no IPv6
network interfaces are available.

PR-URL: #4084
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@mscdex
Copy link
Contributor Author

mscdex commented Dec 2, 2015

Landed in 49b5445.

@mscdex mscdex closed this Dec 2, 2015
@imran-iq imran-iq mentioned this pull request Dec 4, 2015
Closed
mscdex added a commit that referenced this pull request Dec 5, 2015
@mscdex @rvagg
test: improve cluster-disconnect-handles test
cb55c67 
This commit fixes two issues in test-cluster-disconnect-handles:

1. If the master's TCP connection to the worker fails, the worker
process stays alive and causes many other tests that use the same
common port number to also fail (with EADDRINUSE).

2. One particular problem that can cause the master's TCP connection
to fail is attempting an IPv6 connection to the worker when no IPv6
network interfaces are available.

PR-URL: #4084
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This was referenced Dec 8, 2015
Closed
Merged
@rvagg rvagg mentioned this pull request Dec 17, 2015
Closed
@mscdex mscdex deleted the fix-test-cluster-disconnect-handles branch December 17, 2015 18:01
mscdex added a commit that referenced this pull request Dec 29, 2015
@mscdex
test: improve cluster-disconnect-handles test
ebfbf55 
This commit fixes two issues in test-cluster-disconnect-handles:

1. If the master's TCP connection to the worker fails, the worker
process stays alive and causes many other tests that use the same
common port number to also fail (with EADDRINUSE).

2. One particular problem that can cause the master's TCP connection
to fail is attempting an IPv6 connection to the worker when no IPv6
network interfaces are available.

PR-URL: #4084
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
@mscdex
test: improve cluster-disconnect-handles test
ac939d5 
This commit fixes two issues in test-cluster-disconnect-handles:

1. If the master's TCP connection to the worker fails, the worker
process stays alive and causes many other tests that use the same
common port number to also fail (with EADDRINUSE).

2. One particular problem that can cause the master's TCP connection
to fail is attempting an IPv6 connection to the worker when no IPv6
network interfaces are available.

PR-URL: #4084
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
Merged
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
@mscdex
test: improve cluster-disconnect-handles test
cf5a3a6 
This commit fixes two issues in test-cluster-disconnect-handles:

1. If the master's TCP connection to the worker fails, the worker
process stays alive and causes many other tests that use the same
common port number to also fail (with EADDRINUSE).

2. One particular problem that can cause the master's TCP connection
to fail is attempting an IPv6 connection to the worker when no IPv6
network interfaces are available.

PR-URL: nodejs#4084
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mscdex @cjihrig @jasnell @MylesBorins