From fe81c491cd45fc21ffcb38cb6842bcffcbebc1f2 Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira Date: Thu, 29 Oct 2026 17:30:10 +0000 Subject: [PATCH] test: give collection database URL one cleanup owner --- tests/conftest.py | 15 ++++++++++++++- tests/helpers/worker_runtime.py | 5 ----- tests/test_database_test_isolation.py | 12 ++++++++++-- tests/test_worker_runtime.py | 1 - 4 files changed, 24 insertions(+), 9 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index d6d60233c..ff5a354ea 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,9 +12,17 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) # File-backed databases remain fixture-owned. Cleanup restores the caller. # Keep the existing generated environment-reference locations stable. _database_environment = pytest.MonkeyPatch() -_database_environment.setenv("DATABASE_URL", "sqlite:///:memory:") from tests.helpers.worker_runtime import bootstrap_runtime, configure_runtime _runtime_environment = bootstrap_runtime() +# A file keeps the collection schema consistent across TestClient threads; +# SingletonThreadPool can evict the populated in-memory main-thread database. +# One patcher owns this variable, including restoration of an inherited URL. +_database_environment.setenv( + "DATABASE_URL", + f"sqlite:///{_runtime_environment.root / 'collection.db'}" + if _runtime_environment is not None else "sqlite:///:memory:", +) +_collection_database_engine = None # Pre-import real heavy modules BEFORE any test file's module-level stubs can # replace them with MagicMock. Some test files (e.g. test_llm_core_sanitize_*) @@ -26,6 +34,7 @@ try: import sqlalchemy.orm # noqa: F401 import core.database # noqa: F401 import src.database + _collection_database_engine = core.database.engine except ImportError: pass # not installed - the stubs below will handle it @@ -100,6 +109,10 @@ def pytest_configure(config): only registers marker names; it imports no production module. """ config.add_cleanup(_database_environment.undo) + if _collection_database_engine is not None: + # Registered after runtime cleanup, so connections close before the + # owned file is removed (also required on Windows). + config.add_cleanup(_collection_database_engine.dispose) import pathlib from tests._taxonomy import discover_markers diff --git a/tests/helpers/worker_runtime.py b/tests/helpers/worker_runtime.py index fd912180c..0e8b36138 100644 --- a/tests/helpers/worker_runtime.py +++ b/tests/helpers/worker_runtime.py @@ -60,11 +60,6 @@ def isolated_runtime(worker="main"): path.mkdir(mode=0o700) patcher.setenv(name, str(path)) (root / "data" / "agent_workspace").mkdir(mode=0o700, exist_ok=True) - # SQLite's default in-memory pool gives each thread an empty DB - # and can evict the main connection as TestClient threads churn. - # A worker-owned file preserves the collection schema across real - # connections. Tests that write rows still own explicit fixtures. - patcher.setenv("DATABASE_URL", f"sqlite:///{root / 'collection.db'}") # Browser resolution falls back to our XDG runtime directory. patcher.delenv("AGENT_BROWSER_SOCKET_DIR", raising=False) tmp = root / "tmp" diff --git a/tests/test_database_test_isolation.py b/tests/test_database_test_isolation.py index f0e72e27a..7540f72b0 100644 --- a/tests/test_database_test_isolation.py +++ b/tests/test_database_test_isolation.py @@ -85,6 +85,8 @@ def test_actual_tests_restore_process_state_and_ignore_inherited_database(tmp_pa import os import sys import pytest + from pathlib import Path + from sqlalchemy.engine import make_url def snapshot(): import core @@ -107,7 +109,11 @@ def test_actual_tests_restore_process_state_and_ignore_inherited_database(tmp_pa class StateGuard: def pytest_sessionstart(self): self.initial = snapshot() - assert self.initial[0] == "sqlite:///:memory:" + assert self.initial[0] != inherited_url + self.default_path = Path(make_url(self.initial[0]).database) + assert self.default_path.is_file() + assert self.default_path.name == "collection.db" + assert self.default_path.parent == Path(os.environ["ODYSSEUS_DATA_DIR"]).parent def pytest_collection_finish(self): assert snapshot() == self.initial, "collection changed database bindings" @@ -118,9 +124,11 @@ def test_actual_tests_restore_process_state_and_ignore_inherited_database(tmp_pa assert snapshot() == self.initial, "test leaked database or module state" inherited_url = os.environ["DATABASE_URL"] + guard = StateGuard() result = pytest.main(["-q", "-p", "no:cacheprovider", *sys.argv[1:]], - plugins=[StateGuard()]) + plugins=[guard]) assert os.environ["DATABASE_URL"] == inherited_url + assert not guard.default_path.exists(), "collection database outlived its runtime" raise SystemExit(result) ''') result = subprocess.run( diff --git a/tests/test_worker_runtime.py b/tests/test_worker_runtime.py index 99c88cfdf..b99d58817 100644 --- a/tests/test_worker_runtime.py +++ b/tests/test_worker_runtime.py @@ -34,7 +34,6 @@ def test_runtime_restores_environment_and_removes_files(monkeypatch, tmp_path, f path = Path(os.environ[name]) assert path.is_dir() and path.is_relative_to(root) (path / "owned").write_text("test") - assert os.environ["DATABASE_URL"] == f"sqlite:///{root / 'collection.db'}" assert Path(tempfile.gettempdir()).is_relative_to(root) if fail: raise RuntimeError("test failure")