-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes to dumpData #43
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved because the code change seems fine, but please fix the tests before merging.
name: string | ||
} | ||
interface TestDataFixture { | ||
[partitionName: string]: TestData[] | ||
} | ||
|
||
describe('HashBase baselet', function () { | ||
const memlet = makeMemlet(makeMemoryDisklet()) | ||
let hashbaseDb: HashBase<TestData> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for the future, since this is out of scope for your changes:
Each test suite is supposed to be standalone. Mutating a single hashbaseDb
from test-to-test means a failure earlier, like in the "insert" test, will also cause fake failures in the "query" test, because the state is wrong. Each test should create & initialize its own unique HashBase instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is inherited. I can create a task for this, but which project should such a task go (backlog?)
This includes the test for `dumpData` with and without a partition argument.
We shouldn't be assuming that an empty partition will be used by the user, so we shouldn't initialize all CountBase instances with the empty partition in the config.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
none