Allow convenient spying on abstract classes about mockito HOT 8 CLOSED

Also, Brice suggested to me that there have been some concerns about supporting "partial mocks" because it could "bring another way to test/design stuff the wrong way".

I suppose the worry is mostly about not extracting collaborators into separate interfaces, but rather clinging them as abstract methods, resulting in the class doing too much. a.k.a the "template method" pattern.

A few reasons I don't think that concern out-weighs the benefits:

• One can already do crude "partial mock" by declaring the not-to-be-mocked methods final, such that only the abstract methods are mocked by mockito. It's a good idea to do so regardless of testing anyway to avoid too-many-choices when subclassing.
• From my experience in my workplace, it's rare that people would adopt the template-method pattern inappropriately only because they have the partial-mock support. Quite contrary, what we see happen more often is the abuse of mocks. That is, one should not have used mocks at all. A fake would have made the test more readable/maintainable. In example 1 above, UserAccount as a value-type object should have used a fake.
• A main reason why people tend to abuse Mockito/EasyMock appears to be the inconvenience in implementing fakes compared to how ridiculously easy it is to use mocks. In example 1 above, implementing a FakeUserAccount would require implementing all 14 methods even when the test only cares about two of them. Adding support for "fake" makes it easier to create fakes so people have less temptation to make mistakes.
• IMHO, template method pattern has its place. I can imagine if I'm charged to test JDK AbstractList, I'd probably want to use partial mock so that I can verify whether or how many times the get(int) user code is called into when iterator().next() is called.

Hopefully that helps to clarify the intent of this proposal.

from mockito.

Hey fluentfuture,

The use cases you described make perfect sense to me.

Basically, you propose that Mockito should offer an API to create partial
mocks of classes, without providing a class instance? For example, a
overloaded version of Spy() method that takes a class to mock as parameter,
instead of taking an instance of an object?

I think this is a very good idea and we should have it in mockito. Can you
tell use what API change do you have in mind? And yes, we would love to get
a PR :)

@brice, thoughts?

Cheers!

On Tuesday, September 30, 2014, fluentfuture [email protected]
wrote:

Also, Brice suggested to me that there have been some concerns about
supporting "partial mocks" because it could "bring another way to
test/design stuff the wrong way".

I suppose the worry is mostly about not extracting collaborators into
separate interfaces, but rather clinging them as abstract methods,
resulting in the class doing too much. a.k.a the "template method" pattern.

A few reasons I don't think that concern out-weighs the benefits:

• One can already do crude "partial mock" by declaring the
  not-to-be-mocked methods final, such that only the abstract methods are
  mocked by mockito. It's a good idea to do so regardless of testing anyway
  to avoid too-many-choices when subclassing.
• From my experience in my workplace, it's rare that people would
  adopt the template-method pattern inappropriately only because they have
  the partial-mock support. Quite contrary, what we see happen more often is
  the abuse of mocks. That is, one should not have used mocks at all. A fake
  would have made the test more readable/maintainable. For example, in my
  example 1, UserAccount as a value-type object should have used a fake.
• A main reason why people tend to abuse Mockito/EasyMock is the
  inconvenience in implementing fakes compared to how ridiculously easy it is
  to use mocks. In example 1 above, implementing a FakeUserAccount would
  require implementing all 14 methods even when the test only cares about two
  of them. Adding support for "fake" makes it easier to create fakes.
• IMHO, template method pattern has its place. I can imagine if I'm
  charged to test JDK AbstractList, I'd probably want to use partial mock so
  that I can verify whether or how many times the get(int) user code is
  called into when iterator().next() is called.

Hopefully that helps to clarify the intent of this proposal.

Reply to this email directly or view it on GitHub
#92 (comment).

Szczepan Faber
Core dev@gradle; Founder@mockito

from mockito.

Just of info this PR comes from the following issue 242.

Also I'm not strongly opinionated on this, however partial mocks have always been a concern. Yet spies can be stubbed. So if this is being developed (which have been made in a previous issue issue 242) the documentation should explain how this could be used, and the eventual danger if one chose to go the wrong way.

And other topics of where attention must be put

1. The @Fake annotation collides with test doubles terminology, as there are dummy, fakes, stubs &mocks, it may be wanted. Fakes have a precise definition, I'm not sure partial mocks with/or method template such the use cases proposed here fits in.
2. For a partial mock, I would tend to report an error when one attempt to stub a defined method. But I'm not sure about this.
3. On a technical point of view, there are challenges to instantiate the partial mock. i.e objenesis is not an option there. Calling the constructor may be the most safe option but it raises other choices to make, what if the constructor needs parameters.
   1. The best choice in the right now is to only support parameterless constructor.
   2. If constructor requires types, maybe use the constructor injection already existing. The engine is not perfect yet it maybe be easily configurable to only perform constructor injection.

That being said. I don't think it is bad idea, it's concerns that need to be addressed or postponed.

from mockito.

Thanks for the thoughts, Szczepan and Brice.

I'll answer Brice's concern and Szczepan's question together.

Fake or PartialMock?

I think even though it can be viewed and used as a partial mock to allow a poorly designed class to be mocked. It's not the point. If I have some bad class, I can already partial-mock it, simply by making the methods I don't want to mock as final:

class BadTemplateClass {
  final void doIt() {
    ...
    Thing thing = getTheOtherThing();
    ...
  }

  abstract Thing getTheOtherThing();
}

@Mock private BadTemplateClass bad;

when(bad.getTheOtherThing()).thenReturn(thing);

That is to say, not adding the proposed support doesn't do much to prevent this kind of bad design. Although, not telling people that they could use Mockito this way may help?

But then it comes to my point of naming it @Fake, because it's the real intent from users' perspective, or, put in another way, how we want it to be perceived and used by users: you can create fakes with it.

A fake differs from mocks in that it can have its own state and behavior, as in the FakeUserAccount example.

Report error on misuse.

Sounds reasonable to me. I'm not clear on how easy it is to implement in Mockito. From my experience of using our internal implementation, it hasn't become a real issue that people mistakenly call when(foo.notMockedMethod()). In fact, doesn't Mockito already report error for such case? It sees a when() call but no mockable invocation precedes it.

Instantiation

I think we definitely need the constructor to be invoked. Otherwise the fake would be in uninitialized state. In the FakeUserAccount example, the final emails field would be null.

What then if the class has constructor parameters? Or, what if the fake needs to use other mock objects?

I'll use an example to demo how we use it internally:

abstract class Player {
  Player(Buddy buddy) {...}
}

public class FooTest {
  @Mock private Buddy buddy; 
  @Fake private FakePlayer player;

  abstract class FakePlayer extends Player {
    FakePlayer() {
      super(buddy);
    }
  }
}

When we inject @Fake fields into the test object, we have the enclosing test instance. So if the class is a non-static inner class of FooTest, we pass the FooTest instance to FakePlayer's constructor.

This actually allows us to create fakes that can access any arbitrary state managed by the test.

There are also uncommon cases where we do not hope to invoke the constructor (for example, if it's a class with too many dependencies and none are needed for the purpose of the current test). For that, we just mark the constructor private. Because cglib cannot invoke private constructors, constructor is skipped in such case.

from mockito.

Hey guys,

I agree cglib is not viable for this scenario. We could introduce new methods:

a) mock(Foo.class, withSettings().spyConstructorArgs(...))
b) spy(Foo.class, Object ... constructorArgs); //delegates to method a)

Are there compelling reasons for introducing another annotation type? We could create an instance of a @SPY if the user have not provided an instance.

Thoughts?

from mockito.

Hi Szczepan,

Did you mean to say Objenesis not viable?

Regarding the mock() or spy() call, I haven't found that users need that level of power/flexibility at the cost of sacrificing static type safety and the API looking reflective. It could be used by other framework-ish code. But such utility isn't for everyday testing.

What I found so far, with the support of non-static inner class and the framework injecting the "this$0" enclosing instance automatically, it already solves the "arbitrary constructor parameter" problem. And the code is completely static type safe.

So my suggestion is to support non-static inner class but not arbitrary constructor parameters.

Either

Foo foo = spy(Foo.class);

or use annotation

@Spy private Foo foo;

MockitoAnnotations.initMocks(this);

Inner classes are only created with the annotation and initMocks().

from mockito.

While implementing it. I realized that there is already @Spy field injection when the user doesn't provide an instance:
http://docs.mockito.googlecode.com/hg/1.9.5/org/mockito/Spy.html

So we can't reuse @Spy unless we are willing to break existing users.

Does any of @Partial, @PartialMock, @MockAbstract or @Fake sound okay to add?

from mockito.

So we can't reuse @SPY https://github.com/Spy unless we are willing to
break existing users.

Why would we break existing users?

Feel free to work on the java API first, we can deal with an annotation
later.

Cheers!

On Sun, Oct 12, 2014 at 6:15 AM, fluentfuture [email protected]
wrote:

While implementing it. I realized that there is already @SPY
https://github.com/Spy field injection when the user doesn't provide an
instance:
http://docs.mockito.googlecode.com/hg/1.9.5/org/mockito/Spy.html

So we can't reuse @SPY https://github.com/Spy unless we are willing to
break existing users.

Does any of @partial https://github.com/Partial, @PartialMock or @Fake
https://github.com/Fake sound okay to add?

Reply to this email directly or view it on GitHub
#92 (comment).

Szczepan Faber
Core dev@gradle; Founder@mockito

from mockito.