error: methods spied on in module_function style are not properly reverted

When mocking module_functions / extend self modules, Spy does not recognize the ownership of the original method and won't re-apply it to the module during teardown. Here is a quick Minitest that shows this happening:

require 'test_helper'

class SpyTesting < ActiveSupport::TestCase
  module Example
    extend self

    def hello
      'hello world'
    end
  end

  test 'one' do
    Spy.on(Example, :hello).and_return('yo')
    assert_equal Example.hello, 'yo'
  end

  test 'two' do
    Spy.on(Example, :hello).and_return('yo')
    assert_equal Example.hello, 'yo'
  end

end

Running this code will always have one of the two tests fail.

Error:
SpyTesting#test_two:
NameError: undefined method `hello' for class `#<Class:SpyTesting::Example>'
    test/spy_testing.rb:18:in `block in <class:SpyTesting>'

2 tests, 1 assertions, 0 failures, 1 errors, 0 skips

This is due to the fact that in lib/spy/subroutine.rb:87 the method_owner does not equal the original_method.owner.

method_owner
=> #<Class:SpyTesting::Example>
method_owner.class
=> Class

original_method.owner
=> SpyTesting::Example
original_method.owner.class
=> Module

I'm still looking for the easiest fix, but. I figure I should share the issue either way. This works without issue in spy 1.0.0, which has been our solution for a while, but we will need to go to ruby 3 eventually. Removing the second conditional to the if statement will cause the tests to pass, but I assume it's there for an excellent reason. I'm still learning the internals.

Hey Ryan,

Yeah, this is broken in Spy 1.0.1. Here’s my test suite I was using in the forked gem to see how everything is performing. I just re-confirmed the fact using this, so found it came in handy.

require 'test_helper'

module Spy class TestUnhook < Minitest::Test module ModuleFunctionStyle extend self

  def hello
    'hello world'
  end
end

module Injected
  def hello
    'hello world'
  end
end

class ModuleInjectedStyle
  include Injected
end

class ModuleExtendedStyle
  extend Injected
end

class SingletonMethodStyle
  def self.hello
    'hello world'
  end
end

class InstanceMethodStyle
  def hello
    'hello world'
  end
end

def test_ModuleFunctionStyle
  spy = Spy.on(ModuleFunctionStyle, :hello).and_return('yo')
  assert_equal ModuleFunctionStyle.hello, 'yo'
  spy.unhook
  assert_equal ModuleFunctionStyle.hello, 'hello world'
end

def test_ModuleInjectedStyle
  instance = ModuleInjectedStyle.new
  spy = Spy.on(instance, :hello).and_return('yo')
  assert_equal instance.hello, 'yo'
  spy.unhook
  assert_equal instance.hello, 'hello world'
end

def test_ModuleExtendedStyle
  spy = Spy.on(ModuleExtendedStyle, :hello).and_return('yo')
  assert_equal ModuleExtendedStyle.hello, 'yo'
  spy.unhook
  assert_equal ModuleExtendedStyle.hello, 'hello world'
end

def test_SingletonMethodStyle
  spy = Spy.on(SingletonMethodStyle, :hello).and_return('yo')
  assert_equal SingletonMethodStyle.hello, 'yo'
  spy.unhook
  assert_equal SingletonMethodStyle.hello, 'hello world'
end

def test_InstanceMethodStyle
  instance = InstanceMethodStyle.new
  spy = Spy.on(instance, :hello).and_return('yo')
  assert_equal instance.hello, 'yo'
  spy.unhook
  assert_equal instance.hello, 'hello world'
end

end end

With the following change in 1.0.2 everything passes nicely. Running it in my full app with 6 years of tests (many Spy cases) the only thing that happened was some of my instance method block tests using args failed because the first argument there seems to be a reflection (self) of the spy instance. I didn’t bother testing to see if the arguments changed from 1.0.0 to 1.0.2, I just saw a comment in the fork talking about it being intentional somewhere and just fixed the issue by adding the |_, args|.

Spy.on_instance_method(PageText, :push!).andreturn { |, args| @page_text.update!(args) }

# unhooks method from object
# @return [self]
def unhook
  raise NeverHookedError, "'#{method_name}' method has not been hooked" unless hooked?

  method_owner.send(:remove_method, method_name)
  if original_method #&& method_owner == original_method.owner
    original_method.owner.send(:define_method, method_name, original_method)
    original_method.owner.send(original_method_visibility, method_name) if original_method_visibility
  end

  clear_method!
  Agency.instance.retire(self)
  self
end

So far, I’ve not been able to find any negative situations with this as the fix.

~ Adam

On Jul 13, 2022, at 8:41 AM, Ryan Ong @.***> wrote:

Interesting. So this is broken in 1.0.1?

— Reply to this email directly, view it on GitHub https://github.com/ryanong/spy/issues/25#issuecomment-1183174738, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPUVIMBWISYOAMRLMF7THDVT22RDANCNFSM53IOJEBQ. You are receiving this because you authored the thread.

ryanong / spy

error: methods spied on in module_function style are not properly reverted #25