tapdb: add minimalistic test framework for testing DB migrations #707
Conversation
This commit extracts the actual execution of the DB migration code into its separate method.
This commit allows us to either run all migrations or up to a given version on the respective stores. This will enable us to test individual migration steps in unit tests.
This commit adds database specific detection for errors that represent a mismatch between the query and the actual database schema. This will mainly be used for unit tests and should never occur in production code.
This was found in the unit test that will be added in the next commit. We need to wrap database errors in order for us to parse them correctly as database specific typed errors.
This test is mostly a demo for how to use the migration testing framework. We start with the DB that only contains the very first migration file, which we assert by making sure we encounter a schema error when attempting to read assets. We then move forward to version 11, insert some dummy data, then run all migrations up to the latest version.
|
Super cool! |
|
Thanks for opening this PR! One question I'd have w.r.t to the approach is what kind of tests we're exactly running here? |
The idea is that we have a way to test migrations that do data manipulations. Those are quite hard to test with just a unit test that looks at the state after all migrations have been applied. One of those PRs is #684 (which was taken as the inspiring PR to influence some of the design in this PR). |
There was a problem hiding this comment.
Very neat! I think this would have worked for validating the latest changes to the anchor_txid view, and repro'ing that user issue.
Re: test data; I imagine we could generate a 'golden' DB state from one of itests and use that for testing future migrations. Feels like using this in future tests shouldn't be too much work.
Needs rebase.
There was a problem hiding this comment.
Was a bit skeptical initially, but I think this nicely resolves issues that would pop up in a purely unit test approach, namely: we wouldn't be able to programmatically create older versions of certain tables as the Go code is generated from the latest version of the migration.
It's also the case that all the generated code already has SQL fragments in them, and you can look at the older versions to extract raw SQL used for the insertion files.
LGTM 🥕
| @@ -861,7 +861,7 @@ func fetchAssetsWithWitness(ctx context.Context, q ActiveAssetsStore, | |||
| // First, we'll fetch all the assets we know of on disk. | |||
| dbAssets, err := q.QueryAssets(ctx, assetFilter) | |||
| if err != nil { | |||
| return nil, nil, fmt.Errorf("unable to read db assets: %v", err) | |||
| return nil, nil, fmt.Errorf("unable to read db assets: %w", err) | |||
This is an attempt at solving #694.
There are many ways to solve this, and this is IMO the most easy to implement one.
So this is mainly a proposal to get the discussion started. If we feel like we need something more involved, we can discuss based on this example code.
I mainly had this PR in mind when coming up with the design for this: #684
There we need to have a way to insert existing data into the DB before applying the latest migration.
Here are the pros/cons of the current approach:
Pros:
Cons:
Fixes #694