-
Notifications
You must be signed in to change notification settings - Fork 42
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
db migration testing #1068
Comments
@creviera @emkll I think this would be a good preparatory change in support of read/unread, which will almost certainly require a client DB migration, and other near term new features. What do you think? |
Yes, this should come first, before #1149 and the migration to move update null Thinking about order of operations, ideally we would:
|
I've tentatively added this to the next sprint candidates, but per the above it may make sense to start work on this still in the current sprint (ending Thursday), and move #1149 (which is in the current sprint) to the next one. |
On SecureDrop server we have alembic db migration tests, see this documentation for background/explanation: https://docs.securedrop.org/en/release-1.2.2/development/database_migrations.html?highlight=alembic#unit-testing-migrations
We added the same test runner here but: we haven't been adding the corresponding tests to ensure that the database updates cleanly. I've found these tests pretty useful, so we could start writing them to catch issues with database migrations (otherwise we need to manually upgrade the database from
master
to test, which we could add to the PR checklist for now).The text was updated successfully, but these errors were encountered: