-
Notifications
You must be signed in to change notification settings - Fork 92
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Update River test helpers to allow for River drivers
We recently added River database drivers that allow it to be more agnostic about the database package in use with River rather than being tightly tied to pgx v5. The test helpers were also quite a recent addition and when the drivers came in weren't updated to use a similar system, thereby still being coupled to pgx v5. The purpose of this change is to get test helpers onto the drivers system as well. Like with top-level River, it's mostly about getting the right APIs in place -- we're still tightly coupled to pgx v5 under the covers, and will have to refactor all of that at a later time. The old API doesn't quite work anymore, so we bifurcate the helpers into something more akin to what we see in `river.Client`, with a transaction and non-transaction variant. Furthermore, because these are top-level package helpers, we need to take a driver generic (at least for the transaction-based variants), making them look like `JobCompleteTx`: * `RequireInserted(ctx, t, riverpgxv5.New(dbPool), &RequiredArgs{}, nil)` * `RequireInsertedTx[*riverpgxv5.Driver](ctx, t, tx, &RequiredArgs{}, nil)` * `RequireManyInserted(ctx, t, riverpgxv5.New(dbPool), []rivertest.ExpectedJob{...}) * `RequireManyInsertedTx[*riverpgxv5.Driver](ctx, t, tx, []rivertest.ExpectedJob{...}) One approach I experimented with was to do something similar to the `require` package by allowing a struct to be created that'd then give you a shortcut without type parameters necessary (kind of like what `Client` does currently): tRiver := rivertest.New(riverpgxv5.New(dbPool)) tRiver.RequireInsert(ctx, t, &RequiredArgs{}, nil) tRiver.RequireInsertTx(ctx, t, tx, &RequiredArgs{}, nil) The problem I ran into there is similar to the top-level insert helpers we ended up removing from `river` -- namely that Go has the limitation that struct-level functions can't have their own type parameters, so going with that model would no longer allow `RequireInsert` to return `Job[T]` (it'd have to be `JobRow` instead) which is a non-starter right now because we want people to be able to easily assert on the return values of these functions. I'm still not sure this is the package's perfect form, but is pretty goo,d and I think this is about as good as we're going to be able to for now, and is ~identical functionality-wise to what we have before. I'm still open to continuing to experiment with args equality assertions being built-in, but we should try to get a reasonably quick fix for the missing driver problem in so we don't have anyone accidentally start using an API that's definitively outdated.
- Loading branch information
Showing
4 changed files
with
308 additions
and
82 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.