BurntSushi / quickcheck

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

Stack overflows if shrinking goes too deep (usually an ill-behaved shrink) #133

Closed critiqjo closed 4 years ago

critiqjo commented 8 years ago

If this line was instead:

let mut ok = false;

then all tests will fail, and stack overflows. Is this a known/expected behavior?

I will update this issue with a simpler example soon...

BurntSushi commented 8 years ago

I don't know what ok = false actually means, so I'm not sure.

I will note that if your shrinker isn't "well behaved," then it looks like shrink_failure could certainly end up in a stack overflow. "Well behaved" in this instance I guess means that the shrinker either never terminates or emits too many values.

critiqjo commented 8 years ago

I will note that if your shrinker isn't "well behaved," then it looks like shrink_failure could certainly end up in a stack overflow.

Thanks for pointing that out. I figured the reason -- one of the shrunk values will generate the same value in the next shrink phase. So that recursion will never end. An easy fix!

So, basically, the shrinking should be done in a way that shrink(shrink(...)) should terminate at some shallow enough depth for all possible shrunk values. But wouldn't it be better to keep a separate stack instead of using recursion; and raise a panic when that stack exceeds a certain size (say 100)? The panic could provide a message such as "shrinking not well-behaved" and also print a few values in stack (which would have all been equal in my case).

BurntSushi commented 8 years ago

@critiqjo Yes, the code could be improved to make the failure mode more obvious.

critiqjo commented 8 years ago

Then, let's leave this issue open for a better implementation. I'll try my hand within a week.

BurntSushi commented 4 years ago

It sounds like this may be trickier to fix than I thought. I'm going to close this for now since I'm not sure it's worth tracking, but happy to re-open if someone has fresh ideas!