-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: refactor to const/let and common.mustCall #9934
Conversation
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'), | ||
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem') | ||
}; | ||
|
||
var server = tls.Server(options, function(socket) { | ||
const server = tls.Server(options, function(socket) { |
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.
common.mustCall()
takes an optional second argument that specifies the number of times the function should be called. You could use that here, and then completely remove the process.on('exit', ...)
block.
adjusted |
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'), | ||
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem') | ||
}; | ||
|
||
var server = tls.Server(options, function(socket) { | ||
if (++serverConnected === 2) { |
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.
I don't think you can drop this check. It changes the behavior. The server will be closed the first time instead of the second.
refactor var -> const/let refactor process.on('exit') into common.mustCall
adjusted |
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.
LGTM pending CI.
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.
LGTM pending CI (will run the CI soon, just waiting for other jobs to finish)
Single CI failure is unrelated. |
refactor var -> const/let refactor process.on('exit') into common.mustCall PR-URL: nodejs#9934 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in d648f2b. |
refactor var -> const/let refactor process.on('exit') into common.mustCall PR-URL: #9934 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
refactor var -> const/let refactor process.on('exit') into common.mustCall PR-URL: nodejs#9934 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
refactor var -> const/let refactor process.on('exit') into common.mustCall PR-URL: nodejs#9934 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
refactor var -> const/let refactor process.on('exit') into common.mustCall PR-URL: #9934 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
refactor var -> const/let refactor process.on('exit') into common.mustCall PR-URL: #9934 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
refactor var -> const/let refactor process.on('exit') into common.mustCall PR-URL: #9934 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
test: refactor to const/let and common.mustCall
Checklist
make -j8 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
test
Description of change
task assigned at NINA 2016 Austin code and learn
refactor var -> const/let
refactor process.on('exit') into common.mustCall