From db633777543606d57d7503feb5545aab440c982d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 21 Sep 2026 14:15:52 -0400 Subject: [PATCH] test: placement API tests must commit, not flush (4246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR --- tests/test_api_placement.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/test_api_placement.py b/tests/test_api_placement.py index 4bbe5e6..d9f0f49 100644 --- a/tests/test_api_placement.py +++ b/tests/test_api_placement.py @@ -35,9 +35,12 @@ def test_placement_runs_on_the_long_maintenance_lane(): def _run(db, status="ready", moves=None, artist_id=None): - """Adds the row; the caller awaits the flush. `db` is the ASYNC session, - so flushing here would leave an un-awaited coroutine and the row would - never reach the database.""" + """Adds the row; the caller awaits the COMMIT. + + 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( status=status, artist_id=artist_id, moves=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, ) db.add(run) - await db.flush() + await db.commit() resp = await client.get("/api/cleanup/placement/runs") assert resp.status_code == 200 @@ -74,7 +77,7 @@ async def test_run_detail_carries_the_moves(client, db): planned_count=1, ) db.add(run) - await db.flush() + await db.commit() resp = await client.get(f"/api/cleanup/placement/runs/{run.id}") 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") db.add(artist) - await db.flush() + await db.commit() resp = await client.post( "/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 be re-applied by a stray POST.""" run = _run(db, status="applied") - await db.flush() + await db.commit() resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/apply") 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 async def test_revert_refuses_a_run_that_was_never_applied(client, db): run = _run(db, status="ready") - await db.flush() + await db.commit() resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/revert") 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), ) run = _run(db, status="ready") - await db.flush() + await db.commit() resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/apply") assert resp.status_code == 202