-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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: various test improvements #8468
Conversation
assert.equal(4, addressType); | ||
}); | ||
dns.lookup(null, common.mustCall((error, result, addressType) => { | ||
if (error) throw error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert.ifError(error)
?
Is there any motivation for the changes from |
`"${process.execPath}"`, | ||
'-e', | ||
'"console.error(process.argv)"', | ||
'foo', 'bar'].join(' '); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: there is one element per line now, what about putting 'bar'
on a new line?
LGTM once |
effca79
to
12ecd42
Compare
Updated! PTAL |
|
||
var dns = require('dns'); | ||
const dns = require('dns'); | ||
|
||
// Stub `getaddrinfo` to *always* error. | ||
cares.getaddrinfo = function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: use an arrow function?
LGTM if CI is green. |
* Favor strictEqual * Use const where appropriate * Modernize where possible
12ecd42
to
efa87af
Compare
New New CI: https://ci.nodejs.org/job/node-test-pull-request/4434/ (since the last one had to be partially aborted) |
Failures seem to be unrelated to these changes. |
* Favor strictEqual * Use const where appropriate * Modernize where possible PR-URL: #8468 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Landed in 07b54ec. |
* Favor strictEqual * Use const where appropriate * Modernize where possible PR-URL: #8468 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* Favor strictEqual * Use const where appropriate * Modernize where possible PR-URL: #8468 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Setting as don't land for v4.x @jasnell please feel free to manually backport |
Checklist
make -j4 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
test
Description of change
Various test improvements