Assertj-core: containsExactly does not work with the recursive comparison when comparing different types

[jakubbrodzinski]

Summary

I faced an unexpected behavior (for me) with usingRecursiveFieldByFieldElementComparator when using it with containsExactly.

When i use .usingRecursiveFieldByFieldElementComparator() with .containsExactlyInAnyOrder(), it works just fine, so the elements are correct.
When I extract every field from class and use containsExtacly with tuples, it works just fine, so the order is correct.
But when I combine usingRecursiveFieldByFieldElementComparator() with containsExactly the outcome is different than I expected.

In addition without calling asList() my example won't compile, because it expects ListEntryDto... in containsExatcyl method. I am not sure what I am doing wrong, since it's almost identicial situation to the one from example(https://assertj.github.io/doc/#assertj-core-recursive-comparison-for-iterable).

Example

public class Test {

    @org.junit.jupiter.api.Test
    void testTest() {
        final var le1 = createListEntry(12, "xyz", "abc", "432432", "asdsa");
        final var le2 = createListEntry(13, "xyzx", "abcds", "32432432", "asdsdfsa");
        final var le3 = createListEntry(14, "xyzsa", "axbc", "4sd32432", "asdsfsda");
        final var le4 = createListEntry(15, "xyzdsa", "abcb", "43sdf2432", "asdsdsa");

        List<ListEntryDto> actualResultList = List.of(ListEntryDto.from(le1), ListEntryDto.from(le2), ListEntryDto.from(le3), ListEntryDto.from(le4));
//passes
        Assertions.assertThat(actualResultList)
                .asList()
                .hasSize(4)
                .usingRecursiveFieldByFieldElementComparator()
                .containsExactlyInAnyOrder(le1, le2, le3, le4);
//passes
        Assertions.assertThat(actualResultList)
                .usingRecursiveComparison()
                .isEqualTo(list(le1, le2, le3, le4));
//fails (passes when "randomProperty" field is commented out)
        Assertions.assertThat(actualResultList)
                .asList()
                .hasSize(4)
                .usingRecursiveFieldByFieldElementComparator()
                .containsExactly(le1, le2, le3, le4);
    }

    private ListEntry createListEntry(int id, String name, String secondName, String street, String postCode) {
        final var le = new ListEntry();
        le.setId(id);
        le.setName(name);
        le.setSecondName(secondName);
        le.setStreet(street);
        le.setPostCode(postCode);
        le.setRandomProperty("xyz xyz"); // <- comment

        return le;
    }
}

@Getter
@Setter
abstract class SuperListEntry {
    private int id;
}

@Getter
@Setter
class ListEntry extends SuperListEntry {
    private String randomProperty; // <- comment

    private String name;

    private String secondName;

    private String street;

    private String postCode;
}

@Getter
@Setter
class ListEntryDto {
    private final int id;
    private final String name;
    private final String secondName;
    private final String street;
    private final String postCode;

    public ListEntryDto(int id, String name, String secondName, String street, String postCode) {
        this.id = id;
        this.name = name;
        this.secondName = secondName;
        this.street = street;
        this.postCode = postCode;
    }

    static ListEntryDto from(ListEntry le) {
        return new ListEntryDto(le.getId(), le.getName(), le.getSecondName(), le.getStreet(), le.getPostCode());
    }
}

[joel-costigliola]

Thanks for reporting this behavior.

I could not reproduce the issue with assertj core 3.19.0 using the following code:

  static class ListEntryDto {
    private Integer id;
    private String name;
    public ListEntryDto(String name, Integer id) {
      this.name = name;
      this.id = id;
    }
  }

  static class ListEntry extends GeneratedIdBase {
    private String name;
    private String secondName;
    private String street;
    private String postCode;
    public ListEntry(String name, String secondName, String street, String postCode, Integer id) {
      super(id);
      this.name = name;
      this.secondName = secondName;
      this.street = street;
      this.postCode = postCode;
    }
  }

  static class GeneratedIdBase {
    private Integer id;
    public GeneratedIdBase(Integer id) {
      this.id = id;
    }
  }

  @Test
  public void should_work() {
    ListEntry le1 = new ListEntry("name1", "secondName1", "street1", "postCode1", 1);
    ListEntry le2 = new ListEntry("name2", "secondName2", "street2", "postCode2", 2);
    ListEntry le3 = new ListEntry("name3", "secondName3", "street3", "postCode3", 3);

    ListEntryDto led1 = new ListEntryDto("name1", 1);
    ListEntryDto led2 = new ListEntryDto("name2", 2);
    ListEntryDto led3 = new ListEntryDto("name3", 3);

    List<ListEntryDto> dtoList = list(led1, led2, led3);

    assertThat(dtoList).asList()
                       .hasSize(3)
                       .usingRecursiveFieldByFieldElementComparator()
                       .containsExactlyInAnyOrder(le1, le2, le3);
  }

Could you provide a complete reproducible test? (preferably without annotations)

usingRecursiveFieldByFieldElementComparator() has been introduced before the recursive comparison and thus is not based on it, something we should probably do even if it is a breaking change (it would be a shortcut for usingRecursiveFieldByFieldElementComparator( RecursiveComparisonConfiguration.builder().build()).

In addition without calling asList() my example won't compile

That's true, when using usingRecursiveFieldByFieldElementComparator AssertJ enforces the elements type being compared, asList() converts the iterable under test into a List<Object> which explains why the code compile with it.

As a side note, instead of using usingRecursiveFieldByFieldElementComparator and containsExactlyInAnyOrder you can use usingRecursiveComparison with like this:

    assertThat(dtoList).usingRecursiveComparison()
                       .ignoringCollectionOrder()
                       .isEqualTo(list(le1, le2, le3));

[jakubbrodzinski]

Firstly, thank you for the suggestions regarding my AssertJ usage. The reproductive issue was my fault. Before posting my code i wanted to reduce it as much as I could to improve readability. Sadly I think I reduced too much. I edited my original post and put complete example (Only annontations left by me are Getter/Setter from Lombok).
Like I mentioned earlier, only the thrid assertions is failing. When u comment private String randomProperty; and le.setRandomProperty("xyz xyz"); the whole test is green.
I am not really sure what is going on or what i am doing wrong. Since containsExactlyInAnyOrder passes and containsExatcly not, I feel like the issue is on AssertJ's side.

[joel-costigliola]

Thanks! I was able to reproduce the issue.

This is a bug in containsExactly implementation that assumes the comparison of elements is symmetrical but the recursive comparison is not! It compares actual's fields vs expected fields and fails if expected does not have all actual fields (without even needing to compare their values).

In your example, we can compare ListEntryDto to ListEntry because ListEntry has all fields of ListEntryDto (and an additional one: randomProperty) but comparing ListEntry to ListEntryDto will always fail as ListEntryDto does not have a randomProperty field. This explains why commenting randomProperty made the test to succeed.

The best option for you right now is to use:

    assertThat(actualResultList).usingRecursiveComparison()
                                .isEqualTo(list(le1, le2, le3, le4));

which has the benefit to be more elegant - IMO at least ;-)

This bug affects also usingRecursiveFieldByFieldElementComparator(RecursiveComparisonConfiguration) so this does not work either:

   assertThat(actualResultList).asList()
                                .hasSize(4)
                                .usingRecursiveFieldByFieldElementComparator(RecursiveComparisonConfiguration.builder().build())
                                .containsExactly(le1, le2, le3, le4);

[jakubbrodzinski]

Thanks for quick reply, now everything is clear to me. I agree with you about usingRecursiveComparison().isEqualTo(list(...))) being more elegant. Already using it (since your yearlier reply).
Regarding to your first reply, I feel like you are right and usingRecursiveFieldByFieldElementComparator() should be changed even if it's breaking change.