getodk / central-backend

Node.js based backend for ODK Central
https://docs.getodk.org/central-intro/
Apache License 2.0
50 stars 76 forks source link

Revisit testing database migrations #466

Open ktuite opened 2 years ago

ktuite commented 2 years ago

In PR #464, we added a new column description to Projects, which broke tests of previous database migrations (here).

The problem with the migration tests is that the code (frames.js) reflects the latest structure of the database (with projects.description column), but the migration test starts by applying migrations only up to an earlier database state (without that description column). So the code to populate the database with the initial project doesn't work because it's expecting a database column that doesn't exist. The way these database migration tests work needs to fundamentally change to not have this code state / database state inconsistency.

Some thoughts: 1) maybe the migrations served their purpose at the time but they don’t need to be re-tested going forward? 2) we need to see how other people test migrations because surely they run into this problem. 3) the data needs to be put in the database in a different way (e.g. raw sql or loading a pgdump or something, not created by the server) that is more consistent with how it will look at the time of the migration in question. 4) there has been talk of more extensive database testing that looks at performance, and that could potentially include migration performance.

For now, we will just skip the migration tests until we come up with a better solution.

matthew-white commented 1 week ago

@alxndrsn is working on a new approach to testing migrations here: #1252