Bug with @InjectMocks in 5.3.0

[robertotru]

Hello,

With 5.3.0, due to https://github.com/mockito/mockito/pull/2942/commits, a regression was introduced.
I investigated the issue with my colleague @sftwr-ngnr (thanks Urs for pairing)

The issue happens when:

• You have a parent class for your test classes.
• You have a class:
  • Initialized using @InjectMocks.
  • Some fields are injected through the constructor and other fields are not.
  • There are multiple candidate mocks for the field not initialized in the constructor.

In this case, since we have a parent test class, you will run the initialization twice. Consider that, everytime that you call MockInjection (one per test class, the actual and the parent), you use the inner class OngoingMockInjection to add two injectionStrategies:

• ConstructorInjection
• PropertyAndSetterInjection

So, for the first invocation, the method processInjection in ConstructorInjection will initialize the class annotated with @InjectMocks inside helper class FieldInitializationReport by checking that Plugins.getMemberAccessor() in FieldInitializer has no value for the fieldInstance (see line 141 and 142 in FieldInitializer).
For the second invocation, since Plugins.getMemberAccessor() in FieldInitializer has already an initialized value for the fieldInstance, then we return false and enable propagating the execution of PropertyAndSetterInjection.

Here it comes the problem. With the changes in https://github.com/mockito/mockito/pull/2942/commits, the TypeBasedCandidateFilter is triggered to find for all the remaining fields not set via constructor any candidate mock to be set. If there is more than one, it throws the exception from method moreThanOneMockCandidate.

How to reproduce it. Create a JUnit5 project with Mockito 5.3.0
Add the following production code in dedicated Java files

public interface Dependency {
}

public interface Dependency1 extends Dependency{
}

public interface Dependency2 extends Dependency{
}

public class Service implements Dependency {

	private Dependency dependency;

	private final Dependency1 dependency1;
	private final Dependency2 dependency2;

	public Service(Dependency1 dependency1, Dependency2 dependency2) {
		this.dependency1 = dependency1;
		this.dependency2 = dependency2;
	}

	void postConstruct() {
		dependency = null; // it could be something else, like a call to a context or to new
	}

}

and then in the test folder add dedicated Java files for the following classes

public abstract class AbstractTest {
}

@ExtendWith(MockitoExtension.class)
class ServiceTest extends AbstractTest {

	@Mock
	Dependency1 dependency1;
	@Mock
	Dependency2 dependency2;
	@InjectMocks
	Service service;

	@Test
	void injectionFails() {
		Assertions.assertNotNull(service);
	}

}

You should get

Mockito couldn't inject mock dependency on field 'private org.example.Dependency org.example.Service.dependency' that is annotated with @InjectMocks in your test, 
because there were multiple matching mocks (i.e. fields annotated with @Mock and having matching type): dependency1, dependency2.
If you have multiple fields of same type in your class under test then consider naming the @Mock fields identically to the respective class under test's fields, so Mockito can match them by name.
org.mockito.exceptions.base.MockitoException: 
Mockito couldn't inject mock dependency on field 'private org.example.Dependency org.example.Service.dependency' that is annotated with @InjectMocks in your test, 
because there were multiple matching mocks (i.e. fields annotated with @Mock and having matching type): dependency1, dependency2.
If you have multiple fields of same type in your class under test then consider naming the @Mock fields identically to the respective class under test's fields, so Mockito can match them by name.
	at app//org.mockito.junit.jupiter.MockitoExtension.beforeEach(MockitoExtension.java:153)

[TimvdLippe]

@jfrantzius we have another one here

[jfrantzius]

@TimvdLippe hm, according to @InjectMocks Javadoc, setter injection shouldn't happen if constructor injection was successful.

So I wonder why setter injection is happening here at all?

@robertotru did you put a breakpoint or log output in the constructor, so we can be sure that it was called with the two expected arguments?

It would be nice if you could put the Java files into a zip that makes this easy to reproduce.

[robertotru]

setter injection shouldn't happen if constructor injection was successful.

This is what we have seen, yes. If the mock is created with the constructor injection, field injection is not attempted. But when the initialization is done for the parent class, the mock is already in the context for the constructor injection, and so the field injection is attempted.

@robertotru did you put a breakpoint or log output in the constructor, so we can be sure that it was called with the two expected arguments?

Yes, it was. First round, on the test class, used constructor injection. Second round on the parent test class, didn't do that because the mock was already created and just tried to inject the mocks in the fields.

@jfrantzius you can clone this repo to reproduce the issue https://github.com/robertotru/MockitoBug2978

[robertotru]

I think that probably an easy way to fix this is to keep a map of the already initialized mocks and avoid to reinitialize them when a super class is found. But I'm pretty sure this will introduce regressions in those who have tests relying on this feature.
So, maybe the decision should be taken by considering what the docs are saying about @Injectmocks

Mockito will try to inject mocks only either by constructor injection, property injection or setter injection in order and as described below.

Hence it is not expected that Mockito applies multiple mock initialization strategy on the same field and therefore my approach for tacking this regression should be fine.

Let me know what you think.

[TimvdLippe]

Based on the additional explanation, I indeed consider this working as intended. Apologies for the breakage.

[robertotru]

@TimvdLippe I disagree. My English is far from being perfect, but only either I think it means only one of the following strategies, while in the example is using more than one.

In addition, the current implementation is not deterministic and can lead to different results based on the test class hierarchy.

E.g. In my demo project https://github.com/robertotru/MockitoBug2978 it is sufficient to remove the parent abstract class (that does nothing, i.e. no mocks defined, no junit extensions, nothing) and then the exception will disappear.
So, in my humble opinion either you always apply all the available strategies or you just apply one in the defined order, but you cannot let this depend on the presence of a parent class.

Hence, please, reconsider the decision of closing the issue.

[jfrantzius]

There is something weird there, but unfortunately I didn't have the time yet to have a closer look.

[robertotru]

@jfrantzius in case we agree on the expected behavior, I could try to work on a fix (inputs are welcome). Up to you.

[JohnMTu]

i just tried to update large project from mockito 5.2.0 to 5.3.1 and have probably related or same issue:

org.mockito.exceptions.base.MockitoException: Mockito couldn't inject mock dependency on field 'private ...ProductOrderUtils ...ProductOrderingMain.productOrderUtils' that is annotated with @InjectMocks in your test, because there were multiple matching mocks (i.e. fields annotated with @Mock and having matching type): variables, productOrderUtils, orderUpdater, resourceInventoryService, serviceInventoryService, productOrderUtils, productOrderingMainConfigValues, orderStore. If you have multiple fields of same type in your class under test then consider naming the @Mock fields identically to the respective class under test's fields, so Mockito can match them by name.

ProductOrderUtils is simple class, no interfaces, no inheritance.
unit itself is having abstract parent and abstract parent-of-parent and on different levels of this hierarchy are also interfaces implemented.

In our case, all should be injected into private fields (which are normally autowired by spring over field injection).

[Sheikah45]

I believe this is caused by the same issue as noted in #2602 but just has been exacerbated now since an exception is thrown when the super class fields cannot be properly injected

[jfrantzius]

Hi @JohnMTu ,
if there is no inheritance involved in your code, then it would be good if you could file a separate issue and attach reproducer code as a ZIP file!

[1-alex98]

Injection shouldn't happen if constructor injection was successful

This is what I tried to fix more than a year ago :D But sadly nobody understood my PR I guess. #2602

[jfrantzius]

@1-alex98 wow that's a long and winding story with a lot of digging in the history of Mockito!

@TimvdLippe I also think that #2602 is the underlying cause

[jfrantzius]

@robertotru do you agree?

[1-alex98]

But I think the fix is super mega easy. See my PR just stop rerunning the DI in this strange loop https://github.com/mockito/mockito/pull/2603/files#diff-98b2e887ea324c1fba28579cc0347aed844875dc56c99886a003551982d00327L56 that does it for the number of super classes the test case has.

[TimvdLippe]

Given the feedback here, I think #2603 is what we want as well. Can anybody affected by this bug check out the code in #2603, build it locally and confirm that it fixes the issue for you?

We do want to fix InjectMocks, but the challenge is finding the correct small fix for it. We don't want to massively expand the implementation for this feature, with the risk of introducing even more subtle behavioural changes.

[robertotru]

@TimvdLippe I'll give it a try tomorrow, thanks.

[robertotru]

@TimvdLippe I can confirm that #2603 fixes the problem on our organization project

[jfrantzius]

Thx a lot @robertotru !

[TimvdLippe]

@1-alex98 the build is failing on your PR. Do you mind fixing the assertions in the test so that we can merge it? Once again apologies for the delay and thanks for picking this up.

[sashanks]

Thak you guys, this was helpful , We are trying to upgrade spring-boot-starter 2.7.0 to 3.1.5 , as spring-boot-starter-test 3.1.x brings mockito core 5.3.1 and mockito-junit-jupiter 5.3.1 , wanted to know if there is a plan of including mockito core 5.6.0 and mockito-junit-jupiter 5.6.0 in next 3.1.x release(assuming mockito core 5.6.0 have the issue fixes) . This will help us decide whether to upgrade to spirng boot starter 3.1.x or 3.0.x.