Closed GoogleCodeExporter closed 8 years ago
Doing the same but using plain vanilla Mockito 1.9.5-rc1 outside of Android
(i.e., no DexMaker involved), works just fine.
Original comment by klyu...@google.com
on 28 Jun 2012 at 7:52
It looks like Mockito assumes that MockHandler can, for primitive values,
return a boxed type that doesn't completely match the primitive type. For
example, in the above example the MockHandler returns the java.lang.Integer
generated by the code below:
https://code.google.com/p/mockito/source/browse/src/org/mockito/internal/stubbin
g/defaultanswers/ReturnsEmptyValues.java?r=f08f5d7b948bc3eb6310f86fd17105e0d80ee
b22#123
Original comment by klyu...@google.com
on 28 Jun 2012 at 8:52
Thanks for the perfect bug report. I've filed it upstream and will prepare a
pull request.
https://code.google.com/p/mockito/issues/detail?id=352
Original comment by limpbizkit
on 29 Jun 2012 at 8:15
I'm not sure it's a bug in Mockito. The above example works fine with Mockito
outside of Android land. I wonder if Mockito's design/expecation is that
MockHandler can return boxed primitive types that the proxy class needs to
convert to the correct type (if necessary).
Original comment by klyu...@google.com
on 29 Jun 2012 at 3:46
It's a bug in Mockito. Dexmaker's behavior is consistent with
java.lang.reflect.Proxy. It also throws if you return the wrong boxed primitive.
This code:
interface A {
long returnLong();
}
public void testDynamicProxy() {
InvocationHandler invocationHandler = new InvocationHandler() {
@Override public Object invoke(Object proxy, Method method, Object[] args) {
return Integer.valueOf(5);
}
};
A a = (A) Proxy.newProxyInstance(getClass().getClassLoader(),
new Class[]{A.class}, invocationHandler);
System.out.println(a.returnLong());
}
yields this exception:
java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.Long
at $Proxy6.returnLong(Unknown Source)
at Test.testDynamicProxy(Test.java:53)
Original comment by limpbizkit
on 29 Jun 2012 at 4:10
However, if you use Mockito + DexMaker to create a mock of the above interface
A, then it works just fine despite MockHandler returning a java.lang.Integer
(example below).
interface A {
long returnLong();
}
A a = Mockito.mock(A.class)
a.returnLong();
Original comment by klyu...@google.com
on 29 Jun 2012 at 4:38
@klyubin: did you misspeak? I assume you meant "Mockito + cglib" ? Otherwise we
wouldn't be having this discussion!
Cglib does a lot of unnecessary type-munging work that probably has a
performance cost. I don't think that motivates us to do the same work in
dexmaker.
Original comment by limpbizkit
on 29 Jun 2012 at 5:34
No, I didn't misspeak. If you run the above example (from comment #6) on
Android using Mockito 1.9.5-rc1 + DexMaker, then it will work just fine. Behind
covers, the MockHandler provided to DexMaker by Mockito still returns a
java.lang.Integer as the result of the invocation, which in turn makes
DexMaker's InvocationHandlerAdapter return the same java.lang.Integer from its
"invoke" method. I'm puzzled myself as to why this does not blow up inside the
proxy object generated by java.lang.reflect.Proxy inside DexmakerMockMaker.
Original comment by klyu...@google.com
on 29 Jun 2012 at 5:43
A bit of experimenting seems to suggest that the proxy generated by
java.lang.reflect.Proxy behaves differently between Dalvik and Java VMs. On
Dalvik it seems to be fine with accepting an Integer where Long needs to be
returned, whereas on JVM it isn't. This explains why Mockito + DexMaker works
fine on Android for mocking interfaces, but not for classes: DexMaker's proxy
for classes is not as lenient as the Dalvik VMs java.lang.reflect.Proxy. In
particular, Dalvik VM's proxy class seems to permit conversions that don't lose
precision (e.g., Integer -> Long, Float -> Double, Short -> Integer), but still
doesn't permit the reverse conversions.
Original comment by klyu...@google.com
on 29 Jun 2012 at 6:04
Weird. That's a bug in Dalvik. Regardless, I think we want to be consistent
with the JVM's Proxy class. Conversions of any kind make me anxious.
Original comment by limpbizkit
on 29 Jun 2012 at 6:10
To sum up, when using Mockito to mock a method returning a primitive long, the
MockHandler passed by Mockito to MockMaker instances returns a
java.lang.Integer by default. This:
1. Works fine on JVM both for classes and interfaces. This suggests that
Mockito uses an extra layer somewhere between the MockHandler and proxy classes.
2. Works fine on Dalvik VM for interfaces, because proxies generated by
DexMaker using java.lang.reflect.Proxy seem to upscale boxed primitive values
if necessary (e.g., Integer -> Long) -- this appears to be a "feature/bug" of
Dalvik's java.lang.reflect.Proxy.
3. Does not work on Dalvik VM for classes, because the proxy generated by
DexMaker's ProxyBuilder does not upscale boxed primitive values.
Now to mitigation:
* It's best to make DexMaker's ProxyBuilder generate proxies that work similar
to those generated by java.lang.reflect.Proxy on Dalvik VM. This means adding
upscaling, unless that's a bug in Dalvik VM.
* We need to find out why Mockito's MockHandler returning a java.lang.Integer
for a method that is supposed to return a java.lang.Long does not blow up on
the JVM. We can then see if that's a bug or not. This, in turn, will tell us
how/whether to fix DexMaker.
Original comment by klyu...@google.com
on 29 Jun 2012 at 6:26
One workaround until this is resolved properly, would be to add upscaling to
DexMaker's InvocationHandlerAdapter. What do you think?
Original comment by klyu...@google.com
on 29 Jun 2012 at 6:38
Original comment by klyu...@google.com
on 29 Jun 2012 at 6:52
P.S. To complicate matters, Mockito + DexMaker doesn't work for mocking
interfaces whose methods return primitive byte or short. This is because the
java.lang.reflect.Proxy on Dalvik does not downscale java.lang.Integer to
java.lang.Byte or java.lang.Short.
So, in addition to the differences between the java.lang.reflect.Proxy behavior
on Delvik and JVM, there's a fundamental mismatch between what Mockito's
MockHandlers are returning and what DexMaker is expecting them to return. I'm
still wondering whether it's a bug in Mockito or whether it expects (but does
not declare) that MockHandler's results may need to be converted (up- or
down-scaled) for primitive types.
Original comment by klyu...@google.com
on 29 Jun 2012 at 8:19
Here's a patch against ebb25ef16a03f478946593d2d2ada043e0007220. The patch
fixes all of these issues by making DexMaker's InvocationHandlerAdapter adjust
the boxed primitive values it returns. What do you think?
Original comment by klyu...@google.com
on 29 Jun 2012 at 9:15
Attachments:
I suspect (but haven't confirmed) that, on the JVM, Mockito does not experience
the issues because it uses the following code (from cglib) in its proxy (used
for interfaces and classes):
https://code.google.com/p/mockito/source/browse/cglib-and-asm/src/org/mockito/cg
lib/core/CodeEmitter.java?r=f08f5d7b948bc3eb6310f86fd17105e0d80eeb22#733.
The code casts the boxed value to java.lang.Number and then invokes the
to-primitive-value method (e.g., intValue, byteValue). The behavior is very
similar to that in the patch attached to comment #15.
Original comment by klyu...@google.com
on 29 Jun 2012 at 9:30
We should just get this fixed in Mockito. We've already got a patch and it
shouldn't be much trouble for them to integrate it. The casting to Number stuff
is wacky, especially because the conversion may be lossy.
Original comment by limpbizkit
on 30 Jun 2012 at 2:07
Fixed in Mockito.
Original comment by limpbizkit
on 1 Jul 2012 at 6:09
Thanks! Good call.
Original comment by klyu...@google.com
on 2 Jul 2012 at 8:25
Original issue reported on code.google.com by
klyu...@google.com
on 28 Jun 2012 at 7:50