BurntSushi / quickcheck

Automated property based testing for Rust (with shrinking).
The Unlicense
2.4k stars 149 forks source link

Remove Send bound for Arbitrary? #262

Closed benwr closed 3 years ago

benwr commented 3 years ago

In my current project, I've got a complicated data structure I'd like to make an Arbitrary instance for; unfortunately the implementation relies on Rc for performance reasons, so it isn't Send.

I've considered using an ad-hoc structure, like a Vec or something, and generating the data structure in the test from that, but interpreting results of this test would be really annoying.

My other option at the moment is generating two versions of the data structure, one using Rc and the other using Arc. But this is also annoying to do nicely in Rust, and everything that depends on this data structure will need to do the "cfg(test)" dance to choose whether to use the thread-safe version or the other one.

So my question is: Instead of Sending generated values to threads, could quickcheck send a random seed, and then have the thread itself generate the value? I imagine this has significant tradeoffs, but it would make it much easier to test libraries that otherwise don't want to be thread-safe.

Edited to add: I might be willing to spend a significant amount of time working on this if it turns out to be compatible with your goals for the project.

BurntSushi commented 3 years ago

@benwr So it turns out that the Send bound is not necessary at all. I should have removed it long ago, but didn't because I overlooked it. QuickCheck stopped running tests in separate threads when std::panic::catch_unwind became available. It only used separate threads at the time because it was the only way to recover from a panic.

I opened https://github.com/BurntSushi/quickcheck/pull/263 that removes the Send bound. Quite an easy change! The only thing preventing me from merging it at the moment is that it's a breaking change. I'd like to incorporate a few other breaking changes before the next semver release, but I'm not sure when I'll get to that unfortunately...

benwr commented 3 years ago

Ah, that's awesome! Thanks for quick response! Excited to see it land!