Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add something like clues from Kotest #534

Open
dalewking opened this issue May 2, 2024 · 5 comments
Open

Add something like clues from Kotest #534

dalewking opened this issue May 2, 2024 · 5 comments

Comments

@dalewking
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

JakeWharton commented May 2, 2024

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()
    }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants