Skip to content

Commit

Permalink
[core] Merge pull request rspec/rspec-core#2874 from rspec/unify-mult…
Browse files Browse the repository at this point in the history
…i-condition-filtering-2

Unify multi-condition filtering

---
This commit was imported from rspec/rspec-core@65cff81.
  • Loading branch information
pirj authored Mar 4, 2021
2 parents 17ebfb0 + 507f238 commit 9142659
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 57 deletions.
1 change: 1 addition & 0 deletions rspec-core/Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Breaking Changes:
* Remove deprecated `rerun_argument` example method. (Phil Pirozhkov, #2864)
* Raise on attempt to use a legacy formatter without `rspec-legacy_formatters`.
(Phil Pirozhkov, #2864)
* Unify multi-condition filtering to use "all" semantic. (Phil Pirozhkov, #2874)

Enhancements:

Expand Down
2 changes: 1 addition & 1 deletion rspec-core/benchmarks/module_inclusion_filtering.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def include(mod, *filters)

def old_configure_group(group)
@include_extend_or_prepend_modules.each do |include_extend_or_prepend, mod, filters|
next unless filters.empty? || RSpec::Core::MetadataFilter.apply?(:any?, filters, group.metadata)
next unless filters.empty? || RSpec::Core::MetadataFilter.apply?(filters, group.metadata)
__send__("safe_#{include_extend_or_prepend}", mod, group)
end
end
Expand Down
10 changes: 5 additions & 5 deletions rspec-core/lib/rspec/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -413,9 +413,9 @@ def initialize
@start_time = $_rspec_core_load_started_at || ::RSpec::Core::Time.now
# rubocop:enable Style/GlobalVars
@expectation_frameworks = []
@include_modules = FilterableItemRepository::QueryOptimized.new(:any?)
@extend_modules = FilterableItemRepository::QueryOptimized.new(:any?)
@prepend_modules = FilterableItemRepository::QueryOptimized.new(:any?)
@include_modules = FilterableItemRepository::QueryOptimized.new
@extend_modules = FilterableItemRepository::QueryOptimized.new
@prepend_modules = FilterableItemRepository::QueryOptimized.new

@bisect_runner = RSpec::Support::RubyFeatures.fork_supported? ? :fork : :shell
@bisect_runner_class = nil
Expand Down Expand Up @@ -455,7 +455,7 @@ def initialize
@profile_examples = false
@requires = []
@libs = []
@derived_metadata_blocks = FilterableItemRepository::QueryOptimized.new(:any?)
@derived_metadata_blocks = FilterableItemRepository::QueryOptimized.new
@threadsafe = true
@max_displayed_failure_line_count = 10
@world = World::Null
Expand Down Expand Up @@ -2134,7 +2134,7 @@ def on_existing_matching_groups(meta)
end

def metadata_applies_to_group?(meta, group)
meta.empty? || MetadataFilter.apply?(:any?, meta, group.metadata)
meta.empty? || MetadataFilter.apply?(meta, group.metadata)
end

def safe_prepend(mod, host)
Expand Down
4 changes: 2 additions & 2 deletions rspec-core/lib/rspec/core/filter_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def prune(examples)

examples.select do |ex|
file_scoped_include?(ex.metadata, ids, locations) do
!exclusions.include_example?(ex) && non_scoped_inclusions.include_example?(ex)
(exclusions.empty? || !exclusions.include_example?(ex)) && non_scoped_inclusions.include_example?(ex)
end
end
end
Expand Down Expand Up @@ -166,7 +166,7 @@ def description
end

def include_example?(example)
MetadataFilter.apply?(:any?, @rules, example.metadata)
MetadataFilter.apply?(@rules, example.metadata)
end
end

Expand Down
10 changes: 5 additions & 5 deletions rspec-core/lib/rspec/core/hooks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -540,18 +540,18 @@ def hooks_for(position, scope)
def ensure_hooks_initialized_for(position, scope)
if position == :before
if scope == :example
@before_example_hooks ||= @filterable_item_repo_class.new(:all?)
@before_example_hooks ||= @filterable_item_repo_class.new
else
@before_context_hooks ||= @filterable_item_repo_class.new(:all?)
@before_context_hooks ||= @filterable_item_repo_class.new
end
elsif position == :after
if scope == :example
@after_example_hooks ||= @filterable_item_repo_class.new(:all?)
@after_example_hooks ||= @filterable_item_repo_class.new
else
@after_context_hooks ||= @filterable_item_repo_class.new(:all?)
@after_context_hooks ||= @filterable_item_repo_class.new
end
else # around
@around_example_hooks ||= @filterable_item_repo_class.new(:all?)
@around_example_hooks ||= @filterable_item_repo_class.new
end
end

Expand Down
11 changes: 5 additions & 6 deletions rspec-core/lib/rspec/core/metadata_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ module Core
module MetadataFilter
class << self
# @private
def apply?(predicate, filters, metadata)
filters.__send__(predicate) { |k, v| filter_applies?(k, v, metadata) }
def apply?(filters, metadata)
filters.all? { |k, v| filter_applies?(k, v, metadata) }
end

# @private
Expand Down Expand Up @@ -88,8 +88,7 @@ module FilterableItemRepository
class UpdateOptimized
attr_reader :items_and_filters

def initialize(applies_predicate)
@applies_predicate = applies_predicate
def initialize
@items_and_filters = []
end

Expand All @@ -108,7 +107,7 @@ def delete(item, metadata)
def items_for(request_meta)
@items_and_filters.each_with_object([]) do |(item, item_meta), to_return|
to_return << item if item_meta.empty? ||
MetadataFilter.apply?(@applies_predicate, item_meta, request_meta)
MetadataFilter.apply?(item_meta, request_meta)
end
end
end
Expand All @@ -129,7 +128,7 @@ class QueryOptimized < UpdateOptimized
alias find_items_for items_for
private :find_items_for

def initialize(applies_predicate)
def initialize
super
@applicable_keys = Set.new
@proc_keys = Set.new
Expand Down
21 changes: 14 additions & 7 deletions rspec-core/spec/rspec/core/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1828,20 +1828,18 @@ def exclude?(line)
expect(e3.metadata).not_to include(:reverse_description)
end

it 'applies if any of multiple filters apply (to align with module inclusion semantics)' do
it 'applies if all of multiple filters apply (to align with module inclusion semantics)' do
RSpec.configure do |c|
c.define_derived_metadata(:a => 1, :b => 2) do |metadata|
metadata[:reverse_description] = metadata[:description].reverse
end
end

g1 = RSpec.describe("G1", :a => 1)
g1 = RSpec.describe("G1", :a => 1, :b => 2)
g2 = RSpec.describe("G2", :b => 2)
g3 = RSpec.describe("G3", :c => 3)

expect(g1.metadata).to include(:reverse_description => "1G")
expect(g2.metadata).to include(:reverse_description => "2G")
expect(g3.metadata).not_to include(:reverse_description)
expect(g2.metadata).not_to include(:reverse_description)
end

it 'allows a metadata filter to be passed as a raw symbol' do
Expand Down Expand Up @@ -2099,15 +2097,24 @@ def exclude?(line)
expect(group.included_modules).to include(mod)
end

it "requires only one matching filter" do
it "requires exact matching filter" do
mod = Module.new
group = RSpec.describe("group", :foo => :bar)
group = RSpec.describe("group", :foo => :bar, :baz => :bam)

config.include(mod, :foo => :bar, :baz => :bam)
config.configure_group(group)
expect(group.included_modules).to include(mod)
end

it "doesn't include on incomplete matching filter" do
mod = Module.new
group = RSpec.describe("group", :foo => :bar)

config.include(mod, :foo => :bar, :baz => :bam)
config.configure_group(group)
expect(group.included_modules).not_to include(mod)
end

module IncludeExtendOrPrependMeOnce
def self.included(host)
raise "included again" if host.instance_methods.include?(:foobar)
Expand Down
10 changes: 5 additions & 5 deletions rspec-core/spec/rspec/core/filter_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -292,15 +292,15 @@ def add_filter(options)
end

context "with multiple inclusion filters" do
it 'includes objects that match any of them' do
it 'includes objects that match all of them' do
examples = [
included_1 = example_with(:foo => true),
included_2 = example_with(:bar => true),
example_with(:bazz => true)
included = example_with(:foo => true, :bar => true),
example_with(:foo => true),
example_with(:baz => true)
]

filter_manager.include :foo => true, :bar => true
expect(prune(examples)).to contain_exactly(included_1, included_2)
expect(prune(examples)).to contain_exactly(included)
end
end

Expand Down
36 changes: 10 additions & 26 deletions rspec-core/spec/rspec/core/filterable_item_repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Core
FilterableItem = Struct.new(:name)

def self.it_behaves_like_a_filterable_item_repo(&when_the_repo_has_items_with_metadata)
let(:repo) { described_class.new(:any?) }
let(:repo) { described_class.new }
let(:item_1) { FilterableItem.new("Item 1") }
let(:item_2) { FilterableItem.new("Item 2") }
let(:item_3) { FilterableItem.new("Item 3") }
Expand Down Expand Up @@ -96,28 +96,12 @@ def self.it_behaves_like_a_filterable_item_repo(&when_the_repo_has_items_with_me
end
end

context "when initialized with the `:any?` predicate" do
let(:repo) { FilterableItemRepository::QueryOptimized.new(:any?) }
it 'matches against multi-entry items when all of the metadata entries match' do
add_item item_4, :key_1 => "val_1", :key_2 => "val_2"

it 'matches against multi-entry items when any of the metadata entries match' do
add_item item_4, :key_1 => "val_1", :key_2 => "val_2"

expect(repo.items_for(:key_1 => "val_1")).to contain_exactly(item_4)
expect(repo.items_for(:key_2 => "val_2")).to contain_exactly(item_4)
expect(repo.items_for(:key_1 => "val_1", :key_2 => "val_2")).to contain_exactly(item_4)
end
end

context "when initialized with the `:all?` predicate" do
let(:repo) { FilterableItemRepository::QueryOptimized.new(:all?) }

it 'matches against multi-entry items when all of the metadata entries match' do
add_item item_4, :key_1 => "val_1", :key_2 => "val_2"

expect(repo.items_for(:key_1 => "val_1")).to eq([])
expect(repo.items_for(:key_2 => "val_2")).to eq([])
expect(repo.items_for(:key_1 => "val_1", :key_2 => "val_2")).to contain_exactly(item_4)
end
expect(repo.items_for(:key_1 => "val_1")).to eq([])
expect(repo.items_for(:key_2 => "val_2")).to eq([])
expect(repo.items_for(:key_1 => "val_1", :key_2 => "val_2")).to contain_exactly(item_4)
end

module_eval(&when_the_repo_has_items_with_metadata) if when_the_repo_has_items_with_metadata
Expand Down Expand Up @@ -156,8 +140,8 @@ def self.it_behaves_like_a_filterable_item_repo(&when_the_repo_has_items_with_me
[item_2, { :foo => true }]
]).
and change { repo.items_for({ :foo => true }) }.
from([item_1, item_1, item_1, item_2]).
to([item_1, item_1, item_2]).
from([item_1, item_1, item_2]).
to([item_1, item_2]).
and change { repo.items_for({ :foo => true, :bar => true }) }.
from([item_1, item_1, item_1, item_2]).
to([item_1, item_1, item_2]).
Expand Down Expand Up @@ -221,9 +205,9 @@ def self.it_behaves_like_a_filterable_item_repo(&when_the_repo_has_items_with_me

def track_metadata_filter_apply_calls
Hash.new(0).tap do |call_counts|
allow(MetadataFilter).to receive(:apply?).and_wrap_original do |original, predicate, item_meta, request_meta|
allow(MetadataFilter).to receive(:apply?).and_wrap_original do |original, item_meta, request_meta|
call_counts[item_meta] += 1
original.call(predicate, item_meta, request_meta)
original.call(item_meta, request_meta)
end
end
end
Expand Down

0 comments on commit 9142659

Please sign in to comment.