diff --git a/tests/TESTING_STANDARD.md b/tests/TESTING_STANDARD.md index fd8c944e4..cb0eb71de 100644 --- a/tests/TESTING_STANDARD.md +++ b/tests/TESTING_STANDARD.md @@ -99,8 +99,10 @@ cleanup. Specifically: `os.environ[...] = ...` that outlives the test. - **Current working directory** - never `chdir` without restoring; never assert against cwd-relative paths like `./data`. Use a temp workspace helper instead. -- **Database** - the root `conftest.py` defaults `DATABASE_URL` to an in-memory - SQLite for collection safety. A test that needs a real file-backed DB must opt +- **Database** - the root `conftest.py` defaults `DATABASE_URL` to a private, + worker-owned SQLite file for collection safety and consistent schema across + TestClient threads. This fallback is not a per-test data fixture. A test that + writes or depends on particular rows must opt in explicitly via `tests.helpers.sqlite_db.make_temp_sqlite` and bind its `SessionLocal` onto the module under test. Do not rely on a persistent on-disk DB existing. 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/test_database_test_isolation.py b/tests/test_database_test_isolation.py index f0e72e27a..308ff18ec 100644 --- a/tests/test_database_test_isolation.py +++ b/tests/test_database_test_isolation.py @@ -85,6 +85,9 @@ def test_actual_tests_restore_process_state_and_ignore_inherited_database(tmp_pa import os import sys import pytest + import tempfile + from pathlib import Path + from sqlalchemy.engine import make_url def snapshot(): import core @@ -107,7 +110,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(tempfile.gettempdir()).parent def pytest_collection_finish(self): assert snapshot() == self.initial, "collection changed database bindings" @@ -118,9 +125,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 a91157902..b99d58817 100644 --- a/tests/test_worker_runtime.py +++ b/tests/test_worker_runtime.py @@ -79,3 +79,32 @@ def test_tmp_path_under_private_runtime_fits_unix_socket(tmp_path): # witness. A nested pytest-of- basetemp made it 110 bytes under xdist. with socket.socket(socket.AF_UNIX) as sock: sock.bind(str(tmp_path / "tmux.sock")) + + +def test_collection_database_survives_thread_connection_churn(): + """TestClient threads must not lose the collection database or its schema. + + SingletonThreadPool's default five-connection limit used to close the main + thread's in-memory database after repeated route clients. Each new thread + also saw an empty database. Exercise more threads than that limit and then + verify both compatibility factories still see all registered tables. + """ + from concurrent.futures import ThreadPoolExecutor + from sqlalchemy import inspect + import core.database as database + import src.database as compatibility + + expected = set(database.Base.metadata.tables) + + def table_names(factory): + with factory() as session: + return set(inspect(session.connection()).get_table_names()) + + assert {"documents", "model_endpoints", "mcp_servers", "calendars"} <= expected + assert expected <= table_names(database.SessionLocal) + for _ in range(8): + # A fresh executor creates a fresh thread rather than reusing one. + with ThreadPoolExecutor(max_workers=1) as executor: + assert expected <= executor.submit(table_names, database.SessionLocal).result() + assert expected <= table_names(database.SessionLocal) + assert expected <= table_names(compatibility.SessionLocal)