test: try other ipv6 localhost alternatives#4325
Closed
Uh oh!
There was an error while loading. Please reload this page.
Conversation
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
mscdex added net mscdexforce-pushed the fix-flaky-net-connect-options-ipv6 branch from 5c0cfa1 to 878f245Compare
Member
bnoordhuis commented Dec 17, 2015
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. |
Member
jasnell commented Dec 17, 2015
Generally agree but it seems worthwhile to be a bit flexible and work around broken resolvers. |
mscdexforce-pushed the fix-flaky-net-connect-options-ipv6 branch from 878f245 to a924136Compare
ContributorAuthor
mscdex commented Dec 17, 2015
I adjusted the comments a bit, LGTY @bnoordhuis ? |
Member
bnoordhuis commented Dec 17, 2015
LGTM if the CI likes it. |
ContributorAuthor
mscdex commented Dec 17, 2015
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]>
ContributorAuthor
mscdex commented Dec 17, 2015
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 freeto join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
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.