rspec-mocks icon indicating copy to clipboard operation
rspec-mocks copied to clipboard

Issue with respond_to_missing? being called on mock

Open t3hk0d3 opened this issue 8 years ago • 6 comments

class TestMe
  class << self

    def callme
      'failure'
    end

    private

    def method_missing(name, *args, &block)
      raise "Failure!"
    end

    def respond_to_missing?(name, include_private = false)
      callme.respond_to?(name, include_private) || super
    end
  end
end

require 'rspec'

describe TestMe do
  it 'mock static method' do
    allow(TestMe).to receive(:callme).and_return('success') # will trigger method_missing
    expect(TestMe.callme).to eq('success')
  end
end

That will produce following output:

Failures:

  1) TestMe mock static method
     Failure/Error: raise "Failure!"

     RuntimeError:
       Failure!
     # ./stack_level.rb:11:in `method_missing'
     # ./stack_level.rb:15:in `respond_to_missing?'
     # ./stack_level.rb:25:in `block (2 levels) in <top (required)>'

Finished in 0.00523 seconds (files took 0.11831 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./method_missing_spec.rb:24 # TestMe mock static method

FUNFACT: It worked well in RSpec 3.2, and broke only after update to 3.4

t3hk0d3 avatar Mar 23 '16 17:03 t3hk0d3

FUNFACT: It worked well in RSpec 3.2, and broke only after update to 3.4

Thanks for mentioning this; it helped me bisect it back to the culprit -- 8ea8b6016d2b7cd70fef0a0c2687ba70ecf8adb5 (the change from #1047 for 3-4-maintenance). To avoid some Ruby 2.3 warnings we fixed a bug in the InstanceMethodStasher that causes the stubbed method to be removed. It's not obvious to me what, if anything, we can do to fix this -- or even if we should.

In general, respond_to? (and by extension, respond_to_missing?) is part of Ruby's meta-object protocol that rspec-mocks relies upon. To provide the features of rspec-mocks we sometimes have to introduce new calls to it (this is what happened between 3.2 and 3.4). We expect respond_to? to return true or false (or at least a truthy or falsey value). An object that raises an exception from respond_to? is a mis-behaving object that we're not able to support.

Your case is a weird case, though; it looks like it shouldn't generally raise an error from respond_to?. I'm not sure what the solution is yet.

myronmarston avatar Mar 23 '16 18:03 myronmarston

@myronmarston It's not about exception. Its about stubbed method being called from respond_to_missing? being actually routed to method_missing. Exception here is only for indication and backtrace.

t3hk0d3 avatar Mar 23 '16 18:03 t3hk0d3

Can you provide another example showing the problem w/o the exception?

myronmarston avatar Mar 23 '16 18:03 myronmarston

callme is being called here and triggering method_missing, why that's the case isn't clear to me though

JonRowe avatar Mar 24 '16 03:03 JonRowe

@JonRowe exactly! That could be happening because real method is already undefined (renamed) at moment respond_to_missing? is called.

That code is extracted from proxy object, that proxy all calls to encapsulated object. Something like: https://gist.github.com/t3hk0d3/8dc7420c74b1e05e49b2

Problem that calling stubbed object method from method_missing cause infinite recursion (Stack Level Too Deep)

@myronmarston What the point? Exception here only for indication that method_missing being called.

Also i've tested that spec on 3.4.1 and its okay (method_missing not being called), but fail on 3.4.4.

t3hk0d3 avatar Mar 24 '16 12:03 t3hk0d3

Test case, mimicking real code:

class Something
  class << self
    def method_missing(name, *args, &block)
      if in? name
        collection[name]
      else
        super
      end
    end

    def respond_to_missing?(method, include_private = false)
      in?(method) || super
    end

    private

    def in?(name)
      collection.key? name
    end

    def random_private_method
    end

    def collection
      @collection ||= {}
    end
  end
end

RSpec.describe Something do
  before do
    allow(described_class).to receive(:random_private_method).and_return('anything')
    allow(described_class).to receive(:collection).and_return(test: 'wow') # this blows up
  end

  it 'blows up' do
    expect(described_class.test).to eq('wow')
  end
end

https://github.com/rspec/rspec-mocks/blob/master/lib/rspec/mocks/method_reference.rb#L106 - this is the line where it explodes on second visit during single mock. The method gets called twice for every mock performed.

https://github.com/rspec/rspec-mocks/blob/v3.4.1/lib/rspec/mocks/method_reference.rb#L97 - this is the version of that method that worked ok, because vis.nil? evaluates to true after method gets removed.

I'd suggest memoizing visibility state on the first pass, when method gets removed. Checking for it after removal... I can't think of a scenario where such check could return a useful change against a memoized value.

Slotos avatar Sep 22 '16 13:09 Slotos