FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: improve cluster-disconnect-handles test by mscdex · Pull Request #4084 · nodejs/node · GitHub

/ node Public

test: improve cluster-disconnect-handles test - #4084

Closed
mscdex wants to merge 1 commit into
nodejs:masterfrom
mscdex:fix-test-cluster-disconnect-handles
ConversationCommits (1)ChecksFiles changed
Closed

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

Conversation

mscdex commented Dec 1, 2015

Copy link
Copy Markdown
Contributor

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 added cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests. labels Dec 1, 2015

mscdex commented Dec 1, 2015

Copy link
Copy Markdown
Contributor Author

CI is all green except unrelated errors on Windows.

cjihrig reviewed Dec 1, 2015
View reviewed changes
Comment thread test/parallel/test-cluster-disconnect-handles.js

cjihrig Dec 1, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

mscdex Dec 1, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

cjihrig commented Dec 1, 2015

Copy link
Copy Markdown
Contributor

LGTM with a question and comment.

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 force-pushed the fix-test-cluster-disconnect-handles branch from 73d2dd4 to e22c756 Compare December 1, 2015 18:34

cjihrig commented Dec 1, 2015

Copy link
Copy Markdown
Contributor

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

mscdex commented Dec 1, 2015

Copy link
Copy Markdown
Contributor Author

@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 commented Dec 1, 2015

Copy link
Copy Markdown
Contributor

Does listening on 'localhost' work for you?

mscdex commented Dec 1, 2015

Copy link
Copy Markdown
Contributor Author

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 commented Dec 1, 2015

Copy link
Copy Markdown
Contributor

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
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 commented Dec 2, 2015

Copy link
Copy Markdown
Contributor Author

Landed in 49b5445.

mscdex closed this Dec 2, 2015
imran-iq mentioned this pull request Dec 4, 2015
Closed
mscdex added a commit that referenced this pull request Dec 5, 2015
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 mentioned this pull request Dec 17, 2015
Closed
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
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
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 mentioned this pull request Jan 19, 2016
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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

Back | FazBrowse Home | New Git URL