Fix adding config hooks to existing example groups

When an example group is created, the existing config hooks are added to
it if their metadata filters match and none of the new example group's
parent groups already have the hook added.

The current implementation adds new config hooks to all existing example
groups, which means they can be run multiple times if several nested
example groups all match their metadata filters, which doesn't happen if
the config hook is defined first.

To get the same behaviour regardless of the order in which hooks and
groups are defined, we can walk the inheritence heirarchy starting from
the top level example groups, and only add the new hook to the first
matching group on each branch.

Author: eugeneius

lib/rspec/core/configuration.rb

@@ -1749,7 +1749,7 @@ def before(scope=nil, *meta, &block)
         handle_suite_hook(scope, meta) do
           @before_suite_hooks << Hooks::BeforeHook.new(block, {})
         end || begin
-          on_existing_matching_groups({}) { |g| g.before(scope, *meta, &block) }
+          add_hook_to_existing_groups(:append, :before, scope, *meta, &block)
           super(scope, *meta, &block)
         end
       end
@@ -1772,7 +1772,7 @@ def prepend_before(scope=nil, *meta, &block)
         handle_suite_hook(scope, meta) do
           @before_suite_hooks.unshift Hooks::BeforeHook.new(block, {})
         end || begin
-          on_existing_matching_groups({}) { |g| g.prepend_before(scope, *meta, &block) }
+          add_hook_to_existing_groups(:prepend, :before, scope, *meta, &block)
           super(scope, *meta, &block)
         end
       end
@@ -1790,7 +1790,7 @@ def after(scope=nil, *meta, &block)
         handle_suite_hook(scope, meta) do
           @after_suite_hooks.unshift Hooks::AfterHook.new(block, {})
         end || begin
-          on_existing_matching_groups({}) { |g| g.after(scope, *meta, &block) }
+          add_hook_to_existing_groups(:prepend, :after, scope, *meta, &block)
           super(scope, *meta, &block)
         end
       end
@@ -1813,7 +1813,7 @@ def append_after(scope=nil, *meta, &block)
         handle_suite_hook(scope, meta) do
           @after_suite_hooks << Hooks::AfterHook.new(block, {})
         end || begin
-          on_existing_matching_groups({}) { |g| g.append_after(scope, *meta, &block) }
+          add_hook_to_existing_groups(:append, :after, scope, *meta, &block)
           super(scope, *meta, &block)
         end
       end
@@ -1822,8 +1822,7 @@ def append_after(scope=nil, *meta, &block)
       #
       # See {Hooks#around} for full `around` hook docs.
       def around(scope=nil, *meta, &block)
-        on_existing_matching_groups({}) { |g| g.around(scope, *meta, &block) }
-
+        add_hook_to_existing_groups(:prepend, :around, scope, *meta, &block)
         super(scope, *meta, &block)
       end
 
@@ -2033,6 +2032,12 @@ def on_existing_matching_groups(meta)
         end
       end
 
+      def add_hook_to_existing_groups(prepend_or_append, position, *meta, &block)
+        world.example_groups.each do |group|
+          group.hooks.register_global_hook(prepend_or_append, position, *meta, &block)
+        end
+      end
+
       if RSpec::Support::RubyFeatures.module_prepends_supported?
         def safe_prepend(mod, host)
           host.__send__(:prepend, mod) unless host < mod

lib/rspec/core/hooks.rb

@@ -438,6 +438,18 @@ def register_global_singleton_context_hooks(example, globals)
           process(example, parent_groups, globals, :after,  :context) { {} }
         end
 
+        def register_global_hook(prepend_or_append, position, *args, &block)
+          scope, options = scope_and_options_from(*args)
+
+          if scope == :example || options.empty? || MetadataFilter.apply?(:all?, options, @owner.metadata)
+            register_hook(prepend_or_append, position, scope, options, &block)
+          else
+            @owner.children.each do |group|
+              group.hooks.register_global_hook(prepend_or_append, position, *args, &block)
+            end
+          end
+        end
+
         def register(prepend_or_append, position, *args, &block)
           scope, options = scope_and_options_from(*args)
 
@@ -451,8 +463,7 @@ def register(prepend_or_append, position, *args, &block)
             return
           end
 
-          hook = HOOK_TYPES[position][scope].new(block, options)
-          ensure_hooks_initialized_for(position, scope).__send__(prepend_or_append, hook, options)
+          register_hook(prepend_or_append, position, scope, options, &block)
         end
 
         # @private
@@ -553,6 +564,11 @@ def ensure_hooks_initialized_for(position, scope)
           end
         end
 
+        def register_hook(prepend_or_append, position, scope, options, &block)
+          hook = HOOK_TYPES[position][scope].new(block, options)
+          ensure_hooks_initialized_for(position, scope).__send__(prepend_or_append, hook, options)
+        end
+
         def process(host, parent_groups, globals, position, scope)
           hooks_to_process = globals.processable_hooks_for(position, scope, host)
           return if hooks_to_process.empty?

spec/rspec/core/hooks_filtering_spec.rb

@@ -129,6 +129,62 @@ module RSpec::Core
         group.run
         expect(sequence).to eq [:example]
       end
+
+      it "only runs example hooks once when there are multiple nested example groups" do
+        sequence = []
+
+        group = RSpec.describe do
+          context do
+            example { sequence << :ex_1 }
+            example { sequence << :ex_2 }
+          end
+        end
+
+        RSpec.configure do |c|
+          c.before(:example) { sequence << :before_ex_2 }
+          c.prepend_before(:example) { sequence << :before_ex_1 }
+
+          c.after(:example)  { sequence << :after_ex_1 }
+          c.append_after(:example) { sequence << :after_ex_2 }
+
+          c.around(:example) do |ex|
+            sequence << :around_before_ex
+            ex.run
+            sequence << :around_after_ex
+          end
+        end
+
+        group.run
+
+        expect(sequence).to eq [
+          :around_before_ex, :before_ex_1, :before_ex_2, :ex_1, :after_ex_1, :after_ex_2, :around_after_ex,
+          :around_before_ex, :before_ex_1, :before_ex_2, :ex_2, :after_ex_1, :after_ex_2, :around_after_ex
+        ]
+      end
+
+      it "only runs context hooks around the highest level group with matching filters" do
+        sequence = []
+
+        group = RSpec.describe do
+          before(:context) { sequence << :before_context }
+          after(:context)  { sequence << :after_context }
+
+          context "", :match do
+            context "", :match do
+              example { sequence << :example }
+            end
+          end
+        end
+
+        RSpec.configure do |config|
+          config.before(:context, :match) { sequence << :before_hook }
+          config.after(:context, :match)  { sequence << :after_hook }
+        end
+
+        group.run
+
+        expect(sequence).to eq [:before_context, :before_hook, :example, :after_hook, :after_context]
+      end
     end
 
     describe "unfiltered hooks" do