peter/siftlode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

test(migrations): only restore schema in finally when the re-upgrade failed

Browse Source 
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.
This commit is contained in:
peter
2026-08-18 03:40:11 +02:00
parent 0340d0981c
commit bfc104a9b4
1 changed files with 7 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/test_migrations.py
+7 -3
View File
@@ -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. # it lands, not during a rollback. Leaves the schema back at head for the rest of the session.
cfg = Config("alembic.ini") cfg = Config("alembic.ini")
command.downgrade(cfg, "-1") command.downgrade(cfg, "-1")
reupgraded = False
try: try:
command.upgrade(cfg, "head") command.upgrade(cfg, "head")
reupgraded = True
assert _structural_drift() == [] assert _structural_drift() == []
finally: finally:
# Always leave the SHARED schema at head, whatever the upgrade/assert above did — otherwise a # If the re-upgrade itself failed, restore the SHARED schema to head so the failure doesn't
# failure here strands the DB one revision behind and cascades into every later DB-lane test. # cascade into every later DB-lane test. Skip it when the upgrade already succeeded — re-running
command.upgrade(cfg, "head") # 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")