8000 Tune maxShrinks for small checks count by darl · Pull Request #4023 · zio/zio · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Tune maxShrinks for small checks count #4023

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

Closed
wants to merge 1 commit into from
Closed

Conversation

darl
Copy link
Contributor
@darl darl commented Jul 30, 2020

Currently maxShrinks always equal to 1000, even for checkN(1).
With this change maxShrinks will correlate with checks count.
So with long failing test it will not timeout on shrinking phase.
Default values not changed: check(...) == checkN(200)(...)

Better control over maxShrinks covered in #4007

@CLAassistant
Copy link
CLAassistant commented Jul 30, 2020

CLA assistant check
All committers have signed the CLA.

@adamgfraser
Copy link
Contributor

I think It would definitely be good to have more ways to control the number of shrinks, but this feels a bit ad hoc. We have two concepts: (1) how much samples to test and (2) how many shrinks to do. And they are not necessarily related. We might want to test only a few samples but do a lot of shrinking, or test a lot of sample but do only a little shrinking.

I think the best way to solve this problem is to create a TestConfig service in the environment that would contain a bunch of different variables such as default number of samples for property based tests, number of shrinks, etc... Then we could provide aspects to easily modify these. I think that should be relatively straightforward and I can probably do it in a week or so after 1.0 is released.

If we need something in the shorter term there are a couple of other options we could pursue:

  • We could add an additional parameter to each of the check methods with a default value for maxShrinks. I think this would allow you to specify it if you wanted with no change to ergonomics for other users.
  • I think we could implement a combinator on Gen and Sample to limit the size of the shrink tree

Today you can also use Gen#noShink if shrinking is taking too long, though that is obviously not ideal since you don't get any shrinking versus being able to control the amount of shrinking.

I think my preference would be to just do what we think the "right" thing is since we have a clear line of sight on it and I think can implement it relatively quickly.

@jdegoes
Copy link
Member
jdegoes commented Aug 13, 2020

I agree we have to have more control here and the idea suggested by @adamgfraser will pave the way for future tweaks.

@darl Any chance you have the bandwidth to push this through? Feel free to re-open if you are able!

@jdegoes jdegoes closed this Aug 13, 2020
@adamgfraser
Copy link
Contributor

I've been working on this. Will have something today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0