Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Fix adding config hooks to existing example groups #2280

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions lib/rspec/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1749,7 +1749,8 @@ 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) }
metadata = Metadata.build_hash_from(meta.dup)
on_existing_matching_groups(metadata, scope) { |g| g.before(scope, *meta, &block) }
super(scope, *meta, &block)
end
end
Expand All @@ -1772,7 +1773,8 @@ 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) }
metadata = Metadata.build_hash_from(meta.dup)
on_existing_matching_groups(metadata, scope) { |g| g.prepend_before(scope, *meta, &block) }
super(scope, *meta, &block)
end
end
Expand All @@ -1790,7 +1792,8 @@ 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) }
metadata = Metadata.build_hash_from(meta.dup)
on_existing_matching_groups(metadata, scope) { |g| g.after(scope, *meta, &block) }
super(scope, *meta, &block)
end
end
Expand All @@ -1813,7 +1816,8 @@ 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) }
metadata = Metadata.build_hash_from(meta.dup)
on_existing_matching_groups(metadata, scope) { |g| g.append_after(scope, *meta, &block) }
super(scope, *meta, &block)
end
end
Expand All @@ -1822,8 +1826,8 @@ 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) }

metadata = Metadata.build_hash_from(meta.dup)
on_existing_matching_groups(metadata, scope) { |g| g.around(scope, *meta, &block) }
super(scope, *meta, &block)
end

Expand Down Expand Up @@ -2027,9 +2031,18 @@ def configure_group_with(group, module_list, application_method)
end
end

def on_existing_matching_groups(meta)
world.all_example_groups.each do |group|
yield group if meta.empty? || MetadataFilter.apply?(:any?, meta, group.metadata)
def on_existing_matching_groups(meta, scope=:ignore)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooks have some scope-specific behaviour, but helper modules don't have a scope - the optional argument feels ugly.

groups = world.example_groups.dup

until groups.empty?
group = groups.shift

if scope == :example || scope == :each || scope.nil? ||
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This duplicates some very specific hook-related logic:

In the previous implementation this logic were still duplicated, but at least it was still in the Hooks module.

meta.empty? || MetadataFilter.apply?(:any?, meta, group.metadata)
yield group
else
groups.concat(group.children)
end
end
end

Expand Down
56 changes: 56 additions & 0 deletions spec/rspec/core/hooks_filtering_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down