Repository navigation
TEST: Harden Copilot allocation cancellation observation - #3142
Open
Richard Lundeen (richlundeen) wants to merge 1 commit into
Open
Richard Lundeen (richlundeen) wants to merge 1 commit into
Richard Lundeen (richlundeen) wants to merge 1 commit into
Conversation
Use the shared non-cancelling watchdog for allocation and cleanup waits. Check controlled observer expiry without changing caller cancellation or session cleanup ownership. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9403c88b-0662-48f4-ba33-cc11dda41f11
This branch has not been deployed
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.
Description
Fix an unrelated test timeout that blocked #3124. This branch starts from current main, which already includes #3122; that PR hardened a different lifecycle test in this file.
The full macOS Python 3.11 dev_all job log fails at
test_github_copilot_target.py:2040:asyncio.wait_for(allocated.wait(), timeout=2.0). The test had not reachedrequest_task.cancel(...), so this failure is an allocation-readiness timeout, not evidence of failed cancellation cleanup. The log does not establish why allocation missed the two-second bound. A controlled three-second delay before allocation reproduces the same failure with working cleanup.Use the existing
wait_for_completion_asynchelper, with its bounded 30-second watchdog, for allocation readiness, deletion readiness, and request completion. Unlikeasyncio.wait_for, an expired observer does not cancel the operation under test. Gate allocation and deletion with events, and add controlled zero-timeout observer cases. These check that allocation observation adds no caller cancellation, cleanup observation adds no second cancellation, and the original cancellation object, message, cause, retry behavior, and unrelated session remain intact. Teardown releases the gates and drains the request and observer tasks.Only this test changes. Production behavior and component responsibilities do not change. #3124, #3138, #3139, and the Crescendo follow-up #3140 are outside this branch.
Tests and Documentation
Local validation used Windows and Python 3.12.12:
uv sync --frozen --extra all— passed; lockfile unchanged.uv run --no-sync pytest tests\unit\prompt_target\target\test_github_copilot_target.py -k creation_is_cancelled_after_allocation -q— baseline passed, 3 tests.uv run --no-sync pytest tests\unit\prompt_target\target\test_github_copilot_target.py -q— passed, 94 tests, including all 6 updated cancellation cases.uv run --no-sync pre-commit run --all-fileswithUV_NO_SYNC=1— all hooks passed.git diff --check— passed.Controlled checks used a session-local pytest harness, not committed test tooling:
await asyncio.sleep(3)before the realPromptNormalizer._calc_hash_async. Execute the original test from main: all 3 cases fail at the original allocation-readiness wait, as expected. Execute the updated test with the same delay: all 6 cases pass.GitHubCopilotTarget._cleanup_allocated_session_asyncwith a no-opAsyncMock: all 6 updated cases fail because deletion never starts, as expected. The watchdog change does not hide lost cleanup.The new committed
expired-watchdogcases provide deterministic checks without wall-clock sleeps. macOS/Python 3.11 validation is left to CI. Documentation and notebook execution are not applicable to this test-only change.