Closed GoogleCodeExporter closed 8 years ago
Hi Mathieu, very good test conventions and coding, FEST-Assert, BDD stye,
Guava. You got my +1 ;)
About your issue it's not about guava, but because you just got caught by one
of the limitations of partial mocks (which is one of the reason why we don't
recommend them).
You are writing this stub "willReturn(e2).given(mapper).mapToE2(e1);"
However this method is not called directly, it is called inside the Mapper
class. And Mockito don't intercept call like this.something.
You have to change your design to avoid the partial mock.
Original comment by brice.du...@gmail.com
on 9 Mar 2012 at 11:28
>However this method is not called directly, it is called inside the Mapper
class. And Mockito don't intercept call like this.something.
I might not understand the problem correctly, however mockito does intercept
internal calls like this.something on *spies*. That's the whole point of spying
for many users =) Mockito doesn't intercept final methods nor private methods.
Original comment by szcze...@gmail.com
on 11 Mar 2012 at 7:39
Mmmh yeah I should have written Mapper.this.mapToE2 and explain it in detail.
It was interesting to look further to explains the thing in detail ;P
Mapper is compiled to something like this (avoiding mentions of
MockitoGuavaTest)
class Mapper {
private Mapper() {
E1_TO_E2 = new Mapper$1(this);
}
public E2 mapToE2(final E1 e1) {
return new E2();
}
public List<E2> mapIterableToE2(final Iterable<E1> it_e1) {
return newArrayList(transform(it_e1, E1_TO_E2));
}
private final Function<E1, E2> E1_TO_E2;
}
class Mapper$1 implements Function<E1, E2> {
final Mapper this$0;
public Mapper$1(Mapper mapper) {
this$0 = mapper;
}
public E2 apply(E1 input) {
return this$0.mapToE2(input);
}
}
So what is happening at spy creation time:
------------------------------------------
1. A new real Mapper instance is created using the default constructor
The default constructor will initialize E1_TO_E2 with the anonymous function that the compiler named Mapper$1
2. The constructor of Mapper$1 is generated by the compiler and takes a Mapper
argument representing the instance of the owner class Mapper.
So this$0 holds a reference to the real instance of Mapper
3. When you spy the real mapper instance
3a. You first create a mock of Mapper => something like Mapper$$EnhancedByCGLIB
3b. Then all instance field values are copied to the mock instance
Mapper$$EnhancedByCGLIB including the reference to the final field E1_TO_E2.
What is then happening at the execution time:
---------------------------------------------
1. When using mapperMock.mapIterableToE2(...), guava utility methods are
invoked fine then the anonymous function (aka Mapper$1) is called.
2. This is the instance of the real Mapper class, it means that Mapper$1.this$0
holds the reference to the real Mapper instance, NOT the mock
Mapper$$EnhancedByCGLIB
3. real method of Mapper.mapToE2 is used
I hope I was clear enough.
Original comment by brice.du...@gmail.com
on 11 Mar 2012 at 9:17
Ok, I understand now. Freaky use case. I don't think we can solve it in
reasonable way.
Original comment by szcze...@gmail.com
on 11 Mar 2012 at 9:29
Yeah that was the reason I marked it as WontFix.
Fixing it would probably be feasible, but would mean to explore more deeply the
spied objects.
Original comment by brice.du...@gmail.com
on 11 Mar 2012 at 9:47
Thanks Brice,
I was working with someone that teach to write tests like this.
I was wondering, like @szcze why it wasn't working. Your explanation makes
sense.
I rewrite my test without using willReturn, but by using actual data to test.
Thanks,
Mathieu
Original comment by math.cla...@gmail.com
on 12 Mar 2012 at 9:18
The safest is mocking interfaces. Mocking concrete classes is less safe as you
may hit problems with final methods and private methods. Partial mocking is
least safe because it is mocking concrete classes plus you may hit problems
with state.
Original comment by szcze...@gmail.com
on 12 Mar 2012 at 9:32
You can congrat him then, this is very good in my opinion.
Though I would avoid partial mocks at all costs. You should change the design
of the test or production code.
Now I feel we're a bit like Steve Jobs with his "hold it differently!" during
the Antennagate, anyway there is actually a very good reason for saying that.
Regards,
Brice
Original comment by brice.du...@gmail.com
on 12 Mar 2012 at 9:39
Original issue reported on code.google.com by
math.cla...@gmail.com
on 7 Mar 2012 at 2:52Attachments: