diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 9bec2f5e..d1cc5483 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -21,7 +21,8 @@ from alembic import command from alembic.config import Config from fastapi import HTTPException, Request from fastapi.testclient import TestClient -from sqlalchemy import create_engine, text +from sqlalchemy import create_engine, event, text +from starlette.requests import HTTPConnection import app.models # noqa: F401 — register every model on Base.metadata import app.state as app_state @@ -100,17 +101,57 @@ def pytest_configure(config): ) +def _request_dep(db, unfinished: list[str]): + """`get_db` for a test request: the test's own session, plus the check a shared session hides. + + A route helper flushes and its HANDLER commits (CLAUDE.md). Production's `get_db` closes the + session after the request, which rolls back anything flushed but not committed — yet here every + request shares the test's session, so a follow-up GET or an assertion reads the flushed write + and the suite stays green over a handler that forgot to commit (deep review, debt burn). So a + request that ends normally with a flushed, uncommitted write is recorded, and the test fails on + it at teardown. The test's own staged rows are flushed before the request starts — they are the + test's business, not the handler's. A request that raises is exempt: its writes are meant to + roll back.""" + + def dep(conn: HTTPConnection): + db.flush() + state = {"dirty": False} + + def flushed(session, ctx): + state["dirty"] = True + + def ended(session): + state["dirty"] = False + + hooks = (("after_flush", flushed), ("after_commit", ended), ("after_rollback", ended)) + for name, fn in hooks: + event.listen(db, name, fn) + try: + yield db + if state["dirty"]: + unfinished.append(f"{conn.scope.get('method', 'WS')} {conn.url.path}") + finally: + for name, fn in hooks: + event.remove(db, name, fn) + + return dep + + @pytest.fixture def db(_schema): """A real session against the test Postgres, on a clean (empty) database. The code under test - may commit freely.""" + may commit freely. `db.info["request"]` is the `get_db` override for a request against it — the + one every test client uses — and a handler that left a write uncommitted fails the test here.""" _truncate_all() session = SessionLocal() + unfinished: list[str] = [] + session.info["request"] = _request_dep(session, unfinished) try: yield session finally: session.rollback() session.close() + assert unfinished == [], "a handler flushed a write and never committed it" @pytest.fixture @@ -168,7 +209,7 @@ def client(db, override_dep, _configured): `current_user` override — so login, the wallet gate and epoch revoke are exercised end to end. Starlette's SessionMiddleware round-trips the signed cookie, so a login in one call authenticates the next on this same instance. No lifespan (the scheduler / HLS sweep stay off).""" - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) return TestClient(app) @@ -187,7 +228,7 @@ def as_user(db, override_dep, _configured): raise HTTPException(status_code=401, detail="Not authenticated") return registry[uid] - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, _resolver) def _as(user: User) -> TestClient: diff --git a/backend/tests/test_channel_backfill.py b/backend/tests/test_channel_backfill.py index 5520a3a1..10fc90c1 100644 --- a/backend/tests/test_channel_backfill.py +++ b/backend/tests/test_channel_backfill.py @@ -40,7 +40,7 @@ def test_channel_detail_returns_normal_shorts_live_counts(db, monkeypatch, overr # The request gate 503s an unconfigured instance; flip the process-level flag (auto-restored). monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: me) resp = TestClient(app).get(f"/api/channels/{ch.id}") @@ -65,7 +65,7 @@ def test_channel_detail_surfaces_subscription_hidden_flag(db, monkeypatch, hidde db.commit() monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: me) resp = TestClient(app).get(f"/api/channels/{ch.id}") @@ -81,7 +81,7 @@ def test_channel_detail_hidden_defaults_false_when_not_subscribed(db, monkeypatc db.commit() monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: me) resp = TestClient(app).get(f"/api/channels/{ch.id}") @@ -111,7 +111,7 @@ def _patch_deep_requested(db, monkeypatch, override_dep, user): # background.add_task runs after the response in TestClient; record what would be scheduled. monkeypatch.setattr(channels_route, "backfill_channel_now", lambda *a, **k: calls.append(a)) monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) resp = TestClient(app).patch("/api/channels/UC_wire", json={"deep_requested": True}) return resp, calls diff --git a/backend/tests/test_channel_tags.py b/backend/tests/test_channel_tags.py index 49bad533..1bd3a531 100644 --- a/backend/tests/test_channel_tags.py +++ b/backend/tests/test_channel_tags.py @@ -43,7 +43,7 @@ def test_channel_detail_returns_only_this_users_tag_ids(db, monkeypatch, overrid # The request gate 503s an unconfigured instance; flip the process-level flag (auto-restored). monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: me) client = TestClient(app) # no context-manager: skip lifespan (scheduler/HLS sweep) resp = client.get(f"/api/channels/{ch.id}") diff --git a/backend/tests/test_feed_cursor.py b/backend/tests/test_feed_cursor.py index 0c1fcd5a..39bafcdd 100644 --- a/backend/tests/test_feed_cursor.py +++ b/backend/tests/test_feed_cursor.py @@ -19,7 +19,7 @@ def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr( state, "_configured_cache", True ) # the request gate 503s an unconfigured app - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) client = TestClient(app) # no context manager: skip lifespan (scheduler/HLS sweep) return client.get(path) diff --git a/backend/tests/test_hls_reaper.py b/backend/tests/test_hls_reaper.py index 81a396f5..50babeae 100644 --- a/backend/tests/test_hls_reaper.py +++ b/backend/tests/test_hls_reaper.py @@ -143,7 +143,7 @@ def _library_item(db, rating_key: str, *, playable: str = "remux"): def _client(db, user, override_dep): - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app) diff --git a/backend/tests/test_override_dep_convention.py b/backend/tests/test_override_dep_convention.py index d631bfd0..222009a7 100644 --- a/backend/tests/test_override_dep_convention.py +++ b/backend/tests/test_override_dep_convention.py @@ -21,3 +21,17 @@ def test_no_test_writes_dependency_overrides_directly(): if "dependency_overrides" in line ] assert offenders == [], "use the override_dep fixture: " + ", ".join(offenders) + + +def test_a_request_gets_the_session_that_checks_its_commit(): + """`get_db` is overridden with `db.info["request"]` — the test session plus the check that the + handler committed what it flushed (conftest `_request_dep`). A bare `lambda: db` hands the route + the same session without the check, and a handler that forgot its commit passes again.""" + offenders = [ + f"{p.name}:{n}" + for p in sorted(TESTS.rglob("*.py")) + if p.name != Path(__file__).name + for n, line in enumerate(p.read_text(encoding="utf-8").splitlines(), 1) + if "override_dep(get_db," in line and 'override_dep(get_db, db.info["request"])' not in line + ] + assert offenders == [], 'override get_db with db.info["request"]: ' + ", ".join(offenders) diff --git a/backend/tests/test_plex_clear_resume_push.py b/backend/tests/test_plex_clear_resume_push.py index c594bb87..3d267696 100644 --- a/backend/tests/test_plex_clear_resume_push.py +++ b/backend/tests/test_plex_clear_resume_push.py @@ -52,7 +52,7 @@ def _clear(db, user, it, monkeypatch, override_dep) -> list[tuple]: "push_state_to_plex", lambda user_id, item_id, rating_key, action, *rest: pushes.append((rating_key, action)), ) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) resp = TestClient(app).post(f"/api/plex/item/{it.rating_key}/state", json={"status": "new"}) assert resp.status_code == 200, resp.text diff --git a/backend/tests/test_plex_collection_sort.py b/backend/tests/test_plex_collection_sort.py index 255d66a3..4c5948fa 100644 --- a/backend/tests/test_plex_collection_sort.py +++ b/backend/tests/test_plex_collection_sort.py @@ -233,7 +233,7 @@ def _seed(db): def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app).get(path) diff --git a/backend/tests/test_plex_collections_filter.py b/backend/tests/test_plex_collections_filter.py index dbfb04b9..c90c2319 100644 --- a/backend/tests/test_plex_collections_filter.py +++ b/backend/tests/test_plex_collections_filter.py @@ -60,7 +60,7 @@ def _seed(db): def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app).get(path) @@ -148,7 +148,7 @@ def test_create_collection_rejects_duplicate_name(db, monkeypatch, override_dep) user = User(email="dup@example.com") db.add(user) db.commit() - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) override_dep(admin_user, lambda: user) r = TestClient(app).post( @@ -273,7 +273,7 @@ def _admin_client(db, user, monkeypatch, override_dep, stub, resync=_resync_ok): monkeypatch.setattr(state, "_configured_cache", True) monkeypatch.setattr(coll_routes, "PlexClient", lambda db: stub) monkeypatch.setattr(coll_routes.plex_sync, "resync_collection", resync) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) override_dep(admin_user, lambda: user) return TestClient(app) diff --git a/backend/tests/test_plex_detail.py b/backend/tests/test_plex_detail.py index a0f47c4e..2e524a0d 100644 --- a/backend/tests/test_plex_detail.py +++ b/backend/tests/test_plex_detail.py @@ -193,7 +193,7 @@ def test_enrich_people_batch_persists_detail(db): def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr(state, "_configured_cache", True) # pass the request gate - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app).get(path) diff --git a/backend/tests/test_plex_discover.py b/backend/tests/test_plex_discover.py index 7ad4cc57..75477af8 100644 --- a/backend/tests/test_plex_discover.py +++ b/backend/tests/test_plex_discover.py @@ -122,7 +122,7 @@ def _seed(db): def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) client = TestClient(app) # no context manager: skip lifespan return client.get(path) diff --git a/backend/tests/test_plex_facets.py b/backend/tests/test_plex_facets.py index 3d4c87e4..3589db2e 100644 --- a/backend/tests/test_plex_facets.py +++ b/backend/tests/test_plex_facets.py @@ -33,7 +33,7 @@ def test_facets_returns_studio_vocabulary(db, monkeypatch, override_dep): db.add(user) db.commit() monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) resp = TestClient(app).get("/api/plex/facets?scope=movie") assert resp.status_code == 200, resp.text @@ -58,7 +58,7 @@ def test_facets_survives_json_null_genres(db, monkeypatch, override_dep): db.execute(text("UPDATE plex_shows SET genres = 'null'::jsonb WHERE rating_key = 's1'")) db.commit() - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) client = TestClient(app) # no context-manager: skip lifespan (scheduler/HLS sweep) resp = client.get("/api/plex/facets?scope=show") diff --git a/backend/tests/test_plex_field_scoped_search.py b/backend/tests/test_plex_field_scoped_search.py index 9c590838..2b375f17 100644 --- a/backend/tests/test_plex_field_scoped_search.py +++ b/backend/tests/test_plex_field_scoped_search.py @@ -102,7 +102,7 @@ def _seed(db): def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app).get(path) diff --git a/backend/tests/test_plex_keyword_filter.py b/backend/tests/test_plex_keyword_filter.py index 930d3407..4a0cbbdb 100644 --- a/backend/tests/test_plex_keyword_filter.py +++ b/backend/tests/test_plex_keyword_filter.py @@ -43,7 +43,7 @@ def _seed(db): def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app).get(path) diff --git a/backend/tests/test_plex_lang.py b/backend/tests/test_plex_lang.py index 9600cbfc..1113320a 100644 --- a/backend/tests/test_plex_lang.py +++ b/backend/tests/test_plex_lang.py @@ -60,7 +60,7 @@ def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr( state, "_configured_cache", True ) # the request gate 503s an unconfigured app - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) client = TestClient(app) # no context manager: skip lifespan return client.get(path) diff --git a/backend/tests/test_plex_last_watched_sort.py b/backend/tests/test_plex_last_watched_sort.py index 6839c9ee..9f34e808 100644 --- a/backend/tests/test_plex_last_watched_sort.py +++ b/backend/tests/test_plex_last_watched_sort.py @@ -113,7 +113,7 @@ def _seed(db): def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app).get(path) diff --git a/backend/tests/test_plex_person.py b/backend/tests/test_plex_person.py index 2d1b4bad..c0748e51 100644 --- a/backend/tests/test_plex_person.py +++ b/backend/tests/test_plex_person.py @@ -27,7 +27,7 @@ NAME = "Robert De Niro" def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr(state, "_configured_cache", True) # pass the request gate - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app).get(path) # no lifespan (skip scheduler/HLS sweep) diff --git a/backend/tests/test_plex_person_bio.py b/backend/tests/test_plex_person_bio.py index 532a9e19..ec18c777 100644 --- a/backend/tests/test_plex_person_bio.py +++ b/backend/tests/test_plex_person_bio.py @@ -170,7 +170,7 @@ def test_enrich_person_noop_without_tmdb_id(db, monkeypatch): def _get(db, user, monkeypatch, override_dep, path): monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app).get(path) diff --git a/backend/tests/test_plex_playback_class.py b/backend/tests/test_plex_playback_class.py index 07788930..b08b2ee1 100644 --- a/backend/tests/test_plex_playback_class.py +++ b/backend/tests/test_plex_playback_class.py @@ -349,7 +349,7 @@ def _seed_classes(db): def _client(db, user, monkeypatch, override_dep): monkeypatch.setattr(state, "_configured_cache", True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app) diff --git a/backend/tests/test_plex_playback_session.py b/backend/tests/test_plex_playback_session.py index ef22ad07..5dd921da 100644 --- a/backend/tests/test_plex_playback_session.py +++ b/backend/tests/test_plex_playback_session.py @@ -67,7 +67,7 @@ def _post(db, user, monkeypatch, override_dep, rating_key, *, captured): ) monkeypatch.setattr(plex_stream, "start_session", fake_start) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) return TestClient(app).post(f"/api/plex/stream/{rating_key}/session") diff --git a/backend/tests/test_plex_query_conversion.py b/backend/tests/test_plex_query_conversion.py index 602fe810..ba919ba3 100644 --- a/backend/tests/test_plex_query_conversion.py +++ b/backend/tests/test_plex_query_conversion.py @@ -100,7 +100,7 @@ def _request(db, user, monkeypatch, override_dep, method, path, json=None): monkeypatch.setattr( state, "_configured_cache", True ) # the request gate 503s an unconfigured app - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) client = TestClient(app) # no context manager: skip lifespan (scheduler/HLS sweep) return client.request(method, path, json=json) diff --git a/backend/tests/test_plex_subtitle_auth.py b/backend/tests/test_plex_subtitle_auth.py index a4f6b2fe..edbc5fb3 100644 --- a/backend/tests/test_plex_subtitle_auth.py +++ b/backend/tests/test_plex_subtitle_auth.py @@ -28,7 +28,7 @@ def test_a_suspended_account_cannot_drive_the_subtitle_extractor( db, make_user, override_dep, monkeypatch, tmp_path ): user = make_user(is_suspended=True) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) monkeypatch.setattr(app_state, "_configured_cache", True) # A validly SIGNED cookie for that account — which is all the old gate looked at. for mod in (auth, playback): diff --git a/backend/tests/test_saved_views_source.py b/backend/tests/test_saved_views_source.py index 7db013f9..215c2642 100644 --- a/backend/tests/test_saved_views_source.py +++ b/backend/tests/test_saved_views_source.py @@ -15,7 +15,7 @@ def _client(db, user, monkeypatch, override_dep): monkeypatch.setattr( state, "_configured_cache", True ) # the request gate 503s an unconfigured app - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(current_user, lambda: user) # require_human resolves through this return TestClient(app) diff --git a/backend/tests/test_search_scrape_ingest.py b/backend/tests/test_search_scrape_ingest.py index e4f30a43..7686f146 100644 --- a/backend/tests/test_search_scrape_ingest.py +++ b/backend/tests/test_search_scrape_ingest.py @@ -145,7 +145,7 @@ def test_search_youtube_route_returns_422_when_ingest_fails(db, monkeypatch, ove monkeypatch.setattr(search_mod, "_ingest_candidates", _boom) - override_dep(get_db, lambda: db) + override_dep(get_db, db.info["request"]) override_dep(require_human, lambda: user) resp = TestClient(app).get("/api/search/youtube?q=test+term")