freepascal / mockito

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

New annotation: @Spy for creating spy objects. #104

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago

Would be great to have something like this:

public class Test {

  @Spy
  List list = new LinkedList();

  @Test public void test() {
    when(spy.size()).thenReturn(100);
    ....
    verify(list).size();
  }
}

Original issue reported on code.google.com by tom.rathbone@gmail.com on 9 Jul 2009 at 6:11

GoogleCodeExporter commented 9 years ago
Neat idea. At first, we couldn't figure how to do it since spy requires a 
created
instance.

You're very welcome to contribute a patch.

Original comment by szcze...@gmail.com on 9 Jul 2009 at 8:46

GoogleCodeExporter commented 9 years ago
patch to add @Spy annotation.  patched created against trunk r1562.

Original comment by tom.rathbone@gmail.com on 10 Jul 2009 at 8:57

Attachments:

GoogleCodeExporter commented 9 years ago
I reviewed the patch. Here are my comments:

1. It's better if AnnotationEngine does not set anything on the test instance. 
It
should just create an instance and return it. The framework will set this field 
for you.
2. If you fix #1 you won't have to change the signature of createMockFor() 
method.
It's better not to change the signature of it because it is a part of public 
api.
3. @Mock annotation creates mocks with a name - taking field name for it. Spy
annotation should follow this pattern.
4. It would be nice if the framework detected that someone forgot to initialize 
the
field at the declaration point. If the field is null the spy cannot be created.

If you get this stuff done I will merge it to trunk :)

Original comment by szcze...@gmail.com on 20 Jul 2009 at 8:32

GoogleCodeExporter commented 9 years ago
I had some time to spare. I've done my own patch - a bit of an experiment.
o) The public API was not modified. Using ThreadLocal to get the Object under 
test to 
DefaultAnnotationEngine. Didn't made a test but it should work. Argh;)
o) Don't know how set the name
o) Modified behavior for null and set fields. Now @Mock and @Captor will leave 
non-
null fields alone and @Spy will leave null fields alone. This is needed for...

I was wrong to say that there is a problem with field initialization. The @Spy 
JavaDoc is wrong (updated in the patch). With using @Before everything works 
(almost) 
as expected. There is one twist with field initialization. It is possible to 
support 
stuff like this

@Mock View view;
@Spy Presenter presenter = new Presenter(view);

where view in presenter is an actual Mocked object. But the TestBase needs to 
look 
like this

public class TestBase{
 public TestBase() {MockitoAnnotations.initMocks(this);}
 @Before public void setup() {MockitoAnnotations.initMocks(this);}
}

Looks very strange but using initMocks in superclass will set the view field 
before 
Presenter uses it. And @Before initMocks will properly wrap the Presenter in 
the 
spied instance. If you don't use the initMocks in superclass c'tor then the 
view will 
be null. And if not using @Before then new Presenter will not be wraped in 
spied 
object.

Just some ideas.

Original comment by alen_vre...@yahoo.com on 18 Feb 2010 at 2:00

Attachments:

GoogleCodeExporter commented 9 years ago
Hey,

>o) The public API was not modified. Using ThreadLocal to get the Object under 
test to 
DefaultAnnotationEngine. Didn't made a test but it should work. Argh;)

Cool idea but I think it's better to deprecate existing createMockFor() method 
and
something with better name (e.g. processAnnotation()) and with extra argument 
that
takes the context (e.g. usually the test class). Are you willing to update the 
patch
and add sensible javadocs to why the method is deprecated?

>o) Modified behavior for null and set fields. Now @Mock and @Captor will leave 
non-
null fields alone and @Spy will leave null fields alone. This is needed for...

Ok.

>public class TestBase{
 public TestBase() {MockitoAnnotations.initMocks(this);}
 @Before public void setup() {MockitoAnnotations.initMocks(this);}
}

Yeah it's weird. How about we just fix the annotation engine to always process 
@Mock
annotation first? Will it get rid of initMocks() in constructor?

I'm thinking of moving mockito sources to mercurial so that it is way easier to
contribute.

Cheers & thanks a lot for help!
Szczepan

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

GoogleCodeExporter commented 9 years ago
Hi,

thanks for taking the time to look into this.

The modified behavior has a bad effect on not detecting fields with multiple 
mockito
annotations and @Mock foo = new Foo() will not get replaced by a mocked 
instance.

When the test instance is constructed the @Spy Foo foo = new Foo(mock) already 
has
foo=null. I can only think of some ASM magic to make it work without ctor.

I think best way is to revert to old behavior and figure some magic to make 
@Spy work
with @Mock.

Cheers
Alen

Original comment by alen_vre...@yahoo.com on 20 Feb 2010 at 7:28

Attachments:

GoogleCodeExporter commented 9 years ago
Hey,

Check out the trunk - I added some implementation based on few other patches. 
Will
use some tests from your latest patch.

Thanks!
Szczepan

Original comment by szcze...@gmail.com on 20 Feb 2010 at 7:50

GoogleCodeExporter commented 9 years ago
It is implemented

Original comment by szcze...@gmail.com on 25 Feb 2010 at 7:50

GoogleCodeExporter commented 9 years ago

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

GoogleCodeExporter commented 9 years ago

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