-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Fix flaky HTTP/2 test #41181
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
Fix flaky HTTP/2 test #41181
Conversation
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs
Outdated
Show resolved
Hide resolved
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs
Show resolved
Hide resolved
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
This PR seems to have made stuff flaky? Or for some reason flaky tests only show up with this change 😆 The three CI runs have all had different kestrel tests fail. |
Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. |
Triage: Closing this for bookkeeping reasons. Feel free to re-open if we find a way to reduce the flakiness. |
Hi @amcasey. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
I'm taking my own advice from #39588 (comment) and ensuring we lose the AsyncTestSyncContext for the duration of the Kestrel's in-memory HTTP/2 tests. Previously we were just doing this for the duration of InitializeConnectionAsync which fixed some flakiness.
By ensuring the echo app in the AcceptNewStreamsDuringClosingConnection test runs inline with
await SendDataAsync(3, _helloBytes, true);
and completes, we can ensure the final goaway gets sent without resorting to hacks like callingTriggerTick()
in a loop until a timeout.I'm not sure how applicable this fix will be after @davidfowl's #40925. I think it should still be good because Http2FrameWriter's channel runs inline in tests.
Fixes (but not really fixes because unquarantine rules)
#39479#41172