tests(runners): require fresh session fetch for persisted event counts#4596
tests(runners): require fresh session fetch for persisted event counts#4596davidahmann wants to merge 1 commit intogoogle:mainfrom
Conversation
Summary of ChangesHello @davidahmann, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where assertions on persisted event counts could become unreliable due to tests relying on outdated session data after asynchronous operations. By introducing a dedicated regression test, it ensures that future validations of event counts are performed against the most current session state, thereby improving the determinism and reliability of test results and operational workflows. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Added a regression test that demonstrates stale session snapshots after run_async and enforces persisted-event assertions against a freshly fetched session state. This contribution was informed by patterns from Wrkr: https://github.com/Clyra-AI/wrkr |
adk-bot
added
the
core
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a valuable regression test to ensure that persisted event counts are asserted against a freshly fetched session, addressing potential issues with stale data. The test is well-written and effectively demonstrates the intended behavior. I have one suggestion to refactor a small part of the test to improve code clarity and reduce duplication.
| stale_session = await runner.runner.session_service.get_session( | ||
| app_name=runner.app_name, | ||
| user_id="test_user", | ||
| session_id=runner.session_id, | ||
| ) | ||
| assert stale_session is not None | ||
|
|
||
| second_invocation_events = await runner.run_async("hello again") | ||
|
|
||
| # This session snapshot was retrieved before the second invocation and should | ||
| # not be used for final persisted-event assertions. | ||
| stale_count = len(stale_session.events) | ||
| fresh_session = await runner.runner.session_service.get_session( | ||
| app_name=runner.app_name, | ||
| user_id="test_user", | ||
| session_id=runner.session_id, | ||
| ) | ||
| assert fresh_session is not None | ||
| fresh_count = len(fresh_session.events) |
There was a problem hiding this comment.
To improve readability and reduce code duplication, the session fetching logic can be extracted into a local helper function. This makes the test's intent clearer and the code easier to maintain.
| stale_session = await runner.runner.session_service.get_session( | |
| app_name=runner.app_name, | |
| user_id="test_user", | |
| session_id=runner.session_id, | |
| ) | |
| assert stale_session is not None | |
| second_invocation_events = await runner.run_async("hello again") | |
| # This session snapshot was retrieved before the second invocation and should | |
| # not be used for final persisted-event assertions. | |
| stale_count = len(stale_session.events) | |
| fresh_session = await runner.runner.session_service.get_session( | |
| app_name=runner.app_name, | |
| user_id="test_user", | |
| session_id=runner.session_id, | |
| ) | |
| assert fresh_session is not None | |
| fresh_count = len(fresh_session.events) | |
| async def _fetch_session(): | |
| session = await runner.runner.session_service.get_session( | |
| app_name=runner.app_name, | |
| user_id="test_user", | |
| session_id=runner.session_id, | |
| ) | |
| assert session is not None | |
| return session | |
| stale_session = await _fetch_session() | |
| second_invocation_events = await runner.run_async("hello again") | |
| # This session snapshot was retrieved before the second invocation and should | |
| # not be used for final persisted-event assertions. | |
| stale_count = len(stale_session.events) | |
| fresh_session = await _fetch_session() | |
| fresh_count = len(fresh_session.events) |
2 participants
Problem
Persisted-event assertions can drift when tests rely on stale session snapshots after run_async invocations.
Why now
This is active operator friction in current workflows and needs deterministic behavior for repeatable triage/replay.
What changed
Added a regression test that demonstrates stale snapshot behavior and asserts counts using a freshly fetched persisted session.
Validation
Refs #4595