fix(tests): keep local backend test runs off the real library database

[VanshajPoonia]

Closes #1483

cd backend && pytest, the command in backend/AGENTS.md, pointed DATABASE_PATH at the user's real PictoPy.db and created every application table inside it. Several tests DROP TABLE and DELETE FROM, so on an indexed library a local test run could destroy real data. CI was never affected.

Why it happened

Two independent faults, and fixing either one alone leaves the bug live.

1. settings.py gated the test path on the wrong variable.

if os.getenv("GITHUB_ACTIONS") == "true":
    DATABASE_PATH = os.path.join(os.getcwd(), "test_db.sqlite3")
else:
    DATABASE_PATH = os.path.join(user_data_dir("PictoPy"), "database", "PictoPy.db")

GitHub sets GITHUB_ACTIONS=true automatically, so CI got isolation for free and nobody noticed. Locally nothing sets it.

2. conftest.py set TEST_MODE too late to matter.

It set it inside the setup_before_all_tests fixture body. But conftest.py's own module-level from app.database.faces import ... imports run during collection, long before any fixture executes, and settings.py computes DATABASE_PATH at import time. By the time the fixture ran, every module had already frozen the wrong value.

The fix

settings.py now honours TEST_MODE alongside GITHUB_ACTIONS, and a new backend/tests/db_isolation.py sets it before any app import. conftest.py imports that module as its first statement, so ordering is enforced by the import graph rather than by a comment asking people not to move a line.

Same module refuses to start a run whose resolved DATABASE_PATH lands inside user_data_dir("PictoPy"). If the redirect ever silently breaks again, the session dies with a message instead of quietly writing to the library.

The fixture's teardown no longer deletes TEST_MODE. Unsetting it mid-process would let any later import of settings.py resolve straight back to the real library.

Evidence

Recorded the library database's mtime and size, ran the full suite, compared.

branch	before	after	result
main	1787161142 98713600	1787162533 98713600	mtime moved
this branch	1787162533 98713600	1787162533 98713600	untouched

Tables land in backend/test_db.sqlite3 instead, which backend/.gitignore already covers. I hit the main row by accident while verifying an unrelated branch, which is a fair illustration of how easy this is to trip.

Tests

backend/tests/test_db_isolation.py, four cases:

• the redirect lands and TEST_MODE is set
• DATABASE_PATH resolves outside user_data_dir("PictoPy")
• app/database/connection.py sees the redirected value, since it binds DATABASE_PATH at import time and a redirect landing after it would leave every query pointed at the library
• the guard raises when handed a library path

Backend suite goes from 1110 to 1114.

Credit

@tanmaysachann's analysis on the issue reached the same two-part root cause independently, and added a point worth recording: because each module does from app.config.settings import DATABASE_PATH, a name copy rather than a live reference, patching app.config.settings.DATABASE_PATH alone is not enough. The existing "safe" tests each have to patch several targets by hand. Quantified across the suite, 17 test files carry that pattern, between 1 and 5 targets each:

test_face_clusters.py  5    test_folders.py       5    test_memories_db.py   5
test_video_frames.py   5    test_albums_db.py     4    test_images_db.py     4
test_images_rescan.py  4    test_share_routes.py  4    test_faces_db.py      3
...

Fixing the value at the source does not remove that per-file patching, since those tests still want their own isolated tempfile per test. What it does remove is the consequence of forgetting: a file that misses a target now falls back to a throwaway database rather than the user's library.

@Ankushh0027 also offered to take this one. I was assigned the issue so I carried it through, but the analysis above is largely shared ground.

Docs

Three files stated the old behaviour. agent-kit/skills/add-backend-endpoint/SKILL.md read "so tests get a real database", which was true in a way nobody intended.

Scope

THUMBNAIL_IMAGES_PATH and VIDEO_FRAMES_PATH still resolve to user_data_dir unconditionally, and this PR does not change that.

I looked at whether they are a second live exposure. They are not, but the reason is worth stating precisely rather than waving at. Tests do reach filesystem writes under those paths, including video_util_purge_frame_cache, which calls shutil.rmtree(VIDEO_FRAMES_PATH). Every call site redirects the path to a tempdir first: both purge tests request the frames_dir fixture (tests/test_video_frames.py:71-78), and the thumbnail tests patch app.utils.videos.THUMBNAIL_IMAGES_PATH per test. Confirmed empirically too, the real thumbnails directory's mtime predates any of this work.

So the filesystem side is safe by per-test fixture discipline, not by construction. If someone later adds a test that calls video_util_purge_frame_cache without requesting frames_dir, it recursively deletes the user's real frame cache and nothing stops it. That is the same shape of latent fault this PR fixes for the database, and I have deliberately left it out rather than widen the diff. Happy to file it separately if a maintainer wants it tracked.

One note for anyone who ran pytest locally before this

The fix stops future runs from writing to the library database. It does not undo writes that already happened. If you ran cd backend && pytest on an indexed library, that database may contain tables the fixture created, and rows that tests inserted or deleted. Worth checking before assuming a clean slate.

Verification

1114 passed in 13.91s
black    117 files unchanged
ruff     All checks passed!
markdownlint clean on the three changed docs

Summary by CodeRabbit

• Bug Fixes

  • Tests now consistently use a disposable local database instead of the user’s real PictoPy library.
  • Improved protection against accidental access to production data during testing.
  • Test database selection now works in both automated builds and explicit test mode.

• Tests

  • Added coverage verifying database isolation, path selection, and protection against unsafe database locations.
  • Updated test setup documentation and instructions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c79ba28f-98a4-41c5-a60b-677f724653e1

📥 Commits

Reviewing files that changed from the base of the PR and between 510d4d0 and 52e26ac.

📒 Files selected for processing (7)

• agent-kit/references/backend-endpoint-walkthrough.md
• agent-kit/skills/add-backend-endpoint/SKILL.md
• backend/AGENTS.md
• backend/app/config/settings.py
• backend/tests/conftest.py
• backend/tests/db_isolation.py
• backend/tests/test_db_isolation.py

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

Walkthrough

The backend test setup now selects a disposable SQLite database when TEST_MODE=true, initializes that mode before application imports, rejects unsafe database paths, and documents the revised fixture behavior.

Changes

Test database isolation

Layer / File(s)	Summary
Database path selection and safety guard backend/app/config/settings.py, backend/tests/db_isolation.py, backend/tests/test_db_isolation.py	TEST_MODE=true selects backend/test_db.sqlite3. Startup validation rejects database paths inside the user PictoPy library. Tests verify path redirection, import-time consistency, and unsafe-path rejection.
Import-time test bootstrap backend/tests/conftest.py, backend/AGENTS.md, agent-kit/references/backend-endpoint-walkthrough.md, agent-kit/skills/add-backend-endpoint/SKILL.md	tests.db_isolation loads before application imports. The fixture no longer manages TEST_MODE. Documentation describes table creation separately from database isolation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 52e26

The change redirects local test runs away from the real library database and adds guard coverage; no actionable merge-blocking risk remains after normal checks and review.

Suggested labels: Python, Documentation

Suggested reviewers: rohan-pandeyy

Poem

I’m a rabbit guarding tables with care,
No real PictoPy files shall meet testware.
TEST_MODE hops in before imports begin,
A safe little database keeps tests in.
Thump, thump—the live library stays clean!

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue #1483 by honoring TEST_MODE, setting it before app imports, and guarding against production database paths.
Out of Scope Changes check	✅ Passed	All code, tests, and documentation changes directly support local backend database isolation described in issue #1483.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing local backend tests from using the real library database.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gitcordapp]

Link your account with Gitcord

Thanks for opening this PR, @VanshajPoonia!

To receive Discord notifications and contributor tracking for this organization:

1. Join Discord: https://discord.gg/hjUhu33uAn
2. In Discord, run /link VanshajPoonia
3. Paste the verification code into your GitHub bio (or a public gist)
4. Click Verify in Discord (or run /verify-link VanshajPoonia)

Once linked, Gitcord can notify you about reviews, merges, and more.

— Posted by Gitcord