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/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 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?