Deflake background task removal E2E tests - #2190
Merged
Merged
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3ebc2e7-3ffd-4573-becd-bb1954344115
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3ebc2e7-3ffd-4573-becd-bb1954344115
Contributor
There was a problem hiding this comment.
Pull request overview
Updates background-agent E2E tests to reduce task-removal race failures.
Changes:
- Polls for terminal task states before removal.
- Allows automatic completion cleanup to win the removal race.
- Verifies task absence before awaiting completion notification.
Show a summary per file
| File | Description |
|---|---|
dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs |
Revises C# task completion/removal ordering. |
python/e2e/test_rpc_tasks_and_handlers_e2e.py |
Aligns Python E2E behavior with C#. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3ebc2e7-3ffd-4573-becd-bb1954344115
Contributor
There was a problem hiding this comment.
Review details
Resolved since last review (2)
- ✅ python/e2e/test_rpc_tasks_and_handlers_e2e.py#L461 — Resolved by @stephentoub - The new wait also returns when the task is absent, but the following
found_task is not None… - ✅ dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs#L155 — Resolved by @stephentoub - This still races with completion-driven cleanup: the predicate now returns as soon as the task is…
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3ebc2e7-3ffd-4573-becd-bb1954344115
Contributor
Cross-SDK Consistency Review ✅This PR modifies E2E test files only — no public SDK API surfaces are changed:
Both tests are updated in parallel to fix the same race condition (task removal racing with completion-event delivery). The logic and assertions are kept aligned across both implementations, so no consistency issues are introduced. No action needed from other SDK owners.
|
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.
Faster completion-event delivery exposed a race in the background-agent E2E test: consuming the completion notification can automatically remove the finished task before the test's explicit removal call.
This change waits for a removable terminal state and attempts explicit removal before awaiting notification completion. It permits
removed: falseonly when completion delivery has already occurred, while always asserting that the task is absent afterward. The equivalent Python test is kept aligned with the C# behavior.Validation
Generated by Copilot