typelevel / scalacheck

Property-based testing for Scala
http://www.scalacheck.org
BSD 3-Clause "New" or "Revised" License
1.94k stars 407 forks source link

retryUntil fails to produce anything when used to check list size inside a Prop #413

Open nigredo-tori opened 6 years ago

nigredo-tori commented 6 years ago

I'm not sure what to make of it:

scala> Prop.forAll(
  arbitrary[List[Int]].retryUntil(_.nonEmpty)
)(_ => true).check
! Exception raised on property evaluation.
> Exception: org.scalacheck.Gen$RetryUntilException: retryUntil failed after 10001 attempts
org.scalacheck.Gen.loop$2(Gen.scala:131)
org.scalacheck.Gen.$anonfun$retryUntil$1(Gen.scala:135)
org.scalacheck.Gen$Parameters.useInitialSeed(Gen.scala:294)
org.scalacheck.Gen$$anon$3.doApply(Gen.scala:254)
org.scalacheck.Prop$.$anonfun$forAllShrink$1(Prop.scala:757)

From reading the scaladoc, it seems that .retryUntil should basically be the same as .suchThat for the purpose of using it in a Prop, except that it uses some internal filtering to circumvent the maxDiscardRatio. Am I wrong in thinking that?

nigredo-tori commented 6 years ago

The problem seems to arise because of the shrinking. When the maximum size is set to zero, this generator won't be able to output anything, and will eventually reach its threshold. If we limit the minimum size, the check passes:

scala> Prop.forAll(
  arbitrary[List[Int]].retryUntil(_.nonEmpty)
)(_ => true).check(Test.Parameters.default.withMinSize(1))
+ OK, passed 100 tests.

So this can be explained and circumvented. But isn't there a way to make generators like this one work with shrinking?

ashawley commented 6 years ago

Yes, retryUntil is meant to specify a condition on generated values like suchThat, but with less restriction. It's risky and should be used with caution, as Rickard pointed out in the original commit message in 6211037.

Yes, if you try to override the minimum size parameter to one in your Gen of non-empty lists, then retryUntil will not have to discard empty list values that are generated. See the documentation for Gen.Parameters.size for that relationship:

The size of the generated value. Generator implementations are allowed to freely interpret (or ignore) this value. During test execution, the value of this parameter is controlled by Test.Parameters.minSize and Test.Parameters.maxSize.

To avoid discarding empty lists, setting the size to a minimum of 1 is how Gen.nonEmptyListOf is implemented:

Gen.nonEmptyListOf(Arbitrary.arbitrary[Int])

Unfortunately, as you found out generator parameters are not respected by ScalaCheck's shrinking facility, see #129.