People like the way how Mockito is able to mock Spring’s auto-wired fields with the @InjectMocks
annotation. When I read this post of Lubos Krnac last week, I thought I should explain why I think the use of InjectMocks is a bad signal and how you should avoid it. Hint: it’s about visibility.
Let’s say we have a PlannerServiceImpl
which delegates to a PlannerClient
. Uses Spring for auto-wiring all together; there’s no constructor, but Spring is able to use field injection.
@Service public class PlannerServiceImpl implements PlannerService { private static final Logger LOG = LoggerFactory.getLogger(PlannerServiceImpl.class); @Autowired private PlannerClient plannerClient; @Override public Long createWeddingPlan() { try { CreateWeddingPlanResponse response = plannerClient.createWeddingPlan(); return convert(response).getId(); } catch (Exception e) { LOG.error("Unable to create wedding plan", e); return null; } }
An associated test could look like:
@RunWith(MockitoJUnitRunner.class) public class PlannerServiceImplTest { @Mock private PlannerClient plannerClient; @InjectMocks private final PlannerServiceImpl plannerService = new PlannerServiceImpl(); @Test public void testCreateWeddingPlanWhenClientReturnsUndefinedResponseThenNullIsReturned() throws Exception { when(plannerClient.createWeddingPlan()).thenReturn(null); final Long actual = plannerService.createWeddingPlan(); assertThat(actual, is(nullValue())); }
The org.mockito.InjectMocks
annotation can be seen as an equivalent of Spring’s own dependency injection. The Javadoc states:
Mockito will try to inject mocks only either by constructor injection, setter injection, or property injection in order and as described below. If any of the following strategy fail, then Mockito won’t report failure; i.e. you will have to provide dependencies yourself.
(Whoever would design this to fail silently at all?)
So what if someone decides to create a new dependency, say an AuditService and upgrades a bunch of services by adding it as an additional property, also marked as @Autowired
?
@Service public class PlannerServiceImpl implements PlannerService { private static final Logger LOG = LoggerFactory.getLogger(PlannerServiceImpl.class); @Autowired private PlannerClient plannerClient; @Autowired private AuditService auditService; @Override public Long createWeddingPlan() { try { CreateWeddingPlanResponse response = plannerClient.createWeddingPlan(); auditService.addEntry("Wedding plan created."); return convert(response).getId(); }
The test will fail, probably on a NullPointerException
on a missing AuditService – and it is not visible why. InjectMocks
will fail silently and there’s no indication the test needs this. Did I already ask whoever would design something like this to fail silently?
If you’re doing TDD or not (and we are able to change the test first) – clients of this code don’t know about an additional dependency, because it’s completely hidden. You shouldn’t use InjectMocks to deal with injecting private fields (err..or at all) , because this kind of Dependency Injection is evil – and signals you should change your design.
There, I said it.
Fix #1: Solve your design and make your dependencies visible.
Create a constructor. Pass along the PlannerClient.
@Service public class PlannerServiceImpl implements PlannerService { private static final Logger LOG = LoggerFactory.getLogger(PlannerServiceImpl.class); private final PlannerClient plannerClient; @Autowired public PlannerServiceImpl(final PlannerClient plannerClient) { this.plannerClient = plannerClient; }
Now, when there are more dependencies needed, they’re clearly in sight because the constructor says so. So don’t go creating a bunch of setters now – they still don’t force you to pass along your required dependencies!
@Service public class PlannerServiceImpl implements PlannerService { private static final Logger LOG = LoggerFactory.getLogger(PlannerServiceImpl.class); private final PlannerClient plannerClient; private final AuditService auditService; @Autowired PlannerServiceImpl(PlannerClient plannerClient, AuditService auditService) { this.plannerClient = plannerClient; this.auditService = auditService; }
The test itself won’t compile any more (luckily, because of the way we’ve been instantiating the field as plannerService = new PlannerServiceImpl()
!) as soon as e.g. the AuditService is added to the constructor. So it’s time to..
Fix #2: Get rid of @InjectMocks
There’s no need to use @InjectMocks
anymore. Instead instantiate the class-under-test properly in a @Before
-annotated method – where it belongs, passing along all needed dependencies.
@RunWith(MockitoJUnitRunner.class) public class PlannerServiceImplTest { @Mock private PlannerClient plannerClient; @Mock private AuditService auditService; private PlannerServiceImpl plannerService; @Before void setUp() { plannerService = new PlannerServiceImpl(plannerClient, auditService); }
Luckily, Lubos – which I mentioned earlier – completely independently came to the same conclusion in the mean time 🙂
Edit July 2020
Not long after this blog post, someone actually created an Mockito GitHub issue based on it, asking to have Mockito no longer silently fail. Apart from the issue itself, I value the response of one of the Mockito contributors explaining some of the history behind it.
“Nowhere in the javadoc, Spring, @Autowired, @Inject, etc is mentioned. And the shown behavior is documented. In that sense this blog post advocates to read the javadoc when one is using some product!
This feature was implemented when annotation based injection wasn’t even there in spring or in JEE.
And anyway the code being tested may even not live inside a Spring container, the code may even be legacy, i.e. without proper separation of concerns, the code may have various valid combination to initialize the bean (setters, fields), the code may have no tests at all. Still when one have to test these kind of code be it the full object or just a portion, it was kinda useful to have a framework that just did the boilerplate code.
Also note that so many Spring beans or maybe custom WhateverCompany beans have optional setters, i.e. they have default component.
So yes it fails silently, because Mockito is not able to confirm an object is correctly initialized or not when this object relies on fields/setters, it’s just impossible. And yes constructor injection is probably the best and the correct approach to dependency injection as the author even suggest (as a reminder @InjectMocks tries first to use constructor injection) ; I totally agree that the bean-y way (ie via fields/setters) to design object initialization is flawed.
The bottom line being @InjectMocks feature in mockito was never designed to be a full featured dependency injection mechanism and will probably never be. At best it’s shorthand way to inject mocks, but that’s all.”
Update: In the mean time I posted a sort of @InjectMocks follow-up.
You have a good point, but I disagree with you for two reasons.
The first is that the intent of TDD is to do white box testing, not just black box testing. So it is permissible, in my view, for a unit test to know more about what is going on than what is visible via the public facade of a class. Note I said “permissible”, not “desirable”. 🙂
The second reason is bigger. While it is a good thing to have the compiler tell you when you’re missing a dependency, you are essentially advocating that people should not use the Spring or JEE dependency injection annotations, which is a far bigger topic than just how do you do unit testing using Mockito. Essentially you are arguing against that paradigm of development in favor of explicit dependency injection via constructors. That may be a superior programming approach (or it may not) but either way it is a big change from common practice. I’d love to hear your thoughts on that.
Cheers,
Darrell
LikeLiked by 2 people
IMHO, setter injection should be banished forever, and should be replaced with constructor injection. I have never had a situation where I couldn’t use constructor injection instead of setter injection. My experience includes 2 large projects using Guice and 1 large project using Spring (which we migrated from setter injection to constructor injection).
Darrell, when Spring started I think setter injection was strongly favoured. I’m not sure why, but anyways, that paradigm became very entrenched. Slowly, constructor injection is becoming more popular.
Check out this blog post:
http://spring.io/blog/2007/07/11/setter-injection-versus-constructor-injection-and-the-use-of-required
where they talk about the history of setter vs constructor injection and give a recommendation (use constructor injection).
LikeLiked by 1 person
Good blog post. Like I said, having the compiler track the dependencies is superior than runtime injection usually. But there are still cases to be made for deferring the binding until the last moment, along with the many cases against it. Food for thought ….
LikeLiked by 1 person
Thanks David, you beat me to it. 🙂
Darrell, I’m not advocating that people should not use the Spring or JEE dependency injection annotations, because I’ve added a constructor, still to be used with Spring’s auto-wiring right? But it’s a constructor (instead of field or setter injection) and that’s the proper way of doing it IMHO.
Consequently, in the test itself no @InjectMocks magic is needed anymore.
Both thanks for your responses!
LikeLiked by 1 person