test: fail the test that leaks a bare src/core module stub

#25 fixed two sys.modules writes in test_auth_regressions.py that left empty
stub modules behind for the rest of the session, breaking 23 tests under one
collection order while the full suite stayed green. The class is wider than
that file, and an audit is the wrong answer to it: nothing stops the next one,
and the failure it causes lands on an unrelated test in a different file.

So this is a guard instead. An autouse fixture in the root conftest snapshots
which src.* / core.* names are bound to a bare ModuleType, and fails any test
that adds one. "Bare" is the same test the clear_fake_* helpers already use -
a plain types.ModuleType with no on-disk __file__. MagicMock stand-ins are out
of scope: they answer every attribute, so they fail at the point of use rather
than silently, and several files install them deliberately.

Three details that matter:

- It lives in the root conftest, so it is set up before any test-module
  fixture and torn down after all of them. A stub a test's own teardown
  removes is not reported.
- It drops the leaked entries as well as reporting them, so the failure stays
  on the test that introduced it instead of cascading through the rest of the
  run.
- It only reports stubs added during the test. Import state the session starts
  with, including this conftest's own src.database stub, is left alone.

It found one beyond #25 on the first full run: _stub_heavy in
test_scheduler_restart_doublefire.py leaks the same five src.* modules as the
test #25 fixed, via sys.modules.setdefault. It already receives monkeypatch,
so the fix is to register through it. Fixed here because the guard has to land
green.

Full suite, macOS, default collection order:

  this branch   10655 passed, 6 failed, 6 skipped   406s
  lab           10655 passed, 6 failed, 6 skipped   371s

Same six either way, which is the point - none of this is visible in the
default order. Four are pre-existing macOS environment failures:
test_glob_confined_e2e and the two test_code_nav_tools document cases resolve
/tmp to /private/tmp, and
test_real_socket_falls_back_from_dead_first_to_live_second is connect-refused
timing on real sockets. The other two are the rich-colour and ffmpeg items
from the same ledger, fixed on their own branches.

Not verified: Linux, and any collection order other than the default. The
guard is order-independent by construction - it compares before and after
within a single test - but I have only run the default order.
This commit is contained in:
Léo
2026-09-30 13:02:34 +02:00
parent eb98aa6dc2
commit fba6f73260
3 changed files with 83 additions and 3 deletions
+40
View File
@@ -159,3 +159,43 @@ def _serve_test_static():
os.environ["ODYSSEUS_TEST_STATIC_ORIGIN"] = previous_origin
server.shutdown()
server.server_close()
@pytest.fixture(autouse=True)
def _no_leaked_module_stubs():
"""Fail the test that leaves a bare ``src.*``/``core.*`` stub behind.
Several test modules install empty stand-in modules so an import-heavy
production module can be loaded under the mocks above. When one of those
writes is not undone, the stub stays in ``sys.modules`` for the rest of the
session and every later test that imports the real module silently gets an
empty one instead. The suite still passes as a whole, because the victims
usually run before the leak; it only breaks under a different collection
order, which is why this class of bug reaches CI green.
This fixture is declared in the root conftest, so it is set up before any
test-module fixture and torn down after all of them — a stub that a test's
own teardown removes is not reported. The leaked entries are dropped here
as well as reported, so the failure stays attributed to the test that
introduced it instead of cascading into the rest of the run.
Bare stubs present before the test starts are ignored: this guards against
new leaks, it does not police import state the session began with.
"""
from tests.helpers.import_state import bare_module_stubs, clear_module
before = bare_module_stubs()
yield
leaked = sorted(bare_module_stubs() - before)
if not leaked:
return
for name in leaked:
clear_module(name)
pytest.fail(
"test left bare module stub(s) in sys.modules: "
+ ", ".join(leaked)
+ ". Register the stub through monkeypatch.setitem(sys.modules, ...) "
"or tests.helpers.import_state.preserve_import_state so it is undone "
"at teardown.",
pytrace=False,
)
+31
View File
@@ -31,6 +31,7 @@ safe for callers that pass both a parent package and a child module.
"""
import sys
import types
from contextlib import contextmanager
_ABSENT = object()
@@ -167,3 +168,33 @@ def preserve_import_state(*module_names):
# Phase 2: restore all parent-package attributes.
for name, (_, saved_attr) in saved.items():
_restore_parent_attr(name, saved_attr)
# Names under these prefixes are the ones a leaked stub actually breaks: a
# later test doing ``import src.x`` or ``import core.x`` silently gets the
# empty stub instead of the real module.
_GUARDED_PREFIXES = ("src.", "core.")
def bare_module_stubs():
"""Return the ``src.*``/``core.*`` names currently bound to a bare stub.
A bare stub is a plain :class:`types.ModuleType` with no on-disk
``__file__`` — the object ``types.ModuleType(name)`` produces. That is the
same "is this a fake?" test the ``clear_fake_*`` helpers above use, so a
module imported from disk is never reported.
``MagicMock`` stand-ins are deliberately out of scope: they answer every
attribute, so they fail loudly at use rather than silently, and several
test modules install them on purpose.
"""
found = set()
for name, mod in list(sys.modules.items()):
if not name.startswith(_GUARDED_PREFIXES):
continue
if type(mod) is not types.ModuleType:
continue
if getattr(mod, "__file__", None):
continue
found.add(name)
return found
+12 -3
View File
@@ -21,12 +21,21 @@ def _test_utcnow():
return datetime.now(timezone.utc).replace(tzinfo=None)
def _stub_heavy():
def _stub_heavy(monkeypatch):
"""Stub the heavy modules ``task_scheduler`` imports, for this test only.
Registered through ``monkeypatch.setitem`` so every entry is removed at
teardown. A bare ``sys.modules[name] = ...`` leaves an empty module behind
for the rest of the session, and any later test that imports the real one
silently gets the stub instead - a failure that only shows up under a
different collection order.
"""
for name in [
"src.builtin_actions", "src.ai_interaction", "src.endpoint_resolver",
"src.agent_loop", "src.session_manager",
]:
sys.modules.setdefault(name, types.ModuleType(name))
if name not in sys.modules:
monkeypatch.setitem(sys.modules, name, types.ModuleType(name))
def _setup_isolated_db():
@@ -74,7 +83,7 @@ def test_scheduler_utcnow_preserves_naive_utc_contract():
def _drive_scheduler(monkeypatch, pre_start_setup=None):
"""Build a TaskScheduler bypassing __init__ and run start() + two polls."""
_stub_heavy()
_stub_heavy(monkeypatch)
cd, ScheduledTask, TaskRun = _setup_isolated_db()
from src.task_scheduler import TaskScheduler