Improve removing assertj stack trace related elements.

Assertj stack trace elements were already removed but not the elements coming indirectly from assertj, for example java.lang.reflect.Constructor.newInstance when an assertion error is built dynamically.

Fix #3449

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

@@ -14,6 +14,7 @@
 
 import static java.lang.String.format;
 import static java.util.Arrays.stream;
+import static java.util.Collections.reverse;
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 import static org.assertj.core.extractor.Extractors.byName;
@@ -25,7 +26,6 @@
 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;
@@ -39,9 +39,10 @@
  * @author Daniel Zlotin
  */
 public final class Throwables {
-  private static final String ORG_ASSERTJ_CORE_ERROR_CONSTRUCTOR_INVOKER = "org.assertj.core.error.ConstructorInvoker";
-  private static final String JAVA_LANG_REFLECT_CONSTRUCTOR = "java.lang.reflect.Constructor";
+
   private static final String ORG_ASSERTJ = "org.assert";
+  private static final String JAVA_BASE = "java.";
+  private static final String JDK_BASE = "jdk.";
 
   private Throwables() {}
 
@@ -121,28 +122,36 @@ private static List<StackTraceElement> stackTraceInCurrentThread() {
    */
   public static void removeAssertJRelatedElementsFromStackTrace(Throwable throwable) {
     if (throwable == null) return;
-    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)) {
-        noAssertjStackTraceElementFoundYet = false;
-        continue;
+    List<StackTraceElement> purgedStack = list();
+    boolean firstAssertjStackTraceElementFound = false;
+    StackTraceElement[] stackTrace = throwable.getStackTrace();
+    // traverse stack from the root element (main program) as it makes it easier to identify the first assertj element
+    // then we ignore all assertj and java or jdk elements.
+    for (int i = stackTrace.length - 1; i >= 0; i--) {
+      StackTraceElement stackTraceElement = stackTrace[i];
+      if (isFromAssertJ(stackTraceElement)) {
+        firstAssertjStackTraceElementFound = true;
+        continue; // skip element
+      }
+      if (!firstAssertjStackTraceElementFound) {
+        // keep everything before first assertj stack trace element
+        purgedStack.add(stackTraceElement);
+      } else {
+        // we already are in assertj stack, so now we also ignore java elements too as they come from assertj
+        if (!isFromJavaOrJdkPackages(stackTraceElement)) purgedStack.add(stackTraceElement);
       }
-      if (noAssertjStackTraceElementFoundYet) continue; // elements above assertj
-      filtered.add(element);
     }
-    StackTraceElement[] newStackTrace = filtered.toArray(new StackTraceElement[0]);
-    throwable.setStackTrace(newStackTrace);
+    reverse(purgedStack); // reverse as we traversed the stack in reverse order when purging it.
+    throwable.setStackTrace(purgedStack.toArray(new StackTraceElement[0]));
   }
 
-  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;
+  private static boolean isFromAssertJ(StackTraceElement stackTrace) {
+    return stackTrace.getClassName().contains(ORG_ASSERTJ);
+  }
+
+  private static boolean isFromJavaOrJdkPackages(StackTraceElement stackTrace) {
+    String className = stackTrace.getClassName();
+    return className.contains(JAVA_BASE) || className.contains(JDK_BASE);
   }
 
   /**

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

@@ -30,7 +30,7 @@ public static boolean hasAssertJStackTraceElement(Throwable throwable) {
                  .anyMatch(stackTraceElement -> stackTraceElement.getClassName().contains("org.assertj"));
   }
 
-  public static boolean checkNoAssertjStackTraceElementIn(Throwable throwable) {
+  public static void checkNoAssertjStackTraceElementIn(Throwable throwable) {
     StackTraceElement[] stackTrace = throwable.getStackTrace();
     then(stackTrace).noneSatisfy(stackTraceElement -> assertThat(stackTraceElement.toString()).contains("org.assertj"));
   }

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

@@ -49,15 +49,32 @@ void stacktrace_should_not_include_assertj_elements_nor_elements_coming_from_ass
     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");
+    checkTestClassStackTraceElementsAreConsecutive(assertionError);
+  }
+
+  private static void checkTestClassStackTraceElementsAreConsecutive(AssertionError assertionError) {
+    // as we removed assertj related elements, first element is the test class and there should not be elements
+    // between the test class elements themselves, i.e. the indexes of the test class elements should be consecutive
+    String testClassName = Remove_assertJ_stacktrace_elements_Test.class.getName();
+    StackTraceElement[] stackTrace = assertionError.getStackTrace();
+    then(stackTrace[0].getClassName()).contains(testClassName);
+    int lastTestClassStackTraceElementIndex = 0;
+    int i = 1; // 0 index is already checked
+    while (i < stackTrace.length) {
+      if (stackTrace[i].getClassName().contains(testClassName))
+        then(i).isEqualTo(lastTestClassStackTraceElementIndex + 1);
+      i++;
+    }
 
   }
 
   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)));
+                     () -> assertThat(0).satisfies(x -> assertThat(x).isEqualTo(1)),
+                     () -> assertThat(0).satisfies(x -> {
+                       assertThat(0).satisfies(y -> {
+                         assertThat(2).isEqualTo(1);
+                       });
+                     }));
   }
-
 }