motief/thoughts/shared/designs/2026-03-23-test-refactor-no-mocks-design.md

date	topic	status
2026-03-23	Test refactor: replace monkeypatching with real implementations	validated

Test Refactor — Remove Monkeypatching, Use Real Implementations

Problem Statement

The new test suite (added during the motion content enrichment hardening sprint) relies on:

• sys.modules injection of a fake duckdb module
• monkeypatch.setattr to replace pipeline functions wholesale
• Exception-swallowing smoke tests that verify nothing meaningful

These tests give false confidence. They patch away the very code paths we want to exercise. The goal is to replace them with tests that run real production code paths using in-memory DuckDB and injected fake callables.

Constraints

• Do NOT modify app.py or scheduler.py
• Keep the module-level db = MotionDatabase() singleton in database.py — do not remove it (production code depends on it)
• Production-code changes must be minimal and backwards-compatible (all existing callers continue to work with default parameters)
• Tests run with .venv/bin/python -m pytest -q
• No print() in library modules — use logging.getLogger(__name__)

Approach: Dependency Injection + In-Memory DuckDB

Thread an optional db: MotionDatabase = None parameter through the three pipeline functions that currently hard-import the singleton. Pass MotionDatabase(":memory:") in tests. Replace fake ai_provider calls with a real FakeEmbedder callable injected as a parameter.

Rejected alternatives:

• Repository/Protocol pattern — major refactor of database.py and all callers; YAGNI for current test coverage needs
• tmp_path real DB files — slower, requires full schema migration in fixtures, still needs singleton patching

Architecture

Production Changes (minimal, backwards-compatible)

pipeline/ai_provider_wrapper.py

• Add db: MotionDatabase = None param to get_embeddings_with_retry
• Add embedder=None param (callable replacing get_embeddings_batch)
• If None, fall back to imported singleton / real provider

pipeline/text_pipeline.py

• ensure_text_embeddings: already accepts db_path; also accept db: MotionDatabase = None as override (takes precedence over db_path)
• ensure_text_embeddings_for_ids: same db override param
• _select_text: accept db: MotionDatabase (already does); when db.db_path == ":memory:" the function returns [] (no motions seeded at that level) — this is fine for retry-path tests

similarity/compute.py

• compute_similarities: already constructs MotionDatabase(db_path) locally; add db: MotionDatabase = None override param

Test Infrastructure (tests/conftest.py)

New fixtures added alongside existing ones:

mem_db fixture (function-scoped)

• Creates MotionDatabase(":memory:") — triggers _init_database() which creates all tables/sequences in-memory
• Seeds a minimal set of motions rows for tests that need them
• Returned to test; closed after test

fake_embedder fixture

• Returns a FakeEmbedder instance — a real callable class with signature (texts, model=None, batch_size=50) -> list[list[float]]
• Returns deterministic embeddings: [[0.1] * 1536] per text
• Accepts fail_ids: list[int] kwarg to simulate per-item permanent failures (raises RuntimeError for those items)

Components

FakeEmbedder (tests/conftest.py)

• Real class, not a Mock
• __call__(texts, model=None, batch_size=50) — returns list of float vectors
• fail_ids set at construction time: any call where the batch index is in fail_ids raises RuntimeError
• Tracks call_count for assertions

mem_db fixture

• MotionDatabase(":memory:") — schema initialized by __init__
• Optionally seeds motions via db.insert_motion(...) for tests that need real data
• No filesystem side effects

Data Flow

Retry test (replaces test_rerun_embeddings_retry.py)

1. mem_db fixture creates in-memory DB
2. FakeEmbedder(fail_ids=[101, 102]) created
3. Call ensure_text_embeddings(db=mem_db, embedder=fake_embedder)
4. Wrapper internally retries failed items
5. Assert fake_embedder.call_count > 1 (retries happened)
6. Assert failed_ids returned contains [101, 102]

Audit event test (replaces test_database_audit.py)

1. mem_db fixture — in-memory DB with audit_events table
2. Call mem_db.append_audit_event(None, "test_action", ...)
3. Query audit_events table directly: assert row exists
4. No filesystem writes

Similarity filter test (replaces test_similarity_compute_filter.py)

1. mem_db seeded with 2 motions with identical short titles ("Aangenomen.")
2. Embeddings seeded for both motions (identical vectors → score 1.0)
3. Call compute_similarities(vector_type="fused", window_id=None, db=mem_db)
4. Assert store_similarity_batch stored 0 pairs (filtered out)

Embedding failure + audit event test (new, test_ai_provider_wrapper.py)

1. mem_db fixture
2. FakeEmbedder(fail_ids=[5]) — motion 5 permanently fails
3. Call get_embeddings_with_retry(texts=[...], motion_ids=[5], db=mem_db, embedder=fake_embedder)
4. Assert return contains failure sentinel for motion 5
5. Query mem_db audit_events table: assert row with action="embedding_failed", target_id=5

Error Handling Strategy

• _select_text with in-memory DB returns [] (no rows) — callers treat empty list as "no motions to embed", which is correct
• FakeEmbedder.fail_ids raises RuntimeError to exercise the retry/backoff path without sleeping — the wrapper's retry loop catches Exception, so this exercises the real retry path
• Audit event writes go to the in-memory audit_events table; the JSON ledger fallback path is tested separately if needed

Testing Strategy

All tests use .venv/bin/python -m pytest -q. No sys.modules tricks. No monkeypatch.setattr on production module attributes.

Test file	What it exercises
test_database_audit.py	append_audit_event → in-memory DB row
test_ai_provider_wrapper.py	Empty input; transient retry; permanent fail + audit event
test_rerun_embeddings_retry.py	Real ensure_text_embeddings → retry path → ensure_text_embeddings_for_ids called
test_similarity_compute_filter.py	Real compute_similarities → filter removes perfect-score identical-title pairs

Open Questions

1. Does MotionDatabase(":memory:").__init__ successfully create the audit_events table? (Needs verification — if _init_database conditionally skips it, the fixture needs to run it explicitly.)
2. Does _select_text need the DB connection passed directly rather than reconstructed from db.db_path? For :memory: DBs, a new duckdb.connect(":memory:") opens an empty DB, not the seeded one. Resolution: ensure_text_embeddings should pass the seeded connection rather than the path, OR accept that _select_text returns [] for in-memory DBs (sufficient for retry-path tests).