Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.0 final
    • Fix Version/s: None
    • Component/s: Testing
    • Labels:
      None

      Description

      The following test will break:

      @TestFor(MyService)
      class MyServiceTests {
      
          void testSomething() {
              def mockControl = mockFor(Object),
                  myMock = mockControl.createMock()
              assert true
          }
      }
      

      I wanted to mock this: http://lucene.apache.org/solr/api/org/apache/solr/client/solrj/impl/CommonsHttpSolrServer.html

      But it required an argument for the constructor and the Grails mocking framework doesn't support supplying those arguments. So I decided for using a generic Object as I only wanted to test the arguments passed to the request() method.

      Here is some output:

              ...
              at grails.test.GrailsMock$_createMock_closure1.doCall(GrailsMock.groovy:103)
              at grails.test.GrailsMock$_createMock_closure1.doCall(GrailsMock.groovy:103)
      Failure:  null
      |  java.lang.StackOverflowError
              at java.lang.Thread.getId(Thread.java:1623)
              at java.util.concurrent.locks.ReentrantReadWriteLock$Sync.tryReleaseShared(ReentrantReadWriteLock.java:360)
              at java.util.concurrent.locks.AbstractQueuedSynchronizer.releaseShared(AbstractQueuedSynchronizer.java:1317)
              at java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock.unlock(ReentrantReadWriteLock.java:745)
              at grails.test.GrailsMock$_createMock_closure1.doCall(GrailsMock.groovy:103)
              at grails.test.GrailsMock$_createMock_closure1.doCall(GrailsMock.groovy:103)
              at grails.test.GrailsMock$_createMock_closure1.doCall(GrailsMock.groovy:103)
              at grails.test.GrailsMock$_createMock_closure1.doCall(GrailsMock.groovy:103)
              ...
      

        Activity

        Hide
        Rodrigo Rosenfeld Rosas added a comment -

        Sorry about the formatting, couldn't find how to change the description after the bug was created.

        Show
        Rodrigo Rosenfeld Rosas added a comment - Sorry about the formatting, couldn't find how to change the description after the bug was created.
        Hide
        Lari Hotari added a comment -

        Does the same code work in 1.3.7? I don't think you can mock java.lang.Object since GrailsMock modifies the MetaClass of the mocked class.

        Show
        Lari Hotari added a comment - Does the same code work in 1.3.7? I don't think you can mock java.lang.Object since GrailsMock modifies the MetaClass of the mocked class.
        Hide
        Rodrigo Rosenfeld Rosas added a comment -

        Hi Lari, thanks for your comment. Now that I'm understanding better how the mock framework bundled with Grails works (or actually, how it doesn't work), I have to agree with you. I shouldn't be mocking Object nor any other object using this framework, since it is too dangerous.

        In JavaScript or Ruby, we're allowed to add methods to individual instances, but we can't do that in Groovy as far as I've researched. Since this mock framework messes up with the metaClass of the classes being mocked, we shouldn't really use it as it is too dangerous and will led to bugs like this (also filled this afternoon): http://jira.grails.org/browse/GRAILS-8568.

        The least this approach should do is undo all modifications after each test is run, which doesn't seem to happen.

        Also, from a Mock's perspective, in languages like Groovy, we shouldn't be required to specify a class for creating a mock. Rspec-mock won't require this, for an example, nor do Sinon.js. I just used it because as I was reading the documentation on Testing for Grails 2, the mockFor was mentioned for dealing with the case I needed to deal with.

        But after dealing with those problems and thinking a bit more about the problem, it turns out that in lots of situation, a stub would suffice and instead of using mockFor(class, true), a single map with closures as values when a method is needed would be a much simpler way to implement those.

        But if you need to be sure about the call order and frequency, you still need some framework for doing it. I guess some builder would be the right way of implementing such framework without messing out with any class' metaClass. A promising approach is the one used by GSpec:

        http://groovy.codehaus.org/Using+GSpec+with+Groovy
        http://codeforfun.wordpress.com/2007/04/09/gspec-for-java-bdd/

        It would be awesome to have something like this integrated to Grails in some way that we could use the other MixIn's. This should be the standard approach for mock in Grails in my opinion. I'm not talking about the syntax itself. Even preferring the BDD style of writing specs, the builder could be used to write in both asserts and specs style.

        Show
        Rodrigo Rosenfeld Rosas added a comment - Hi Lari, thanks for your comment. Now that I'm understanding better how the mock framework bundled with Grails works (or actually, how it doesn't work), I have to agree with you. I shouldn't be mocking Object nor any other object using this framework, since it is too dangerous. In JavaScript or Ruby, we're allowed to add methods to individual instances, but we can't do that in Groovy as far as I've researched. Since this mock framework messes up with the metaClass of the classes being mocked, we shouldn't really use it as it is too dangerous and will led to bugs like this (also filled this afternoon): http://jira.grails.org/browse/GRAILS-8568 . The least this approach should do is undo all modifications after each test is run, which doesn't seem to happen. Also, from a Mock's perspective, in languages like Groovy, we shouldn't be required to specify a class for creating a mock. Rspec-mock won't require this, for an example, nor do Sinon.js. I just used it because as I was reading the documentation on Testing for Grails 2, the mockFor was mentioned for dealing with the case I needed to deal with. But after dealing with those problems and thinking a bit more about the problem, it turns out that in lots of situation, a stub would suffice and instead of using mockFor(class, true), a single map with closures as values when a method is needed would be a much simpler way to implement those. But if you need to be sure about the call order and frequency, you still need some framework for doing it. I guess some builder would be the right way of implementing such framework without messing out with any class' metaClass. A promising approach is the one used by GSpec: http://groovy.codehaus.org/Using+GSpec+with+Groovy http://codeforfun.wordpress.com/2007/04/09/gspec-for-java-bdd/ It would be awesome to have something like this integrated to Grails in some way that we could use the other MixIn's. This should be the standard approach for mock in Grails in my opinion. I'm not talking about the syntax itself. Even preferring the BDD style of writing specs, the builder could be used to write in both asserts and specs style.
        Hide
        Rodrigo Rosenfeld Rosas added a comment -

        Actually, I've just found that Groovy 1.6 and higher allows us to define methods to a single instance only:

        http://groovy.codehaus.org/Per-Instance+MetaClass

        Show
        Rodrigo Rosenfeld Rosas added a comment - Actually, I've just found that Groovy 1.6 and higher allows us to define methods to a single instance only: http://groovy.codehaus.org/Per-Instance+MetaClass
        Hide
        Lari Hotari added a comment -

        I agree. There should be an option in GrailsMock to use per-instance metaclass. I don't think mixing per-instance metaclasses and class-level metaclasses work and that's why it should be a configurable option. Contributions in pull-request form are welcome!

        Show
        Lari Hotari added a comment - I agree. There should be an option in GrailsMock to use per-instance metaclass. I don't think mixing per-instance metaclasses and class-level metaclasses work and that's why it should be a configurable option. Contributions in pull-request form are welcome!
        Hide
        Lari Hotari added a comment -

        btw. per-instance metaclasses are only supported for classes that implement groovy.lang.GroovyObject . Other classes have to be wrapped with groovy.util.Proxy.
        In your example, groovy.util.Expando is better suited for your requirements than java.lang.Object.

        Show
        Lari Hotari added a comment - btw. per-instance metaclasses are only supported for classes that implement groovy.lang.GroovyObject . Other classes have to be wrapped with groovy.util.Proxy. In your example, groovy.util.Expando is better suited for your requirements than java.lang.Object.
        Hide
        Rodrigo Rosenfeld Rosas added a comment -

        Hey Lari, if you take a look at my last link, you'll see that the GroovyObject requirement is valid only for Groovy prior to 1.6. Higher versions don't have this limitation.

        In my specific case I only need a stub for now, so I'm using:

        def storedArg, myMock = [mockedMethod: {storedArg = it}]
        

        But sometimes I need to mock a static method from a domain class. If I use @Mock or mockDomain, it will add 100ms to my tests, when I want something simpler. Also, sometimes I'll prefer to come with my own implementation of count() and list() for instance. The problem is that differently from Ruby and JavaScript, there is no way to undefine (delete) a method in Groovy yet:

        http://jira.codehaus.org/browse/GROOVY-5225?focusedCommentId=287284#comment-287284

        In Rspec-mock, for example, all changes to any class will be undone after each spec, so the second spec will fail in this example:

        describe 'Some scenario' do
          it 'allows mocking any class and will unwind any changes' do
            MyDomainClass.should_receive(:list).with(no_args).once.and_return([])
            MyDomainClass.list.should == []
          end
        
          it 'fails, since MyDomainClass doesn't have list defined anymore' do
            MyDomainClass.list.should == [] # raises an exception
          end
        end
        
        Show
        Rodrigo Rosenfeld Rosas added a comment - Hey Lari, if you take a look at my last link, you'll see that the GroovyObject requirement is valid only for Groovy prior to 1.6. Higher versions don't have this limitation. In my specific case I only need a stub for now, so I'm using: def storedArg, myMock = [mockedMethod: {storedArg = it}] But sometimes I need to mock a static method from a domain class. If I use @Mock or mockDomain, it will add 100ms to my tests, when I want something simpler. Also, sometimes I'll prefer to come with my own implementation of count() and list() for instance. The problem is that differently from Ruby and JavaScript, there is no way to undefine (delete) a method in Groovy yet: http://jira.codehaus.org/browse/GROOVY-5225?focusedCommentId=287284#comment-287284 In Rspec-mock, for example, all changes to any class will be undone after each spec, so the second spec will fail in this example: describe 'Some scenario' do it 'allows mocking any class and will unwind any changes' do MyDomainClass.should_receive(:list).with(no_args).once.and_return([]) MyDomainClass.list.should == [] end it 'fails, since MyDomainClass doesn't have list defined anymore' do MyDomainClass.list.should == [] # raises an exception end end
        Hide
        Lari Hotari added a comment -

        There is comments about "undoing" metaclass changes in GRAILS-8568 (link for others following these comments).

        Show
        Lari Hotari added a comment - There is comments about "undoing" metaclass changes in GRAILS-8568 (link for others following these comments).

          People

          • Assignee:
            Unassigned
            Reporter:
            Rodrigo Rosenfeld Rosas
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Last Reviewed:

              Development