test(db): harden the DB lane per code-review (R6 S3)
/code-review high on S3 — 3 low findings, 2 fixed, 1 no-change: - _db_reachable() now probes via a throwaway engine with connect_timeout=3, so a slow/firewalled DATABASE_URL can't hang the whole suite at import (the app engine has no connect timeout). - test_plex_facets sets the "configured" gate flag via monkeypatch (auto-restored) instead of poking state._configured_cache in a manual finally — no leak risk, and the now-unnecessary mark_configured DB write is dropped. No change: test_plex_jsonb_normalize copying migration 0060's SQL is deliberate — migrations must stay self-contained (no app imports) and a shipped migration is immutable, so there's no real drift to guard against. Gate: ruff clean; DB lane 176 passed; pure-logic 172 passed / 4 skipped (fast).
This commit is contained in:
@@ -15,19 +15,26 @@ tests that COMMIT (the whole point of this lane — transaction-ownership behavi
|
|||||||
import pytest
|
import pytest
|
||||||
from alembic import command
|
from alembic import command
|
||||||
from alembic.config import Config
|
from alembic.config import Config
|
||||||
from sqlalchemy import text
|
from sqlalchemy import create_engine, text
|
||||||
|
|
||||||
import app.models # noqa: F401 — register every model on Base.metadata
|
import app.models # noqa: F401 — register every model on Base.metadata
|
||||||
|
from app.config import settings
|
||||||
from app.db import Base, SessionLocal, engine
|
from app.db import Base, SessionLocal, engine
|
||||||
|
|
||||||
|
|
||||||
def _db_reachable() -> bool:
|
def _db_reachable() -> bool:
|
||||||
|
# A throwaway engine with a bounded connect_timeout so a slow/firewalled DATABASE_URL can't
|
||||||
|
# hang the whole suite at import (the app engine has no connect timeout). The compose lane
|
||||||
|
# gates on `service_healthy`, but this keeps a manual run honest too.
|
||||||
|
probe = create_engine(settings.database_url, connect_args={"connect_timeout": 3})
|
||||||
try:
|
try:
|
||||||
with engine.connect() as conn:
|
with probe.connect() as conn:
|
||||||
conn.execute(text("SELECT 1"))
|
conn.execute(text("SELECT 1"))
|
||||||
return True
|
return True
|
||||||
except Exception:
|
except Exception:
|
||||||
return False
|
return False
|
||||||
|
finally:
|
||||||
|
probe.dispose()
|
||||||
|
|
||||||
|
|
||||||
_DB_REACHABLE = _db_reachable()
|
_DB_REACHABLE = _db_reachable()
|
||||||
|
|||||||
@@ -15,16 +15,17 @@ from app.main import app
|
|||||||
from app.models import PlexLibrary, PlexShow, User
|
from app.models import PlexLibrary, PlexShow, User
|
||||||
|
|
||||||
|
|
||||||
def test_facets_survives_json_null_genres(db):
|
def test_facets_survives_json_null_genres(db, monkeypatch):
|
||||||
lib = PlexLibrary(plex_key="1", title="Shows", kind="show", enabled=True)
|
lib = PlexLibrary(plex_key="1", title="Shows", kind="show", enabled=True)
|
||||||
db.add(lib)
|
db.add(lib)
|
||||||
db.flush()
|
db.flush()
|
||||||
db.add(PlexShow(rating_key="s1", library_id=lib.id, title="No Genre Show"))
|
db.add(PlexShow(rating_key="s1", library_id=lib.id, title="No Genre Show"))
|
||||||
user = User(email="facets@example.com")
|
user = User(email="facets@example.com")
|
||||||
db.add(user)
|
db.add(user)
|
||||||
# The request gate 503s an unconfigured instance; mark this throwaway DB configured.
|
|
||||||
state.mark_configured(db)
|
|
||||||
db.commit()
|
db.commit()
|
||||||
|
# The request gate 503s an unconfigured instance; flip the process-level "configured" flag the
|
||||||
|
# gate reads. monkeypatch auto-restores it, so it can't leak "configured" into another test.
|
||||||
|
monkeypatch.setattr(state, "_configured_cache", True)
|
||||||
# Simulate a pre-0.53.1 row: genres persisted as a JSONB scalar `null` (what SQLAlchemy wrote
|
# Simulate a pre-0.53.1 row: genres persisted as a JSONB scalar `null` (what SQLAlchemy wrote
|
||||||
# before none_as_null=True), NOT SQL NULL — this is the value jsonb_array_elements_text() chokes
|
# before none_as_null=True), NOT SQL NULL — this is the value jsonb_array_elements_text() chokes
|
||||||
# on. A Python None now maps to SQL NULL, so we set the scalar directly.
|
# on. A Python None now maps to SQL NULL, so we set the scalar directly.
|
||||||
@@ -38,7 +39,6 @@ def test_facets_survives_json_null_genres(db):
|
|||||||
resp = client.get("/api/plex/facets?scope=show")
|
resp = client.get("/api/plex/facets?scope=show")
|
||||||
finally:
|
finally:
|
||||||
app.dependency_overrides.clear()
|
app.dependency_overrides.clear()
|
||||||
state._configured_cache = False # one-way process cache — don't leak "configured" to other tests
|
|
||||||
|
|
||||||
assert resp.status_code == 200, resp.text
|
assert resp.status_code == 200, resp.text
|
||||||
# The scalar-null show contributes no genres — and, crucially, doesn't abort the whole endpoint.
|
# The scalar-null show contributes no genres — and, crucially, doesn't abort the whole endpoint.
|
||||||
|
|||||||
Reference in New Issue
Block a user