From 52e26ace90fb05eded149a34f56bbcc7e5c87ea8 Mon Sep 17 00:00:00 2001 From: Vanshaj Poonia Date: Wed, 19 Aug 2026 23:28:08 +0530 Subject: [PATCH] fix: keep local backend tests off the real library database settings.py gated the test database path on GITHUB_ACTIONS alone. CI sets that for free, so `cd backend && pytest` locally resolved DATABASE_PATH to the user's PictoPy.db and the session fixture created tables inside it. Several tests drop and truncate, so a run on an indexed library could destroy real data. settings.py now honours TEST_MODE too, and a new tests/db_isolation.py sets it before any app import: settings.py resolves the path at import time, so the old assignment inside the session fixture ran too late to redirect anything. The same module refuses to start a run whose DATABASE_PATH lands inside user_data_dir("PictoPy"), turning a silent redirect failure into a loud one. Closes #1483 --- .../backend-endpoint-walkthrough.md | 6 ++-- .../skills/add-backend-endpoint/SKILL.md | 5 +-- backend/AGENTS.md | 7 +++- backend/app/config/settings.py | 4 ++- backend/tests/conftest.py | 13 ++++--- backend/tests/db_isolation.py | 24 +++++++++++++ backend/tests/test_db_isolation.py | 35 +++++++++++++++++++ 7 files changed, 81 insertions(+), 13 deletions(-) create mode 100644 backend/tests/db_isolation.py create mode 100644 backend/tests/test_db_isolation.py diff --git a/agent-kit/references/backend-endpoint-walkthrough.md b/agent-kit/references/backend-endpoint-walkthrough.md index 3ae5c2784..f01b3e42b 100644 --- a/agent-kit/references/backend-endpoint-walkthrough.md +++ b/agent-kit/references/backend-endpoint-walkthrough.md @@ -86,5 +86,7 @@ source of truth. ## Tests `backend/tests/test_videos.py` runs against a real SQLite database: the session fixture in -`conftest.py` creates every table and sets `TEST_MODE=true` before any test runs, then -tears down afterwards. There is no mocking layer to satisfy. +`conftest.py` creates every table before any test runs, then tears down afterwards. There +is no mocking layer to satisfy. The database is a throwaway `backend/test_db.sqlite3`: +`tests/db_isolation.py` sets `TEST_MODE=true` ahead of every app import so `DATABASE_PATH` +never resolves to the user's real library. diff --git a/agent-kit/skills/add-backend-endpoint/SKILL.md b/agent-kit/skills/add-backend-endpoint/SKILL.md index 861b32046..5a06ff6c9 100644 --- a/agent-kit/skills/add-backend-endpoint/SKILL.md +++ b/agent-kit/skills/add-backend-endpoint/SKILL.md @@ -70,8 +70,9 @@ Template: `agent-kit/templates/route.py.md`. ## 6. Tests — `backend/tests/test_.py` Follow `backend/tests/test_videos.py`. Cover the success path, the empty case, and at least -one failure path. The session fixture in `conftest.py` creates every table and sets -`TEST_MODE=true`, so tests get a real database. +one failure path. The session fixture in `conftest.py` creates every table, so tests get a +real SQLite database. It is a throwaway `backend/test_db.sqlite3`, not the user's library: +`tests/db_isolation.py` sets `TEST_MODE=true` before any app import to redirect it. ## 7. Verify diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 352b0a9e1..684d6feef 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -58,7 +58,12 @@ Miss one of these and it silently half-works: ## Tests - `tests/test_.py`. Config in `pytest.ini`: `pythonpath = .`, `testpaths = tests`. -- The session fixture in `conftest.py` creates every table and sets `TEST_MODE=true`. +- `tests/db_isolation.py` sets `TEST_MODE=true` before any app import, which points + `DATABASE_PATH` at a throwaway `backend/test_db.sqlite3`. Never move that import + below the others in `conftest.py`, and never unset `TEST_MODE` mid-run: `settings.py` + resolves the path at import time, so either one sends the suite at the user's real + library, which it then drops and truncates. +- The session fixture in `conftest.py` creates every table. - Run with `cd backend && pytest`. ## Environment diff --git a/backend/app/config/settings.py b/backend/app/config/settings.py index e639015c3..d429fc56b 100644 --- a/backend/app/config/settings.py +++ b/backend/app/config/settings.py @@ -38,7 +38,9 @@ TEST_INPUT_PATH = "tests/inputs" TEST_OUTPUT_PATH = "tests/outputs" -if os.getenv("GITHUB_ACTIONS") == "true": +# TEST_MODE has to be honoured here, not just GITHUB_ACTIONS: CI sets the latter +# for free, so a local pytest run would otherwise write to the real library. +if os.getenv("GITHUB_ACTIONS") == "true" or os.getenv("TEST_MODE") == "true": DATABASE_PATH = os.path.join(os.getcwd(), "test_db.sqlite3") else: DATABASE_PATH = os.path.join(user_data_dir("PictoPy"), "database", "PictoPy.db") diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index ccf14b4ae..6fee93037 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -1,5 +1,8 @@ +# Must come first: it sets TEST_MODE, which settings.py reads at import time to +# keep the suite off the user's real library database. +import tests.db_isolation # noqa: F401 + import pytest -import os # Import database table creation functions from app.database.faces import db_create_faces_table @@ -20,9 +23,6 @@ def setup_before_all_tests(): print("\n=== Running manual setup fixture ===") - # Set test environment - os.environ["TEST_MODE"] = "true" - # Create all database tables in the same order as main.py print("Creating database tables...") try: @@ -49,6 +49,5 @@ def setup_before_all_tests(): # Teardown code runs after all tests print("\n=== Running cleanup after all tests ===") - # Cleanup code here - if "TEST_MODE" in os.environ: - del os.environ["TEST_MODE"] + # TEST_MODE stays set: unsetting it would let any late import of settings.py + # resolve DATABASE_PATH back to the real library. diff --git a/backend/tests/db_isolation.py b/backend/tests/db_isolation.py new file mode 100644 index 000000000..e8acfe78c --- /dev/null +++ b/backend/tests/db_isolation.py @@ -0,0 +1,24 @@ +"""Points the database at a throwaway file before anything imports app.config.""" + +import os + +# settings.py resolves DATABASE_PATH at import time, so this has to happen before +# any app import. CI gets the same redirect for free via GITHUB_ACTIONS. +os.environ["TEST_MODE"] = "true" + +from app.config.settings import DATABASE_PATH # noqa: E402 +from platformdirs import user_data_dir # noqa: E402 + + +def _assert_not_user_library() -> None: + # The suite drops and truncates tables, so a silent redirect failure would + # destroy a real library. Fail the session instead. + library_dir = os.path.abspath(user_data_dir("PictoPy")) + resolved = os.path.abspath(DATABASE_PATH) + if os.path.commonpath([resolved, library_dir]) == library_dir: + raise RuntimeError( + f"Refusing to run tests against the PictoPy library database: {resolved}" + ) + + +_assert_not_user_library() diff --git a/backend/tests/test_db_isolation.py b/backend/tests/test_db_isolation.py new file mode 100644 index 000000000..6b9f78164 --- /dev/null +++ b/backend/tests/test_db_isolation.py @@ -0,0 +1,35 @@ +import os + +import pytest +from platformdirs import user_data_dir + +from app.config.settings import DATABASE_PATH +from app.database.connection import DATABASE_PATH as CONNECTION_DATABASE_PATH +from tests import db_isolation + + +def test_test_mode_redirects_database_path() -> None: + assert os.environ["TEST_MODE"] == "true" + assert os.path.basename(DATABASE_PATH) == "test_db.sqlite3" + + +def test_database_path_is_outside_the_user_library() -> None: + library_dir = os.path.abspath(user_data_dir("PictoPy")) + resolved = os.path.abspath(DATABASE_PATH) + assert os.path.commonpath([resolved, library_dir]) != library_dir + + +def test_connection_module_uses_the_redirected_path() -> None: + # The connection module binds DATABASE_PATH at import time, so a redirect that + # lands after it would leave every query pointed at the real library. + assert CONNECTION_DATABASE_PATH == DATABASE_PATH + + +def test_guard_rejects_a_path_inside_the_user_library( + monkeypatch: pytest.MonkeyPatch, +) -> None: + library_db = os.path.join(user_data_dir("PictoPy"), "database", "PictoPy.db") + monkeypatch.setattr(db_isolation, "DATABASE_PATH", library_db) + + with pytest.raises(RuntimeError, match="Refusing to run tests"): + db_isolation._assert_not_user_library()