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: improve cluster-disconnect-handles test #4084

Closed

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.

process.send(['listening', server.address()]);
debugger;
});
};
if (common.hasIPv6)
Copy link
Contributor

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

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.

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
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
mscdex added a commit that referenced this pull request Dec 5, 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.

PR-URL: #4084
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@rvagg rvagg mentioned this pull request Dec 17, 2015
@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
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
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
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
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
Labels
cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants