dhamini-poornachandra / mockito

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

Mocking a Groovy class results in Mockito throwing a NullPointerException #303

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
Mocking a Groovy class results in a NullPointerException

Steps to reproduce:
// Make 2 Production classes

//This can be Java or Groovy
class Simple {
    MyGroovyDependency dependency
    void hello(){
        dependency.say('hello')
    }
}

//This must be Groovy to demonstrate problem
class MyGroovyDependency {
    String say(String message) {
        println(message);
        return "";
    }
}

2.
// Write a test (Groovy or Java), and mock a groovy classes
class SimpleTest {
    MyGroovyDependency dependency
    Simple simple

    @Before
    void setUp() {
        dependency = mock(MyGroovyDependency.class)
        simple = new Simple(dependency: dependency)
    }

    @Test
    void hello() {
        when(dependency.say('message')).thenReturn('') //this line NPEs
        simple.hello()
    }
}

3. Run the test

Output:
java.lang.NullPointerException
    at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.call(PogoMetaClassSite.java:39)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:42)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:108)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)

This issue has been around for a while. See this thread from April 2010.
http://groups.google.com/group/mockito/browse_thread/thread/7ce820362cefbb61/af7
41a1e87ea0656

Debugging into the code the problem can be observed in 
MethodInterceptorFilter::intercept. A toString() of the 'method' parameter 
evaluates to "public groovy.lang.MetaClass MyGroovyDependency.getMetaClass()" 
as opposed to "public groovy.lang.MetaClass 
MyGroovyDependency.say(java.lang.String)"

The suggestions to date have been 'Use GMock', however the codebase I'm working 
on has significant amounts of Mockito and a conversion is not feasible right 
now.

Any suggestions, workarounds or help would be really appreciated.

Original issue reported on code.google.com by steve.qu...@gmail.com on 14 Dec 2011 at 11:17

GoogleCodeExporter commented 8 years ago
The problem is that calls from Groovy code to methods on Groovy classes go 
through the MetaClass, so any Groovy method call starts by calling 
getMetaClass(), for which Mockito returns null.

Here is a patch that fixes the problem. (It also requires groovy-all-1.8.6.jar 
to be added to lib/compile -- it's available at 
http://groovy.codehaus.org/Download).

We're working on a small library that can work around the problem in the 
current version of Mockito.

Thanks,
Laura Dean and Moss Collum, Cyrus Innovation

Original comment by mcol...@cyrusinnovation.com on 5 Apr 2012 at 5:53

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks for the patch, we'll take a look at it :)

Original comment by szcze...@gmail.com on 5 Apr 2012 at 6:30

GoogleCodeExporter commented 8 years ago
Hi Laura, hi Moss,

Nice hack, and thx for the patch. However we don't want to introduce any groovy 
dependency in our code.

Looking at the code you provided it might be possible to do it via 
introspection anyway. But we'll loose compile time safety.

> We're working on a small library that can work around the problem in the 
current version of Mockito.

What do you have in mind aside from this patch ?

Cheers,
Brice

Original comment by brice.du...@gmail.com on 5 Apr 2012 at 6:30

GoogleCodeExporter commented 8 years ago
Hi Brice,

Thanks for the speedy feedback! We thought the Groovy dependency might be an 
issue. The introspection idea sounds appealing.

We've just written a library that provides a MockitoConfiguration and an Answer 
implementation that do what this patch does, but from outside of Mockito. It's 
not quite as convenient as having Mockito work with Groovy out of the box, but 
it is a great workaround for now.

We've shared our code at 
https://github.com/cyrusinnovation/mockito-groovy-support

Thanks,
Laura and Moss

Original comment by mcol...@cyrusinnovation.com on 5 Apr 2012 at 9:03

GoogleCodeExporter commented 8 years ago
Wow, this is cool :)

Agreed it would be nice if it worked out of the box. Still, using introspection 
though needs discussion, as it is an unsafe development that might impose 
maintainability issues as well.
Might be worth enquiring to the groovy developers on the stability of these 
APIs.

Cheers,
Brice

Original comment by brice.du...@gmail.com on 5 Apr 2012 at 10:10

GoogleCodeExporter commented 8 years ago
Laura and Moss,

I love the idea :)

I think you should be able to put MockitoConfiguration class in your jar and 
that would work, too. It would be less explicit but perhaps more convenient.

Feel free to give us feedback on what would you like the API for extending 
Mockito to look like. Since you are interested only in providing custom default 
answer - perhaps there should be a dedicated extension point for the default 
answer?

Cheers!

Original comment by szcze...@gmail.com on 6 Apr 2012 at 10:44

GoogleCodeExporter commented 8 years ago
We thought about having a MockitoConfiguration in ours, but figured it would 
cause trouble if someone already had a MockitoConfiguration in place, because 
then they'd have two in the classpath.  The extension point is an interesting 
idea!

Original comment by lgd...@cyrusinnovation.com on 6 Apr 2012 at 4:59

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Substituting the default answer only works to a certain degree. Groovy's calls 
to getMetaClass() will still make verification impossible.

The way I see it, getMetaClass is a special method that can not be mocked it 
just need to be handled. This can (should?) be done in the same fashion as 
mockito handles equals and hashCode, in the MethodInterceptor.

I've created a patch to Laura and Moss' mockito-groovy-support library. It 
substitues Mockito's default implementation of MethodInterceptor with an 
implementation that handles getMetaClass().

It would be nice if the MethodInterceptor was picked up from 
MockitoConfiguration. With the current implementation of mockito, the only way 
to substitute the method interceptor is by directly modifying the mockito proxy.

Patch: 
https://github.com/gogstad/mockito-groovy-support/commit/297b58a8d46fca373d58f56
c0c51daccd4c2b1b3

Original comment by jostein....@gmail.com on 9 Nov 2012 at 5:19

GoogleCodeExporter commented 8 years ago
Thanks for reporting.

We could add a direct handling for getMetaClass() in Mockito. Although it is 
not super nice to add special handling for specific languages it feels cheap 
enough.

Alternatively we could add/improve an extension point. Few options:
1. Add an extension point to provide own MethodInterceptor with a way to access 
the default one. This way, the groovy extension can wrap the default 
interceptor and add any handling that is needed.
2. Add extension point to short-circuit special methods. Currently methods with 
special handling are equals() and hashCode(). To some extent, also toString() 
but let's not worry about toString() just now.

In general, spock framework can be used for groovy mocking. It does have very 
nice support for mockito style mocks (e.g. spies).

Hope that helps!

Original comment by szcze...@gmail.com on 10 Nov 2012 at 6:39

GoogleCodeExporter commented 8 years ago
Special handling of getMetaClass in mockito does feel cheap. Thumbs up for 
alternative  1.

Original comment by jostein....@gmail.com on 12 Nov 2012 at 8:06

GoogleCodeExporter commented 8 years ago
I've been tinkering with the mockito sources this afternoon, and it looks to me 
like option 2 would actually be pretty straightforward to implement, and would 
provide everything needed for Groovy support. I'm experimenting here:

https://github.com/moss/mockito/tree/short-circuit-method-extension-point

and I'll send a pull request when it's put together a little more completely.

Original comment by mcol...@cyrusinnovation.com on 3 Feb 2013 at 11:37

GoogleCodeExporter commented 8 years ago
Any updates on this issue? Even if adding support for getMetaClass() directly 
to Mockito doesn't make sense, an optional Groovy Mockito dependency to add it 
would fix what's really a blocker for any Groovy classes.

Original comment by christop...@artsquare.com on 18 Jun 2014 at 8:36

GoogleCodeExporter commented 8 years ago
It seems like the mockito-grovy-support dependency should fix this. Is that 
true? is this issue closed now?

Original comment by taylory...@gmail.com on 24 Jun 2014 at 10:38

GoogleCodeExporter commented 8 years ago
Interesting, that may be a better alternative, as it does not imply groovy code 
inside mockito. And it use the MockMaker extension point.

Maybe groovy user can contribute wiki page for that. And adding links to this 
groovy MockMaker https://github.com/cyrusinnovation/mockito-groovy-support

I'd like to close this issue.

Original comment by brice.du...@gmail.com on 27 Jun 2014 at 3:04