Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
pirj committed Feb 20, 2021
1 parent e0f638d commit 441a29b
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 55 deletions.
1 change: 1 addition & 0 deletions 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, #????)

Enhancements:

Expand Down
2 changes: 1 addition & 1 deletion 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 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 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 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 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
19 changes: 14 additions & 5 deletions spec/rspec/core/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1828,19 +1828,19 @@ 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(g2.metadata).not_to include(:reverse_description)
expect(g3.metadata).not_to include(:reverse_description)
end

Expand Down Expand Up @@ -2099,15 +2099,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
11 changes: 6 additions & 5 deletions spec/rspec/core/filter_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -292,15 +292,16 @@ 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(:bar => 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 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 441a29b

Please sign in to comment.