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: fix test-dns-idna2008.js #27208

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Apr 13, 2019

The test should pass if ESERVFAIL is the result.

Refs: #25870 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

The test should pass if ESERVFAIL is the result.

Refs: nodejs#25870 (comment)
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 13, 2019
@Trott Trott requested review from refack and bnoordhuis April 13, 2019 03:57
@Trott
Copy link
Member Author

Trott commented Apr 13, 2019

Custom CI to run internet suite: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/6166/

@refack
Copy link
Contributor

refack commented Apr 13, 2019

Did this fail on CI? Because that indicates a wonkey system DNS setup. (It's a nice canary, but not a healthy situation in any other respect).

@Trott
Copy link
Member Author

Trott commented Apr 13, 2019

Did this fail on CI? Because that indicates a wonkey system DNS setup. (It's a nice canary, but not a healthy situation in any other respect).

Yes, it failed in CI, but only showed up as yellow rather than red, which was something I was going to ask about.

https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/6132/

00:01:57 not ok 9 internet/test-dns-idna2008
00:01:57   ---
00:01:57   duration_ms: 1.413
00:01:57   severity: fail
00:01:57   exitcode: 1
00:01:57   stack: |-
00:01:57     /home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/test/common/index.js:681
00:01:57     const crashOnUnhandledRejection = (err) => { throw err; };
00:01:57                                                  ^
00:01:57     
00:01:57     Error: queryA ESERVFAIL straße.de
00:01:57         at QueryReqWrap.onresolve [as oncomplete] (internal/dns/promises.js:167:17)
00:01:57   ...

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

refack commented Apr 13, 2019

showed up as yellow

Should have been red. There was a bug in the CI config were it interpreted return value of 1 as UNSTABLE instead of FAIL (it's fixed now). Now I'll see what's up with https://ci.nodejs.org/computer/test-rackspace-ubuntu1604-x64-1/

@richardlau
Copy link
Member

showed up as yellow

Should have been red. There was a bug in the CI config were it interpreted return value of 1 as UNSTABLE instead of FAIL (it's fixed now). Now I'll see what's up with https://ci.nodejs.org/computer/test-rackspace-ubuntu1604-x64-1/

https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/jobConfigHistory/showDiffFiles?timestamp1=2019-04-09_11-24-54&timestamp2=2019-04-13_13-13-15 reintroduces skipping the test (e.g. on 8.x) causing the job to fail, i.e. it undoes part of #26933 (comment)

@refack
Copy link
Contributor

refack commented Apr 13, 2019

i.e. it undoes part of #26933 (comment)

https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/jobConfigHistory/showDiffFiles?timestamp1=2019-04-09_11-24-54&timestamp2=2019-04-13_13-42-35

Good eye!

Also added a comment. Hopefully I'll see it (also 2 should be more eyebrow raising then 1)
image

@Trott
Copy link
Member Author

Trott commented Apr 15, 2019

Landed in f6bd3b2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants