Remove stack trace elements coming from AssertJ in addition to Assert…

…J elements.

Fix #3449

assertj-core/src/main/java/org/assertj/core/util/Throwables.java

@@ -18,12 +18,14 @@
 import static java.util.stream.Collectors.toList;
 import static org.assertj.core.extractor.Extractors.byName;
 import static org.assertj.core.groups.FieldsOrPropertiesExtractor.extract;
+import static org.assertj.core.util.Lists.list;
 import static org.assertj.core.util.Lists.newArrayList;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.lang.reflect.Constructor;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.function.Function;
 import java.util.stream.Stream;
@@ -94,7 +96,10 @@ private static List<StackTraceElement> stackTraceInCurrentThread() {
 
   /**
    * Removes the AssertJ-related elements from the <code>{@link Throwable}</code> stack trace that have little value for
-   * end user. Therefore, instead of seeing this:
+   * end user, that is assertj elements and the ones coming from assertj (for example assertj calling some java jdk
+   * classes to build assertion errors dynamically).
+   * <p>
+   * Therefore, instead of seeing this:
    * <pre><code class='java'> org.junit.ComparisonFailure: expected:&lt;'[Ronaldo]'&gt; but was:&lt;'[Messi]'&gt;
    *   at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    *   at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39)
@@ -111,37 +116,35 @@ private static List<StackTraceElement> stackTraceInCurrentThread() {
    *
    * We get this:
    * <pre><code class='java'> org.junit.ComparisonFailure: expected:&lt;'[Ronaldo]'&gt; but was:&lt;'[Messi]'&gt;
-   *   at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
-   *   at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39)
-   *   at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27)
    *   at examples.StackTraceFilterExample.main(StackTraceFilterExample.java:20)</code></pre>
    * @param throwable the {@code Throwable} to filter stack trace.
    */
   public static void removeAssertJRelatedElementsFromStackTrace(Throwable throwable) {
     if (throwable == null) return;
-    List<StackTraceElement> filtered = newArrayList(throwable.getStackTrace());
-    StackTraceElement previous = null;
+    List<StackTraceElement> filtered = list();
+    boolean noAssertjStackTraceElementFoundYet = true;
+    // ignore assertj elements and the one above assertj (i.e. coming from assertj)
     for (StackTraceElement element : throwable.getStackTrace()) {
       if (element.getClassName().contains(ORG_ASSERTJ)) {
-        filtered.remove(element);
-        // Handle the case when AssertJ builds a ComparisonFailure/AssertionFailedError by reflection
-        // (see ShouldBeEqual.newAssertionError method), the stack trace looks like:
-        //
-        // java.lang.reflect.Constructor.newInstance(Constructor.java:501),
-        // org.assertj.core.error.ConstructorInvoker.newInstance(ConstructorInvoker.java:34),
-        //
-        // We want to remove java.lang.reflect.Constructor.newInstance element because it is related to AssertJ.
-        if (previous != null && JAVA_LANG_REFLECT_CONSTRUCTOR.equals(previous.getClassName())
-            && element.getClassName().contains(ORG_ASSERTJ_CORE_ERROR_CONSTRUCTOR_INVOKER)) {
-          filtered.remove(previous);
-        }
+        noAssertjStackTraceElementFoundYet = false;
+        continue;
       }
-      previous = element;
+      if (noAssertjStackTraceElementFoundYet) continue; // elements above assertj
+      filtered.add(element);
     }
     StackTraceElement[] newStackTrace = filtered.toArray(new StackTraceElement[0]);
     throwable.setStackTrace(newStackTrace);
   }
 
+  private static Collection<StackTraceElement> getElementsBeforeAssertJ(StackTraceElement[] stackTraceElements) {
+    Collection<StackTraceElement> elementsBeforeAssertJ = new ArrayList<>();
+    for (StackTraceElement stackTraceElement : stackTraceElements) {
+      if (stackTraceElement.toString().contains(ORG_ASSERTJ)) break;
+      elementsBeforeAssertJ.add(stackTraceElement);
+    }
+    return elementsBeforeAssertJ;
+  }
+
   /**
    * Get the root cause (i.e., the last non-null cause) from a {@link Throwable}.
    *

assertj-core/src/test/java/org/assertj/core/error/ShouldBeEqual_assertj_elements_stack_trace_filtering_Test.java

@@ -12,9 +12,11 @@
  */
 package org.assertj.core.error;
 
-import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.BDDAssertions.then;
-import static org.assertj.core.util.StackTraceUtils.hasStackTraceElementRelatedToAssertJ;
+import static org.assertj.core.util.AssertionsUtil.expectAssertionError;
+import static org.assertj.core.util.StackTraceUtils.checkNoAssertjStackTraceElementIn;
+import static org.assertj.core.util.StackTraceUtils.hasAssertJStackTraceElement;
 
 import org.assertj.core.api.Fail;
 import org.junit.jupiter.api.Test;
@@ -31,21 +33,19 @@ void assertj_elements_should_be_removed_from_assertion_error_stack_trace() {
     // GIVEN
     Fail.setRemoveAssertJRelatedElementsFromStackTrace(true);
     // WHEN
-    Throwable error = catchThrowable(() -> then("Xavi").isEqualTo("Xabi"));
+    AssertionError error = expectAssertionError(() -> assertThat("Xavi").isEqualTo("Xabi"));
     // THEN
-    then(error).isInstanceOf(AssertionError.class);
-    then(hasStackTraceElementRelatedToAssertJ(error)).isFalse();
+    checkNoAssertjStackTraceElementIn(error);
   }
 
   @Test
   void assertj_elements_should_be_kept_in_assertion_error_stack_trace() {
     // GIVEN
     Fail.setRemoveAssertJRelatedElementsFromStackTrace(false);
     // WHEN
-    Throwable error = catchThrowable(() -> then("Messi").isEqualTo("Ronaldo"));
+    AssertionError error = expectAssertionError(() -> then("Messi").isEqualTo("Ronaldo"));
     // THEN
-    then(error).isInstanceOf(AssertionError.class);
-    then(hasStackTraceElementRelatedToAssertJ(error)).isTrue();
+    then(hasAssertJStackTraceElement(error)).isTrue();
   }
 
 }

assertj-core/src/test/java/org/assertj/core/util/StackTraceUtils.java

@@ -14,6 +14,9 @@
 
 import java.util.Arrays;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.BDDAssertions.then;
+
 public class StackTraceUtils {
 
   /**
@@ -22,8 +25,14 @@ public class StackTraceUtils {
    * @param throwable the {@link Throwable} we want to check stack trace for AssertJ related elements.
    * @return true if given {@link Throwable} stack trace contains AssertJ related elements, false otherwise.
    */
-  public static boolean hasStackTraceElementRelatedToAssertJ(Throwable throwable) {
+  public static boolean hasAssertJStackTraceElement(Throwable throwable) {
     return Arrays.stream(throwable.getStackTrace())
                  .anyMatch(stackTraceElement -> stackTraceElement.getClassName().contains("org.assertj"));
   }
+
+  public static boolean checkNoAssertjStackTraceElementIn(Throwable throwable) {
+    StackTraceElement[] stackTrace = throwable.getStackTrace();
+    then(stackTrace).noneSatisfy(stackTraceElement -> assertThat(stackTraceElement.toString()).contains("org.assertj"));
+  }
+
 }

assertj-core/src/test/java/org/example/custom/CustomAsserts_filter_stacktrace_Test.java

@@ -13,17 +13,42 @@
 package org.example.custom;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.BDDAssertions.then;
+import static org.assertj.core.util.AssertionsUtil.expectAssertionError;
+
+import java.util.stream.Stream;
 
 import org.assertj.core.api.AbstractObjectAssert;
 import org.assertj.core.api.Assertions;
 import org.assertj.core.api.Condition;
+import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
 import org.assertj.core.error.BasicErrorMessageFactory;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 
 public class CustomAsserts_filter_stacktrace_Test {
 
+  public static final String ORG_ASSERTJ = "org.assertj";
+
+  public static Stream<ThrowingCallable> stacktrace_should_not_include_assertj_elements_nor_elements_coming_from_assertj() {
+    return Stream.of(() -> assertThat(0).isEqualTo(1),
+                     () -> assertThat(0).satisfies(x -> assertThat(x).isEqualTo(1)));
+  }
+
+  @ParameterizedTest
+  @MethodSource
+  void stacktrace_should_not_include_assertj_elements_nor_elements_coming_from_assertj(ThrowingCallable throwingCallable) {
+    // WHEN
+    AssertionError assertionError = expectAssertionError(throwingCallable);
+    // THEN
+    StackTraceElement[] stackTrace = assertionError.getStackTrace();
+    then(stackTrace).noneSatisfy(stackTraceElement -> assertThat(stackTraceElement.toString()).contains(ORG_ASSERTJ));
+    then(stackTrace[0].toString()).contains("CustomAsserts_filter_stacktrace_Test");
+  }
+
   @Test
   public void should_filter_when_custom_assert_fails_with_message() {
     try {

assertj-core/src/test/java/org/example/test/Remove_assertJ_stacktrace_elements_Test.java

@@ -0,0 +1,63 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * Copyright 2012-2024 the original author or authors.
+ */
+package org.example.test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.setRemoveAssertJRelatedElementsFromStackTrace;
+import static org.assertj.core.api.BDDAssertions.then;
+import static org.assertj.core.util.AssertionsUtil.expectAssertionError;
+import static org.assertj.core.util.StackTraceUtils.checkNoAssertjStackTraceElementIn;
+
+import java.util.stream.Stream;
+
+import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
+import org.assertj.core.internal.Failures;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class Remove_assertJ_stacktrace_elements_Test {
+
+  private boolean initialRemoveAssertJRelatedElementsFromStackTraceValue;
+
+  @BeforeEach
+  public void beforeTest() {
+    initialRemoveAssertJRelatedElementsFromStackTraceValue = Failures.instance().isRemoveAssertJRelatedElementsFromStackTrace();
+    setRemoveAssertJRelatedElementsFromStackTrace(true);
+  }
+
+  @AfterEach
+  public void afterTest() {
+    setRemoveAssertJRelatedElementsFromStackTrace(initialRemoveAssertJRelatedElementsFromStackTraceValue);
+  }
+
+  @ParameterizedTest
+  @MethodSource
+  void stacktrace_should_not_include_assertj_elements_nor_elements_coming_from_assertj(ThrowingCallable throwingCallable) {
+    // WHEN
+    AssertionError assertionError = expectAssertionError(throwingCallable);
+    // THEN
+    checkNoAssertjStackTraceElementIn(assertionError);
+    // since we remove assertj elements, there is no easy way to check we have removed elements before/above assertj
+    // -> we check that the first element is the test class itself.
+    then(assertionError.getStackTrace()[0].toString()).contains("Remove_assertJ_stacktrace_elements_Test");
+
+  }
+
+  static Stream<ThrowingCallable> stacktrace_should_not_include_assertj_elements_nor_elements_coming_from_assertj() {
+    return Stream.of(() -> assertThat(0).isEqualTo(1),
+                     () -> assertThat(0).satisfies(x -> assertThat(x).isEqualTo(1)));
+  }
+
+}

assertj-core/src/test/java/org/assertj/core/util/Throwables_removeAssertJElementFromStackTrace_Test.java → assertj-core/src/test/java/org/example/test/Throwables_removeAssertJElementFromStackTrace_Test.java

@@ -10,11 +10,15 @@
  *
  * Copyright 2012-2024 the original author or authors.
  */
-package org.assertj.core.util;
+package org.example.test;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.util.StackTraceUtils.hasStackTraceElementRelatedToAssertJ;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.assertj.core.util.StackTraceUtils.checkNoAssertjStackTraceElementIn;
+import static org.assertj.core.util.StackTraceUtils.hasAssertJStackTraceElement;
+import static org.assertj.core.util.Throwables.removeAssertJRelatedElementsFromStackTrace;
 
+import org.assertj.core.util.Throwables;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -23,15 +27,21 @@
  * @author Joel Costigliola
  */
 class Throwables_removeAssertJElementFromStackTrace_Test {
+
   @Test
   void should_add_stack_trace_of_current_thread() {
-    try {
-      throw new AssertJThrowable();
-    } catch (AssertJThrowable throwable) {
-      assertThat(hasStackTraceElementRelatedToAssertJ(throwable)).isTrue();
-      Throwables.removeAssertJRelatedElementsFromStackTrace(throwable);
-      assertThat(hasStackTraceElementRelatedToAssertJ(throwable)).isFalse();
-    }
+    // GIVEN
+    Throwable throwable = catchThrowable(this::throwAssertJThrowable);
+    // THEN
+    assertThat(hasAssertJStackTraceElement(throwable)).isTrue();
+    // WHEN
+    removeAssertJRelatedElementsFromStackTrace(throwable);
+    // THEN
+    checkNoAssertjStackTraceElementIn(throwable);
+  }
+
+  private void throwAssertJThrowable() throws AssertJThrowable {
+    throw new AssertJThrowable();
   }
 
   private static class AssertJThrowable extends Throwable {