Skip to content

test: use invalid host according to RFC2606#14863

Closed
tniessen wants to merge 2 commits into
nodejs:masterfrom
tniessen:shoot-the-stars
Closed

test: use invalid host according to RFC2606#14863
tniessen wants to merge 2 commits into
nodejs:masterfrom
tniessen:shoot-the-stars

Conversation

@tniessen
Copy link
Copy Markdown
Member

Refs: #14781
Refs: https://tools.ietf.org/html/rfc2606#section-2

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

test

@tniessen tniessen added the test Issues and PRs related to the tests. label Aug 16, 2017
@tniessen tniessen self-assigned this Aug 16, 2017
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Aug 16, 2017
@silverwind
Copy link
Copy Markdown
Contributor

This could be used in more tests, for example test/internet/test-dns.js.

@tniessen
Copy link
Copy Markdown
Member Author

This could be used in more tests, for example test/internet/test-dns.js.

@silverwind Are you referring to this part of the file?

const invalidHost = 'fasdfdsaf';
assert.throws(() => {
dns.lookupService(invalidHost, 0, common.mustNotCall());
}, common.expectsError({
code: 'ERR_INVALID_OPT_VALUE',
type: TypeError,
message: `The value "${invalidHost}" is invalid for option "host"`
}));

@silverwind
Copy link
Copy Markdown
Contributor

silverwind commented Aug 17, 2017

@lpinca yes, that's another example, but I was refering to the file in internet tests, for example:

const req = dns.lookup('does.not.exist', 4, function(err, ip, family) {

const req = dns.lookup('nosuchhostimsure', function(err) {

@lpinca
Copy link
Copy Markdown
Member

lpinca commented Aug 17, 2017

@silverwind I guess the comment was for @tniessen.

Copy link
Copy Markdown
Contributor

@silverwind silverwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@tniessen
Copy link
Copy Markdown
Member Author

Landed in 467385a. If you see any other occurrences which should be changed, feel free to comment here.

@tniessen tniessen closed this Aug 18, 2017
tniessen added a commit that referenced this pull request Aug 18, 2017
PR-URL: #14863
Refs: #14781
Refs: https://tools.ietf.org/html/rfc2606#section-2
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
@MylesBorins
Copy link
Copy Markdown
Contributor

This appears to rely on a semver-major change. setting dont-land-on-v8.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Issues and PRs related to the tests.

9 participants