Merge pull request rspec#1897 from rspec/fix-filtering-regression

Fix filtering regression

Author: JonRowe

lib/rspec/core/filter_manager.rb

@@ -35,12 +35,15 @@ def prune(examples)
         examples = prune_conditionally_filtered_examples(examples)
 
         if inclusions.standalone?
-          examples.select { |e| include?(e) }
+          examples.select { |e| inclusions.include_example?(e) }
         else
-          locations = inclusions.fetch(:locations) { Hash.new([]) }
-          ids       = inclusions.fetch(:ids)       { Hash.new([]) }
+          locations, ids, non_scoped_inclusions = inclusions.split_file_scoped_rules
 
-          examples.select { |e| priority_include?(e, ids, locations) || (!exclude?(e) && include?(e)) }
+          examples.select do |ex|
+            file_scoped_include?(ex.metadata, ids, locations) do
+              !exclusions.include_example?(ex) && non_scoped_inclusions.include_example?(ex)
+            end
+          end
         end
       end
 
@@ -76,14 +79,6 @@ def add_path_to_arrays_filter(filter_key, path, values)
         inclusions.add(filter_key => filter)
       end
 
-      def exclude?(example)
-        exclusions.include_example?(example)
-      end
-
-      def include?(example)
-        inclusions.include_example?(example)
-      end
-
       def prune_conditionally_filtered_examples(examples)
         examples.reject do |ex|
           meta = ex.metadata
@@ -96,10 +91,14 @@ def prune_conditionally_filtered_examples(examples)
       # and there is a `:slow => true` exclusion filter), but only for specs
       # defined in the same file as the location filters. Excluded specs in
       # other files should still be excluded.
-      def priority_include?(example, ids, locations)
-        return true if MetadataFilter.filter_applies?(:ids, ids, example.metadata)
-        return false if locations[example.metadata[:absolute_file_path]].empty?
-        MetadataFilter.filter_applies?(:locations, locations, example.metadata)
+      def file_scoped_include?(ex_metadata, ids, locations)
+        no_location_filters = locations[ex_metadata[:absolute_file_path]].empty?
+        no_id_filters = ids[ex_metadata[:rerun_file_path]].empty?
+
+        return yield if no_location_filters && no_id_filters
+
+        MetadataFilter.filter_applies?(:ids, ids, ex_metadata) ||
+        MetadataFilter.filter_applies?(:locations, locations, ex_metadata)
       end
     end
 
@@ -119,8 +118,8 @@ def self.build
         [exclusions, inclusions]
       end
 
-      def initialize(*args, &block)
-        @rules = Hash.new(*args, &block)
+      def initialize(rules={})
+        @rules = rules
       end
 
       def add(updated)
@@ -196,6 +195,14 @@ def standalone?
         is_standalone_filter?(@rules)
       end
 
+      def split_file_scoped_rules
+        rules_dup = @rules.dup
+        locations = rules_dup.delete(:locations) { Hash.new([]) }
+        ids       = rules_dup.delete(:ids)       { Hash.new([]) }
+
+        return locations, ids, self.class.new(rules_dup)
+      end
+
     private
 
       def apply_standalone_filter(updated)

lib/rspec/core/metadata_filter.rb

@@ -53,7 +53,7 @@ def id_filter_applies?(rerun_paths_to_scoped_ids, metadata)
 
         def location_filter_applies?(locations, metadata)
           line_numbers = example_group_declaration_lines(locations, metadata)
-          line_numbers.empty? || line_number_filter_applies?(line_numbers, metadata)
+          line_number_filter_applies?(line_numbers, metadata)
         end
 
         def line_number_filter_applies?(line_numbers, metadata)

spec/integration/filtering_spec.rb

@@ -5,14 +5,14 @@
   before { clean_current_dir }
 
   it 'prints a rerun command for shared examples in external files that works to rerun' do
-    write_file "spec/support/shared_examples.rb", """
+    write_file "spec/support/shared_examples.rb", "
       RSpec.shared_examples 'with a failing example' do
         example { expect(1).to eq(2) } # failing
         example { expect(2).to eq(2) } # passing
       end
-    """
+    "
 
-    write_file "spec/host_group_spec.rb", """
+    write_file "spec/host_group_spec.rb", "
       load File.expand_path('../support/shared_examples.rb', __FILE__)
 
       RSpec.describe 'A group with shared examples' do
@@ -22,7 +22,7 @@
       RSpec.describe 'A group with a passing example' do
         example { expect(1).to eq(1) }
       end
-    """
+    "
 
     run_command ""
     expect(last_cmd_stdout).to include("3 examples, 1 failure")
@@ -40,7 +40,7 @@ def run_rerun_command_for_failing_spec
 
   context "with a shared example containing a context in a separate file" do
     it "runs the example nested inside the shared" do
-      write_file_formatted 'spec/shared_example.rb', """
+      write_file_formatted 'spec/shared_example.rb', "
         RSpec.shared_examples_for 'a shared example' do
           it 'succeeds' do
           end
@@ -50,15 +50,15 @@ def run_rerun_command_for_failing_spec
             end
           end
         end
-      """
+      "
 
-      write_file_formatted 'spec/simple_spec.rb', """
+      write_file_formatted 'spec/simple_spec.rb', "
         require File.join(File.dirname(__FILE__), 'shared_example.rb')
 
         RSpec.describe 'top level' do
           it_behaves_like 'a shared example'
         end
-      """
+      "
 
       run_command 'spec/simple_spec.rb:3 -fd'
       expect(last_cmd_stdout).to match(/2 examples, 0 failures/)
@@ -67,7 +67,7 @@ def run_rerun_command_for_failing_spec
 
   context "passing a line-number filter" do
     it "trumps exclusions, except for :if/:unless (which are absolute exclusions)" do
-      write_file_formatted 'spec/a_spec.rb', """
+      write_file_formatted 'spec/a_spec.rb', "
         RSpec.configure do |c|
           c.filter_run_excluding :slow
         end
@@ -82,7 +82,7 @@ def run_rerun_command_for_failing_spec
           example('ex 4', :slow       ) { }
           example('ex 5', :if => false) { }
         end
-      """
+      "
 
       run_command "spec/a_spec.rb -fd"
       expect(last_cmd_stdout).to include("1 example, 0 failures", "ex 3").and exclude("ex 1", "ex 2", "ex 4", "ex 5")
@@ -100,14 +100,14 @@ def run_rerun_command_for_failing_spec
 
   context "passing a line-number-filtered file and a non-filtered file" do
     it "applies the line number filtering only to the filtered file, running all specs in the non-filtered file except excluded ones" do
-      write_file_formatted "spec/file_1_spec.rb", """
+      write_file_formatted "spec/file_1_spec.rb", "
         RSpec.describe 'File 1' do
           it('passes') {      }
           it('fails')  { fail }
         end
-      """
+      "
 
-      write_file_formatted "spec/file_2_spec.rb", """
+      write_file_formatted "spec/file_2_spec.rb", "
         RSpec.configure do |c|
           c.filter_run_excluding :exclude_me
         end
@@ -117,31 +117,53 @@ def run_rerun_command_for_failing_spec
           it('passes') { }
           it('fails', :exclude_me) { fail }
         end
-      """
+      "
 
       run_command "spec/file_1_spec.rb:2 spec/file_2_spec.rb -fd"
       expect(last_cmd_stdout).to match(/3 examples, 0 failures/)
       expect(last_cmd_stdout).not_to match(/fails/)
     end
+
+    it 'applies command line tag filters only to files that lack a line number filter' do
+      write_file_formatted "spec/file_1_spec.rb", "
+        RSpec.describe 'File 1' do
+          it('is selected by line')   { }
+          it('is not selected', :tag) { }
+        end
+      "
+
+      write_file_formatted "spec/file_2_spec.rb", "
+        RSpec.describe 'File 2' do
+          it('is not selected')          { }
+          it('is selected by tag', :tag) { }
+        end
+      "
+
+      run_command "spec/file_1_spec.rb:2 spec/file_2_spec.rb --tag tag -fd"
+      expect(last_cmd_stdout).to include(
+        "2 examples, 0 failures",
+        "is selected by line", "is selected by tag"
+      ).and exclude("not selected")
+    end
   end
 
   context "passing example ids at the command line" do
     it "selects matching examples" do
-      write_file_formatted "spec/file_1_spec.rb", """
+      write_file_formatted "spec/file_1_spec.rb", "
         RSpec.describe 'File 1' do
           1.upto(3) do |i|
             example('ex ' + i.to_s) { expect(i).to be_odd }
           end
         end
-      """
+      "
 
-      write_file_formatted "spec/file_2_spec.rb", """
+      write_file_formatted "spec/file_2_spec.rb", "
         RSpec.describe 'File 2' do
           1.upto(3) do |i|
             example('ex ' + i.to_s) { expect(i).to be_even }
           end
         end
-      """
+      "
 
       # Using the form that Metadata.relative_path returns...
       run_command "./spec/file_1_spec.rb[1:1,1:3] ./spec/file_2_spec.rb[1:2]"
@@ -162,7 +184,7 @@ def run_rerun_command_for_failing_spec
     end
 
     it "selects matching example groups" do
-      write_file_formatted "spec/file_1_spec.rb", """
+      write_file_formatted "spec/file_1_spec.rb", "
         RSpec.describe 'Group 1' do
           example { fail }
 
@@ -175,7 +197,7 @@ def run_rerun_command_for_failing_spec
             example { fail }
           end
         end
-      """
+      "
 
       run_command "./spec/file_1_spec.rb[1:2]"
       expect(last_cmd_stdout).to match(/2 examples, 0 failures/)

spec/integration/order_spec.rb

@@ -4,7 +4,7 @@
   include_context "aruba support"
 
   before :all do
-    write_file 'spec/simple_spec.rb', """
+    write_file 'spec/simple_spec.rb', "
       RSpec.describe 'group 1' do
         specify('group 1 example 1') {}
         specify('group 1 example 2') {}
@@ -15,9 +15,9 @@
           specify('group 1-1 example 3') {}
         end
       end
-    """
+    "
 
-    write_file 'spec/simple_spec2.rb', """
+    write_file 'spec/simple_spec2.rb', "
       RSpec.describe 'group 2' do
         specify('group 2 example 1') {}
         specify('group 2 example 2') {}
@@ -28,9 +28,9 @@
           specify('group 2-1 example 3') {}
         end
       end
-    """
+    "
 
-    write_file 'spec/order_spec.rb', """
+    write_file 'spec/order_spec.rb', "
       RSpec.describe 'group 1' do
         specify('group 1 example 1')  {}
         specify('group 1 example 2')  {}
@@ -76,7 +76,7 @@
       RSpec.describe('group 8')  { specify('example') {} }
       RSpec.describe('group 9')  { specify('example') {} }
       RSpec.describe('group 10') { specify('example') {} }
-    """
+    "
   end
 
   describe '--order rand' do
@@ -150,7 +150,7 @@
     after { remove_file 'spec/custom_order_spec.rb' }
 
     before do
-      write_file 'spec/custom_order_spec.rb', """
+      write_file 'spec/custom_order_spec.rb', "
         RSpec.configure do |config|
           config.register_ordering :global do |list|
             list.sort_by { |item| item.description }
@@ -167,7 +167,7 @@
         RSpec.describe 'group A' do
           specify('group A example 1')  {}
         end
-      """
+      "
     end
 
     it 'orders the groups and examples by the provided strategy' do

spec/rspec/core/filter_manager_spec.rb

@@ -129,6 +129,25 @@ def example_with(*args)
 
           expect(filter_manager.prune([included, excluded])).to eq([included])
         end
+
+        it "still applies inclusion filters to examples from files with no #{type} filters" do
+          group = RSpec.describe("group")
+          included_via_loc_or_id = group.example("inc via #{type}"); line = __LINE__
+          excluded_via_loc_or_id = group.example("exc via #{type}", :foo)
+
+          included_via_tag, excluded_via_tag = instance_eval <<-EOS, "some/other_spec.rb", 1
+            group = RSpec.describe("group")
+            [group.example("inc via tag", :foo), group.example("exc via tag")]
+          EOS
+
+          add_filter(:line_number => line, :scoped_id => "1:1")
+          filter_manager.include_with_low_priority :foo => true
+
+          expect(filter_manager.prune([
+            included_via_loc_or_id, excluded_via_loc_or_id,
+            included_via_tag, excluded_via_tag
+          ]).map(&:description)).to eq([included_via_loc_or_id, included_via_tag].map(&:description))
+        end
       end
 
       describe "location filtering" do
@@ -299,6 +318,24 @@ def add_filter(options)
           filter_manager.add_ids(Metadata.relative_path(__FILE__), %w[ 2 ])
           expect(filter_manager.prune([ex_1, ex_2, ex_3])).to eq([ex_2, ex_3])
         end
+
+        it 'uses the rerun file path when applying the id filter' do
+          ex_1, ex_2 = instance_eval <<-EOS, "./some/spec.rb", 1
+            ex_1 = ex_2 = nil
+
+            RSpec.shared_examples "shared" do
+              ex_1 = example("ex 1")
+              ex_2 = example("ex 2")
+            end
+
+            [ex_1, ex_2]
+          EOS
+
+          RSpec.describe { include_examples "shared" }
+
+          filter_manager.add_ids(__FILE__, %w[ 1:1 ])
+          expect(filter_manager.prune([ex_1, ex_2]).map(&:description)).to eq([ex_1].map(&:description))
+        end
       end
     end
 

spec/support/aruba_support.rb

@@ -13,6 +13,8 @@ module ArubaLoader
   attr_reader :last_cmd_stdout, :last_cmd_stderr
 
   def run_command(cmd)
+    RSpec.configuration.color = true
+
     temp_stdout = StringIO.new
     temp_stderr = StringIO.new
     RSpec::Core::Metadata.instance_variable_set(:@relative_path_regex, nil)
@@ -23,6 +25,7 @@ def run_command(cmd)
     end
   ensure
     RSpec.reset
+    RSpec.configuration.color = true
     RSpec::Core::Metadata.instance_variable_set(:@relative_path_regex, nil)
 
     # Ensure it gets cached with a proper value -- if we leave it set to nil,