Gen.posNum fails to produce a value, listOfN and other combinators don't handle this

[zakpatterson]

On 1.14.2 and 1.14.1, I get the following:

scala> (1 to 10000).toList.map(_ => Gen.posNum[Int].sample).flatten.length
res99: Int = 9904

scala> (1 to 10000).toList.map(_ => Gen.posNum[Int].sample).flatten.length
res100: Int = 9887

This issue is absent on 1.14.0:

scala> (1 to 10000).toList.map(_ => Gen.posNum[Int].sample).flatten.length
res1: Int = 10000

scala> (1 to 10000).toList.map(_ => Gen.posNum[Int].sample).flatten.length
res2: Int = 10000

This might be related to #567

1.14.1 failing build
1.14.2 failing build
My project's version pr
My use case

[non]

@zakpatterson This is an intentional result of a change that enables floating point numbers between 0 and 1 to be generated. We could do something more sophisticated to change this behavior for integral types, but for fractional types it's very unlikely we'll be able to avoid throwing away some values.

[zakpatterson]

@non interesting. Thanks for the explanation. It seems though other parts of scalacheck are not checking for this behavior, for example Gen.listOfN will just return None forever given a large N:

scala> Gen.listOfN(10000, Gen.posNum[Int]).sample
res2: Option[List[Int]] = None

[non]

@zakpatterson Yeah, I think listOfN needs to be able to handle occasional failures to generate A values without failings to generate a valid List[A]. There are two issues here:

• Feature request: make posNum always succeed on integral types
• Bug fix: make listOfN (and other combinators) resilient to occasional failures in the underlying generator

[zakpatterson]

Ok, I'll take a shot at a PR for at least the first.

[zakpatterson]

About the listOfN behavior, if a user has a generator in hand that they know fails with some probability, then is it not reasonable to expect to get a failure out of listOfN(1, gen) with the same probability? This part seems ok to me actually.

[non]

@zakpatterson My claim is that listOfN should do some amount of retrying. I think it's not unreasonable to expect a 1% individual failure rate to be something we can paper over.

I'm planning to work on that part myself.

[mdedetrich]

@non

but for fractional types it's very unlikely we'll be able to avoid throwing away some values.

Whats the reason why?

EDIT: Also I agree if Gen.posNum fails then listOf should also fail, we shouldn't silently suppress warnings.

[zakpatterson]

I'd support the property that generator g fails with the same probability as Gen.listOfN(1, g).

With Gen.posNum merged in #569 I move to close the [rest of the] issue as wontfix.

[satorg]

Hi there. Looks like things became even worse after upgrading from 1.14.3 to 1.15.1.
Consider the following minimal example:

  val nestedGen = Gen.listOf(Gen.resize(5, Gen.nonEmptyListOf(Gen.asciiChar)))

  Prop
    .forAll(nestedGen, nestedGen, nestedGen, nestedGen, nestedGen, nestedGen, nestedGen, nestedGen) {
      (_, _, _, _, _, _, _, _) => true
    }
    .check()

It always succeeds on 1.14.3 but almost always fails on 1.15.1 with the following error:

! Gave up after only 43 passed tests. 501 tests were discarded.

As far I as can see, the implementation of nonEmptyBuildableOf has changed since 1.14.3 and that change may cause the failures.

In my particular case I have a quite sophisticated generator in my tests (with nested sub-generators) that started failing after the upgrade. I managed to elaborate a work-around by replacing Gen.nonEmptyLisftOf with my custom method:

  def Gen_nonEmptyListOf[T](gen: => Gen[T]): Gen[List[T]] =
    for {
      head <- gen
      tail <- Gen.listOf(gen)
    } yield head :: tail

which works fine for me, although I'd prefer to call to Gen methods of course.

But I wonder, wouldn't it better to take a similar approach for nonEmptyBuildableOf implementation:

def nonEmptyBuildableOf[C,T](g: Gen[T])(implicit
    evb: Buildable[T,C], evt: C => Traversable[T]
  ): Gen[C] = g.flatMap { head =>
    evb.builder += head
    buildableOf(g)(evb, evt)
  }

Comparing to the current implementation:

  def nonEmptyBuildableOf[C,T](g: Gen[T])(implicit
    evb: Buildable[T,C], evt: C => Traversable[T]
  ): Gen[C] =
    buildableOf(g)(evb, evt).suchThat(c => evt(c).size > 0)

I.e. to guarantee that a buildable always has an extra element rather than discarding empty buildables (which makes the whole generator unreliable in some cases).

wdyt?

[ashawley]

If there is a regression with nonEmptyListOf we should open a new ticket. Would you be willing to open a new ticket? We can link the new ticket to this one given the overlap.

[satorg]

@ashawley sure, here it is: #714.

[ashawley]

With Gen.posNum merged in #569 I move to close the [rest of the] issue as wontfix.

The second issue should also be fixed now after #709 was merged and released in version 1.15.2. Thanks for the bug reports.