Skip to content

test: consolidate scripted MockClient doubles into a shared conftest fixture#76

Open
SuperMarioYL wants to merge 2 commits into
antoinezambelli:mainfrom
SuperMarioYL:tests/consolidate-mockclient-fixture
Open

test: consolidate scripted MockClient doubles into a shared conftest fixture#76
SuperMarioYL wants to merge 2 commits into
antoinezambelli:mainfrom
SuperMarioYL:tests/consolidate-mockclient-fixture

Conversation

@SuperMarioYL
Copy link
Copy Markdown

Summary

The runner, slot-worker, and eval-budget unit tests each ship their own
hand-rolled mock LLMClient. The three started as copies and have since
quietly diverged — they now disagree on observable behavior, so picking
"a" MockClient no longer gives a predictable contract.

Behavior test_runner.py test_slot_worker.py test_eval_budget.py
exhausted responses raises IndexError raises IndexError returns TextResponse("stuck")
send_stream TEXT_DELTA + FINAL raises NotImplementedError FINAL only
call-spy lists yes no no
api_format attr absent absent "ollama"

The exhaustion split is the riskiest: a test moved between files (or a new test
copying the "wrong" double) silently changes from a hard error to a soft
fallback, which can mask a real defect.

Change

Replaces the three local classes with one configurable MockClient in
tests/conftest.py (previously empty). It is a behavioral superset, with
the variations promoted to explicit constructor knobs:

  • on_exhausted"raise" (default) or a fallback response object
  • stream_mode"deltas" (default) / "final" / "unsupported"
  • api_format, context_length — defaulted to the common case
  • call-spy lists (send_calls / send_stream_calls) are always populated

Each test file imports the shared class and passes only the kwargs needed to
reproduce its original behavior. No test assertion or test body was changed —
only the mock's source moved. The single-use inline NoFinalClient in
test_runner.py is intentionally left in place; it is a one-off for one
negative-path test, not a duplicate.

Why not just keep three mocks

A single shared double is only safe if it does not erase the per-test behavior
the three variants encoded — so the behavior is made configurable, not
collapsed. Each call site keeps its exact prior semantics, and the contract is
now visible at the construction site instead of hidden in three drifting class
bodies.

Test plan

  • pytest tests/unit/test_runner.py tests/unit/test_slot_worker.py tests/unit/test_eval_budget.py — 105 passed, unchanged from before.
  • pytest tests/unit/ — full suite green.

…fixture

The runner, slot-worker, and eval-budget unit tests each carried their own
hand-rolled mock LLM client. The three had quietly diverged: differing
exhaustion behavior (IndexError vs. a fallback TextResponse), differing
stream semantics (text deltas, FINAL-only, or unsupported), and uneven
call-spy and api_format coverage.

Replace them with one configurable MockClient in tests/conftest.py that is
a behavioral superset, with knobs (on_exhausted, stream_mode, api_format,
context_length) defaulting to the most common case. Each test file imports
the shared class and passes only the kwargs needed to reproduce its
original behavior; no test assertions or bodies changed.
Removing the three local mock classes left their type-only imports
(AsyncIterator, LLMResponse, ChunkType, StreamChunk, ToolSpec) unused;
drop the ones no longer referenced.
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.

1 participant