From eabdf84669179d2fe88e239f39b2762369e09d43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o?= Date: Wed, 30 Sep 2026 10:36:00 +0200 Subject: [PATCH] fix(tests): stop test_auth_regressions leaking stub modules Running test_auth_regressions.py before the email modules failed 23 tests that pass in isolation: pytest -p no:randomly tests/test_auth_regressions.py \ tests/test_email_urgency_checkpoint.py 23 failed, 15 passed (38 passed in the reverse order) Every failure was ImportError "cannot import name X (unknown location)" against a module already in sys.modules, which is what an empty stub module looks like to a later import. Two writes leaked, both in this file: - test_pop_notifications_owner_filtered inserted five empty stub modules with a bare sys.modules[name] = mod and never removed them. - _ensure_stub wrote its stub into sys.modules itself, so the autouse fixture's monkeypatch.setitem three lines later captured that stub as the value to restore. The fixture looked like it cleaned up and could not. Both now go through monkeypatch, including the parent-package stub and the attribute wiring, so everything is undone at teardown. The redundant setitem calls in the fixture are gone: re-setting a key whose stub is already installed is what made the leak invisible. Pre-existing on lab, not introduced by any open PR. Verified against the email subpackage branch too: identical numbers there before this fix. --- tests/test_auth_regressions.py | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/tests/test_auth_regressions.py b/tests/test_auth_regressions.py index 62b479748..b7403f4ca 100644 --- a/tests/test_auth_regressions.py +++ b/tests/test_auth_regressions.py @@ -21,7 +21,7 @@ from unittest.mock import MagicMock # (Same trick as test_null_owner_gates.py — the real modules instantiate # SQLAlchemy declarative classes at import-time which blow up under the # conftest's `sqlalchemy.*` MagicMock stubs.) -def _ensure_stub(name: str, **attrs): +def _ensure_stub(monkeypatch, name: str, **attrs): """Create or augment a stub module with the given attributes. Augments existing entries because earlier-run tests may have already stubbed the same module with a different attribute set. @@ -48,7 +48,7 @@ def _ensure_stub(name: str, **attrs): *parent_name.split("."), ) parent.__path__ = [real_path] if os.path.isdir(real_path) else [] - sys.modules[parent_name] = parent + monkeypatch.setitem(sys.modules, parent_name, parent) else: parent = sys.modules[parent_name] else: @@ -58,17 +58,17 @@ def _ensure_stub(name: str, **attrs): mod = sys.modules.get(name) if mod is None: mod = types.ModuleType(name) - sys.modules[name] = mod + monkeypatch.setitem(sys.modules, name, mod) for k, v in attrs.items(): if not hasattr(mod, k): - setattr(mod, k, v) + monkeypatch.setattr(mod, k, v, raising=False) if parent is not None and not hasattr(parent, child_name): - setattr(parent, child_name, mod) + monkeypatch.setattr(parent, child_name, mod, raising=False) return mod @pytest.fixture(autouse=True) def _auth_regressions_stubs(monkeypatch): - db = _ensure_stub("core.database", + db = _ensure_stub(monkeypatch, "core.database", SessionLocal=MagicMock(), ScheduledTask=MagicMock(), TaskRun=MagicMock(), ModelEndpoint=MagicMock(), Session=MagicMock(), ChatMessage=MagicMock(), CalendarCal=MagicMock(), CalendarEvent=MagicMock(), @@ -76,17 +76,18 @@ def _auth_regressions_stubs(monkeypatch): GalleryImage=MagicMock(), GalleryAlbum=MagicMock(), Note=MagicMock(), McpServer=MagicMock(), ) - auth = _ensure_stub("core.auth", AuthManager=MagicMock()) - ep = _ensure_stub("src.endpoint_resolver", + auth = _ensure_stub(monkeypatch, "core.auth", AuthManager=MagicMock()) + ep = _ensure_stub(monkeypatch, "src.endpoint_resolver", resolve_endpoint=MagicMock(return_value=("", "", {})), normalize_base=MagicMock(), build_chat_url=MagicMock(), build_models_url=MagicMock(), build_headers=MagicMock(), ) - monkeypatch.setitem(sys.modules, "core.database", db) - monkeypatch.setitem(sys.modules, "core.auth", auth) - monkeypatch.setitem(sys.modules, "src.endpoint_resolver", ep) + # _ensure_stub now registers each stub through monkeypatch itself, so the + # whole set is undone at teardown. Re-setting them here would capture the + # stub as the restore target and leave it behind for the rest of the run. + assert db and auth and ep from fastapi import HTTPException @@ -293,7 +294,7 @@ def test_research_spinoff_rejects_wrong_owner(): # pop_notifications owner filter # --------------------------------------------------------------------------- -def test_pop_notifications_owner_filtered(): +def test_pop_notifications_owner_filtered(monkeypatch): """pop_notifications(owner='alice') must return only alice's items. bob's and legacy ownerless items stay behind in the queue.""" # Build a minimal scheduler instance that we can hit directly. @@ -302,11 +303,15 @@ def test_pop_notifications_owner_filtered(): import sys, types from unittest.mock import MagicMock as _MM # `task_scheduler` pulls in lots of helpers — stub the ones it uses. + # monkeypatch.setitem, not a bare assignment: a plain write leaves these + # empty stubs in sys.modules for the rest of the session, and every later + # test that imports a real name from one of them fails with + # "cannot import name ... (unknown location)". The stubs above in this file + # already use monkeypatch for the same reason. for s in ["src.builtin_actions", "src.ai_interaction", "src.endpoint_resolver", "src.agent_loop", "src.session_manager"]: if s not in sys.modules: - mod = types.ModuleType(s) - sys.modules[s] = mod + monkeypatch.setitem(sys.modules, s, types.ModuleType(s)) from src.task_scheduler import TaskScheduler sch = TaskScheduler.__new__(TaskScheduler) # bypass __init__ network etc. sch._pending_notifications = []