From 76e98f340823add9ae54ba8e11e5cdde90f912fa Mon Sep 17 00:00:00 2001 From: Jorg Jenni Date: Sat, 5 Sep 2026 16:08:07 +0100 Subject: [PATCH 1/2] Show which attempts the messages report as retried The TestCaseFinished message of a scenario claims it will be retried based on --retry alone, ignoring the --retry-total circuit breaker, and is off by one. The HTML formatter relies on that flag to pick the final attempt of every scenario, so it drops or duplicates scenarios. The new scenario lists every attempt reported by the message formatter with its willBeRetried flag, for a run that hits --retry-total. See #1905. --- features/docs/cli/retry_failing_tests.feature | 14 +++++++++++ .../lib/step_definitions/message_steps.rb | 24 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/features/docs/cli/retry_failing_tests.feature b/features/docs/cli/retry_failing_tests.feature index 531a124a4c..75118768a3 100644 --- a/features/docs/cli/retry_failing_tests.feature +++ b/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 | diff --git a/features/lib/step_definitions/message_steps.rb b/features/lib/step_definitions/message_steps.rb index 40dec1927e..bf79c094fa 100644 --- a/features/lib/step_definitions/message_steps.rb +++ b/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 From 990ed6cf75048c56c9f3ba39ff10cd5a7d0fd36c Mon Sep 17 00:00:00 2001 From: Jorg Jenni Date: Sat, 5 Sep 2026 14:21:34 +0100 Subject: [PATCH 2/2] Report will_be_retried from the retry filter's decision The runner reported will_be_retried from --retry alone, ignoring the --retry-total circuit breaker of the retry filter. Once that limit was hit, the TestCaseFinished message of a failed scenario still claimed it would be retried, so the HTML formatter dropped the scenario from the report. The value was also off by one, as the runner was given the number of retries where it expected the number of attempts. A RetryPolicy, shared by the retry filter and the runner, now owns the decision. The filter asks it and records what it did, the runner asks it when reporting the outcome of a test case. Fixes #1905. --- CHANGELOG.md | 5 +++++ Gemfile | 3 +++ lib/cucumber/configuration.rb | 5 +++++ lib/cucumber/filters/retry.rb | 29 +++++++---------------------- lib/cucumber/retry_policy.rb | 27 +++++++++++++++++++++++++++ lib/cucumber/runtime.rb | 2 +- 6 files changed, 48 insertions(+), 23 deletions(-) create mode 100644 lib/cucumber/retry_policy.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 0301355858..d7f3274c80 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/Gemfile b/Gemfile index 5020a79819..ca7f0934c3 100644 --- a/Gemfile +++ b/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' diff --git a/lib/cucumber/configuration.rb b/lib/cucumber/configuration.rb index 2df363799f..1ab38b9567 100644 --- a/lib/cucumber/configuration.rb +++ b/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 diff --git a/lib/cucumber/filters/retry.rb b/lib/cucumber/filters/retry.rb index 313be68614..c45c76a578 100644 --- a/lib/cucumber/filters/retry.rb +++ b/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 diff --git a/lib/cucumber/retry_policy.rb b/lib/cucumber/retry_policy.rb new file mode 100644 index 0000000000..ac01236ed5 --- /dev/null +++ b/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 diff --git a/lib/cucumber/runtime.rb b/lib/cucumber/runtime.rb index 8a8507c89c..8c68229e92 100644 --- a/lib/cucumber/runtime.rb +++ b/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?