From 46f3cdb8bec650689b4781bfc2258b2a7245c925 Mon Sep 17 00:00:00 2001 From: Myron Marston Date: Wed, 3 Apr 2013 08:47:52 -0700 Subject: [PATCH] Externalize `any_instance` methods and state like we did mock proxies. This is similar to #250, but doing a similar refactoring for `any_instance`. --- lib/rspec/mocks.rb | 4 ++ lib/rspec/mocks/any_instance.rb | 30 +-------------- lib/rspec/mocks/any_instance/recorder.rb | 7 +++- lib/rspec/mocks/method_double.rb | 4 +- lib/rspec/mocks/space.rb | 20 ++++++++-- spec/rspec/mocks/any_instance_spec.rb | 7 ++-- spec/rspec/mocks/methods_spec.rb | 14 +++++-- spec/rspec/mocks/mock_space_spec.rb | 47 ++++++++++++++++++++++++ 8 files changed, 90 insertions(+), 43 deletions(-) diff --git a/lib/rspec/mocks.rb b/lib/rspec/mocks.rb index ddf4c9df3..2f2f4bb63 100644 --- a/lib/rspec/mocks.rb +++ b/lib/rspec/mocks.rb @@ -27,6 +27,10 @@ def proxy_for(object) space.proxy_for(object) end + def any_instance_recorder_for(klass) + space.any_instance_recorder_for(klass) + end + def configuration @configuration ||= Configuration.new end diff --git a/lib/rspec/mocks/any_instance.rb b/lib/rspec/mocks/any_instance.rb index 4993ec946..bae8d3d92 100644 --- a/lib/rspec/mocks/any_instance.rb +++ b/lib/rspec/mocks/any_instance.rb @@ -26,35 +26,7 @@ module AnyInstance # # @return [Recorder] def any_instance - AnyInstance.tracked_classes << self - __recorder - end - - # @private - def rspec_verify - __recorder.verify - ensure - __recorder.stop_all_observation! - @__recorder = nil - end - - # @private - def __recorder - @__recorder ||= AnyInstance::Recorder.new(self) - end - - def self.verify_all - tracked_classes.each do |klass| - klass.rspec_verify - end - end - - def self.reset_all - tracked_classes.clear - end - - def self.tracked_classes - @tracked_classes ||= [] + ::RSpec::Mocks.any_instance_recorder_for(self) end end end diff --git a/lib/rspec/mocks/any_instance/recorder.rb b/lib/rspec/mocks/any_instance/recorder.rb index a1c22d124..27a4cacfc 100644 --- a/lib/rspec/mocks/any_instance/recorder.rb +++ b/lib/rspec/mocks/any_instance/recorder.rb @@ -84,6 +84,9 @@ def verify if @expectation_set && !message_chains.all_expectations_fulfilled? raise RSpec::Mocks::MockExpectationError, "Exactly one instance should have received the following message(s) but didn't: #{message_chains.unfulfilled_expectations.sort.join(', ')}" end + ensure + stop_all_observation! + ::RSpec::Mocks.space.remove_any_instance_recorder_for(@klass) end # @private @@ -181,7 +184,7 @@ def observe!(method_name) @klass.class_eval(<<-EOM, __FILE__, __LINE__ + 1) def #{method_name}(*args, &blk) klass = ::RSpec::Mocks.method_handle_for(self, :#{method_name}).owner - klass.__recorder.playback!(self, :#{method_name}) + ::RSpec::Mocks.any_instance_recorder_for(klass).playback!(self, :#{method_name}) self.__send__(:#{method_name}, *args, &blk) end EOM @@ -193,7 +196,7 @@ def mark_invoked!(method_name) def #{method_name}(*args, &blk) method_name = :#{method_name} klass = ::RSpec::Mocks.method_handle_for(self, :#{method_name}).owner - invoked_instance = klass.__recorder.instance_that_received(method_name) + invoked_instance = ::RSpec::Mocks.any_instance_recorder_for(klass).instance_that_received(method_name) raise RSpec::Mocks::MockExpectationError, "The message '#{method_name}' was received by \#{self.inspect} but has already been received by \#{invoked_instance}" end EOM diff --git a/lib/rspec/mocks/method_double.rb b/lib/rspec/mocks/method_double.rb index c5284d9f2..1cadd4090 100644 --- a/lib/rspec/mocks/method_double.rb +++ b/lib/rspec/mocks/method_double.rb @@ -70,12 +70,12 @@ def original_method def original_unrecorded_any_instance_method return nil unless any_instance_class_recorder_observing_method?(@object.class) - alias_name = @object.class.__recorder.build_alias_method_name(@method_name) + alias_name = ::RSpec::Mocks.any_instance_recorder_for(@object.class).build_alias_method_name(@method_name) @object.method(alias_name) end def any_instance_class_recorder_observing_method?(klass) - return true if klass.__recorder.already_observing?(@method_name) + return true if ::RSpec::Mocks.any_instance_recorder_for(klass).already_observing?(@method_name) superklass = klass.superclass return false if superklass.nil? any_instance_class_recorder_observing_method?(superklass) diff --git a/lib/rspec/mocks/space.rb b/lib/rspec/mocks/space.rb index aa98cb75a..d4b63725e 100644 --- a/lib/rspec/mocks/space.rb +++ b/lib/rspec/mocks/space.rb @@ -2,10 +2,11 @@ module RSpec module Mocks # @api private class Space - attr_reader :proxies + attr_reader :proxies, :any_instance_recorders def initialize @proxies = {} + @any_instance_recorders = {} end def verify_all @@ -13,18 +14,20 @@ def verify_all object.verify end - AnyInstance.verify_all + any_instance_recorders.each_value do |recorder| + recorder.verify + end end def reset_all ConstantMutator.reset_all - AnyInstance.reset_all proxies.each_value do |object| object.reset end proxies.clear + any_instance_recorders.clear expectation_ordering.clear end @@ -32,6 +35,17 @@ def expectation_ordering @expectation_ordering ||= OrderGroup.new end + def any_instance_recorder_for(klass) + id = klass.__id__ + any_instance_recorders.fetch(id) do + any_instance_recorders[id] = AnyInstance::Recorder.new(klass) + end + end + + def remove_any_instance_recorder_for(klass) + any_instance_recorders.delete(klass.__id__) + end + def proxy_for(object) id = id_for(object) proxies.fetch(id) do diff --git a/spec/rspec/mocks/any_instance_spec.rb b/spec/rspec/mocks/any_instance_spec.rb index 1fe5763a6..09b5efed9 100644 --- a/spec/rspec/mocks/any_instance_spec.rb +++ b/spec/rspec/mocks/any_instance_spec.rb @@ -711,7 +711,7 @@ def foo; end end context "when resetting post-verification" do - let(:space) { RSpec::Mocks::Space.new } + let(:space) { RSpec::Mocks.space } context "existing method" do before(:each) do @@ -843,8 +843,9 @@ def foo; end end it "adds an class to the current space when #any_instance is invoked" do - klass.any_instance - expect(AnyInstance.tracked_classes).to include(klass) + expect { + klass.any_instance + }.to change { space.any_instance_recorders.size }.by(1) end it "adds an instance to the current space when stubbed method is invoked" do diff --git a/spec/rspec/mocks/methods_spec.rb b/spec/rspec/mocks/methods_spec.rb index cea3d639d..024ba8c67 100644 --- a/spec/rspec/mocks/methods_spec.rb +++ b/spec/rspec/mocks/methods_spec.rb @@ -3,22 +3,28 @@ module RSpec module Mocks describe Methods, :if => (Method.method_defined?(:owner)) do - def methods_added_to_all_objects - some_object = Object.new + def added_methods(klass, owner) + some_object = klass.new (some_object.methods + some_object.private_methods).select do |method| - some_object.method(method).owner == RSpec::Mocks::Methods + some_object.method(method).owner == owner end.map(&:to_sym) end it 'limits the number of methods that get added to all objects' do # If really necessary, you can add to this list, but long term, # we are hoping to cut down on the number of methods added to all objects - expect(methods_added_to_all_objects).to match_array([ + expect(added_methods(Object, RSpec::Mocks::Methods)).to match_array([ :as_null_object, :null_object?, :received_message?, :should_not_receive, :should_receive, :stub, :stub!, :stub_chain, :unstub, :unstub! ]) end + + it 'limits the number of methods that get added to all classes ' do + # If really necessary, you can add to this list, but long term, + # we are hoping to cut down on the number of methods added to all classes + expect(added_methods(Class, RSpec::Mocks::AnyInstance)).to match_array([:any_instance]) + end end end end diff --git a/spec/rspec/mocks/mock_space_spec.rb b/spec/rspec/mocks/mock_space_spec.rb index de7034f18..bd39ef97c 100644 --- a/spec/rspec/mocks/mock_space_spec.rb +++ b/spec/rspec/mocks/mock_space_spec.rb @@ -24,6 +24,25 @@ module Mocks expect(verifies).to match_array([:dbl_1, :dbl_2]) end + def define_singleton_method_on_recorder_for(klass, name, &block) + recorder = space.any_instance_recorder_for(klass) + (class << recorder; self; end).send(:define_method, name, &block) + end + + it 'verifies all any_instance recorders within' do + klass_1, klass_2 = Class.new, Class.new + + verifies = [] + + # We can't `stub` a method on the recorder because it defines its own `stub`... + define_singleton_method_on_recorder_for(klass_1, :verify) { verifies << :klass_1 } + define_singleton_method_on_recorder_for(klass_2, :verify) { verifies << :klass_2 } + + space.verify_all + + expect(verifies).to match_array([:klass_1, :klass_2]) + end + it "resets all mocks within" do resets = [] @@ -41,6 +60,13 @@ module Mocks }.to change { space.proxies.size }.to(0) end + it 'does not leak any instance recorders between examples' do + space.any_instance_recorder_for(Class.new) + expect { + space.reset_all + }.to change { space.any_instance_recorders.size }.to(0) + end + it "resets the ordering" do space.expectation_ordering.register :some_expectation @@ -60,6 +86,27 @@ module Mocks space.ensure_registered(m1) }.not_to change { space.proxies } end + + it 'returns a consistent any_instance_recorder for a given class' do + klass_1, klass_2 = Class.new, Class.new + + r1 = space.any_instance_recorder_for(klass_1) + r2 = space.any_instance_recorder_for(klass_1) + r3 = space.any_instance_recorder_for(klass_2) + + expect(r1).to be(r2) + expect(r1).not_to be(r3) + end + + it 'removes an any_instance_recorder when requested' do + klass = Class.new + + space.any_instance_recorder_for(klass) + + expect { + space.remove_any_instance_recorder_for(klass) + }.to change { space.any_instance_recorders.size }.by(-1) + end end end end