test: http test improvements#14315
test: http test improvements #14315
Uh oh!
There was an error while loading. Please reload this page.
Conversation
jasnellforce-pushed the http-test-improvements branch from a03f0e0 to 8f50438Compare
There was a problem hiding this comment.
Just tightening up the tests to be more strict.
There was a problem hiding this comment.
this can be a shorthand. {method, path: '/'}. Also this test structure is a bit strange - and I'm not sure that in this case the code is clearer after the refactoring. Personally I would prefer a for... of but it's not a big deal.
There was a problem hiding this comment.
It might be good to add a Symbol to the list of expectedFails while we're touching these
benjamingr left a comment
benjamingr left a comment There was a problem hiding this comment.
Left a few nits - overall nice work with the countdown.
This is also a pretty big diff.
refack commented Jul 17, 2017
/cc @nodejs/testing @nodejs/http |
jasnellforce-pushed the http-test-improvements branch from efcd158 to 582ce9bCompare
jasnell commented Jul 17, 2017
@benjamingr ... nits addressed! |
vsemozhetbyt commented Jul 20, 2017
It seems this should be added before the code example in <!-- eslint-disable strict, required-modules --> |
jasnellforce-pushed the http-test-improvements branch from 582ce9b to 1fccc7bComparejasnell commented Jul 24, 2017
* Add common/countdown utility * Numerous improvements to http tests
jasnellforce-pushed the http-test-improvements branch from 1fccc7b to 9062d47Compare* Add common/countdown utility * Numerous improvements to http tests PR-URL: #14315 Reviewed-By: Benjamin Gruenbaum <[email protected]>
jasnell commented Jul 24, 2017
Landed in b0a8a7c, squashed and a lint issue fixed in the process. |
jasnell commented Jul 24, 2017
@nodejs/codeandlearn ... As a future exercise for code-n-learn participants, there are a non-trivial number of existing tests that can be modified to use the new common/countdown module... for instance, to shutdown a server after a given number of test requests have completed. |
addaleax commented Jul 27, 2017
This doesn’t land cleanly on 8.x; if you can, please follow the guide and raise a backport PR. |
* Add common/countdown utility * Numerous improvements to http tests PR-URL: nodejs#14315 Reviewed-By: Benjamin Gruenbaum <[email protected]>
* Add common/countdown utility * Numerous improvements to http tests Backport-PR-URL: #14583 Backport-Reviewed-By: Anna Henningsen <[email protected]> PR-URL: #14315 Reviewed-By: Benjamin Gruenbaum <[email protected]>
MylesBorins commented Aug 16, 2017
This does not land cleanly in LTS. Please feel free to manually backport by following the guide. Please also feel free to replace do-not-land if it is being backported |
* Add common/countdown utility * Numerous improvements to http tests PR-URL: #14315 Reviewed-By: Benjamin Gruenbaum <[email protected]>
9 participants
Generalized http test suite improvements...
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test