Skip to content

Commit

Permalink
Restore 'owned' stubbed methods only in ruby 2.3+.
Browse files Browse the repository at this point in the history
Fixes #1057.

In b94fe10, we changed behavior to
to restore 'owned' stubbed methods for all rubies.

This doesn't work for all rubies because Method#owner for aliased methods
is inconsistent [1].

Since, ruby 2.3+ broke our prior backup/restore mechanism, it's safer to change
the original behavior for ruby 2.3+ only.

A @klass can have methods implemented (see Method#owner) in @klass
or inherited from a superclass. In ruby 2.2 and earlier, we can copy
a method regardless of the 'owner' and restore it to @klass after
because a call to 'super' from @klass's copied method would end up
calling the original class's superclass's method.

With the commit below [2], available starting in 2.3.0, ruby changed
this behavior and a call to 'super' from the method copied to @klass
will call @klass's superclass method, which is the original
implementer of this method!  This leads to very strange errors
if @klass's copied method calls 'super', since it would end up
calling itself, the original method implemented in @klass's
superclass.

For ruby 2.3 and above, we need to only restore methods that
@klass originally owned.

[1] https://gist.github.com/jrafanie/0e5b8a9f4e8b4f84913f
[2] ruby/ruby@c8854d2
  • Loading branch information
jrafanie committed Feb 4, 2016
1 parent 2fcdbae commit f5f95f5
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
23 changes: 22 additions & 1 deletion lib/rspec/mocks/any_instance/recorder.rb
Expand Up @@ -193,7 +193,28 @@ def restore_original_method!(method_name)
alias_method_name = build_alias_method_name(method_name)
@klass.class_exec(@backed_up_method_owner) do |backed_up_method_owner|
remove_method method_name
alias_method method_name, alias_method_name if backed_up_method_owner[method_name.to_sym] == self

# A @klass can have methods implemented (see Method#owner) in @klass
# or inherited from a superclass. In ruby 2.2 and earlier, we can copy
# a method regardless of the 'owner' and restore it to @klass after
# because a call to 'super' from @klass's copied method would end up
# calling the original class's superclass's method.
#
# With the commit below, available starting in 2.3.0, ruby changed
# this behavior and a call to 'super' from the method copied to @klass
# will call @klass's superclass method, which is the original
# implementer of this method! This leads to very strange errors
# if @klass's copied method calls 'super', since it would end up
# calling itself, the original method implemented in @klass's
# superclass.
#
# For ruby 2.3 and above, we need to only restore methods that
# @klass originally owned.
#
# https://github.com/ruby/ruby/commit/c8854d2ca4be9ee6946e6d17b0e17d9ef130ee81
if RUBY_VERSION < "2.3" || backed_up_method_owner[method_name.to_sym] == self
alias_method method_name, alias_method_name
end
remove_method alias_method_name
end
end
Expand Down
21 changes: 21 additions & 0 deletions spec/rspec/mocks/any_instance_spec.rb
Expand Up @@ -5,12 +5,22 @@ class GrandparentClass
def foo(_a)
'bar'
end

def grandparent_method
1
end
end

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

def caller_of_parent_aliased_method
parent_aliased_method
end

alias parent_aliased_method grandparent_method
end

class ChildClass < ParentClass
Expand Down Expand Up @@ -174,6 +184,17 @@ def private_method; :private_method_return_value; end
end
end

context 'aliased methods' do
it 'tracks aliased method calls' do
instance = AnyInstanceSpec::ParentClass.new
expect_any_instance_of(AnyInstanceSpec::ParentClass).to receive(:parent_aliased_method).with(no_args).and_return(2)
expect(instance.caller_of_parent_aliased_method).to eq 2

reset_all
expect(instance.caller_of_parent_aliased_method).to eq 1
end
end

context "with argument matching" do
before do
allow_any_instance_of(klass).to receive(:foo).with(:param_one, :param_two).and_return(:result_one)
Expand Down

0 comments on commit f5f95f5

Please sign in to comment.