aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

Fix flaky HTTP/2 test

Open halter73 opened this issue 3 years ago • 3 comments

I'm taking my own advice from https://github.com/dotnet/aspnetcore/pull/39588#issuecomment-1016025465 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 calling TriggerTick() in a loop until a timeout.

I'm not sure how applicable this fix will be after @davidfowl's https://github.com/dotnet/aspnetcore/pull/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

halter73 avatar Apr 13 '22 21:04 halter73

/azp run

BrennanConroy avatar Apr 28 '22 21:04 BrennanConroy

Azure Pipelines successfully started running 3 pipeline(s).

azure-pipelines[bot] avatar Apr 28 '22 21:04 azure-pipelines[bot]

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. https://dev.azure.com/dnceng/public/_build/results?buildId=1717072&view=ms.vss-test-web.build-test-results-tab https://dev.azure.com/dnceng/public/_build/results?buildId=1716614&view=ms.vss-test-web.build-test-results-tab https://dev.azure.com/dnceng/public/_build/results?buildId=1742997&view=ms.vss-test-web.build-test-results-tab

BrennanConroy avatar May 03 '22 21:05 BrennanConroy

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. To make sure no breaking changes are introduced, please remove the pending ci rerun label to kick off a new CI run.

ghost avatar Jan 25 '23 15:01 ghost

Triage: Closing this for bookkeeping reasons. Feel free to re-open if we find a way to reduce the flakiness.

amcasey avatar Mar 23 '23 17:03 amcasey

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.

ghost avatar Mar 23 '23 17:03 ghost