nick8325 / quickcheck

Automatic testing of Haskell programs.
Other
713 stars 119 forks source link

quickCheckWith does zero tests if maxSuccess is too large #361

Closed loafey closed 5 months ago

loafey commented 9 months ago

When using quickCheckWith and setting a high maxSuccess value, QuickCheck seems to give up, and not do any tests.

For example:

quickCheckWith stdArgs { maxSuccess = maxBound :: Int } $ \xs -> xs === reverse (reverse xs)

instantly yields

*** Gave up! Passed only 0 tests tests.

Using a simple test program I was able to figure out that the biggest value maxSuccess can be is 922337203685477580 as 922337203685477581 and above yields the erroneous result.

As a user would most likely never need 922337203685477580+ tests I would assume that this is pretty low severity, but worth reporting in any case.

Rewbert commented 9 months ago

Everything seems to be set up correctly, but when we enter the main test loop the checks fail. This is the code in question

test :: State -> Property -> IO Result
test st f
  | numSuccessTests st   >= maxSuccessTests st && isNothing (coverageConfidence st) =
    doneTesting st f
  | numDiscardedTests st >= maxDiscardedRatio st * max (numSuccessTests st) (maxSuccessTests st) =
    giveUp st f
  | otherwise =
    runATest st f

Before we proceed to run a test, we make sure that we've not already executed enough, and that we've not discarded too many.

The value you supply for maxSuccessends up in maxSuccessTests. max (numSuccessTests st) (maxSuccessTests st) is reduced

max (numSuccessTests st) (maxSuccessTests st)
==
max 0 (maxSuccessTests st)
==
max 0 maxBound
==
maxBound

When this is multiplied with maxDiscardedRatio st, which is a magic 10, the number overflows and becomes -10. This is smaller than numDiscardedTests st (which is 0), and the branch is selected, indicating that we've discarded too many tests.

A quick and greasy fix is to just use arbitrary precision Integers for the check instead?

test :: State -> Property -> IO Result
test st f
  | numSuccessTests st   >= maxSuccessTests st && isNothing (coverageConfidence st) =
    doneTesting st f
  | (toInteger $ numDiscardedTests st) >= (toInteger $ maxDiscardedRatio st) * (toInteger $ max (numSuccessTests st) (maxSuccessTests st)) =
    giveUp st f
  | otherwise =
    runATest st f

I have a parallel version of the testing loop under development, and this 'feature' seems to not manifest in my fork.

MaximilianAlgehed commented 5 months ago
div (maxDiscardedRatio st) (max 1 $ maxDiscardedRatio st) >= max (numSuccessTests st) (maxSuccessTests st)

ought to do the trick.