-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: add http2 test for method CONNECT #15052
Conversation
Adds test case for default handling of method CONNECT, as well as the ability to bind a connect listener and handle the request. Refs: nodejs#14985
83685c4
to
260f29d
Compare
|
||
function testMethodConnect(testsToRun) { | ||
if (!testsToRun) { | ||
return server.close(); |
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.
Just pointing out... we now have a "countdown" utility that can be used to improve this kind of logic.
For instance,
const Countdown = require('../common/countdown');
const countdown = new Countdown(2, () => console.log('done!') );
countdown.dec();
countdown.dec();
The callback based into Countdown will be called automatically and synchronously when the internal counter reaches zero.
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.
Thanks @jasnell, that's super useful. Will definitely use it in the future! Did you want me to update this test as well or good as is?
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.
It's good as is. We can convert over to the countdown util separately
Landed in 88441f6 |
Adds test case for default handling of method CONNECT, as well as the ability to bind a connect listener and handle the request. PR-URL: nodejs#15052 Refs: nodejs#14985 Reviewed-By: James M Snell <jasnell@gmail.com>
Adds test case for default handling of method CONNECT, as well as the ability to bind a connect listener and handle the request. PR-URL: nodejs/node#15052 Refs: nodejs/node#14985 Reviewed-By: James M Snell <jasnell@gmail.com>
Adds test case for default handling of method CONNECT, as well as the ability to bind a connect listener and handle the request. Part of the request for increased code coverage for http2, as per #14985
Let me know if I can change anything. Thanks!
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test