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

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

Open
AzimMuradov opened this issue May 2, 2024 · 1 comment
Open

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

AzimMuradov opened this issue May 2, 2024 · 1 comment
Labels
quality-of-life This issue proposes a change that will improve the UX of proptest but isn't necessarily a "feature"

Comments

@AzimMuradov
Copy link

AzimMuradov commented May 2, 2024

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 matthew-russo added the quality-of-life This issue proposes a change that will improve the UX of proptest but isn't necessarily a "feature" label Jun 8, 2024
@matthew-russo
Copy link
Member

matthew-russo commented Jun 8, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quality-of-life This issue proposes a change that will improve the UX of proptest but isn't necessarily a "feature"
Projects
None yet
Development

No branches or pull requests

2 participants