Provide isMock outside of org.mockito.internal

[GoogleCodeExporter]

A recent poster on StackOverflow 
(http://stackoverflow.com/questions/9035843/how-can-i-tell-if-an-object-is-a-moc
kito-mock) needs to tell whether an object is a mock, possibly via 
MockUtil.isMock() method.  It would be good if this method were available 
outside of the "internal" packages.

If the team agree, I will provide a patch adding isMock() to the main Mockito 
class.

Original issue reported on code.google.com by dmwallace.nz on 27 Jan 2012 at 6:42

[GoogleCodeExporter]

I'm not sure about that.
And I'm against Mockito class. It could however takes it places in some new 
MockInspector, or something like that.
We could stuff in other utility method such as the one proposed for debugging 
(in some older issue).

Original comment by brice.du...@gmail.com on 31 Jan 2012 at 4:55

[GoogleCodeExporter]

Yeah, let's add this functionality. The only question is where? I'm ok with 
adding it to Mockito but isMock() might be not very friendly in case we need to 
grow the functionality. Maybe something like:

MockDetails details = Mockito.getDetails(mock);
details.isMock()
details.isSpy()
details.getMockedTypes()

//or later if need that:
details.getInvocations() //etc.

Not sure... it might be an overkill :)

Original comment by szcze...@gmail.com on 1 Feb 2012 at 4:40

[GoogleCodeExporter]

I like this idea.  May I suggest MockProperties rather than MockDetails as its 
name?

Original comment by dmwallace.nz on 1 Feb 2012 at 8:10

[GoogleCodeExporter]

Yeah, sure.

Original comment by szcze...@gmail.com on 1 Feb 2012 at 8:44

[GoogleCodeExporter]

Good idea Szczepan.

Just for teasing, don't you like the MockInspector name ? ;)

Original comment by brice.du...@gmail.com on 2 Feb 2012 at 8:27

[GoogleCodeExporter]

I'd like the name that is broad enough so that it works if we add more stuff in 
future (e.g. some 'inspecting' stuff, like tell me interactions, give me 
unstubbed interactions, give me stubbed-but-not-used interactions, etc.).

MockInspector is fun, yet I kinda prefer the name to describe the information 
content. Anyways, I'm not so fuzz about that name so I let you guys to decide :)

Come to think about it, MockProperties seems a bit to similar to java's 
Properties so it gives me first impression it's map. But maybe it's just my 
weird brain.

Original comment by szcze...@gmail.com on 2 Feb 2012 at 3:25

[GoogleCodeExporter]

To me, MockInspector sounds like something that doesn't have the mock's 
properties, but can return them from the mock.  So you'd write something like

MockInspector inspector = new MockInspector();
if( inspector.isMock( someObject )){
  // ...
}

and you could use the same MockInspector over and over, for different mocks (or 
not mocks).  But with MockDetails or MockProperties, you'd write something like

MockDetails details = new MockDetails( someObject );
if( details.isMock()){
  // ...
}

And now that Szczepan has mentioned the parallel between MockProperties and 
Properties, I'm leaning away from MockProperties.

Now, my only reservation about MockDetails is whether it's the right name to 
wrap some object that isn't necessarily a mock.

Original comment by family.o...@gmail.com on 3 Feb 2012 at 7:59

[GoogleCodeExporter]

Whoops, that last comment was from me, David.  I wasn't aware that my wife had 
left us logged in to google.  Sorry.

Original comment by dmwallace.nz on 3 Feb 2012 at 8:04

[GoogleCodeExporter]

Good point, David. We can broaden the name into ObjectDetails, Details or 
mockingDetails. We can keep mockdetails and make the getter fail if not a mock. 
We can add extra method isMock.

Other ideas?

Original comment by szcze...@gmail.com on 3 Feb 2012 at 11:08

[GoogleCodeExporter]

Yes! I pick MockingDetails! (although I'm starting to feel a bit like Tigger 
looking for breakfast).  I'll start work on this in the next few days.

Original comment by dmwallace.nz on 3 Feb 2012 at 11:30

[GoogleCodeExporter]

The more I'm thinking about this, I'm against adding such call to the Mockito 
class. we were talking about not cluttering the API for answers, aliases for 
times(1), etc. 'getDetails' falls in that category.

Instead I would probably make another class with a factory method. Something 
like :

MockingDetails.of(mock);

David If you want lunch you can still code the debug utility method in issue 
178 ;)

Original comment by brice.du...@gmail.com on 3 Feb 2012 at 7:35

[GoogleCodeExporter]

Why wouldn't I just write 
new MockingDetails( someObject );
as per my comment #7?  This won't be going near the Mockito class.

Very kind of you to offer to buy me lunch, Brice.

Original comment by dmwallace.nz on 3 Feb 2012 at 8:03

[GoogleCodeExporter]

;)

You had your Tiger appetite ;P
Sometime I feel the same.

Also why I don't propose the syntax with "new": I think that a factory method 
is better, as it does hide instantiation details. I think this is safer 
implementation wise. Look for example guava's ImmutableMap or JDK's 
ServiceLoader to name a few, they use a factory method so they can hide 
instantiation details or the real returned type.

  public static <K, V> ImmutableMap<K, V> of(K k1, V v1) {
    return new SingletonImmutableMap<K, V>(checkNotNull(k1), checkNotNull(v1));
  }

  public static <K, V> ImmutableMap<K, V> of(K k1, V v1, K k2, V v2) {
    return new RegularImmutableMap<K, V>(entryOf(k1, v1), entryOf(k2, v2));
  }

    public static <S> ServiceLoader<S> load(Class<S> service, ClassLoader loader) {
        return new ServiceLoader<S>(service, loader);
    }

Besides I find it better to read "MockingDetails.of(mock)" than "new 
MockingDetails(mock)" and in general more in line with the current Mockito API.

Original comment by brice.du...@gmail.com on 3 Feb 2012 at 9:12

[GoogleCodeExporter]

OK, fair enough.  I'll do it as a factory method.  

BTW, the "Tigger" reference wasn't about me being hungry, but me being 
indecisive.  In a popular English children's story, Tigger wants honey for 
breakfast, then discovers he doesn't like honey.  Then he wants acorns for 
breakfast, then discovers he doesn't like acorns.  Then he wants thistles for 
breakfast, then discovers he doesn't like thistles.  

I was trying to say that the way I kept changing my mind about how this feature 
should work was like Tigger changing his mind about breakfast.  In future, I'll 
try to keep culture-specific references out of multi-cultural mailing groups :-)

Original comment by dmwallace.nz on 3 Feb 2012 at 9:48

[GoogleCodeExporter]

Ooooh ok, that was a reference to "Winnie the Pooh", in France we had the same 
"Winnie l'Ourson" and "Tigger" is called "Tigrou". However I didn't make the 
connection as I didn't watch it much as a kid.

>  In future, I'll try to keep culture-specific references out of 
multi-cultural mailing groups :-)
Don't. I actually find it fun to have some culture-specific references. As long 
as there is some explanation if required ;)
Isn't it great to talk Winnie the Pooh on a Mockito issue ;P

Original comment by brice.du...@gmail.com on 5 Feb 2012 at 2:49

[GoogleCodeExporter]

Didn't "watch" it much?  What do you mean "watch"?  

I must be quite a bit older than you.  I never "watched" Winnie the Pooh as a 
kid, but it was one of my favourite BOOKS.  How times have changed!

Original comment by dmwallace.nz on 5 Feb 2012 at 8:39

[GoogleCodeExporter]

Yeah I believe there was an animated show at some point, though I'm not sure if 
this show existed in my childhood, it could have been movies ...not sure. I 
also remember some illustrated books too, but I wasn't a big time reader at 
that time, I preferred french comics then.

Actually some memory comes back on some story of the birthday and the tail of 
"Eeyore".

Agreed times have changed ;)

Original comment by brice.du...@gmail.com on 5 Feb 2012 at 8:59

[GoogleCodeExporter]

This is probably the first time in the history of the Internet, when Winnie the 
Pooh hijacked a thread! ;)

Original comment by kaczanow...@gmail.com on 7 Feb 2012 at 9:30

[GoogleCodeExporter]

Yeah I wouldn't have bet on this before it actually happened there ;)

Original comment by brice.du...@gmail.com on 7 Feb 2012 at 9:45

[GoogleCodeExporter]

Tomek - I doubt that very much.  This bear is very popular with native English 
speakers of a particular age.

Original comment by dmwallace.nz on 7 Feb 2012 at 10:28

[GoogleCodeExporter]

Original comment by brice.du...@gmail.com on 5 Mar 2012 at 12:24

• Added labels: Priority-High
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Szczepan - you suggested
details.isMock()
details.isSpy()
details.getMockedTypes()
details.getInvocations()

of which the first two are trivially easy.  I think getMockedTypes() is 
actually impossible; this isn't stored anywhere.  

I'm in the middle of doing getInvocations(), but I'm a little unsure about 
whether this would actually be a good feature to have.  The Invocation class is 
in the "internal" package; so if I include this feature, we don't have the 
separation between the "internal" and "public" parts of the API any more.  This 
new class is intended to increase the separation, not decrease it.

So I'm really tempted just to stop with isMock() and isSpy().  Anyone have any 
other thoughts? 

Original comment by dmwallace.nz on 24 Mar 2012 at 9:37

[GoogleCodeExporter]

OK, I've pushed this change.  Only has isMock() and isSpy() at this point - 
getInvocations() may or may not come later.  Hopefully, I've done this in time 
to get it into 1.9.1.

Original comment by dmwallace.nz on 25 Mar 2012 at 6:09

[GoogleCodeExporter]

In my opinion, this issue is not a priority.  Having the first two operation 
for 1.9.1 is good enough in my opinion.

Original comment by brice.du...@gmail.com on 25 Mar 2012 at 8:18

[GoogleCodeExporter]

Yeah - what I suggested is only in case someone asks for it. Certainly not for 
now. So, to solve the initial use case (to find out if the given object is a 
mockito mock) one has to call:

details.isMock() || details.isSpy()

I.e. the isMock() and isSpy() are mutually exclusive?

Original comment by szcze...@gmail.com on 25 Mar 2012 at 5:19

[GoogleCodeExporter]

Nope the real call is delegated to MockUtil.isMock which returns true for a spy 
as well.

Original comment by brice.du...@gmail.com on 25 Mar 2012 at 5:26

[GoogleCodeExporter]

Which I guess makes sense. Worth mentioning in the docs, though.

I'm thinking if MockingDetails shouldn't be an interface...

Original comment by szcze...@gmail.com on 25 Mar 2012 at 5:38

[GoogleCodeExporter]

Responding to comment 26:

Gosh, I didn't even think to test what MockUtil.isMock returns for a spy.  I 
find it confusing that it returns true; I think we probably want the 
implementation of isMock in MockingDetails to return false for a spy.

How do others feel about this?  Obviously, I can make it work either way.  I'll 
also amend the documentation accordingly.

Responding to comment 27:

Well, of course it COULD be an interface.  What advantage would this give 
anyone though?  Surely, we don't expect people to make their own 
implementations of MockingDetails!  In fact, this comment has made me wonder 
whether MockingDetails should be final.

Regards,
David.

Original comment by dmwallace.nz on 26 Mar 2012 at 7:17

[GoogleCodeExporter]

Hi, as said in a previous comment I was more thinking to something a la guava 
like the ImmuableList with factory methods returning concrete implementations.

http://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/comm
on/collect/ImmutableList.java

For now I agree with David, I don't see an interface really useful here, do you 
have something in mind ?

Original comment by brice.du...@gmail.com on 26 Mar 2012 at 8:31

[GoogleCodeExporter]

>Surely, we don't expect people to make their own implementations of 
MockingDetails!  In fact, this comment has made me wonder whether 
MockingDetails should be final.

It's always safer for us to hide the implementation. As the domain model grows 
this type might participate in the API in other places, too. Final class == no 
way! :)

Interface might not bring great benefits immediately but at least it does not 
bring any extra baggage :) I know you can find some classes in the API that 
should be interfaces (like Invocation) but still... I'm going to be pushing for 
hiding the implementation details of the MockingDetails.

Some options to solve the isMock/isSpy:
1. hide the isSpy() for now as nobody asked about it yet.
2. leave it as is. I think it is consistent albeit it might be surprising for 
users.
3. change the isSpy() to canSpy(), this means the object has been created with 
spiedInstance and has default answer of 'callsRealMethods()'

There are other options, I'm sure :)

Original comment by szcze...@gmail.com on 26 Mar 2012 at 7:30

[GoogleCodeExporter]

OK, I've made it an abstract class, instead of an interface, as discussed in 
mockito-dev; and pushed it to the repository.  I don't quite see how that's 
better, but there it is.  Oh, and I've also mentioned in the javadoc that 
isMock() returns true for a spy.

Original comment by dmwallace.nz on 2 Apr 2012 at 2:31

[GoogleCodeExporter]

Original comment by szcze...@gmail.com on 3 Jun 2012 at 6:34

• Changed state: Fixed
• Added labels: Milestone-Release1.9.5-rc1