-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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: try other ipv6 localhost alternatives #4325
Conversation
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. |
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/ |
PR-URL: #4325 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in 852313a. |
PR-URL: nodejs#4325 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#4325 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #4325 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #4325 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#4325 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
ubuntu1404-64
has been having issues withtest-net-connect-options-ipv6
fairly frequently. I'm not exactly sure why the IPv6localhost
resolving 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 iflocalhost
should 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.