Repository: mockito/mockito
Issue: modify StackTraceFilter to not strip away "good" elements from the middle
Labels: enhancement

according to javadoc of org.mockito.internal.exceptions.stacktrace.StackTraceFilter, filter works like this:
[a+, b+, c-, d+, e+, f-, g+] -> [a+, b+, g+]
it would be better to not strip away "good" d and e elements:
[a+, b+, c-, d+, e+, f-, g+] -> [a+, b+, d+, e+, g+]
What do you think about this? For me current behaviour is very inconvenient, I had to turn it off.


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/org/mockito/internal/exceptions/stacktrace/StackTraceFilter.java b/src/main/java/org/mockito/internal/exceptions/stacktrace/StackTraceFilter.java
index cd462dcee1..2b5bb658cc 100644
--- a/src/main/java/org/mockito/internal/exceptions/stacktrace/StackTraceFilter.java
+++ b/src/main/java/org/mockito/internal/exceptions/stacktrace/StackTraceFilter.java
@@ -10,8 +10,6 @@
 
 import java.io.Serializable;
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.LinkedList;
 import java.util.List;
 
 public class StackTraceFilter implements Serializable {
@@ -20,38 +18,22 @@ public class StackTraceFilter implements Serializable {
 
     private static final StackTraceCleaner CLEANER =
             Plugins.getStackTraceCleanerProvider().getStackTraceCleaner(new DefaultStackTraceCleaner());
-    
+
     /**
      * Example how the filter works (+/- means good/bad):
-     * [a+, b+, c-, d+, e+, f-, g+] -> [a+, b+, g+]
-     * Basically removes all bad from the middle. If any good are in the middle of bad those are also removed. 
+     * [a+, b+, c-, d+, e+, f-, g+] -> [a+, b+, d+, e+, g+]
+     * Basically removes all bad from the middle.
+     * <strike>If any good are in the middle of bad those are also removed.</strike>
      */
     public StackTraceElement[] filter(StackTraceElement[] target, boolean keepTop) {
         //TODO: profile
-        List<StackTraceElement> unfilteredStackTrace = Arrays.asList(target);
-        
-        int lastBad = -1;
-        int firstBad = -1;
-        for (int i = 0; i < unfilteredStackTrace.size(); i++) {
-            if (!CLEANER.isOut(unfilteredStackTrace.get(i))) {
-                continue;
-            }
-            lastBad = i;
-            if (firstBad == -1) {
-                firstBad = i;
+        final List<StackTraceElement> filtered = new ArrayList<StackTraceElement>();
+        for (StackTraceElement aTarget : target) {
+            if (!CLEANER.isOut(aTarget)) {
+                filtered.add(aTarget);
             }
         }
-        
-        List<StackTraceElement> top;
-        if (keepTop && firstBad != -1) {
-            top = unfilteredStackTrace.subList(0, firstBad);
-        } else {
-            top = new LinkedList<StackTraceElement>();
-        }
-        
-        List<StackTraceElement> bottom = unfilteredStackTrace.subList(lastBad + 1, unfilteredStackTrace.size());
-        List<StackTraceElement> filtered = new ArrayList<StackTraceElement>(top);
-        filtered.addAll(bottom);
-        return filtered.toArray(new StackTraceElement[]{});
+        StackTraceElement[] result = new StackTraceElement[filtered.size()];
+        return filtered.toArray(result);
     }
 }
\ No newline at end of file


Test patch:
diff --git a/src/test/java/org/mockito/internal/exceptions/stacktrace/StackTraceFilterTest.java b/src/test/java/org/mockito/internal/exceptions/stacktrace/StackTraceFilterTest.java
index 3d4185e7c9..0b1e588d86 100644
--- a/src/test/java/org/mockito/internal/exceptions/stacktrace/StackTraceFilterTest.java
+++ b/src/test/java/org/mockito/internal/exceptions/stacktrace/StackTraceFilterTest.java
@@ -40,7 +40,7 @@ public void shouldFilterOutMockitoPackage() {
     }
     
     @Test
-    public void shouldFilterOutTracesMiddleBadTraces() {
+    public void shouldNotFilterOutTracesMiddleGoodTraces() {
         StackTraceElement[] t = new TraceBuilder().classes(
                 "org.test.MockitoSampleTest",
                 "org.test.TestSupport",
@@ -51,7 +51,7 @@ public void shouldFilterOutTracesMiddleBadTraces() {
         
         StackTraceElement[] filtered = filter.filter(t, false);
         
-        assertThat(filtered, hasOnlyThoseClasses("org.test.TestSupport", "org.test.MockitoSampleTest"));
+        assertThat(filtered, hasOnlyThoseClasses("org.test.TestSupport", "org.test.TestSupport", "org.test.MockitoSampleTest"));
     }
     
     @Test