Improve HTTP/2 MultipleConnectionsEnabled tests #89788
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Potentially fixes #41078 and fixes #45204.
I ran these changes through CI with more iterations a few times in #89742 and didn't see any failures.
I think it's worth re-enabling these to see whether they are fixed or to get better exception info as to where they're failing.
The main differences are:
WhenAllCompletedOrAnyFailed
instead ofWhenAll
in a bunch of placesreadTimeout
control of H2 connections - for whatever reason these tests were using timeout values up to 10x smaller than what we would otherwise default to for other tests.PrepareConnection
test helper to no longer do a retry loop while waiting for the client setting ACK - just consider it implied based on the fact that reading & ACKing the settings is the first thing an HTTP2 connection does in SocketsHttpHandler.expectedWarmUpTasks
parameter that just seemed like a bug if we didn't immediately see the settings ACK - we would end up trying to receive more requests than we sentHandleAllPendingRequests
andAcceptRequests
to stop swallowing exceptions, so we can get more meaningful info if they do end up failing