Only run feature hooks for features that will actually be run (#209)

* feature hooks only execute if the feature would run given the tags

Previously, all feature hooks were executing no matter what tags were provided. Files + line numbers didn't have this behavior; they actually resulted in only some feature hooks running.
Now, we only run a feature (and therefore it's hooks) if, for any of its scenarios, the combination of that feature's tags and that scenario's tags matches the run's tags.
This required changes to setup in some Runner tests, as Feature instances with no associated Scenario instances can't be included in a Runner run (because they have no Scenarios to match against).

* clarify documentation on filtering runs with tags

* fix typos

---------

Co-authored-by: Oriol Gual <[email protected]>

README.markdown

@@ -238,19 +238,19 @@ Feature: So something great
   Scenario: Ensure no regression on this
 ```
 
-Then you can run all Scenarios in your suite related to `@feat-1` using:
+Then you can run all Scenarios in your suite tagged `@feat-1` using:
 
 ```shell
 $ spinach --tags @feat-1
 ```
 
-Or only Scenarios related to `@feat-1` and `@bug-12` using:
+Or only Scenarios tagged either `@feat-1` or `@bug-12` using:
 
 ```shell
 $ spinach --tags @feat-1,@bug-12
 ```
 
-Or only Scenarios related to `@feat-1` excluding `@bug-12` using:
+Or only Scenarios tagged `@feat-1` that aren't tagged `@bug-12` using:
 
 ```shell
 $ spinach --tags @feat-1,~@bug-12

features/feature_hooks_and_tags.feature

@@ -0,0 +1,22 @@
+Feature: Feature Hooks and Tags
+  In order to run only the appropriate setup and teardown code
+  As a developer
+  I want spinach to only run feature hooks if those features would be run under the tags I provided
+
+  Scenario: No tags specified
+    Given I have a tagged feature with an untagged scenario
+    And I have an untagged feature with a tagged scenario
+    When I don't specify tags
+    Then all the feature hooks should have run
+
+  Scenario: Tags specified
+    Given I have a tagged feature with an untagged scenario
+    And I have an untagged feature with a tagged scenario
+    When I specify a tag the features and scenarios are tagged with
+    Then all the feature hooks should have run
+
+  Scenario: Tags excluded
+    Given I have a tagged feature with an untagged scenario
+    And I have an untagged feature with a tagged scenario
+    When I exclude a tag the features and scenarios are tagged with
+    Then no feature hooks should have run

features/steps/feature_hooks_and_tags.rb

@@ -0,0 +1,55 @@
+class Spinach::Features::FeatureHooksAndTags < Spinach::FeatureSteps
+  include Integration::SpinachRunner
+
+  step 'I have a tagged feature with an untagged scenario' do
+    write_file 'features/a.feature', <<-FEATURE
+@tag
+Feature: A
+  Scenario: A1
+    Then a1
+    FEATURE
+
+    write_file 'features/steps/a.rb', <<-STEPS
+class Spinach::Features::A < Spinach::FeatureSteps
+  step 'a1' do; end
+end
+    STEPS
+  end
+
+  step 'I have an untagged feature with a tagged scenario' do
+    write_file 'features/b.feature', <<-FEATURE
+Feature: B
+  @tag
+  Scenario: B1
+    Then b1
+    FEATURE
+
+    write_file 'features/steps/b.rb', <<-STEPS
+class Spinach::Features::B < Spinach::FeatureSteps
+  step 'b1' do; end
+end
+    STEPS
+  end
+
+  step "I don't specify tags" do
+    run_spinach
+  end
+
+  step 'I specify a tag the features and scenarios are tagged with' do
+    run_spinach({append: "--tags @tag"})
+  end
+
+  step 'I exclude a tag the features and scenarios are tagged with' do
+    run_spinach({append: "--tags ~@tag"})
+  end
+
+  step 'all the feature hooks should have run' do
+    @stdout.must_match("Feature: A")
+    @stdout.must_match("Feature: B")
+  end
+
+  step 'no feature hooks should have run' do
+    @stdout.wont_match("Feature: A")
+    @stdout.wont_match("Feature: B")
+  end
+end

lib/spinach/runner.rb

@@ -151,7 +151,7 @@ def fail_fast?
     end
 
     def features_to_run
-      unordered_features = filenames.map do |filename|
+      unordered_features = filenames.reduce([]) do |features, filename|
         file, *lines = filename.split(":") # little more complex than just a "filename"
 
         # FIXME Feature should be instantiated directly, not through an unrelated class method
@@ -160,7 +160,9 @@ def features_to_run
 
         feature.lines_to_run = lines if lines.any?
 
-        feature
+        features << feature if TagsMatcher.match_feature(feature)
+
+        features
       end
 
       orderer.order(unordered_features)

lib/spinach/tags_matcher.rb

@@ -6,9 +6,9 @@ module TagsMatcher
     class << self
 
       # Matches an array of tags (e.g. of a scenario) against the tags present
-      # in Spinach' runtime options.
+      # in Spinach's runtime options.
       #
-      # Spinach' tag option is an array which consists of (possibly) multiple
+      # Spinach's tag option is an array which consists of (possibly) multiple
       # arrays containing tags provided by the user running the features and
       # scenarios. Each of these arrays is considered a tag group.
       #
@@ -23,6 +23,15 @@ def match(tags)
         }
       end
 
+      # Matches the tags of a feature (and its scenarios) against the tags present
+      # in Spinach's runtime options.
+      #
+      # A feature matches when, for any of its scenarios, the combination of the
+      # feature's tags and that scenario's tags match the configured tags.
+      def match_feature(feature)
+        feature.scenarios.any? { |scenario| match(feature.tags + scenario.tags) }
+      end
+
       private
 
       def tag_groups

test/spinach/runner_test.rb

@@ -77,15 +77,21 @@
   describe '#run' do
     before(:each) do
       @feature_runner = stub
+      @feature_runner.stubs(:run).returns(true)
+
       filenames.each do |filename|
-        Spinach::Parser.stubs(:open_file).with(filename).returns parser = stub
-        parser.stubs(:parse).returns feature = Spinach::Feature.new
+        parser = stub
+        Spinach::Parser.stubs(:open_file).with(filename).returns(parser)
+
+        feature = Spinach::Feature.new
+        feature.scenarios << Spinach::Scenario.new(feature)
+        parser.stubs(:parse).returns(feature)
+
         Spinach::Runner::FeatureRunner.stubs(:new).
           with(feature, anything).
           returns(@feature_runner)
       end
 
-      @feature_runner.stubs(:run).returns(true)
       runner.stubs(required_files: [])
     end
 
@@ -133,12 +139,18 @@
       let(:runner) { Spinach::Runner.new(filenames) }
 
       before(:each) do
+        parser = stub
+        Spinach::Parser.stubs(:open_file).with(filename).returns(parser)
+
+        @feature = Spinach::Feature.new
+        @feature.scenarios << Spinach::Scenario.new(@feature)
+        parser.stubs(:parse).returns(@feature)
+
         @feature_runner = stub
-        Spinach::Parser.stubs(:open_file).with(filename).returns parser = stub
-        parser.stubs(:parse).returns @feature = Spinach::Feature.new
         Spinach::Runner::FeatureRunner.stubs(:new).
           with(@feature, anything).
           returns(@feature_runner)
+
         runner.stubs(required_files: [])
       end
 
@@ -174,8 +186,13 @@
 
       before(:each) do
         filenames.each_with_index do |filename, i|
-          Spinach::Parser.stubs(:open_file).with(filename).returns parser = stub
-          parser.stubs(:parse).returns feature = Spinach::Feature.new
+          parser = stub
+          Spinach::Parser.stubs(:open_file).with(filename).returns(parser)
+
+          feature = Spinach::Feature.new
+          feature.scenarios << Spinach::Scenario.new(feature)
+          parser.stubs(:parse).returns(feature)
+
           Spinach::Runner::FeatureRunner.stubs(:new).
             with(feature, anything).
             returns(feature_runners[i])