From 50d91084385b9ac650ad6d9e046acf338af371c0 Mon Sep 17 00:00:00 2001 From: Erik Osheim Date: Sat, 31 Aug 2019 00:56:44 -0400 Subject: [PATCH] Fix for #530 Here's the basic issue: When using viewSeed, we need to put an initialSeed in the Gen.Parameters used to evaluate the Prop, so we can recover the seed later and display it. So far, so good. In some cases, we need to "slide" the seed that's embedded in the parameters, so we don't reuse it. Any time we need to evaluate several properties and don't want identical RNG state (which could lead to identical inputs) we need to slide. We were missing a "slide" in flatMap -- we were reusing the same parameters in both cases. Since flatMap was used to define && (via for-comprehension) that meant that when you said p0 && p1, you'd use the same seed for both if viewSeed was set. Refined's property was unusual, but valid. Basically, one property had (x >= 0) ==> and one had (x < 0) ==>. So no single x value would satisfy both, but on average you'd satisfy each 50% of the time, and together you'd get a non-discarded case about 25% of the time. Previously, this worked OK. But since 1.14.0 we started always displaying seeds. This meant that the bug associated with failing to slide manifested, and we always generated the same x value for both sides of the property. This meant we ended up discarding every x generated. The fix was to slide correctly in this case. I also added a toString to Gen.Parameters which was missing, since this helped with debugging. --- src/main/scala/org/scalacheck/Gen.scala | 9 ++++++++ src/main/scala/org/scalacheck/Prop.scala | 23 +++++++++++-------- .../org/scalacheck/PropSpecification.scala | 5 ++++ 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/main/scala/org/scalacheck/Gen.scala b/src/main/scala/org/scalacheck/Gen.scala index 28c3f68de..b52589979 100644 --- a/src/main/scala/org/scalacheck/Gen.scala +++ b/src/main/scala/org/scalacheck/Gen.scala @@ -260,6 +260,15 @@ object Gen extends GenArities with GenVersionSpecific { /** Generator parameters, used by [[org.scalacheck.Gen.apply]] */ sealed abstract class Parameters extends Serializable { outer => + override def toString: String = { + val sb = new StringBuilder + sb.append("Parameters(") + sb.append(s"size=$size, ") + sb.append(s"initialSeed=$initialSeed, ") + sb.append(s"useLegacyShrinking=$useLegacyShrinking)") + sb.toString + } + /** * The size of the generated value. Generator implementations are * allowed to freely interpret (or ignore) this value. During test diff --git a/src/main/scala/org/scalacheck/Prop.scala b/src/main/scala/org/scalacheck/Prop.scala index 42499c8d1..37ff149df 100644 --- a/src/main/scala/org/scalacheck/Prop.scala +++ b/src/main/scala/org/scalacheck/Prop.scala @@ -50,7 +50,12 @@ sealed abstract class Prop extends Serializable { self => def map(f: Result => Result): Prop = Prop(prms => f(this(prms))) - def flatMap(f: Result => Prop): Prop = Prop(prms => f(this(prms))(prms)) + def flatMap(f: Result => Prop): Prop = + Prop { prms0 => + val res = this(prms0) + val prms1 = Prop.slideSeed(prms0) + f(res)(prms1) + } def combine(p: => Prop)(f: (Result, Result) => Result) = for(r1 <- this; r2 <- p) yield f(r1,r2) @@ -422,23 +427,23 @@ object Prop { /** Combines properties into one, which is true if and only if all the * properties are true */ - def all(ps: Prop*) = if(ps.isEmpty) proved else Prop(prms => - ps.map(p => p(prms)).reduceLeft(_ && _) - ) + def all(ps: Prop*): Prop = + ps.foldLeft(proved)(_ && _) /** Combines properties into one, which is true if at least one of the * properties is true */ - def atLeastOne(ps: Prop*) = if(ps.isEmpty) falsified else Prop(prms => - ps.map(p => p(prms)).reduceLeft(_ || _) - ) + def atLeastOne(ps: Prop*): Prop = + ps.foldLeft(falsified)(_ || _) /** A property that holds if at least one of the given generators * fails generating a value */ - def someFailing[T](gs: Seq[Gen[T]]) = atLeastOne(gs.map(_ == Gen.fail):_*) + def someFailing[T](gs: Seq[Gen[T]]): Prop = + atLeastOne(gs.map(_ == Gen.fail):_*) /** A property that holds iff none of the given generators * fails generating a value */ - def noneFailing[T](gs: Seq[Gen[T]]) = all(gs.map(_ !== Gen.fail):_*) + def noneFailing[T](gs: Seq[Gen[T]]): Prop = + all(gs.map(_ !== Gen.fail):_*) /** Returns true if the given statement throws an exception * of the specified type */ diff --git a/src/test/scala/org/scalacheck/PropSpecification.scala b/src/test/scala/org/scalacheck/PropSpecification.scala index 6a3fa5a0a..b05479d04 100644 --- a/src/test/scala/org/scalacheck/PropSpecification.scala +++ b/src/test/scala/org/scalacheck/PropSpecification.scala @@ -223,4 +223,9 @@ object PropSpecification extends Properties("Prop") { val prop = Prop.forAll(Bogus.gen) { b => Prop(false) } Prop(prop(params).failure && !Bogus.shrunk) } + + // make sure the two forAlls are seeing independent values + property("regression #530: failure to slide seed") = + forAll((x: Int) => (x >= 0) ==> true) && + forAll((x: Int) => (x < 0) ==> true) }