Fix flaky .NET session resume E2E test - #2109
Merged
Merged
Conversation
Subscribe for completion events before sending the initial prompt so a fast response cannot be missed between SendAsync and the post-send test helper subscription. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a race in the .NET session-resume E2E test by subscribing before sending the initial message.
Changes:
- Replaces separate send/wait calls with
SendAndWaitAsync. - Preserves resume and continuation assertions.
Show a summary per file
| File | Description |
|---|---|
dotnet/test/E2E/SessionE2ETests.cs |
Makes initial response handling race-safe. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Medium
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.
Summary
SendAndWaitAsyncfor the initial message in the .NET session-resume E2E testFailure analysis
Run 30356278419 / job 90265115048 failed only in
Should_Resume_A_Session_Using_A_New_Client, which timed out for two minutes inTestHelper.GetFinalAssistantMessageAsyncwhile waiting for the initial1+1response. The remaining 457 tests passed.The test called
SendAsyncbefore installing the helper subscription. A fast response could therefore deliver its assistant and idle events before the subscription existed, leaving completion dependent on a separate post-sendGetEventsAsyncbackfill. This is the same synchronization race addressed for the ask-user tests in #2107.SendAndWaitAsyncsubscribes before sending and removes that window.Validation
dotnet format --verify-no-changes --no-restoredotnet build --no-restore