test: Expand CoreKV repo testing #6
Merged
Conversation
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
Note how only one test in the suites was ever actually executed, the rest were commented out of the executing set.
TestNewDatastore was also removed, as all our tests will check this and its not really worth re-creating it IMO.
The migrate test has been changed slightly, as calling `Close` after cancelling does not IMO add any value. It loooks like it made sense when Closing a closed store errored, but that changed a while back in order to match badger behaviour.
This is a sneaky bug fix, where for this kind of error, the memory and badger implementations were returning different errors. This is tested by a test migrated in a later commit (previously badger was untested).
Note, the two original tests are duplicates of each other as one sets the value in newLoadedDatastore and the other just sets it inline
Badger and our memory store differ here, and the badger error suggested that I should not try and tackle the difference within this PR as the error is returned for multiple reasons.
Case is already covered by already-migrated tests.
nasdf
approved these changes
Jan 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relevant issue(s)
Resolves #13
Description
Replaces the old tests with a more formal framework that easily provides coverage for all store implementations (there are two open PRs for new implementations, so hopefully it should speed those up too), and enforces their consistency, as well as hopefully being a little easier to read.
The framework is designed to allow the bulk of the stuff to be used within bench tests, as well as integration tests.
Currently excluded from this PR are the transaction and the iterator tests, as currently the memory store does not implement the corekv interfaces there. Those tests can be sorted out in #14 .
It also fixes a minor inconsistency (bug) regarding a returned error message.
I recommend reviewing commit by commit, it should be easier to see from where tests have been moved. Some commit messages provide a little more info on the reasoning behind the changes.