search

proptest-rs / proptest

Hypothesis-like property testing for Rust
Apache License 2.0
1.63k stars 152 forks source link

Update "shrinks to" comment on the same seed failures #448

Open AzimMuradov opened 2 months ago

AzimMuradov commented 2 months ago

As far as I understand, failure persistence file lists all the once failed seeds and their minimized test cases (in the comments).

But if the tested code was updated, and it still fails on a saved seed, the minimized test case comment remains the same, even if it is actually outdated.

I'm not sure if it's a bug or an intended behaviour, but I think the associated comments should change as soon as minimized test cases change.

Example:

Bug on a = 20 (seed1) -> shrinks to 2

cc seed1 # shrinks to a = 2

Code changed, bug on a = 20 (seed1) -> shrinks to 5

Current behaviour:

cc seed1 # shrinks to a = 2

Desired behaviour:

cc seed1 # shrinks to a = 5
matthew-russo commented 3 weeks ago

Thanks for the report and sorry for the delay. I'd classify it somewhere in between a bug and intended behavior. I don't think its intended but I wouldn't quite call it a bug since its not adversely affecting anything per se, just not providing the benefit it intended.

Having an updated comment is relatively straightforward but I think that actually hides the underlying problem even more so I don't think we'd want to go that route. if you have a situation of

Bug on a = 20 (seed1) -> shrinks to 2

cc seed1 # shrinks to a = 2

the problem is that later on when you update the strategy, seed1 no longer shrinks to a = 2 and so you're not actually testing the regression you intended to test. you may be testing a = 5 but that didn't cause a bug to begin with so its not relevant to continually test it. updating the comment would hide the fact that you wanted to test the case where a = 2.

having the system be smart enough to figure that out though, properly persist some info that encodes the intent, rather than the seed, and then pick up from that persisted intent doesn't have an obvious solution. there have been some other reports of dissatisfaction with the current persistence strategy (#439 & #440) and also some discussions of things we could improve if we took the liberty of a 2.x release that allowed backwards incompatible changes.

one idea that i've been toying with but haven't taken the time to really explore is to emit source code for a regular rust unit test of the shrunken case. i'm not sure how feasible it would actually be to emit valid rust code, but I think it would be a more faithful approach to the intent -- we want to give people a specific input that failed their conditions and what more explicit way than an unambiguous unit test.