ssacher-tgm / mockito

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

New annotation: @Captor #166

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Support for creating ArgumentCaptor for the field if annotated with @Captor
e.g.

@Captor
ArgumentCaptor<AsyncCallback<Foo>> callbackCaptor;

need to call MockitoAnnotations.initMocks(testClass); to do the creation.

Hopefully will come up with a patch soon.

Original issue reported on code.google.com by alen_vre...@yahoo.com on 28 Jan 2010 at 7:58

GoogleCodeExporter commented 8 years ago
Excellent! Thank you

Original comment by szcze...@gmail.com on 1 Feb 2010 at 7:59

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Been playing Mass Effect 2 otherwise would have done it sooner;)

Needs a tiny bit of polishing but in attachment is svn diff of what I've done 
so far.

As test goes:
renamed AnnotationsTest to MockAnnotationTest
added CaptorAnnotationTest

Didn't change the signature of AnnotationEngine#createMockFor. Not the best 
name. Anyway:

MockitoAnnotations (now checks if there are multiple annotations i.e. 
createMockFor
returns twice)
DefaultAnnotationEngine now supports @Captor

o) Non-generic Captor is allowed e.g.
@Captor ArgumentCaptor captor is same as ArgumentCaptor.forClass(Object.class).

o) Do the types make any difference e.g. forClass(Object.class) vs 
forClass(List.class)?

I tried using casting to put ArgumentCaptor for List in place of String and 
Mockito
didn't complain. But when I've put Integer captor it did complain the types 
don't match.

o) Error messages should be extracted to maybe Errors class?

As @Spy goes do you think it is a good idea to do it? I am not very convinced

@Spy Foo foo = new Foo(); is better than
Foo foo = spy(new Foo());

but the real caveat I see is that if you put init in superclass it will not see 
the
field instances until the superclass returns. Might confuse a lot of people who 
put
init in superclass constructor.

When the superclass constructor returns it also overrides any non-null fields 
that
might be set by Mockito if init is in super.

Original comment by alen_vre...@yahoo.com on 7 Feb 2010 at 6:46

Attachments:

GoogleCodeExporter commented 8 years ago
>but the real caveat I see is that if you put init in superclass it will not 
see the
field instances until the superclass returns.

Well spotted. In this case I don't we should implement @Spy. Unless someone 
figures a
way to work around the issue you mentioned.

Thanks!
Will try to get captor into the trunk soon.

Original comment by szcze...@gmail.com on 14 Feb 2010 at 12:31

GoogleCodeExporter commented 8 years ago

Original comment by szcze...@gmail.com on 14 Feb 2010 at 12:31

GoogleCodeExporter commented 8 years ago

Original comment by szcze...@gmail.com on 14 Feb 2010 at 10:11

GoogleCodeExporter commented 8 years ago
Can you tell me why do we need RawType processing code (e.g. begining at line 
559 of
the patch file, inside createMockFor() method)? Test method related is:
shouldScreamWhenWildCardGenericsAreUsedWrong()

Original comment by szcze...@gmail.com on 17 Feb 2010 at 7:32

GoogleCodeExporter commented 8 years ago
E.g. ArgumentCaptor<AsyncCallback<Foo>> here the RawType is 
AsyncCallback.class. Then
ArgumentCaptor.forClass(AsyncCallback.class) is used. If no raw type processing 
would
be done we can just default to forClass(Object.class).

Is there any added value for using the specific class instead of Object.class? 
You
know best what to do.

The shouldScreamWhenWildCardGenericsAreUsedWrong checks that exception is 
thrown on
usages as ArgumentCaptor<? extends AsyncCallback<Foo>> this is valid java code 
but it
makes no sense.

Original comment by alen_vre...@yahoo.com on 17 Feb 2010 at 7:47

GoogleCodeExporter commented 8 years ago
This issue was closed by revision r1855.

Original comment by szcze...@gmail.com on 17 Feb 2010 at 8:13

GoogleCodeExporter commented 8 years ago
This issue was closed by revision r1856.

Original comment by szcze...@gmail.com on 17 Feb 2010 at 8:13

GoogleCodeExporter commented 8 years ago
I looked at the r1856 and shouldScreamWhenWildCardGenericsAreUsedWrong is not 
needed.
Very cool you put this in the trunk so soon. Great!

Original comment by alen_vre...@yahoo.com on 17 Feb 2010 at 10:36

GoogleCodeExporter commented 8 years ago
>usages as ArgumentCaptor<? extends AsyncCallback<Foo>> this is valid java code 
but it
makes no sense.

Hahaha :) Usually I am extra fail-fast in cases when user can write senseless 
code
*and* think that the test is still sensible (e.g. verify(mock); ) @Captor using
'?'-generics is harmless so let's forget about it.

Thanks for help!

Original comment by szcze...@gmail.com on 20 Feb 2010 at 1:51

GoogleCodeExporter commented 8 years ago

Original comment by szcze...@gmail.com on 28 Feb 2010 at 9:06