test: assert full chain has single head in hybrid test#15
Closed
Conversation
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🔴 👀 Review RequirementsThis rule is failing.
🔴 🔎 ReviewsThis rule is failing.
🟢 🤖 Continuous IntegrationWonderful, this rule succeeded.
🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 📕 PR descriptionWonderful, this rule succeeded.
|
jd
force-pushed
the
devs/jd/worktree-no-break-on-changes/I69504832cf0d9f6caaf0290df1f6ca0575adcd53
branch
from
1c178e5 to
28c9ad5
Compare
There was a problem hiding this comment.
Pull request overview
Extends the hybrid-chain regression test to reconstruct the complete migration graph (static + dynamic) and assert there is exactly one Alembic head, preventing a recurrence of “multiple heads” scenarios when dynamic migrations are inserted before a hybrid migration.
Changes:
- Reconstructs a full
{revision: down_revision}mapping by reading staticdown_revisionfrom files and resolving dynamic ones viaget_down_revision(). - Adds a head computation (
heads = all_revisions - pointed_to) and asserts a single expected head.
Comments suppressed due to low confidence (1)
tests/test_chain.py:480
- The dynamic/static detection here uses a raw substring check (
"get_down_revision(" in content), which can misclassify files if the string appears in a comment or import. Since the library already has_DYNAMIC_DOWN_REVISION_REfor this purpose, consider using that regex to match the same logic as production code and keep the test robust to harmless text changes.
"""get_down_revision auto-discovers versions_dir from caller's location."""
versions_dir = tmp_path / "versions"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/test_chain.py
Outdated
Comment on lines
+469
to
+474
| files = _chain._parse_migration_files(versions_dir, git_order) | ||
| full = {f.revision: f.static_down_revision for f in files if not f.is_dynamic} | ||
| full.update(chain) | ||
| all_revs = set(full) | ||
| pointed_to = {v for v in full.values() if v is not None} | ||
| heads = all_revs - pointed_to |
There was a problem hiding this comment.
build_chain is a functools.cache and this test clears it before the patch, but doesn’t clear it afterward. For better test isolation (and to avoid leaving cached entries behind when this assertion fails), wrap this block in a try/finally that calls build_chain.cache_clear() in the finally (similar to nearby tests).
jd
force-pushed
the
devs/jd/worktree-no-break-on-changes/I69504832cf0d9f6caaf0290df1f6ca0575adcd53
branch
from
28c9ad5 to
f4cc95c
Compare
Extend test_dynamic_inserted_before_hybrid_no_multiple_heads to reconstruct the complete chain (static + dynamic) using get_down_revision for dynamic migrations and file parsing for static ones, then verify there is exactly one head. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Change-Id: I69504832cf0d9f6caaf0290df1f6ca0575adcd53 Claude-Session-Id: f2902019-6258-4c62-9887-f23b8128665f
jd
force-pushed
the
devs/jd/worktree-no-break-on-changes/I69504832cf0d9f6caaf0290df1f6ca0575adcd53
branch
from
f4cc95c to
fa2900f
Compare
jd
deleted the
devs/jd/worktree-no-break-on-changes/I69504832cf0d9f6caaf0290df1f6ca0575adcd53
branch
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
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.
Extend test_dynamic_inserted_before_hybrid_no_multiple_heads to
reconstruct the complete chain (static + dynamic) using
get_down_revision for dynamic migrations and file parsing for
static ones, then verify there is exactly one head.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com