Python: Fix flaky deepcopy service tests using unpickleable async state#13456
Python: Fix flaky deepcopy service tests using unpickleable async state#13456yash27-lab wants to merge 2 commits into
Conversation
github-actions
Bot
changed the title
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
This PR updates Python unit tests to make Kernel deepcopy and model dump failure behavior deterministic by using an intentionally unpickleable service, instead of relying on OpenAI client finalization behavior during garbage collection.
Changes:
- Replaced
OpenAIChatCompletionusage in deepcopy and model dump failure tests with a minimal in test unpickleable service storing an async generator. - Removed inline comments that tied the failure to OpenAI client serialization behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class _UnserializableService: | ||
| service_id = "unserializable" | ||
|
|
||
| def __init__(self): | ||
| async def _agen(): | ||
| yield "tick" | ||
|
|
||
| # Async generator objects are not deepcopy/pickle friendly. | ||
| self._bad = _agen() | ||
|
|
||
| kernel.add_service(_UnserializableService()) |
There was a problem hiding this comment.
Kernel.add_service is typed to accept AIServiceClientBase, but this test inserts a plain object. That can make model_dump(deep=True)/model_copy(deep=True) fail for reasons unrelated to the intended “unpickleable internal state” (for example, serializer/type handling of non-AIServiceClientBase values). Consider making _UnserializableService a minimal AIServiceClientBase subclass (with required ai_model_id and a field holding the async generator) so the failure is specifically attributable to the unpickleable state while staying within the API contract.
| class _UnserializableService: | ||
| service_id = "unserializable" | ||
|
|
||
| def __init__(self): | ||
| async def _agen(): | ||
| yield "tick" | ||
|
|
||
| # Async generator objects are not deepcopy/pickle friendly. | ||
| self._bad = _agen() | ||
|
|
||
| kernel.add_service(_UnserializableService()) | ||
|
|
||
| with pytest.raises(TypeError): | ||
| # This will fail because OpenAIChatCompletion is not serializable, more specifically, | ||
| # the client is not serializable | ||
| kernel.model_dump(deep=True) | ||
|
|
||
|
|
||
| def test_kernel_deep_copy_fail_with_services(kernel: Kernel): | ||
| open_ai_chat_completion = OpenAIChatCompletion(ai_model_id="abc", api_key="abc") | ||
| kernel.add_service(open_ai_chat_completion) | ||
| class _UnserializableService: | ||
| service_id = "unserializable" | ||
|
|
||
| def __init__(self): | ||
| async def _agen(): | ||
| yield "tick" | ||
|
|
||
| # Async generator objects are not deepcopy/pickle friendly. | ||
| self._bad = _agen() | ||
|
|
||
| kernel.add_service(_UnserializableService()) |
There was a problem hiding this comment.
_UnserializableService is defined twice with identical logic. To reduce duplication and keep the two failure-path tests aligned over time, consider defining a single reusable helper (module-level class or small factory) and using it in both tests.
This PR fixes flaky kernel deepcopy tests caused by OpenAI async client
finalizers raising unraisable exceptions during garbage collection.
Instead of relying on external SDK behavior, the tests now use a minimal
unpickleable service (async generator / lambda) to deterministically
validate deepcopy and model_dump failure paths.
This makes the tests stable across Python versions and environments.
No production code changes; test-only fix.