Fix two more test bugs surfaced by the full integration run
CI / lint (push) Canceled after 0s
CI / typecheck (push) Canceled after 0s
CI / unit-tests (push) Canceled after 0s
CI / integration-tests (push) Canceled after 0s
CI / container-build (push) Canceled after 0s
CI / dependency-audit (push) Canceled after 0s
CI / lint (push) Canceled after 0s
CI / typecheck (push) Canceled after 0s
CI / unit-tests (push) Canceled after 0s
CI / integration-tests (push) Canceled after 0s
CI / container-build (push) Canceled after 0s
CI / dependency-audit (push) Canceled after 0s
1. test_ingestion_cache_invalidation.py: the job was created through
the test's db_session fixture, whose commit() only ends a savepoint
that gets rolled back at teardown (see tests/conftest.py) -- it's
never a real commit. _run_ingestion looks the job up through its
own session_scope() (a genuinely separate connection), which could
never see it, so the very first _require_job call raised "job
vanished mid-run" every time. Worse, test_failed_ingestion_does_not_
invalidate_the_cache still passed, coincidentally -- it expected a
RuntimeError and got one, just the wrong one, masking the bug.
Fixed by having _make_job create the job through session_scope()
itself, matching test_ingestion_retry.py's existing pattern.
2. Same class of bug as the video-collision fix in the last commit:
FAKE_MOVIE's fixed imdb_id ("tt0133093"), not just its video, also
collides across two different fake movies sharing it unmodified --
uq_external_id_source is a real unique constraint on
(provider_id, entity_type, external_id). Fixed by also overriding
imdb_id on the second movie in both affected tests.
All caught by running the actual integration suite against Postgres --
these paths were previously only collect-only'd locally (no local DB).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
29fffc8919
commit
0a50e0ffb6
@@ -57,9 +57,10 @@ async def test_ingest_collection_only_links_already_ingested_movies(db_session:
|
||||
already knows about, in release-date order, never fabricate a stub for
|
||||
the other two (section 15)."""
|
||||
|
||||
# 604's videos overridden to empty: FAKE_MOVIE's embedded video has a
|
||||
# fixed provider_video_id, and two different movies both ingesting it
|
||||
# would collide on the real uq_video_provider_source unique constraint.
|
||||
# 604's videos and imdb_id overridden: FAKE_MOVIE's embedded video and
|
||||
# imdb_id are both fixed identifiers, and two different movies both
|
||||
# ingesting them unmodified would collide on the real
|
||||
# uq_video_provider_source / uq_external_id_source unique constraints.
|
||||
provider = FakeTMDbCollectionProvider(
|
||||
movie_payloads={
|
||||
"603": {**FAKE_MOVIE, "id": 603, "title": "The Matrix"},
|
||||
@@ -68,6 +69,7 @@ async def test_ingest_collection_only_links_already_ingested_movies(db_session:
|
||||
"id": 604,
|
||||
"title": "The Matrix Reloaded",
|
||||
"videos": {"results": []},
|
||||
"imdb_id": "tt0234215",
|
||||
},
|
||||
},
|
||||
collection_payload=FAKE_COLLECTION,
|
||||
|
||||
@@ -14,13 +14,21 @@ import uuid
|
||||
import pytest
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.core.database import session_scope
|
||||
from app.core.ids import aurora_id
|
||||
from app.repositories.provider_repository import create_ingestion_job, get_or_create_provider
|
||||
from app.services.cache import build_key
|
||||
from app.services.ingestion import runner
|
||||
|
||||
|
||||
async def _make_job(session: AsyncSession, *, attempts_already_used: int = 0) -> uuid.UUID:
|
||||
async def _make_job(*, attempts_already_used: int = 0) -> uuid.UUID:
|
||||
# _run_ingestion looks the job up through its own session_scope() (a
|
||||
# separate connection from the test's db_session fixture, which only
|
||||
# fakes commits inside a savepoint that's rolled back at teardown --
|
||||
# see tests/conftest.py). Creating the job through session_scope() too
|
||||
# (matching test_ingestion_retry.py) makes it a real, visible commit
|
||||
# instead of one only db_session's own connection can see.
|
||||
async with session_scope() as session:
|
||||
provider = await get_or_create_provider(session, "tmdb", "TMDB")
|
||||
job = await create_ingestion_job(
|
||||
session,
|
||||
@@ -39,8 +47,10 @@ async def _make_job(session: AsyncSession, *, attempts_already_used: int = 0) ->
|
||||
async def test_successful_ingestion_invalidates_the_entity_cache_key(
|
||||
db_session: AsyncSession, monkeypatch
|
||||
):
|
||||
job_id = await _make_job(db_session)
|
||||
await db_session.commit()
|
||||
# db_session is unused directly (see _make_job's docstring-comment) but
|
||||
# kept as a fixture dependency so this test skips cleanly when Postgres
|
||||
# isn't reachable, matching test_ingestion_retry.py's pattern.
|
||||
job_id = await _make_job()
|
||||
|
||||
result_id = uuid.uuid4()
|
||||
deleted_keys: list[str] = []
|
||||
@@ -66,8 +76,7 @@ async def test_successful_ingestion_invalidates_the_entity_cache_key(
|
||||
async def test_ingestion_without_cache_entity_type_does_not_touch_the_cache(
|
||||
db_session: AsyncSession, monkeypatch
|
||||
):
|
||||
job_id = await _make_job(db_session)
|
||||
await db_session.commit()
|
||||
job_id = await _make_job()
|
||||
|
||||
async def fake_work(session, provider):
|
||||
return uuid.uuid4()
|
||||
@@ -92,8 +101,7 @@ async def test_failed_ingestion_does_not_invalidate_the_cache(
|
||||
# attempts_already_used=4 (default max_attempts is 5) so this failure is
|
||||
# terminal and raises the original error rather than arq.Retry -- either
|
||||
# way the cache must not be touched, but this keeps the assertion simple.
|
||||
job_id = await _make_job(db_session, attempts_already_used=4)
|
||||
await db_session.commit()
|
||||
job_id = await _make_job(attempts_already_used=4)
|
||||
|
||||
async def failing_work(session, provider):
|
||||
raise RuntimeError("simulated failure")
|
||||
|
||||
@@ -188,16 +188,17 @@ async def test_movie_recommendations_only_include_already_ingested_movies(
|
||||
external id (section 15: never fabricate a stub for an unseen entity),
|
||||
and GET .../recommendations must return them in TMDb's ranked order."""
|
||||
|
||||
# videos overridden to empty: FAKE_MOVIE's embedded video has a fixed
|
||||
# provider_video_id, and two different movies both ingesting it would
|
||||
# collide on the real uq_video_provider_source unique constraint --
|
||||
# a video id is only ever attached to one entity, unlike genres/companies
|
||||
# which are legitimately shared.
|
||||
# videos and imdb_id overridden: FAKE_MOVIE's embedded video and imdb_id
|
||||
# are both fixed identifiers, and two different movies both ingesting
|
||||
# them unmodified would collide on the real uq_video_provider_source /
|
||||
# uq_external_id_source unique constraints -- each belongs to exactly
|
||||
# one entity, unlike genres/companies which are legitimately shared.
|
||||
sequel_payload = {
|
||||
**FAKE_MOVIE,
|
||||
"id": 604,
|
||||
"title": "The Matrix Reloaded",
|
||||
"videos": {"results": []},
|
||||
"imdb_id": "tt0234215",
|
||||
}
|
||||
await ingest_movie(db_session, FakeTMDbProvider(sequel_payload), "604")
|
||||
await db_session.flush()
|
||||
|
||||
Reference in New Issue
Block a user