Fix .NET E2E event capture race#1221
Merged
stephentoub merged 2 commits intomainfrom May 7, 2026
Merged
Conversation
Use a thread-safe collection for SessionE2ETests event type capture so assertions do not enumerate a List while background event delivery appends to it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a flaky .NET E2E test race by making the test-side session event capture collection safe for concurrent enqueue (from background event dispatch) and enumeration (during assertions).
Changes:
- Switched event-type capture in two
SessionE2ETestscases fromList<string>toConcurrentQueue<string>. - Updated the event handler to use
Enqueueand added the requiredSystem.Collections.Concurrentimport.
Show a summary per file
| File | Description |
|---|---|
| dotnet/test/E2E/SessionE2ETests.cs | Makes event capture thread-safe to prevent concurrent-modification failures during E2E assertions. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0
Run long-lived shell edge-case commands from the system temp directory so killed or timed-out child processes cannot keep the fixture workspace directory locked on Windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| ? $"echo started>\"{startedPath}\" & for /L %i in (1,1,2147483647) do @rem & echo should-not-exist>\"{markerPath}\"" | ||
| : $"printf 'started' > '{startedPath}'; sleep 30; printf 'should-not-exist' > '{markerPath}'"; | ||
|
|
||
| var result = await session.Rpc.Shell.ExecAsync(command, timeout: TimeSpan.FromMilliseconds(200)); |
| session.On(evt => events.Enqueue(evt.Type)); | ||
|
|
||
| // Use a slow command so we can verify SendAsync() returns before completion | ||
| await session.SendAsync(new MessageOptions { Prompt = "Run 'sleep 2 && echo done'" }); |
There was a problem hiding this comment.
Transfer my money back
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.
This fixes two .NET E2E flakes caused by test-side races rather than SDK behavior regressions.
Changes
ConcurrentQueue<string>for event type capture in the affectedSessionE2ETestscases so assertions can safely enumerate while background event delivery may still append.Validation
dotnet build dotnet\test\GitHub.Copilot.SDK.Test.csproj --no-restoredotnet format dotnet\test\GitHub.Copilot.SDK.Test.csproj --verify-no-changes --include dotnet\test\E2E\SessionE2ETests.csdotnet format dotnet\test\GitHub.Copilot.SDK.Test.csproj --verify-no-changes --include dotnet\test\E2E\RpcShellEdgeCaseE2ETests.csdotnet test dotnet\test\GitHub.Copilot.SDK.Test.csproj --filter "FullyQualifiedName~RpcShellEdgeCaseE2ETests" --logger "console;verbosity=minimal"The targeted
SessionE2ETests.SendAndWait_Blocks_Until_Session_Idle_And_Returns_Final_Assistant_MessageE2E path gets past local dependency startup, but cannot complete in this environment because the CLI auth token lookup fails withFailed to get token for auth info.