silentbicycle / theft

property-based testing for C: generate input to find obscure bugs, then reduce to minimal failing input
ISC License
611 stars 31 forks source link

Add per-type-info flag to copy argument (to allow mutation) #9

Closed silentbicycle closed 7 years ago

silentbicycle commented 7 years ago

Mutating the generated instance during the test can cause strange behavior during shrinking, because the already-modified instance will be passed to shrinking code as the original. This is an oversight in the design -- there should be an option to run the test on a copy, so the original can be retained for shrinking. (This could be the default, depending on how much overhead it adds.)

If this isn't possible to implement in the general case, then the instance passed to the test should be const, but only if other options are exhausted.

(Both @DRMacIver and @sw17ch reported this.)

sw17ch commented 7 years ago

Another option might be to generate two copies using the same seed or generating a new instance on demand with the original seed.

This assumes all instances are deterministically defined by their seed.

On Jun 21, 2017 12:08 AM, "Scott Vokes" notifications@github.com wrote:

Mutating the generated instance during the test can cause strange behavior during shrinking, because the already-modified instance will be passed to shrinking code as a the original. This is an oversight in the design -- there should be an option to run the test on a copy, so the original can be retained for shrinking. (This could be the default, depending on how much overhead it adds.)

If this isn't possible to implement in the general case, then the instance passed to the test should be const, but only if other options are exhausted.

(Both @DRMacIver https://github.com/drmaciver and @sw17ch https://github.com/sw17ch reported this.)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/silentbicycle/theft/issues/9, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF12csbXcIJVo-jUULRT00QOnFtY4xbks5sGLNvgaJpZM4OAfzu .

DRMacIver commented 7 years ago

Another option might be to generate two copies using the same seed or generating a new instance on demand with the original seed.

FWIW this is more or less how it works in Hypothesis - everything operates on a uniform representation, so we never touch the generated objects at all and it doesn't matter if they've been mutated.

I don't think it's feasible in theft though, because theft supports arbitrary user defined shrinks, so an instance doesn't just come from a seed it comes from a seed + a series of shrinks. Though I guess you could work around that by remembering the instance it came from and the shrink number in that case.

silentbicycle commented 7 years ago

It could also be resolved by forking and using copy-on-write, though there are some subtleties about which process calls various hooks.

I already have optional forking working on a local branch, for shrinking assertions, crashes, and timeouts. There's a lot less overhead than I expected, except on OSX, where each crash adds tens of milliseconds of latency (apparently due to slow crash reporting infrastructure -- I remember afl-fuzz docs observing the same thing).

silentbicycle commented 7 years ago

I think forking addresses this -- modifying the copy-on-write instance in the child process is fine, and users shouldn't expect changes to the instance itself to persist between trials.

DRMacIver commented 7 years ago

I think forking addresses this -- modifying the copy-on-write instance in the child process is fine, and users shouldn't expect changes to the instance itself to persist between trials.

Which tangentially reminds me of something I'd been wondering about: Doesn't autoshrinking currently rely on mutating the generated values (because of the lazy fill)? How does that interact with forking? I had a brief peek at the code but didn't see anything obvious and haven't tried experimenting to satisfy my curiousity yet.

silentbicycle commented 7 years ago

The mutating happens on the main process, only running the fork_post hook and actual property test happens in the child process. Also, each mutation pass makes a copy.

The current autoshrinking setup is complicated by, essentially, being bolted on to the regular setup in a not-quite-type-safe way. (See #18; the theft_autoshrink_wrap stuff should be done differently.) I'm working on fixing that right now. :/

DRMacIver commented 7 years ago

The mutating happens on the main process, only running the fork_post hook and actual property test happens in the child process. Also, each mutation pass makes a copy.

Oh, right! I was confused because I forgot that your/normal people's data generation is more distinct from your test case execution than in Hypothesis so that's an option. This is my major obstacle to getting good process isolation in Hypothesis, so I was hoping that you had some clever trick that I'd missed. Alas.