Closed GoogleCodeExporter closed 9 years ago
Original comment by JohnnyJianHY
on 3 Feb 2009 at 1:08
There is a small problem with the DSL, we can only record expectations on a
"shadow"
object instead of the original object:
def tagLib = new MyTagLib()
def mockOut = mock()
def mockTagLib = mock(tagLib) // mockTagLib != tagLib
mockTagLib.out.returns(mockOut) // or simply "mock(tagLib).out.returns(mockOut)"
// we cannot call "tagLib.out.returns(mockOut)" to record expectations
play {
tagLib.doSomething()
}
Original comment by JohnnyJianHY
on 3 Feb 2009 at 1:17
Yes you are absolutely correct.
I presume people would probably use the shortcut version:
mock(tagLib).out.returns(mockOut)
Which read pretty well.
Original comment by julien.g...@gmail.com
on 3 Feb 2009 at 6:26
Original comment by julien.g...@gmail.com
on 22 Feb 2009 at 6:31
I had a look at how this could be implemented and it seems to me that the only
option is to use the
def tagLib = new MyTagLib()
mock(tagLib)
This would attach a MockProxyMetaClass to the actual object.
I can't see how the other approach could work. When we say def mockTagLib =
mock(tagLib) we would have
to somehow link the tabLib object to its mock so when tagLib.doSomething is
called we can react.
What's your thought about it?
Original comment by julien.g...@gmail.com
on 26 Feb 2009 at 6:38
Yes, mock(tagLib) will attach a MockProxyMetaClass to tagLib, but only inside
the
play closure.
Actually, 'def mockTagLib = mock(tagLib)' should be equal to 'def mockTagLib =
mock(tagLib.class)', mockTagLib is just a mock of the class of tagLib. When
entering
the play closure, the MockProxyMetaClass of mockTagLib will be attached to
tagLib.
I cannot see another approach, either.
Original comment by JohnnyJianHY
on 27 Feb 2009 at 1:23
Ok it start making sense
Original comment by julien.g...@gmail.com
on 27 Feb 2009 at 7:31
These two tests are failed:
void testRegexMethodName() {
def tagLib = new FakeTagLib()
mock(tagLib)./say.*/().returns('regex')
play {
assertEquals 'regex', tagLib.saySomething()
}
}
void testConcreteObjectShouldNotBeMockedOutsidePlayClosure() {
def tagLib = new FakeTagLib()
mock(tagLib)
tagLib.saySomething()
play {}
}
Original comment by JohnnyJianHY
on 8 Mar 2009 at 1:44
Thanks for reviewing the code. I'll try to make them pass.
One more thing I need to do is to support this style of mocking.
mock(tagLib).goodbye()
mock(tagLib).hello()
And always return the same mock on successive call to mock on the same concrete
object.
Original comment by julien.g...@gmail.com
on 8 Mar 2009 at 1:50
I think we might also want to support:
mock(tagLib, name('tagLib')) {
goodbye()
hello()
}
but disallow:
mock(tagLib, constructor())
mock(tagLib, invokeConstructor())
mock(tagLib).static.init() // static mocking on concrete object doesn't make
any sense
Original comment by JohnnyJianHY
on 8 Mar 2009 at 2:44
I come back to your example call
testConcreteObjectShouldNotBeMockedOutsidePlayClosure. I realise I
actually don't need a shadow object to record expectation. This simplify the
DSL to my original thought.
def tagLib = new FakeTagLib()
mock(tagLib)
tagLib.saySomething()
play {
tagLib.saySomething()
}
Will pass.
Original comment by julien.g...@gmail.com
on 9 Mar 2009 at 7:54
I am afraid that I cannot agree on this. If it acts as you said, then after
"mock(tagLib)", tagLib will be always mocked, and there is no way stop mocking
on it.
What's more, I think users will prefer to use the simpler form
"mock(tagLib).say()"
or "mock(tagLib) {...}", so I think we don't need to allow users to setup
expectations on the original object.
Original comment by JohnnyJianHY
on 9 Mar 2009 at 11:04
I understand your point but this is going to make the code slightly more
complicated.
At the moment the concrete and the mock are sharing the same metaClass. In
order to make it happen I'll have
to swap the metaclass of the concrete one during the play closure only. I'll
have a look at it.
Original comment by julien.g...@gmail.com
on 9 Mar 2009 at 7:31
What's more, consider:
def mockTagLib = mock(tagLib)
mockTagLib.say().stub()
play {
tagLib.say() // mocked
mockTagLib.say() // also mocked
tagLib.hello() // not mocked, call on tagLib
mockTagLib.hello() // call on mockTagLib which may not be correctly constructed, as
the constructor was not called
}
Therefore, I suggest that "mockTagLib.hello()" should delegate to tagLib, so the
behaviors of tagLib and mockTagLib will be consistent inside the play closure.
It
will be less surprising to the users.
It is easy to implement as ConcreteMockProxyMetaClass is a per-instance meta
class,
so just call "return adaptee.invokeMethod(sender, concreteObject, ...)" instead
of
"return adaptee.invokeMethod(sender, receiver, ...)".
Original comment by JohnnyJianHY
on 10 Mar 2009 at 1:47
I notice that you have change the behavior of regex matching to bidirectional.
But
consider the following code:
mockTagLib.say()
play {
tagLib./say.*/() // I don't think it is correct that this method matches the
expectation
}
I suggest to keep it unidirectional, and matching a regex should do like:
mockTagLib./say\.\*/()
play {
tagLib./say.*/()
}
Original comment by JohnnyJianHY
on 15 Mar 2009 at 1:16
I thought about that when writing this biderectional regex and cannot see who
would have regex method in their
code. But maybe we should cope with that.
I am thinking about supporting the name() idea but I don't think we should
support the closure one - it get
really hard to read.
Original comment by julien.g...@gmail.com
on 15 Mar 2009 at 10:58
Why the expectation closure is hard to read?
Original comment by JohnnyJianHY
on 15 Mar 2009 at 11:03
Ok it's not that hard to read but I would prefer to enforce people to use one
of the more explicit form of mocking
out method.
mock(tagLib).foo()
or
using the explicit mock version of it.
I am not too fast about it but as it's not essential for this issue we should
push it to a later release. 0.9? maybe.
Original comment by julien.g...@gmail.com
on 15 Mar 2009 at 11:25
I prefer to use expectation closure very much. You can see that I use
expectation
closures almost everywhere since it was implemented.
Original comment by JohnnyJianHY
on 15 Mar 2009 at 11:56
Ok then.
Original comment by julien.g...@gmail.com
on 15 Mar 2009 at 12:07
It look like not using the bidirectRegexMatch implies quite a bit of changes.
Until we find someone we a
requirement to test "tagLib./say.*/()" then I think we should leave it.
I think this issue is completed now and we should be able to release gmock 0.7
pretty soon now.
Original comment by julien.g...@gmail.com
on 16 Mar 2009 at 7:23
Should we disallow mocking as follow?
def mock = mock()
mock(mock).a() // mocking a mock object doesn't make any sense
Original comment by JohnnyJianHY
on 17 Mar 2009 at 11:04
I agree with you mock(mock) doesn't mean anything but we should assume that
most of our user know what
they are doing ;)
Original comment by julien.g...@gmail.com
on 17 Mar 2009 at 6:17
Original issue reported on code.google.com by
julien.g...@gmail.com
on 3 Feb 2009 at 8:12