diff --git a/tests/conftest.py b/tests/conftest.py index 5fcf02113..97e249b41 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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, + ) diff --git a/tests/helpers/import_state.py b/tests/helpers/import_state.py index 0eea62d9d..f58c86c37 100644 --- a/tests/helpers/import_state.py +++ b/tests/helpers/import_state.py @@ -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 diff --git a/tests/test_scheduler_restart_doublefire.py b/tests/test_scheduler_restart_doublefire.py index 9f0c87372..ca90c55bc 100644 --- a/tests/test_scheduler_restart_doublefire.py +++ b/tests/test_scheduler_restart_doublefire.py @@ -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