Uh oh!
There was an error while loading. Please reload this page.
test: update test-http-timeout-overflow to use common.mustCall - #17528
test: update test-http-timeout-overflow to use common.mustCall#17528collin5 wants to merge 2 commits into
Conversation
apapirovski
left a comment
There was a problem hiding this comment.
Hi @collin5 — thank you for taking this on! this particular test would be better with using common.mustCall for both the function within http.createServer and within res.on('end'). Then the process.on('exit') block can also be removed.
Countdown is better when used for tests where n > 1.
| response.end('OK'); | ||
| }); | ||
| const server = http.createServer(function(req, res) { |
There was a problem hiding this comment.
This could be common.mustCall(function(req, res) { /* function body */ }) which would then confirm that it executes during the duration of the test. Then no Countdown is needed.
There was a problem hiding this comment.
@apapirovski Right, that actually makes more sense. Let me do it ASAP 👍
| @@ -45,8 +50,7 @@ server.listen(0, function() { | |||
| req.clearTimeout(callback); | |||
| res.on('end', function() { | |||
There was a problem hiding this comment.
This can also use common.mustCall so common.mustCall(function() { /* function body */ }).
@apapirovski Thanks for the feedback, I've pushed the changes 👍. |
apapirovski
left a comment
There was a problem hiding this comment.
LGTM. Thanks for making the changes!
apapirovski
commented
Dec 8, 2017
apapirovski
commented
Dec 9, 2017
PR-URL: #17528 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
collin5
commented
Dec 9, 2017
@apapirovski Thank you too 👍 |
PR-URL: #17528 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
PR-URL: #17528 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
PR-URL: #17528 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
PR-URL: #17528 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
PR-URL: #17528 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
Updated tests in /test/parallel/test-http-timeout-overflow.js to use Countdown module.
Ref issue #17169