https://kotlinlang.org logo
Title
w

wasyl

10/20/2020, 3:52 PM
Another topic: is it a known issue that inline classes don’t work well in forall? Or maybe it’s something wrong with our setup (I don’t see what it could be though)
When using
kotlin.time.Duration
, this test will pass:
class WorkingSpec : DescribeSpec() {

    init {
        describe("Inline class test") {
            forAll(
                row(1.minutes),
            ) { wrapped: Duration ->
                println(wrapped)
            }
        }
    }
}
while this one will fail:
class FailingSpec : DescribeSpec() {

    init {
        describe("Inline class test") {
            forAll(
                row(1.minutes, true),
            ) { wrapped: Duration, _ ->
                println(wrapped)
            }
        }
    }
}
The only difference is that the failing test uses
row
with two parameters. And the failure is
java.lang.AssertionError: Test failed for (wrapped, 60.0s), (b, true) with error java.lang.ClassCastException: kotlin.time.Duration cannot be cast to java.lang.Number
s

sam

10/20/2020, 4:00 PM
I suppose they are experimental (still)
w

wasyl

10/20/2020, 4:04 PM
They are, I just wondered if it’s a known issue and what’s the explanation. Maybe it’s a bug in inline classes themselves? Guess I’ll investigate, then 👍
I kind of expected these would work unless Kotest expected some specific bytecode or used reflection on the parameters there
s

sam

10/20/2020, 4:05 PM
Yeah it should work
It does use reflection to work out the param names
Maybe switch to 4.3.0 and use the new data test support ?
w

wasyl

10/20/2020, 4:09 PM
I am on 4.3.0 now but I think I missed the new data test support. If I read the blog correctly, it’s basically that I can now use data classes instead of
row
functions?
s

sam

10/20/2020, 4:09 PM
Yes, but the framework will also generate the test's in the output for you.
before it would just effectively do a forEach over each value testing them one by one
now you actually can see multiple tests created
w

wasyl

10/20/2020, 4:14 PM
Always thought forAll worked like this. Anyway, seems like this fixes the inline class issue too 🙂 I suppose it is a bug in Kotlin then, with inline classes used in inline suspendable functions or something.
s

sam

10/20/2020, 4:14 PM
yeah possibly
the old forAll was ok, it would give you a nice error trace
but it wouldn't add individual test cases for you in the output tree
👌 1
w

wasyl

10/20/2020, 4:15 PM
The new data testing seems great btw, we would sometimes already create wrappers over `row`s to have named parameters, and would add names as well. Both look much nicer now
Thanks again! 🙂
s

sam

10/20/2020, 4:15 PM
you're welcome
w

wasyl

10/21/2020, 1:37 PM
Just realized that the new data-driven framework actually generates test cases, in our case
it {}
(in Describe specs). This has an unintended consequence that we no longer can nest `describe`s inside, right?
This can be misleading because we had no error for nested describe (and an
it
inside it), but in fact no code was really executed for these tests. Seems like either allowing more complex tests inside forall, or crashing if unsupported nesting is found, would be nice
s

sam

10/21/2020, 3:34 PM
Ok, so what we need to do is disallow describe inside its
👍 1
w

wasyl

10/21/2020, 3:38 PM
Silent fail is just a bit confusing. I also opened an issue with another silent fail on github issues
So, for the case when I want to nest `forAll`s (we have a couple), should we use old forAll or move away from data-driven tests completely?
We have tests like
forAll(row(sourceA, sourceB)) { source -> 
  describe("when we do X with $source") {
    forAll(row(fooA, fooB)) { foo ->
      describe("when we do Y with $foo") {
         // more stuff with tests
      }
    }
  }
}
s

sam

10/21/2020, 3:41 PM
I would probably just use for all
w

wasyl

10/21/2020, 3:48 PM
So the old one isn’t going anywhere?
s

sam

10/21/2020, 3:50 PM
I don't think we'd remove it no, there's no point
👍 1