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

Adding a quickcheck interoperability layer #18

Closed
Centril opened this issue Dec 16, 2017 · 7 comments
Closed

Adding a quickcheck interoperability layer #18

Centril opened this issue Dec 16, 2017 · 7 comments

Comments

@Centril
Copy link
Collaborator

Centril commented Dec 16, 2017

I've created an interoperability layer between proptest and quickcheck, should I add this to proptest as a PR under a feature flag quickcheck or do you believe this belongs in a separate crate?

The benefit of the latter is that proptest is not then constrained in dependencies by quickcheck while the benefit of the former is that the user doesn't have to write in another dependency to their project.

What it amounts to is essentially (details omitted):

use quickcheck::Arbitrary;

pub fn from_qc<A: Arbitrary + Debug>() -> QCStrategy<A> {
    QCStrategy::new(qc_gen_size())
}

pub fn from_qc_sized<A: Arbitrary + Debug>(size: usize) -> QCStrategy<A> {
    QCStrategy::new(size)
}
@AltSysrq
Copy link
Collaborator

I would prefer a separate crate. Besides the benefits you mentioned, it also makes it clearer (e.g., for someone looking at it on crates.io) that it's an add-on should one find the use for it rather than a significant-but-optional core feature.

@Centril
Copy link
Collaborator Author

Centril commented Dec 16, 2017

A new crate coming your way in a bit =)

@Centril
Copy link
Collaborator Author

Centril commented Dec 16, 2017

(Perhaps we should organize stuff into a GitHub organization soon to have repositories in the same place-ish?)

@Centril
Copy link
Collaborator Author

Centril commented Dec 16, 2017

The promised crate: https://github.com/Centril/proptest-quickcheck-interop .
Add a note about this to docs perhaps?

@AltSysrq
Copy link
Collaborator

The crate looks reasonable to me. Only one comment:

https://github.com/Centril/proptest-quickcheck-interop/blob/master/src/lib.rs#L332

Could that impl be moved under mod test or otherwise made #[cfg(test)]? Then it wouldn't escape into the public API.

Add a note about this to docs perhaps?

Certainly.

@Centril
Copy link
Collaborator Author

Centril commented Dec 17, 2017

Oh! Why didn't I think of that... ;)

Fixed in 1.04.

@AltSysrq
Copy link
Collaborator

The docs now link to this crate in the paragraph after the introduction.

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

2 participants