Consider replacing quickcheck with an alternative

[VictorKoenders]

Libp2p is still on rand 0.7 (sometimes 0.7.2, sometimes 0.7.3). In an effort to reduce the number of duplicate dependencies in our project, we'd like to bump this to rand 0.8.

It looks like this is stuck on the quickcheck bump from 0.9 to 1.0. This change removed rand as a public dependency, but has caused the .gen_range() method in quickcheck to become hidden.

There have been a couple of issues on the quickcheck repo requesting a new gen_range function: https://github.com/BurntSushi/quickcheck/issues/279, https://github.com/BurntSushi/quickcheck/issues/267

The crate author has suggested that at one point quickcheck will expose the seed of the rng, so a crate can use that to seed their own generator: https://github.com/BurntSushi/quickcheck/issues/279#issuecomment-800639203. This seems like a complicated way to implement Arbitrary for structs that contain ranges, but it is technically possible. This change has not been implemented at moment.

Someone suggested a workaround:

pub fn range(gen: &mut Gen, range: Range<u64>) -> u64 {
    u64::arbitrary(gen) % (range.end - range.start) + range.start
}

However there does not seem to be any progress on this, or any updates to quickcheck, for over a year now.

This issue is intended to start the conversation on either updating quickcheck or replacing it, so the rand dependency can be updated.

If we have a plan forward I'm willing to make a PR to implement it.

[mxinden]

However there does not seem to be any progress on this, or any updates to quickcheck, for over a year now.

Exactly right. That is the reason why rust-libp2p is still using an old version.

This issue is intended to start the conversation on either updating quickcheck or replacing it, so the rand dependency can be updated.

I am fine with either solution. I like that test input generation is specified at the type level, that is why I didn't use proptest thus far.

See also https://github.com/libp2p/rust-libp2p/pull/1929#issuecomment-770762317

If we have a plan forward I'm willing to make a PR to implement it.

Help here would be very much appreciated @VictorKoenders.

@VictorKoenders what is your preferred way forward?

[thomaseizinger]

I am fine with either solution. I like that test input generation is specified at the type level, that is why I didn't use proptest thus far.

Proptest on the other hand is nice because you can write the generation as free functions, without implementing a trait and thus without a dependency to proptest in the original crate and thus without orphan-rule issues.

[VictorKoenders]

@VictorKoenders what is your preferred way forward?

I have no strong preference either way. I can make a proptest PR if you want

[mxinden]

without a dependency to proptest in the original crate and thus without orphan-rule issues.

In this case, we control all crates and we would depend on proptest anyways, thus I don't see the benefit.

Someone suggested a workaround:

pub fn range(gen: &mut Gen, range: Range<u64>) -> u64 {
    u64::arbitrary(gen) % (range.end - range.start) + range.start
}

This would be my preference today.

[thomaseizinger]

without a dependency to proptest in the original crate and thus without orphan-rule issues.

In this case, we control all crates and we would depend on proptest anyways, thus I don't see the benefit.

The benefit is in other people's crates / projects who wish to write property-based tests.

Strategies, as proptest calls them, can easily be extracted into another crate and thus be shared long term (something like libp2p-proptest) without bloating the original crate with feature flags that are test-only.

I am happy to go with the workaround now. Perhaps we can discuss migration to proptest in a different issue as I still think that it is superior to quickcheck.

[mxinden]

Thanks @kpp and @thomaseizinger for fixing this.