test(google): disarm GenAI client finalizers to prevent task leak across unit tests (#6881) - #7247
test(google): disarm GenAI client finalizers to prevent task leak across unit tests (#6881)#7247rrfunde wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Devin Review: 2 flags
Not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ba8d8dfb1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
|
|
||
| # Disarm finalizers on classes as well to prevent aclose() tasks scheduling on active event loops | ||
| AsyncClient.__del__ = lambda self: None # type: ignore[assignment] |
There was a problem hiding this comment.
When this module is collected alongside other Google tests, this assignment runs during collection and is never restored, so every google.genai.AsyncClient created anywhere in that pytest process loses its normal finalizer. Unfiltered or combined-category runs can consequently leak clients from unrelated tests and exercise different lifecycle behavior depending on whether this module was collected; use a fixture/monkeypatch with teardown or limit the change to the session instances.
Useful? React with 👍 / 👎.
| if coro_name.endswith(".aclose"): | ||
| frame = getattr(coro, "cr_frame", None) | ||
| module = frame.f_globals.get("__name__", "") if frame else "" | ||
| if module.startswith("google.genai"): | ||
| return True |
There was a problem hiding this comment.
If code under test forgets to close a GenAI client and its finalizer schedules a pending aclose(), this branch now classifies that actual cleanup task as ignorable, so fail_on_leaked_tasks passes instead of detecting the regression. The module-specific fixture can drain known finalizer tasks, but the global checker should not suppress every google.genai aclose() created by any test.
Useful? React with 👍 / 👎.
…obal conftest change
|
Addressed feedback: reverted the global GenAI check in |
1 participant
Summary
google.genai.client.AsyncClient.__del__andBaseApiClient.__del__intests/test_plugin_google_realtime.pyso that asynchronous client finalizers do not scheduleaclose()tasks on subsequent unrelated tests' event loops during GC.gc.collect()and task drain in_make_sessionand_make_configured_session.tests/conftest.pyso boolean category flags (like--unit) do not swallow subsequent test file arguments.Fixes #6881