jira.codehaus.org

  • Log In Access more options
    • Online Help
    • Keyboard Shortcuts
    • About JIRA
    • JIRA Credits
    • What?s New
  • Dashboards Access more options (Alt+d)
  • Projects Access more options (Alt+p)
  • Issues Access more options (Alt+i)
  • groovy
  • GROOVY-2503 MOP 2.0 design inflluencing issues
  • GROOVY-2282

ProxyMetaClass doesn't work as expected with StubFor/Mockfor

  • Log In
  • Views
    • XML
    • Word
    • Printable

Details

  • Type: Sub-task Sub-task
  • Status: Open Open
  • Priority: Critical Critical
  • Resolution: Unresolved
  • Affects Version/s: 1.1-rc-2
  • Fix Version/s: 3.0
  • Component/s: mocks and stubs
  • Labels:
    None

Description

Example from the mailing list

import groovy.mock.interceptor.StubFor

class Speaker {
   String name
   def startLecture() { "Starting..." }
}

def speakerStub = new StubFor(Speaker)
speakerStub.demand.startLecture() { "Intercepted!" }
def speaker1 = new Speaker()
speakerStub.use {
   def speaker2 = new Speaker()
   assert speaker1.startLecture() == "Starting..." // I would have expected "Intercepted!" here
   assert speaker2.startLecture() == "Intercepted!"
}

Dierk's answer:
MockFor and StubFor work by replacing the MetaClass with a ProxyMetaClass
and therefore should not be dependend on whether you instantiate the
object before or inside the use closure.
If they do, it's a bug (maybe introduced by the latest changes to
the MOP impl). Please raise an issue.

I'm flagging this as a blocker because it is MetaClass related and we're on crunch mode.

  • Options
    • Sort By Name
    • Sort By Date
    • Ascending
    • Descending
    • Download All

Attachments

  1. File
    StubTest.groovy
    10/Nov/07 10:02 PM
    2 kB
    Andres Almiray

Activity

Ascending order - Click to sort in descending order
  • All
  • Comments
  • Work Log
  • History
  • Activity
Hide
Permalink
Paul King added a comment - 09/Nov/07 9:18 PM

Added confluence code tags

Show
Paul King added a comment - 09/Nov/07 9:18 PM Added confluence code tags
Hide
Permalink
Paul King added a comment - 09/Nov/07 9:46 PM

I am not sure the above is true for Groovy objects which each get their own metaClass. For Java objects which share a metaClass it would be true I think.

Show
Paul King added a comment - 09/Nov/07 9:46 PM I am not sure the above is true for Groovy objects which each get their own metaClass. For Java objects which share a metaClass it would be true I think.
Hide
Permalink
Andres Almiray added a comment - 09/Nov/07 11:21 PM

The above script was executed with groovyConsole. I've attached a simple testcase and ran it againts the current HEAD (9124) and it failed, as you may notice the CUT is a GroovyObject.

Show
Andres Almiray added a comment - 09/Nov/07 11:21 PM The above script was executed with groovyConsole. I've attached a simple testcase and ran it againts the current HEAD (9124) and it failed, as you may notice the CUT is a GroovyObject.
Hide
Permalink
Paul King added a comment - 10/Nov/07 1:33 AM

I can replicate the issue that you have raised but also observe that behaviour in 1.0 and thinking a bit more about it am not sure the current behaviour isn't the correct one. Happy to be proved wrong though - we certainly should be certain of what the correct bahaviour should be.

Show
Paul King added a comment - 10/Nov/07 1:33 AM I can replicate the issue that you have raised but also observe that behaviour in 1.0 and thinking a bit more about it am not sure the current behaviour isn't the correct one. Happy to be proved wrong though - we certainly should be certain of what the correct bahaviour should be.
Hide
Permalink
Andres Almiray added a comment - 10/Nov/07 1:54 AM

Well if any instance of the class being mocked shares the same MetaClass while inside use() then we have a bug (this is just by looking at the base level)
Now at the upper level, just looking at how mocks should behave, does it make sense that only instances of said class should share the same MetaClass if they are created inside use() (the current behavior)?

To give you more insight, I wanted to see if OGB + Mocks are a good idea to create test data from a non-anemic domain model, but because OGB relies on property setters and those methods were not mocked I got into the following scenarios:

a) test model graph is created outside use() so any mocked instance does not have any expectation.
b) test model graph is created inside use(), as setters are not demanded the test will fail while creating the graph.

Show
Andres Almiray added a comment - 10/Nov/07 1:54 AM Well if any instance of the class being mocked shares the same MetaClass while inside use() then we have a bug (this is just by looking at the base level) Now at the upper level, just looking at how mocks should behave, does it make sense that only instances of said class should share the same MetaClass if they are created inside use() (the current behavior)? To give you more insight, I wanted to see if OGB + Mocks are a good idea to create test data from a non-anemic domain model, but because OGB relies on property setters and those methods were not mocked I got into the following scenarios: a) test model graph is created outside use() so any mocked instance does not have any expectation. b) test model graph is created inside use(), as setters are not demanded the test will fail while creating the graph.
Hide
Permalink
Paul King added a comment - 10/Nov/07 3:28 AM

OK, I can see your issue now. It may pay to try to come up with a minimalistic testcase involving OGB. I was hoping to add more test cases myself in this area over the next few days around using mock instances but wasn't planning to involve OGB. Would you need support for property setters/getters even after this fix is in place?

Show
Paul King added a comment - 10/Nov/07 3:28 AM OK, I can see your issue now. It may pay to try to come up with a minimalistic testcase involving OGB. I was hoping to add more test cases myself in this area over the next few days around using mock instances but wasn't planning to involve OGB. Would you need support for property setters/getters even after this fix is in place?
Hide
Permalink
Andres Almiray added a comment - 10/Nov/07 10:02 PM

Attached new version of testcase with two OGB tests.

Show
Andres Almiray added a comment - 10/Nov/07 10:02 PM Attached new version of testcase with two OGB tests.
Hide
Permalink
Alex Tkachman added a comment - 12/Nov/07 11:39 PM

The story is that Groovy objects initialize metaClass field in
constructor. So if you create your Groovy object outside stub closure
it has regular metaclass instead of mocked one.

I really don't know what the right behaviour should be.

In theory, we can change compiler and runtime to do some check in
getMetaClass () that metaclass stored in Groovy object was replaced in
the registry by another one and reassign stored value.

What I really don't like is the design where one thread replace
metaclass to do testing with mock and that effect all threads
executing objects of the class.

Show
Alex Tkachman added a comment - 12/Nov/07 11:39 PM The story is that Groovy objects initialize metaClass field in constructor. So if you create your Groovy object outside stub closure it has regular metaclass instead of mocked one. I really don't know what the right behaviour should be. In theory, we can change compiler and runtime to do some check in getMetaClass () that metaclass stored in Groovy object was replaced in the registry by another one and reassign stored value. What I really don't like is the design where one thread replace metaclass to do testing with mock and that effect all threads executing objects of the class.
Hide
Permalink
Andres Almiray added a comment - 13/Nov/07 1:00 AM

I think that for the case of stub/mocks the proxyMetaClass should be changed for all instances of the mocked class regardless when they were instantiated while they are under the influence of the use closure, in the same way that categories do. This means we forfeit multi thread testing with stubs/mocks and the metaClass will only be available in the current change for the duration of the use closure, once the closure finishes, the previous metaClass is restored in the registry. This will cover the A scenario I outlined.

Now on the B scenario, what's happening is something i did no expect coming from using JMock's stubs, which is that the Company.getEmployees() was not demanded thus throwing an AFE. It seems like StubFor is a weaker version of MockFor (does not care of record sequence) but stronger than JMock's stubs (which will let you call any method without cardinality check). What I would love is have the option for stubs to be behave like partial suppliers of behavior when used with concrete classes and providers of default values when used with interfaces/abstract classes; something different of what StubFor provides now.

Meaning that the following scenarios may be possible:

interface MyInterface {
   boolean doSomething()
   Object doAnotherThing( String param )
}

class CUT {
   MyInterface collaborator
   def doit() { collaborator.doSomething() }
}

def stub = new GroovierStub(MyInterface)
// configure demands if any
stub.use {
   // instantiate the CUT which will use instances of MyInterface
   def cut = new CUT()
   assert !cut.doIt()
   // because the default impl for doSomething() would be to return false
   // as it is the default value for a boolean
}

When stubbing a concrete you may override any method you'd like, those that were not overridden will still have their normal behavior. Stubbing an abstract class follows a mix of concrete/interface where concrete methods may or may not be overridden with demands (as in concrete, retaining their behaviors those who were not overridden) and default return values for those abstract methods that were not demanded.

In short, this is something that StubFor can't provide unless it breaks compatibility, and if something like this proceeds it should be done after 1.1 ships.

</rant>

Show
Andres Almiray added a comment - 13/Nov/07 1:00 AM I think that for the case of stub/mocks the proxyMetaClass should be changed for all instances of the mocked class regardless when they were instantiated while they are under the influence of the use closure, in the same way that categories do. This means we forfeit multi thread testing with stubs/mocks and the metaClass will only be available in the current change for the duration of the use closure, once the closure finishes, the previous metaClass is restored in the registry. This will cover the A scenario I outlined. Now on the B scenario, what's happening is something i did no expect coming from using JMock's stubs, which is that the Company.getEmployees() was not demanded thus throwing an AFE. It seems like StubFor is a weaker version of MockFor (does not care of record sequence) but stronger than JMock's stubs (which will let you call any method without cardinality check). What I would love is have the option for stubs to be behave like partial suppliers of behavior when used with concrete classes and providers of default values when used with interfaces/abstract classes; something different of what StubFor provides now. Meaning that the following scenarios may be possible:
interface MyInterface {
   boolean doSomething()
   Object doAnotherThing( String param )
}

class CUT {
   MyInterface collaborator
   def doit() { collaborator.doSomething() }
}

def stub = new GroovierStub(MyInterface)
// configure demands if any
stub.use {
   // instantiate the CUT which will use instances of MyInterface
   def cut = new CUT()
   assert !cut.doIt()
   // because the default impl for doSomething() would be to return false
   // as it is the default value for a boolean
}
When stubbing a concrete you may override any method you'd like, those that were not overridden will still have their normal behavior. Stubbing an abstract class follows a mix of concrete/interface where concrete methods may or may not be overridden with demands (as in concrete, retaining their behaviors those who were not overridden) and default return values for those abstract methods that were not demanded. In short, this is something that StubFor can't provide unless it breaks compatibility, and if something like this proceeds it should be done after 1.1 ships. </rant>
Hide
Permalink
Alex Tkachman added a comment - 13/Nov/07 3:48 AM

I have submited test, which shows how to achieve behavior you need. The trick is that for groovy objects you have to use stab.use(object)

Show
Alex Tkachman added a comment - 13/Nov/07 3:48 AM I have submited test, which shows how to achieve behavior you need. The trick is that for groovy objects you have to use stab.use(object)
Hide
Permalink
Paul King added a comment - 13/Nov/07 6:00 AM - edited

Can I check everyone's understanding for what the situation is for 1.1?

Using this base class:

class Dice {
    int roll() { 6 }
}

If you instantiate a Groovy object outside the use and don't use the object version of use, the outside instantiations won't be stubbed/mocked, e.g.:

def diceStub = new StubFor(Dice)
private count = 1
diceStub.demand.roll(0..10) { count++ }
def d1 = new Dice()
println d1.roll() // => 6
diceStub.use {
    def d2 = new Dice()
    println d1.roll() // => 6
    println d1.roll() // => 6
    println d2.roll() // => 1
    println d2.roll() // => 2
    println d2.roll() // => 3
}

If you use the object version of use, that object will be stubbed/mocked but others won't, e.g.:

diceStub = new StubFor(Dice)
count = 1
diceStub.demand.roll(0..10) { count++ }
def d3 = new Dice()
println d3.roll() // => 6
diceStub.use(d3) {
    def d4 = new Dice()
    println d3.roll() // => 1
    println d3.roll() // => 2
    println d4.roll() // => 6
    println d4.roll() // => 6
    println d4.roll() // => 6
}

If you want to mock/stub multiple objects of the same type you would need nested use statements with the demands representing the demands across all objects, e.g.:

diceStub = new StubFor(Dice)
count = 1
diceStub.demand.roll(4..4) { count++ }
def d5 = new Dice()
println d5.roll() // => 6
diceStub.use(d5) {
    def d6 = new Dice()
    println d6.roll() // => 6
    diceStub.use(d6) {
        println d5.roll() // => 1
        println d5.roll() // => 2
        println d6.roll() // => 3
        println d6.roll() // => 4
    }
}
Show
Paul King added a comment - 13/Nov/07 6:00 AM - edited Can I check everyone's understanding for what the situation is for 1.1? Using this base class:
class Dice {
    int roll() { 6 }
}
If you instantiate a Groovy object outside the use and don't use the object version of use, the outside instantiations won't be stubbed/mocked, e.g.:
def diceStub = new StubFor(Dice)
private count = 1
diceStub.demand.roll(0..10) { count++ }
def d1 = new Dice()
println d1.roll() // => 6
diceStub.use {
    def d2 = new Dice()
    println d1.roll() // => 6
    println d1.roll() // => 6
    println d2.roll() // => 1
    println d2.roll() // => 2
    println d2.roll() // => 3
}
If you use the object version of use, that object will be stubbed/mocked but others won't, e.g.:
diceStub = new StubFor(Dice)
count = 1
diceStub.demand.roll(0..10) { count++ }
def d3 = new Dice()
println d3.roll() // => 6
diceStub.use(d3) {
    def d4 = new Dice()
    println d3.roll() // => 1
    println d3.roll() // => 2
    println d4.roll() // => 6
    println d4.roll() // => 6
    println d4.roll() // => 6
}
If you want to mock/stub multiple objects of the same type you would need nested use statements with the demands representing the demands across all objects, e.g.:
diceStub = new StubFor(Dice)
count = 1
diceStub.demand.roll(4..4) { count++ }
def d5 = new Dice()
println d5.roll() // => 6
diceStub.use(d5) {
    def d6 = new Dice()
    println d6.roll() // => 6
    diceStub.use(d6) {
        println d5.roll() // => 1
        println d5.roll() // => 2
        println d6.roll() // => 3
        println d6.roll() // => 4
    }
}
Hide
Permalink
Guillaume Laforge added a comment - 13/Nov/07 9:37 AM

Have we got a potential fix for making this work as expected?

Show
Guillaume Laforge added a comment - 13/Nov/07 9:37 AM Have we got a potential fix for making this work as expected?
Hide
Permalink
Guillaume Laforge added a comment - 13/Nov/07 9:38 AM

I'm moving it back to 1.2, not 2.0.
And if there's a potential fix for 1.1, we may potentially put it it back in 1.1.

Show
Guillaume Laforge added a comment - 13/Nov/07 9:38 AM I'm moving it back to 1.2, not 2.0. And if there's a potential fix for 1.1, we may potentially put it it back in 1.1.
Hide
Permalink
Andres Almiray added a comment - 13/Nov/07 12:05 PM

Thanks Alex, but as Paul noted 'stub.use(object)' will only work for object and not other instances of the same class. I still think the category approach (anything that is under the influence of the use closure and matches the stubbed class) is the way to go though in terms of how it should be implemented poses the question on how to handle multi threading.

Because of this I wouldn't want to rush things to get it into 1.1 as now what we have discussed so far makes me think this may not be exactly a bug but a false expectation on how StubFor should work (though if the ProxyMetaClass is added to the registry then any instance regardless of its creation time should be affected by it during the use closure, right? at least that is what I gather).

The nested use(object) work but seems a little un-Groovy by the way.

Show
Andres Almiray added a comment - 13/Nov/07 12:05 PM Thanks Alex, but as Paul noted 'stub.use(object)' will only work for object and not other instances of the same class. I still think the category approach (anything that is under the influence of the use closure and matches the stubbed class) is the way to go though in terms of how it should be implemented poses the question on how to handle multi threading. Because of this I wouldn't want to rush things to get it into 1.1 as now what we have discussed so far makes me think this may not be exactly a bug but a false expectation on how StubFor should work (though if the ProxyMetaClass is added to the registry then any instance regardless of its creation time should be affected by it during the use closure, right? at least that is what I gather). The nested use(object) work but seems a little un-Groovy by the way.
Hide
Permalink
Paul King added a comment - 14/Nov/07 7:06 AM

Not helpful for the final solution of this issue but as a note, you can now use "instance" meta mocks as well as "shared" meta mocks as follows:

diceStub = new StubFor(Dice)
diceStub.demand.roll() { 3 }
diceStub.demand.roll() { 4 }
def d1 = diceStub.proxyInstance()
def d2 = diceStub.proxyInstance()
def d3 = diceStub.proxyInstance()
println d1.roll() // => 3
println d1.roll() // => 4
println d2.roll() // => 3
println d3.roll() // => 3
println d3.roll() // => 4
println d2.roll() // => 4
Show
Paul King added a comment - 14/Nov/07 7:06 AM Not helpful for the final solution of this issue but as a note, you can now use "instance" meta mocks as well as "shared" meta mocks as follows:
diceStub = new StubFor(Dice)
diceStub.demand.roll() { 3 }
diceStub.demand.roll() { 4 }
def d1 = diceStub.proxyInstance()
def d2 = diceStub.proxyInstance()
def d3 = diceStub.proxyInstance()
println d1.roll() // => 3
println d1.roll() // => 4
println d2.roll() // => 3
println d3.roll() // => 3
println d3.roll() // => 4
println d2.roll() // => 4
Hide
Permalink
Paul King added a comment - 07/Feb/10 7:21 AM

Wondering whether this should be closed. Current behavior:

import groovy.mock.interceptor.StubFor

class Speaker {
   String name
   def startLecture() { "Starting..." }
}

def stub1 = new StubFor(Speaker)
stub1.demand.startLecture() { "Intercepted!" }
def stub2 = new StubFor(Speaker)
stub2.demand.startLecture() { "Intercepted!" }
def speaker1 = new Speaker()
stub1.use {
   def speaker2 = new Speaker()
   assert speaker1.startLecture() == "Starting..."
   assert speaker2.startLecture() == "Intercepted!"
}
stub2.use(speaker1) {
   def speaker2 = new Speaker()
   assert speaker1.startLecture() == "Intercepted!"
   assert speaker2.startLecture() == "Starting..."
}

The behavior while not perfect is now reasonably documented and well-known. In some ways it seems a reasonable approach. The metaClass is something which seldom changes hence it might be argued that once an instance (created outside the 'use') has one it should be left untouched. The more frequent use of EMC versus traditional meta-programming makes this less clear though.

Show
Paul King added a comment - 07/Feb/10 7:21 AM Wondering whether this should be closed. Current behavior:
import groovy.mock.interceptor.StubFor

class Speaker {
   String name
   def startLecture() { "Starting..." }
}

def stub1 = new StubFor(Speaker)
stub1.demand.startLecture() { "Intercepted!" }
def stub2 = new StubFor(Speaker)
stub2.demand.startLecture() { "Intercepted!" }
def speaker1 = new Speaker()
stub1.use {
   def speaker2 = new Speaker()
   assert speaker1.startLecture() == "Starting..."
   assert speaker2.startLecture() == "Intercepted!"
}
stub2.use(speaker1) {
   def speaker2 = new Speaker()
   assert speaker1.startLecture() == "Intercepted!"
   assert speaker2.startLecture() == "Starting..."
}
The behavior while not perfect is now reasonably documented and well-known. In some ways it seems a reasonable approach. The metaClass is something which seldom changes hence it might be argued that once an instance (created outside the 'use') has one it should be left untouched. The more frequent use of EMC versus traditional meta-programming makes this less clear though.
Hide
Permalink
Paul King added a comment - 08/Feb/10 5:04 AM

Or if not closed, should we set fix version to 2.0?

Show
Paul King added a comment - 08/Feb/10 5:04 AM Or if not closed, should we set fix version to 2.0?
Hide
Permalink
Guillaume Laforge added a comment - 08/Feb/10 5:12 AM

I would say "close" (current status okay for me), but I'm okay either way.
We could indeed revisit things in 2.0, if a new MOP can help further, so that's why moving to 2.0 is not a bad idea either

Show
Guillaume Laforge added a comment - 08/Feb/10 5:12 AM I would say "close" (current status okay for me), but I'm okay either way. We could indeed revisit things in 2.0, if a new MOP can help further, so that's why moving to 2.0 is not a bad idea either

People

  • Assignee:
    Unassigned
    Reporter:
    Andres Almiray
Vote (0)
Watch (2)

Dates

  • Created:
    07/Nov/07 1:03 PM
    Updated:
    05/Apr/10 12:35 AM
  • Atlassian JIRA (v5.0.4#731-sha1:3aa7374)
  • Report a problem
  • Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.