willowtreeapps / assertk

assertions for kotlin inspired by assertj
MIT License
757 stars 84 forks source link

Add something like clues from Kotest #534

Open dalewking opened 3 months ago

dalewking commented 3 months ago

Kotest has the concept of Clues (https://kotest.io/docs/assertions/clues.html) which is a way to add context to assertion messages and is something I greatly miss about AssertK. The goal with tests is to make the error message crystal clear as to why somethihg failed so that one does not have to read the code to understand what failed. Sometimes that can be very difficult to do with AssertK.

The implementation is that they use a thread local to store the clues, they have functions that wrap a block to push and pop the clue and when outputting any assertion failure you also print the clues from the thread local

JakeWharton commented 3 months ago

Their need to use thread locals is because they operate directly on subject values whereas here we have the benefit of a wrapper type to carry along information. There's already the "path" built through through names supplied via transform (and it's higher-level APIs like prop). The entrypoint also allows a custom name. It's really about asserting the leaf nodes can always take a custom name (which we both found out isn't always possible such as in #526).

dalewking commented 3 months ago

I realize that one CAN pass that context down through the code and add that context to each and every leaf node assertion but that is extremely cumbersome. The ability to simply use a clue block to wrap all nested assertions and add the context is much more convenient.

If there is a simpler way that is built in i would love to know it, but i haven't found it.

JakeWharton commented 3 months ago

I wasn't saying that you need to carry it to the leaf. I was saying that this is already supported with the path-like naming mechanism that's built in, but it might require adding more places to input that. For example, all does not take a name but probably should allow one.

dalewking commented 3 months ago

I did discover there is a way to get at least one level of context. i never knew you could chain assertThat:

        assertThat("This is outer context").all {
            assertThat(5).isEqualTo(6)
        }

Will output:

expected:<[6]> but was:<[5]> ("This is outer context")
Expected :6
Actual   :5

It doesn't work with multiple levels of nesting but there is probaly a way to get there. Still researching

JakeWharton commented 3 months ago

Yes that is currently thread-local based, but I have a proposal (that I've been sitting on for too long) for changing it to be instance-based. That would improve the ability to nest arbitrarily deep .all nodes and/or the assertAll root.

With what I said above about "all"s taking a name, I think a better design would be

assertAll("This is outer context") {
  assertThat(someObject).all("inner context") {
    prop(SomeType::name).isEqualTo("Alice")
    any("Even more context") {
      prop(SomeType::age).isGreaterThan(25)
      prop(SomeType::isWhatever, "more context").isTrue()
    }
  }
}