allow_any_instance_of(SomeModule) with and_wrap_original causes SystemStackError

[Resonious]

Subject of the issue

When using and_wrap_original on allow_any_instance_of(SomeModule), calling the original method results in a SystemStackError. It appears that the method passed by and_wrap_original is actually the stubbed method, and so it's calling itself.

Your environment

• Ruby version: 2.6.2
• rspec-mocks version: reproduced on 3.9.0.pre 2aab10d and 3.8.0

Steps to reproduce

module MyModule
  def do_work(arg)
    @value += (arg * 2).to_s
  end
end

class MyObject
  include MyModule

  def initialize
    @value = ""
  end
end

RSpec.describe MyObject do
  specify "and_wrap_original works" do
    allow_any_instance_of(MyModule).to receive(:do_work).and_wrap_original { |original, arg|
      original.call(arg)
    }
    MyObject.new.do_work("a")
  end
end

Expected behavior

The stub block calls the stubbed method.

Actual behavior

The stub block calls itself, resulting in SystemStackError.

[pirj]

Would you use allow_any_instance_of(MyClass) instead? Or do you test something different?

[Resonious]

allow_any_instance_of(MyClass) works as expected, but suppose you have multiple classes that include MyModule. You might want to stub a method for all of those classes, right?

This is certainly a bit of a weird case, and I definitely understand if the effort to support it is not worth it, but it's an issue at least worth documenting, I think.

[pirj]

According to the docs, ...any_instance_of is generally not recommended and is considered a code smell. Also, I remember it was mentioned somewhere (code or docs) that it was causing trouble in the past with edge cases like this one.

I'm not sure about the roadmap for RSpec 4, but chances are it may be extracted from rspec-mocks to its own repository that would become an optional dependency.

Can you please provide more context on your spec and the code? Let's try figure out something that would be more idiomatic and would fit your style.

[Resonious]

If the rspec team decides to phase out ...any_instance_of entirely, that's totally fine. They're free to close the issue and not fix the bug.

I appreciate the offer for finding a better solution, but I can't share the code and would rather work out a solution on my own for this one. The issue is here in part so that other people who run into it can know that they're not the only one.

[pirj]

Fair enough 👍

[JonRowe]

:wave: Honestly this isn't an expected usage of ...any_instance_of, as you can't actually have an instance of a module, I'd welcome a PR adding an error / warning / disallowing this particular invocation, or fixing the issue if someone is so inclined and its not too arduous to maintain, but this is low priority for me at the moment :(.

[JonRowe]

Closing as not planned during the monorepo migration.