Assertj-core: SoftAssertions nested in satisfies() or allSatisfy() only output first failing assertion

[maverick1601]

Summary

When using SoftAssertions together with satisfies() or allSatisfy() the additional assertions made in the consumer supplied to these methods are handled differently. In fact, the first failing assertion prevents other in the same consumer to be reported, as expected.

Example

public class MyTest {

  @Test
  public void softAssertionTest() {
    String str = "hello";

    SoftAssertions.assertSoftly(sa -> {
      sa.assertThat(str).satisfies(s -> {

        // fails
        sa.assertThat(s).hasSize(2);

        // not reported, unless all previous assertions succeed
        sa.assertThat(s).isEmpty();
      });
    });
  }
}

[joel-costigliola]

This should work, thanks for reporting this.

[maverick1601]

As far as I understood the code, it is a problem regarding the SoftProxies and nesting with respect to the ErrorCollector. The nested satisfy block seems to let errors propagate, which results in the first assertion (failing) to prevent follow up assertions to even execute.

[joel-costigliola]

It seems the problems comes from the way AssertJ handles nested proxy calls.

Example:
softly.assertThat(true).isFalse() calls the isEqualTo(false) which is also proxied, in that case the assertion error is bubbled up. It looks like in this issue the assertion error should simply be recorded and not bubbled up .

We count the nested level by count of ErrorCollector.intercept which happens with satisfies and the assertions inside (hasSize).

org.eclipse.jdt.internal.junit.runner.RemoteTestRunner at localhost:36031   
    Thread [main] (Suspended)   
        ErrorCollector.intercept(ErrorCollector, Object, Callable<?>, Method, Object) line: 64  
        StringAssert$ByteBuddy$ZwZnwlBB.hasSize(int) line: not available    
        StringAssert$ByteBuddy$ZwZnwlBB.hasSize(int) line: not available    
        SoftAssertionsErrorDescriptionTest.lambda$2(SoftAssertions, String) line: 51    
        2052223881.accept(Object) line: not available   
        StringAssert$ByteBuddy$ZwZnwlBB(AbstractAssert<SELF,ACTUAL>).satisfies(Consumer<ACTUAL>) line: 669  
        StringAssert$ByteBuddy$ZwZnwlBB.satisfies$accessor$Qnd4MFMu(Consumer) line: not available   
        StringAssert$ByteBuddy$ZwZnwlBB$AssertJ$SoftProxies$Im2qiR2u.call() line: not available 
        ErrorCollector.intercept(ErrorCollector, Object, Callable<?>, Method, Object) line: 58  
        StringAssert$ByteBuddy$ZwZnwlBB.satisfies(Consumer<String>) line: not available 
        StringAssert$ByteBuddy$ZwZnwlBB.satisfies(Consumer) line: not available 
        SoftAssertionsErrorDescriptionTest.lambda$1(String, SoftAssertions) line: 54    
        1836463382.accept(Object) line: not available   
        SoftAssertions.assertSoftly(Consumer<SoftAssertions>) line: 161 
        SoftAssertionsErrorDescriptionTest.softAssertionTest() line: 47 
        NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method]  
        NativeMethodAccessorImpl.invoke(Object, Object[]) line: 62  
        DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 43  
        Method.invoke(Object, Object...) line: 498  
        ReflectionUtils.invokeMethod(Method, Object, Object...) line: 515   
        ExecutableInvoker.invoke(Method, Object, ExtensionContext, ExtensionRegistry) line: 115 
        TestMethodTestDescriptor.lambda$invokeTestMethod$6(ExtensionContext, JupiterEngineExecutionContext) line: 171   
        106374177.execute() line: not available 
        OpenTest4JAwareThrowableCollector(ThrowableCollector).execute(ThrowableCollector$Executable) line: 72   
        TestMethodTestDescriptor.invokeTestMethod(JupiterEngineExecutionContext, Node$DynamicTestExecutor) line: 167    
        TestMethodTestDescriptor.execute(JupiterEngineExecutionContext, Node$DynamicTestExecutor) line: 114 
        TestMethodTestDescriptor.execute(EngineExecutionContext, Node$DynamicTestExecutor) line: 59 
        NodeTestTask<C>.lambda$executeRecursively$5() line: 105 
        957465255.execute() line: not available 
        OpenTest4JAwareThrowableCollector(ThrowableCollector).execute(ThrowableCollector$Executable) line: 72   
        NodeTestTask<C>.executeRecursively() line: 95   
        NodeTestTask<C>.execute() line: 71  
        367746789.accept(Object) line: not available    
        ArrayList<E>.forEach(Consumer<? super E>) line: 1257    
        SameThreadHierarchicalTestExecutorService.invokeAll(List<TestTask>) line: 38    
        NodeTestTask<C>.lambda$executeRecursively$5() line: 110 
        957465255.execute() line: not available 
        OpenTest4JAwareThrowableCollector(ThrowableCollector).execute(ThrowableCollector$Executable) line: 72   
        NodeTestTask<C>.executeRecursively() line: 95   
        NodeTestTask<C>.execute() line: 71  
        367746789.accept(Object) line: not available    
        ArrayList<E>.forEach(Consumer<? super E>) line: 1257    
        SameThreadHierarchicalTestExecutorService.invokeAll(List<TestTask>) line: 38    
        NodeTestTask<C>.lambda$executeRecursively$5() line: 110 
        957465255.execute() line: not available 
        OpenTest4JAwareThrowableCollector(ThrowableCollector).execute(ThrowableCollector$Executable) line: 72   
        NodeTestTask<C>.executeRecursively() line: 95   
        NodeTestTask<C>.execute() line: 71  
        SameThreadHierarchicalTestExecutorService.submit(TestTask) line: 32 
        HierarchicalTestExecutor<C>.execute() line: 57  
        JupiterTestEngine(HierarchicalTestEngine<C>).execute(ExecutionRequest) line: 51 
        DefaultLauncher.execute(TestEngine, ExecutionRequest) line: 170 
        DefaultLauncher.execute(Root, ConfigurationParameters, TestExecutionListener...) line: 154  
        DefaultLauncher.execute(LauncherDiscoveryRequest, TestExecutionListener...) line: 90    
        JUnit5TestReference.run(TestExecution) line: 89 
        TestExecution.run(ITestReference[]) line: 41    
        RemoteTestRunner.runTests(String[], String, TestExecution) line: 541    
        RemoteTestRunner.runTests(TestExecution) line: 763  
        RemoteTestRunner.run() line: 463    
        RemoteTestRunner.main(String[]) line: 209   
    Thread [ReaderThread] (Running) 

[joel-costigliola]

You beat me by 2 min! :D

[joel-costigliola]

One workaround is simply not to use sa to call satisfies:

 SoftAssertions.assertSoftly(sa -> {
      // don't call with like sa.assertThat.satisfies( ... 
      assertThat(str).satisfies(s -> {

        // fails
        sa.assertThat(s).hasSize(2);

        // not reported, unless all previous assertions succeed
        sa.assertThat(s).isEmpty();
      });
    });

[maverick1601]

I know that this is a workaround, but the example given was greatly simplified. This would also affect not using allSatisfy(), I assume, which i do use very often.

[joel-costigliola]

I don't see an easy fix for this issue, I'll try to explain why.

Some of the assertions internally use others like isNotNull() as in:

  public SELF contains(VALUE expectedValue) {
    isNotNull(); // check actual is not null
    if (!actual.isPresent()) throwAssertionError(shouldContain(expectedValue));
    ...

Soft assertion are implemented by proxying assertions and catching any AssertionErrror and report all of them later on.
In the example above, contains and isNotNull() are proxied, if isNotNull() fails the thrown AssertionErrror will be collected and the next line of code executed which will result in a NPE, not what we want!
So what AssertJ does is to propagate the error to contains and let the proxy mechanism to collect it (there is one collector per soft assertion instance). That's better.
Oh and to do that, it counts the number of calls to ErrorCollector.intercept from the stack trace, if > 1 it will see an internal call and propagate the error (in ErrorCollector).

Now in your example, you are nesting soft assertions so the the number of calls to ErrorCollector.intercept will be > 1 which leads AssertJ to think that it is in a situation like contains calling isNotNull(), it propagates the AssertionError and this is why the next assertion is not performed.

To fix that, we would need to know that the nesting of proxied assertions was intended by the user and not an internal implementation, I'm not sure this is quite possible but I need to give it more time, there might be a solution that I don't see at the moment.

[maverick1601]

Isn't the "nesting" limited to calling satisfies() or allSatisfy(), so we could handle those two differently? Let alone the fact, that we could have deeper nesting as well, for instances satisfy() within satisfy().

It's not that we have a major issue with the current behavior. As long as all assertions succeed, we are sure the test cases are covered. Mainly, the issue comes up during refactorings, if many assertions should fail at once, which is why we use SoftAssertions in the first place.

[joel-costigliola]

Maybe but it should be handled only if coming from a soft assertion ... need trying/investigating.

I was also thinking that it might be possible to differentiate internal AssertJ calls from users ones by looking the stack trace to see if the caller of intercept was made from org.assertj or not.

This issue is a minor bugfix since AssertJ does not lead to false positive, as you said if the assertions succeed the test case is covered, and the test would fail if any assertion fails.