From bfc104a9b455b8fc1b80e5ffcee7016b984dbd5c Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 18 Aug 2026 03:40:11 +0200 Subject: [PATCH] test(migrations): only restore schema in finally when the re-upgrade failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2: the unconditional `command.upgrade(cfg, "head")` in finally ran a redundant no-op on the success path and, on a genuinely broken re-upgrade, replaced the try's original exception with its own. Gate the restore on a `reupgraded` flag so it fires only when the upgrade didn't complete — the cascade-prevention intent is kept without the double-run or error-masking. --- backend/tests/test_migrations.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/backend/tests/test_migrations.py b/backend/tests/test_migrations.py index 9d02b6b6..b09b03c3 100644 --- a/backend/tests/test_migrations.py +++ b/backend/tests/test_migrations.py @@ -59,10 +59,14 @@ def test_head_migration_downgrades_and_reupgrades(db): # it lands, not during a rollback. Leaves the schema back at head for the rest of the session. cfg = Config("alembic.ini") command.downgrade(cfg, "-1") + reupgraded = False try: command.upgrade(cfg, "head") + reupgraded = True assert _structural_drift() == [] finally: - # Always leave the SHARED schema at head, whatever the upgrade/assert above did — otherwise a - # failure here strands the DB one revision behind and cascades into every later DB-lane test. - command.upgrade(cfg, "head") + # If the re-upgrade itself failed, restore the SHARED schema to head so the failure doesn't + # cascade into every later DB-lane test. Skip it when the upgrade already succeeded — re-running + # it would be a redundant no-op and could mask a failing assert with the finally's own error. + if not reupgraded: + command.upgrade(cfg, "head")