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.

Sign up for GitHub

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

Jump to bottom

test: de-flake test-dns-idna2008.js #26473

Merged
merged 1 commit into from
Mar 9, 2019
Merged

test: de-flake test-dns-idna2008.js #26473

merged 1 commit into from
Mar 9, 2019

Conversation

refack
Copy link
Contributor

@refack refack commented Mar 6, 2019 •
edited
Loading

Use a known well behaved DNS server.

/CC @nodejs/testing

Fixes: #25870

P.S. ATM the DNS server configured on one of our CI workers, fails for the domain name used in this test:

root@test-rackspace-ubuntu1604-x64-1:~# nslookup xn--strae-oqa.de
;; Got SERVFAIL reply from 119.9.60.62, trying next server
Server:         119.9.60.63
Address:        119.9.60.63#53

** server can't find xn--strae-oqa.de: SERVFAIL

this causes the daily CI test of master to fail: https://ci.nodejs.org/job/node-test-commit-custom-suites/

Lookup works fine when directed to use 8.8.8.8

root@test-rackspace-ubuntu1604-x64-1:~# nslookup xn--strae-oqa.de 8.8.8.8
Server:         8.8.8.8
Address:        8.8.8.8#53

Non-authoritative answer:
Name:   xn--strae-oqa.de
Address: 81.169.145.78
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@refack build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2784/pipeline

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 6, 2019
@refack
Copy link
Contributor Author

refack commented Mar 6, 2019 •
edited
Loading

CI: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/5051/ ✔️

@refack refack added the dns Issues and PRs related to the dns subsystem. label Mar 6, 2019
@refack refack requested a review from Trott March 6, 2019 15:33
@Trott
Copy link
Member

Trott commented Mar 7, 2019

Post-adding-comments custom CI: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/5084/
Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/2796/

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 7, 2019
@Trott
Copy link
Member

Trott commented Mar 7, 2019 •
edited
Loading

node-daily-master failed today for the sixth day in a row due to this. Would love to fast-track this. 👍 here to fast-track.

EDIT: Withdrawing pending outcome of the "use DNS*_SERVER" conversation. Since node-daily-master only runs once a day, we'll live with the red job another day or two if we have to.

@Trott Trott added fast-track PRs that do not need to wait for 48 hours to land. and removed fast-track PRs that do not need to wait for 48 hours to land. labels Mar 7, 2019
@BridgeAR BridgeAR requested a review from joyeecheung March 7, 2019 13:12
@refack refack self-assigned this Mar 7, 2019
@refack refack force-pushed the fix-idna2008-test branch from 8e7f59c to 7852d68 Compare March 7, 2019 22:46
@refack
Copy link
Contributor Author

refack commented Mar 7, 2019 •
edited
Loading

node-daily-master failed today for the sixth day in a row due to this.

https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/5084/ seems like this doesn't fix the issue 🤔.

Trying again https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/5094/

@refack
Copy link
Contributor Author

refack commented Mar 8, 2019 •
edited
Loading

The dns.setServers() method affects only dns.resolve(), dns.resolve*() and dns.reverse() (and specifically not dns.lookup()).

So I patched the hardware.

https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/5099/ ✔️
https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/5100/ ✔️

@refack refack force-pushed the fix-idna2008-test branch 3 times, most recently from d2e957e to d9607dd Compare March 9, 2019 00:11
@refack refack requested review from Trott and gireeshpunathil March 9, 2019 14:09
@gireeshpunathil
Copy link
Member

LGTM!

@refack
test: de-flake test-dns-idna2008.js
9613221 
* use known well-behaved DNS server
* force pass on ESERVFAIL

PR-URL: nodejs#26473
Fixes: nodejs#25870
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@refack refack force-pushed the fix-idna2008-test branch from d9607dd to 9613221 Compare March 9, 2019 15:13
@refack refack removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 9, 2019
@refack refack removed the request for review from Trott March 9, 2019 15:13
@refack refack removed the request for review from gireeshpunathil March 9, 2019 15:13
@refack refack merged commit 9613221 into nodejs:master Mar 9, 2019
@refack refack deleted the fix-idna2008-test branch March 9, 2019 15:14
@refack refack mentioned this pull request Mar 10, 2019
Merged
3 tasks
@refack refack removed their assignment Mar 11, 2019
refack added a commit to refack/node that referenced this pull request Mar 12, 2019
@refack
test: fix syntax error in test-dns-idna2008.js when failing
f77555f 
Fixes a bug I introduced in 9613221

PR-URL: nodejs#26570
Refs: nodejs#25870
Refs: nodejs#26473
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
BridgeAR pushed a commit that referenced this pull request Mar 13, 2019
@refack @BridgeAR
test: de-flake test-dns-idna2008.js
ce260c2 
* use known well-behaved DNS server
* force pass on ESERVFAIL

PR-URL: #26473
Fixes: #25870
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
BridgeAR pushed a commit that referenced this pull request Mar 13, 2019
@refack @BridgeAR
test: fix syntax error in test-dns-idna2008.js when failing
f03498c 
Fixes a bug I introduced in 9613221

PR-URL: #26570
Refs: #25870
Refs: #26473
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Mar 13, 2019
Merged
BridgeAR pushed a commit that referenced this pull request Mar 14, 2019
@refack @BridgeAR
test: de-flake test-dns-idna2008.js
476dc7e 
* use known well-behaved DNS server
* force pass on ESERVFAIL

PR-URL: #26473
Fixes: #25870
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
BridgeAR pushed a commit that referenced this pull request Mar 14, 2019
@refack @BridgeAR
test: fix syntax error in test-dns-idna2008.js when failing
f2eeb38 
Fixes a bug I introduced in 9613221

PR-URL: #26570
Refs: #25870
Refs: #26473
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Mar 14, 2019
@refack @BridgeAR
test: fix syntax error in test-dns-idna2008.js when failing
155cc34 
Fixes a bug I introduced in 9613221

PR-URL: nodejs#26570
Refs: nodejs#25870
Refs: nodejs#26473
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
BridgeAR pushed a commit that referenced this pull request Mar 14, 2019
@refack @BridgeAR
test: fix syntax error in test-dns-idna2008.js when failing
e1a55e7 
Fixes a bug I introduced in 9613221

PR-URL: #26570
Refs: #25870
Refs: #26473
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR left review comments

@richardlau richardlau richardlau left review comments

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@Trott Trott Trott approved these changes

Assignees
No one assigned
Labels
dns Issues and PRs related to the dns 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.

investigate flaky internet/test-dns-idna2008 test
7 participants
@refack @nodejs-github-bot @Trott @gireeshpunathil @joyeecheung @richardlau @BridgeAR