Clean Runner and Statement stack frames.

Before and after examples: Runner: at com.google.something.SomeTest.someMethod(SomeTest.java:77) at [[Reflective call: 4 frames collapsed (https://goo.gl/aH3UyP) at [[Testing framework: 8 frames collapsed (https://goo.gl/aH3UyP) at junitparams.JUnitParamsRunner.runChild(JUnitParamsRunner.java:421) at junitparams.JUnitParamsRunner.runChild(JUnitParamsRunner.java:386) => at com.google.something.SomeTest.someMethod(SomeTest.java:77) Statement: at com.google.common.truth.ObjectArraySubjectTest.isEqualTo_Same(ObjectArraySubjectTest.java:46) at [[Reflective call: 4 frames collapsed (https://goo.gl/aH3UyP) at [[Testing framework: 4 frames collapsed (https://goo.gl/aH3UyP) at com.google.common.truth.ExpectFailure$3.evaluate(ExpectFailure.java:208) => at com.google.common.truth.ObjectArraySubjectTest.isEqualTo_Same(ObjectArraySubjectTest.java:46) To avoid referring to Runner and Statement from GWT-compatible code, make StackTraceCleaner GWT-incompatible (since it already bails out immediately when called from GWT), and make callers use the new Platform.cleanStackTrace, implemented as a no-op in the GWT version of the class. RELNOTES=Added code to remove Runner and Statement frames from the stack trace. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=191109334
google · Apr 4, 2018 · d2bb074 · d2bb074
1 parent 4a51035
commit d2bb074
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 72 deletions.
diff --git a/core/src/main/java/com/google/common/truth/FailureMetadata.java b/core/src/main/java/com/google/common/truth/FailureMetadata.java
@@ -22,7 +22,7 @@
 import static com.google.common.base.Verify.verifyNotNull;
 import static com.google.common.truth.Fact.fact;
 import static com.google.common.truth.LazyMessage.evaluateAll;
-import static com.google.common.truth.StackTraceCleaner.cleanStackTrace;
+import static com.google.common.truth.Platform.cleanStackTrace;
 import static com.google.common.truth.SubjectUtils.append;
 import static com.google.common.truth.SubjectUtils.concat;
 

diff --git a/core/src/main/java/com/google/common/truth/Platform.java b/core/src/main/java/com/google/common/truth/Platform.java
@@ -71,28 +71,8 @@ static Throwable[] getSuppressed(Throwable throwable) {
     }
   }
 
-  /** Returns the class object for the given class name, or {@code null} if no class is found. */
-  static Class<?> classForName(String fullyQualifiedClassName) {
-    try {
-      return Class.forName(fullyQualifiedClassName);
-    } catch (ClassNotFoundException e) {
-      return null;
-    }
-  }
-
-  /** Returns true if the former class is assignable from the latter one. */
-  static boolean isAssignableFrom(Class<?> superClass, Class<?> subClass) {
-    return superClass.isAssignableFrom(subClass);
-  }
-
-  /**
-   * Returns true if stack trace cleaning is explicitly disabled in a system property. This switch
-   * os intended to be used when attempting to debug the frameworks which are collapsed or filtered
-   * out of stack traces by the cleaner.
-   */
-  static boolean isStackTraceCleaningDisabled() {
-    return Boolean.parseBoolean(
-        System.getProperty("com.google.common.truth.disable_stack_trace_cleaning"));
+  static void cleanStackTrace(Throwable throwable) {
+    StackTraceCleaner.cleanStackTrace(throwable);
   }
 
   @Nullable

diff --git a/core/src/main/java/com/google/common/truth/StackTraceCleaner.java b/core/src/main/java/com/google/common/truth/StackTraceCleaner.java
@@ -15,24 +15,30 @@
  */
 package com.google.common.truth;
 
+import com.google.common.annotations.GwtIncompatible;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Set;
+import org.junit.runner.Runner;
+import org.junit.runners.model.Statement;
 
 /** Utility that cleans stack traces to remove noise from common frameworks. */
+@GwtIncompatible
 final class StackTraceCleaner {
 
   static final String CLEANER_LINK = "https://goo.gl/aH3UyP";
 
   /**
-   * Cleans the stack trace on the given {@link Throwable}, replacing the original stack trace
+   * <b>Call {@link Platform#cleanStackTrace} rather than calling this directly.</b>
+   *
+   * <p>Cleans the stack trace on the given {@link Throwable}, replacing the original stack trace
    * stored on the instance (see {@link Throwable#setStackTrace(StackTraceElement[])}).
    *
-   * <p>Strips Truth stack frames from the top and JUnit framework and reflective call frames from
+   * <p>Removes Truth stack frames from the top and JUnit framework and reflective call frames from
    * the bottom. Collapses the frames for various frameworks in the middle of the trace as well.
    */
   static void cleanStackTrace(Throwable throwable) {
@@ -58,7 +64,7 @@ private StackTraceCleaner(Throwable throwable) {
   /** Cleans the stack trace on {@code throwable}, replacing the trace that was originally on it. */
   private void clean(Set<Throwable> seenThrowables) {
     // Stack trace cleaning can be disabled using a system property.
-    if (Platform.isStackTraceCleaningDisabled()) {
+    if (isStackTraceCleaningDisabled()) {
       return;
     }
 
@@ -72,11 +78,18 @@ private void clean(Set<Throwable> seenThrowables) {
 
     int stackIndex = stackFrames.length - 1;
     for (; stackIndex >= 0 && !isTruthEntrance(stackFrames[stackIndex]); stackIndex--) {
-      // Find first frame that enters Truth's world and strips all above
+      // Find first frame that enters Truth's world, and remove all above.
     }
     stackIndex += 1;
 
-    for (; stackIndex < stackFrames.length; stackIndex++) {
+    int endIndex = 0;
+    for (;
+        endIndex < stackFrames.length && !isJUnitIntrastructure(stackFrames[endIndex]);
+        endIndex++) {
+      // Find last frame of setup frames, and remove from there down.
+    }
+
+    for (; stackIndex < endIndex; stackIndex++) {
       StackTraceElementWrapper stackTraceElementWrapper =
           new StackTraceElementWrapper(stackFrames[stackIndex]);
       // Always keep frames that might be useful.
@@ -165,12 +178,26 @@ private void clearStreak() {
       ImmutableSet.<Class<?>>of(Subject.class, StandardSubjectBuilder.class);
 
   private static boolean isTruthEntrance(StackTraceElement stackTraceElement) {
-    Class<?> stackClass = Platform.classForName(stackTraceElement.getClassName());
-    if (stackClass == null) {
+    return isFromClass(stackTraceElement, TRUTH_ENTRANCE_CLASSES);
+  }
+
+  private static final ImmutableSet<Class<?>> JUNIT_INFRASTRUCTURE_CLASSES =
+      ImmutableSet.<Class<?>>of(Runner.class, Statement.class);
+
+  private static boolean isJUnitIntrastructure(StackTraceElement stackTraceElement) {
+    return isFromClass(stackTraceElement, JUNIT_INFRASTRUCTURE_CLASSES);
+  }
+
+  private static boolean isFromClass(
+      StackTraceElement stackTraceElement, ImmutableSet<Class<?>> classes) {
+    Class<?> stackClass;
+    try {
+      stackClass = Class.forName(stackTraceElement.getClassName());
+    } catch (ClassNotFoundException e) {
       return false;
     }
-    for (Class<?> knownEntranceClass : TRUTH_ENTRANCE_CLASSES) {
-      if (Platform.isAssignableFrom(knownEntranceClass, stackClass)) {
+    for (Class<?> knownEntranceClass : classes) {
+      if (knownEntranceClass.isAssignableFrom(stackClass)) {
         return true;
       }
     }
@@ -296,4 +323,14 @@ boolean belongsToType(String fullyQualifiedClassName) {
       return false;
     }
   }
+
+  /**
+   * Returns true if stack trace cleaning is explicitly disabled in a system property. This switch
+   * is intended to be used when attempting to debug the frameworks which are collapsed or filtered
+   * out of stack traces by the cleaner.
+   */
+  private static boolean isStackTraceCleaningDisabled() {
+    return Boolean.parseBoolean(
+        System.getProperty("com.google.common.truth.disable_stack_trace_cleaning"));
+  }
 }
diff --git a/core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java b/core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java
@@ -102,25 +102,8 @@ static Throwable[] getSuppressed(Throwable throwable) {
     return throwable.getSuppressed();
   }
 
-  /*
-   * TODO(cpovirk): Instead of having StackTraceCleaner call into multiple Platform methods, create
-   * Platform.cleanStackTrace(), and make it a no-op under GWT, where stack-trace cleaning already
-   * doesn't work.
-   */
-
-  /** Returns the class object for the class name, always returning {@code null} under gwt. */
-  static Class<?> classForName(String fullyQualifiedClassName) {
-    return null;
-  }
-
-  /** Tests if the former class is assignable from the latter one, always return false under gwt. */
-  static boolean isAssignableFrom(Class<?> superClass, Class<?> subClass) {
-    return false;
-  }
-
-  /** Always returns false. Stack traces will be cleaned by default. */
-  static boolean isStackTraceCleaningDisabled() {
-    return false;
+  static void cleanStackTrace(Throwable throwable) {
+    // Do nothing. See notes in StackTraceCleanerTest.
   }
 
   @Nullable

diff --git a/core/src/test/java/com/google/common/truth/StackTraceCleanerTest.java b/core/src/test/java/com/google/common/truth/StackTraceCleanerTest.java
@@ -17,12 +17,27 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
-import com.google.common.annotations.GwtIncompatible;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.junit.runner.Runner;
 import org.junit.runners.JUnit4;
+import org.junit.runners.model.Statement;
 
 /** Unit tests for {@link StackTraceCleaner}. */
+/*
+ * Cleaning doesn't actually work under j2cl (and presumably GWT): StackTraceElement.getClassName()
+ * doesn't have real data. Some data is available in toString(), albeit along the lines of
+ * "SimpleAssertionError.m_createError__java_lang_String_$pp_java_lang." StackTraceCleaner could
+ * maybe look through the toString() representations to count how many frames to remove, but that's
+ * a bigger project. (While we're at it, we could remove the j2cl-specific boilerplate from the
+ * _bottom_ of the stack, too.) And sadly, it's not necessarily as simple as looking at just _class_
+ * names: The cleaning is applied to causes, too, and it's possible for a cause to legitimately
+ * contain an exception created inside a class like Throwable -- e.g., x.initCause(x) will throw an
+ * exception, and it would be weird (though maybe tolerable) for us to remove that.
+ *
+ * Also note that j2cl includes some extra frames at the _top_, even beyond the ones that we try to
+ * remove: b/71355096
+ */
 @RunWith(JUnit4.class)
 public class StackTraceCleanerTest extends BaseSubjectTestCase {
 
@@ -59,30 +74,13 @@ public void collapseStreaks() {
   }
 
   @Test
-  @GwtIncompatible
-  /*
-   * Stripping doesn't actually work under j2cl (and presumably GWT):
-   * StackTraceElement.getClassName() doesn't have real data. Some data is available in toString(),
-   * albeit along the lines of
-   * "SimpleAssertionError.m_createError__java_lang_String_$pp_java_lang." StackTraceCleaner
-   * could maybe look through the toString() representations to count how many frames to strip, but
-   * that's a bigger project. (While we're at it, we could strip the j2cl-specific boilerplate from
-   * the _bottom_ of the stack, too.) And sadly, it's not necessarily as simple as looking at just
-   * _class_ names: The stripping is applied to causes, too, and it's possible for a cause to
-   * legitimately contain an exception created inside a class like Throwable -- e.g., x.initCause(x)
-   * will throw an exception, and it would be weird (though maybe tolerable) for us to strip that.
-   *
-   * Also note that j2cl includes some extra frames at the _top_, even beyond the ones that we try
-   * to strip: b/71355096
-   */
   public void assertionsActuallyUseCleaner() {
     expectFailure.whenTesting().that(1).isEqualTo(2);
     assertThat(expectFailure.getFailure().getStackTrace()[0].getClassName())
         .isEqualTo(getClass().getName());
   }
 
   @Test
-  @GwtIncompatible
   public void assertionsActuallyUseCleaner_ComparisonFailure() {
     expectFailure.whenTesting().that("1").isEqualTo("2");
     assertThat(expectFailure.getFailure().getStackTrace()[0].getClassName())
@@ -119,7 +117,6 @@ public void dontCollapseStreaksOfOneFrame() {
   }
 
   @Test
-  @GwtIncompatible("Class.forName")
   public void mixedStreaks() {
     Throwable throwable =
         createThrowableWithStackTrace(
@@ -191,7 +188,6 @@ public void packagesAreIgnoredForTestClasses() {
   }
 
   @Test
-  @GwtIncompatible("Class.forName")
   public void allFramesAboveStandardSubjectBuilderCleaned() {
     Throwable throwable =
         createThrowableWithStackTrace(
@@ -210,7 +206,6 @@ public void allFramesAboveStandardSubjectBuilderCleaned() {
   }
 
   @Test
-  @GwtIncompatible("Class.forName")
   public void allFramesAboveSubjectCleaned() {
     Throwable throwable =
         createThrowableWithStackTrace(
@@ -228,6 +223,46 @@ public void allFramesAboveSubjectCleaned() {
             });
   }
 
+  @Test
+  public void allFramesBelowJUnitStatementCleaned() {
+    Throwable throwable =
+        createThrowableWithStackTrace(
+            "com.google.common.truth.StringSubject",
+            "com.google.example.SomeTest",
+            SomeStatement.class.getName(),
+            "com.google.example.SomeClass");
+
+    StackTraceCleaner.cleanStackTrace(throwable);
+
+    assertThat(throwable.getStackTrace())
+        .isEqualTo(
+            new StackTraceElement[] {
+              createStackTraceElement("com.google.example.SomeTest"),
+            });
+  }
+
+  @Test
+  public void allFramesBelowJUnitRunnerCleaned() {
+    Throwable throwable =
+        createThrowableWithStackTrace(
+            "com.google.common.truth.StringSubject",
+            "com.google.example.SomeTest",
+            SomeRunner.class.getName(),
+            "com.google.example.SomeClass");
+
+    StackTraceCleaner.cleanStackTrace(throwable);
+
+    assertThat(throwable.getStackTrace())
+        .isEqualTo(
+            new StackTraceElement[] {
+              createStackTraceElement("com.google.example.SomeTest"),
+            });
+  }
+
+  abstract static class SomeStatement extends Statement {}
+
+  abstract static class SomeRunner extends Runner {}
+
   /**
    * This scenario where truth class is called directly without any subject's subclass or {@link
    * StandardSubjectBuilder} in the call stack should not happen in practical, testing anyway to