From fe1d49d124ac4af15e5f4fa0670f0f43be1fed4d Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Thu, 26 Jan 2017 20:40:37 +0000 Subject: [PATCH] Remove `ModuleMethod#method_exists?` This is similar to the change in commit 8f58eddf0ff658ad255cf60cedab3c767bbb15c7. Commit e87c03b068efc48267fbcd5a295514077c52b901 made `ModuleMethod#method_exists?` redundant by changing the implementation of `ClassMethod#hide_original_method` so that it uses `ClassMethod#method_visibility` instead. This introduced a subtle change in behaviour due to the difference in `ModuleMethod#method_exists?` and `ClassMethod#method_visibility`. The former would return false if the method being stubbed was on `Kernel` or `Object` while the latter will return true in those cases. This gist illustrates the difference - https://gist.github.com/chrisroos/12a1b032b95664448c9e987132f33988. Practically, this meant that _some_ stubbed module methods (those defined on Kernel and Object) wouldn't have had the correct visibility set. The tests added in this commit ensure we don't introduce regressions in future. The tests for the visibility of the protected and private methods failed prior to commit e87c03b068efc48267fbcd5a295514077c52b901. I confirmed this by checking our the commit before that (`git co e87c03b068efc48267fbcd5a295514077c52b901~`) and running these new tests. --- lib/mocha/module_method.rb | 8 --- ...ce_method_defined_on_kernel_module_test.rb | 63 +++++++++++++++++++ 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/lib/mocha/module_method.rb b/lib/mocha/module_method.rb index 8f842a87d..1eb6932cd 100644 --- a/lib/mocha/module_method.rb +++ b/lib/mocha/module_method.rb @@ -3,14 +3,6 @@ module Mocha class ModuleMethod < ClassMethod - - def method_exists?(method) - return true if stubbee.public_methods(false).include?(method) - return true if stubbee.protected_methods(false).include?(method) - return true if stubbee.private_methods(false).include?(method) - return false - end - end end diff --git a/test/acceptance/stub_instance_method_defined_on_kernel_module_test.rb b/test/acceptance/stub_instance_method_defined_on_kernel_module_test.rb index 722566a00..daffe1058 100644 --- a/test/acceptance/stub_instance_method_defined_on_kernel_module_test.rb +++ b/test/acceptance/stub_instance_method_defined_on_kernel_module_test.rb @@ -72,4 +72,67 @@ def my_instance_method ensure Kernel.module_eval { remove_method :my_instance_method } end + + def test_should_stub_public_module_method_and_leave_it_unchanged_after_test + Kernel.module_eval do + def my_instance_method + :original_return_value + end + public :my_instance_method + end + mod = Module.new + assert_snapshot_unchanged(mod) do + test_result = run_as_test do + mod.stubs(:my_instance_method).returns(:new_return_value) + assert_method_visibility mod, :my_instance_method, :public + assert_equal :new_return_value, mod.my_instance_method + end + assert_passed(test_result) + end + assert_equal :original_return_value, mod.my_instance_method + ensure + Kernel.module_eval { remove_method :my_instance_method } + end + + def test_should_stub_protected_module_method_and_leave_it_unchanged_after_test + Kernel.module_eval do + def my_instance_method + :original_return_value + end + protected :my_instance_method + end + mod = Module.new + assert_snapshot_unchanged(mod) do + test_result = run_as_test do + mod.stubs(:my_instance_method).returns(:new_return_value) + assert_method_visibility mod, :my_instance_method, :protected + assert_equal :new_return_value, mod.send(:my_instance_method) + end + assert_passed(test_result) + end + assert_equal :original_return_value, mod.send(:my_instance_method) + ensure + Kernel.module_eval { remove_method :my_instance_method } + end + + def test_should_stub_private_module_method_and_leave_it_unchanged_after_test + Kernel.module_eval do + def my_instance_method + :original_return_value + end + private :my_instance_method + end + mod = Module.new + assert_snapshot_unchanged(mod) do + test_result = run_as_test do + mod.stubs(:my_instance_method).returns(:new_return_value) + assert_method_visibility mod, :my_instance_method, :private + assert_equal :new_return_value, mod.send(:my_instance_method) + end + assert_passed(test_result) + end + assert_equal :original_return_value, mod.send(:my_instance_method) + ensure + Kernel.module_eval { remove_method :my_instance_method } + end end