test(vector-index): resolve the repair migration's parent instead of branch@-1 - #3238
Merged
Merged
Conversation
…branch@-1 `test_migration_drops_stale_global_index` stepped below the repair migration with `command.downgrade(cfg, "f2a6d8c4b1e9@-1")`. That is alembic's branch@relative syntax: it counts back from the *head* of the branch containing the revision, not from the revision itself. It meant "the parent" only for as long as f2a6d8c4b1e9 was head. b3e8d1c6f4a9 (#3214) landed on top of it, so `@-1` began resolving to f2a6d8c4b1e9 itself: the downgrade stopped ON the repair migration, the test planted the stale index after the drop had already run, and the upgrade never re-ran it. The test has been failing on main for every PR since — `assert 1 == 0` — with nothing wrong in the migration it covers. The parent now comes from the revision map, so the next migration added on top cannot break it. Verified it still fails when the DROP INDEX is disabled.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the
test-api (1/3)failure that has been red on main for every PR since #3214 merged (seen on #3237, #3235, #3234).What broke
test_migration_drops_stale_global_indexsteps below the repair migration before planting a stale global index:f2a6d8c4b1e9@-1is alembic's branch@relative syntax — it counts back from the head of the branch containing that revision, not from the revision itself. While f2a6d8c4b1e9 was head, that happened to be its parent, so the test worked.#3214 added
b3e8d1c6f4a9on top of it.@-1now resolves to f2a6d8c4b1e9 itself, so:The stale index gets planted after the drop has already run, the upgrade never re-runs it, and the assertion fails with
assert 1 == 0. The migration under test is fine — only the test's setup was wrong, and any migration landing after the repair would have triggered it.Fix
Resolve the parent from the revision map:
This cannot rot when the next migration lands on top.
Verified
DROP INDEX IF EXISTS ...disabled in f2a6d8c4b1e9, the test fails again (assert 1 == 0) — so it still exercises the behaviour it claims to, rather than passing vacuously.lint.shclean.