@SpyBean and @MockBean fails on verify, after spring-tx is added to classpath

[robersongomes]

I'm using spring-boot 1.4.0.M2 and I having some trouble using @SpyBean and @MockBean in my tests.
Both annotations were working fine, but after I added spring-boot-starter-jdbc to my project, my tests started to fail on spy/mock verifications with:

org.mockito.exceptions.misusing.UnfinishedVerificationException:
Missing method call for verify(mock) here:

I removed spring-boot-starter-jdbc dependency and added spring-tx and the error still there.

I've created a project reproducing the issue: https://github.com/robersonccgomes/spring-boot-spybean-bug

[snicoll]

The @Repository declaration on MyRepository is involved with the issue. I remember some BPP that would adapt the created bean.

[snicoll]

I have a similar problem with caching. Let's say I have FooService that I use in FooController and I am using constructor injection.

If FooService is a proxy, I get the following:

Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [com.example.cfp.web.GithubController]: Illegal arguments for constructor; nested exception is java.lang.IllegalArgumentException: argument type mismatch
    at org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:156)
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:122)
    at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:271)
    ... 40 more
Caused by: java.lang.IllegalArgumentException: argument type mismatch
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:408)
    at org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:147)
    ... 42 more

If I change my vanilla @EnableCaching annotation to @EnableCaching(proxyTargetClass = true) I get the exact same message as @robersonccgomes

org.mockito.exceptions.misusing.UnfinishedVerificationException: 
Missing method call for verify(mock) here:
-> at com.example.cfp.integration.github.GithubClient$$FastClassBySpringCGLIB$$1ff559cd.invoke(<generated>)

[wilkinsona]

Mockito is getting confused by the two layers of proxying. The spy bean has created one proxy and persistence exception translation has created another. The reproduction includes this test:

@Test
public void test() {
    service.doSomething(99);
    Mockito.verify(repository).save(99);
    Mockito.verifyNoMoreInteractions(repository);
}

Things go wrong when save is called. Mockito's MockAwareVerificationMode believes that the mock is MyRepository$$EnhancerBySpringCGLIB$$b2127210, whereas its InvocationImpl believes that it's MyRepository$$EnhancerByMockitoWithCGLIB$$cd452aee. It sees that the two "mocks" are different and re-adds the verification mode to the ThreadSafeMockingProgress. From Mockito's perspective, this verification is never completed so it fails with an UnfinishedVerificationException.

[wilkinsona]

I had thought that one way to fix this would be to ensure that the proxy for the spy is created last. However, this isn't possible as Spring Framework's ProxyProcessorSupport (a super class of PersistenceExceptionTranslationPostProcessor) is ordered with lowest precedence "so that it can just add an advisor to existing proxies rather than double-proxy". This doesn't work in this case as the Mockito-created proxy doesn't implement Advised so another proxy is created.

[wilkinsona]

An alternative is to "unwrap" the proxy so that Mockito's proxy is injected into the test rather than Spring Framework's:

diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java
index e76208e..2833bfc 100644
--- a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java
+++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java
@@ -26,6 +26,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;

+import org.springframework.aop.framework.Advised;
 import org.springframework.beans.BeansException;
 import org.springframework.beans.PropertyValues;
 import org.springframework.beans.factory.BeanClassLoaderAware;
@@ -308,6 +309,9 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda
                        Assert.state(ReflectionUtils.getField(field, target) == null,
                                        "The field " + field + " cannot have an existing value");
                        Object bean = this.beanFactory.getBean(beanName, field.getType());
+                       if (bean instanceof Advised) {
+                               bean = ((Advised) bean).getTargetSource().getTarget();
+                       }
                        ReflectionUtils.setField(field, target, bean);
                }
                catch (Throwable ex) {

I'm unconvinced, though. Any calls that the test makes to the spied class will miss out on any functionality that was added by Spring Framework's proxy.

[philwebb]

@robersonccgomes Thanks for the issue report and for the sample application, it was very helpful for tracking down the cause.

[robersongomes]

I'm glad I could help @philwebb.
Thank you for the awesome job you are doing on spring test support!

[snicoll]

@philwebb our sample app now works as expected with that fix. Thanks!