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

Chapter 6.5 - quickcheck 1.0 has changed trait Gen to struct Gen with no trait RngCore #34

Closed
Diamondlord opened this issue Jan 8, 2021 · 16 comments

Comments

@Diamondlord
Copy link

As result of the change the book code is not compiled.
I suggest adding specific versions to these commands in the book or update code ;)

cargo add [email protected]  --dev
cargo add quickcheck-macros0.9.1 --dev

Changes:
before

pub trait Gen: RngCore {
   fn size(&self) -> usize;
}

now -> link to the source code

/// Gen represents a PRNG.
///
/// It is the source of randomness from which QuickCheck will generate
/// values. An instance of `Gen` is passed to every invocation of
/// `Arbitrary::arbitrary`, which permits callers to use lower level RNG
/// routines to generate values.
///
/// It is unspecified whether this is a secure RNG or not. Therefore, callers
/// should assume it is insecure.
pub struct Gen {
    rng: rand::rngs::SmallRng,
    size: usize,
}
@LukeMathWalker
Copy link
Owner

Thanks for reporting the issue!
I'll patch the issue before the next release :)

@LukeMathWalker
Copy link
Owner

For the time being I have pinned quickcheck to 0.9.2.
I have opened a PR with a potential solution BurntSushi/quickcheck#271 - I'll update the book according to the outcome :)

@cedeerwe
Copy link

A related issue, after pasting the dependenecies in Cargo.toml (copied from the book)

quickcheck = "0.9.2"
quickcheck-macros = "0.9.2"

We get

> cargo test domain                     
Updating crates.io index
error: no matching package named `quickcheck-macros` found
location searched: registry `https://github.com/rust-lang/crates.io-index`
perhaps you meant: quickcheck_macros

After changing to quickcheck_macros in Cargo.toml we get

> cargo test domain        
Updating crates.io index
error: failed to select a version for the requirement `quickcheck_macros = "^0.9.2"`
candidate versions found which didn't match: 1.0.0, 0.9.1, 0.9.0, ...
location searched: crates.io index

Using the version 0.9.1 resolved the issue, but apparently pinning the version to 0.9.2 does not really work.

Putting all of these here, as it will be resolved by what you linked above.

@emiddleton
Copy link

@cedeerwe I could only get quickcheck to work with

quickcheck = "0.9"
quickcheck_macros = "0.9"

@LukeMathWalker
Copy link
Owner

LukeMathWalker commented Jan 25, 2021

The latest release adopts @cedeerwe's fix - it pins quickcheck_macros to 0.9.1 which should work without any issue.

@kartikynwa
Copy link

kartikynwa commented Apr 23, 2022

I ended up doing this until the compiler stopped yelling at me. Not elegant but it works for now:

    #[derive(Debug, Clone)]
    struct ValidEmailFixture(String);
    
    impl quickcheck::Arbitrary for ValidEmailFixture {
        fn arbitrary(g: &mut quickcheck::Gen) -> Self {
            let mut rand_slice: [u8; 32] = [0; 32];
            for i in 0..32 {
                rand_slice[i] = u8::arbitrary(g);
            }
            let mut seed = StdRng::from_seed(rand_slice);
            let email = SafeEmail().fake_with_rng(&mut seed);
            println!("{}", email);
            Self(email)
        }

Had to add rand and rand_core to the dev dependencies. Is there a better way to achieve this? Just asking out of curiosity.

@ChzenChzen
Copy link

I ended up doing this until the compiler stopped yelling at me. Not elegant but it works for now:

    #[derive(Debug, Clone)]
    struct ValidEmailFixture(String);
    
    impl quickcheck::Arbitrary for ValidEmailFixture {
        fn arbitrary(g: &mut quickcheck::Gen) -> Self {
            let mut rand_slice: [u8; 32] = [0; 32];
            for i in 0..32 {
                rand_slice[i] = u8::arbitrary(g);
            }
            let mut seed = StdRng::from_seed(rand_slice);
            let email = SafeEmail().fake_with_rng(&mut seed);
            println!("{}", email);
            Self(email)
        }

Had to add rand and rand_core to the dev dependencies. Is there a better way to achieve this? Just asking out of curiosity.

I think this is better:

            let mut rng = StdRng::seed_from_u64(u64::arbitrary(g));

@0xAlcibiades
Copy link

0xAlcibiades commented Jun 15, 2022

error[E0277]: the trait bound `G: rand_core::RngCore` is not satisfied
   --> src/domain/subscriber_email.rs:58:51
    |
58  |             let email = SafeEmail().fake_with_rng(g);
    |                                     ------------- ^ the trait `rand_core::RngCore` is not implemented for `G`
    |                                     |
    |                                     required by a bound introduced by this call
    |
    = note: required because of the requirements on the impl of `rand::rng::Rng` for `G`
note: required by a bound in `fake_with_rng`
   --> /Users/user/.cargo/registry/src/github.com-1ecc6299db9ec823/fake-2.5.0/src/lib.rs:192:28
    |
192 |     fn fake_with_rng<U, R: Rng + ?Sized>(&self, rng: &mut R) -> U
    |                            ^^^ required by this bound in `fake_with_rng`
help: consider further restricting this bound
    |
57  |         fn arbitrary<G: quickcheck::Gen + rand_core::RngCore>(g: &mut G) -> Self {
    |                                         ++++++++++++++++++++

For more information about this error, try `rustc --explain E0277`.
error: could not compile `zero2prod` due to previous error
warning: build failed, waiting for other jobs to finish...

Still seeing this with the latest examples from the book, and from this codebase @LukeMathWalker

Seems like pinning fake matters. A more robust solution would be preferable, minor version increments shouldn't be breaking stuff. Not sure if that's on fake or the implementation though.

@LukeMathWalker
Copy link
Owner

This indeed on fake, not on our implementation.
I'll see what can be done in a newer revision.

@JonShort
Copy link

Pinning still required as of December 2022, however the suggestion above (#34 (comment)) ends up being fairly clean in practice:

use rand::{rngs::StdRng, SeedableRng};

//[ ... ]

impl Arbitrary for ValidEmailFixture {
    fn arbitrary(g: &mut Gen) -> Self {
        let mut rng = StdRng::seed_from_u64(u64::arbitrary(g));
        let email = SafeEmail().fake_with_rng(&mut rng);
        Self(email)
    }
}

@dsaghliani
Copy link

dsaghliani commented Mar 15, 2023

#34 (comment) works, and if and until a new edition of the book is released, the code and dependencies have to stay the same, but for people like me, I'd appreciate a "sneak peek" in this thread regarding how this would be implemented if the book had been written with today's crate ecosystem in mind.

Would it still use fake? What about quickcheck? Would it go with proptest instead? If so, how would that work?

@bitcoiner-dev
Copy link

bitcoiner-dev commented Apr 14, 2023

The code in #34 works for me with the following:

fake = "2.3.0"
quickcheck = "0.9"
quickcheck_macros = "0.9.1"
rand = "0.8.5"
rand_core = "0.6.4"

@cleverjam
Copy link

cleverjam commented Apr 28, 2023

I am now getting the following error:

error[E0782]: trait objects must include the `dyn` keyword
  --> src/domain/subscriber_email.rs:36:30
   |
36 |         fn arbitrary(g: &mut Gen) -> Self {
   |                              ^^^
   |
help: add `dyn` keyword before this trait
   |
36 |         fn arbitrary(g: &mut dyn Gen) -> Self {
   |                              +++

When using the proposed solution above comment

Not sure if this is a quickcheck version issue - I have also copied those from comment

[dev-dependencies]
reqwest = { version = "0.11", features = ["json"] }
claims = "0.7.0"
quickcheck = "0.9.2"
quickcheck_macros = "0.9.1"
fake = "~2.3.0"
once_cell = "1.7.2"
rand = "0.8.5"

I will keep digging for a solution and update this thread if I find one, but still wanted to share as it seems there are other issues in this area

Update: I think I got my solutions mixed up, when downgrading the quickcheck dependency the approach in the book works just fine:

    impl quickcheck::Arbitrary for ValidEmailFixture {
        fn arbitrary<G: quickcheck::Gen>(g: &mut G) -> Self {
            let email = SafeEmail().fake_with_rng(g);
            Self(email)
        }
    }

@ryanseipp
Copy link

Pinning seems no longer necessary.

[dev-dependencies]
fake = "2.6.1"
rand = "0.8.5"
quickcheck = "1.0.3"
quickcheck_macros = "1.0.0"
    #[derive(Debug, Clone)]
    struct ValidEmailFixture(pub String);

    impl quickcheck::Arbitrary for ValidEmailFixture {
        fn arbitrary(g: &mut quickcheck::Gen) -> Self {
            let mut rng = StdRng::seed_from_u64(u64::arbitrary(g));
            let email = SafeEmail().fake_with_rng(&mut rng);

            Self(email)
        }
    }

    #[quickcheck_macros::quickcheck]
    fn valid_emails_are_parsed_successfully(valid_email: ValidEmailFixture) -> bool {
        SubscriberEmail::parse(valid_email.0).is_ok()
    }

@LukeMathWalker
Copy link
Owner

Sweet!
I'll set aside some time to update the book then.

trinhlehainamvice added a commit to trinhlehainamvice/zero2prod that referenced this issue Jul 27, 2023
- Reference: LukeMathWalker/zero-to-production#34
- Update `fake` to 2.6 and `quickcheck` to 1
- `fake` 2.6 depending on `rand` 0.8
@LukeMathWalker
Copy link
Owner

This has been fixed in the next book release. Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests