Hi... I've been playing a bit more with my POC for...

# kotest-contributors

[dr.dreigh]

Hi... I've been playing a bit more with my POC for data class diffs, making it recursive for nested data classes, got it looking like:

data class diff for com.sksamuel.kotest.matchers.Baz
├ booFoo: data class diff for com.sksamuel.kotest.matchers.BooFoo
│  ├ a: expected:<9L> but was:<8L>
│  └ foo: data class diff for com.sksamuel.kotest.matchers.Foo
│     ├ a: expected:<123L> but was:<321L>
│     ├ b: expected:<"a"> but was:<"b">
│     └ c: expected:<"a"> but was:<"c">
├ i: expected:<66> but was:<67>
└ things: expected:<["cheese", 22]> but was:<["egg", 1]>

... however, it's lots of string manipulation. it uses shouldBe to choose the correct Eq instance. Then it gets the string from the the Throwable.message, and if it's a nested data class it does a little more manipulation.

My only thoughts to make this a little cleaner would be to change the Eq interface (and the instances), such that instead of returning a Throwable?, it returns a type which contains the differences. That way we don't have string manipulate as we'd have access to the diff data. However this might complicate things for the purpose of one feature. Be good to get the thoughts of people a bit more familiar with the codebase, I'm sure there's a much simper way - I just don't can't see it! 🙈 (If you want to see my extremely hacky POC code for reference).

[sam]

I'll review today

[dr.dreigh]

Cheers Sam, it's totally just a rough POC at the moment, very hacky to see how far I could get, and if I can do anything better without changing the Eq interface. Your eyes would be greatly appreciated. Changing EQ seems a bit of a big change though for a nicer diff, so didn't want to just go ahead ripping things apart!

[sam]

Sorry for delay in reviewing

[sam]

I agree with you, Eq should return a better object.

[sam]

I think what we need is a subclass of

AssertionError

which contains the error, the expected and the actual values.

[sam]

Then Eq could return that

[sam]

This is similar to tricks that are done elsewhere, for example in the console runner

when (error) {
   is org.opentest4j.AssertionFailedError ->
      if (error.isActualDefined && error.isExpectedDefined) {
         type("comparisonFailure")
            .actual(error.actual.stringRepresentation)
            .expected(error.expected.stringRepresentation)
      }
}

[sam]

AssertionFailedError is the opentest4k's version of an error that contains actual and expected

[dr.dreigh]

Cool, thanks - I'll have a go at that. Should give me more structure to work with.

[dr.dreigh]

Hi, think I have hit a bit of a stumbling block with this. I have no way of knowing if a data class has overridden equals. In which case a detailed diff of all the fields declared in the constructor would be very irrelevant.

[sam]

I think it's fair to assume a data class doesn't have an overriden equals

[sam]

If it does then that's circumventing normal expectations

[sam]

The whole point of a data class is the equals hashcode and component methods

[sam]

We can also use a flag to disable this behavior if needed

[dr.dreigh]

ok cool, I'll crack on. Yeah I must admit we don't override equals in our codebase... and I had to even second guess if it was possible to override equals in a data class, but I can't see any way via reflection to determine where the equals method is defined.

I'm still a little unsure of the best way to do the recursive bit nicely but I'm sure I'm come up with several nasty ways and we can choose the least worst

[sam]

Ok