Add proptest-impl feature to zebra-state
#2529
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
This prepares the `zebra-state` crate to be able to export some test-specific helper types and functions.
jvff
force-pushed
the
zebra-state-proptest-impl-feature
branch
from
33bf540 to
6279dfa
Compare
This comment has been minimized.
This comment has been minimized.
teor2345
reviewed
Jul 26, 2021
A separate module to contain the `Prepare` trait, since it's required by some prop-test strategies and therefore can't be in the `tests` module.
Use the same trait but placed in a new module that's accessible based on the feature flag.
It was obsoleted by the new copy in the `arbitrary` module.
jvff
force-pushed
the
zebra-state-proptest-impl-feature
branch
from
6279dfa to
1b84d40
Compare
Prepare for it to be accessible in some test modules.
Import the function directly, instead of just its containing module.
Create a new module for the strategy functions that are only used internally.
jvff
force-pushed
the
zebra-state-proptest-impl-feature
branch
from
1b84d40 to
71e9a6b
Compare
jvff
commented
Jul 27, 2021
There was a problem hiding this comment.
It looks like the new arbitrary.rs file is missing from this branch?
Oops, my bad 😓 Fixed now.
Reviewable status: 0 of 13 files reviewed, 1 unresolved discussion (waiting on @jvff and @teor2345)
zebra-state/src/service/arbitrary.rs, line 151 at r1 (raw file):
Previously, teor2345 (teor) wrote…
It's a bit confusing mixing
#[cfg(test)]and#[cfg(any(test, feature = "proptest-impl"))]code in the same module.Can you move these functions and their imports to a new
tests::setupmodule instead?
Done.
dconnolly
approved these changes
Jul 27, 2021
mpguerra
added a commit
that referenced
this pull request
Jul 29, 2021
* Draft CHANGELOG for Zebra 1.0.0-alpha.14 * Add PR #2533 to CHANGELOG * Apply suggestions from code review Co-authored-by: teor <[email protected]> * Remove entry about updating the changelog * move #2497 * add #2529 * Add a missing space * Add #2458, #2525, #2486, #2542 and #2539 to CHANGELOG Co-authored-by: teor <[email protected]> Co-authored-by: Deirdre Connolly <[email protected]>
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.
Motivation
The
zebra-statecrate was previously already using#[cfg(feature = "proptest-impl")]feature gates, but it didn't have theproptest-implfeature defined inCargo.toml. This became necessary as part of the work for #1334, because having azebra_state::init_testhelper function to be used for tests on other crates would simplify some code.Solution
The
proptest-implfeature was added, and it enables thezebra-testand theproptestdependencies with it.Unfortunately, just adding the feature breaks the build, because the code had partial usages of the feature already. Therefore, some refactoring was needed to fix the errors and also eliminate warnings.
Review
@teor2345 had already started looking at the #2519 draft PR for #1334.
Reviewer Checklist
Follow Up Work
A PR after this one will add the
zebra_state::init_testhelper function and refactor some other crates' test code to use it.This change is