Another topic: is it a known issue that inline cla...
# kotest
w
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:
Copy code
class WorkingSpec : DescribeSpec() {

    init {
        describe("Inline class test") {
            forAll(
                row(1.minutes),
            ) { wrapped: Duration ->
                println(wrapped)
            }
        }
    }
}
while this one will fail:
Copy code
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
Copy code
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
I suppose they are experimental (still)
w
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
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
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
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
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
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
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
you're welcome
w
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
Ok, so what we need to do is disallow describe inside its
👍 1
w
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
Copy code
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
I would probably just use for all
w
So the old one isn’t going anywhere?
s
I don't think we'd remove it no, there's no point
👍 1