Refactor migrations to allow to easily add new test models #6244
Conversation
Closes #6236 changelog_begin changelog_end
stefanobaghino-da
requested review from
aherrmann-da,
cocreature and
garyverhaegen-da
as code owners
There was a problem hiding this comment.
Awesome, thanks a lot for cleaning up the mess I created! Very happy to get feedback from someone that knows Scala on my Scala code 🙂
One thing that is still missing and is just as important is the logic for validating the result from the step in the Haskell binary. This is currently tied completely to the specific model. I have some ideas for making that more generic but I’d definitely be interested in hearing your thoughts! (Let’s leave it out of this PR)
compatibility/BUILD
Outdated
| @@ -115,3 +115,15 @@ migration_test( | |||
| tags = ["exclusive"], | |||
| versions = platform_versions, | |||
| ) if not is_windows else None | |||
|
|
|||
| migration_test( | |||
| name = "migration-test", | |||
There was a problem hiding this comment.
If you want to keep this can we give it a more descriptive name? Also why is this particular combination useful? Latest stable vs HEAD makes more sense to me.
There was a problem hiding this comment.
I'll remove it for now and re-introduce it later. I was thinking that if you had a single test that covers all migrations in one go you can lower the chances of missing weird regressions when using this rule to quickly test while developing. Let me know what you think about it (and what name could make more sense: migration-meta-test, meta-migration-test, test-migration-test, migration-test-test? Something else?)
There was a problem hiding this comment.
Maybe just make it very explicit, migration-1.0.0-to-head? I don’t mind keeping it, I just find the name to be a bit too vague.
There was a problem hiding this comment.
Addressed by 8845557
Not really a mess, I mostly broke pieces apart.
I know, but I opened a separate ticket for that (#6237), I didn't want to do too much in a single PR. |
Closes #6236
changelog_begin
changelog_end
Pull Request Checklist
CHANGELOG_BEGINandCHANGELOG_ENDtagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines runtotrigger the build.