keeganwitt / gmock

Automatically exported from code.google.com/p/gmock
6 stars 2 forks source link

Setup expectation in mock closure #35

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
We could support the declaration of expectation in the mock closure for
conciseness. 

def mock = mock(Loader){ 
  load(1).returns("one")
}

Original issue reported on code.google.com by julien.g...@gmail.com on 9 Dec 2008 at 7:53

GoogleCodeExporter commented 9 years ago

Original comment by julien.g...@gmail.com on 9 Dec 2008 at 7:53

GoogleCodeExporter commented 9 years ago

Original comment by JohnnyJianHY on 9 Dec 2008 at 8:47

GoogleCodeExporter commented 9 years ago
What about the static mocking and constructor mocking? Should the following 
features
be supported?

mock(Loader) {
  new Loader(1)
  Loader.staticMethod().returns(1)
}

Original comment by JohnnyJianHY on 21 Jan 2009 at 1:41

GoogleCodeExporter commented 9 years ago
Any thought?

Original comment by JohnnyJianHY on 26 Jan 2009 at 1:41

GoogleCodeExporter commented 9 years ago
Sorry I missed your original comment. 

I think it might be clearer to have a similar syntax to what we have.

For constructor, using the 'new' keyword is a bit misleading as we can't really
having many of them. We can't have
def mock = mock(Loader) {
  new Loader(1)
  new Loader(2)
}
Using the current way of mocking constructor is probably fine. 

For static ideally we should say:
mock(Loader){
  static.init()
}
But we can't as it's reserved. What about:
mock(Loader){
  'static'.init()
}
?

Original comment by julien.g...@gmail.com on 26 Jan 2009 at 6:34

GoogleCodeExporter commented 9 years ago
I agree that it is a bit misleading.

However, after we support mocking constructor like "mock.new(1)", we would do 
the
same in the mock closure:
mock(Loader) {
  'new'(1)
}
I don't quite like the quotes here, as well as the ones of static mocking.

Then we should think out, after we have two ways of mocking constructor('new' 
and
'constructor'), why cannot we have a third one? If we have two or three ways of
mocking constructor, why cannot we have two ways of static mocking?

Although I say so, I don't like the inconsistency here, either. Any suggestions?

Original comment by JohnnyJianHY on 27 Jan 2009 at 3:41

GoogleCodeExporter commented 9 years ago
After using the constructor(...) for a while I find it really intuitive and 
readable.
I doubt we will ever need to use the 'new' (didn't we say it wouldn't work with
raises...). 

I don't really like adding another way of mocking static call. This wouldn't 
help
general code lisibility, people would always wonder if this different way of 
doing is
the same as the other one. And personaly I won't be using it.

We should keep closure mocking simple, we provide it to give people shortcut. So
let's stick with method and property mocking.

Original comment by julien.g...@gmail.com on 27 Jan 2009 at 7:35

GoogleCodeExporter commented 9 years ago
OK then. And I think we can suggest the users to do like this:
mock(Loader) {
  it.static.init()
}
if they don't like the quotes.

And also OK for not introducing the 'new' method.

Original comment by JohnnyJianHY on 27 Jan 2009 at 1:31

GoogleCodeExporter commented 9 years ago
Yes that a really good idea.

Original comment by julien.g...@gmail.com on 27 Jan 2009 at 6:35

GoogleCodeExporter commented 9 years ago
Can I pick this issue then?

Original comment by julien.g...@gmail.com on 27 Jan 2009 at 6:53

GoogleCodeExporter commented 9 years ago
Sure :)

Original comment by JohnnyJianHY on 28 Jan 2009 at 1:08

GoogleCodeExporter commented 9 years ago
Wait,
mock(Loader){
  'static'.init()
}
will not work, it will call init() on the string 'static'.

Original comment by JohnnyJianHY on 29 Jan 2009 at 3:06

GoogleCodeExporter commented 9 years ago
Obviously it will... So we should go with it.static.init()

Original comment by julien.g...@gmail.com on 29 Jan 2009 at 7:12

GoogleCodeExporter commented 9 years ago

Original comment by julien.g...@gmail.com on 1 Feb 2009 at 12:17

GoogleCodeExporter commented 9 years ago

Original comment by julien.g...@gmail.com on 1 Feb 2009 at 7:19

GoogleCodeExporter commented 9 years ago
I think we had better change the resolve strategy of expectation closure to 
delegate
first, as all methods in it are apt to be mocked, expect the match() method (and
strict() method in 0.7). For example:

mock {
  setUp().returns(1)
}

Here setUp() should not be regarded as the one of the TestCase but of the mock 
object.

I have committed the changes of it, please have a look.

Original comment by JohnnyJianHY on 4 Feb 2009 at 7:38

GoogleCodeExporter commented 9 years ago
I was looking at the changes and you are absolutely right. 

Original comment by julien.g...@gmail.com on 4 Feb 2009 at 7:57