Report will_be_retried from the retry filter's decision

CHANGELOG.md

@@ -26,6 +26,11 @@ of the test run ([#1889](https://github.com/cucumber/cucumber-ruby/pull/1889) [l
 - All events now inherit from the new `Cucumber::Core::Event::Base` class ([luke-hill](https://github.com/luke-hill))
 - Updated the `MessageBuilder` to not create the messages now created by cucumber-ruby-core ([#1882](https://github.com/cucumber/cucumber-ruby/pull/1882) [brasmusson](https://github.com/brasmusson))
 
+### Fixed
+- The `testCaseFinished` message now states `willBeRetried` correctly when `--retry-total` stops further retries, or when
+`--retry` is used at all (it was off by one), so the HTML formatter no longer drops or duplicates scenarios
+([#1905](https://github.com/cucumber/cucumber-ruby/issues/1905) [Enceradeira](https://github.com/Enceradeira))
+
 ### Removed
 - Removed the concept of `strict` from cucumber-ruby ([luke-hill](https://github.com/luke-hill))
   - This was a long-standing feature that was used to determine if a test run should fail if there were any undefined, pending or flaky steps.

Gemfile

@@ -4,6 +4,9 @@ source 'https://rubygems.org'
 
 gemspec
 
+# Temporary, until cucumber-core 20 is released with cucumber/cucumber-ruby-core#345
+gem 'cucumber-core', git: 'https://github.com/cucumber/cucumber-ruby-core.git', branch: 'fix/cucumber-ruby-1905-will-be-retried'
+
 # To hack on Cucumber together with any of these libraries, uncomment the line below:
 # gem 'cucumber-core', path: '../cucumber-ruby-core'
 # gem 'cucumber-cucumber-expressions', path: '../cucumber-expressions/ruby'

features/docs/cli/retry_failing_tests.feature

@@ -84,3 +84,17 @@ Feature: Retry failing tests
       Solid
         Solid ✓
       """
+
+  Scenario: Report which attempts are retried, so a test report shows the final attempt of every scenario
+    Given a scenario "Fails-forever-1" that fails
+    And a scenario "Fails-forever-2" that fails
+    When I run `cucumber -q --retry 1 --retry-total 2 --format message`
+    Then the messages report these attempts of the scenarios:
+      | scenario        | attempt | willBeRetried |
+      | Fails-forever-1 | 1       | true          |
+      | Fails-forever-1 | 2       | false         |
+      | Fails-forever-2 | 1       | true          |
+      | Fails-forever-2 | 2       | false         |
+      | Fails-once      | 1       | false         |
+      | Fails-twice     | 1       | false         |
+      | Solid           | 1       | false         |

features/lib/step_definitions/message_steps.rb

@@ -36,3 +36,27 @@
 
   expect(message_contents).to include(key => boolean)
 end
+
+Then('the messages report these attempts of the scenarios:') do |expected_attempts|
+  scenario_names = {}
+  pickle_ids = {}
+  attempts = []
+
+  command_line.stdout(format: :lines).each do |line|
+    case JSON.parse(line, symbolize_names: true)
+    in { pickle: { id:, name: } }
+      scenario_names[id] = name
+    in { testCase: { id:, pickleId: pickle_id } }
+      pickle_ids[id] = pickle_id
+    in { testCaseStarted: { id:, testCaseId: test_case_id, attempt: } }
+      attempts << { 'id' => id, 'scenario' => scenario_names.fetch(pickle_ids.fetch(test_case_id)), 'attempt' => attempt.to_s }
+    in { testCaseFinished: { testCaseStartedId: started_id, willBeRetried: will_be_retried } }
+      expect(started_id).to eq(attempts.last['id'])
+      attempts.last['willBeRetried'] = will_be_retried.to_s
+    else
+      nil
+    end
+  end
+
+  expect(attempts.map { |attempt| attempt.except('id') }).to eq(expected_attempts.hashes)
+end

lib/cucumber/configuration.rb

@@ -6,6 +6,7 @@
 require 'cucumber/messages'
 require 'cucumber/core/event_bus'
 require 'cucumber/core/test/result'
+require 'cucumber/retry_policy'
 require 'forwardable'
 require 'cucumber'
 
@@ -280,6 +281,10 @@ def id_generator
       @id_generator ||= Cucumber::Messages::Helpers::IdGenerator::UUID.new
     end
 
+    def retry_policy
+      @retry_policy ||= RetryPolicy.new(retry_attempts, retry_total_tests)
+    end
+
     def test_run_started_id
       @test_run_started_id ||= id_generator.new_id
     end

lib/cucumber/filters/retry.rb

@@ -7,16 +7,11 @@
 module Cucumber
   module Filters
     class Retry < Core::Filter.new(:configuration)
-      def initialize(*_args)
-        super
-        @total_permanently_failed = 0
-      end
-
       def test_case(test_case)
         configuration.on_event(:test_case_finished) do |event|
-          next unless retry_required?(test_case, event)
+          next unless event.test_case == test_case
+          next unless retry?(test_case, event.result)
 
-          test_case_counts[test_case] += 1
           test_case.describe_to(receiver)
         end
 
@@ -25,27 +20,17 @@ def test_case(test_case)
 
       private
 
-      def retry_required?(test_case, event)
-        return false unless event.test_case == test_case
-
-        return false unless event.result.failed?
-
-        return false if @total_permanently_failed >= configuration.retry_total_tests
-
-        retry_required = test_case_counts[test_case] < configuration.retry_attempts
-        if retry_required
-          # retry test
+      def retry?(test_case, result)
+        if retry_policy.will_be_retried?(test_case, result)
+          retry_policy.record_retry(test_case)
           true
         else
-          # test failed after max. attempts
-          @total_permanently_failed += 1
+          retry_policy.record_permanent_failure if result.failed?
           false
         end
       end
 
-      def test_case_counts
-        @test_case_counts ||= Hash.new { |h, k| h[k] = 0 }
-      end
+      def retry_policy = configuration.retry_policy
     end
   end
 end

lib/cucumber/retry_policy.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module Cucumber
+  # Decides whether a failed test case is run again, honouring both the `--retry` and the `--retry-total` options.
+  #
+  # The retry filter asks it and then records what it did, the test runner asks it when it reports the outcome of a
+  # test case. Both have to get the same answer, so #will_be_retried? never changes the state of the policy.
+  class RetryPolicy
+    def initialize(max_retries, max_permanent_failures)
+      @max_retries = max_retries
+      @max_permanent_failures = max_permanent_failures
+      @retries = Hash.new(0)
+      @permanent_failures = 0
+    end
+
+    def will_be_retried?(test_case, result)
+      return false unless result.failed?
+      return false if @permanent_failures >= @max_permanent_failures
+
+      @retries[test_case] < @max_retries
+    end
+
+    def record_retry(test_case) = @retries[test_case] += 1
+
+    def record_permanent_failure = @permanent_failures += 1
+  end
+end

lib/cucumber/runtime.rb

@@ -48,7 +48,7 @@ def run!
       fire_install_plugin_hook
       create_formatters
 
-      receiver = Test::Runner.new(@configuration.event_bus, @configuration.id_generator, Cucumber::Formatter::BacktraceFilter, @configuration.retry_attempts)
+      receiver = Test::Runner.new(@configuration.event_bus, @configuration.id_generator, Cucumber::Formatter::BacktraceFilter, @configuration.retry_policy)
       compile features, receiver, filters, @configuration.event_bus
       fire_after_all_hook unless dry_run?
       @configuration.notify :test_run_finished, !failure?