test: try other ipv6 localhost alternatives #4325
Closed
Conversation
This file contains 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
mscdex
added
net
mscdex
force-pushed
the
fix-flaky-net-connect-options-ipv6
branch
from
5c0cfa1
to
878f245
Compare
|
I don't know about this PR. The test queries for 'localhost' with family=AF_INET6. If the local resolver doesn't know how to resolve that, I would argue it's the resolver that is broken, not the test. The change itself LGTM except that there should be two spaces before comments. |
|
Generally agree but it seems worthwhile to be a bit flexible and work around broken resolvers. |
mscdex
force-pushed
the
fix-flaky-net-connect-options-ipv6
branch
from
878f245
to
a924136
Compare
|
I adjusted the comments a bit, LGTY @bnoordhuis ? |
|
LGTM if the CI likes it. |
|
CI looks alright: https://ci.nodejs.org/job/node-test-pull-request/1020/ |
mscdex
added a commit
that referenced
this pull request
Dec 17, 2015
PR-URL: #4325 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
|
Landed in 852313a. |
Closed
Fishrock123
pushed a commit
to Fishrock123/node
that referenced
this pull request
Dec 22, 2015
PR-URL: nodejs#4325 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fishrock123
pushed a commit
to Fishrock123/node
that referenced
this pull request
Jan 6, 2016
PR-URL: nodejs#4325 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
mscdex
added a commit
that referenced
this pull request
Jan 7, 2016
PR-URL: #4325 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 19, 2016
PR-URL: #4325 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Merged
scovetta
pushed a commit
to scovetta/node
that referenced
this pull request
Apr 2, 2016
PR-URL: nodejs#4325 Reviewed-By: Ben Noordhuis <[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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ubuntu1404-64has been having issues withtest-net-connect-options-ipv6fairly frequently. I'm not exactly sure why the IPv6localhostresolving is flaky on that particular machine (it could be the way its network is configured?), but being an Ubuntu system it typically has other IPv6 localhost hostnames.This change tries those special IPv6 hostnames first before trying
localhost. If those special hostnames fail to resolve and iflocalhostshould error withENOTFOUND, it retries several times before giving up.I'm hoping this is enough to fix things. I did perform a stress test on that particular node and it was ok on every run after this change.