https://kotlinlang.org logo
#kotest-contributors
Title
# kotest-contributors
m

mitch

09/18/2021, 12:05 AM
Hi all. just bouncing some ideas around property testing. I just got some inspiration based on some recent work in property and suspend funs, as well as @sam’s branch on prop spec. This is 100% possible with some tweaks in the proptest and PropertyContext. so before
Copy code
test(...) {
  checkAll(arbFirst, arbSecond) { first, second ->
    sut.execute(first, second)
  }
}
and after
Copy code
test(...) { sut.execute(arbFirst.value(), arbSecond.value()) }
It’s really mindblowing! I can imagine how many hundreds lines of code I can simplify in our current tests if kotest has this.
the challenge that i can think of is classification. i don’t know how to make that work in here
@sam would be awesome to get your expert opinion around this.
s

sam

09/18/2021, 12:23 AM
What is it that you're trying to simplify here (because I don't have the before to compare)
m

mitch

09/18/2021, 12:36 AM
ah yes. hang on
we have a lot of tests like following
Copy code
// if we'd need additional setups - which alot of time is possible 
test("should test using arbs with setup") {
  checkAll(arbFirst, arbSecond) { first, second
    mongo.createDocument(first.copy(id = null))
    val testFirst = mongo.findByName(first.name)
    val testSecond = second.copy(id = testFirst.id)

    systemUnderTest.execute(testFirst, testSecond)
  }
}
I think this is possible
Copy code
test("should test using arbs with setup") {
  val first = mongo.createDocument(arbFirst.value().copy(id = null))
  systemUnderTest.execute(
    mongo.findByName(first.name),
    arbSecond.value().copy(id = first.id)
  )
}
as a user it’s pretty convenient to just call an arb inside a test and one just got a generated value out of it
and plus because there’s a rs, it’s repeatable when things fail
@sam added the examples 🙌
moved more nitty gritty implementation detail here:
Copy code
test("arbitraries should be used and registered") {
  propertyContext(proptestConfig, ...etc) { // produces a random source, this is PropertyContext
    val first = arbFirst.value()   // calls arbFirst.generate(rs) under the hood and record the sample & shrinks.
    val second = arbSecond.value() // calls arbSecond.generate(rs) under the hood and record the sample & shrinks.
    
    systemUnderTest.execute(first, second)
  }
}

// or simply the following, with Sam's property syntax
context("given some context") {
  configureProperty(proptestConfig)

  proptest("arbitraries should be used and registered") {
    systemUnderTest.execute(arbFirst.value(), arbSecond.value())
  }
}
s

sam

09/18/2021, 1:35 AM
Getting a single arb is easy, you just call .next() on the arb
I guess having a repeatable rs requires as bit more boilerplate
I guess it would be easy to expose the rs so you can call next and have the value be repeatable.
Copy code
checkAll { 
  val first = <http://Arb.int|Arb.int>().value()
  val second = Arb.string().value()
  sut.execute(first, second)
}
But isn't that just
Copy code
checkAll<Int, String> {  first, second -> 
  sut.execute(first, second)
}
Which is actually less code ?
m

mitch

09/18/2021, 2:33 AM
Yeah. The first one is what im thinking. Here we got the benefit of convenience and repeatability from rs as well. We actually never really use the second one (simple type inference), because they aren't explicit which arb were used. Its utility is also quite limited to simple types.. maybe that's something to expand on as well
s

sam

09/20/2021, 12:44 AM
If we want to support this I think it would be really easy.
Copy code
checkAll(iterations, [seed]) {
  val first = <http://Arb.int|Arb.int>().value()
  val second = Arb.string().value()
  sut.execute(first, second)
}

And for anything other than that:

checkAll(config) {
  val first = <http://Arb.int|Arb.int>().value()
  val second = Arb.string().value()
  sut.execute(first, second)
}
We could make checkAll accept a name too, if we want to even avoid having to wrap inside a test("") block, but I don't know if that's conflating two things
Eg
Copy code
test("all strings should be commutative under addition") {
   checkAll(1000) {
      val a = Arb.string().value
      val b = Arb.string().value
      a + b shouldBe b + a
   }
}
vs
Copy code
checkAll("all strings should be commutative under addition", 1000) {
   val a = Arb.string().value
   val b = Arb.string().value
   a + b shouldBe b + a
}
m

mitch

09/20/2021, 12:47 AM
yeah you read my mind, I’m thinking if we can perhaps piggyback on the current
checkAll
like in your first example. Because all infrastructure are already set in there.
s

sam

09/20/2021, 12:49 AM
which one do you like best
m

mitch

09/20/2021, 12:49 AM
i think this is definitely tempting (probably as a follow up)
Copy code
test("...") {
   val a = Arb.string().value()
   val b = Arb.string().value()
   a + b shouldBe b + a
}
which kinda suggest we need to be propagate some base PropertyConfig in the CoroutineContext
the thing i can’t still figure out is classification, it can’t easily work, can it?
s

sam

09/20/2021, 12:50 AM
why not
the existing classification or the new auto classification
m

mitch

09/20/2021, 12:51 AM
existing classification. you see, right now, it creates some sort of map storing the classification instances of the arbs that got passed in. but with the new mechanism, you can conjure a new arb inside the lambda.
like this
Copy code
test("...") {
   val a = Arb.string().value() // that's new, so everytime it loops, you get a new arb
   val b = Arb.string().value()
   a + b shouldBe b + a
}
s

sam

09/20/2021, 12:52 AM
the classification values are held inside the PropertyContext
which is setup by checkAll
it doesn't matter what the arb does, so it can be a fresh arb each time
m

mitch

09/20/2021, 12:55 AM
checking..
where do we put value inside the
classifications
mutable map?
s

sam

09/20/2021, 12:58 AM
when is it added ?
m

mitch

09/20/2021, 12:58 AM
yeah
s

sam

09/20/2021, 12:59 AM
classify methods on PropertyContext
m

mitch

09/20/2021, 1:01 AM
ah right! yes you’re right, that’ll continue to work
s

sam

09/20/2021, 1:01 AM
yep
m

mitch

09/20/2021, 1:01 AM
why did i think it won’t work
i must have found something else…?
s

sam

09/20/2021, 1:01 AM
maybe you think the arbs held onto state or something
m

mitch

09/20/2021, 1:02 AM
would autoclassification works?
Copy code
fun autoclassifications(): Map<String, Map<String, Int>> = autoclassifications.toMap()
s

sam

09/20/2021, 1:03 AM
yep
all the state is held inside property context
m

mitch

09/20/2021, 1:03 AM
man that’s super awesome!
s

sam

09/20/2021, 1:03 AM
yep 🙂
m

mitch

09/20/2021, 1:07 AM
awesome! i think we can’t use this function for that then? because the arbs are now dynamically registered. I can start looking into it if you haven’t already.
Copy code
internal suspend fun test(
   context: PropertyContext,
   config: PropTestConfig,
   shrinkfn: suspend () -> List<ShrinkResult<Any?>>,
   inputs: List<Any?>,
   classifiers: List<Classifier<out Any?>?>, // this
   seed: Long,
   fn: suspend () -> Any
) {
   require(inputs.size == classifiers.size)
   try {
      inputs.indices.forEach { k ->
         val value = inputs[k]
         val classifier = classifiers[k]
         if (classifier != null) {
            val label = (classifier as Classifier<Any?>).classify(value) // this
            if (label != null) context.classify(k, label)
         }
      }
}
s

sam

09/20/2021, 1:08 AM
Hmm that might be the sticking point - that test method is used for shrinking
I think we can work around that
we would need to hold onto a copy of every value returned from .value in that loop - and then use those values if an exception is throw to shrink them
m

mitch

09/20/2021, 1:11 AM
yeah that list of values (which can be stored in an atomic) can be cleared afresh when the test lambda completes, to conserve memory.
s

sam

09/20/2021, 1:11 AM
right
so .value on an arb would need to get access to property context and could register the values there
but now you need to define two receivers
which we can't do
unless .value was actually a member method...
inside Arb: fun PropertyContext.value() : A { val a = next(rs) store(a) return a }
m

mitch

09/20/2021, 1:16 AM
yeah, or
Copy code
sealed class Gen<A> {
  suspend fun TestContext.value(): A = getOrCreatePropertyContext { ctx ->
    val a = next(rs)
    ctx.store(a)
    a
  }
}
s

sam

09/20/2021, 1:17 AM
yeah you could pull it out of the coroutine context
the extension method stops you trying to do it elsewhere though
m

mitch

09/20/2021, 1:17 AM
but that’ll require property context to be propagated through and created when users opens up any sort of
Copy code
test("stuff") { // new context here
}
ah shoot you’re right
s

sam

09/20/2021, 1:18 AM
the check all's currently expose PropertyContexxt as the receiver
so we could do the same in the new checkAll
m

mitch

09/20/2021, 1:19 AM
yeah we don’t want to end up with this
Copy code
interface TestContext : PropertyContext, CoroutineScope { ... 
}
conflation alert
s

sam

09/20/2021, 1:19 AM
well that would be tricky as a) the kotest framework doesn't bring in the property framework, b) it means you couldn't use it in junit c) breakaing changes
m

mitch

09/20/2021, 1:20 AM
yeah exactly
or maybe we can enrich property context, since we have a new scope
Copy code
interface DynamicPropertyContext : PropertyContext {
  fun <A> Arb<A>.value(): A { ... }
}
s

sam

09/20/2021, 1:22 AM
yes that's another option
or just add that method to the existing property context
PropertyContext
is not an interface so you can add to it safely
m

mitch

09/20/2021, 1:23 AM
that’s currently calling that internal test method.. so it can be tricky
s

sam

09/20/2021, 1:23 AM
I think PC is a better place for it then the Arb class
m

mitch

09/20/2021, 1:23 AM
yeah i definitely +1 on that
s

sam

09/20/2021, 1:29 AM
All we need to do is pass RandomSource to the property context when it's created, and create a new checkAll method that sets up a property context and calls reset
m

mitch

09/20/2021, 1:30 AM
mind blown
sam you’re so fast
s

sam

09/20/2021, 1:30 AM
I will add the checkall now
m

mitch

09/20/2021, 1:30 AM
🙇
s

sam

09/20/2021, 1:36 AM
Ok this now works (pushed on branch)
Copy code
test("checkAll should setup a property context") {
   var count = 0
   checkAll(100) {
      val a = Arb.string().value()
      val b = Arb.string().value()
      (a + b).length shouldBe b.length + a.length
      count++
   }
   count shouldBe 100
}

test("checkAll should use a repeatable seed") {
   checkAll(1, 52104482139021L) {
      val a = Arb.string().value()
      a shouldBe "cUj%x=.f6ktw\"icenM(AEw&5CP3q+8FxU*xi<p!Jbd"
   }
}
the count in the first test is just to confirm the iterations parameter is being respected
m

mitch

09/20/2021, 1:37 AM
nice!!
s

sam

09/20/2021, 1:39 AM
its nice this
Copy code
test("auto labelling") {
   val context = checkAll(100, 98173L) {
      Arb.string().value()
   }
   context.autoclassifications()["1"] shouldBe mapOf(
      "MAX LENGTH" to 3,
      "ANY LENGTH LETTER OR DIGITS" to 3
   )
}
auto labelling wired in
bug there, should not be 3
ok all good
we need shrinking now
Ok I added some hooks for shrinking. I can leave that bit to someone else 🙂
m

mitch

09/20/2021, 3:57 AM
i’ll pick that up @sam thanks a lot! 😉
3 Views