allow_any_instance_of with block followed by expect_any_instance_of without block leads to doubled unshifting of the original object

[contentfree]

Your environment

• Ruby version: 2.5.x
• rspec-mocks version: 3.10

Steps to reproduce

In a before block I have:

before(:each) do
  allow_any_instance_of(MyClass).to receive(:my_method) { |instance, arg1, arg2| 
    debugger 
    true  # whatever
  } 
end

then in a spec I have:

expect_any_instance_of(MyClass).to receive(:my_method).with(1,2)

Expected behavior

My expectation is that the allowed implementation would be called

Actual behavior

The allowed implementation is called, but it recurses once and ends of with the orig_object being passed to both instance and arg1. This, understandably leads to problems.

Partial Stacktrace

block (3 levels) in <top (required)> [my_class_spec.rb:18]
call [message_expectation.rb:694] (RSpec::Mocks::Implementation)
call [message_expectation.rb:693] (RSpec::Mocks::Implementation)
invoke_incrementing_actual_calls_by [message_expectation.rb:572] (RSpec::Mocks::MessageExpectation::ImplementationDetails)
invoke [message_expectation.rb:427] (RSpec::Mocks::MessageExpectation::ImplementationDetails)
invoke_incrementing_actual_calls_by [message_expectation.rb:574] (RSpec::Mocks::MessageExpectation::ImplementationDetails)
invoke [message_expectation.rb:427] (RSpec::Mocks::MessageExpectation::ImplementationDetails)
message_received [proxy.rb:217] (RSpec::Mocks::Proxy)
message_received [proxy.rb:361] (RSpec::Mocks::PartialDoubleProxy)
proxy_method_invoked [method_double.rb:78] (RSpec::Mocks::MethodDouble)
download [method_double.rb:64] (RSpec::Mocks::MethodDouble)
download [recorder.rb:262] (RSpec::Mocks::AnyInstance::Recorder)
download_results [my_class.rb:40] (MyClass)
... rest of the stack

You can see where it jumps into invoke twice.

[JonRowe]

Are you calling my_method twice? As that would cause that, can you provide a reproducible snippet?

[pirj]

The following spec is all green.
Can you please clarify what the issue is exactly, @contentfree ?

class MyClass
  def my_method(a, b)

  end
end

RSpec.describe MyClass do
  before do
    allow_any_instance_of(MyClass).to receive(:my_method) { |instance, arg1, arg2|-
      true  # whatever
    }-
  end

  it do
    expect_any_instance_of(MyClass).to receive(:my_method).with(1,2)

    MyClass.new.my_method(1, 2)
  end

  it do
    expect(MyClass.new.my_method(nil, nil)).to be(true)
  end
end

[contentfree]

Thanks for looking, @pirj and @JonRowe. Here's a recreation of the failure:

describe "Recreating bug" do
  class MyClass
    def my_method(value); value end
  end

  before(:each) do
    allow_any_instance_of(MyClass).to receive(:my_method) { |instance, value|
      expect(value).to be_a(String)
      true
    }
  end

  subject { MyClass.new.my_method("test") }

  it "works?" do
    expect_any_instance_of(MyClass).to receive(:my_method).with("test")
    subject 
  end
end

This fails with expected #<MyClass:0x00007ff7ef06c7e8> to be a kind of String because invoke is called twice and both times the orig_object is unshifted into the args (I believe)

[contentfree]

Here's the first call to invoke_incrementing_actual_calls_by with the correct args:

Here's the second call to that method with the now-incorrect args:

[pirj]

Interesting. It seems that the instance is passed in twice to receive's block.

[contentfree]

I believe the issue is related to lib/rspec/mocks/message_expectation.rb:556 which is args.unshift(orig_object) if yield_receiver_to_implementation_block?. The flow then goes to line 574 where parent_stub is invoked which eventually hits that unshift again.