Skip to content

Commit

Permalink
Only restore an instance method on the owning class
Browse files Browse the repository at this point in the history
Save the the original method owner before we back it up so we can
only restore the method for the owning class.

Fixes #1042

```ruby
class Shape
  def area(value)
    value
  end
end

class Circle < Shape
  def area
    super(2)
  end
end

class SmallCircle < Circle
end
```

When using: allow_any_instance_of(SmallCircle).to receive(:area)

Previously, we'd backup the area method from the first superclass
that implements it.  In this case, we'd copy Circle#area into
SmallCircle with a backup name.  Once this method is copied into
SmallCircle, we lose the original ownership of Circle since SmallCircle
now implements the method.

At the end of the example, we remove the stubbing and rename the copied
method back, causing SmallCircle to implement area when it never did
previously.

Any calls to SmallCircle#area would end up calling the contents of
Circle#area, which calls super(2) on the Circle class, not Shape.
Because Circle#area doesn't take any arguments, you get an ArgumentError.

Previously, ruby 2.2 and below allowed this strange behavior to work,
because it skipped calling the same method in super via this commit:
ruby/ruby@c8854d2
  • Loading branch information
jrafanie committed Jan 5, 2016
1 parent 7dc7afc commit b94fe10
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
10 changes: 7 additions & 3 deletions lib/rspec/mocks/any_instance/recorder.rb
Expand Up @@ -18,6 +18,7 @@ def initialize(klass)
@stubs = Hash.new { |hash, key| hash[key] = [] }
@observed_methods = []
@played_methods = {}
@backed_up_method_owner = {}
@klass = klass
@expectation_set = false
end
Expand Down Expand Up @@ -190,9 +191,9 @@ def restore_original_method!(method_name)
return unless @klass.instance_method(method_name).owner == @klass

alias_method_name = build_alias_method_name(method_name)
@klass.class_exec do
@klass.class_exec(@backed_up_method_owner) do |backed_up_method_owner|
remove_method method_name
alias_method method_name, alias_method_name
alias_method method_name, alias_method_name if backed_up_method_owner[method_name.to_sym] == self
remove_method alias_method_name
end
end
Expand All @@ -204,10 +205,13 @@ def remove_dummy_method!(method_name)
end

def backup_method!(method_name)
return unless public_protected_or_private_method_defined?(method_name)

alias_method_name = build_alias_method_name(method_name)
@backed_up_method_owner[method_name.to_sym] ||= @klass.instance_method(method_name).owner
@klass.class_exec do
alias_method alias_method_name, method_name
end if public_protected_or_private_method_defined?(method_name)
end
end

def public_protected_or_private_method_defined?(method_name)
Expand Down
25 changes: 25 additions & 0 deletions spec/rspec/mocks/any_instance_spec.rb
@@ -1,5 +1,22 @@
require 'delegate'

module AnyInstanceSpec
class GrandparentClass
def foo(_a)
'bar'
end
end

class ParentClass < GrandparentClass
def foo
super(:a)
end
end

class ChildClass < ParentClass
end
end

module RSpec
module Mocks
RSpec.describe "#any_instance" do
Expand Down Expand Up @@ -1225,6 +1242,14 @@ def dup(_)
expect(instance.existing_method).to eq :existing_method_return_value
end

it "does not restore a stubbed method not originally implemented in the class" do
allow_any_instance_of(::AnyInstanceSpec::ChildClass).to receive(:foo).and_return :result
expect(::AnyInstanceSpec::ChildClass.new.foo).to eq :result

reset_all
expect(::AnyInstanceSpec::ChildClass.new.foo).to eq 'bar'
end

it "restores the original behaviour, even if the expectation fails" do
expect_any_instance_of(klass).to receive(:existing_method).with(1).and_return(:stubbed_return_value)

Expand Down

0 comments on commit b94fe10

Please sign in to comment.