spock icon indicating copy to clipboard operation
spock copied to clipboard

Don't mock interface default methods

Open chrylis opened this issue 7 years ago • 6 comments

I have an interface that has a default method overload that can transform an instance of any sort of bar (in this case, message format) into a canonical format for processing.

interface Foo {
  void doThing(SpecificType bar);

  default void doThing(SuperType bar) {
    doThing(new SpecificType(bar));
  }
}

I want to test that when I call Foo#doThing with a SomeOtherBar, the message gets correctly transformed and sent out the SpecificType overload. However, Spock's Mock() intercepts all methods, including default methods.

Since one main purpose of default methods is to support adaptation onto legacy APIs, this behavior will prevent backwards-compatibility bridges from working if a provider API is upgraded. Instead, it should be possible to either by default (my preference) or explicitly exclude default interface methods from being implemented by the mock proxy.

chrylis avatar May 15 '17 00:05 chrylis

You could use a mechanism usually used in Spy() to instruct Spock to call a real method on the underlying object.

given:
    Foo foo = Mock()
    foo.doThingDefault(_) >> { callRealMethod() }

However, in the situation where both methods have the same name the syntax is more complicated as Spock takes the first stubbing for the same method name, even with a different input argument type. Therefore, you need to use something like:

given:
    Foo foo = Mock()
    foo.doThing(_) >> { it[0] instanceof String ? it[0] : callRealMethod() }

(to get know why you can't return just it, but it[0] you can check my presentation from Gr8Conf 2016 :) ).

It works fine with Spock 1.1 which migrated to ByteBuddy (I haven't tested it with cglib).

Btw, I'm not sure if it is worth to have an ability to disable mocking default methods globally.

szpak avatar May 15 '17 07:05 szpak

I'm aware of the "first stub" issue, and it feels like a bug (specifically, that it happens even if I explicitly say doThing(_ as SpecificType)). And I've been using Spock for a while and reflexively dance around the it semantics now. ;-)

Why would it not be worth it? The rule is unambiguous, default methods are easy to identify, and there are numerous instances, such as with the Collection types retrofit, where checking that the type under test Does The Right Thing when a default method does some sort of wrapping or delegation is immensely useful, especially for regression control where you start out using a general-purpose unoptimized default implementation and then later come back to add a custom one.

chrylis avatar May 17 '17 10:05 chrylis

This same bug bit me again today on Spock 1.2 for this exact case: The default method is a wrapper that does some useful conversion and delegates to the underlying. Mocking the underlying does no good because Spock wipes out the default method.

chrylis avatar Dec 08 '19 05:12 chrylis

And callRealMethod() doesn't work in 1.2. In fact, the situation is actually worse, because the documented default behavior for mock with a return type of Optional is empty, but a default method on a mocked interface returns null. Debugging, it appears that once you use callRealMethod, you have permanently exited the Spock mock system, and the attempt by the default method to self-call the underlying method that should be mocked results in "Cannot invoke real method in interface based mock object".

chrylis avatar Dec 08 '19 05:12 chrylis

Optional.empty() is used by default only for stubs, not mocks.

@chrylis Could you send a complete specification (a test class) that reproduce the problem? Does that particular example work in Spock 1.1?

szpak avatar Dec 08 '19 10:12 szpak

I have a repro of the underlying concerns.

To summarize:

  • Wiping out a default implementation in any test double that does not have a declared interaction violates the Rule of Least Surprise. I want to test that the subject makes the correct DB call, not whether it uses the default method to perform type conversion or does it inline (an implementation detail and the purpose of default methods).

  • Every test double in Spock overwrites default methods.

  • Neither Mock() nor Spy() can use callRealMethod() to explicitly re-delegate to the default method.

  • Stub() can use callRealMethod() to the default method but then cannot make cardinality assertions on the test double, which severely reduces its usefulness.

Proposed behavior change in Spock: If an interface method is a default method, then Spock should simply do nothing (that is, let the default stand) unless it has an explicit interaction.

chrylis avatar Dec 20 '19 00:12 chrylis