Assertj-core: Consider possibility of using soft assertions for zipSatisfy

[MikeShysh]

Summary

version 3.9.1

If we compare lists by pairs, we want to check all fails. Currently, zipSatisfy assertion fails by the very first inconsistency

Example

val list1 = listOf(
                tariffInfo(name= "test1", iCustomer = 12, iCClass = 23),
                tariffInfo(name= "test2", iCustomer = 13, iCClass = 45),
                tariffInfo(name= "test3", iCustomer = 14, iCClass = 65)
        )

        val list2 = listOf(
                tariffInfo(name= "test1", iCustomer = 13, iCClass = 99),
                tariffInfo(name= "test2", iCustomer = 13, iCClass = 99),
                tariffInfo(name= "test3", iCustomer = 15, iCClass = 99)
        )

assertThat(list1).zipSatisfy(list2,
                { resp: tariffInfo, refResp: tariffInfo ->
                    assertThat(resp).isEqualToIgnoringGivenFields(refResp,
                            "iCClass")
                })

Here is an assert

ava.lang.AssertionError: 
Expecting zipped elements of:
  <[tariffInfo(name=test1, iCustomer=12 <<<, iCClass=23),
    tariffInfo(name=test2, iCustomer=13, iCClass=45),
    tariffInfo(name=test3, iCustomer=14 <<<, iCClass=65)]>
and:
  <[tariffInfo(name=test1, iCustomer=13 <<<, iCClass=99),
    tariffInfo(name=test2, iCustomer=13, iCClass=99),
    tariffInfo(name=test3, iCustomer=15 <<<, iCClass=99)]>
to satisfy given requirements, but this pair of elements did not:
  <(tariffInfo(name=test1, iCustomer=12, iCClass=23), tariffInfo(name=test1, iCustomer=13, iCClass=99))> 
Reason: "
Expecting value <13> in field <"iCustomer"> but was <12> in <tariffInfo(name=test1, iCustomer=12, iCClass=23)>.
Comparison was performed on all fields but <["iCClass"]>"

As we can see the last inconsistency was not checked in the assertion error.

Please consider this case. Thank you.

[joel-costigliola]

Good catch! Will improve this in the next version.

[joel-costigliola]

@MikeShysh I have investigated this, the behavior from your test is normal, zipSatisfy fails at the first error, soft assertions will just collect that error and report when executing assertAll().

What could have worked (but does not) would be to have used softly inside zipSatisfy like that:

softly.assertThat(resp).isEqualToIgnoringGivenFields(refResp, "iCClass");

The reason is that we rethrow soft assertions nested call error (i.e. softly.assertThat(resp)) up to the main level (that is assertThat(list1).zipSatisfy(list2, ...).

Why ? Let's take AbstractOptionalAssert.contains implementation as an example:

  public SELF contains(VALUE expectedValue) {
    isNotNull();
    if (!actual.isPresent()) throwAssertionError(shouldContain(expectedValue));
    if (!optionalValueComparisonStrategy.areEqual(actual.get(), expectedValue))
      throwAssertionError(shouldContain(actual, expectedValue));
    return myself;
  }

If we don't propagate potential errors in isNotNull() then we will be executing if (!actual.isPresent()) with a null actual, this is not the correct behavior, we should have stopped the assertion.

There is a solution to your suggestion though, the real improvement here is not about soft assertions but to change zipSatisfy so that it collects all errors instead of failing at the first one.

[MikeShysh]

@joel-costigliola Thank you for your explanations.

change zipSatisfy so that it collects all errors instead of failing at the first one.

Yeah, it's even better.