Recursive comparison uses equals on root object when useOverriddenEquals is enabled

[austinarbor-wk]

Describe the bug
It appears that 3.25.0 has changed behavior of recursive comparison

• assertj core version: 3.25.0
• java version: 17
• test framework version: junit platform 5.10.1

The below test succeeds in 3.24.2 but fails in 3.25.0

  @Test
  void testRecursiveCompare() {
    var a1 = new A();
    a1.name = "test1";
    var b1 = new B();
    a1.b = b1;
    b1.name = "test2";

    var a2 = new A();
    a2.name = "test1";
    a2.b = b1;

    var list1 = List.of(a1);
    var list2 = List.of(a2);

    var cfg = RecursiveComparisonConfiguration.builder().withComparedFields("name", "b").build();
    cfg.useOverriddenEquals();

    assertThat(list1)
        .usingRecursiveFieldByFieldElementComparator(cfg)
        .containsExactlyElementsOf(list2);
  }

  private static class A {
    private String name;

    private String guid = UUID.randomUUID().toString();

    private B b;

    @Override
    public boolean equals(Object o) {
      if (this == o) {
        return true;
      }
      if (!(o instanceof A a)) {
        return false;
      }
      return Objects.equals(name, a.name) && Objects.equals(guid, a.guid) && Objects.equals(b, a.b);
    }

    @Override
    public int hashCode() {
      return Objects.hash(name, guid, b);
    }
  }

  private static class B {
    private String name;

    private String guid = UUID.randomUUID().toString();

    @Override
    public boolean equals(Object o) {
      if (this == o) {
        return true;
      }
      if (!(o instanceof B b)) {
        return false;
      }
      return Objects.equals(name, b.name) && Objects.equals(guid, b.guid);
    }
  }

[scordio]

Thanks for reporting it, @austinarbor-wk!

[scordio]

Possibly related to 0eb44cc and still unsure if the behavior in version 3.24.2 was correct, though.

What should be the expected behavior when withComparedFields("name", "b") and useOverriddenEquals() are used together, and the overridden equals compares also guid?

I feel useOverriddenEquals() should win on top of any withComparedFields(...) config, like the behavior of 3.25. Maybe we should prevent such config combinations or fail fast with some IllegalStateException.

Any thoughts @joel-costigliola?

[austinarbor-wk]

I think my expectation from the original code sample above based on the javadoc is that the comparison would be equivalent to the compound assertion of

assertThat(a1.name).isEqualTo(a2.name)
assertThat(a1.b).isEqualTo(a2.b)

Is my understanding incorrect?

[scordio]

Do I understand correctly that you'd like to use the overridden equals only for B but not for A?

[joel-costigliola]

I would expect both compared fields to be compared with equals, compared fields limits the comparison to the specified fields and useOverriddenEquals() instructs the recursive comparison to compare them with their equals method.

I'll have a look at the issue this week.

[austinarbor-wk]

My expectation is how @joel-costigliola describes

[joel-costigliola]

Alright, the issue is that the code that decides whether to use the overridden equals or not does not honor the compared fields, in the given example test it decides to compare A and B with equals instead of only comparing the name and b fields

[schlosna]

Thank you for the assertj project, it makes reading & writing tests enjoyable.

I am curious if there is a timeline for 3.25.2 release with the fix for this issue. We have had to roll back a number of projects to 3.24.2 due to tests using recursive comparison timing out and failing.

Thank you!

[scordio]

@schlosna yes, we plan to release 3.25.2 later this month, as soon as we finish looking into its remaining issues.

[schlosna]

Thank you for the 3.25.2 release! I ran some quick tests using 3.25.2 and it has resolved the issues we were encountering with usingRecursiveComparison.

[austinarbor-wk]

Confirmed 3.25.2 resolves the issues we were experiencing as well, thanks!

[joel-costigliola]

Thanks for the feedback, appreciated!

[ash211]

@schlosna and I work together -- 3.25.2 solves most of the recursive comparison issues we had, but not all. I've identified a regression in comparing deeply-nested objects and opened a demonstrative test case at #3350.

Thoughts on that PR would be much appreciated! Thanks again for assertj.