Skip to content

Conversation

tniessen
Copy link
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
Contributor

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

@tniessen
Copy link
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
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
Member

lpinca commented Aug 17, 2017

@silverwind I guess the comment was for @tniessen.

Copy link
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
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 <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@MylesBorins
Copy link
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants