BurntSushi / quickcheck

Automated property based testing for Rust (with shrinking).
The Unlicense
2.4k stars 149 forks source link

Upgrade rand to 0.8 and rand_core to 0.6 #264

Closed lopopolo closed 3 years ago

lopopolo commented 3 years ago

This commit upgrades the rand dependency from 0.7 to the semver incompatible 0.8. This commit upgrades the rand_core dependency from 0.5 to the semver incompatible 0.6.

This commit updates all calls to Gen::gen_range to use a Range literal instead of calling with two parameters as this API now accepts either or Range or a RangeInclusive:

https://docs.rs/rand/0.8.0/rand/distributions/uniform/trait.SampleRange.html

This was the only breaking change that impacted quickcheck and it was isolated to the arbitrary module.

lopopolo commented 3 years ago

rand's MSRV has changed. Per the crate docs:

Rust version requirements

Since version 0.8, Rand requires Rustc version 1.36 or greater. Rand 0.7 requires Rustc 1.32 or greater while versions 0.5 require Rustc 1.22 or greater, and 0.4 and 0.3 (since approx. June 2017) require Rustc version 1.15 or greater. Subsets of the Rand code may work with older Rust versions, but this is not supported.

Rust 1.36.0 brought the stabilization of the alloc crate. rand has extern crate alloc in its source.

It looks likequickcheck CI expects to support an MSRV of 1.34.0: https://github.com/BurntSushi/quickcheck/pull/264/checks?check_run_id=1580281547.

Is bumping quickcheck's MSRV something you would entertain @BurntSushi?

vorot93 commented 3 years ago

@BurntSushi can this be merged soon? Would be happy to help push this over the finish line.

PvdBerg1998 commented 3 years ago

Just hit this when upgrading my rand dependency.

BurntSushi commented 3 years ago

What problems are folks running into exactly? Worst case, you have multiple versions of rand in your dependency tree, no? Or is something preventing that?

lopopolo commented 3 years ago

@BurntSushi I tend to have #![warn(clippy::cargo)] in the lib.rs of the crates I publish on crates.io. This lint fails CI when multiple versions of a crate are in Cargo.lock. Running cargo-deny in CI would also fail when duplicate deps appear in the dep tree.

One of the crates I'd like to publish is rand_mt which uses an implementation of rand_core::RngCore as its primary public interface.

As it is a rand ecosystem RNG which uses quickcheck in some of its reseeding tests, multiple rand_core crates in this crate's tree is not acceptable for me. This PR being merged and published blocks publishing a mew major version of rand_mt based on the new rand_core.

BurntSushi commented 3 years ago

Multiple versions of rand/rand_core isn't something I consider to be a high priority problem. So this will have to wait until this gets done. That way it can be done in a single breaking change release instead of two.

lopopolo commented 3 years ago

Thanks for the context @BurntSushi. For rand_mt I've found a way to work around this by using getrandom directly to generate seeds and test input. I do find quickcheck to be more ergonomic.

jhpratt commented 3 years ago

@BurntSushi While there are no technical restrictions, I won't be upgrading the dependency for the time crate until this is done. Not a huge issue so far, as there's still work to do before the breaking release is out.