Closed GoogleCodeExporter closed 9 years ago
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
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
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
Yeah, sure.
Original comment by szcze...@gmail.com
on 1 Feb 2012 at 8:44
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
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
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
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
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
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
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
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
;)
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
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
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
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
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
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
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
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
Original comment by brice.du...@gmail.com
on 5 Mar 2012 at 12:24
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
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
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
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
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
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
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
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
>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
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
Original comment by szcze...@gmail.com
on 3 Jun 2012 at 6:34
Original issue reported on code.google.com by
dmwallace.nz
on 27 Jan 2012 at 6:42