Make `discardRatio == 0` behave as advertised; have a way to cut off after too many discards
Closed this issue · 8 comments
Just from other frameworks...
Quicktheories has an "assuming(Predicate)" clause which comes with this description:
"As difficult to find values probably represent a coding error, QuickTheories will throw an error if less than 10% of the generated values pass the assumptions."
I thought one of the other frameworks had a global maxDiscards setting but I couldn't find it just now - google just showed a Haskell library with that setting. I'll post another comment if I find a further reference.
@paulk-asert I looked at what junit-quickcheck does with a discardRatio
of zero on @When
. It appears that I did add a safeguard against never-satisfied constraint expressions -- junit-quickcheck bails out if the number of discards exceeds the desired sample size in such a case. With discardRatio
> 0, junit-quickcheck will bail out if (discards / successes) >= discardRatio
.
I wonder if changing the docs to reflect these realities, and changing the message of the exception raised if the too-many-discards condition occurs with discardRatio
of zero, would suffice to resolve this issue? Let me know what you think.
The main thing is to have the documentation match the implementation. Having said that, I am not a big fan of the existing behavior. The number of discards exceeding the sample size seems a very aggressive default. For the example I was doing:
@Property(trials = 200)
void shouldHold(
@When(satisfies='#_.length() < 20', discardRatio = 30000) String s1,
@When(satisfies='#_.length() < 20', discardRatio = 30000) String s2,
@When(satisfies='#_.length() < 20', discardRatio = 30000) String s3) {
}
it never completes - failing after some 5 million attempts while almost but not quite reaching the 200 trials target. Having it bomb out after only 200 attempts (if I left discardRatio as the default) seems very harsh.
I know there are better ways to do the above but I would prefer a more friendly default - QuickTheories is 10 times friendlier for instance.
@paulk-asert Thanks for the feedback. I'm not sure how often @When
is used in practice -- if we changed the behavior of discardRatio == 0
to be closer to something like what QuickTheories does, it may be that no one would notice (except, certainly, for you and I 8^)). So, I'm happy to change the behavior.
Would having a default discardRatio
of 10 suit? Perhaps, then, disallowing 0 as a valid value?
That sounds reasonable. And later, if maxDiscards
was ever added or the junit-quickcheck runner was made compatible with something like the junit timeout Rule (it didn't seem to be at the moment) the default value could remain the same but the value of 0 could be reinstated with the currently 'documented' behaviour.
@paulk-asert Sounds great. I can get this in reasonably soon for further evaluation. Thanks!
@paulk-asert re: TimeoutRule
-- junit-quickcheck should run @Test(timeout = ___)
methods, but you're right, the runner doesn't honor any sort of timeout configuration on a @Property
method. It does respect @Rule
s...however, if we bog down during attempting to generate values to feed the property method, the timer will not have started yet (because we haven't begun the test).
This is in latest 0.8-alpha snapshot at oss.sonatype.org. Thanks for the feedback!