-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Improve migration integrity tests #16577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
expect(latestDevDocs).toEqual(devDocs) | ||
expect(latestProdDocs).toEqual(prodDocs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these valid assertions? Aren't migrations by definition going to change some documents?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what this test is testing. Each migration should be idempotent, and you should be able to run it multiple times. If it succeeded once, subsequent runs should not change anything (as everything is migrated already). I renamed the variables to make it more verbose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh I totally misunderstood, the rename is much better. Thank you 🙏
Description
Updating the way we run the migration integrity tests to use the
migrationProcessor
. As the logic in there changed, with migrations and app syncs, we want to replicate the same flow.Along the way, I realised that we are updating the createdAt/updatedAt wrongly on user sync. Fixed it and added some tests for it