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

Expectation with method missing `self.class.define_method name && send(name)` fails

Open ojab opened this issue 4 years ago • 2 comments

Subject of the issue

This https://github.com/drapergem/draper/blob/master/lib/draper/helper_proxy.rb

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rspec", github: "rspec/rspec"
  gem "rspec-core", github: "rspec/rspec-core"
  gem "rspec-expectations", github: "rspec/rspec-expectations"
  gem "rspec-mocks", github: "rspec/rspec-mocks"
  gem "rspec-support", github: "rspec/rspec-support"
end

require 'rspec'

class ViewContext
  def xxx
  end
end

class Proxy
  def initialize
    @view_context = ViewContext.new
  end

  def method_missing(name, *args, **kwargs, &block)
    self.class.define_method(name) { @view_context.send(name) }
    send(name)
  end
end

# If this is uncommented — everything works
# Proxy.new.xxx

RSpec.describe 'fail' do
  subject { Proxy.new }

  it do
    expect(subject).to receive(:xxx).and_call_original
    subject.xxx
  end
end

RSpec::Core::Runner.invoke

Your environment

  • Ruby version: 2.6.5
  • rspec-mocks version: git main

Steps to reproduce

Run testcase

Expected behavior

It works!

Actual behavior

Failures:

  1) fail is expected to receive xxx(*(any args)) 1 time
     Failure/Error: send(name)

     SystemStackError:
       stack level too deep
     # /tmp/tst.rb:29:in `method_missing'
     # /tmp/tst.rb:29:in `method_missing'

ojab avatar Oct 22 '20 12:10 ojab

Wow, interesting find. Thanks for reporting.

pirj avatar Oct 22 '20 23:10 pirj

This is because we "stash" the original method at the time of the expectation, to later invoke if needed, in this case it appears we stash the method missing implementation, which I don't think is the intent, so when later defined it doesn't (indeed can't) short circuit.

As an aside I would say this kind of expectation is not something we would recommend as a good idea; you're asserting that a method is called, then calling it, which in itself is pretty pointless. If you tested the side effects of calling this method you wouldn't see this bug.

JonRowe avatar Oct 23 '20 09:10 JonRowe