Binitkumarsingh / mockito

Automatically exported from code.google.com/p/mockito
0 stars 0 forks source link

Android support #308

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Hi, I'm a member of Android's Dalvik team. Lots of Android developers have been 
asking for Mockito support for Android, so we've written some code to make that 
possible.

The first part is a new library called 'dexmaker' that assumes the role of 
ASM+cglib on Android. Read more about dexmaker here:
http://code.google.com/p/dexmaker/

The second part is a clone to Mockito that integrates dexmaker. The clone is 
here:
  http://code.google.com/r/jessewilson-mockito/

Unfortunately it's slightly cumbersome to confirm that the tests pass on 
Android because there's no easy way to run JUnit 4 tests on Android. I hacked 
together the vogar test runner (http://code.google.com/p/vogar/) to run the 
tests; it would also be possible to run tests as a part of an Android 
application.

Original issue reported on code.google.com by jessewil...@google.com on 13 Jan 2012 at 3:38

GoogleCodeExporter commented 8 years ago
Can you clarify why this is a Mockito issue?  If it's not easy to run JUnit 
tests on Android, I'm not sure how the Mockito team can fix this.  Are you 
saying you want a version of Mockito that works with vogar tests?  Or are you 
asking to convert our unit tests for Mockito into something different?  Please 
make it clearer what you're asking for.

Original comment by dmwallace.nz on 13 Jan 2012 at 3:48

GoogleCodeExporter commented 8 years ago
I'm sorry for being confusing!

Desktop Java uses a JVM that fundamentally operates on .class files. Android 
uses a Dalvik virtual machine that does not use .class files. Instead Android 
uses .dex files which are similar to .class files, except that one .dex file 
can describe the contents of many types. They're a bit more efficient!

Since Mockito uses cglib behind-the-scenes, and cglib can only generate .class 
files, there is no way to run Mockito on Android.

This change adds a new backend to Mockito called "dexmaker" as an alternative 
to cglib. Dexmaker generates .dex files and works just fine on Android.

If the Mockito team would rather not have to maintain internal Android support, 
please let me know. If that's the case I'll create a separate project that 
exists to port mockito to Android.

Original comment by jessewil...@google.com on 13 Jan 2012 at 4:02

GoogleCodeExporter commented 8 years ago
The major patch is here: 
http://code.google.com/r/jessewilson-mockito/source/detail?r=805e98fd4190acbbdaa
b87172c31b8dc0b78236e

Original comment by jessewil...@google.com on 13 Jan 2012 at 4:03

GoogleCodeExporter commented 8 years ago
OK, understood.  I'm not sure whether anyone in the Mockito team would be 
interested in maintaining Android support; I'm not, but I can't speak for the 
entire team.  Watch this space.

Original comment by dmwallace.nz on 13 Jan 2012 at 4:18

GoogleCodeExporter commented 8 years ago
Hi Jesse,

This is great work. However I don't think anybody in the team actually works in 
the Android environment, so we are clearly not proficient in this area. You are 
probably the expert here ;)

Also, for some time we have bean thinking to rearchitecture the backend to 
allow things such as changing the bytecode engine. I already started research 
on this matter. But free time is scarce these past months, so this research 
didn't progressed at an acceptable rate (in my opinion).

However your port is interesting because it is not just about changing the 
bytecode engine, but to support another platform.

(Thinking loudly) When this backend stuff will be mature enough, it could be 
possible to create a project that will run the test using Vogar with Dalvik, 
and on the Mockito side you could set internal settings to use Android support. 
Maybe with a DefaultInternalConfiguration.

Regards,
Brice

Original comment by brice.du...@gmail.com on 13 Jan 2012 at 9:41

GoogleCodeExporter commented 8 years ago
Would you guys be willing to accept a different patch that made the codegen 
component pluggable? I'm imagining it taking the form of a simple interface in 
Mockito's internal package:

  public interface MockMaker {
    T newMock(Class<T> classToMock, MockitoInvocationHandler handler,
        Class<?>[] ancillaryTypes, ClassLoader loader);
    MockitoInvocationHandler getHandler(Object mock);
  }

plus a helper class:

  public class MockMakerFactory {
    /**
     * Returns the best mock maker for this runtime.
     */
    public static MockMaker get() {
      // inspect the classpath, looking for a MockMaker implementation
      // this will use cglib+asm on the JVM and dexmaker on Android
    }
  }

That way I can ship a small 'mockito-android.jar' file that provides the 
MockMaker implementation only. Android applications would just need to make 
sure that .jar is on their classpath for things to work.

Original comment by jessewil...@google.com on 13 Jan 2012 at 7:57

GoogleCodeExporter commented 8 years ago
One advantage of this pluggable codegen approach is that Mockito's build and 
test execution won't be affected, since there isn't any Android-specific code.

Original comment by tball@google.com on 13 Jan 2012 at 8:19

GoogleCodeExporter commented 8 years ago
@Tom Yes that would be definitely a part of the target.

@Jesse You can hack something, but it is not sure it will be integrated as is, 
i.e. we need to think how to expose a good internal API, not a clunky one. 
However it will definitely be a good starting point, it'll help to fathom 
implications here and there. Also Android support is a bit special as it 
requires to modify other objects such as the BeanPropertySetter.

Original comment by brice.du...@gmail.com on 16 Jan 2012 at 8:51

GoogleCodeExporter commented 8 years ago
1. @Jesse, so with the patch you presented do all mockito tests pass on dalvik? 
If so that's pretty awsome.

2. The pluggability. We already have 'some' infrastructure to make it happen. 
It's very simple as we didn't use it much and I think very little users (if 
any) actually take advantage of it. However, it might a good start. Take a look 
at the documentation of the IMockitoConfiguration. In theory, you could place 
the implementation of that interface in your jar, in a correct package and 
Mockito will use it. Trouble is that IMockitoConfiguration does not have any 
means of configuring the algorithm that creates mocks. This is where an eager 
contributor @Jesse comes in :D It would be very cool if you could come up with 
some configurability there, for example:

IMockitoConfiguration {
  MockMaker getMockMaker(); //or something like that.
}

I'd love to pull in such patch!

I looked at the codebase yesterday to find out if it's easy to unbake the cglib 
stuff from Mockito. It didn't look easy enough for a late Sunday :/ 
Unfortunately, cglib stuff is in Mockito since day 1, the time when I was 
focused on the fluent mocking DSL rather than on configurability. Good luck and 
feel free to ask / reach out for our help.

Thanks a lot for the Dalvik team for reaching out! @David, @Brice, let's help 
out as much as we can so that we can offer mockito to android devs.

Cheers :)

Original comment by szcze...@gradle.biz on 16 Jan 2012 at 3:13

GoogleCodeExporter commented 8 years ago
Yeah, the tests pass on Dalvik.

I took a look at IMockitoConfiguration, and it's the right idea but the wrong 
interface. The main problem is that Android's implementation would also need to 
define things like the AnnotationEngine, for which we don't care. This would 
also prevent a mix of configuration options, like Android's MockMaker but my 
application's own AnnotationEngine.

But we're really close; perhaps we just want a second interface for just the 
cglib/dexmaker difference. Perhaps we'd just add a second method to 
ClassPathLoader

    public IMockMaker loadMockMaker() {
      ...
    }

From my studying, Cglib is only used in a few places. It's straightforward to 
encapsulate all of Mockito's use in an IMockMaker interface.

The other changes I made were to fix execution against classes not included in 
Android's API (BufferedImage and BeanProperty). We can fix this on the Android 
side also, by including Apache Harmony's implementations of these classes in 
the Android plugin jar. This is an easy fix. The slightly less complicated fix 
is to just avoid these APIs in Mockito, but I'm happy to do whatever your team 
prefers.

I'll put together a quick change that adds the IMockMaker interface. It'll 
probably be easier to discuss the consequences by looking at real code!

Thanks a lot guys!

Original comment by limpbizkit on 16 Jan 2012 at 3:45

GoogleCodeExporter commented 8 years ago
Very cool.

Yeah, we realize the limitations of IMockitoConfiguration but I though it might 
be a good start (there's a public implementation DefaultMockitoConfiguration 
that can be used to avoid configuring options you don't care about). However, 
IMockitoConfiguration has some fundamental limitations so if you want to 
introduce a new smartly loadable type (MockMaker) that's fine. We can sort out 
the pluggability properly a bit later so that we don't hold up the development 
of the feature. 

I need to think about this pluggability a bit more. Perhaps all extension 
points should just be well known interfaces that mockito will try to find in a 
well known location/package. Perhaps there should be something in the top level 
API to declare a plugin that is used in a given test. Will see :)

>From my studying, Cglib is only used in a few places. It's straightforward to 
encapsulate all of Mockito's use in an IMockMaker interface.

If you could offer that as a contribution that would be most awesome!

Original comment by szcze...@gradle.biz on 16 Jan 2012 at 5:07

GoogleCodeExporter commented 8 years ago
Agreed.
This is already a feat to have achieved that compatibility, but we need to 
think things beyond :P

Original comment by brice.du...@gmail.com on 16 Jan 2012 at 6:58

GoogleCodeExporter commented 8 years ago
Pull request! Here's a branch that adds the IMockMaker interface. All of the 
commits in this branch pass all of the Mockito tests when run on a JVM.
http://code.google.com/r/jessewilson-mockito/source/list?name=imockmaker

Commit #1 is a simple refactoring. It gets basic mocking working on Android 
when coupled with my private mockito-android.jar file.
http://code.google.com/r/jessewilson-mockito/source/detail?r=2865ef9cb396d41d336
de93340e6e012f2837335&name=imockmaker

Commit #2 fixes ReturnSmartNulls.
http://code.google.com/r/jessewilson-mockito/source/detail?r=884fa735e656a817b6a
2796fee88195abb5fcd11&name=imockmaker

Commit #3 changes a test's modifiers from package-private to private so that 
test passes on Android.
http://code.google.com/r/jessewilson-mockito/source/detail?r=3c5bbfdb9670abeed29
84d7585b9364f2a89c8c3&name=imockmaker

Tests still broken on Android:
 - bean properties
 - named mocks (anything that requests the name will just get 'null')
 - cglib-specific tests (ClassImposterizerTest, CGLIBHackerTest, a few in MockUtilTest)

The attached .jar file can be used to run the mockito tests on an Android 
device. Putting that .jar on the class path is sufficient; ClassPathLoader will 
find it.

My personal preference is to commit this stuff early and then iterate on the 
API between Mockito and the Android extension. This can remain an internal API 
or become a public API as you prefer. But the sooner you guys can take these 
patches the sooner we can start running Mockito on our phones!

Original comment by jessewil...@google.com on 16 Jan 2012 at 9:28

Attachments:

GoogleCodeExporter commented 8 years ago
Cool thx, I'll take a look as soon as possible :)

Original comment by brice.du...@gmail.com on 17 Jan 2012 at 8:36

GoogleCodeExporter commented 8 years ago
Just want to say that having Mockito on Android would be a true game changer 
for Android developers.

Thrilled to see progress on this.

Original comment by matth...@qype.com on 23 Jan 2012 at 3:49

GoogleCodeExporter commented 8 years ago
+1 on this. Would be happy to help

Original comment by charroch on 27 Jan 2012 at 5:22

GoogleCodeExporter commented 8 years ago
Ok. I'm starting to work on it. Hopefully it's going to be officially pulled 
shortly :)

Original comment by szcze...@gmail.com on 29 Jan 2012 at 10:25

GoogleCodeExporter commented 8 years ago
I pushed the changes. There's some things pending still (the bean setter, jvm5 
runnability, rename IMockMacker -> MockMacker). I'll do those changes later 
today :) This is good stuff Jesse, thanks a lot!!!

Original comment by szcze...@gmail.com on 29 Jan 2012 at 12:02

GoogleCodeExporter commented 8 years ago
@Jesse, question: do you really need access to MockitoInvocationHandler? I'd 
like to make the MockMaker simpler, e.g. having only:

<T> T createMock(Class<T> typeToMock, Class<?>[] extraInterfaces, 
MockSettingsInfo settings);
void resetMock(Object mock, MockSettingsInfo settings);

That would make things simpler, we wouldn't have to expose 
MockitoInvocationHandler and consequently the Invocation object.

I didn't have time to complete all stuff I wanted. Will give it a spin next 
weekend. Here're the things pending:
1. bean setter stuff. This is quite easy to implement - simply not use this 
offending introspector, etc.
2. add coverage for the custom mock maker
3. More work around MockitoInvocationHandler and Invocation interface (not 
existing yet). That depends on the answer to the question above.

Original comment by szcze...@gmail.com on 29 Jan 2012 at 6:38

GoogleCodeExporter commented 8 years ago
Awesome, thanks for your work.

I think it needs MockitoInvocationHandler. With the createMock() method you've 
proposed, I don't know how we'd handle invocations! We need some mechanism to 
tell Mockito that a method was invoked, what its arguments are, how to call 
super, and then return a value. MockitoInvocationHandler is pretty near perfect 
for that.

Original comment by jessewil...@google.com on 29 Jan 2012 at 9:21

GoogleCodeExporter commented 8 years ago
Ah, yeah of course :)

I guess in future we will figure out some way of communicating in case we need 
to tweak something. Are you going to host the AndroidMockMaker on googlecode, 
too? 

Here're the things that are likely to change on our end:
1. MockitoInvocationHandler leaks the Invocation object which at the moment is 
an internal concrete class. We will change it into an interface and place it in 
some public package.
2. The plugin loading logic might change. I still want it to be seamless, e.g. 
no explicit configuration needed in case the right jar happens to be on the 
classpath. I still want to be at (or close to) the convention laid out by 
java's ServiceLoader.

Original comment by szcze...@gmail.com on 29 Jan 2012 at 9:38

GoogleCodeExporter commented 8 years ago
Yeah, I'll host AndroidMockMaker on googlecode. I'll set up a new, clean 
repository that's forked from Mockito's mercurial repo.

Original comment by jessewil...@google.com on 29 Jan 2012 at 9:46

GoogleCodeExporter commented 8 years ago
Hi all, that is even more great news :)

About the Bean Introspector, we shall be carful as this class is supposed to be 
the standard access to Bean Properties, if we replace it it should be compliant 
with all the rules that define Properties in a Bean.
For example Spring is actually using this class.

Original comment by brice.du...@gmail.com on 30 Jan 2012 at 8:29

GoogleCodeExporter commented 8 years ago
>About the Bean Introspector

Yeah. It shouldn't be very difficult just calling the setter and comply with 
most of the rules (or the rules important to us and our users).

Original comment by szcze...@gmail.com on 30 Jan 2012 at 8:47

GoogleCodeExporter commented 8 years ago
Agreed, I just wanted to make sure it's not forgotten ;)

@Jesse By the way on Android, you probably use or have some equivalent to the 
Bean convention and also the related utility classes?

Original comment by brice.du...@gmail.com on 30 Jan 2012 at 9:22

GoogleCodeExporter commented 8 years ago
Android doesn't have much of a java.beans package. The JDK has 38 types in its 
java.beans; Android has only 4.

But it's a relatively trivial amount of code to replace PropertyDescriptor. We 
can even include Apache Harmony's copy of it in android-mockito.jar if we want 
strict API compatibility.
http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/modules/beans/
src/main/java/java/beans/PropertyDescriptor.java?view=markup

My personal preference is for Mockito to shed its dependency on the java.beans 
package and instead use a small amount of reflective code. But you can go 
either way and it's not much work for us to track that.

Original comment by jessewil...@google.com on 30 Jan 2012 at 3:54

GoogleCodeExporter commented 8 years ago
@Jesse Granted it's not that much and if java.beans don't exists on Android 
then we should implement it ourselves.

All in all I just wanted to careful on that matter. A few years ago I remember 
frameworks that didn't respect the Bean property scheme correctly which caused 
mapping problems. For example what is the property name's case of the setter 
"setURL(...)" ?

Original comment by brice.du...@gmail.com on 30 Jan 2012 at 4:33

GoogleCodeExporter commented 8 years ago
Yeah, good point. Test coverage is what we'll need there.

Original comment by jessewil...@google.com on 30 Jan 2012 at 4:36

GoogleCodeExporter commented 8 years ago
Yeah, good point. Test coverage is what we'll need there.

Original comment by jessewil...@google.com on 30 Jan 2012 at 4:36

GoogleCodeExporter commented 8 years ago
@Brice All the Java Beans introspection magic is in the java.beans.Introsepctor 
class.  To answer your specific question, its decapitalize() method has the 
following doc-comment:

     * Utility method to take a string and convert it to normal Java variable
     * name capitalization.  This normally means converting the first
     * character from upper case to lower case, but in the (unusual) special
     * case when there is more than one character and both the first and
     * second characters are upper case, we leave it alone.
     * <p>
     * Thus "FooBah" becomes "fooBah" and "X" becomes "x", but "URL" stays
     * as "URL".

Original comment by tball@google.com on 30 Jan 2012 at 5:34

GoogleCodeExporter commented 8 years ago
Yup I know since a long time about all these property (de)capitalization rules 
:)

I just raised the question as an example to make sure we don't forgot that. I 
guess this specific matter has been well discussed then :)
Anyway thx for the Javadoc pointer.

Original comment by brice.du...@gmail.com on 30 Jan 2012 at 5:47

GoogleCodeExporter commented 8 years ago
We're almost there. There are currently only two steps to running Mockito tests 
on Android:

1. Get dexmaker.jar. For now I've decided it's simple and easy to just embed an 
Android implementation of MockMaker in the main dexmaker.jar. You can get this 
here:
  http://code.google.com/p/dexmaker/

2. Replace Mockito's 
src/org/mockito/internal/util/reflection/BeanPropertySetter.java with an 
Android-compatible version, like this one:
http://code.google.com/r/jessewilson-mockito/source/browse/src/org/mockito/inter
nal/util/reflection/BeanPropertySetter.java

To run Mockito's own tests you need one more change. Fest depends on 
java.awt.image.BufferedImage, a class not available on Android. We don't 
actually exercise this code so you can drop in any class with this name. I've 
been using an empty class; available here:
http://code.google.com/r/jessewilson-mockito/source/browse/lib/test/

Mockito folks: if you can break the dependency on BeanPropertySetter then 
you'll be Android-capable without modification. I think that's pretty awesome.

Original comment by jessewil...@google.com on 30 Jan 2012 at 11:44

GoogleCodeExporter commented 8 years ago
Just a reminder as the code is using a ServiceLoader. This class is only 
available in JDK 6. And mockito is also targeting JDK 5 users. We can't use it 
as the default implementation in ClassPathLoader.

See comment here 
http://code.google.com/p/mockito/source/browse/src/org/mockito/internal/configur
ation/ClassPathLoader.java?spec=svn2d74de36c814b8e240ef94e5d0192cd11277cdf5&r=2d
74de36c814b8e240ef94e5d0192cd11277cdf5#7

In other news I commited a revised BeanPropertyStter that don't use 
Introspector.

Original comment by brice.du...@gmail.com on 31 Jan 2012 at 5:00

GoogleCodeExporter commented 8 years ago
I'm happy to contribute an alternate implementation of ClassPathLoader that 
doesn't use JDK 6's ServiceLoader. I'll put together something today.

Original comment by jessewil...@google.com on 31 Jan 2012 at 5:22

GoogleCodeExporter commented 8 years ago
@jesse For now I think the code use a simple Class.forName. I think that's the 
best thing for now.

Original comment by brice.du...@gmail.com on 31 Jan 2012 at 5:32

GoogleCodeExporter commented 8 years ago
@Jesse Also don't worry, I think ClassPathLoader might need some refactoring. 
So we might change stuff here and there. Thank you so much for your 
contribution anyway :)

Original comment by brice.du...@gmail.com on 31 Jan 2012 at 5:47

GoogleCodeExporter commented 8 years ago
Yeah, calling into ServiceLoader reflectively is a good idea. We don't need to 
support Mockito+Android on anything lower than the Java 6 APIs.

Original comment by jessewil...@google.com on 31 Jan 2012 at 6:57

GoogleCodeExporter commented 8 years ago
>We don't need to support Mockito+Android on anything lower than the Java 6 
APIs.

Agreed. However, if the general capability of plugging own MockMaker only works 
for jdk6+ then that is a bit awkward. @Jesse if you have some good ideas about 
not using ServiceRegistry we would appreciate a patch - we might use it or 
inspire on it :)

I think down the road we need something much better than ServiceLoader, e.g. 
here are the use cases:
- it needs to be testable (ServiceLoader caches eagerly which might be a 
problem)
- it needs to behave if multiple services are found (e.g. ServiceLoader just 
uses the first one on path). At the least it should warn/fail. Maybe it should 
support multiple services (although it does not make much sense for the mock 
maker).

Original comment by szcze...@gmail.com on 1 Feb 2012 at 2:13

GoogleCodeExporter commented 8 years ago
I'll put together a patch then!

Original comment by jessewil...@google.com on 1 Feb 2012 at 3:36

GoogleCodeExporter commented 8 years ago
Here's a change that gives us ServiceLoader compatibility without depending on 
ServiceLoader and JDK 6:
  http://code.google.com/r/jessewilson-mockito/source/detail?r=fa42d5e193d003e47753f4268ed9c2bb2e2ec317

This passes tests on both Android and the JDK.

Original comment by jessewil...@google.com on 3 Feb 2012 at 4:17

GoogleCodeExporter commented 8 years ago
Hey thx Jesse, your implementation looks like what we've been discussing with 
Szczepan the other day.

However I would favor the approach of using the 'getResource' instead of the 
'getResources' call because this actually scan the whole classpath. This have 
the same performance drawback as ServiceLoader. On simple projects it is not an 
issue, but on larger ones this could be become an issue. Well I don't feel 
comfortable enough on this matter.

That's why I propose that Mocklito should only use the first implementation on 
the classpath / classloader. And using the 'getResouce' call still allows 
plugability classloader wise.

Thinking about that I suppose it would be clever to cache the implementation if 
already found in the current ClassLoader. What do you think ?

Original comment by brice.du...@gmail.com on 3 Feb 2012 at 7:21

GoogleCodeExporter commented 8 years ago
Since we're only using the first resource anyway, using getResource() is a good 
idea. Would you like to make that change? It should be quite straightforward.

My patch caches the MockMaker in a static field; I think that's all the caching 
you'll need.

Original comment by jessewil...@google.com on 3 Feb 2012 at 7:32

GoogleCodeExporter commented 8 years ago
It's the end of the day and the week-end begins here in Paris.
Yep go for the change, we'll pull your code :)

> My patch caches the MockMaker in a static field; I think that's all the 
caching you'll need.
Oh yeah right forgot that fact, while reading your code this morning on my 
phone ;)
Though I wonder if that's ok if Mockito is executing in a child classloaders. 
But maybe that's over-engineering at the present time.

Original comment by brice.du...@gmail.com on 3 Feb 2012 at 7:44

GoogleCodeExporter commented 8 years ago
This changes ClassPathLoader to fetch only one resource:
http://code.google.com/r/jessewilson-mockito/source/detail?r=a696b446f9ad47411dc
ed28f6247acc8de89be78

Original comment by jessewil...@google.com on 5 Feb 2012 at 8:32

GoogleCodeExporter commented 8 years ago
Cool, thx Jesse :)

Original comment by brice.du...@gmail.com on 5 Feb 2012 at 8:37

GoogleCodeExporter commented 8 years ago
Friendly ping!

Original comment by jessewil...@google.com on 21 Feb 2012 at 9:15

GoogleCodeExporter commented 8 years ago
Hi Jesse, sorry we didn't had the time to actually make things happen lately.
But I will have some time to spend on mockito soon, so you can expect some 
feedback :)

Original comment by brice.du...@gmail.com on 22 Feb 2012 at 5:47

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Could be problematic to keep the MockMaker configuration in the META-INF folder 
as the below extract from the APK builder shows:

http://source-android.frandroid.com/sdk/sdkmanager/libs/sdklib/src/com/android/s
dklib/internal/build/SignedJarBuilder.java

// do not take directories or anything inside a potential META-INF folder.
if (entry.isDirectory() || name.startsWith("META-INF/")) {
    continue;
}

Original comment by charroch on 28 Feb 2012 at 2:36

GoogleCodeExporter commented 8 years ago
That would be problematic indeed. Jesse do you confirm ?
I don't know much about APK builder or APK, but do Android devs need to build 
the package to use mockito (at least while developping).

Original comment by brice.du...@gmail.com on 29 Feb 2012 at 11:59