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

Adds quickcheck::Arbitrary impls for BigInt, BigUint, Complex, and Ratio. #293

Closed
wants to merge 7 commits into from

Conversation

remexre
Copy link

@remexre remexre commented May 18, 2017

Resolves #241. Gated by the quickcheck feature.

Would it be okay to do a release after this is merged?

@cuviper
Copy link
Member

cuviper commented May 24, 2017

Sorry I haven't reviewed this yet, but I just want you to know I'm not ignoring it. I need to familiarize myself with how quickcheck actually works. :)

One request up front: can you add some tests that exercise the new code? First just some basic tests to make sure the new trait implementations actually work at all. Then I imagine it should also be possible to write some useful quickcheck tests to validate num itself.

@cuviper cuviper self-assigned this May 24, 2017
@remexre
Copy link
Author

remexre commented May 24, 2017

Yep, I can do that. I'm actually working on landing #[derive(Arbitrary)] in the quickcheck crate (BurntSushi/quickcheck#98, BurntSushi/quickcheck#175); I'll probably might make this use that instead.

I can definitely add the tests, although my schedule's a bit busy until the weekend.

@cuviper
Copy link
Member

cuviper commented Dec 19, 2017

Did you ever get anywhere with this? If you'd like to continue, please move them to the separate repos.

The new separate issues are:

@cuviper cuviper closed this Dec 19, 2017
@remexre
Copy link
Author

remexre commented Dec 20, 2017

I haven't gotten around to getting BurntSushi/quickcheck#175 merged yet; I'll get around to it at some point...

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

Successfully merging this pull request may close these issues.

None yet

2 participants