Feature/rerun formatter

.rubocop.yml

@@ -33,6 +33,18 @@ Layout/TrailingWhitespace:
     - spec/cucumber/formatter/pretty_spec.rb
     - spec/cucumber/formatter/progress_spec.rb
 
+# This file has a couple of very long complex methods which are hard to break down further meaningfully
+#   -> They also will only ever run one single branch (Due to the fact messages only are ever of one type at a time)
+Metrics/CyclomaticComplexity:
+  Exclude:
+    - lib/cucumber/repository.rb
+
+# This file has a couple of very long complex methods which are hard to break down further meaningfully
+#   -> They also will only ever run one single branch (Due to the fact messages only are ever of one type at a time)
+Metrics/PerceivedComplexity:
+  Exclude:
+    - lib/cucumber/repository.rb
+
 # Rubocop doesn't like method names in other languages but as Cucumber supports multiple languages, this cop needs to be disabled
 Naming/AsciiIdentifiers:
   Enabled: false

lib/cucumber/formatter/html.rb

@@ -7,13 +7,10 @@
 module Cucumber
   module Formatter
     class HTML < MessageBuilder
-      include Io
-
       def initialize(config)
         @io = ensure_io(config.out_stream, config.error_stream)
         @html_formatter = Cucumber::HTMLFormatter::Formatter.new(@io)
         @html_formatter.write_pre_message
-
         super(config)
       end
 

lib/cucumber/formatter/message_builder.rb

@@ -15,6 +15,7 @@ module Cucumber
   module Formatter
     class MessageBuilder
       include Cucumber::Messages::Helpers::TimeConversion
+      include Io
 
       def initialize(config)
         @config = config
@@ -25,6 +26,8 @@ def initialize(config)
         @step_definitions_by_test_step = Query::StepDefinitionsByTestStep.new(config)
         @test_case_started_by_test_case = Query::TestCaseStartedByTestCase.new(config)
         @test_run_started = Query::TestRunStarted.new(config)
+        @repository = Cucumber::Repository.new
+        @query = Cucumber::Query.new(@repository)
 
         config.on_event :envelope, &method(:on_envelope)
         config.on_event :gherkin_source_read, &method(:on_gherkin_source_read)
@@ -41,9 +44,6 @@ def initialize(config)
         @current_test_run_started_id = nil
         @current_test_case_started_id = nil
         @current_test_step_id = nil
-
-        @repository = Cucumber::Repository.new
-        @query = Cucumber::Query.new(@repository)
       end
 
       def attach(src, media_type, filename)
@@ -87,6 +87,8 @@ def on_gherkin_source_read(event)
       end
 
       def on_test_case_ready(event)
+        # TODO: Switch this over to using the Repo Query object -> `test_step_by_id`
+        # TODO: The finder in query is `find_test_step_by` (Using +TestStepStarted+ message)
         event.test_case.test_steps.each do |step|
           @test_case_by_step_id[step.id] = event.test_case
         end
@@ -100,6 +102,9 @@ def on_test_case_ready(event)
           )
         )
 
+        # TODO: Once we're comfortable switching this over. Call @repository.update(message) alongside output_envelope
+        # however this may not be necessary as output_envelope may/should already be doing this?
+
         output_envelope(message)
       end
 
@@ -108,6 +113,8 @@ def test_step_to_message(step)
 
         Cucumber::Messages::TestStep.new(
           id: step.id,
+          # TODO: This "fake query" is only used once. It can likely be replace by `find_pickle_step_by` which
+          # takes a +TestStep+ message from the repo directly.
           pickle_step_id: @pickle_step_by_test_step.pickle_step_id(step),
           step_definition_ids: @step_definitions_by_test_step.step_definition_ids(step),
           step_match_arguments_lists: step_match_arguments_lists(step)
@@ -152,6 +159,7 @@ def parameter_type_name(step_match_argument)
       end
 
       def on_test_run_started(*)
+        # TODO: Switch this over to using the Query object -> `find_test_run_started`
         @current_test_run_started_id = @test_run_started.id
 
         message = Cucumber::Messages::Envelope.new(

lib/cucumber/formatter/rerun.rb

@@ -1,52 +1,81 @@
 # frozen_string_literal: true
 
 require 'cucumber/formatter/io'
+require 'cucumber/formatter/message_builder'
 
 module Cucumber
   module Formatter
-    class Rerun
-      include Formatter::Io
-
+    class Rerun < MessageBuilder
       def initialize(config)
         @io = ensure_io(config.out_stream, config.error_stream)
-        @config = config
-        @failures = {}
-        config.on_event :test_case_finished do |event|
-          test_case, result = *event.attributes
-          if @config.strict.strict?(:flaky)
-            next if result.ok?(strict: @config.strict)
-
-            add_to_failures(test_case)
-          else
-            unless @latest_failed_test_case.nil?
-              if @latest_failed_test_case != test_case
-                add_to_failures(@latest_failed_test_case)
-                @latest_failed_test_case = nil
-              elsif result.ok?(strict: @config.strict)
-                @latest_failed_test_case = nil
-              end
-            end
-            @latest_failed_test_case = test_case unless result.ok?(strict: @config.strict)
+        super(config)
+      end
+
+      def output_envelope(envelope)
+        @repository.update(envelope)
+        finish_report if envelope.test_run_finished
+      end
+
+      private
+
+      def finish_report
+        @query.find_all_test_case_started.each do |test_case|
+          status = @query.find_most_severe_test_step_result_by(test_case).status
+          # RULE: Don't log test cases without a pickle (Unsure what these could be?)
+          pickle = @query.find_pickle_by(test_case)
+          next if pickle.nil?
+
+          # RULE: (Configuration specific)
+          #   -> If the test case has already been logged (And so we're retrying), we remove prior references of failures
+          if passing?(test_case) && !rerun_flaky_tests?
+            uri_and_location_hash[pickle.uri].delete(pickle.location.line)
+            next
           end
+
+          # RULE: (Configuration specific - to be amended once CCK conformance is finalised)
+          #   -> If the strict configuration permits the result - handle it accordingly
+          next if status == 'UNDEFINED' && [email protected]?(:undefined)
+          next if status == 'PENDING' && [email protected]?(:pending)
+
+          # RULE: Passing test cases are not considered failures (Don't log these)
+          next if passing?(test_case)
+
+          # RULE: Skipped test cases are not considered failures (on their own, don't log these)
+          next if skipped?(test_case)
+
+          # RULE: Before logging a failure, ensure we are not on a retried test case (Don't log a retry multiple times)
+          next if test_case.attempt > 1
+
+          # Log the failure if every other skip rule has not been met, and the failure has not already been logged
+          uri_and_location_hash[pickle.uri] << pickle.location.line
         end
-        config.on_event :test_run_finished do
-          add_to_failures(@latest_failed_test_case) unless @latest_failed_test_case.nil?
-          next if @failures.empty?
 
-          @io.print file_failures.join("\n")
+        # Generate the final output from the logged failures to be formatted in the io output
+        @io.print(failure_array.join("\n"))
+      end
+
+      def failure_array
+        uri_and_location_hash.filter_map do |uri, lines|
+          "#{uri}:#{lines.join(':')}" if lines.any?
         end
       end
 
-      private
+      def uri_and_location_hash
+        @uri_and_location_hash ||= Hash.new { |hash, key| hash[key] = Set.new }
+      end
+
+      def rerun_flaky_tests?
+        @config.strict.strict?(:flaky)
+      end
 
-      def file_failures
-        @failures.map { |file, lines| [file, lines].join(':') }
+      def passing?(test_case_started)
+        most_severe_test_step_result = @query.find_most_severe_test_step_result_by(test_case_started)
+        most_severe_test_step_result.status == Cucumber::Messages::TestStepResultStatus::PASSED
       end
 
-      def add_to_failures(test_case)
-        location = test_case.location
-        @failures[location.file] ||= []
-        @failures[location.file] << location.lines.max unless @failures[location.file].include?(location.lines.max)
+      def skipped?(test_case_started)
+        most_severe_test_step_result = @query.find_most_severe_test_step_result_by(test_case_started)
+        most_severe_test_step_result.status == Cucumber::Messages::TestStepResultStatus::SKIPPED
       end
     end
   end

lib/cucumber/query.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'cucumber/repository'
+require 'cucumber/messages'
 
 # Given one Cucumber Message, find another.
 #
@@ -12,6 +13,9 @@ class Query
     attr_reader :repository
     private :repository
 
+    include Cucumber::Messages::Helpers::TimeConversion
+    include Cucumber::Messages::Helpers::TestStepResultComparator
+
     def initialize(repository)
       @repository = repository
     end
@@ -23,31 +27,15 @@ def initialize(repository)
     #   Missing: findAllUndefinedParameterTypes
 
     # TODO: find****By methods (16/25) Complete
-    #   Missing: findLocationOf (1 variant) - This strictly speaking isn't a findBy but is located within them
     #   Missing: findSuggestionsBy (2 variants)
     #   Missing: findUnambiguousStepDefinitionBy (1 variant)
     #   Missing: findTestStepFinishedAndTestStepBy (1 variant)
-    #   Missing: findMostSevereTestStepResultBy (2 variants)
     #   Missing: findAttachmentsBy (2 variants)
     #   Missing: findTestCaseDurationBy (2 variant)
-    #   Missing: findLineageBy (9 variants!)
+    #   REDUNDANT: findLineageBy (9 variants!)
+    #   REDUNDANT: findLocationOf (1 variant) - This strictly speaking isn't a findBy but is located within them
+    #   To Review: findMostSevereTestStepResultBy (2 variants)
     #   To Review: findTestRunDuration (1 variant) - This strictly speaking isn't a findBy but is located within them
-    #   Complete: findMeta (1 variant)
-    #   Complete: findTestRunHookStartedBy (1 variant)
-    #   Complete: findTestRunHookFinishedBy (1 variant)
-    #   Complete: findPickleStepBy (1 variant)
-    #   Complete: findStepDefinitionsBy (1 variant)
-    #   Complete: findStepBy (1 variant)
-    #   Complete: findTestRunFinished (1 variant)
-    #   Complete: findTestRunStarted (1 variant)
-    #   Fully Complete (2/2): findTestStepsStartedBy (2 variants)
-    #   Fully Complete (2/2): findTestStepBy (2 variants)
-    #   Fully Complete (3/3): findTestCaseStartedBy (3 variants)
-    #   Fully Complete (1/1): findTestCaseFinishedBy (1 variant)
-    #   Fully Complete (4/4): findTestCaseBy (4 variants)
-    #   Fully Complete (5/5): findPickleBy (5 variants)
-    #   Fully Complete (3/3): findHookBy (3 variants)
-    #   Fully Complete (2/2): findTestStepsFinishedBy (2 variants)
 
     def count_test_cases_started
       find_all_test_case_started.length
@@ -121,6 +109,23 @@ def find_meta
       repository.meta
     end
 
+    # This method will be called with 1 of these 2 messages
+    #   [TestCaseStarted || TestCaseFinished]
+    def find_most_severe_test_step_result_by(message)
+      ensure_only_message_types!(message, %i[test_case_started test_case_finished], '#find_most_severe_test_step_result_by')
+
+      if message.is_a?(Cucumber::Messages::TestCaseStarted)
+        find_test_steps_finished_by(message)
+          .map(&:test_step_result)
+          .max_by { |test_step_result| test_step_result_rankings[test_step_result.status] }
+        # Java code: "PREVIOUS".max(comparing(TestStepResult::getStatus, new TestStepResultStatusComparator()));
+      else
+        test_case_started_message = find_test_case_started_by(message)
+        test_case_started_message && find_most_severe_test_step_result_by(test_case_started_message)
+        # Java code: return findTestCaseStartedBy(testCaseFinished).flatMap(this::findMostSevereTestStepResultBy);
+      end
+    end
+
     # This method will be called with 1 of these 5 messages
     #   [TestCase || TestCaseStarted || TestCaseFinished || TestStepStarted || TestStepFinished]
     def find_pickle_by(message)
@@ -235,7 +240,7 @@ def find_test_steps_finished_by(message)
       ensure_only_message_types!(message, %i[test_case_started test_case_finished], '#find_test_steps_finished_by')
 
       if message.is_a?(Cucumber::Messages::TestCaseStarted)
-        test_steps_finished_by_test_case_started_id.fetch(message.id, [])
+        repository.test_steps_finished_by_test_case_started_id.fetch(message.id, [])
       else
         test_case_started_message = find_test_case_started_by(message)
         test_case_started_message.nil? ? [] : find_test_steps_finished_by(test_case_started_message)

lib/cucumber/repository.rb

@@ -12,8 +12,7 @@ class Repository
                 :test_run_hook_started_by_id, :test_run_hook_finished_by_test_run_hook_started_id,
                 :test_step_by_id, :test_steps_started_by_test_case_started_id, :test_steps_finished_by_test_case_started_id
 
-    # TODO: Missing structs (3)
-    #   final Map<Object, Lineage> lineageById = new HashMap<>();
+    # TODO: Missing structs (2)
     #   final Map<String, List<Suggestion>> suggestionsByPickleStepId = new LinkedHashMap<>();
     #   final List<UndefinedParameterType> undefinedParameterTypes = new ArrayList<>();
 

spec/cucumber/formatter/rerun_spec.rb

@@ -36,7 +36,7 @@ module Formatter
           end
 
           described_class.new(config)
-          execute [gherkin], [StandardStepActions.new], config.event_bus
+          execute [gherkin], [StandardStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus
           config.event_bus.test_run_finished
 
           expect(io.string).to eq 'foo.feature:3:6'
@@ -70,7 +70,7 @@ module Formatter
           end
 
           described_class.new(config)
-          execute [foo, bar], [StandardStepActions.new], config.event_bus
+          execute [foo, bar], [StandardStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus
           config.event_bus.test_run_finished
 
           expect(io.string).to eq "foo.feature:3:6\nbar.feature:3"
@@ -88,14 +88,14 @@ module Formatter
           end
 
           described_class.new(config)
-          execute [gherkin], [StandardStepActions.new], config.event_bus
+          execute [gherkin], [StandardStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus
           config.event_bus.test_run_finished
 
-          expect(io.string).to eq ''
+          expect(io.string).to eq('')
         end
       end
 
-      context 'with only a flaky scenarios' do
+      context 'with a flaky scenario' do
         context 'with option --no-strict-flaky' do
           it 'prints nothing' do
             gherkin = gherkin('foo.feature') do
@@ -107,18 +107,19 @@ module Formatter
             end
 
             described_class.new(config)
-            execute [gherkin], [FakeObjects::FlakyStepActions.new], config.event_bus
+            execute [gherkin], [FakeObjects::FlakyStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus
+
             config.event_bus.test_run_finished
 
-            expect(io.string).to eq ''
+            expect(io.string).to eq('')
           end
         end
 
         context 'with option --strict-flaky' do
           let(:config) { Configuration.new(out_stream: io, strict: Core::Test::Result::StrictConfiguration.new([:flaky])) }
 
           it 'prints the location of the flaky scenario' do
-            foo = gherkin('foo.feature') do
+            gherkin = gherkin('foo.feature') do
               feature do
                 scenario do
                   step 'flaky'
@@ -127,14 +128,14 @@ module Formatter
             end
 
             described_class.new(config)
-            execute [foo], [FakeObjects::FlakyStepActions.new], config.event_bus
+            execute [gherkin], [FakeObjects::FlakyStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus
             config.event_bus.test_run_finished
 
-            expect(io.string).to eq 'foo.feature:3'
+            expect(io.string).to eq('foo.feature:3')
           end
 
-          it 'does not include retried failing scenarios more than once' do
-            foo = gherkin('foo.feature') do
+          it 'does not include the same failing scenario more than once' do
+            gherkin = gherkin('foo.feature') do
               feature do
                 scenario do
                   step 'failing'
@@ -143,10 +144,10 @@ module Formatter
             end
 
             described_class.new(config)
-            execute [foo, foo], [StandardStepActions.new], config.event_bus
+            execute [gherkin, gherkin], [StandardStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus
             config.event_bus.test_run_finished
 
-            expect(io.string).to eq 'foo.feature:3'
+            expect(io.string).to eq('foo.feature:3')
           end
         end
       end