fast-check icon indicating copy to clipboard operation
fast-check copied to clipboard

Allow specifying max number of shrinks per test

Open AlexeyRaga opened this issue 2 years ago • 12 comments

🚀 Feature Request

Allow specifying max number of shrinks. The idea is not new, other property testing framework (QuickCheck,HedgeHog, FSCheck in Haskell and FSharp have it)

Motivation

Performance and friendliness with integration tests. Also related to https://github.com/dubzzz/fast-check/issues/4161.

Example

fc.assert(myProperty, { numRuns: 5, maxShrinks: 5 });

AlexeyRaga avatar Aug 26 '23 12:08 AlexeyRaga

Any example of such option in the other libraries? How it can be configured compared to another test...?

On it's own reducing shrinker can be done at various level: the arbitrary itself (what we do with noShrink) vs the runner (what you suggest). It can also be done at different levels: how far it goes in a given level? how many levels it can run into? As actually the shrinker runs on a 2D-plan so we may want to limit one direction and not the other or a combination of both of them.

Having more context would help as it seems that you know how they do it might reduce the exploratory time required just to put a status (do/don't do) on such ticket.


Side-note: You may use noShrink or endOnFailure.

Like:

fc.uuid().noShrink()

or:

fc.assert(myProperty, { numRuns: 5, endOnFailure: true })

dubzzz avatar Aug 26 '23 14:08 dubzzz

@dubzzz sure,

Hedgehog: https://hackage.haskell.org/package/hedgehog-1.4/docs/Hedgehog.html#v:withShrinks QuickCheck in Haskell: https://hackage.haskell.org/package/QuickCheck-2.14.3/docs/Test-QuickCheck.html#t:Args

Alternatives are not really the same because I do not really want to give up on shrinking...

AlexeyRaga avatar Aug 26 '23 15:08 AlexeyRaga

As we already have noShrink, I think I'll try to find an option that could cover both noShrink and maxShrinks at the same time. Probably re-using noShrink but passing it a number for v3 and move to another name for v4.

The remaining part to decide is: what to stop shrinking as shrinker lives in a 2D-plan so 2 directions can be stopped separately or together.

dubzzz avatar Sep 04 '23 23:09 dubzzz

Maybe the existing noShrink should be replaced by some kind of limitShrink({ maxDepth, maxValuesPerLevel }) or a simpler one limitShrink(maxValues) with maxValues somehow aggregating the two others.

With such option, noShrink could just be dropped in V4.

Another option would be to make a dedicated arbitrary to handle that.

dubzzz avatar Sep 17 '23 09:09 dubzzz

I re-checked this issue as I'm planning to solve it in the next major (on it). The solution I currently came up with is to handle it at Arbitrary level as I already do for the noShrink case.

The idea is the following:

  • the Value, more precisely it's attached context has to integrate the depth
  • based on that the shrink can decide

It has the huge benefits not to slow down nominal users, to be implementable on user land too... But I'll need to assess the case in which the shrink gets called without any context. It can be the case when nesting arbitraries to re-use their shrink capabilities.

The real huge benefit of the approach is that it fully fulfills my plugin-based target. As for now context is untyped it does bring any concerns related to mapping on an existing context to extend it with extra pieces of data.

dubzzz avatar Feb 02 '24 08:02 dubzzz

Very interesting 👍 Essentially, endOnFailure: true is like a global way of disable shrinking, right?

moodmosaic avatar May 25 '24 20:05 moodmosaic

endOnFailure is even more powerful than disabling shrinking actually. In classic usages of fast-check, it behaves fully the same with shrink being cut. But in usages such as reproducing an issue based on a known path it does not really stop shrinking, as it does allow it until you reach the target of the path and then forbid it.

endOnFailure: true can be seen as: stop immediately the execution if you fall on failure. An alternative less aggressive way to limit shrinking is probably interruptAfterTimeLimit. The later allows users to still benefit from shrinking capabilities while not spending too many time into it. Both options can be seen as global ones as they can be set for all tests using configureGlobal.

dubzzz avatar May 27 '24 06:05 dubzzz

I often use fast-check as a fuzzer where shrinking doesn't always make sense (or becomes interesting if combined with shrinkray). In this case, instead of using .noShrink() everywhere, endOnFailure: true is much more explicit and cleaner. Thank you 👍

moodmosaic avatar May 28 '24 13:05 moodmosaic

My plan for v4 is to move .noShrink as any other arbitrary. Users could still do it but rather than doing arb.noShrink() they will have to do fc.noShrink(arb). As such I also feel we should prefer endOnFailure or interruptAfterTimeLimit in most cases and rely on noShrink-like only when there is a real need to drop the shrinking capabilities of a precise arbitrary.

So my next step on this PR are:

  • [ ] Extract noShrink as a dedicated arbitrary
  • [ ] Create a limitedShrink arbitrary allowing users to only preserve part of the shrinks
  • [ ] Drop .noShrink() for v4

dubzzz avatar May 29 '24 06:05 dubzzz

Yes, fc.noShrink sounds reasonable. It's more explicit than .noShrink(). How would limitedShrink work?

moodmosaic avatar May 29 '24 07:05 moodmosaic

The detailed POC implementation is available at: https://github.com/dubzzz/fast-check/pull/5006. The idea would be to offer a way to limit shrinker either in depth or in width or both. It's basically the same as noShrink but with much more control on what we want to preserve. I feel it can solve solve complaints around some shrinkers taking too long in some cases and allow some users to truncate the shrinks whenever they consider it needed while still preserving part of the shrink working.

dubzzz avatar May 29 '24 07:05 dubzzz

Some points to consider:

  • Limiting shrink values can make shrinking faster, but might miss some important edge cases(?)
  • Should be flexible to handle different use cases and data types(?)
  • Adding depth and level parameters might make the API more complex. (Clear docs needed to explain how these work.)

Apart from that, it's for sure an interesting advanced feature to have.

moodmosaic avatar May 29 '24 07:05 moodmosaic

Released with 3.20.0

dubzzz avatar Jul 12 '24 15:07 dubzzz