Repository navigation
Consolidate test fixtures: one field added to WatcherLifecycle broke 19 unrelated tests #119
Description
Activity
- added a commit that references this issue
on Aug 16, 2026 Done, and the count in this issue was low
tests/helpers.pynow carriesmake_lifecycle,make_manager,make_watcher,make_core_configand oneMockAgentBackend. All ten hand-builtWatcherLifecycle.__new__sites, threemake_managercopies, four agent doubles and threemake_watchercopies are gone.Proof rather than assertion. The refactor's whole claim is "adding a field to a shared structure no longer breaks unrelated tests", so I did exactly what caused the original damage: added a new attribute to
WatcherLifecycle.__init__and ran the suite.before (adding _started): 19 failed after (adding a field): 3426 passed, 0 failedInjected after committing, removed with the inverse edit.
Two things only the migration revealed
The list was incomplete.
test_integration.pyhad its own copy ofIsolatedTestCase— a fourth kind of duplication that was not in this issue, and that I only found because removing the agent double left it undefined. Enumerating duplicates finds the ones you thought to grep for; migrating finds the rest.Keyword-only caught a real hazard.
watcher_configsis keyword-only on the shared builder. Two of the three copies took it positionally in the third slot and the third hadtimeoutthere — three signatures already drifting toward binding an argument to the wrong parameter in silence. Making it keyword-only turned that into an error at migration time.What the fixtures look like now
The four bookkeeping initialisations every fixture used to repeat (
_processors,_states,_watcher_locks,_blocked_agents) are gone from all of them — that is the constructor's job. What is left at each site is only the collaborator that test actually substitutes, which reads shorter and says more.Related: the coverage half is measured in #114. The rule is in
CLAUDE.md(Test Fixtures Are Shared By Default), including the case where not sharing is correct.
Summary
Test fixtures are copied per suite rather than shared, and the cost surfaces whenever a shared structure gains a field.
Concrete: adding one attribute to
WatcherLifecyclebroke nineteen tests at once, none of which had anything to do with the change. Every one was a hand-builtWatcherLifecycle.__new__(...)object with attributes assigned by hand, missing a field no real instance can lack.Current duplication
WatcherLifecycle.__new__hand-built objectsmake_managerhelpertest_integration.py,test_watcher_lifecycle.py,test_list_watchers.py)MockAgentBackend/_RecordingAgent)tests/helpers.pyWhy
__new__fixtures are the worst of itBypassing
__init__produces objects in states no code path can create — a record in_stateswith no corresponding processor,_roomspopulated with an empty refcount, and so on. Those do not fail where they are wrong; they fail several layers away, looking like a product bug. One such fixture was found to be passing its assertion for the wrong reason (it asserted a watermark was unchanged, in a fixture where nothing could have changed it).Shape of the fix
make_lifecycle(**overrides)/make_manager(**overrides)intests/helpers.pythat runs the real constructor and takes keyword overrides for the collaborators a test needs to substitute.__new__sites.The rule this comes from is now written down in
CLAUDE.md(Test Fixtures Are Shared By Default), including the case where not sharing is correct: reuse must not fuse two independent layers, and a helper that growsifon who called it should be split instead.Companion work
Pairs with #114 (measure coverage). Both are about the same thing from opposite ends — #114 finds what is untested, this finds what is tested in a way that resists change. Worth doing together, and worth doing before the remaining dynamic-watcher increments rather than after, since those add fields to exactly these structures.