From 0567616b2531cfa52b327f2252e4782e0b32451d Mon Sep 17 00:00:00 2001 From: BADRI NARAYANAN Date: Mon, 7 Sep 2026 00:58:52 -0700 Subject: [PATCH] fix(ci): stop test_orders.py's session-scoped fixture leaking tables into the full suite CI has been red on every push to main since PR #25 -- confirmed via `gh run list`, green through PR #24, failing on every merge since. Root cause, reproduced deterministically from a genuinely fresh, isolated Postgres/Redis container matching CI's exact setup (not my local dev DB, which had accumulated its own unrelated cross-session cruft this session and was a red herring for earlier, separate investigations): test_orders.py's `_order_tables` fixture was `scope="session", autouse=True` -- the only session-scoped table fixture in the entire backend test suite; every other API test file (test_portfolios_api.py, test_portfolios_holdings.py, test_valuation_api.py, test_funding.py, ...) uses a function-scoped autouse fixture that creates its tables before each test and drops them after. Session scope meant test_orders.py's 16 tables -- critically `security`, which nearly every other file also manages independently via its own `checkfirst=True` create -- stayed alive for the rest of the pytest *process*, not just for test_orders.py's own tests. Any other file that ran afterward in the same process silently reused this still-alive `security` table instead of getting a fresh one, and when that file's own per-test teardown tried its usual non-CASCADE `DROP TABLE "security"`, it failed with DependentObjectsStillExist because test_orders.py's `tax_lot` (also still alive) still referenced it. That single failed DROP rolls back its whole teardown transaction, so every table in that file's list survives with its data into the next file too -- explaining both observed CI symptoms: `uq_security_symbol` duplicate-key errors and a leftover "Growth" model_portfolio row bleeding into an unrelated assertion. This is why it only ever showed up in CI/a true full-suite run and never in any of this session's many scoped-file or small-combo local test runs -- confirmed by reproducing it with test_orders.py + test_portfolios_api.py in one pytest invocation (fails) versus the exact same two files run as separate invocations against the same DB state (passes) -- the collision requires both files' fixtures to be alive in the same pytest session, not just the same database. Fixed by making `_order_tables` function-scoped like every other file, folding in the now-redundant `_clean_order_tables` TRUNCATE-based second fixture (full create/drop per test already resets everything a TRUNCATE would have). Costs more CREATE/DROP churn across this file's ~50 tests, matching the cost every other file already pays for correct isolation. Verified: full `uv run pytest --cov=app --cov-report=term-missing` (CI's exact command) against a fresh, isolated Postgres/Redis container -- 932 passed, 0 failed, 0 errors, 84.52% coverage (floor 77%). --- backend/tests/api/test_orders.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/backend/tests/api/test_orders.py b/backend/tests/api/test_orders.py index 018d559..fc9a691 100644 --- a/backend/tests/api/test_orders.py +++ b/backend/tests/api/test_orders.py @@ -50,8 +50,20 @@ DEFAULT_TEST_CASH = Money("1000000.00") -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(autouse=True) def _order_tables(owner_engine: Engine) -> Iterator[None]: + """Function-scoped, matching every other API test file's own convention (test_portfolios_api.py, + test_portfolios_holdings.py, test_valuation_api.py, test_funding.py, ...) -- this fixture was + previously `scope="session"` as a performance shortcut, which left `security` (and 15 other + tables) alive for the entire pytest process instead of tearing down when this file's tests + finished. Every other file that also creates its own `security` table via `checkfirst=True` + then silently reused this file's still-alive one instead of getting a fresh table, and their + own per-test teardown's non-CASCADE `DROP TABLE "security"` failed with + `DependentObjectsStillExist` once this file's `tax_lot` (still alive) referenced it -- + reproducible only in a full-suite run, never in isolation, which is exactly how it stayed + hidden through several PRs' worth of scoped-file-only local testing. Function scope costs + more CREATE/DROP churn across this file's ~50 tests but that's the price every other file in + this suite already pays for correct isolation.""" tables = [ Security.__table__, InboundEvent.__table__, @@ -89,20 +101,6 @@ def _order_tables(owner_engine: Engine) -> Iterator[None]: connection.execute(text(f'DROP TABLE IF EXISTS "{table.name}" CASCADE')) -@pytest.fixture(autouse=True) -def _clean_order_tables(owner_engine: Engine, _order_tables: None) -> Iterator[None]: - yield None - with owner_engine.begin() as connection: - connection.execute( - text( - "TRUNCATE approval_hold, wash_sale_adjustment, lot_consumption, tax_lot, " - "order_event, \"order\", customer_cash_lock, " - "idempotency_key, job_outbox, admin_audit_log, settlement_obligation, posting, " - "journal_entry, account, inbound_event RESTART IDENTITY CASCADE" - ) - ) - - class _LedgerLikeUow: """Duck-typed stand-in for `LedgerUnitOfWork`, matching `test_cash_policy.py`'s fixture helper."""