MockitoMockMaker provides mocking of static methods

[AndreasTu]

MockitoMockMaker supports to mock static methods
of Java classes with the new Spec API:

SpyStatic(Class)
SpyStatic(Class, IMockMakerSettings)

This also supports Java callers of the static methods.

Usage sample:

given:
SpyStatic(StaticClass)
when:
def result = StaticClass.staticMethod()
then:
1 * StaticClass.staticMethod() >> MOCK_VALUE
result == MOCK_VALUE

[codecov]

Codecov Report

Attention: Patch coverage is 94.41341% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 81.53%. Comparing base (2c7db77) to head (9ece5c8).
Report is 15 commits behind head on master.

Files	Patch %	Lines
...ork/mock/runtime/mockito/MockitoMockMakerImpl.java	83.67%	6 Missing and 2 partials ⚠️
...rg/spockframework/mock/runtime/MockController.java	93.33%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1756      +/-   ##
============================================
+ Coverage     80.44%   81.53%   +1.08%     
- Complexity     4337     4470     +133     
============================================
  Files           441      442       +1     
  Lines         13534    13840     +306     
  Branches       1707     1735      +28     
============================================
+ Hits          10888    11284     +396     
+ Misses         2008     1912      -96     
- Partials        638      644       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[AndreasTu]

@leonard84 As a hint, I needed to push the ThreadLocal data into the MockitoInlineMockMakerImpl, because the thread-local behavior of Mockito only works, if you create a new MockMaker.StaticMockControl for each thread.
So the MockitoInlineMockMakerImpl needs to create a StaticMockControl for every Thread, which will activate the static mocks, so we can't handle the ThreadLocal-State in the IStaticMockController.

I also adapted the javadoc on IMockMaker.IStaticMock that different 3rd-party implementations need to handle the thread-local stuff on their own.

[AndreasTu]

I have created an issue in the Mockito repo to request the API for the same inline mockMaker instance, and not relying on private method from Mockito in the method org.spockframework.mock.runtime.mockito.MockitoInlineMockMakerImpl.resolveInlineMockMaker()

[leonard84]

LGTM, but I'd like some additional feedback from @spockframework/supporter as we are now extending the API in Specification in a bigger way.

[leonard84]

A thing that came to mind is, whether we should use global: true or static: true for java mocks instead of adding these new methods.

given:
Mock(StaticClass, static: true)
when:
def result = StaticClass.staticMethod()
then:
1 * StaticClass.staticMethod() >> MOCK_VALUE
result == MOCK_VALUE

[AndreasTu]

@leonard84 that would not work as you might expect. The Mock/Stub/SpyStatic do two things differently as the others methods.

1. The return void because there is not mock.
2. the interaction do not delegate to the type <T> instance, but to the Class<T>.
   If we use the existing Mock/Stub/Spy methods it would also break the interactions closures.

[leonard84]

The return void because there is not mock.

We could just return null and document that fact.

the interaction do not delegate to the type instance, but to the Class.
If we use the existing Mock/Stub/Spy methods it would also break the interactions closures.

Correct me if I'm wrong, but the declarations look indentical, except that you leave out the @ClosureParams(FirstParam.FirstGenericType.class), so what would break here?

  @Beta
  public <T> T Mock(
    @DelegatesTo.Target
      Class<T> type,
    @DelegatesTo(strategy = Closure.DELEGATE_FIRST, genericTypeIndex = 0)
    @ClosureParams(FirstParam.FirstGenericType.class)
      Closure interactions) {
    throw invalidMockCreation();
  }

  public <T> void MockStatic(
    @DelegatesTo.Target
    Class<T> type,
    @DelegatesTo(strategy = Closure.DELEGATE_FIRST, genericTypeIndex = 0)
    Closure<?> interactions) {
    throw invalidMockCreation();
  }

[leonard84]

I was wondering IIRC the inline mocking of Mockito would actually enable global mocking, as it instruments every class looks up if the current instance is "mocked" so if you return always true, then it would behave like a global mock right?

[AndreasTu]

except that you leave out the @ClosureParams(FirstParam.FirstGenericType.class), so what would break here?

So the @ClosureParams would suggest that the parameter passed to the interaction closure is an instance of Class<T>, but there is no instance we can pass to that Closure in the static case, because we do not create one, or maybe we even can't create one.
And the delegate is working because we can delegate to the class instance instead.

[AndreasTu]

I was wondering IIRC the inline mocking of Mockito would actually enable global mocking, as it instruments every class looks up if the current instance is "mocked" so if you return always true, then it would behave like a global mock right?

Theoretically yes, but this would at least require more API on the org.mockito.plugins.MockMaker interface, that we can register a global callback for every instance of a certain class. Currently the API only provides such callbacks for an object created by mockito not for any object. See org.mockito.plugins.MockMaker.createMock() and org.mockito.plugins.MockMaker.createSpy()

We could ask the mockito guys, how they would feel about that.

But what do you want to accomplish with that?

[leonard84]

except that you leave out the @ClosureParams(FirstParam.FirstGenericType.class), so what would break here?

So the @ClosureParams would suggest that the parameter passed to the interaction closure is an instance of Class<T>, but there is no instance we can pass to that Closure in the static case, because we do not create one, or maybe we even can't create one.
And the delegate is working because we can delegate to the class instance instead.

You are mistaken, if you leave out the explicit param it will use it if you look at the generated code via gw coreConsole or an AST test, you can see that

    MockStatic(StaticClass) {
      staticMethod() >> "MockValue"
    }

will be compiled to

    this.MockStaticImpl(null, null, StaticClass, { ->
        this.getSpecificationContext().getMockController().addInteraction(new org.spockframework.mock.runtime.InteractionBuilder(3, 7, 'staticMethod() >> \"MockValue\"').addEqualTarget(it).addEqualMethodName('staticMethod').setArgListKind(true, false).addConstantResponse('MockValue').build())
    })

it doesn't use the delegate but .addEqualTarget(it).

The closure parameter is only used to rename it to something else

    MockStatic(StaticClass) { sc ->
      sc.staticMethod() >> "MockValue"
    }

    this.MockStaticImpl(null, null, StaticClass, { java.lang.Object sc ->
        this.getSpecificationContext().getMockController().addInteraction(new org.spockframework.mock.runtime.InteractionBuilder(3, 7, 'sc.staticMethod() >> \"MockValue\"').addEqualTarget(sc).addEqualMethodName('staticMethod').setArgListKind(true, false).addConstantResponse('MockValue').build())
    })

and here .addEqualTarget(sc)

[leonard84]

I was wondering IIRC the inline mocking of Mockito would actually enable global mocking, as it instruments every class looks up if the current instance is "mocked" so if you return always true, then it would behave like a global mock right?

Theoretically yes, but this would at least require more API on the org.mockito.plugins.MockMaker interface, that we can register a global callback for every instance of a certain class. Currently the API only provides such callbacks for an object created by mockito not for any object. See org.mockito.plugins.MockMaker.createMock() and org.mockito.plugins.MockMaker.createSpy()

We could ask the mockito guys, how they would feel about that.

But what do you want to accomplish with that?

Well, the same as in Groovy, it intercept things that you could't create and inject yourself.
For example, a singleton that uses instance methods.

[AndreasTu]

Yes it is the Class<T> instance and not T during runtime inside of the closure.
It is more for the IDE not to try to complete and display the type for T.
My point is, that the User should not be mislead by the type inference displaying the type T.

You can see the runtime resolution in the class org.spockframework.lang.SpecInternals.createStaticMock():
Where it is called with GroovyRuntimeUtil.invokeClosure(closure, type); which sets the param to type.

Maybe in general: I find the behaviour and user expectation of static mocks different enough to justify the new *Static() methods.
But if the new API in MockingApi is the main concern, I also can live with the mock option static=true.
But this would get me back to the discussion in #1878 about the mock options.

[leonard84]

Maybe in general: I find the behaviour and user expectation of static mocks different enough to justify the new *Static() methods.
But if the new API in MockingApi is the main concern, I also can live with the mock option static=true.

I think they are comparable to global: true and think we should go with static: true.

[AndreasTu]

I will change the API to onyl provide:

*SpyStatic(Class)
*SpyStatic(Class, IMockMakerSettings)

instead of the static:true and the other methods, because the main usage is to intercept a single static method with SpyStatic instead of mocking all static methods of a class.

[Vampire]

Sorry that I didn't have time to have a look at this earlier.
I had a cursory look now (mainly at the docs) and generally quite like it.
Some points though:

• It seems there used to be StubStatic and MockStatic too. I actually wonder why they were removed and there is also no "A la carte" way to create the mock for example with a different or custom defaultResponse. Is that intentional, or can we add this? I mean, you can of course easily use StaticClass._ >> { ZeroOrNullResponse.INSTANCE.respond(delegate) } and StaticClass._ >> _ to achieve the same, it is just not as convenient. :-)
• "Used to be there" I assumed, because it is still in the docs in the mockito mock maker section: "Mock/Stub/SpyStatic", and also the plural form in "The semantics are the same as for the non-static variants:", and the plural form in "The static mock methods will".
• The whole documentation of the feature is extremely unlucky, because the "Mocking Static Methods" chapter is a sub-chapter of "Groovy Mocks" as you just extended the section about global Groovy mocks. This has to be split or at least moved. I'm not sure whereto, but probably to "Other Kinds of Mock Objects". If you look at the all-in-one page, you can quite nicely see the overall structure on the left.

[Vampire]

And the link in the release notes on the all-in-one page is broken

[AndreasTu]

@Vampire I will open up a new PR to fix the documentation issues.
To your questions:

It seems there used to be StubStatic and MockStatic too. I actually wonder why they were removed

Yes there were also MockStatic/StubStatic, but after a long discussion with @leonard84 we decided to remove them together with all the option and interaction overloads to not complicate the MockingApi with so many methods.
Especially a user should probably only mock 1 or 2 static methods of a Class in a certain test case.
To take over all the static methods of a class is far more invasive than for a single mock instance.
So we thought, just start with the SpyStatic variant and wait, if this would be sufficient.

And yes you ca still do that as you decribed with the interaction, so there is also a work around, I will document that.