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/parallel/test-dgram-connect.js fails locally #27341

Closed
joyeecheung opened this issue Apr 22, 2019 · 5 comments
Closed

test/parallel/test-dgram-connect.js fails locally #27341

joyeecheung opened this issue Apr 22, 2019 · 5 comments
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. good first issue Issues that are suitable for first-time contributors. test Issues and PRs related to the tests.

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Apr 22, 2019

/Users/joyee/projects/node/test/parallel/test-dgram-connect.js:40
    assert.ok(err.code === 'ENOTFOUND' || err.code === 'EAI_AGAIN');
                  ^

TypeError: Cannot read property 'code' of undefined
    at Socket.<anonymous> (/Users/joyee/projects/node/test/parallel/test-dgram-connect.js:40:19)
    at Socket.<anonymous> (/Users/joyee/projects/node/test/common/index.js:373:15)
    at Object.onceWrapper (events.js:284:20)
    at Socket.emit (events.js:196:13)
    at dgram.js:418:31
    at processTicksAndRejections (internal/process/task_queues.js:81:9)

Does not reproduce if I turn my Wifi off (it's probably hijacking DNS requests to invalid domains), so I guess either this should be moved to internet instead, or we should mock the lookup.

@ChALkeR ChALkeR added test Issues and PRs related to the tests. dgram Issues and PRs related to the dgram subsystem / UDP. labels Apr 22, 2019
@Trott

This comment has been minimized.

@Trott

This comment has been minimized.

@joyeecheung
Copy link
Member Author

joyeecheung commented May 2, 2019

@Trott yeah, the issue is that the ISP may not be doing what it's supposed to do and it could e.g. hijack the invalid DNS request so that when you visit it in the browser it redirects you to a page full of their ads :/

@joyeecheung joyeecheung added the good first issue Issues that are suitable for first-time contributors. label May 2, 2019
@joyeecheung
Copy link
Member Author

I guess the easiest solution is to move this test to test/internet because it sends out actual DNS requests.

@santigimeno
Copy link
Member

I guess the easiest solution is to move this test to test/internet because it sends out actual DNS requests.

I think this is fine, but I would just move that specific test not the whole file.

benrki pushed a commit to benrki/node that referenced this issue May 4, 2019
This moves a dgram test from `parallel` to `internet` because it relies
on a DNS request.
In certain cases, ISPs hijack invalid IETF-reserved invalid names which
causes a false negative failure.

Fixes: nodejs#27341
Trott pushed a commit to Trott/io.js that referenced this issue May 13, 2019
This test is not parallelized and so we can use the test commons PORT
variable.

Refs: nodejs#27565 (comment)

PR-URL: nodejs#27565
Fixes: nodejs#27341
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@Trott Trott closed this as completed in 402a793 May 13, 2019
targos pushed a commit that referenced this issue May 14, 2019
This moves a dgram test from `parallel` to `internet` because it relies
on a DNS request.
In certain cases, ISPs hijack invalid IETF-reserved invalid names which
causes a false negative failure.

Fixes: #27341

PR-URL: #27565
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this issue May 14, 2019
This test is not parallelized and so we can use the test commons PORT
variable.

Refs: #27565 (comment)

PR-URL: #27565
Fixes: #27341
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. good first issue Issues that are suitable for first-time contributors. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants