easyforgood / mockito

Automatically exported from code.google.com/p/mockito
0 stars 0 forks source link

Javadoc fix / @InjectMocks wires mocks incorrectly when target has multiple fields of a common base interface #368

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The attached zip contains a contrived example to demonstrate the problem. The 
'Bean' class has three java.io.Writer fields, named fileWriter, stringWriter 
and printWriter, and three pairs of corresponding getter / setter methods.

WireOneMockTest is a JUnit 4 test which has a 'fileWriter' field of type 
java.io.FileWriter, annotated with @Mock. It also has a field of type 'Bean' 
annotated with @InjectMocks. The setUp() method is annotated with @Before and 
calls MockitoAnnotations.initMocks(this).

According to the documentation for @InjectMocks, it will try to wire by 
constructor, then property setter, then field name in that order. Since Bean 
only has a zero-arg constructor, I'd therefore expect it to wire by setter. 
However, what I actually see is that the 'fileWriter' mock is actually wired 
into the 'printWriter' field of Bean, with the fileWriter and stringWriter 
fields being null (see attached debugger screenshot - 
WireOneMockTest_debug.png).

The second attached test, WireTwoMocksTest, is similar but tries to wire up 
mocks for both the 'fileWriter' and 'printWriter' properties. What I actually 
see is that the fileWriter property is null, the stringWriter property is set 
to the 'fileWriter' mock, and the printWriter property is set to the 
'printWriter' mock (see WireTwoMocksTest_debug.png).

I get the same results even if I explicitly declare 'name' attributes on @Mock. 
The only way I can get it to work is by declaring @Mock fields for all three 
properties.

I've tried this with Mockito 1.9.0 and 1.9.5-rc1 (using mockito-all JAR), with 
JUnit 4.10 on Sun / Oracle JDK 1.5.0_22 and 1.6.0_26.

Original issue reported on code.google.com by alastair...@gmail.com on 23 Aug 2012 at 1:01

Attachments:

GoogleCodeExporter commented 8 years ago
Hi,

That is expected with the current code, as the javadoc specify : "mocks will 
first be resolved by type, then, if there is several property of the same type, 
by the match of the field name and the mock name"

http://docs.mockito.googlecode.com/hg/latest/org/mockito/InjectMocks.html

We could enhance the autoinjection mechanism to work for these cases as well, 
but it's not our priority.

Cheers,
Brice

Original comment by brice.du...@gmail.com on 23 Aug 2012 at 4:09

GoogleCodeExporter commented 8 years ago

Original comment by brice.du...@gmail.com on 23 Aug 2012 at 4:09

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Hi Brice,

I've read and re-read the documentation, and it states that it will try 
constructor injection, followed by property setter injection, followed by field 
injection. I don't have a matching constructor, so it should drop through to 
property setter injection, since I have matching setters. 

Please note that although it is doing some wiring by setter method, it is 
wiring the mocks using the wrong setters, e.g. in the first screen shot, it has 
wired the mock field called 'fileWriter' of type FileWriter using the 
setPrintWriter(Writer) method rather than using the setFileWriter(Writer) 
method.

This seems to me a clear contradiction of point 2 (Property Setter Injection) 
in the JavaDoc: "mocks will first be resolved by type, then, if there is 
several property of the same type, by the match of the property name and the 
mock name." I get the same behaviour even if I explicitly set the mock name, 
i.e. @Mock(name="fileWriter").

Thanks,
Al.

Original comment by alastair...@gmail.com on 24 Aug 2012 at 9:16

GoogleCodeExporter commented 8 years ago
Hi,

Let's forget about constructor injection, as it is not what is interesting us.

I think the issue is located in the javadoc, as it indicates how the algorithm 
proceeds when injecting through a field or a setter. What this phrase meant to 
say is that, mockito will look by type, if the mock type matches, then it will 
be injected regardless of the name.

Anyway I don't mean that you are wrong, but rather that this is something to 
enhance. But mockito is a mocking framework not a dependency injection one, so 
we are not focusing the effort right now. Though it might be worth to mention 
this current shortcoming !

Original comment by brice.du...@gmail.com on 24 Aug 2012 at 9:35

GoogleCodeExporter commented 8 years ago

Original comment by brice.du...@gmail.com on 3 Sep 2012 at 9:59

GoogleCodeExporter commented 8 years ago

Original comment by brice.du...@gmail.com on 9 Jan 2013 at 2:47

GoogleCodeExporter commented 8 years ago
I have the same issue with field injection and I would prefer if Mockito tries 
to inject by name first.

Original comment by blys...@gmail.com on 29 Jul 2013 at 4:02

GoogleCodeExporter commented 8 years ago
This behavior won't change. Because many people don't name their mock to match 
the exact property. It's safer to look at the type first.

And doing so might break exiting tests. I'm not sure this worth it.

Original comment by brice.du...@gmail.com on 24 Jul 2014 at 7:03

GoogleCodeExporter commented 8 years ago

Original comment by brice.du...@gmail.com on 24 Jul 2014 at 7:03

GoogleCodeExporter commented 8 years ago
Javadoc fixes here : 
https://github.com/mockito/mockito/commit/f4f17245a28a82450725257c6c433dfc721af1
5b

Original comment by brice.du...@gmail.com on 24 Jul 2014 at 7:06

GoogleCodeExporter commented 8 years ago

Original comment by szcze...@gmail.com on 16 Aug 2014 at 2:43

GoogleCodeExporter commented 8 years ago

Original comment by szcze...@gmail.com on 24 Aug 2014 at 3:14

GoogleCodeExporter commented 8 years ago

Original comment by szcze...@gmail.com on 24 Aug 2014 at 3:50