test: placement API tests must commit, not flush (4246)
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
CI / extension-version (push) Successful in 2s
Build images / build-agent (push) Successful in 8s
Build images / build-ml (push) Successful in 8s
Build images / build-web (push) Successful in 6s
Build images / smoke-web (push) Skipped
Build images / promote (push) Skipped
CI / frontend-build (push) Successful in 23s
CI / backend-lint-and-test (push) Successful in 37s
CI / integration (push) Failing after 2m14s
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
CI / extension-version (push) Successful in 2s
Build images / build-agent (push) Successful in 8s
Build images / build-ml (push) Successful in 8s
Build images / build-web (push) Successful in 6s
Build images / smoke-web (push) Skipped
Build images / promote (push) Skipped
CI / frontend-build (push) Successful in 23s
CI / backend-lint-and-test (push) Successful in 37s
CI / integration (push) Failing after 2m14s
Five of the new tests failed in CI: every one that created a LibraryPlacementRun and then read it back through the client. The ones that touched no rows passed. A flush stays inside the test's own transaction, and the app under test runs on a separate session and connection — so the endpoint queried a database where the row did not exist yet and got its 404 / empty list honestly. `_seed_runs` in test_api_system_backup already commits for this reason; the idiom was there to copy and I did not look first. Recorded in the helper's docstring rather than just fixed, since the next person writing a create-then-fetch API test will reach for flush too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
This commit is contained in:
@@ -35,9 +35,12 @@ def test_placement_runs_on_the_long_maintenance_lane():
|
|||||||
|
|
||||||
|
|
||||||
def _run(db, status="ready", moves=None, artist_id=None):
|
def _run(db, status="ready", moves=None, artist_id=None):
|
||||||
"""Adds the row; the caller awaits the flush. `db` is the ASYNC session,
|
"""Adds the row; the caller awaits the COMMIT.
|
||||||
so flushing here would leave an un-awaited coroutine and the row would
|
|
||||||
never reach the database."""
|
Commit, not flush: the app under test runs on its own session and
|
||||||
|
connection, so a flush that stays inside this test's transaction is
|
||||||
|
invisible to the endpoint — the row simply is not there yet. Same reason
|
||||||
|
`_seed_runs` in test_api_system_backup commits."""
|
||||||
run = LibraryPlacementRun(
|
run = LibraryPlacementRun(
|
||||||
status=status, artist_id=artist_id, moves=moves or [],
|
status=status, artist_id=artist_id, moves=moves or [],
|
||||||
planned_count=len(moves or []),
|
planned_count=len(moves or []),
|
||||||
@@ -56,7 +59,7 @@ async def test_runs_list_omits_the_moves(client, db):
|
|||||||
planned_count=1, moved_count=1,
|
planned_count=1, moved_count=1,
|
||||||
)
|
)
|
||||||
db.add(run)
|
db.add(run)
|
||||||
await db.flush()
|
await db.commit()
|
||||||
|
|
||||||
resp = await client.get("/api/cleanup/placement/runs")
|
resp = await client.get("/api/cleanup/placement/runs")
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
@@ -74,7 +77,7 @@ async def test_run_detail_carries_the_moves(client, db):
|
|||||||
planned_count=1,
|
planned_count=1,
|
||||||
)
|
)
|
||||||
db.add(run)
|
db.add(run)
|
||||||
await db.flush()
|
await db.commit()
|
||||||
|
|
||||||
resp = await client.get(f"/api/cleanup/placement/runs/{run.id}")
|
resp = await client.get(f"/api/cleanup/placement/runs/{run.id}")
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
@@ -109,7 +112,7 @@ async def test_plan_accepts_an_artist_scope(client, db, monkeypatch):
|
|||||||
)
|
)
|
||||||
artist = Artist(name="Conto", slug="conto")
|
artist = Artist(name="Conto", slug="conto")
|
||||||
db.add(artist)
|
db.add(artist)
|
||||||
await db.flush()
|
await db.commit()
|
||||||
|
|
||||||
resp = await client.post(
|
resp = await client.post(
|
||||||
"/api/cleanup/placement/plan", json={"artist_id": artist.id},
|
"/api/cleanup/placement/plan", json={"artist_id": artist.id},
|
||||||
@@ -123,7 +126,7 @@ async def test_apply_refuses_a_run_that_is_not_ready(client, db):
|
|||||||
"""The gate is here as well as in the service — an applied run must not
|
"""The gate is here as well as in the service — an applied run must not
|
||||||
be re-applied by a stray POST."""
|
be re-applied by a stray POST."""
|
||||||
run = _run(db, status="applied")
|
run = _run(db, status="applied")
|
||||||
await db.flush()
|
await db.commit()
|
||||||
|
|
||||||
resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/apply")
|
resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/apply")
|
||||||
assert resp.status_code == 400
|
assert resp.status_code == 400
|
||||||
@@ -133,7 +136,7 @@ async def test_apply_refuses_a_run_that_is_not_ready(client, db):
|
|||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_revert_refuses_a_run_that_was_never_applied(client, db):
|
async def test_revert_refuses_a_run_that_was_never_applied(client, db):
|
||||||
run = _run(db, status="ready")
|
run = _run(db, status="ready")
|
||||||
await db.flush()
|
await db.commit()
|
||||||
|
|
||||||
resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/revert")
|
resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/revert")
|
||||||
assert resp.status_code == 400
|
assert resp.status_code == 400
|
||||||
@@ -150,7 +153,7 @@ async def test_apply_dispatches_for_a_ready_run(client, db, monkeypatch):
|
|||||||
lambda run_id: sent.update(run_id=run_id),
|
lambda run_id: sent.update(run_id=run_id),
|
||||||
)
|
)
|
||||||
run = _run(db, status="ready")
|
run = _run(db, status="ready")
|
||||||
await db.flush()
|
await db.commit()
|
||||||
|
|
||||||
resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/apply")
|
resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/apply")
|
||||||
assert resp.status_code == 202
|
assert resp.status_code == 202
|
||||||
|
|||||||
Reference in New Issue
Block a user