test: refactor test-http-exit-delay #4055
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: 16b59cbc Fixes: #4045
Trott
added
test
|
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... |
|
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... |
|
Bump! Simplified/improved/faster test FTW? Or unnecessary churn? |
|
LGTM. Is a fixed 1000 ms enough for the slower buildbots? |
|
For debug builds likely not. |
|
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 |
|
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 |
|
LGTM |
|
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: 16b59cbc Fixes: #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: 16b59cbc Fixes: #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: 16b59cbc Fixes: #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: 16b59cbc Fixes: #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: 16b59cbc Fixes: #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@16b59cbc Fixes: 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