Repository: mockito/mockito
Issue: mockito 2.0.14 fails to mock jetty httpclient
Labels: bug

this fails:

<pre>
     import org.eclipse.jetty.client.HttpClient;
    HttpClient httpClient = mock(HttpClient.class);
</pre>

(jetty 9.2.10.v20150310)

with mockito 2.0.2-beta i can mock that class without a problem. 

stacktrace:

<pre>
java.lang.IllegalAccessError: tried to access class org.eclipse.jetty.util.component.ContainerLifeCycle$Managed from class org.eclipse.jetty.client.HttpClient$MockitoMock$362486671
    at org.eclipse.jetty.client.HttpClient$MockitoMock$362486671.<clinit>(Unknown Source)
    at sun.reflect.GeneratedSerializationConstructorAccessor2.newInstance(Unknown Source)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:408)
    at org.objenesis.instantiator.sun.SunReflectionFactoryInstantiator.newInstance(SunReflectionFactoryInstantiator.java:45)
    at org.objenesis.ObjenesisBase.newInstance(ObjenesisBase.java:73)
    at org.mockito.internal.creation.instance.ObjenesisInstantiator.newInstance(ObjenesisInstantiator.java:14)
    at org.mockito.internal.creation.bytebuddy.ByteBuddyMockMaker.createMock(ByteBuddyMockMaker.java:27)
    at org.mockito.internal.util.MockUtil.createMock(MockUtil.java:33)
    at org.mockito.internal.MockitoCore.mock(MockitoCore.java:59)
    at org.mockito.Mockito.mock(Mockito.java:1392)
    at org.mockito.Mockito.mock(Mockito.java:1270)
</pre>


Issue comments:
Is this an OSGI package ?

Also a general principle when mocking is _to not mock types you don't own_, there's several reasons for that see that [wiki page](https://github.com/mockito/mockito/wiki/How-to-write-good-tests).

However the issue is probably true. But I cannot reproduce locally, do you have additional informations like jetty version, jvm version (OpenJDK, hotspot, J9, ...) ?

yeah, i know about that rule. this is from a really tiny method and i just want to check that it invokes the start method. 

oracle jdk8_11, jetty 9.2.10

The `Managed` inner class is package-private: http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.jetty/jetty-util/9.1.3.v20140225/org/eclipse/jetty/util/component/ContainerLifeCycle.java#ContainerLifeCycle.Managed

This class should never be accessible from `org.eclipse.jetty.client.HttpClient` or any mock which both live in another package. I assume that you have a version clash on your class path. Creating a mock can trigger lazy class path resolutions that are not even triggered in your production code via the class introspection that is required to create the mock. 

ok but the HttpClient class can be instantiated just fine by calling its constructor

client = new HttpClient();

also the managed enum is only used inside ContainerLifeCycle. 

I now see what the problem is. Byte Buddy overrides the public method `ContainerLifeCycle::addBean(Object o, Managed managed)` in order to allow mocking the method call. For this, the mock references the `Managed` class which is however package-private.

While this is a rather poorly designed API as the method should itself be package-private as it cannot be used outside of the package, Byte Buddy should be smart enough to figure this out. It is a small fix, I have it ready some time this week. Thanks for reporting!

thanks. I agree that the package structure and api design of jetty is a bit strange.

@raphw Thanks for the investigation

@christophsturm Thanks for the feddback too, note mockito 2.0.0 is still in beta phase

sure! we will just continue to use 2.0.2-beta for now which works great.

You can use a more recent beta `2.0.8-beta`, the last version before switching to bytebuddy

Byte Buddy 0.6.9 is released which should fix the issue. Sorry for the hick-up. (In JCenter, currently synchronizing to Maven Central.)

No problem, you were blazingly fast !


Pull request review comments:

Patch that resolved the issue:
diff --git a/build.gradle b/build.gradle
index 8f7c9ac72f..939764ee4f 100644
--- a/build.gradle
+++ b/build.gradle
@@ -60,7 +60,7 @@ tasks.withType(JavaCompile) {
 
 //TODO we should remove all dependencies to checked-in jars
 dependencies {
-    compile 'net.bytebuddy:byte-buddy:0.6.8'
+    compile 'net.bytebuddy:byte-buddy-dep:0.6.10'
 
     provided "junit:junit:4.10"
     compile "org.hamcrest:hamcrest-core:1.1", "org.objenesis:objenesis:2.1"
diff --git a/gradle/javadoc.gradle b/gradle/javadoc.gradle
index 7439273357..c221c6277b 100644
--- a/gradle/javadoc.gradle
+++ b/gradle/javadoc.gradle
@@ -35,8 +35,8 @@ task mockitoJavadoc(type: Javadoc) {
         <script type="text/javascript">
           var usingOldIE = \$.browser.msie && parseInt(\$.browser.version) < 9;
           if(!usingOldIE) {
-              \$("head").append("<link rel=\\"shortcut icon\\" href=\\"/favicon.ico?v=cafebabe\\">")
-              \$("head", window.parent.document).append("<link rel=\\"shortcut icon\\" href=\\"/favicon.ico?v=cafebabe\\">")
+              \$("head").append("<link rel=\\"shortcut icon\\" href=\\"@{docRoot}/favicon.ico?v=cafebabe\\">")
+              \$("head", window.parent.document).append("<link rel=\\"shortcut icon\\" href=\\"@{docRoot}/favicon.ico?v=cafebabe\\">")
               hljs.initHighlightingOnLoad();
               injectProjectVersionForJavadocJDK6("Mockito ${project.version} API",
                                                  "em#mockito-version-header-javadoc7-header",
diff --git a/mockmaker/bytebuddy/main/java/org/mockito/internal/creation/bytebuddy/MockBytecodeGenerator.java b/mockmaker/bytebuddy/main/java/org/mockito/internal/creation/bytebuddy/MockBytecodeGenerator.java
index d89e4250cb..8d7c42d252 100644
--- a/mockmaker/bytebuddy/main/java/org/mockito/internal/creation/bytebuddy/MockBytecodeGenerator.java
+++ b/mockmaker/bytebuddy/main/java/org/mockito/internal/creation/bytebuddy/MockBytecodeGenerator.java
@@ -1,21 +1,24 @@
 package org.mockito.internal.creation.bytebuddy;
 
-import java.util.Random;
-import org.mockito.internal.creation.bytebuddy.ByteBuddyCrossClassLoaderSerializationSupport.CrossClassLoaderSerializableMock;
-import org.mockito.internal.creation.util.SearchingClassLoader;
 import net.bytebuddy.ByteBuddy;
 import net.bytebuddy.ClassFileVersion;
-import net.bytebuddy.description.modifier.FieldManifestation;
-import net.bytebuddy.description.modifier.Ownership;
-import net.bytebuddy.description.modifier.Visibility;
 import net.bytebuddy.dynamic.DynamicType;
 import net.bytebuddy.dynamic.loading.ClassLoadingStrategy;
 import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy;
-import net.bytebuddy.implementation.FieldAccessor;
-import net.bytebuddy.implementation.MethodDelegation;
 import net.bytebuddy.implementation.attribute.MethodAttributeAppender;
 import net.bytebuddy.implementation.attribute.TypeAttributeAppender;
-import net.bytebuddy.matcher.ElementMatchers;
+import org.mockito.internal.creation.bytebuddy.ByteBuddyCrossClassLoaderSerializationSupport.CrossClassLoaderSerializableMock;
+import org.mockito.internal.creation.util.SearchingClassLoader;
+
+import java.util.Random;
+
+import static net.bytebuddy.description.modifier.FieldManifestation.FINAL;
+import static net.bytebuddy.description.modifier.Ownership.STATIC;
+import static net.bytebuddy.description.modifier.Visibility.PRIVATE;
+import static net.bytebuddy.implementation.FieldAccessor.ofBeanProperty;
+import static net.bytebuddy.implementation.MethodDelegation.to;
+import static net.bytebuddy.implementation.MethodDelegation.toInstanceField;
+import static net.bytebuddy.matcher.ElementMatchers.*;
 
 class MockBytecodeGenerator {
     private final ByteBuddy byteBuddy;
@@ -31,20 +34,20 @@ public MockBytecodeGenerator() {
     }
 
     public <T> Class<? extends T> generateMockClass(MockFeatures<T> features) {
-        DynamicType.Builder<T> builder = byteBuddy.subclass(features.mockedType, ConstructorStrategy.Default.IMITATE_SUPER_TYPE)
-                .name(nameFor(features.mockedType))
-                .implement(features.interfaces.toArray(new Class<?>[features.interfaces.size()]))
-                .method(ElementMatchers.any()).intercept(MethodDelegation
-                        .toInstanceField(MockMethodInterceptor.class, "mockitoInterceptor")
-                        .filter(ElementMatchers.isDeclaredBy(MockMethodInterceptor.class)))
-                .implement(MockMethodInterceptor.MockAccess.class).intercept(FieldAccessor.ofBeanProperty())
-                .method(ElementMatchers.isHashCode()).intercept(MethodDelegation.to(MockMethodInterceptor.ForHashCode.class))
-                .method(ElementMatchers.isEquals()).intercept(MethodDelegation.to(MockMethodInterceptor.ForEquals.class))
-                .defineField("serialVersionUID", long.class, Ownership.STATIC, Visibility.PRIVATE, FieldManifestation.FINAL).value(42L);
-            if (features.crossClassLoaderSerializable) {
-                builder = builder.implement(CrossClassLoaderSerializableMock.class)
-                        .intercept(MethodDelegation.to(MockMethodInterceptor.ForWriteReplace.class));
-            }
+        DynamicType.Builder<T> builder =
+                byteBuddy.subclass(features.mockedType, ConstructorStrategy.Default.IMITATE_SUPER_TYPE)
+                         .name(nameFor(features.mockedType))
+                         .implement(features.interfaces.toArray(new Class<?>[features.interfaces.size()]))
+                         .method(any()).intercept(toInstanceField(MockMethodInterceptor.class, "mockitoInterceptor")
+                                                          .filter(isDeclaredBy(MockMethodInterceptor.class)))
+                         .implement(MockMethodInterceptor.MockAccess.class).intercept(ofBeanProperty())
+                         .method(isHashCode()).intercept(to(MockMethodInterceptor.ForHashCode.class))
+                         .method(isEquals()).intercept(to(MockMethodInterceptor.ForEquals.class))
+                         .defineField("serialVersionUID", long.class, STATIC, PRIVATE, FINAL).value(42L);
+        if (features.crossClassLoaderSerializable) {
+            builder = builder.implement(CrossClassLoaderSerializableMock.class)
+                             .intercept(to(MockMethodInterceptor.ForWriteReplace.class));
+        }
         Class<?>[] allMockedTypes = new Class<?>[features.interfaces.size() + 1];
         allMockedTypes[0] = features.mockedType;
 //            System.arraycopy(interfaces.toArray(), 0, allMockedTypes, 1, interfaces.size());
@@ -53,8 +56,8 @@ public <T> Class<? extends T> generateMockClass(MockFeatures<T> features) {
             allMockedTypes[index++] = type;
         }
         return builder.make()
-                .load(SearchingClassLoader.combineLoadersOf(allMockedTypes), ClassLoadingStrategy.Default.INJECTION)
-                .getLoaded();
+                      .load(SearchingClassLoader.combineLoadersOf(allMockedTypes), ClassLoadingStrategy.Default.INJECTION)
+                      .getLoaded();
     }
 
     // TODO inspect naming strategy (for OSGI, signed package, java.* (and bootstrap classes), etc...)


Test patch:
diff --git a/test/org/mockitousage/bugs/creation/PublicMethodInParentWithNonPublicTypeInSignatureTest.java b/test/org/mockitousage/bugs/creation/PublicMethodInParentWithNonPublicTypeInSignatureTest.java
new file mode 100644
index 0000000000..9d4e340bf3
--- /dev/null
+++ b/test/org/mockitousage/bugs/creation/PublicMethodInParentWithNonPublicTypeInSignatureTest.java
@@ -0,0 +1,22 @@
+package org.mockitousage.bugs.creation;
+
+
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.mockitousage.bugs.creation.api.PublicClass;
+
+// see GH-233
+public class PublicMethodInParentWithNonPublicTypeInSignatureTest {
+
+    private Object ref;
+
+    @Test
+    public void java_object_creation() throws Exception {
+        ref = new PublicClass();
+    }
+
+    @Test
+    public void should_not_fail_when_instantiating() throws Exception {
+        ref = Mockito.mock(PublicClass.class);
+    }
+}
diff --git a/test/org/mockitousage/bugs/creation/api/PublicClass.java b/test/org/mockitousage/bugs/creation/api/PublicClass.java
new file mode 100644
index 0000000000..243036a0d4
--- /dev/null
+++ b/test/org/mockitousage/bugs/creation/api/PublicClass.java
@@ -0,0 +1,6 @@
+package org.mockitousage.bugs.creation.api;
+
+import org.mockitousage.bugs.creation.otherpackage.PublicParentClass;
+
+public class PublicClass extends PublicParentClass {
+}
diff --git a/test/org/mockitousage/bugs/creation/otherpackage/PublicParentClass.java b/test/org/mockitousage/bugs/creation/otherpackage/PublicParentClass.java
new file mode 100644
index 0000000000..8f560d4645
--- /dev/null
+++ b/test/org/mockitousage/bugs/creation/otherpackage/PublicParentClass.java
@@ -0,0 +1,6 @@
+package org.mockitousage.bugs.creation.otherpackage;
+
+public class PublicParentClass {
+    public void method_with_non_public_argument(PackageLocalArg arg) { }
+    static class PackageLocalArg { }
+}