ResetMocksTestExecutionListener does not tolerate bean factory methods that throw an exception

[dsyer]

This test fails if you run it with Spring Boot 1.4: https://github.com/spring-cloud/spring-cloud-netflix/blob/master/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientOverrideDefaultsTests.java. the proxies are created by Feign (not Mockito) and yet Mockito seems to think they belong to it, and null everything out in them. Splat!

[wilkinsona]

@dsyer The behaviour that I'm seeing doesn't match the behaviour you have described. The failure that I see is:

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.cloud.netflix.feign.FeignClientOverrideDefaultsTests$FooClient': FactoryBean threw exception on object creation; nested exception is java.lang.IllegalStateException: Method get not annotated with HTTP method type (ex. GET, POST)
    at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.doGetObjectFromFactoryBean(FactoryBeanRegistrySupport.java:175)
    at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.getObjectFromFactoryBean(FactoryBeanRegistrySupport.java:103)
    at org.springframework.beans.factory.support.AbstractBeanFactory.getObjectForBeanInstance(AbstractBeanFactory.java:1595)
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:254)
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:197)
    at org.springframework.boot.test.mock.mockito.ResetMocksTestExecutionListener.resetMocks(ResetMocksTestExecutionListener.java:61)
    at org.springframework.boot.test.mock.mockito.ResetMocksTestExecutionListener.resetMocks(ResetMocksTestExecutionListener.java:49)
    at org.springframework.boot.test.mock.mockito.ResetMocksTestExecutionListener.beforeTestMethod(ResetMocksTestExecutionListener.java:39)
    at org.springframework.test.context.TestContextManager.beforeTestMethod(TestContextManager.java:269)
    at org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks.evaluate(RunBeforeTestMethodCallbacks.java:74)
    at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:86)
    at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:84)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:252)
    at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:94)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
    at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:191)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: java.lang.IllegalStateException: Method get not annotated with HTTP method type (ex. GET, POST)
    at feign.Util.checkState(Util.java:128)
    at feign.Contract$BaseContract.parseAndValidateMetadata(Contract.java:97)
    at feign.Contract$BaseContract.parseAndValidatateMetadata(Contract.java:64)
    at feign.ReflectiveFeign$ParseHandlersByName.apply(ReflectiveFeign.java:146)
    at feign.ReflectiveFeign.newInstance(ReflectiveFeign.java:53)
    at feign.Feign$Builder.target(Feign.java:198)
    at org.springframework.cloud.netflix.feign.FeignClientFactoryBean$HystrixTargeter.target(FeignClientFactoryBean.java:233)
    at org.springframework.cloud.netflix.feign.FeignClientFactoryBean.loadBalance(FeignClientFactoryBean.java:156)
    at org.springframework.cloud.netflix.feign.FeignClientFactoryBean.getObject(FeignClientFactoryBean.java:177)
    at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.doGetObjectFromFactoryBean(FactoryBeanRegistrySupport.java:168)
    ... 29 more

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.cloud.netflix.feign.FeignClientOverrideDefaultsTests$FooClient': FactoryBean threw exception on object creation; nested exception is java.lang.IllegalStateException: Method get not annotated with HTTP method type (ex. GET, POST)
    at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.doGetObjectFromFactoryBean(FactoryBeanRegistrySupport.java:175)
    at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.getObjectFromFactoryBean(FactoryBeanRegistrySupport.java:103)
    at org.springframework.beans.factory.support.AbstractBeanFactory.getObjectForBeanInstance(AbstractBeanFactory.java:1595)
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:254)
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:197)
    at org.springframework.boot.test.mock.mockito.ResetMocksTestExecutionListener.resetMocks(ResetMocksTestExecutionListener.java:61)
    at org.springframework.boot.test.mock.mockito.ResetMocksTestExecutionListener.resetMocks(ResetMocksTestExecutionListener.java:49)
    at org.springframework.boot.test.mock.mockito.ResetMocksTestExecutionListener.afterTestMethod(ResetMocksTestExecutionListener.java:44)
    at org.springframework.test.context.TestContextManager.afterTestMethod(TestContextManager.java:319)
    at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:94)
    at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:84)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:252)
    at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:94)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
    at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:191)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: java.lang.IllegalStateException: Method get not annotated with HTTP method type (ex. GET, POST)
    at feign.Util.checkState(Util.java:128)
    at feign.Contract$BaseContract.parseAndValidateMetadata(Contract.java:97)
    at feign.Contract$BaseContract.parseAndValidatateMetadata(Contract.java:64)
    at feign.ReflectiveFeign$ParseHandlersByName.apply(ReflectiveFeign.java:146)
    at feign.ReflectiveFeign.newInstance(ReflectiveFeign.java:53)
    at feign.Feign$Builder.target(Feign.java:198)
    at org.springframework.cloud.netflix.feign.FeignClientFactoryBean$HystrixTargeter.target(FeignClientFactoryBean.java:233)
    at org.springframework.cloud.netflix.feign.FeignClientFactoryBean.loadBalance(FeignClientFactoryBean.java:156)
    at org.springframework.cloud.netflix.feign.FeignClientFactoryBean.getObject(FeignClientFactoryBean.java:177)
    at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.doGetObjectFromFactoryBean(FactoryBeanRegistrySupport.java:168)
    ... 28 more

This is happening as a result of beanFactory.getBean(name) which is before any attempt is made to reset a mock. This modified version of ResetMocksTestExecutionListener.resetMocks is enough to trigger the problem:

private void resetMocks(ConfigurableApplicationContext applicationContext,
        MockReset reset) {
    ConfigurableListableBeanFactory beanFactory = applicationContext.getBeanFactory();
    String[] names = beanFactory.getBeanDefinitionNames();
    for (String name : names) {
        BeanDefinition definition = beanFactory.getBeanDefinition(name);
        if (AbstractBeanDefinition.SCOPE_DEFAULT.equals(definition.getScope())) {
            beanFactory.getBean(name);
            // if (reset.equals(MockReset.get(bean))) {
            // Mockito.reset(bean);
            // }
        }
    }
    if (applicationContext.getParent() != null) {
        resetMocks(applicationContext.getParent(), reset);
    }
}

[wilkinsona]

As far as I can tell, the problem is that Feign considers FooClient to be misconfigured. This test fails with Boot 1.3.3.RELEASE:

@Test
public void fooClientBean() {
    this.applicationContext.getBean(
            "org.springframework.cloud.netflix.feign.FeignClientOverrideDefaultsTests$FooClient");
}

[dsyer]

Yes, but it isn't misconfigured in the original test case (it passes with Spring Boot 1.3), it just thinks it is. So I guess the finger is still pointing at the ResetMocksTestExecutionListener.

[wilkinsona]

I disagree. ResetMocksTestExecutionListener is just calling getBean which blows up. It's down to luck that the original test case passes purely because nothing is trying the retrieve the misconfigured bean. I don't think it's reasonable for a test to rely upon that.

[dsyer]

Dave Syer [4:47] 
But you could use a BPP to collect the data
and reset using a public method on that
Then the listener might only need to know about a single bean (the BPP)

Andy Wilkinson [4:48 PM] 
Yeah, that might work