-
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-session-timeout #15328
Conversation
With this change, I am able to reliably run 96 concurrent versions of the test without any failures. Prior to this, the test was reliably failing with 32 concurrent versions or even less. It was also observed failing on CI on FreeBSD. |
(Can't run a stress test until #15300 lands.) |
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.
LGTM
As a reference, this was introduced in #15188. I do not see any strategy to test this differently. cc @apapirovski @nodejs/http2 |
This no longer tests the timeout condition because the test only runs 40 requests 10ms apart, so it ends way before the timeout would trigger. I have a fix in progress with a slightly different approach. Will open a PR, if you don't mind. I don't think increasing the values is enough because it seems like the reason it's failing on FreeBSD is that the server close takes too long (successful runs on that system take less than 500ms and failing ones take almost 2s). |
I would prefer #15338 |
I also prefer #15338 |
Increase server timeout to reduce likelihood of triggering race conditions. Fixes: nodejs#15326
Increase server timeout to reduce likelihood of triggering race
conditions.
Fixes: #15326
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test http2