satisfiesExactlyInAnyOrder fails if actual overrides equals

[PBoddington]

equals and hashCode are used in the implementation of satisfiesExactlyInAnyOrder (when they definitely aren't needed). This can result in incorrect assertion results.

• assertj core version: 3.25.1
• java version: 17
• test framework version: n/a
• os (if relevant):

Example of the issue:

public class MyTest {

  @Test
  void shouldPassButFails() {
    StringAndInt first = new StringAndInt("X", 1);
    StringAndInt second = new StringAndInt("X", 2);
    assertThat(List.of(first, second)).satisfiesExactlyInAnyOrder(
        stringAndInt -> assertThat(stringAndInt.integer).isEqualTo(2),
        stringAndInt -> assertThat(stringAndInt.integer).isEqualTo(1)
    );
  }

  private static final class StringAndInt {
    private final String string;
    private final int integer;

    private StringAndInt(
        String string,
        int integer
    ) {
      this.string = string;
      this.integer = integer;
    }

    @Override
    public int hashCode() {
      return string.hashCode();
    }

    @Override
    public boolean equals(Object o) {
      return (o instanceof StringAndInt other) && other.string.equals(string);
    }

    @Override
    public String toString() {
      return "StringAndInt{" +
          "string='" + string + '\'' +
          ", integer=" + integer +
          '}';
    }
  }
}

This test fails with

Expecting actual:
  [StringAndInt{string='X', integer=1}, StringAndInt{string='X', integer=2}]
to satisfy all the consumers in any order.

but should pass.

The problem is that the use of List::remove in the implementation results in the wrong element being removed.

In general we believe there are a number of bugs in assertj due to unnecessary usages of equals and hashCode, and think that the best way to weed them out is to have unit tests using classes where equals and hashCode throw exceptions.

[scordio]

Thank you for reporting it, @PBoddington!

[scordio]

In general we believe there are a number of bugs in assertj due to unnecessary usages of equals and hashCode, and think that the best way to weed them out is to have unit tests using classes where equals and hashCode throw exceptions.

Do you have any specific examples in mind?

[PBoddington]

Thanks for fixing so quickly!

In general we believe there are a number of bugs in assertj due to unnecessary usages of equals and hashCode, and think that the best way to weed them out is to have unit tests using classes where equals and hashCode throw exceptions.

Do you have any specific examples in mind?

One other thing was #3340 but it's hard to give other specific examples right now. This came about because as an experiment to see if people in our org were writing code relying on undocumented equals/hashCode behaviour of certain core classes, we changed the implementations to simply throw exceptions to see what tests failed. Many of the exceptions we saw were coming from assertj itself. The reason it's so difficult to say whether these are bugs or not is that it would not be a bug in itself for assertj to be calling equals/hashCode on the arguments passed. I'd have to go through the exceptions and find if equals/hashCode was being used correctly. In this particular case I was able to see quite quickly that it was a bug, but other exceptions require more analysis. I'll create more issues if I do find any more.

[scordio]

Thanks, Paul!

@joel-costigliola is already looking at #3340. I couldn't spot any other obvious case where the same mistake happened but we'll keep an eye on it.