Skip to content

Add Ollama client tests#36

Open
ElyssaN wants to merge 1 commit into
mainfrom
11-add-ollama-client-tests
Open

Add Ollama client tests#36
ElyssaN wants to merge 1 commit into
mainfrom
11-add-ollama-client-tests

Conversation

@ElyssaN

@ElyssaN ElyssaN commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@ElyssaN ElyssaN linked an issue Jul 16, 2026 that may be closed by this pull request
from src.infrastructure import llm


def mock_async_client(json_data=None, raise_for_status_side_effect=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test behaviour looks correct! One code style suggestion:

The mock_async_client helper currently uses patcher.start() and every test needs its own try/finally: patcher.stop() to clean up. If you make the helper a @contextmanager, cleanup becomes automatic and the per-test boilerplate goes away. The response/client setup at the top is unchanged — only the patcher.start() lines and the return change:

  • Add from contextlib import contextmanager at the top
  • Add @contextmanager above the helper definition
  • Setup stays the same up to client.post = AsyncMock(return_value=response). After that, replace:
patcher = patch("src.infrastructure.llm.httpx.AsyncClient")
mocked_async_client_cls = patcher.start()

with

with patch("src.infrastructure.llm.httpx.AsyncClient") as mocked_async_client_cls:

and put the rest of the lines indented inside the with block

  • The final return would just become yield client, response

Then each test can drop the patcher, client, _ = ... and try/finally lines and just have something like:

with mock_async_client(json_data={"embedding": fake_embedding}) as (client, _):
    <test code...>

If a test doesn't use client/response (like the one on line 64 currently), you can drop the as (client, _) entirely and just write with mock_async_client(...):

These changes help trim some boilerplate code in the tests and help prevent a hypothetical future test from forgetting the stop() for cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Ollama client tests

2 participants