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

Support arbitrary::Arbitrary #166

Merged
merged 3 commits into from
Nov 2, 2020
Merged

Support arbitrary::Arbitrary #166

merged 3 commits into from
Nov 2, 2020

Conversation

e00E
Copy link
Contributor

@e00E e00E commented Sep 11, 2020

num-bigint already supports quickcheck::Arbitrary which works roughly
like arbitrary::Arbitrary. arbitrary::Arbitrary used by cargo-fuzz and
makes it easier to use num-bigint BigInt and BigUint in projects that
want to fuzz that way.

https://github.com/rust-fuzz/arbitrary
https://github.com/rust-fuzz/cargo-fuzz

I have included an example fuzz target but I'm not sure whether we want to merge it.

cargo +nightly install cargo-fuzz
cargo +nightly fuzz run fuzz_target_1

num-bigint already supports quickcheck::Arbitrary which works roughly
like arbitrary::Arbitrary. arbitrary::Arbitrary used by cargo-fuzz and
makes it easier to use num-bigint BigInt and BigUint in projects that
want to fuzz that way.
@cuviper
Copy link
Member

cuviper commented Sep 16, 2020

It's unfortunate that there are similar but separate Arbitrary traits, but I guess they have different models for generating data.

I'm OK with this, but can you add something to ci/test_full.sh to at least build with that feature?

@e00E e00E force-pushed the arbitrary branch 2 times, most recently from 293056c to 52dbe6c Compare October 9, 2020 10:32
@e00E
Copy link
Contributor Author

e00E commented Oct 9, 2020

I've added building the arbitrary feature to CI. I found the minimum rust version for arbitrary through testing with the Github CI.

Before merging we should still delete the fuzz folder. This was meant for reviewers but I don't think it makes sense in the repository.

@cuviper
Copy link
Member

cuviper commented Oct 30, 2020

Before merging we should still delete the fuzz folder. This was meant for reviewers but I don't think it makes sense in the repository.

Ok, please do, and then I think we're good to go.

@cuviper
Copy link
Member

cuviper commented Nov 2, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 2, 2020

@bors bors bot merged commit a2ad16d into rust-num:master Nov 2, 2020
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.

2 participants