Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot use Rng methods on Gen when implementing Arbitrary #279

Open
nbraud opened this issue Mar 16, 2021 · 5 comments
Open

Cannot use Rng methods on Gen when implementing Arbitrary #279

nbraud opened this issue Mar 16, 2021 · 5 comments

Comments

@nbraud
Copy link

nbraud commented Mar 16, 2021

I noticed when trying to update a project to quickcheck 1.x, that Gen is now an opaque struct and users implementing Arbitrary can only use Gen::choose to get randomness. This isn't sufficient in some usecases, which were previously served by using Gen's Rng methods: for instance, in uutils' factor I need to draw integers from large ranges known at runtime, to generate integers of known factorization:

impl quickcheck::Arbitrary for Factors {
    fn arbitrary(gen: &mut quickcheck::Gen) -> Self {
        use rand::Rng;
        let mut f = Factors::one();
        let mut g = 1u64;
        let mut n = u64::MAX;

        // Adam Kalai's algorithm for generating uniformly-distributed
        // integers and their factorization.
        //
        // See Generating Random Factored Numbers, Easily, J. Cryptology (2003)
        'attempt: loop {
            while n > 1 {
                n = gen.gen_range(1, n);
                if miller_rabin::is_prime(n) {
                    if let Some(h) = g.checked_mul(n) {
                        f.push(n);
                        g = h;
                    } else {
                        // We are overflowing u64, retry
                        continue 'attempt;
                    }
                }
            }

            return f;
        }
    }
}

(Technically, I could repeatedly use gen.choose(&[0, 1]) to draw individual, random bits, but this would be beyond silly)

@nbraud
Copy link
Author

nbraud commented Mar 16, 2021

Seems related to #267

@BurntSushi
Copy link
Owner

Yes. The plan is to make a seed available, and then you should be able to materialize any RNG from that.

@audunska
Copy link

audunska commented Mar 25, 2021

This really hurts the ability to write good Arbitrary impls for complicated data. I'm sure there are lots of examples, but my use case can be reduced to just a vector of vectors, Vec<Vec<u32>>. To make sure I sample the data properly I want to generate both tall skinny arrays and short wide ones. This means that the size parameter must be divided randomly into two factors in the arbitrary method, which requires a bounded random float.

If the issue with gen_range is that it exposes rand types, could I propose simply adding a public method Gen::gen_uniform(&mut self) -> f32 which generates a uniformly distributed float in the unit interval? That should satisfy most needs.

@divagant-martian
Copy link

Is there a known plan to make this work that would be followed by a first time contributor?

@bergmark
Copy link

Perhaps a bit hacky, but the workaround I ended up using was:

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

dead-claudia added a commit to dead-claudia/journald-exporter that referenced this issue Jul 6, 2024
...and switch the 32-bit integer parser to just exhaustive checking.
(More on that later.)

Why move away from QuickCheck?

1. The maintainer appears to have little interest in actually
   maintaining it. BurntSushi/quickcheck#315

2. Its API is incredibly inefficient, especially on failure, and it's
   far too rigid for my needs. For one, I need something looser than
   `Arbitrary: Clone` so things like `std::io::Error` can be generated
   more easily. Also, with larger structures, efficiency will directly
   correlate to faster test runs. Also, I've run into the limitations
   of not being able to access the underlying random number generator
   far too many times to count, as I frequently need to generate random
   values within ranges, among other things.
   - BurntSushi/quickcheck#279
   - BurntSushi/quickcheck#312
   - BurntSushi/quickcheck#320
   - BurntSushi/quickcheck#267

3. It correctly limits generated `Vec` and `String` length, but it
   doesn't similarly enforce limits on test length.

4. There's numerous open issues in it that I've addressed, in some
   cases by better core design. To name a few particularly bad ones:
   - Misuse of runtime bounds in `Duration` generation, `SystemTime`
     generation able to panic for unrelated reasons:
     BurntSushi/quickcheck#321
   - Incorrect generation of `SystemTime`:
     BurntSushi/quickcheck#321
   - Unbounded float shrinkers:
     BurntSushi/quickcheck#295
   - Avoiding pointless debug string building:
     BurntSushi/quickcheck#303
   - Signed shrinker shrinks to the most negative value, leading to
     occasional internal panics:
     BurntSushi/quickcheck#301

There's still some room for improvement, like switching away from a
recursive loop: BurntSushi/quickcheck#285.
But, this is good enough for my use cases right now. And this code
base is structured such that such a change is *much* easier to do.
(It's also considerably simpler.)

As for the integer parser change, I found a way to re-structure it so
I could perform true exhaustive testing on it. Every code path has
every combination of inputs tested, except for memory space as a whole.
This gives me enough confidence that I can ditch the randomized
property checking for it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants