Property Tests: make_reward_set proptest and documentation #6640
+375
−3
Conversation
This file contains hidden or 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
* force duplication
|
|
|
|
||
| ## Reusing Strategies | ||
|
|
||
| Writing new input strategies may be the most tedious part of writing property tests, so it is worthwhile figuring out if the input you are looking for (or maybe a component of the input you're looking for) already has a strategy in the codebase. If you search for functions that return `impl Strategy<Value = ?>` in the codebase, you should find the set of functions that have already been written. |
Contributor
There was a problem hiding this comment.
Should we have a convention of defining these all in some common place, for discoverability?
Comment on lines
+215
to
+217
| 1. Executes once a PR has been approved. | ||
| 2. Discovers the set of new tests (this is probably easiest to achieve by running `cargo nextest list` on the source and target branches and then diffing the outputs). | ||
| 3. Executes only the new tests with the environment variable `PROPTEST_CASES` set to 2500. |
| } | ||
| ``` | ||
|
|
||
| This technique allows to be sure that proptest generates a lot of cases where there are multiple entries for the same reward address. Unfortunately, this kind of thing tends to be more art than science, which means that PR authors and reviewers will need to be careful about the input strategies for property tests (this should also be aided by the CI task for PRs). This is one of the reasons that property tests can't totally supplant unit tests. However, a lot of the work of property tests helps with writing unit tests: many unit tests can be essentially fixed inputs to the property test. |
Contributor
There was a problem hiding this comment.
I like the idea of using a common function which is called in the property test(s) and also called with some known edge cases in standard unit test style. Can we include that in the example here?
Co-authored-by: Brice <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This PR adds some docs for how we should handle property testing in this repo going forward, and it includes an example property test for the
make_reward_setfunction.