Merge pull request #2874 from rspec/unify-multi-condition-filtering-2

Unify multi-condition filtering

Changelog.md

@@ -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:
 

benchmarks/module_inclusion_filtering.rb

@@ -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

lib/rspec/core/configuration.rb

@@ -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
@@ -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
@@ -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)

lib/rspec/core/filter_manager.rb

@@ -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
@@ -166,7 +166,7 @@ def description
       end
 
       def include_example?(example)
-        MetadataFilter.apply?(:any?, @rules, example.metadata)
+        MetadataFilter.apply?(@rules, example.metadata)
       end
     end
 

lib/rspec/core/hooks.rb

@@ -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
 

lib/rspec/core/metadata_filter.rb

@@ -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
@@ -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
 
@@ -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
@@ -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

spec/rspec/core/configuration_spec.rb

@@ -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
@@ -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)

spec/rspec/core/filter_manager_spec.rb

@@ -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
 

spec/rspec/core/filterable_item_repository_spec.rb

@@ -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") }
@@ -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
@@ -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]).
@@ -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