fix(tests): isolate test DB from dev DB and plug admin-db override gap
All checks were successful
Mirror to GitHub / mirror (push) Successful in 3s
All checks were successful
Mirror to GitHub / mirror (push) Successful in 3s
Root cause of the 06:32 AM outage: running 'pytest tests/' inside the
resolutionflow_backend container silently dropped the public schema on
the DEV database. Two layered bugs made this possible; both are fixed.
Bug 1 — env-var lookup in conftest.TEST_DATABASE_URL put DATABASE_URL
(which normally points at the dev/prod DB) ahead of DATABASE_TEST_URL.
When DATABASE_URL is set, pytest used the dev DB as the 'test' DB and
the test_db fixture's DROP SCHEMA public CASCADE wiped it. Fixed:
- Honor only DATABASE_TEST_URL (or the localhost fallback).
- Assert at module load that the DB name contains 'test' — refuses
to run otherwise. Makes future misconfiguration impossible.
Bug 2 — conftest overrode app.dependency_overrides[get_db] but not
get_admin_db. Endpoints using get_admin_db (register, admin routes)
bypassed the test session and hit the real admin DB. Before Bug 1 was
fixed this was hidden because both engines pointed at the same dev DB.
With isolation in place, register started failing 'Email already
registered' because of stale users in the dev DB. Fixed:
- Also override get_admin_db to yield the same test session. RLS is
not enabled in the create_all-managed test schema, so sharing is
safe.
Also adds DATABASE_TEST_URL=resolutionflow_test to docker-compose.dev.yml
so pytest in the container works out of the box.
Verified: 49/50 Phase 8 + 9 tests pass against resolutionflow_test; the
1 failure is the pre-existing Phase 8 Issue #4
(test_record_decision_persists_and_bumps_state_version).
Refs gitea #145 (will update that issue with this as the primary fix).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -14,21 +14,33 @@ from sqlalchemy.pool import NullPool
|
||||
|
||||
from app.main import app
|
||||
from app.core.database import Base, get_db
|
||||
from app.core.admin_database import get_admin_db
|
||||
from app.core.config import settings
|
||||
|
||||
# Disable invite code requirement for tests
|
||||
settings.REQUIRE_INVITE_CODE = False
|
||||
|
||||
# Test database URL (separate from production)
|
||||
# Use DATABASE_TEST_URL env var if set (e.g. inside Docker where host is 'db'),
|
||||
# otherwise fall back to localhost for local development.
|
||||
# Test database URL — NEVER reuse DATABASE_URL. The test_db fixture does
|
||||
# `DROP SCHEMA public CASCADE` on every test; if DATABASE_URL (which normally
|
||||
# points at the dev/prod DB) leaked into this value, running `pytest tests/`
|
||||
# would silently nuke the dev database. Only DATABASE_TEST_URL is honored,
|
||||
# and the safety assertion below refuses to run against a DB whose name
|
||||
# doesn't contain "test".
|
||||
import os
|
||||
TEST_DATABASE_URL = os.environ.get(
|
||||
"DATABASE_URL",
|
||||
os.environ.get(
|
||||
"DATABASE_TEST_URL",
|
||||
"postgresql+asyncpg://postgres:postgres@localhost:5432/patherly_test",
|
||||
),
|
||||
"DATABASE_TEST_URL",
|
||||
"postgresql+asyncpg://postgres:postgres@localhost:5432/resolutionflow_test",
|
||||
)
|
||||
|
||||
# Belt-and-suspenders: refuse to run tests against a DB whose name doesn't
|
||||
# contain "test". Parses the last path segment of the URL (everything after
|
||||
# the final '/', with query string stripped) so credentials / hosts that
|
||||
# happen to contain "test" can't bypass the check.
|
||||
_test_db_name = TEST_DATABASE_URL.rsplit("/", 1)[-1].split("?", 1)[0].lower()
|
||||
assert "test" in _test_db_name, (
|
||||
f"Refusing to run tests against database {_test_db_name!r} — "
|
||||
f"the DB name must contain 'test'. Set DATABASE_TEST_URL to a dedicated "
|
||||
f"test database (e.g. resolutionflow_test)."
|
||||
)
|
||||
|
||||
|
||||
@@ -131,6 +143,11 @@ async def client(test_db: AsyncSession):
|
||||
yield test_db
|
||||
|
||||
app.dependency_overrides[get_db] = override_get_db
|
||||
# Endpoints that use get_admin_db (register, admin routes, service accounts)
|
||||
# must also hit the test DB; otherwise they leak into the real admin DB.
|
||||
# RLS is not enabled in the test schema (create_all, not alembic), so sharing
|
||||
# the same session is safe.
|
||||
app.dependency_overrides[get_admin_db] = override_get_db
|
||||
|
||||
transport = ASGITransport(app=app)
|
||||
async with AsyncClient(transport=transport, base_url="http://test") as ac:
|
||||
|
||||
@@ -33,6 +33,9 @@ services:
|
||||
- DEBUG=true
|
||||
- DATABASE_URL=postgresql+asyncpg://postgres:postgres@db:5432/resolutionflow
|
||||
- DATABASE_URL_SYNC=postgresql://postgres:postgres@db:5432/resolutionflow
|
||||
# Dedicated test database — pytest will refuse to run against any DB
|
||||
# whose name doesn't contain 'test' (conftest.py safety assertion).
|
||||
- DATABASE_TEST_URL=postgresql+asyncpg://postgres:postgres@db:5432/resolutionflow_test
|
||||
- SECRET_KEY=${SECRET_KEY}
|
||||
- ALGORITHM=HS256
|
||||
- ACCESS_TOKEN_EXPIRE_MINUTES=15
|
||||
|
||||
Reference in New Issue
Block a user