Merge pull request #265 from rspec/refactor_any_instance

Externalize `any_instance` methods and state like we did mock proxies.

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

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

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

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)

lib/rspec/mocks/space.rb

@@ -2,36 +2,50 @@ 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
         proxies.each_value do |object|
           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
 
       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

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

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

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