PropertyTesting used to work, no longer does

[russel]

The test:

class Factorial_KotlinTest_PropertyBased : PropertyTesting() {
  init {

    val algorithms = listOf(
        "iterative" to {x:Int -> iterative(x)},
        "reductive" to {x:Int -> reductive(x)},
        "naïve_recursive" to {x:Int -> naïve_recursive(x)},
        "tail_recursive" to {x:Int -> tail_recursive(x)}
    )

    forAll(algorithms) { a ->
      val f = a.second
      property(a.first + ":recurrence relation is true for all non-negative integer values").
          forAll{i: Int ->
            if (0< i && i < 500) { f(i) == (i.bigint * f(i - 1)) }
            else { true }
          }
    }

  }
}

was working fine 10 days ago. With the 1.3.1 release I thought I'd update. I thought:

class Factorial_KotlinTest_PropertyBased : StringSpec() {
  init {

    "Factorial recurrence relation works for all algorithms for all non-negative integers" {

      val algorithms = listOf(
          "iterative" to { x: Int -> iterative(x) },
          "reductive" to { x: Int -> reductive(x) },
          "naïve_recursive" to { x: Int -> naïve_recursive(x) },
          "tail_recursive" to { x: Int -> tail_recursive(x) }
      )

      forAll(algorithms){a ->
        val f = a.second
        forAll{i: Int ->
          if (0 < i && i < 500) { f(i) == (i.bigint * f(i +1)) }
          else { true }
        }
      }
    }

  }
}

to be the replacement. However it is clearly broken since it passes and clearly should not. There is an error introduced which means it should fail.

[russel]

I can confirm the PropertyTesting version generates four tests which pass with 1.2. This is actually a very good behaviour that seems may have been lost.

[russel]

And:

class Factorial_KotlinTest_PropertyBased : StringSpec() {
  init {

    val algorithms = listOf(
        "iterative" to {x:Int -> iterative(x)},
        "reductive" to {x:Int -> reductive(x)},
        "naïve_recursive" to {x:Int -> naïve_recursive(x)},
        "tail_recursive" to {x:Int -> tail_recursive(x)}
    )

    forAll(algorithms) { a ->
      val f = a.second
      a.first + ":recurrence relation is true for all non-negative integer values" {
        forAll { i: Int ->
          if (0 < i && i < 500) { f(i) == (i.bigint * f(i - 1)) }
          else { true }
        }
      }
    }

  }
}

generates the error:

java.lang.AssertionError: 0 passed tests but expected 4

    at io.kotlintest.Inspectors$DefaultImpls.forAll(Inspectors.kt:10)
    at io.kotlintest.matchers.Matchers$DefaultImpls.forAll(Matchers.kt)
    at io.kotlintest.TestBase.forAll(TestBase.kt:18)
    at uk.org.winder.maths.factorial.Factorial_KotlinTest_PropertyBased.<init>(Factorial_KotlinTest_PropertyBased.kt:15)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
    at java.lang.Class.newInstance(Class.java:442)
    at io.kotlintest.KTestJUnitRunner.<init>(KTestJUnitRunner.kt:9)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
    at org.junit.internal.builders.AnnotatedBuilder.buildRunner(AnnotatedBuilder.java:104)
    at org.junit.internal.builders.AnnotatedBuilder.runnerForClass(AnnotatedBuilder.java:86)
    at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
    at org.junit.internal.builders.AllDefaultPossibilitiesBuilder.runnerForClass(AllDefaultPossibilitiesBuilder.java:26)
    at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
    at org.junit.internal.requests.ClassRequest.getRunner(ClassRequest.java:33)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:96)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:42)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:253)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:84)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

[sksamuel]

Do you have the implementation for iterative and reductive

[russel]

It's from my Factorial Git repository on GitHub, https://github.com/russel/Factorial/tree/master/Kotlin I can post the code here if that is better.

[sksamuel]

Ok thanks. So the test "description" must be top level, and then the for all nested inside.

class Factorial_KotlinTest_PropertyBased : StringSpec() {
  init {

    val algorithms = listOf(
        "iterative" to { x: Int -> iterative(x) },
        "reductive" to { x: Int -> reductive(x) },
        "naïve_recursive" to { x: Int -> naïve_recursive(x) },
        "tail_recursive" to { x: Int -> tail_recursive(x) }
    )

    ":recurrence relation is true for all non-negative integer values" {
      forAll(algorithms) { a ->
        val f = a.second
        a.first +
            forAll { i: Int ->
              if (0 < i && i < 500) {
                f(i) == (i.bigint * f(i - 1))
              } else {
                true
              }
            }
      }
    }
  }
}

[russel]

This is nowhere near as good from the perspective of reporting. The 1.2 version allowed for construction of test cases table style with the tests being property style. This is a great model since it is "unrolling" over the deterministic variable but leaving the random stuff "hidden". This way of working is available in QuickCheck and Hypothesis, because it is the right thing to do. KotlinTest 1.2 followed this model. Now with 1.3 we have given up that way of working and are now constrained in a way that seems unnecessary: the test cases are not actually top level it is just that the ability to table drive the creation of the test cases has been removed. For now I'll stick with 1.2.

[sksamuel]

So you just want to create the tests themselves programatically. The outer forAll is a test matcher which should only be used in a test proper, I'm surprised it worked before. You can do what you want with standard forEach and I think it's actually a bit more readable while doing what you want (I think).

Try:

class Factorial_KotlinTest_PropertyBased : StringSpec() {
  init {

    val algorithms = listOf(
        "iterative" to { x: Int -> iterative(x) },
        "reductive" to { x: Int -> reductive(x) },
        "naïve_recursive" to { x: Int -> naïve_recursive(x) },
        "tail_recursive" to { x: Int -> tail_recursive(x) }
    )

    algorithms.forEach {
      val f = it.second
      (it.first + ":recurrence relation is true for all non-negative integer values") {
        forAll { i: Int ->
          if (0 < i && i < 500) {
            f(i) == (i.bigint * f(i - 1))
          } else {
            true
          }
        }
      }
    }
  }
}

[russel]

This looks spot on. I was being blinkered by the previous working version. I am upgrading now.

Note to self: work harder at this.

[sksamuel]

No that's fine, the previous property testing was undocumented so wasn't clear what it was I was intending.

[sksamuel]

I notice you're using a filter of sorts (the if statement and else with true) so what we need to add in is the equivalent of ScalaCheck's when so you can do

      forAll { i: Int ->
          when (0 < i && i < 500) {
            f(i) == (i.bigint * f(i - 1))
          }
        }

We need another keyword than when as when is a keyword itself in Kotlin of course

[russel]

Something like this, the underlying need anyway. Some frameworks say do not filter the standard generator (since this may mean selecting 200 values to get 100 useful one) but do away with the default generator and introduce a specific one which generates in the right range. Specks has this for Ceylon for example. It uses the names parameters technology to do this.

[sksamuel]

I suppose that makes sense. Perhaps either way would be handy. We can always add Gen.nonNegInt etc.

[russel]

That route is how Frege has gone for conditioning things, pointfree function composition helps for using that technique though.

As long as the forAll/when results in 100 (or whatever the number is at the time) tests applied no matter how big the sample has to be to achieve it, then that is what matters most.

[russel]

@sksamuel The forAll/when combination above doesn't work on latest release Kotlin/KotlinTest (1.0.5) the forAll gives a type inference fail.

     (name + ": recurrence relation is true for all non-negative integer values") {
        forAll{i: Int ->
          when (0 < i && i < 500) { f(i) == (i.bigint * f(i - 1)) }
        }
      }

results in:

e: /home/users/russel/Progs/Applications/Factorial/Kotlin/src/test/kotlin/uk/org/winder/maths/factorial/Factorial_KotlinTest_PropertyBased.kt: (25, 66): Expecting '->'
e: /home/users/russel/Progs/Applications/Factorial/Kotlin/src/test/kotlin/uk/org/winder/maths/factorial/Factorial_KotlinTest_PropertyBased.kt: (24, 9): Type inference failed: inline fun <reified A> forAll(noinline fn: (A) -> Boolean): kotlin.Unit
cannot be applied to
((Int) -> kotlin.Unit)

whereas using an if statement works fine.

[helmbold]

@russel I think it would be best to file a new issue for this.