Ensure idempotent hook registration if groups/examples are configured multiple times.

Author: myronmarston

lib/rspec/core/configuration.rb

@@ -1635,7 +1635,7 @@ def disable_monkey_patching!
       #   end
       def define_derived_metadata(*filters, &block)
         meta = Metadata.build_hash_from(filters, :warn_about_example_group_filtering)
-        @derived_metadata_blocks.append(block, meta)
+        @derived_metadata_blocks.idempotently_append(block, meta)
       end
 
       # Defines a callback that runs after the first example with matching
@@ -1670,7 +1670,7 @@ def when_first_matching_example_defined(*filters, &block)
           block.call
         end
 
-        @derived_metadata_blocks.append(callback, specified_meta)
+        @derived_metadata_blocks.idempotently_append(callback, specified_meta)
       end
 
       # @private
@@ -2009,7 +2009,7 @@ def define_mixed_in_module(mod, filters, mod_list, config_method, &block)
         end
 
         meta = Metadata.build_hash_from(filters, :warn_about_example_group_filtering)
-        mod_list.append(mod, meta)
+        mod_list.idempotently_append(mod, meta)
         on_existing_matching_groups(meta, &block)
       end
     end

lib/rspec/core/hooks.rb

@@ -195,7 +195,7 @@ module Hooks
       #       `RSpec.configuration` since they exist independently of any
       #       example or example group.
       def before(*args, &block)
-        hooks.register :append, :before, *args, &block
+        hooks.register :idempotently_append, :before, *args, &block
       end
 
       alias_method :append_before, :before
@@ -205,7 +205,7 @@ def before(*args, &block)
       #
       # See {#before} for scoping semantics.
       def prepend_before(*args, &block)
-        hooks.register :prepend, :before, *args, &block
+        hooks.register :idempotently_prepend, :before, *args, &block
       end
 
       # @api public
@@ -269,7 +269,7 @@ def prepend_before(*args, &block)
       #       `RSpec.configuration` since they exist independently of any
       #       example or example group.
       def after(*args, &block)
-        hooks.register :prepend, :after, *args, &block
+        hooks.register :idempotently_prepend, :after, *args, &block
       end
 
       alias_method :prepend_after, :after
@@ -279,7 +279,7 @@ def after(*args, &block)
       #
       # See {#after} for scoping semantics.
       def append_after(*args, &block)
-        hooks.register :append, :after, *args, &block
+        hooks.register :idempotently_append, :after, *args, &block
       end
 
       # @api public
@@ -330,7 +330,7 @@ def append_after(*args, &block)
       #     around(:example) {|ex| FakeFS(&ex)}
       #
       def around(*args, &block)
-        hooks.register :prepend, :around, *args, &block
+        hooks.register :idempotently_prepend, :around, *args, &block
       end
 
       # @private
@@ -563,7 +563,7 @@ def process(host, parent_groups, globals, position, scope)
           return if hooks_to_process.empty?
 
           repository = ensure_hooks_initialized_for(position, scope)
-          hooks_to_process.each { |hook| repository.append hook, (yield hook) }
+          hooks_to_process.each { |hook| repository.idempotently_append hook, (yield hook) }
         end
 
         def scope_and_options_from(*args)

lib/rspec/core/metadata_filter.rb

@@ -91,7 +91,7 @@ def filters_apply?(key, value, metadata)
     # @private
     module FilterableItemRepository
       # This implementation is simple, and is optimized for frequent
-      # updates but rare queries. `append` and `prepend` do no extra
+      # updates but rare queries. `idempotently_append` and `idempotently_prepend` do no extra
       # processing, and no internal memoization is done, since this
       # is not optimized for queries.
       #
@@ -106,14 +106,15 @@ class UpdateOptimized
         def initialize(applies_predicate)
           @applies_predicate = applies_predicate
           @items_and_filters = []
+          @idempotence_set = Set.new
         end
 
-        def append(item, metadata)
-          @items_and_filters << [item, metadata]
+        def idempotently_append(item, metadata)
+          idempotently_update(item, metadata, :<<)
         end
 
-        def prepend(item, metadata)
-          @items_and_filters.unshift [item, metadata]
+        def idempotently_prepend(item, metadata)
+          idempotently_update(item, metadata, :unshift)
         end
 
         def items_for(request_meta)
@@ -135,6 +136,16 @@ def items_for(request_meta)
           end
           # :nocov:
         end
+
+      private
+
+        def idempotently_update(item, metadata, update_method)
+          idempotence_id = [item.__id__, metadata]
+          return if @idempotence_set.include?(idempotence_id)
+
+          @items_and_filters.__send__(update_method, [item, metadata])
+          @idempotence_set << idempotence_id
+        end
       end
 
       # This implementation is much more complex, and is optimized for
@@ -162,12 +173,12 @@ def initialize(applies_predicate)
           end
         end
 
-        def append(item, metadata)
+        def idempotently_append(item, metadata)
           super
           handle_mutation(metadata)
         end
 
-        def prepend(item, metadata)
+        def idempotently_prepend(item, metadata)
           super
           handle_mutation(metadata)
         end

spec/rspec/core/filterable_item_repository_spec.rb

@@ -22,6 +22,24 @@ def self.it_behaves_like_a_filterable_item_repo(&when_the_repo_has_items_with_me
               repo.__send__ add_method, *args
             end
 
+            it "is idempotent" do
+              add_item item_1, {}
+
+              expect {
+                add_item item_1, {}
+              }.not_to change { repo.items_for({}) }.from([item_1])
+            end
+
+            it 'allows the same item to be added with different metadata' do
+              add_item item_1, {:foo => 1}
+              add_item item_1, {:bar => 2}
+
+              expect(repo.items_for({})).to eq []
+              expect(repo.items_for(:foo => 1)).to eq [item_1]
+              expect(repo.items_for(:bar => 2)).to eq [item_1]
+              expect(repo.items_for(:foo => 1, :bar => 2)).to eq [item_1, item_1]
+            end
+
             context "when the repository has items that have no metadata" do
               before do
                 add_item item_1, {}
@@ -125,11 +143,11 @@ def self.it_behaves_like_a_filterable_item_repo(&when_the_repo_has_items_with_me
           end
         end
 
-        it_behaves_like "adding items to the repository", :append do
+        it_behaves_like "adding items to the repository", :idempotently_append do
           let(:items_in_expected_order) { [item_1, item_2, item_3] }
         end
 
-        it_behaves_like "adding items to the repository", :prepend do
+        it_behaves_like "adding items to the repository", :idempotently_prepend do
           let(:items_in_expected_order) { [item_3, item_2, item_1] }
         end
       end