From 755b3142a5692f3b6e5da3b22cbbfdea1678c536 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Sat, 28 Feb 2015 22:03:49 -0700 Subject: [PATCH] Doublespeak: Only store original method once If a method for a class is doubled more than once within the same test run, the original implementation of that method will change from double to double, even if the double is deactivated correctly. Say we have two distinct tests that both double the same method. Here is how that method will be overridden as it goes along: A) START: original method B) ACTIVATE (1st test): method is doubled C) DEACTIVATE (1st test): calls original method (A) D) ACTIVATE (2nd test): original method (C) stored; method is again doubled E) DEACTIVATE (2nd test): calls original method (C) With this commit, this changes to: A) START: original method B) ACTIVATE (1st test): method is doubled C) DEACTIVATE (1st test): calls original method (A) D) ACTIVATE (2nd test): original method not stored again; method is again doubled E) DEACTIVATE (2nd test): calls original method (A) --- lib/shoulda/matchers/doublespeak/double.rb | 18 ++- .../matchers/doublespeak/double_collection.rb | 7 +- lib/shoulda/matchers/doublespeak/world.rb | 22 ++- .../doublespeak/double_collection_spec.rb | 26 +-- .../matchers/doublespeak/double_spec.rb | 151 +++++++++++++++--- .../matchers/doublespeak/world_spec.rb | 11 +- 6 files changed, 185 insertions(+), 50 deletions(-) diff --git a/lib/shoulda/matchers/doublespeak/double.rb b/lib/shoulda/matchers/doublespeak/double.rb index 856be214..895052a1 100644 --- a/lib/shoulda/matchers/doublespeak/double.rb +++ b/lib/shoulda/matchers/doublespeak/double.rb @@ -5,7 +5,8 @@ module Shoulda class Double attr_reader :calls - def initialize(klass, method_name, implementation) + def initialize(world, klass, method_name, implementation) + @world = world @klass = klass @method_name = method_name @implementation = implementation @@ -41,17 +42,20 @@ module Shoulda end def call_original_method(call) - if original_method - original_method.bind(call.object).call(*call.args, &call.block) + unbound_method = world.original_method_for(klass, call.method_name) + + if unbound_method + unbound_method.bind(call.object).call(*call.args, &call.block) end end protected - attr_reader :klass, :method_name, :implementation, :original_method + attr_reader :world, :klass, :method_name, :implementation, + :original_method def store_original_method - @original_method = klass.instance_method(method_name) + world.store_original_method_for(klass, method_name) end def replace_method_with_double @@ -72,8 +76,10 @@ module Shoulda end def restore_original_method - original_method = @original_method + original_method = world.original_method_for(klass, method_name) + klass.__send__(:remove_method, method_name) + klass.__send__(:define_method, method_name) do |*args, &block| original_method.bind(self).call(*args, &block) end diff --git a/lib/shoulda/matchers/doublespeak/double_collection.rb b/lib/shoulda/matchers/doublespeak/double_collection.rb index 77a0b07d..2aadcc00 100644 --- a/lib/shoulda/matchers/doublespeak/double_collection.rb +++ b/lib/shoulda/matchers/doublespeak/double_collection.rb @@ -3,7 +3,8 @@ module Shoulda module Doublespeak # @private class DoubleCollection - def initialize(klass) + def initialize(world, klass) + @world = world @klass = klass @doubles_by_method_name = {} end @@ -40,12 +41,12 @@ module Shoulda protected - attr_reader :klass, :doubles_by_method_name + attr_reader :world, :klass, :doubles_by_method_name def register_double(method_name, implementation_type) implementation = DoubleImplementationRegistry.find(implementation_type) - double = Double.new(klass, method_name, implementation) + double = Double.new(world, klass, method_name, implementation) doubles_by_method_name[method_name] = double double end diff --git a/lib/shoulda/matchers/doublespeak/world.rb b/lib/shoulda/matchers/doublespeak/world.rb index 7011ae13..48bddb74 100644 --- a/lib/shoulda/matchers/doublespeak/world.rb +++ b/lib/shoulda/matchers/doublespeak/world.rb @@ -4,7 +4,19 @@ module Shoulda # @private class World def double_collection_for(klass) - double_collections_by_class[klass] ||= DoubleCollection.new(klass) + double_collections_by_class[klass] ||= + DoubleCollection.new(self, klass) + end + + def store_original_method_for(klass, method_name) + original_methods_for_class(klass)[method_name] ||= + klass.instance_method(method_name) + end + + def original_method_for(klass, method_name) + if original_methods_by_class.key?(klass) + original_methods_by_class[klass][method_name] + end end def with_doubles_activated @@ -31,6 +43,14 @@ module Shoulda def double_collections_by_class @_double_collections_by_class ||= {} end + + def original_methods_by_class + @_original_methods_by_class ||= {} + end + + def original_methods_for_class(klass) + original_methods_by_class[klass] ||= {} + end end end end diff --git a/spec/unit/shoulda/matchers/doublespeak/double_collection_spec.rb b/spec/unit/shoulda/matchers/doublespeak/double_collection_spec.rb index d699609f..2d5ac694 100644 --- a/spec/unit/shoulda/matchers/doublespeak/double_collection_spec.rb +++ b/spec/unit/shoulda/matchers/doublespeak/double_collection_spec.rb @@ -5,7 +5,7 @@ module Shoulda::Matchers::Doublespeak describe '#register_stub' do it 'calls DoubleImplementationRegistry.find correctly' do allow(DoubleImplementationRegistry).to receive(:find) - double_collection = described_class.new(:klass) + double_collection = described_class.new(build_world, :klass) double_collection.register_stub(:a_method) @@ -13,24 +13,25 @@ module Shoulda::Matchers::Doublespeak end it 'calls Double.new correctly' do + world = build_world allow(DoubleImplementationRegistry). to receive(:find). and_return(:implementation) allow(Double).to receive(:new) - double_collection = described_class.new(:klass) + double_collection = described_class.new(world, :klass) double_collection.register_stub(:a_method) expect(Double). to have_received(:new). - with(:klass, :a_method, :implementation) + with(world, :klass, :a_method, :implementation) end end describe '#register_proxy' do it 'calls DoubleImplementationRegistry.find correctly' do allow(DoubleImplementationRegistry).to receive(:find) - double_collection = described_class.new(:klass) + double_collection = described_class.new(build_world, :klass) double_collection.register_proxy(:a_method) @@ -40,24 +41,25 @@ module Shoulda::Matchers::Doublespeak end it 'calls Double.new correctly' do + world = build_world allow(DoubleImplementationRegistry). to receive(:find). and_return(:implementation) allow(Double).to receive(:new) - double_collection = described_class.new(:klass) + double_collection = described_class.new(world, :klass) double_collection.register_proxy(:a_method) expect(Double). to have_received(:new). - with(:klass, :a_method, :implementation) + with(world, :klass, :a_method, :implementation) end end describe '#activate' do it 'replaces all registered methods with doubles' do klass = create_class(first_method: 1, second_method: 2) - double_collection = described_class.new(klass) + double_collection = described_class.new(build_world, klass) double_collection.register_stub(:first_method) double_collection.register_stub(:second_method) @@ -72,7 +74,7 @@ module Shoulda::Matchers::Doublespeak describe '#deactivate' do it 'restores the original methods that were doubled' do klass = create_class(first_method: 1, second_method: 2) - double_collection = described_class.new(klass) + double_collection = described_class.new(build_world, klass) double_collection.register_stub(:first_method) double_collection.register_stub(:second_method) @@ -88,7 +90,7 @@ module Shoulda::Matchers::Doublespeak describe '#calls_to' do it 'returns all calls to the given method' do klass = create_class(a_method: nil) - double_collection = described_class.new(klass) + double_collection = described_class.new(build_world, klass) double_collection.register_stub(:a_method) double_collection.activate @@ -108,7 +110,7 @@ module Shoulda::Matchers::Doublespeak it 'returns an empty array if the method has never been doubled' do klass = create_class - double_collection = described_class.new(klass) + double_collection = described_class.new(build_world, klass) expect(double_collection.calls_to(:non_existent_method)).to eq [] end end @@ -120,5 +122,9 @@ module Shoulda::Matchers::Doublespeak end end end + + def build_world + Shoulda::Matchers::Doublespeak::World.new + end end end diff --git a/spec/unit/shoulda/matchers/doublespeak/double_spec.rb b/spec/unit/shoulda/matchers/doublespeak/double_spec.rb index 5c276c35..61735f3a 100644 --- a/spec/unit/shoulda/matchers/doublespeak/double_spec.rb +++ b/spec/unit/shoulda/matchers/doublespeak/double_spec.rb @@ -11,14 +11,24 @@ module Shoulda::Matchers::Doublespeak implementation.singleton_class.__send__(:define_method, :returns) do |&block| actual_block = block end - double = described_class.new(:klass, :a_method, implementation) + double = described_class.new( + :a_world, + :klass, + :a_method, + implementation + ) double.to_return(&sent_block) expect(actual_block).to eq sent_block end it 'tells its implementation to return the given value' do implementation = build_implementation - double = described_class.new(:klass, :a_method, implementation) + double = described_class.new( + :a_world, + :klass, + :a_method, + implementation + ) double.to_return(:implementation) expect(implementation).to have_received(:returns).with(:implementation) @@ -32,7 +42,12 @@ module Shoulda::Matchers::Doublespeak implementation.singleton_class.__send__(:define_method, :returns) do |&block| actual_block = block end - double = described_class.new(:klass, :a_method, implementation) + double = described_class.new( + :a_world, + :klass, + :a_method, + implementation + ) double.to_return(:value, &sent_block) expect(actual_block).to eq sent_block end @@ -44,7 +59,12 @@ module Shoulda::Matchers::Doublespeak method_name = :a_method klass = create_class(method_name => :some_return_value) instance = klass.new - double = described_class.new(klass, method_name, implementation) + double = described_class.new( + build_world, + klass, + method_name, + implementation + ) args = [:any, :args] block = -> {} call = MethodCall.new( @@ -66,10 +86,17 @@ module Shoulda::Matchers::Doublespeak describe '#deactivate' do it 'restores the original method after being doubled' do - implementation = build_implementation klass = create_class(a_method: 42) + world = build_world( + original_method_for: klass.instance_method(:a_method) + ) instance = klass.new - double = described_class.new(klass, :a_method, implementation) + double = described_class.new( + world, + klass, + :a_method, + build_implementation + ) double.activate double.deactivate @@ -77,10 +104,18 @@ module Shoulda::Matchers::Doublespeak end it 'still restores the original method if #activate was called twice' do - implementation = build_implementation - klass = create_class(a_method: 42) + method_name = :a_method + klass = create_class(method_name => 42) + world = build_world( + original_method_for: klass.instance_method(:a_method) + ) instance = klass.new - double = described_class.new(klass, :a_method, implementation) + double = described_class.new( + world, + klass, + :a_method, + build_implementation + ) double.activate double.activate @@ -89,10 +124,14 @@ module Shoulda::Matchers::Doublespeak end it 'does nothing if the method has not been doubled' do - implementation = build_implementation klass = create_class(a_method: 42) instance = klass.new - double = described_class.new(klass, :a_method, implementation) + double = described_class.new( + :a_world, + klass, + :a_method, + build_implementation + ) double.deactivate expect(instance.a_method).to eq 42 @@ -101,7 +140,12 @@ module Shoulda::Matchers::Doublespeak describe '#record_call' do it 'adds the given call to the list of calls' do - double = described_class.new(:a_klass, :a_method, :an_implementation) + double = described_class.new( + :a_world, + :a_klass, + :a_method, + :an_implementation + ) double.record_call(:some_call) expect(double.calls.last).to eq :some_call end @@ -109,35 +153,82 @@ module Shoulda::Matchers::Doublespeak describe '#call_original_method' do it 'binds the stored method object to the given object and calls it with the given args and block' do - klass = create_class - instance = klass.new - actual_args = actual_block = method_called = nil expected_args = [:one, :two, :three] expected_block = -> { } + actual_args = actual_block = method_called = nil + method_name = :a_method + klass = create_class + klass.__send__(:define_method, method_name) do |*args, &block| + actual_args = args + actual_block = block + method_called = true + end + world = build_world( + original_method_for: klass.instance_method(method_name) + ) + instance = klass.new call = double('call', object: instance, + method_name: method_name, args: expected_args, block: expected_block ) - double = described_class.new(klass, :a_method, :an_implementation) - - klass.__send__(:define_method, :a_method) do |*args, &block| - actual_args = expected_args - actual_block = expected_block - method_called = true - end + double = described_class.new( + world, + klass, + method_name, + :an_implementation + ) double.activate double.call_original_method(call) - expect(expected_args).to eq actual_args - expect(expected_block).to eq actual_block + expect(actual_args).to eq expected_args + expect(actual_block).to eq expected_block expect(method_called).to eq true end it 'does nothing if no method has been stored' do - double = described_class.new(:klass, :a_method, :an_implementation) - expect { double.call_original_method(:a_call) }.not_to raise_error + method_name = :a_method + world = build_world(original_method_for: nil) + call = double('call', method_name: method_name) + double = described_class.new( + world, + :klass, + method_name, + :an_implementation + ) + expect { double.call_original_method(call) }.not_to raise_error + end + + it 'does not store the original method multiple times when a method is doubled multiple times' do + world = Shoulda::Matchers::Doublespeak::World.new + klass = create_class(a_method: :some_return_value) + method_name = :a_method + doubles = 2.times.map do + described_class.new( + world, + klass, + method_name, + build_implementation + ) + end + instance = klass.new + + doubles[0].activate + + was_called = false + klass.__send__(:define_method, method_name) do + was_called = true + end + + doubles[1].activate + + doubles.each(&:deactivate) + + instance.__send__(method_name) + + expect(was_called).to be false end end @@ -152,5 +243,13 @@ module Shoulda::Matchers::Doublespeak def build_implementation double('implementation', returns: nil, call: nil) end + + def build_world(methods = {}) + defaults = { + original_method_for: nil, + store_original_method_for: nil + } + double('world', defaults.merge(methods)) + end end end diff --git a/spec/unit/shoulda/matchers/doublespeak/world_spec.rb b/spec/unit/shoulda/matchers/doublespeak/world_spec.rb index 12b7e151..af385876 100644 --- a/spec/unit/shoulda/matchers/doublespeak/world_spec.rb +++ b/spec/unit/shoulda/matchers/doublespeak/world_spec.rb @@ -10,16 +10,19 @@ module Shoulda::Matchers::Doublespeak world.double_collection_for(:klass) world.double_collection_for(:klass) - expect(DoubleCollection).to have_received(:new).with(:klass).once + expect(DoubleCollection). + to have_received(:new). + with(world, :klass). + once end it 'returns the created DoubleCollection' do + world = described_class.new double_collection = build_double_collection allow(DoubleCollection). to receive(:new). - with(:klass). + with(world, :klass). and_return(double_collection) - world = described_class.new expect(world.double_collection_for(:klass)).to be double_collection end @@ -40,7 +43,7 @@ module Shoulda::Matchers::Doublespeak double_collections.zip(klasses).each do |double_collection, klass| allow(DoubleCollection). to receive(:new). - with(klass). + with(world, klass). and_return(double_collection) world.double_collection_for(klass) end