Add stairway test for DB migrations - #64905
Conversation
jason810496
left a comment
There was a problem hiding this comment.
Thanks for hardening the migration test.
There was a problem hiding this comment.
Pull request overview
Adds a new “stairway” unit test intended to validate that each Alembic migration step is reversible by performing upgrade → downgrade → re-upgrade per revision, starting from the squashed baseline revision.
Changes:
- Introduces a helper to enumerate migration revisions in upgrade order after the squashed base.
- Adds a parametrized migration test that runs upgrade/downgrade/re-upgrade for each revision.
- Attempts to reduce parallel-test DB conflicts via an
xdist_groupmarker.
|
Interesting idea. Can you please address the comments from copilot ? |
No problem, thank you for your feedback! |
…ine and improve DB state management
67cb6a8 to
ff0ab6d
Compare
|
@iting0321 A few things need addressing before review — see our Pull Request quality criteria.
No rush. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
jason810496
left a comment
There was a problem hiding this comment.
Nice! LGTM for hardening the DB migration tests.
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
* add stairway tests for Airflow DB migrations to verify reversibility * refactor migration stairway test to use _REVISION_HEADS_MAP for baseline and improve DB state management * refactor stairway test to start from 3.0.0 revision and improve migration handling * refactor stairway test failure handling to raise AssertionError for better clarity * fix: add space in return statement for consistency in revision order --------- Co-authored-by: iting0321 <iting0321@MacBook-11111156.local>
* add stairway tests for Airflow DB migrations to verify reversibility * refactor migration stairway test to use _REVISION_HEADS_MAP for baseline and improve DB state management * refactor stairway test to start from 3.0.0 revision and improve migration handling * refactor stairway test failure handling to raise AssertionError for better clarity * fix: add space in return statement for consistency in revision order --------- (cherry picked from commit 8a6a79f) Co-authored-by: iting0321 <137410100+iting0321@users.noreply.github.com> Co-authored-by: iting0321 <iting0321@MacBook-11111156.local>
* add stairway tests for Airflow DB migrations to verify reversibility * refactor migration stairway test to use _REVISION_HEADS_MAP for baseline and improve DB state management * refactor stairway test to start from 3.0.0 revision and improve migration handling * refactor stairway test failure handling to raise AssertionError for better clarity * fix: add space in return statement for consistency in revision order --------- (cherry picked from commit 8a6a79f) Co-authored-by: iting0321 <137410100+iting0321@users.noreply.github.com> Co-authored-by: iting0321 <iting0321@MacBook-11111156.local>
* add stairway tests for Airflow DB migrations to verify reversibility * refactor migration stairway test to use _REVISION_HEADS_MAP for baseline and improve DB state management * refactor stairway test to start from 3.0.0 revision and improve migration handling * refactor stairway test failure handling to raise AssertionError for better clarity * fix: add space in return statement for consistency in revision order --------- (cherry picked from commit 8a6a79f) Co-authored-by: iting0321 <137410100+iting0321@users.noreply.github.com> Co-authored-by: iting0321 <iting0321@MacBook-11111156.local>
Adds a test that walks every incremental Alembic migration one step at a time upgrade → downgrade → re-upgrade to verify all migrations are fully reversible.
Why:
Silent breakage can occur when a downgrade forgets to drop a constraint/column added by its upgrade, or references something that doesn't exist. The existing
test_database_schema_and_sqlalchemy_model_are_in_synconly checks the final state; it won't catch a broken intermediate step.How to test:
When a step fails, the error message includes the specific revision ID so the broken migration is immediately identifiable in CI logs.
Was generative AI tooling used to co-author this PR?