nonEmptyListOf.sample started to return None

[SebastianRabiej]

Dear fellow developers,

After migration from 1.14.3 to 1.15 Gen.nonEmptyListOf.sample started to behave differentlly.

This code works on version 1.14.3 but sometimes brakes on version 1.15.0 and 1.15.1:

import org.scalacheck.Gen
import org.scalatest.flatspec.AnyFlatSpecLike
import org.scalatest.OptionValues._
import org.scalatest.matchers.should.Matchers

class ScalaCheckTestSpec extends AnyFlatSpecLike with Matchers {
  it should "work" in {
    for (nr <- 1 to 10000) {
      println("iteration nr: " + nr)
      val values = Gen.nonEmptyListOf(Gen.chooseNum(2, 100)).sample.value
      values shouldNot be(empty)
    }
  }
}

The reason for failure always will be None after the method sample.

[imRentable]

I experience a similar behavior in generators that don't use Gen.nonEmptyListOf as well.

[ashawley]

Is there a reason you're using .sample in your tests? Is that from an example you saw somewhere? Or are you using it here because you believe that's the root cause of the issue? The method sample is good for checking your generators, but it shouldn't be used in property tests. You should be using forAll.

The canonical property test of your generator would be something like:

  property("nonEmptyListOf(chooseNum)") = {
    Prop.forAll(Gen.nonEmptyListOf(Gen.chooseNum(0,100))) { l =>
      l.length > 0
    }
  }

The above works fine for me.

[non]

I agree with @ashawley that it's better to avoid sample and that generators may return None and callers have to deal with it -- you can't necessarily call a generator 10,000 times and expect that all of the results will be Some(...).

However, I've independently been trying to reduce how often this happens, and noticed that it was fairly easy to avoid needing suchThat in the definition of nonEmptyBuildableOf (used by this method). So I've created a PR to address that, which would also make it less likely to receive None values here.

[ashawley]

Also, you need to use Prop.forAllNoShrink, not Prop.forAll. I should have used that above. Otherwise, test failures will shrink your generators to empty lists and produce unexpected and confusing errors.