Consider @Primary annotation when using @MockBean

[alexandreBaronZnk]

#10655

Consider Primary annotation when choose bean candidates and add primary flag to mock bean

[pivotal-issuemaster]

@alexandreBaronZnk Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@alexandreBaronZnk Thank you for signing the Contributor License Agreement!

[neiser]

Thanks a lot for the patch @alexandreBaronZnk, I actually wanted to have a look into it right now myself. Let's hope it will be merged soon. Is there any chance that this bugfix (IMHO) is backported into 1.5.x as well?

[philwebb]

@alexandreBaronZnk Thanks for the PR. I was looking to see if we could merge this for M7 but I think there are a few things more that we need to think about.

I'm not sure that we should change the mockDefinition in findCandidateBeans. That would be a somewhat unexpected side effect of calling that method. It's especially problematic given that findCandidateBeans is called from getBeanName which you would definitely not expect to change beanDefinition.

Another item that we need to consider is that @MockBean can be used to replace existing beans or add new beans. In the case of adding a new bean definition I feel like we also need a way to specify @Primary. Perhaps we need something on MockDefinition to indicate if the bean is primary? Possibly coupled with something in MockitoPostProcessor.registerMock that is able to copy the flag if exists on the bean definition that is being removed.

[neiser]

Hi @philwebb I created another pull request #11077 implementing your comment. I don't think that an extension of MockDefinition is needed. See the new PR for detailed description of my changes. I hope it's okay for @alexandreBaronZnk that I took over this work.

[alexandreBaronZnk]

Hi @neiser, thanks you to improve the PR.
I wasn't completely satisfied by my PR and @philwebb have identify what was incorrect,.

But I have a remark : In review the code, I realize that MockitoPostProcessor manages not only mock bean but also spy bean.
I have done a test based on canMockPrimaryBean to test the spy bean annotation

@Test
public void canSpyPrimaryBean() {
	AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
	MockitoPostProcessor.register(context);
	context.register(SpyPrimaryBean.class);
	context.refresh();
	assertThat(Mockito.mockingDetails(
			context.getBean(MockQualifiedBean.class).mock)
			.isSpy()).isTrue();
	assertThat(Mockito.mockingDetails(
			context.getBean(ExampleService.class))
			.isSpy()).isFalse();
	assertThat(Mockito.mockingDetails(
			context.getBean("examplePrimary", ExampleService.class))
			.isSpy()).isFalse();
	assertThat(Mockito.mockingDetails(
			context.getBean("exampleQualified", ExampleService.class))
			.isSpy()).isTrue();
}

@Configuration
static class SpyPrimaryBean {
	@SpyBean(ExampleService.class)
	private ExampleService spy;

	@Bean
	@Qualifier("test")
	public ExampleService exampleQualified() {
		return new RealExampleService("qualified");
	}

	@Bean
	@Primary
	public ExampleService examplePrimary() {
		return new RealExampleService("primary");
	}
}

And I got an exception like those before the PR with mock bean annotation

org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type 'org.springframework.boot.test.mock.mockito.MockitoPostProcessorTests$MockQualifiedBean' available

	at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBean(DefaultListableBeanFactory.java:348)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBean(DefaultListableBeanFactory.java:335)
	at org.springframework.context.support.AbstractApplicationContext.getBean(AbstractApplicationContext.java:1103)
	at org.springframework.boot.test.mock.mockito.MockitoPostProcessorTests.canSpyPrimaryBean(MockitoPostProcessorTests.java:136)

I think the behavior between mock and spy bean must be identical.
@philwebb and @neiser, I would like to have your point of view about this.

[neiser]

Hi @alexandreBaronZnk the code for registerSpy looks really different than registerMock. This is kind of surprising as I suspected they should behave the same as far as bean management is concerned. Let's see if I can get the functionality into the registerSpy part as well, but some refactoring would be still be nice IMHO such that registerSpy and registerMock share the same code paths. It even has some @Primary detection in determinePrimaryCandidate, but your test shows it doesn't quite work.

[neiser]

I added your test and fixed it (you were still using MockQualifiedBean.class, copy&paste error...). Let's continue the discussion in #11077, where you can also see my additional commits investigating the problem.

[neiser]

I think this PR can be closed in favor of #11077