conda-incubator / conda-store

Data science environments, for collaboration. ✨
https://conda.store
BSD 3-Clause "New" or "Revised" License
139 stars 45 forks source link

Test migrations as part of CI #733

Open nkaretnikov opened 6 months ago

nkaretnikov commented 6 months ago

Context

One thing that takes a lot of time right now is checking that existing installations are not broken. This is currently done by hand, which takes a lot of time and is error-prone.

The goal of this issue is to shift responsibility from the PR author to the testing infrastructure.

It needs to be done as part of CI instead. It could work something like this:

  1. Run the whole testsuite on main to populate the DB with realistic data (this is the important part -- to make sure that data is there) and make sure the tests pass (both SQLite and Postgres)
  2. With the populated DB, run the update migration added in the new PR and run the whole testsuite again + new tests added in the new PR (to make sure the migration is successful and new data is not conflicting with the old data/schema)
  3. Then run the downgrade migration with the DB added on step 2 and make sure that tests from main pass.
  4. During merges, the CI needs to check if there are issues with migrations (migrations have hashes and if two PRs both add migrations, one of them needs to be updated -- the CI needs to detect that and run the testsuite again). If there are problems, the PR should be reverted automatically.

Value and/or benefit

Goals:

This will improve both the quality and the development/iteration speed, which will translate into getting more features in after this initial time investment.

Anything else?

Our unit-tests currently run only on SQLite. We need to add a Postgres DB that will execute the same tests.

conda-store can be configured to use postgres like this:

c.CondaStore.database_url = 'postgresql+psycopg2://postgres:password@localhost:5432/conda-store'

It first requires:

Note that we do have API tests that use Docker/Postgres, but that's different because not everything can be tested via those APIs.

Both SQLite and Postgres need to be tested via unit tests because these DBs have some incompatibilities.

For example:

trallard commented 4 months ago

Pinging @dcmcand and @peytondmurray as they have been looking into testing for conda-store

How does this align with other testing improvements planned?

peytondmurray commented 3 months ago

I'll defer to @dcmcand on this as I don't yet have the context to know what the current experience is. It sounds like any time we release a new database migration we need to manually test that nothing breaks. If this is the case, I can definitely see how implementing a workflow to trigger on migrations would be useful.