tapdb+mssmt: update mssmt unit tests to run against all registered db backends #770
Conversation
This works for both sqlite and postgres. Unlike the existing routines, we don't need to pass in a testing.T and instead want to just return an error.
We use the new `activeTestDB` variable to set the proper DB driver name.
The function will now emit all the registered db drivers vs just special casing for sqlite.
| @@ -402,3 +402,38 @@ func (t *taprootAssetTreeStoreTx) UpdateRoot(rootNode *mssmt.BranchNode) error { | |||
| Namespace: t.namespace, | |||
| }) | |||
| } | |||
|
|
|||
| func init() { | |||
There was a problem hiding this comment.
Should this really be in the non-test code? Since it's only used by unit tests and does spin up an ephemeral container in the Postgres case?
There was a problem hiding this comment.
Good point re test code vs non test code. I'd need to double check, but I don't think an init is run in a _test.go when a package is imported.
Re postgres container: this'll only happen if the New method of the driver is called (won't be created unconditionally).
| @@ -173,6 +173,25 @@ func NewTestPostgresDB(t *testing.T) *PostgresStore { | |||
| return store | |||
| } | |||
|
|
|||
| // NewPostgresDB is a helper function that creates a Postgres database for | |||
| // testing. | |||
| func NewPostgresDB() (*PostgresStore, error) { | |||
There was a problem hiding this comment.
See my other comment about moving the driver registration into a test file. Then we can perhaps create the empty var t testing.T there and still accept it as a parameter here?
Really looks weird having this as non-test code and initialize that variable... Also would allow us to use t.Cleanup() for the sqlFixture.TearDown().
There was a problem hiding this comment.
Yeah the testing.T was a bit of a hack so I could get things working to be able to write the tests that prompted this PR in the first place.
I don't think we want to move the driver init into a test fie though, as this was created so we could write tests in the mssmt package that span all our db versions. Postgres is a bit unique in that it wants an ephemeral container active. We could have the test (which lives in mssmt) create the container itself, but then those details would leak into a package that's only concerned about the mssmt abstraction.
|
!lightninglabs-deploy mute 720h30m |
|
!lightninglabs-deploy unmute |
|
!lightninglabs-deploy mute 360h |
1 similar comment
|
!lightninglabs-deploy mute 360h |
|
@Roasbeef, remember to re-request review from reviewers when ready |
|
!lightninglabs-deploy mute |
Before this PR, we had the logic to execute all registered dbs, but didn't actually register them all. With this PR, we now call
RegisterTreeStore.By default the
mssmtunit tests will run sqlite. You an run postgres with this build tag:-tags=test_db_postgres.