keeganwitt / gmock

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

Support for property mocking #7

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Support for property mocking with the given syntax:

def aMock = mock()
aMock.name.returns("name")
aMock.name.set("another name")
play {
  assertEquals "name", aMock.name
  aMock.name = "another name"
}

Original issue reported on code.google.com by julien.g...@gmail.com on 11 Oct 2008 at 10:07

GoogleCodeExporter commented 9 years ago

Original comment by julien.g...@gmail.com on 11 Oct 2008 at 10:13

GoogleCodeExporter commented 9 years ago

Original comment by JohnnyJianHY on 11 Oct 2008 at 3:20

GoogleCodeExporter commented 9 years ago
Should the setter method be "set" or "sets"? It seems the latter one is more 
consistent with "returns".

Original comment by JohnnyJianHY on 12 Oct 2008 at 12:05

GoogleCodeExporter commented 9 years ago
Yes you are right "sets" is more consistent.

Original comment by julien.g...@gmail.com on 12 Oct 2008 at 12:13

GoogleCodeExporter commented 9 years ago

Original comment by julien.g...@gmail.com on 27 Oct 2008 at 7:34

GoogleCodeExporter commented 9 years ago
I've checkin a working implementation for this issue. I think there is room for 
further refactoring here. 

Original comment by julien.g...@gmail.com on 7 Nov 2008 at 9:58

GoogleCodeExporter commented 9 years ago
The current implementation doesn't support static property mocking. Should we 
create another issue for that 
or include it here?

Original comment by julien.g...@gmail.com on 7 Nov 2008 at 10:02

GoogleCodeExporter commented 9 years ago
Create another issue please, I don't like to make an issue too big.

The test is failed, please check it out.
Failed tests:
  testBasic(org.gmock.FunctionnalPropertyTest)
  testStubSetter(org.gmock.FunctionnalPropertyTest)

And please move all the recorder classes into a new package named "recorder" 
please, 
because they are getting more and more.

Why using methodMissing in PropertyRecorder? Why not define separated methods?

Original comment by JohnnyJianHY on 8 Nov 2008 at 12:18

GoogleCodeExporter commented 9 years ago
And do you think the following usages should be supported?

def mock = mock()
mock.something.sets(1)
play {
  mock.setSomething(1)
}
// and
def mock = mock()
mock.getSomething().returns(1)
play {
  assertEquals 1, mock.something
}

Original comment by JohnnyJianHY on 8 Nov 2008 at 12:21

GoogleCodeExporter commented 9 years ago
When I was working on this it came to me as well that we might what to support 
the
cross usage you describe. This should probably be a separate issue and a later 
release.

I agree about moving recorders object into their own package and using property
missing seems a bit overkill in the PropertyRecoder. I'll look at it or if you 
want
feel free to make those changes.

Regarding the broken tests... hummm... they pass in my ide for strange reason 
but you
are correct about them failing through maven. I'am looking at it right now.

Original comment by julien.g...@gmail.com on 12 Nov 2008 at 7:48

GoogleCodeExporter commented 9 years ago
Done. Issue 24.

Original comment by JohnnyJianHY on 12 Nov 2008 at 8:28

GoogleCodeExporter commented 9 years ago
Those two failing tests are really strange. They seems to be a side affect of 
the way
the closure is evaluated when I affect the value to a property before the end 
of it.

In the "test basic" try:
play {
  assertEquals "a name", mockLoader.name
  assertEquals "a different name", mockLoader.name
  mockLoader.name = "another name"
  println "after name"
}

That should pass. Let me know if you have any thought about it. 

Original comment by julien.g...@gmail.com on 15 Nov 2008 at 11:33

GoogleCodeExporter commented 9 years ago
You are right. Then I suggest to rewrite the test and not to assign at the end 
of a 
closure, or simply add a "return null" to the end of the closure and add some 
comments to clarify the reason.

Original comment by JohnnyJianHY on 16 Nov 2008 at 6:54

GoogleCodeExporter commented 9 years ago
I notice your changes. I was a bit worry about this issue about last property
evaluation. The problem seems unlikely to happen in normal usage. If the code
executed is in a different method then the problem disappear. I've added a
'testLastPropertySetDontGetEvaluated' to prove that.

If we are happy with is issue we should close it (open it again if you think
something is missing).

It's important for me to get 0.3 out a soon as we can so I am moving the "Static
property mocking" to a future release. Functionality plan in 0.4 and 0.5 are 
fare
more important.

Original comment by julien.g...@gmail.com on 16 Nov 2008 at 6:43

GoogleCodeExporter commented 9 years ago
Actually, if you change
    void setName(mockLoader)
into
    def setName(mockLoader)
then the problem appear again.

Original comment by JohnnyJianHY on 17 Nov 2008 at 2:40

GoogleCodeExporter commented 9 years ago
It make sense. At least we understand what the problem is.

Original comment by julien.g...@gmail.com on 18 Nov 2008 at 7:57