`removeAssertJRelatedElementsFromStackTrace` does not remove all the JDK elements triggered by AssertJ

[almondtools]

`removeAssertJRelatedElementsFromStackTrace` does not remove all the JDK elements triggered by AssertJ

[joel-costigliola]

I completely agree to remove the stack before the user's code as the aim is to clear the clutter from the stack.

Let's do it for 3.26.0

[almondtools]

Also check to keep the handling of this code consistent

	@Test
	void testStacktrace2() {
		assertThat(0)
			.satisfies(x -> assertThat(x).isEqualTo(1));
	}

Here the superfluous lines from the example above appear in the middle of the stack trace. I have no preference how to handle this, but just wanted to point out that this test is similar but different.

[scordio]

because junit does report all internal stacktrace items, so why should assertj do not

Starting from 5.10.0, JUnit has:

Stacktrace pruning to hide internal JUnit calls

[scordio]

Good catch, @almondtools!

I would also consider the bug quite obvious but yours seems to be the first feedback on a code that never changed since the forking from Fest Assert (66f11f8) 🙂

According to the Javadoc example of removeAssertJRelatedElementsFromStackTrace, pruning all the stack trace elements triggered by AssertJ was never intended. Only the one right before the first AssertJ method is supposed to be removed.

If you would run the following:

@Test
void testStacktrace() {
	Assertions.setRemoveAssertJRelatedElementsFromStackTrace(false);
	assertThat(0).isEqualTo(1);
}

you would see how the stack trace really looks like:

...
kept -->	at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)
kept -->	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502)
removed -->	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:486)
removed -->	at org.assertj.core.error.ConstructorInvoker.newInstance(ConstructorInvoker.java:28)
removed -->	at org.assertj.core.error.ShouldBeEqual.assertionFailedError(ShouldBeEqual.java:223)
...

I believe any element that is a consequence of org.assertj.core.error.ConstructorInvoker.newInstance is not valuable for the users and should be pruned.

Referring to the original example, what users should see is only:

org.opentest4j.AssertionFailedError: 
expected: 1
 but was: 0
	at net.amygdalum.WorldTest.testStacktrace(WorldTest.java:13)
	at ...

@joel-costigliola how do you see it?

---

How it currently looks without and with the removal of AssertJ elements:

org.opentest4j.AssertionFailedError: 
expected: 1
 but was: 0

[scordio]

Also check to keep the handling of this code consistent

	@Test
	void testStacktrace2() {
		assertThat(0)
			.satisfies(x -> assertThat(x).isEqualTo(1));
	}

Here the superfluous lines from the example above appear in the middle of the stack trace. I have no preference how to handle this, but just wanted to point out that this test is similar but different.

How it currently looks without and with the removal of AssertJ elements:

org.opentest4j.AssertionFailedError: 
expected: 1
 but was: 0

org.assertj.core.error.AssertJMultipleFailuresError: 
Multiple Failures (1 failure)
-- failure 1 --
expected: 1
 but was: 0
at AssertJTest.lambda$testStacktrace2$2(AssertJTest.java:82)

[almondtools]

The lines prefixed with org.assertj.core.tests should be visible. This is probably the case if the test is not in an assertj package, so no reason to worry about.

Yet, I think both stacktraces are not optimal. I think there are two "correct" views on this stacktrace:

1. one where only the two lines prefixed with org.assertj.core.tests and the stack before is shown (hide infix artifacts and suffix artifacts)
2. one where only the lines after the last line prefixed with org.assertj.core.tests are hidden (hide only suffix artifacts)

And "artifacts" are not only code prefixed with org.assertj but also code triggered through methods of assertj.

1. because the user coded only these lines, others are artifacts from assertj
2. because junit does report all internal stacktrace items, so why should assertj do not. Assuming the user expects the last line in the stacktrace to contain the error location the suffix artifacts should be truncated

[scordio]

🤦 I swear I knew I should execute the examples in a separate package to avoid such cases, and still... screenshots updated.

[joel-costigliola]

I think we should remove all stack trace elements starting from the first assertj element.

Example 1:

assertThat(0).isEqualTo(1);

instead of

org.opentest4j.AssertionFailedError: 
expected: 1
 but was: 0
Expected :1
Actual   :0
<Click to see difference>


	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at org.assertj.core.error.ConstructorInvoker.newInstance(ConstructorInvoker.java:28)
	at org.assertj.core.error.ShouldBeEqual.assertionFailedError(ShouldBeEqual.java:223)
	at org.assertj.core.error.ShouldBeEqual.newAssertionError(ShouldBeEqual.java:122)
	at org.assertj.core.internal.Failures.failure(Failures.java:105)
	at org.assertj.core.internal.Comparables.assertEqual(Comparables.java:119)
	at org.assertj.core.api.AbstractIntegerAssert.isEqualTo(AbstractIntegerAssert.java:69)
	at org.example.custom.CustomAsserts_filter_stacktrace_Test.stacktrace_should_not_show_elements_coming_from_assertj(CustomAsserts_filter_stacktrace_Test.java:30)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

we would have

org.opentest4j.AssertionFailedError: 
expected: 1
 but was: 0
Expected :1
Actual   :0
<Click to see difference>

	at org.example.custom.CustomAsserts_filter_stacktrace_Test.stacktrace_should_not_show_elements_coming_from_assertj(CustomAsserts_filter_stacktrace_Test.java:30)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Note: the extra Expected :1 Actual :0 <Click to see difference> comes from Idea outpur

Example 2:

assertThat(0).satisfies(x -> assertThat(x).isEqualTo(1));

instead of

org.assertj.core.error.AssertJMultipleFailuresError: 
Multiple Failures (1 failure)
-- failure 1 --
expected: 1
 but was: 0
at CustomAsserts_filter_stacktrace_Test.lambda$testStacktrace2$0(CustomAsserts_filter_stacktrace_Test.java:36)

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at org.assertj.core.error.ConstructorInvoker.newInstance(ConstructorInvoker.java:28)
	at org.assertj.core.error.AssertionErrorCreator.tryBuildingMultipleFailuresError(AssertionErrorCreator.java:137)
	at org.assertj.core.error.AssertionErrorCreator.multipleAssertionsError(AssertionErrorCreator.java:104)
	at org.assertj.core.api.AbstractAssert.multipleAssertionsError(AbstractAssert.java:1005)
	at org.assertj.core.api.AbstractAssert.satisfiesForProxy(AbstractAssert.java:901)
	at org.assertj.core.api.AbstractAssert.satisfies(AbstractAssert.java:888)
	at org.example.custom.CustomAsserts_filter_stacktrace_Test.testStacktrace2(CustomAsserts_filter_stacktrace_Test.java:36)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)

we would have

org.assertj.core.error.AssertJMultipleFailuresError: 
Multiple Failures (1 failure)
-- failure 1 --
expected: 1
 but was: 0
at CustomAsserts_filter_stacktrace_Test.lambda$testStacktrace2$0(CustomAsserts_filter_stacktrace_Test.java:36)

	at org.example.custom.CustomAsserts_filter_stacktrace_Test.testStacktrace2(CustomAsserts_filter_stacktrace_Test.java:36)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)

[almondtools]

I think we should remove all stack trace elements starting from the first assertj element.

Look at your last example:

org.assertj.core.error.AssertJMultipleFailuresError: 
Multiple Failures (1 failure)
-- failure 1 --
expected: 1
 but was: 0
at CustomAsserts_filter_stacktrace_Test.lambda$testStacktrace2$0(CustomAsserts_filter_stacktrace_Test.java:36) <--

	at org.example.custom.CustomAsserts_filter_stacktrace_Test.testStacktrace2(CustomAsserts_filter_stacktrace_Test.java:36) <--
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)

You did not remove all stack trace elements starting from the first assertj element. You removed all stack trace elements between the user code entries and after the user code entry. My Alternative 1.

This would be more obvious if you put the second assert on a new line like this:

assertThat(0).satisfies(x -> {
  assertThat(x).isEqualTo(1);
});

[joel-costigliola]

I'm not sure I understand what you mean @almondtools, here's the test output with your code snippet

If you expand the folded elements you indeed have non assertj elements between assertj elements but the goal of this feature was to easily find the line of the test where the assertion fails and that is I think the case.

Could you show on an example what you would like to have (I did not really understand when you talked about infix and suffix artefacts, what does that mean concretely ?)

og.opentest4j.AssertionFailedError: 
expected: 1
 but was: 0
Expected :1
Actual   :0
<Click to see difference>


	at org.example.test.Remove_assertJ_stacktrace_elements_Test.lambda$should_$3(Remove_assertJ_stacktrace_elements_Test.java:67)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:992)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
	at org.example.test.Remove_assertJ_stacktrace_elements_Test.should_(Remove_assertJ_stacktrace_elements_Test.java:66)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:728)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:92)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:86)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:218)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:214)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:139)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:69)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:198)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:169)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:93)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:58)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:141)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:57)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:103)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:85)
	at org.junit.platform.launcher.core.DelegatingLauncher.execute(DelegatingLauncher.java:47)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:63)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:57)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:232)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:55)

[almondtools]

Maybe it is just the fact that I read your statement to literal,

In your stacktrace you find two (!) lines concerning the test :

at org.example.test.Remove_assertJ_stacktrace_elements_Test.lambda$should_$3(Remove_assertJ_stacktrace_elements_Test.java:67)
...
at org.example.test.Remove_assertJ_stacktrace_elements_Test.should_(Remove_assertJ_stacktrace_elements_Test.java:66)
...

one at line 67 (satisfies), one at line 68 (isEqualTo). In your proposal you picked both (fine by me).

You described this as:

I think we should remove all stack trace elements starting from the first assertj element.

But you kept the user-code line for isEqualTo which literally is a stacktrace element after the first assertj element. I just wanted to clarify it.

Concerning suffix artifacts and infix artifacts - maybe this is a wrong translation of my thoughts. I mean the following:

In the general case we can have following types of stackelements to be removed

• statements between two user-code statements (e.g. between satisfies and isEqualTo) -> infix lines
• statements after the last user-code statement -> suffix lines

In your last stacktrace there are no suffix lines - but in mine there were. I vote for removing both, but keeping the user-code statements. In the concrete case we would find a stracktrace like this:

at org.example.test.Remove_assertJ_stacktrace_elements_Test.lambda$should_$3(Remove_assertJ_stacktrace_elements_Test.java:67)
at org.example.test.Remove_assertJ_stacktrace_elements_Test.should_(Remove_assertJ_stacktrace_elements_Test.java:66)
... (stacktrace from the testing framework)

[joel-costigliola]

I'll give a try to remove also assertj lines between user test code (infix as you call them), they are not relevant to understand the stack trace.