-
Notifications
You must be signed in to change notification settings - Fork 44
Shrinker improvements to QCheck list, string, and function #242
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
Shrinker improvements to QCheck list, string, and function #242
Conversation
|
Just a remark, but if this modifies shrinking, I don't see it having any impact
on a successful test suite where 0 shrinking happens, correcT?
|
aa1ce64 to
f8bbd31
Compare
Good point 👍 |
|
@gasche - I know you've been hand-writing shrinkers in the past. |
f8bbd31 to
3ba1ce8
Compare
|
I just rebased and added a CHANGELOG entry. |
|
Apologies for the lack of reply. I tried to look at the shrinker I wrote last, it works on the AST of a small programming language, and there the current shrinkers are efficient enough that I couldn't measure a difference (I tried to inject bugs, but all examples get minimized in 10 steps or less; maybe the bugs were not subtle enough). The shrinker does not use |
|
Looks great! The shrinks steps seems to be reduced in the vast majority of cases. |
Co-authored-by: Valentin Chaboche <[email protected]>
|
OK, following @vch9 pointing out a shrink count that went up, I dug, found out it was caused by a linear time |
|
OK, implemented string shrinker conversions which makes the shrinker benchmark run consistently in under 2 secs on my machine. |
|
OK, I added a hand-rolled |
|
OK, thanks for the review @vch9 🙏
Overall, with the inclusion of the Green light to merge @vch9? |
Absolutely! This is a nice improvement :) The char shrinker makes sense! |
This PR contains the QCheck(1) shrinker improvements to
list,string, and functions described in #235.The essential part extracted from #235 (comment):
The reduced shrink counts in, e.g., test/core/QCheck_expect_test.expected are nice, but beware that this only counts successful shrink attempts. #235 (comment) documents how the PR reduces the total shrink attempts significantly, e.g.,
string never has a \255 charis reduced from 9365 attempts to 38 (on seed 6789)lists shorter than 432is reduced from 5118102 attempts to 19461 (on seed 1234)fold_left fold_right uncurriedis reduced from 80630 attempts to 199 (on seed 1234)I would be grateful if someone could give this a quick spin with an
opam pinof https://github.com/jmid/qcheck/tree/shrinker-improvements-list-string-fun on their regular QCheck tests before merging.