willowtreeapps / assertk

assertions for kotlin inspired by assertj
MIT License
760 stars 85 forks source link

Better error messages when things are not equal, but the result of `toString()` is the same. #449

Closed yogurtearl closed 9 months ago

yogurtearl commented 1 year ago

Consider:

import androidx.compose.ui.text.AnnotatedString
...
        assertThat(AnnotatedString("asdf")).isEqualTo("asdf")

This correctly fails, but the error message can be a bit confusing.

as of 0.25, the error message is:

AssertionFailedError: expected:<["asdf"]> but was:<[asdf]>

A couple of thoughts.

  1. add the quotes for any CharSequence not just String. so that AnnotatedString would print as "asdf" (with quotes)
  2. Add type information to the failure message when the toString() result is the same, but they are not equal?
    AssertionFailedError: expected:<["asdf"]> (type `java.lang.String`) but was:<[asdf]> (type `androidx.compose.ui.text.AnnotatedString`)
JakeWharton commented 1 year ago

The types can also be the same but have failing equality and matching string representation, so you still need something which highlights this fact.

Also, bonus points for:

Truth has all these and it's a great thing to copy.

VirtualParticle commented 1 year ago

Perhaps the best solution would be to just explicitly call out that the two objects are different but have the same toString result in the error message? Maybe something like:

class Example {
    override fun toString() = "hello"
}

assertThat(Example()).isEqualTo(Example())
// AssertionFailedError: expected (type: `Example`) and actual (type: `Example`) are not equal but have the same string representation: "hello"

I'm not sure of the best way to refer to each object there.

JakeWharton commented 1 year ago

Another case where calling out the behavior is important is when equals is broken or identity-based on two otherwise-equivalent types. This will produce two equal outputs and the type of the two is the same. So it perhaps only behooves us to include the type when relevant, otherwise simply noting equals returned false but toString()s match.

JakeWharton commented 9 months ago

Another real world example: JDK 20 (well, the CLDR update in it) changed formatting to use a narrow non-break space from a regular space. Very hard to catch until IntelliJ saved me https://jakewharton.com/@jw/111403629212976596

evant commented 9 months ago

The original issue should be fixed, @JakeWharton feel free to file new issues for any of the other improvements you suggested