typelevel / scalacheck

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

PropertySpecifier.update Prop arg is repeatedly reevaluated. Bug or Feature? #424

Open pford19 opened 6 years ago

pford19 commented 6 years ago

Summary

A Prop generated by a function can succeed or fail depending on how it is "registered" in the source code. This seems to be due to the handling of the lazy Prop argument to PropertySpecifier.update (and to the fact that properties and generators encapsulate complex mutable state).

Example Scenario

Assume a Prop generating function makeProp. Suitably defined (see Detailed Example section below for an explicit example) the following source text will succeed:

    val p = makeProp(...)
    property("p succeeds") = p

whereas this source text will fail:

    property("makeProp fails") = makeProp(...)

Discussion

Properties.property method has this signature

    PropertySpecifier.update(String,=>Prop) // note lazy Prop

That is

    property("foo") = p

is syntactic sugar for

    new PropertySpecifier.update("foo",p)

The lazy Prop arg, and how it is handled, leads to surprising behavior. In particular when the Prop value in the source text is a function call, it appears that the function is called for each Prop.apply yielding a fresh version of the Prop for each apply.

In particular, if the property's generator has a fixed non-random first value, the the effect appears to be that the property test only sees that first value.

I did a superficial inspection of the source code. One thing that jumps out is that the =>p: Prop argument to update is wrapped in a Prop.delay. And delay's docs point out that the delayed property is reevaluated each time the wrapper property is evaluated. In contrast, Prop.lzy(=>Prop) seems to have the desired property of only evaluating once. But this is getting way beyond my ken. I defer to the experts to explain.

Detailed Example

object Example extends Properties("example") {

  // Generate `c` followed by -1's. 
  def genOverIterable(c: Iterable[Int]): Gen[Int] = { 
    val it = c.iterator
    Gen.const(-1).map(if (it.hasNext) it.next else _) 
  }

  // A property that asserts the existence of `n` in the generated value stream.
  def makeProp(n: Int) = exists(genOverIterable((0 to n)))(_ == n)

  val p1 = makeProp(1)
  property("p1 succeeds") = p1

  property("makeProp(1) fails") = makeProp(1)
}

In the above example, makeProp creates a new generator instance and a new property instance with each call. The generator's value stream is non-random, always (0, 1, 2, ..., n, -1, -1, -1, ...), in particular the first value is always 0.

Any Prop generated by makeProp should succeed, because the property asserts the existence of nin the stream of generated values, and n is always there, by design.
For any n > 0, the existence check requires multiple attempts since n > 0 is never the first value generated.

What appears to be happening when the makeProp(1) source expression is passed directly to update is that it is called for each apply. This effectively restarts the non-random generator and it repeatedly delivers its first value, which is 0. The existence test then fails after 501 discarded attempts.

Admittedly non-random generators are corner cases (even pathological?), and this behavior will probably never manifest for properties based on well-behaved random generators. (What happens with in the withSeed case?).

Is this is a BUG or FEATURE? If a feature it is a very subtle one.

Workaround

Compute props first, then pass to update.

    // DO THIS
    (0 to 9).foreach { i => 
        val p = makeProp(i)
        property(s"makeProp($i)") = p
    }

    // DON'T DO THIS
    (0 to 9).foreach { i => 
        property(s"makeProp($i)") = makeProp(i)
    }
jwlivongo commented 5 years ago

I have been working on something which relates to the question above "(What happens with in the withSeed case?)"

What I would like to do is to have a property which executes deterministically (from test run to test run) , yet for each iteration of a forAll , generates random values using Gen composition.

The simplest, self contained example I have is

import org.scalacheck.Prop.forAll
import org.scalacheck.Properties

class DeterministicPropertyBasedExample extends Properties("DeterministicPropertyBasedExample") {
  propertyWithSeed("DeterministicPropertyBasedExample", Some("H84AeXXB037k6uRDc8xUqygjoDnJ8XMdAD9TpRY4iqP=")) =
    forAll { (a: Int, p: Int => Boolean) =>
      println(s"a=$a")
      true
    }
}

When I ran this, what I expected to see was different integer values being printed out. What I do see is

a=-2147483648
a=-2147483648
a=-2147483648
a=-2147483648
a=-2147483648
a=-2147483648
a=-2147483648
a=-2147483648
a=-2147483648
a=-2147483648

My question is , is there any way to modify this example or otherwise be able to generate different random values for each iteration of the forAll.

Thanks

jwlivongo commented 5 years ago

Using scalacheck 1.14.0