test: refactor test-http-exit-delay#4055
Uh oh!
There was an error while loading. Please reload this page.
Conversation
test-http-exit-delay was introduced to confirm the removal of a 1 second delay that can occur when exiting node after an http request. This change refactors the test for simplicity and also in the hopes of either eliminating flakiness on CI or, if not that, at least making the source of the flakiness easier to track down. Ref: 16b59cbcFixes: #4045
Trott added test 
Trott commented Nov 28, 2015
So, about the flakiness... the test failed once for no apparent reason: But I have been unable to make it fail with the stress test: So... |
Trott commented Nov 28, 2015
Trott commented Nov 28, 2015
Stress test with this change: If nothing else, the test should at least run marginally faster now. EDIT: Actually, it seems to run a lot faster... |
Trott commented Dec 2, 2015
Bump! Simplified/improved/faster test FTW? Or unnecessary churn? |
bnoordhuis commented Dec 2, 2015
LGTM. Is a fixed 1000 ms enough for the slower buildbots? |
jbergstroem commented Dec 2, 2015
For debug builds likely not. |
Trott commented Dec 2, 2015
Putting aside the debug issue for a moment, I'm pretty sure one second is long enough for the Raspberry Pi machines since the existing test was never flaky on the Raspberry Pi machines as far as I know. And they would be the slowest of what we test on in general, right? I'll run a stress test...
EDIT: Guess I need to do it again and not typo on the test name... https://ci.nodejs.org/job/node-stress-single-test/130/nodes=pi2-raspbian-wheezy/console |
Trott commented Dec 2, 2015
999 consecutive susccessful runs on Raspberry Pi, hooray! I'm inclined to not worry about debug builds until this test is shown to actually be a problem. The implementation in this PR allows for 1 second for the test to run. That is actually longer than the current test allows (no more than 900 ms). So in that regard, this is more debug build friendly than the current test, at least. (And it might not be a problem on debug builds after all? Hopefully?) Aside: The reason I don't want to use |
jasnell commented Dec 3, 2015
LGTM |
Trott commented Dec 4, 2015
Landed in 8a2acd4 |
test-http-exit-delay was introduced to confirm the removal of a 1 second delay that can occur when exiting node after an http request. This change refactors the test for simplicity and also in the hopes of either eliminating flakiness on CI or, if not that, at least making the source of the flakiness easier to track down. Ref: 16b59cbcFixes: #4045 PR-URL: #4055 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
test-http-exit-delay was introduced to confirm the removal of a 1 second delay that can occur when exiting node after an http request. This change refactors the test for simplicity and also in the hopes of either eliminating flakiness on CI or, if not that, at least making the source of the flakiness easier to track down. Ref: 16b59cbcFixes: #4045 PR-URL: #4055 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
test-http-exit-delay was introduced to confirm the removal of a 1 second delay that can occur when exiting node after an http request. This change refactors the test for simplicity and also in the hopes of either eliminating flakiness on CI or, if not that, at least making the source of the flakiness easier to track down. Ref: 16b59cbcFixes: #4045 PR-URL: #4055 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
test-http-exit-delay was introduced to confirm the removal of a 1 second delay that can occur when exiting node after an http request. This change refactors the test for simplicity and also in the hopes of either eliminating flakiness on CI or, if not that, at least making the source of the flakiness easier to track down. Ref: 16b59cbcFixes: #4045 PR-URL: #4055 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
test-http-exit-delay was introduced to confirm the removal of a 1 second delay that can occur when exiting node after an http request. This change refactors the test for simplicity and also in the hopes of either eliminating flakiness on CI or, if not that, at least making the source of the flakiness easier to track down. Ref: 16b59cbcFixes: #4045 PR-URL: #4055 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
test-http-exit-delay was introduced to confirm the removal of a 1 second delay that can occur when exiting node after an http request. This change refactors the test for simplicity and also in the hopes of either eliminating flakiness on CI or, if not that, at least making the source of the flakiness easier to track down. Ref: nodejs@16b59cbcFixes: nodejs#4045 PR-URL: nodejs#4055 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
test-http-exit-delay was introduced to confirm the removal of a 1 second
delay that used to occur when exiting node after an http request.
This change refactors the test for simplicity and also in the hopes of
either eliminating flakiness on CI or, if not that, at least making
the source of the flakiness easier to track down.
Ref: 16b59cbc
Fixes: #4045