4

After upgrading to scalacheck 1.13.3, I'm running into an odd issue where deriving instances of A => B Or C, where Or is essentially a light Either, will almost always fail.

This is the simplest code I could write to reproduce the issue:

import org.scalatest.FunSuite
import org.scalatest.prop.GeneratorDrivenPropertyChecks
import org.scalacheck.Shapeless._

class Testor extends FunSuite with GeneratorDrivenPropertyChecks {
  sealed trait Or[+A, +B] extends Product with Serializable
  case class Left[A](a: A) extends Or[A, Nothing]
  case class Right[B](b: B) extends Or[Nothing, B]

  test("reproduce") {
    forAll { (i: Int, f: Int ⇒ Float Or Boolean) ⇒
      f(i)
    }
  }
}

This fails with:

RetrievalError was thrown during property evaluation.
  Message: couldn't generate value
  Occurred when passed generated values (
    arg0 = 0, // 30 shrinks
    arg1 = <function1>
    )

Note that providing an explicit Arbitrary[Float Or Boolean] solves the issue, so it seems pretty clear that the issue is in the generic derivation.

I'm not convinced the problem lies with shapeless-scalacheck - I tried writing my own generic derivation to see if it helped, and it failed in exactly the same way.

Something weird, but which is probably due to the way arbitrary functions work, is that the function is actually generated, but fails when evaluated.

Would appreciate any help / suggestion with this, as I'm kind of stuck.

Nicolas Rinaudo
  • 6,068
  • 28
  • 41

2 Answers2

3

The problem seems two-sided here:

  • The default test parameters in scalacheck (org.scalacheck.Test.Parameters.default) and scalatest, have a minSize of 0. Prop.check tries to generate values with this size on start up.

  • MkCoproductArbitrary.ccons in scalacheck-shapeless interprets a size of zero as a termination condition when generating arbitrary coproducts, and so fails on it.

As a temporary workaround, in the example from Travis's response, the property can be checked this way:

prop.check(Test.Parameters.default.withMinSize(1))

With scalatest, which is used in the question, the min size parameter can be changed too, possibly by putting something like

implicit val config = PropertyCheckConfiguration(minSize = PosZInt(1))

before the test case (warning: I didn't try / check the scalatest solution, unlike the pure scalacheck one just before).

Alex Archambault
  • 985
  • 1
  • 8
  • 16
  • This could be fixed in scalacheck-shapeless though, by changing / tweaking the termination condition in `MkCoproductArbitrary.ccons`. I can't recall under which conditions the size could go negative. Chances are it is actually always positive, and the termination condition could be changed to `size < 0` (where the size would have gone negative because of the arbitraries derived by scalacheck-shapeless). – Alex Archambault Nov 03 '16 at 23:20
  • I've tried `implicit override val generatorDrivenConfig = PropertyCheckConfiguration(minSize = PosZInt(1))` for scalatest, which doesn't seem to work. But that's fine though - are you going to fix this at the `scalacheck-shapeless` level? I can absolutely disable these tests and re-enable them as soon as a fix is available – Nicolas Rinaudo Nov 04 '16 at 09:01
2

This is a neat problem, and I don't really have a solution, but it's similar to some other issues I've run into now that we have nice Arbitrary instances for functions in ScalaCheck 1.13.

To start with, here's a minimization that doesn't depend on ScalaTest:

import org.scalacheck._, Shapeless._

sealed trait Foo; case object Bar extends Foo

val prop = Prop.forAll { (f: Int => Foo) => f(0); true }

And then:

scala> prop.check
! Exception raised on property evaluation.
> ARG_0: <function1>
> Exception: org.scalacheck.Gen$RetrievalError: couldn't generate value
org.scalacheck.Gen.loop$1(Gen.scala:57)
org.scalacheck.Gen.doPureApply(Gen.scala:58)
...

When I've run into stuff like this before in 1.13, the core problem has always been a generator that's failing when the size it's given is zero, and in fact the culprit seems to be the case 0 => Gen.fail in this line in scalacheck-shapeless.

We'd need to ask Alexandre to be sure, but this seems to be an after-the-fact attempt to avoid stack overflows on recursive ADTs. Here's the context:

Gen.sized {
  case 0 => Gen.fail
  case size =>
    val sig = math.signum(size)

    Gen.frequency(
      1   -> Gen.resize(size - sig, Gen.lzy(headArbitrary.value.arbitrary)).map(Inl(_)),
      n() -> Gen.resize(size - sig, Gen.lzy(tailArbitrary.arbitrary.arbitrary)).map(Inr(_))
    )
}

In the current implementation sig will always be 1, so I'm guessing originally the case 0 wasn't there and the intent was to do something like math.min(0, size - 1) (which will always be equal to size - math.signum(size) for non-negative integers).

If you remove the case 0 => Gen.fail line (or if you set the minimum size to anything larger than zero), your code works just fine. The problem is that without the case 0 line, the derived instance for an ADT like this can overflow the stack:

sealed abstract class Tree
final case class Node(left: Tree, right: Tree, v: Int) extends Tree
case object Leaf extends Tree

…since you might keep picking the recursive Node branch even though size is 0.

What you want is to be able to say something like "give me a value but not from a recursive constructor" when the size is zero, and at a glance I don't see a way to make that happen.

Clearly the case 0 => Gen.fail is bad in our new Cogen-ful world. If scalacheck-shapeless were my library, I'd be tempted to do something awful like delete that line and just catch the StackOverflowError and Gen.fail for recursive ADTs. That would be a hack, but it's still better than the current situation.

We should probably take this up in a scalacheck-shapeless issue. In the meantime I'd just write the Arbitrary for Or by hand (you should be able to let the Right and Left ones be derived, so this isn't too bad).

Travis Brown
  • 138,631
  • 12
  • 375
  • 680