remove rand as a public dependency + rollup and other cleanups #265
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
The Send bound is a relic from the past. Indeed, the docs for the Arbitrary trait have been outdated for quite some time. quickcheck stopped running each test in a separate thread once `std::panic::catch_unwind` was stabilized many moons ago. With `catch_unwind`, the `Send` bound is no longer necessary. We do need to retain the `'static` bound though. Without that, implementing shrink seems implausible. Fixes #262, Closes #263
Closes #258
This commit tweaks the Arbitrary impls of number types (integers, floats) to use the full range with a small bias toward "problem" values. This is a change from prior behavior that would use the `size` parameter to control the range of integers. In retrospect, using the `size` parameter this way was probably misguided. Instead, it should only be used to control the sizes of data structures instead of also constraining numeric ranges. By constraining numeric ranges, we leave out a huge space of values that are never tested. Fixes #27, Fixes #119, Fixes #190, Fixes #233, Closes #240
This removes the use of the rand_core crate as a public dependency. It
is now an implementation detail.
We achieve this primarily by turning the `Gen` trait into a concrete
type and fixing the fallout.
This does make it impossible for callers to use their own `Gen`
implementations, but it's unclear how often this was being used (if at
all). This does also limit the number of RNG utility routines that
callers have easy access to. However, it should be possible to use
rand's `SeedableRng::from_{rng,seed}` routines to get access to more
general RNGs.
Closes #241
This upgrades to the latest version of rand. Closes #264
The next release will be a breaking change release anyway. We update a few other things as well. The examples in particular.
mxinden
added a commit
to paritytech/unsigned-varint
that referenced
this pull request
Jan 11, 2021
With `quickcheck` `v1` `rand` is no longer part of `quickcheck`s public API interface. More concretely `Gen` is now a `struct` with a minimal API. `Arbitrary` implementations should (/must) delegate to `Arbitrary` implementations of primitives instead of making use the random number generator directly. See BurntSushi/quickcheck#265 for details.
mxinden
added a commit
to paritytech/unsigned-varint
that referenced
this pull request
Jan 11, 2021
With `quickcheck` `v1` `rand` is no longer part of `quickcheck`s public API interface. More concretely `Gen` is now a `struct` with a minimal API. `Arbitrary` implementations should (/must) delegate to `Arbitrary` implementations of primitives instead of making use the random number generator directly. See BurntSushi/quickcheck#265 for details. Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Max Inden <[email protected]>
Open
|
Coming here with an eye toward
(specifically the "public dependency" part) |
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.
Part of this PR is meant to address #241, which was an unhappy reaction to how
randwas evolving at the time. Since then,randhas slowed down its development and slimmed its dependency tree. It's still not quite as small as what quickcheck needs (which is really just a non-crypto PRNG), but it's closer.One idea I and @matklad had was to just remove
randfrom the public API of QuickCheck. Doing this has a cost: it removes the ability to provide your own RNG to quickcheck and instead moves the use of the RNG to an implementation detail. My feeling is that providing one's own RNG to quickcheck was seldom-if-ever used, so I judged this to be an acceptable cost. In return, we are no longer pinned torand's release cycle and we now have the freedom to switch out the RNG in the future if we want. For now, we upgrade torand 0.8and otherwise leave it alone for now.In addition to losing the ability to provide your own RNG, we also lose a lot of the lower level RNG convenience routines provided by
rand. I believe we can capture much of that by just exposing concrete routines onquickcheck::Gen, in addition to the variousArbitraryimpls.The reasons why the
Gentrait existed in the first place were:Anywho, I've written all this out so that it's clear what has changed and why. In particular, if you find yourself at this PR because some functionality in quickcheck has been removed, then please open a new issue that includes the problem you're trying to solve and we'll try to figure out how to move forward.