rspec-expectations: Error message when passing block to `and_return` could be improved?

Error message when passing block to `and_return` could be improved?

This is not a bug report, but a suggestion for improvement.

Given the following code and spec

module M                                                                                                                                                                     
  def self.f(x)                                                                 
    x                                                                           
  end                                                                           
end                                                                             
                                                                                
RSpec.describe 'M.f' do                                                              
  it 'returns 0 when stubbed' do                                              
    expect(M).to receive(:f).and_return do                                  
      0                                                                       
    end                                                                       
    expect(M.f(5)).to eq 0                                                  
  end                                                                         
end

we expected the stubbed :f method to return 0. Instead we got the error message

ArgumentError:
       wrong number of arguments (given 0, expected 1+)
# ./reproduction_spec.rb:9:in `block (2 levels) in <top (required)>'

which is correct. However, we spent some time figuring out that it was the and_return itself that was failing, while setting up the spec, instead of something in our code while the spec was being executed (of course both the spec and code involved were much more complex; if it would have been as simple as this example, it would have been obvious). After we figured that out it was of course clear that we should pass the block directly to receive and dispense with the and_return entirely.

Since we spent a relatively large amount of time tracking this down, I figured it could be worthwhile to amend and_return to give a helpful message when a block is passed to it, to save others that time.

Would it be feasible and acceptable to proactively tell the user what they are doing wrong when they call and_return without a parameter and with a block? I’m willing to submit a PR if it is.

Your environment

Ruby version: 2.5.3
rspec-expectations version: 3.8.4

Steps to reproduce

Save the code given above to a file and execute it using rspec.

Suggested behavior

Actual behavior is correct. Suggested behavior is to

About this issue

Original URL
State: closed
Created 5 years ago
Comments: 19 (19 by maintainers)

Most upvoted comments

The current error message is for and_return() the block is not part of it, and its a standard ruby error message, we have generally held to the principle of letting Ruby error when its a such a standard situation, we could work around it with some defensive programming but thats not a policy we’ve believed in either.

JonRowe on Nov 2, 2019

Using and_return with a block is not actually using and_return but instead is using block responses, see https://relishapp.com/rspec/rspec-mocks/v/3-8/docs/configuring-responses/block-implementation.

This is an issue with Ruby semantics, a block with no arguments raises when it is called with arguments.

I’d happily accept a PR that caught this error when invoking the implementation and elaborated, but the solution is a little tricky as I don’t really want to proactively check arguments (and slow down the result) but it must also not hide false positives… Something along the lines of (psuedo coding here)

begin
  implementation.call(*args)
rescue ArgumentError => error
  raise error unless given_argument_arity != expected_argument_arity
  raise MoreHelpfulError
end

JonRowe on Sep 1, 2019