This is definitely an enhancement request, but here goes

1. Start with a final class that implements an interface
2. I want to spy an implementation of that final class
3. And fake some method calls.

=== Given: final class FooImpl implements Foo { @Override Object doStuff() { .... } @Override void close() { .... } }

=== For example: FooImpl real = new FooImpl(); Foo mock = Mockito.mock(Foo.class, withSettings() .spiedInstance(real) .defaultAnswer(Mockito.CALLS_REAL_METHODS)); doNothing().when(mock).close();

// give mock to somebody, have then do work // then validate the state of 'real' before it is closed

=== Error: org.mockito.exceptions.base.MockitoException: Mocked type must be the same as the type of your spied instance. Mocked type must be: FooImpl, but is: Foo //correct spying: spy = mock( ->ArrayList.class<- , withSettings().spiedInstance( ->new ArrayList()<- ); //incorrect - types don't match: spy = mock( ->List.class<- , withSettings().spiedInstance( ->new ArrayList()<- ); at com.tripwire.cap.logger.impl.pipeline.ResendEnvelopeOrderingTest.shouldAdjustPendingForSerial(ResendEnvelopeOrderingTest.java:48)

=== Given the error message, it looks like you've considered and rejected the usage I'm proposing, but I couldn't find anything in issues documenting your rationale.

For myself, I'm a strong proponent of final classes (and am not alone, given "Item 17: Design and document for inheritance or else prohibit it" in Effective Java). It seems unfortunate to make FooImpl non-final to be able to spy it directly.

Thanks, -dg

Hi,

I have mixed feeling about that one, however it might be ok to implemented for this case, if there is a consensus in the team.

About the exception, it's just here to prevent usage not provided by mockito, it can change if the feature is implemented or for some other reasons.

Mockito's core can only subclass existing implementations, for this reason we cannot work with final classes. With PowerMock which alter the class definition you could spy final classes.

While I'm some kind of a fervent admirer of Joshua Bloch, I think test driven development allows you balance well code with an open design, and classes non extendable through inheritance. In this last case, those object tend to be small in feature and code and then never mocked. Finally as I'm not a relly into inheritence, the result is that in essence the overall design is musch better.

Anyway thx for sharing your ideas, they have a point.

Thanks for the comment. Hopefully the low API weight and the fact that you don't actually need (in this instance) to make a subclass of the spied type -- since it should work if you use a proxy based on the interface which delegates all non-intercepted methods to the real class -- will be enough to get this in.

I've only had to do something like this twice: once where the interface was small enough I could do it by hand easily, and once when it was large enough to use a java.lang.reflect.Proxy-based implementation. I did miss the expressiveness of Mockito, but it was not crisis.

So, yeah. Not a core use perhaps. Thanks for the consideration.

Hi, potential feature in issue 275 can help you this case. What do you think of this :

mock(Foo.class, new ForwardingAnswer(real));

Or more user friendly :

mock(Foo.class, withSettings().forwardTo(real));

Both mock(Foo.class, new ForwardingAnswer(real)); and mock(Foo.class, withSettings().forwardTo(real)); read good to me. I think both are understandable.

==Ignorable babbling below==

It's (slightly) unfortunate that neither are type safe -- mock(Class,Answer) parameterizes Class but Answer is parameterized on the Answer type (duh) rather than the target type. And MockSettings isn't type safe anyhow.

In fairness, neither was my suggested syntax.

As an implementation aside, does new ForwardingAnswer(real) need to know about Foo.class? I ask b/c it seems like InvocationOnMock#getMethod will return a method of Foo. And so I guess you just getMethod().invoke(real, getArguments()) and if that doesn't work (say, real no longer implements Foo) then that's an IllegalArgumentException at runtime.

I think that's acceptable for something so edgy.

Thanks again, -dg

(P.S. The more I understand about the Mockito code base, the more I like it. Bravo!)

You raise a point, I guess using MockSettings is more reasonable on this matter; we could actually do some additional checks at mock creation.

I'm jumping in quite late, sorry about that.

Brice, did you check why we force the types to be equal? I don't see a reason why we should but it might be dictated by the internal design. It feels that we should simply allow the suggested use case. I wonder what's going to happen if we remove that check and run all tests :D

I think the real issue here is that the real instance is final and as such it could neither be mocked nor be spied.

About the control over the types, it's is important because the mock is created from the mockedType, and then fields from the original instance are copied to the mock.

So if I write this :

LinkedList subtype = new LinkedList() {{ add("Yup"); }}; List list = mock(List.class, withSettings().spiedInstance(subtype).defaultAnswer(CALLS_REAL_METHODS)); assertNotNull(list.get(0));

Then it won't work because List.class has no real implementation or the wrong implementation. Same can be said if using an upped type such as AbstractList.

And anyway without defining the defaultAnswer a mock is just created but it might be without some or all fields of the spied instance.

Then it won't work because List.class has no real implementation or the wrong implementation.

Hmm, not sure... (feel free to correct me if I'm wrong)

list.get(0) is proxied because it is a method on a List. If it's proxied then our implementation can delegate to whoever implement List and is the 'spied instance'.

Makes sense?

That's the trick here :)

For now we can only write this : mock(ArrayList.class, withSettings().spiedInstance(new ArrayList()));

It works because ArrayList has the real implementations (for a spy). If we allow different types, such as the following : mock(List.class, withSettings().spiedInstance(new ArrayList()));

Then we will only have a mock, the LenientCopy tool won't fail when copying the state of the original instance. After that the original spiedInstance is lost.

list.get(0) is proxied because it is a method on a List. If it's proxied then our implementation can delegate to whoever implement List and is the 'spied instance'.

Agreed. But not with this kind of way, I find it cumbersome : mock(List.class, withSettings().spiedInstance(newArrayList()).defaultAnswer(CALLS_REAL_METHODS));

Beside to make this work, it would require some rewriting on how spies are made. (For now without changing how spies are created the following call list.get(0) raise an exception because the implementation of the method is not found (tested))

That's why I was proposing this way of delegating to a real non proxied instance : mock(List.class, withSettings().forwardTo(realArrayList));

Ok, I understand now. AFAIR, the very first implementation of spying in mockito was based on plain 'forwarding' idea. However, it proved limited due to non-proxied methods (final & private methods) accessing private state and breaking badly in runtime. Hence we 'improved' spying and added the logic that copied over the private state. I think that's the one of the reasons we needed exactly the same types.

Let's say we introduce 'forwardTo'. What would be the difference between 'spiedInstance' + defaultAnswer? Do you want it to be just an alias?

Yeah, makes sense to me.

About the future API, it shouldn't be aliases as I'm not sure if we could introduce regression, if we relax the spying api. And I'd like to think that people use Mockito.spy (or@Spy) to create spies, we should encourage that behavior in the message thrown by MockCreationValidator. So I think the forwardTo, should actually prepare an answer that will forward to a real object reference. At the same time it will have the benefit of solving the case for people working with JNI/JNA stuff, as in issue 275.

In the end, duplicate of issue 145 ?

I think it's a good idea to merge them

Ok, done

Hi all,

We had a very nice "hacketgarten" yesterday in Paris, with a mockito workshop. Brice coached me to implement this new feature, which I did. I sent him the patch file for validation. Maybe it will make it to the source tree ?

José