Repository: mockito/mockito
Issue: Break cyclical compile time dependency on hideRecursiveCall
Labels: bug
1.* incompatible

I am working on upgrading the version of mockito in AOSP to 2.7.13 using byte buddy and byte buddy android rather than dexmaker.

As part of that I need to exclude a number of classes which will not compile on Android because it does not support Java agents/instrumentation. The mockito code has always been very well organized and made that very simple to do but in the latest version I found a minor issue.

The files I need to exclude are:
- src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java
- src/main/java/org/mockito/internal/creation/bytebuddy/InlineBytecodeGenerator.java
- src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java

Unfortunately, when I do that I get a compile error because MockMethodAdvice is trying to use hideRecursiveCall from InlineByteBuddyMockMaker. I looked at the code and it seems as though InlineByteBuddyMockMaker depends on InlineBytecodeGenerator which depends on MockMethodAdvice which depends on hideRecursiveCall from InlineByteBuddyMockMaker.

The only places that actually use hideRecursiveCall are MockMethodAdvice and InlineByteBuddyMockMakerTest. That depends on InlineByteBuddyMockMaker which depends on InlineBytecodeGenerator which depends on MockMethodAdvice which depends on hideRecursiveCall from InlineByteBuddyMockMaker.

So, moving hideRecursiveCall from InlineByteBuddyMockMaker to MockMethodAdvice breaks a dependency cycle and allows the InlineByteBuddyMockMaker.java file to be excluded without problem. Also, MockMethodAdvice seems to be a better place for it.

If you are happy with this proposal or can suggest a better place for hideRecursiveCall then I am happy to submit a fix for this.

Issue comments:
That seems like a reasonable issue. Looking forward to your pull request @paulduffin !
Should I make the change in release/2.x?

On 8 March 2017 at 12:57, Tim van der Lippe <notifications@github.com>
wrote:

> That seems like a reasonable issue. Looking forward to your pull request
> @paulduffin <https://github.com/paulduffin> !
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/mockito/mockito/issues/982#issuecomment-285033757>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AEtcQ-KnL90mWa9jxuLBhEsA8QmxpSJzks5rjqXXgaJpZM4MWrIj>
> .
>

Yes please :)
Done.

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java b/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java
index a98629564c..0258b67985 100644
--- a/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java
+++ b/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java
@@ -311,23 +311,4 @@ public String nonMockableReason() {
         };
     }
 
-    static Throwable hideRecursiveCall(Throwable throwable, int current, Class<?> targetType) {
-        try {
-            StackTraceElement[] stack = throwable.getStackTrace();
-            int skip = 0;
-            StackTraceElement next;
-            do {
-                next = stack[stack.length - current - ++skip];
-            } while (!next.getClassName().equals(targetType.getName()));
-            int top = stack.length - current - skip;
-            StackTraceElement[] cleared = new StackTraceElement[stack.length - skip];
-            System.arraycopy(stack, 0, cleared, 0, top);
-            System.arraycopy(stack, top + skip, cleared, top, current);
-            throwable.setStackTrace(cleared);
-            return throwable;
-        } catch (RuntimeException ignored) {
-            // This should not happen unless someone instrumented or manipulated exception stack traces.
-            return throwable;
-        }
-    }
 }
diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
index 394bd2d2eb..cef75c8830 100644
--- a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
+++ b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
@@ -23,8 +23,6 @@
 import java.lang.reflect.Modifier;
 import java.util.concurrent.Callable;
 
-import static org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker.hideRecursiveCall;
-
 public class MockMethodAdvice extends MockMethodDispatcher {
 
     final WeakConcurrentMap<Object, MockMethodInterceptor> interceptors;
@@ -61,6 +59,26 @@ private static void exit(@Advice.Return(readOnly = false, typing = Assigner.Typi
         }
     }
 
+    static Throwable hideRecursiveCall(Throwable throwable, int current, Class<?> targetType) {
+        try {
+            StackTraceElement[] stack = throwable.getStackTrace();
+            int skip = 0;
+            StackTraceElement next;
+            do {
+                next = stack[stack.length - current - ++skip];
+            } while (!next.getClassName().equals(targetType.getName()));
+            int top = stack.length - current - skip;
+            StackTraceElement[] cleared = new StackTraceElement[stack.length - skip];
+            System.arraycopy(stack, 0, cleared, 0, top);
+            System.arraycopy(stack, top + skip, cleared, top, current);
+            throwable.setStackTrace(cleared);
+            return throwable;
+        } catch (RuntimeException ignored) {
+            // This should not happen unless someone instrumented or manipulated exception stack traces.
+            return throwable;
+        }
+    }
+
     @Override
     public Callable<?> handle(Object instance, Method origin, Object[] arguments) throws Throwable {
         MockMethodInterceptor interceptor = interceptors.get(instance);


Test patch:
diff --git a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java
index ee36f6de77..445c6f8905 100644
--- a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java
+++ b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java
@@ -158,7 +158,7 @@ public void should_remove_recursive_self_call_from_stack_trace() throws Exceptio
 
         Throwable throwable = new Throwable();
         throwable.setStackTrace(stack);
-        throwable = InlineByteBuddyMockMaker.hideRecursiveCall(throwable, 2, SampleInterface.class);
+        throwable = MockMethodAdvice.hideRecursiveCall(throwable, 2, SampleInterface.class);
 
         assertThat(throwable.getStackTrace()).isEqualTo(new StackTraceElement[]{
                 new StackTraceElement("foo", "", "", -1),
@@ -171,7 +171,7 @@ public void should_remove_recursive_self_call_from_stack_trace() throws Exceptio
     public void should_handle_missing_or_inconsistent_stack_trace() throws Exception {
         Throwable throwable = new Throwable();
         throwable.setStackTrace(new StackTraceElement[0]);
-        assertThat(InlineByteBuddyMockMaker.hideRecursiveCall(throwable, 0, SampleInterface.class)).isSameAs(throwable);
+        assertThat(MockMethodAdvice.hideRecursiveCall(throwable, 0, SampleInterface.class)).isSameAs(throwable);
     }
 
     @Test