Skip to content

test(models): exercise real connect() path in test_connect#6394

Open
anxkhn wants to merge 1 commit into
google:mainfrom
anxkhn:test/google-llm-connect-real-path
Open

test(models): exercise real connect() path in test_connect#6394
anxkhn wants to merge 1 commit into
google:mainfrom
anxkhn:test/google-llm-connect-real-path

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Link to Issue or Description of Change

2. Or, if no issue exists, describe the change:

Problem:

test_connect patched the method it is meant to exercise. It did
mock.patch("google.adk.models.google_llm.Gemini.connect", ...) at the class
level, so the real connect() body never ran and the only assertion was
connection is mock_connection, i.e. it checked that the test's own mock
returned the mock the test itself created. That is a tautology: it passes no
matter how connect() actually behaves, so the live-connect path
(live_connect_config assembly, the _live_api_client.aio.live.connect(...)
call, and GeminiLlmConnection construction) has effectively no coverage from
the test named after it.

Solution:

Patch the _live_api_client boundary instead and let the real connect() body
run, matching how the neighboring connect tests already work
(test_connect_with_custom_headers, test_connect_without_custom_headers,
test_connect_forwards_thinking_config). The test now asserts the live client
is called once with the request model and live_connect_config, and that
connect() yields a GeminiLlmConnection wrapping the live session with the
expected _api_backend and _model_version. This is a test-only change; no
production code is modified.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.
$ pytest tests/unittests/models/test_google_llm.py::test_connect
1 passed

$ pytest tests/unittests/models/test_google_llm.py -k connect
8 passed, 58 deselected

$ pytest tests/unittests/models/test_google_llm.py
66 passed

To confirm the rewritten test is no longer tautological, I temporarily
introduced three regressions in connect() and each was caught: yielding the
raw session instead of a GeminiLlmConnection, passing a fresh
LiveConnectConfig() instead of the request's, and setting a wrong
model_version. The old version of the test passed all three.

Manual End-to-End (E2E) Tests:

Not applicable. This is a unit-test-only change to an existing test; it exercises
Gemini.connect() with the live API client mocked at its boundary and adds no
new user-facing behaviour.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end. (N/A - test-only change.)
  • Any dependent changes have been merged and published in downstream modules. (N/A.)

test_connect patched google_llm.Gemini.connect (the method under test)
at the class level, so the real connect() body never ran and the only
assertion checked the test's own mock return value, a tautology that
could not fail on any regression.

Patch the _live_api_client boundary instead and let the real connect()
run, matching how the neighboring connect tests
(test_connect_with_custom_headers, test_connect_without_custom_headers,
test_connect_forwards_thinking_config) already work. Assert the live
client is called with the request model and live_connect_config, and
that connect() yields a GeminiLlmConnection wrapping the session with the
expected api_backend and model_version.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
@adk-bot adk-bot added the live [Component] This issue is related to live, voice and video chat label Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

live [Component] This issue is related to live, voice and video chat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants