-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: fix flaky test-http2-settings-flood #19349
Conversation
Marking as |
d35d667
to
38db513
Compare
The change "worked" in that the test kept trying but it got ECONNRESET before the flood error. Trying again but swallowing ECONNRESET errors. Really, though, thing to do is probably make the flooding threshold configurable (if it is not) and then reduce it to something really low for this test... |
CI: https://ci.nodejs.org/job/node-test-commit/16923/ Well, in that case, it just times out.... |
One option is to skip the test if ECONNRESET is received, but that may be even worse than the current situation, which unfortunately is that if the test fails, we shrug it off because the test is inherently unreliable. |
OMG, doing it in 10K chunks appears to have fixed the problem on Windows! Removing CI: https://ci.nodejs.org/job/node-test-pull-request/13689/ (Failure on LinuxONE appears build related and unrelated to this change. Re-run: https://ci.nodejs.org/job/node-test-commit-linuxone/13659/ ) |
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: nodejs#18251
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.
Nicely done!
Landed in 879f521 |
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: nodejs#18251 PR-URL: nodejs#19349 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: #18251 PR-URL: #19349 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: #18251 PR-URL: #19349 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: #18251 PR-URL: #19349 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: #18251 PR-URL: #19349 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
The test is unreliable on some Windows platforms in its current form.
Make it more robust by using
setInterval()
to repeat the floodinguntil an error is triggered.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes