Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add a flag to enable rocksdb tests #10651

Closed
4 of 6 tasks
robert-zaremba opened this issue Dec 1, 2021 · 1 comment · Fixed by #10430
Closed
4 of 6 tasks

Add a flag to enable rocksdb tests #10651

robert-zaremba opened this issue Dec 1, 2021 · 1 comment · Fixed by #10430
Assignees
@roysc
Labels
C:Store

Comments

@robert-zaremba
Copy link
Collaborator

robert-zaremba commented Dec 1, 2021 •
edited
Loading

Problem Definition

Currently make test will try to test everything in /db, but rocksdb will require additional dependency: the rocksdb-dev (headers) and it could be confusing to people who are running tests and see it failing. We need better reporting.

Proposal

  • Add an environment flag which will:
    • be false by default
    • when false, the rocks db tests will be skipped and we print big warning that a flag must be set to test the rocksdb
    • when true we run all rocksdb tests
  • update CI config to set the flag

Ref: #10472

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@roysc roysc assigned roysc and unassigned i-norden Dec 2, 2021
@roysc
Copy link
Contributor

roysc commented Dec 2, 2021 •
edited
Loading

Fix included in #10430 (40584b7)

@roysc roysc mentioned this issue Dec 7, 2021
Merged
19 tasks
@mergify mergify bot closed this as completed in #10430 Dec 16, 2021
mergify bot pushed a commit that referenced this issue Dec 16, 2021
@roysc
feat: ADR-040: Add RootStore implementation (#10430)
109bc94 
## Description

Part of: #10192

Introduces a new `RootStore` type in the `store/v2` package and an implementation, without yet replacing the `MultiStore` or refactoring its use within the SDK (which will happen in the follow up: #10174).
Specified by [ADR-040](https://github.com/cosmos/cosmos-sdk/blob/1326fa2a7dfc3d83cf23dc1c1f33ff131152ad60/docs/architecture/adr-040-storage-and-smt-state-commitments.md).

Fixes #10651
Fixes #10263

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [x] added a changelog entry to `CHANGELOG.md`
- [x] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [x] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@roysc roysc

Labels
C:Store
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: ADR-040: Add RootStore implementation vulcanize/cosmos-sdk
3 participants
@robert-zaremba @i-norden @roysc