#14 ✓resolved
James Mead

[#8687] Block's return value is dropped on stubbed yielding methods.

Reported by James Mead | January 1st, 2009 @ 07:11 PM

Date: 2007-02-16 01:29 Opener: Erik Hetzner

Best explained with an example:

test:


foo = mock()
bar = mock().stubs(:get_baz).returns(:baz)
foo.stubs(:with_something).yields(bar)

real code:


baz = foo.with_something do |bar|
  bar.get_baz.to_s
end

baz should now be "baz", but is nil because we have not specified a return value for the stub. Now, we could set add .returns("baz"), but that means that the block isn't really tested.

I have attached a file patches on a yields_and_returns method to Mocha::Expecatation & Mocha::Stub which will return the block's return value.


foo = Mocha::Mock.new
bar = Mocha::Mock.new
bar.stubs(:get_baz).returns(:baz)
foo.stubs(:with_something).yields_and_returns(bar)

baz = foo.with_something do |bar|
  bar.get_baz.to_s
end

Comments and changes to this ticket

  • James Mead

    James Mead January 1st, 2009 @ 07:12 PM

    • no changes were found...
  • James Mead

    James Mead January 1st, 2009 @ 07:12 PM

    Date: 2007-04-11 14:03 Sender: James Mead

    Hi Erik,

    Thanks for the bug report & suggested patch. To be honest I can't see anything wrong with Mocha's behaviour in the example you've given. It's not obvious what you are trying to test. Can you not do something like this...

    
      def test_me
        foo = mock()
        bar = mock()
        bar.stubs(:get_baz).returns(:baz)
        foo.stubs(:with_something).yields(bar)
    
        baz = nil
        foo.with_something do |bar|
          baz = bar.get_baz.to_s
        end
        
        assert_equal 'baz', baz
      end
    

    (see http://pastie.caboo.se/53041 for syntax highlighted version)

    ...so I don't really see the need for the yields_and_returns method you are suggesting. Does that make sense?

    If not, can you try to explain more fully what exactly it is that you are trying to test, ideally with full code examples.

    Thanks, James.

  • James Mead

    James Mead January 1st, 2009 @ 07:13 PM

    Date: 2007-04-11 14:31 Sender: Erik Hetzner

    Hi James,

    Sorry I wasn't more clear. I could certainly change my code, but I have existing code that I want to test, & I don't want to have to change my code so that I can test it. My code uses this with_something idiom & returns the value which the yielded block returns. I can't test this with Mocha's yields builder because it does not return the value with is returned by the yield block.

    The code:

    
    baz = foo.with_something do |bar|
      bar.get_baz.to_s
    end
    

    is real code which I want to test with a mocked foo & bar objects.

  • James Mead

    James Mead January 1st, 2009 @ 07:16 PM

    Date: 2007-07-05 19:09 Sender: Erik Hetzner

    I'd like to register my objection to the closing of this bug. I am reporting code which Mocha is currently unable to test without modifying the code which is to be tested, and I do not think that test code should force the modification of real code unless absolutely necessary. In Ruby blocks return values. It is possible for the return value of a block to be returned by the method itself.

    Imagine we have code which opens a connection to a network service:

    
    baz = Connection.open do |conn|
      conn.get_data.transform_data
    end
    

    Imagine that the real connection is implemented like:

    
    def self.open
      open_connection
      retval = yield
      close_connection
      return retval
    end
    

    It is currently impossible to mock this up in Mocha, avoiding the use of a live connection, because Mocha offers no way to yield and return the value returned by the block yielded to.

    If you feel it is outside the scope of Mocha, I can accept that. I just wanted to make the issue clear.

    Thanks for your time and work.

    I am attaching code which works with the latest Mocha.

  • James Mead

    James Mead January 1st, 2009 @ 07:19 PM

    Date: 2007-07-05 19:19 Sender: James Mead

    I apologise. I obviously didn't read the details carefully enough. I'll try and have another look in the next few days.

  • James Mead

    James Mead January 1st, 2009 @ 07:23 PM

    Date: 2007-09-21 14:59
    Sender: James Mead

    Hi Erik. I'm really sorry I haven't responded to this again sooner
    - work has been really busy and I've also taken some holiday. I've just spent a while thinking about this and I think I now
    understand your point. However, I've been tying my brain in knots
    about whether Mocha should support this or not.

    I had another go at thinking about how I would test your example
    - have a look at http://pastie.org/99409 and tell me what's wrong with it.

    I'm going to post a couple more examples to the mailing list
    (http://rubyforge.org/mailman/listinfo/mocha-developer) and ask for some more opinions.

    Thanks for your patience.

  • James Mead

    James Mead January 1st, 2009 @ 07:23 PM

    Date: 2008-02-01 13:06 Sender: James Mead

    More comments on mailing list http://rubyforge.org/pipermail/m...

  • visnu

    visnu August 8th, 2010 @ 01:56 AM

    • Assigned user set to “James Mead”
    • Milestone order changed from “0” to “0”

    +1

    I just ran into this problem trying to test my code that uses the same idiom for open-uri:

    json = open(url) { |f| JSON.parse(f.read)['root'] }
    

    here, I wanted to mock out the open method with json files from disk:

    @controller.
      expects(:open).
        yields(mock('getPhotos1', :read => File.read(Rails.root + 'test/fixtures/getPhotos1.json'))).
        returns(JSON.parse(File.read(Rails.root + 'test/fixtures/getPhotos1.json'))['root'])
    

    as you can see, I had to duplicate my implementation code in my test also (the ['root'] access call). this isn't ideal, as if I were to change my implementation to use ['trunk'] instead, my test would still pass even though my code is totally broken.

    this is also with a very simple example block passed to open. I have other code that does plenty of work inside of the block passed to open and there is no way I'm willing to duplicate 20+ lines of implementation into my test code.

    it's actually hard to believe no one else has run into this problematic use case before.

  • James Mead

    James Mead November 26th, 2010 @ 11:00 AM

    • State changed from “new” to “open”
  • James Mead

    James Mead November 26th, 2010 @ 11:01 AM

    @visnu: Sorry for the (very slow) response. Nothing here has convinced me that it's sensible to add the Expectation#yields_and_returns method that has been suggested. It feels to me like quite a special case - in your example the outer method (Kernel#open) returns the result of calling the block, but there must be plenty of counter examples where the outer method does not return the result of the block.

    I think your concern about duplication in your test could be dealt with like this :-

    json = File.read(Rails.root + 'test/fixtures/getPhotos1.json')
    io = mock('io', :read => json)
    @controller.expects(:open).yields(io).returns(JSON.parse(json)['root'])
    

    In fact I think it might be better to avoid duplicating the code in order to have a test that has some form of double-entry book-keeping :-

    content = { 'root' => 'value' }
    io = mock('io', :read => content.to_json)
    @controller.expects(:open).yields(io).returns(content)
    

    I'm not convinced it's very useful, but if you want to unit test the block itself, you could do something like this (although I realise there is nothing testing what happens to the return value from the block) :-

    content = mock('content')
    content.expects(:[]).with('root').returns('value')
    JSON.expects(:parse).with(content.to_json).returns(content)
    io = mock('io', :read => content.to_json)
    @controller.expects(:open).yields(io).returns(content)
    

    If you really want to unit test the code in the block more thoroughly, I think it would be better to split it out as an object in its own right. This might be especially relevant if you have 20 lines of code in the block. One way to do it might be like this :-

    class Parser
      def self.parse(io)
        JSON.parse(io.read)['root']
      end
    end
    
    def foo
      url = params[:url]
      open(url) { Parser.parse(io) }
    end
    
    def test_foo
      content = { 'root' => 'value' }
      io = mock('io', :read => content.to_json)
      Parser.expects(:parse).with(io).returns('value')
      @controller.expects(:open).yields(io).returns('value')
      get :foo, :url => "http://example.com/sample.json"
    end
    
    def test_parser
      content = { 'root' => 'value' }
      result = Parser.parse(content.to_json)
      assert_equal 'value', result
    end
    

    I suspect that one of the main reasons you want to use Mocha in this case is to avoid Kernel#open hitting a remote URL. I think in your specific example, I'd be included to write more of an integration test, not use Mocha, and to use a local file path instead of a remote URL :-

    def foo
      url = params[:url]
      value = open(url) { Parser.parse(io) }
      render :text => value
    end
    
    def test_foo
      url = File.expand_path('test/fixtures/getPhotos1.json', Rails.root)
      get :foo, :url => url
      assert_equal 'value', @response.body
    end
    

    Note that I haven't tested any of the above code. I hope it's of some use.

    In the meantime, I'm going to close this ticket.

    Cheers, James.

  • James Mead

    James Mead November 26th, 2010 @ 11:02 AM

    • State changed from “open” to “resolved”

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

A mocking & stubbing library for Ruby.

* <a href="http://github.com/floehopper/mocha">GitHub repository</a>
* <a href="http://mocha.rubyforge.org">Documentation</a>
* <a href="http://groups.google.com/group/mocha-developer">Mailing List</a>

People watching this ticket

Attachments

Tags

Pages