-
Notifications
You must be signed in to change notification settings - Fork 30.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: refactor test-tls-reuse-host-from-socket #6756
Conversation
LGTM if CI is green. |
secureSocket.destroy(); | ||
}); | ||
}); | ||
console.log('ok'); |
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.
Maybe remove this log?
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 left it in to preserve the original test's logging. But I'm happy to take it out.
CI: https://ci.nodejs.org/job/node-test-pull-request/2646/. LGTM if CI is green. |
connected = true; | ||
var secureSocket = tls.connect({ socket: socket }, function() { | ||
secure = true; | ||
const socket = net.connect(443, 'www.google.com', common.mustCall(() => { |
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.
Can this be pointed at a non-specific url? www.example.org
for instance?
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.
Changed. Thanks!
Minor nit, otherwise LGTM |
0acd996
to
34cfca9
Compare
Replace booleans with `common.mustCall()`, migrate from `var` to `const`, and apply minor formatting changes.
Fixed a nit, rebased, force pushed. CI: https://ci.nodejs.org/job/node-test-pull-request/2706/ |
Ci is green. Landing... |
Replace booleans with `common.mustCall()`, migrate from `var` to `const`, and apply minor formatting changes. PR-URL: nodejs#6756 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in 395cc88 |
Replace booleans with `common.mustCall()`, migrate from `var` to `const`, and apply minor formatting changes. PR-URL: #6756 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Replace booleans with `common.mustCall()`, migrate from `var` to `const`, and apply minor formatting changes. PR-URL: #6756 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Replace booleans with `common.mustCall()`, migrate from `var` to `const`, and apply minor formatting changes. PR-URL: #6756 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Replace booleans with `common.mustCall()`, migrate from `var` to `const`, and apply minor formatting changes. PR-URL: #6756 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Replace booleans with `common.mustCall()`, migrate from `var` to `const`, and apply minor formatting changes. PR-URL: #6756 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
Affected core subsystem(s)
test tls
Description of change
Replace booleans with
common.mustCall()
, migrate fromvar
toconst
, and apply minor formatting changes./cc @nodejs/testing @nodejs/crypto