Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with kwargs in and_invoke #1594

Closed
kinkou opened this issue Sep 30, 2024 · 5 comments · Fixed by #1595
Closed

Issue with kwargs in and_invoke #1594

kinkou opened this issue Sep 30, 2024 · 5 comments · Fixed by #1595

Comments

@kinkou
Copy link

kinkou commented Sep 30, 2024

Subject of the issue

When lambdas passed to and_invoke are called with keyword arguments, rspec-mocks fails with ArgumentError: wrong number of arguments.

Changing RSpec::Mocks::AndInvokeImplementation#call to…

def call(*args, **kwargs, &block)
  proc = if @procs_to_invoke.size > 1
           @procs_to_invoke.shift
         else
           @procs_to_invoke.first
         end

  proc.call(*args, **kwargs, &block)
end

…seems to fix the issue.

Your environment

  • Ruby version: 3.3.4
  • rspec-mocks version: 3.13.1

Steps to reproduce

# ./and_invoke.rb
class AndInvoke
  def call(positional_arg, keyword_arg:)
    puts "Original method called"
  end
end

# ./spec/and_invoke_spec.rb
require_relative 'spec_helper'
require_relative '../and_invoke'

describe AndInvoke do
  let(:instance) { described_class.new }

  before do
    allow(instance).to(
      receive(:call)
        .and_invoke(
          ->(positional_arg, keyword_arg:) { puts "Replacing lambda called" }
        )
    )
  end

  it 'invokes lambda instead of the original method' do
    instance.call('positional_arg', keyword_arg: 'keyword_arg')

    expect(instance).to(
      have_received(:call)
        .with('positional_arg', keyword_arg: 'keyword_arg')
    )
  end
end

Expected behavior

$ bundle exec rspec spec/and_invoke_spec.rb 
Replacing lambda called
.

Finished in 0.00377 seconds (files took 0.04207 seconds to load)
1 example, 0 failures

Actual behavior

$ bundle exec rspec spec/and_invoke_spec.rb 
F

Failures:

  1) AndInvoke invokes lambda instead of the original method
     Failure/Error: ->(positional_arg, keyword_arg:) { puts "Replacing lambda called" }
     
     ArgumentError:
       wrong number of arguments (given 2, expected 1; required keyword: keyword_arg)
     # ./spec/and_invoke_spec.rb:10:in `block (3 levels) in <top (required)>'
     # ./spec/and_invoke_spec.rb:15:in `block (2 levels) in <top (required)>'

Finished in 0.00358 seconds (files took 0.04281 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/and_invoke_spec.rb:14 # AndInvoke invokes lambda instead of the original method
@pirj
Copy link
Member

pirj commented Oct 1, 2024

It is unclear to me art a glance, what is this example covering? Is it making an expectation that a method is called on an instance? Or does it replace the methid’s implementation and checks how its consumer work?

I see that in theory a combination of allow+receive+and_invoke and a subsequent expect+receive+with intuitively should work, but how often is this needed in practice?

@kinkou
Copy link
Author

kinkou commented Oct 1, 2024

Thank you for the prompt reply.

For example, the mocked method might sit somewhere deep inside our code, and our code would depend on what it returns. And in turn, what the mock will return will depend on what we pass to it from our code:

# somewhere deep inside our code
object_with_mocked_method = ThirdPartyLibrary.new

# NB: The return value from the mock will depend on what we pass to it from our code.
mocked_method_return_value = object_with_mocked_method.mocked_method(argument_passed_from_our_code:)

case mocked_method_return_value
when 'this_result' then do_this
when 'that_result' then do_that
else do_something_different
end

UPD: Of course, we can create a series of with mocks instead:

allow(object_with_mocked_method).to receive(:mocked_method).with('this_argument').and_return('this_result')
allow(object_with_mocked_method).to receive(:mocked_method).with('that_argument').and_return('that_result')

…but it would be not as convenient as this:

let(:responses) do
  {
    'this_argument' => 'this_result',
    'that_argument' => 'that_result',
    # etc.
  }
end

before do
  allow(object_with_mocked_method).to receive(:mocked_method).and_invoke(
    ->(argument_passed_from_our_code:) { responses[argument_passed_from_our_code] }
  )
end

@JonRowe
Copy link
Member

JonRowe commented Oct 1, 2024

Can you give #1595 a spin to see if that fixes your issue? I think its just a spot that hasn't been marked as passing through keyword arguments properly, a lot of our code predates the split of keyword arguments out of *args

@kinkou
Copy link
Author

kinkou commented Oct 2, 2024

@JonRowe I updated my Gemfile like the following:

source 'https://rubygems.org'

ruby '3.3.4'

gem 'rspec-core',         github: 'rspec/rspec-core',         branch: 'main'
gem 'rspec-expectations', github: 'rspec/rspec-expectations', branch: 'main'
gem 'rspec-support',      github: 'rspec/rspec-support',      branch: 'main'
gem 'rspec-mocks',        github: 'rspec/rspec-mocks',        branch: 'support-kw-args-with-and-invoke'

…and ran the specs from the example I provided, it passes!

Thank you and the rest of the team for all the hard work!

@JonRowe
Copy link
Member

JonRowe commented Oct 2, 2024

Released in 3.13.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants