Skip to content

Tests can make real LLM calls: no guard on the chat-completion seams #851

Description

@lmorchard

Problem

Nothing in the test suite prevents a test from making a real LLM request. A test that stubs one of the two LLM seams passes on any machine that can reach the configured endpoint and fails only in CI — so local green is not evidence the call was stubbed.

conftest.py already has the autouse fixture for this class of problem, but only for embeddings:

@pytest.fixture(autouse=True)
def mock_embed_text(monkeypatch, config):
    """Globally mock embed_text to prevent real network calls ..."""

Chat completions never got the same guard.

Why one patch isn't enough

agent._call_llm_with_events branches on resolve_streaming and the two branches reach the LLM through different objects:

  • non-streaming → agent.call_llm, imported at module level (agent.py:36)
  • streaming → call_llm_streaming, imported from .llm inside the function (agent.py:419)

So monkeypatch.setattr(agent, "call_llm", fake) leaves the streaming path live. Two defaults make that the path actually taken:

  • LLMConfig.streaming defaults to True
  • config.llm.url defaults to http://192.168.0.199:4000/v1/chat/completions, a LAN address

The result is a test that issues a real request, passes on a machine on that LAN, and fails in CI with httpx.ConnectError: All connection attempts failed.

Observed instance

Two tests added in #848 (test_llm_end_carries_resolved_model_on_default_path, test_llm_end_prefers_explicit_model_override) did exactly this. Locally: 3895 passed. In CI: both failed with ConnectError after 4m50s. Fixed in that PR by stubbing both seams and asserting the stub recorded the call — but only because CI caught it, and the next test to make the same mistake gets no protection by default.

Proposed fix

Extend the existing convention — an autouse fixture in conftest.py that stubs both LLM seams, mirroring mock_embed_text:

@pytest.fixture(autouse=True)
def mock_call_llm(monkeypatch):
    """Stub both LLM seams. Tests that exercise the real client opt out."""

Two design questions to settle when implementing:

  1. Opt-out mechanism. Tests that genuinely want the real code path (provider-level tests, test_provider_registry.py) need a way out — a marker, or requesting a fixture that undoes it.
  2. Should the stub fail loudly instead of returning a canned response? Raising AssertionError("unstubbed LLM call") by default is stricter and surfaces the mistake at the call site, rather than letting a test pass against a fake response it didn't ask for. Probably the better default, but it will need a per-test stub fixture to pair with it.

Optional follow-up: ban sockets outright

pytest-socket (--disable-socket --allow-unix-socket plus a socket_enabled marker) would cover every network seam, not just the LLM one. Deliberately listed as a follow-up rather than the fix: ~15 test files reference localhost / 127.0.0.1 / uvicorn and would each need triage for whether they bind real loopback sockets or just pass URLs to an in-process TestClient. Worth doing only if this recurs in a seam other than the LLM client.

Done when

  • A test that forgets to stub an LLM seam fails locally, on a machine with the endpoint reachable, for the same reason it fails in CI.
  • The escape hatch for tests that want the real client is documented in docs/ or in the fixture docstring.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions