Quarantine POST_MultipleRequests_PooledStreamAndHeaders - #68167
Open
Vinoth2562000 wants to merge 2 commits into
Open
Quarantine POST_MultipleRequests_PooledStreamAndHeaders#68167Vinoth2562000 wants to merge 2 commits into
Vinoth2562000 wants to merge 2 commits into
Conversation
Contributor
|
Thanks for your PR, @Vinoth2562000. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR unquarantines POST_MultipleRequests_PooledStreamAndHeaders by replacing a polling-based log wait with an event-driven wait tied to TestSink.MessageLogged, aiming to eliminate intermittent CI failures while preserving the Assert.Same cache-miss detection.
Changes:
- Removed the
[QuarantinedTest]attribute forPOST_MultipleRequests_PooledStreamAndHeaders. - Replaced
WaitForLogAsyncpolling for the QUIC"StreamPooled"log with an event-drivenTaskCompletionSourcethat completes when the log is emitted. - Added
try/finallycleanup to unsubscribe theMessageLoggedhandler.
Comment on lines
+338
to
+339
| TestSink.MessageLogged += handler; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Quarantine POST_MultipleRequests_PooledStreamAndHeaders
Description
This unquarantines the test, which failed intermittently on CI due to a coarse 100ms poll for the
"StreamPooled"log.Root cause
The test polled for the
"StreamPooled"log with a 100ms initial delay. On CI, the second request'sAcceptAsyncsometimes ran on a freshly allocatedHttpRequestHeadersbefore the first stream was pushed to the pool — soAssert.Samefailed (same value, different reference).Fix
Replace the polling
WaitForLogAsyncwith an event-driven wait onTestSink.MessageLogged:_poolLock, right afterStreamPool.Push)try/finallyensures the handler is unsubscribed on test failureAssert.Sameis preserved — the test still detects cache misses (its original purpose).Screenshot
Fixes #52573