testutils: add storageutils test utilities
#83389
Conversation
|
This change is |
erikgrinaker
force-pushed
the
storageutils
branch
6 times, most recently
from
02f7c3c to
5da9b72
Compare
There was a problem hiding this comment.
Reviewed 19 of 19 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @aliher1911 and @erikgrinaker)
This will (hopefully) be addressed separately.
It sounds like we want the utils moved out of storage and into storageutils. Then storage (both the test and lib targets) can have a dep on this new package, removing the need for duplication? Or is it more nuanced than that?
If that sounds right, I'll create an issue and we can get this cleaned up in the background.
There was a problem hiding this comment.
Yeah, and this is all fairly straightforward code. TFTR!
Reviewable status:
Previously, nicktrav (Nick Travers) wrote…
This will (hopefully) be addressed separately.
It sounds like we want the utils moved out of
storageand intostorageutils. Thenstorage(both the test and lib targets) can have a dep on this new package, removing the need for duplication? Or is it more nuanced than that?If that sounds right, I'll create an issue and we can get this cleaned up in the background.
Not that simple, I'm afraid. storageutils must depend on storage, because it needs e.g. storage.MVCCKey, but that means that the tests in storage can't depend on storageutils, because it depends on storage, so it's a circular dependency.
For this to work, all storage tests must be in storage_test, which depends on storageutils, which depends on storage.
I really wish it would be possible to just import _test.go contents from a different package, so e.g. batcheval could just import test utils from storage, but no such luck -- this is the canonical way, apparently.
erikgrinaker
force-pushed
the
storageutils
branch
from
5da9b72 to
97fdacb
Compare
This patch adds a bunch of test utilities to `storageutils`, replacing the old `sstutil` package. This is done to ease testing of MVCC range keys in tests outside the `storage` package. Unfortunately, these are mostly duplicates of utilities in `storage`. Storage tests use the `storage` package rather than `storage_test`, and can't make use of `storageutils` yet because it causes an import cycle with `storage`. This will (hopefully) be addressed separately. Release note: None
erikgrinaker
force-pushed
the
storageutils
branch
from
97fdacb to
da020b0
Compare
|
CI failures are unrelated. bors r=nicktrav |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
This patch adds a bunch of test utilities to
storageutils, replacingthe old
sstutilpackage. This is done to ease testing of MVCC rangekeys in tests outside the
storagepackage.Unfortunately, these are mostly duplicates of utilities in
storage.Storage tests use the
storagepackage rather thanstorage_test, andcan't make use of
storageutilsyet because it causes an import cyclewith
storage. This will (hopefully) be addressed separately.Release note: None