Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New option '--retry-total' as a circuit breaker for option '--retry' #1669

Merged
merged 5 commits into from
Dec 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Please visit [cucumber/CONTRIBUTING.md](https://github.com/cucumber/cucumber/blo

## [Unreleased]
### Added
* Add option `--retry-total` ([PR#](https://github.com/cucumber/cucumber-ruby/pull/1669))

### Changed

Expand Down
28 changes: 28 additions & 0 deletions features/docs/cli/retry_failing_tests.feature
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,31 @@ Feature: Retry failing tests

3 scenarios (2 flaky, 1 passed)
"""

Scenario: Retry each test but suspend retrying after 2 failing tests, so later tests are not retried
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 summary`
Then it should fail with:
"""
5 scenarios (4 failed, 1 passed)
"""
And it should fail with:
"""
Fails-forever-1
Fails-forever-1 ✗
Fails-forever-1 ✗

Fails-forever-2
Fails-forever-2 ✗
Fails-forever-2 ✗

Fails-once feature
Fails-once ✗

Fails-twice feature
Fails-twice ✗

Solid
Solid ✓
"""
16 changes: 8 additions & 8 deletions features/lib/step_definitions/scenarios_and_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,26 @@ def snake_case(name)
Given('a scenario {string} that passes') do |name|
create_feature(name) do
create_scenario(name) do
' Given it passes'
" Given it passes in #{name}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these changes here? Just trying to think how we can diet down the complexity?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make my new scenario retry_failing_tests.feature:99 pass I only would need to fix scenarios_and_steps.rb:43 and not all.

The problem was that the steps could only be used once per scenario, my fix makes it possible to use such a step several times in one scenario.

I therefore could only fix scenarios_and_steps.rb:43 and not all but I thought it's better if all steps behave and are implemented the same.

Is that ok or do you want me to only fix the step that caused the problem?

Copy link
Contributor Author

@Enceradeira Enceradeira Oct 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luke-hill I tried to connect to cucumberbdd.slack.com but it seems I need an account there. Can you send me invite to [email protected] please?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an update and from what others have said it's likely we'll just implement this here as we've done for now. I'll take a look and review this in the coming week.

You should be able to sign up on your own to the slack channel

end
end

write_file(
"features/step_definitions/#{name}_steps.rb",
step_definition('/^it passes$/', 'expect(true).to be true')
step_definition("/^it passes in #{name}$/", 'expect(true).to be true')
)
end

Given('a scenario {string} that fails') do |name|
create_feature(name) do
create_scenario(name) do
' Given it fails'
" Given it fails in #{name}"
end
end

write_file(
"features/step_definitions/#{name}_steps.rb",
step_definition('/^it fails$/', 'expect(false).to be true')
step_definition("/^it fails in #{name}$/", 'expect(false).to be true')
)
end

Expand All @@ -58,14 +58,14 @@ def snake_case(name)

create_feature("#{full_name} feature") do
create_scenario(full_name) do
' Given it fails once, then passes'
" Given it fails once, then passes in #{full_name}"
end
end

write_file(
"features/step_definitions/#{name}_steps.rb",
step_definition(
'/^it fails once, then passes$/',
"/^it fails once, then passes in #{full_name}$/",
[
"$#{name} += 1",
"expect($#{name}).to be > 1"
Expand All @@ -84,14 +84,14 @@ def snake_case(name)

create_feature("#{full_name} feature") do
create_scenario(full_name) do
' Given it fails twice, then passes'
" Given it fails twice, then passes in #{full_name}"
end
end

write_file(
"features/step_definitions/#{name}_steps.rb",
step_definition(
'/^it fails twice, then passes$/',
"/^it fails twice, then passes in #{full_name}$/",
[
"$#{name} ||= 0",
"$#{name} += 1",
Expand Down
17 changes: 14 additions & 3 deletions lib/cucumber/cli/options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,12 @@ class Options
NO_PROFILE_LONG_FLAG = '--no-profile'.freeze
FAIL_FAST_FLAG = '--fail-fast'.freeze
RETRY_FLAG = '--retry'.freeze
RETRY_TOTAL_FLAG = '--retry-total'.freeze
OPTIONS_WITH_ARGS = [
'-r', '--require', '--i18n-keywords', '-f', '--format', '-o',
'--out', '-t', '--tags', '-n', '--name', '-e', '--exclude',
PROFILE_SHORT_FLAG, PROFILE_LONG_FLAG, RETRY_FLAG, '-l',
'--lines', '--port', '-I', '--snippet-type'
PROFILE_SHORT_FLAG, PROFILE_LONG_FLAG, RETRY_FLAG, RETRY_TOTAL_FLAG,
'-l', '--lines', '--port', '-I', '--snippet-type'
].freeze
ORDER_TYPES = %w[defined random].freeze
TAG_LIMIT_MATCHER = /(?<tag_name>@\w+):(?<limit>\d+)/x
Expand Down Expand Up @@ -108,6 +109,7 @@ def parse!(args) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
opts.on('-j DIR', '--jars DIR', 'Load all the jars under DIR') { |jars| load_jars(jars) } if Cucumber::JRUBY

opts.on("#{RETRY_FLAG} ATTEMPTS", *retry_msg) { |v| set_option :retry, v.to_i }
opts.on("#{RETRY_TOTAL_FLAG} TESTS", *retry_total_msg) { |v| set_option :retry_total, v.to_i }
opts.on('--i18n-languages', *i18n_languages_msg) { list_languages_and_exit }
opts.on('--i18n-keywords LANG', *i18n_keywords_msg) { |lang| language lang }
opts.on(FAIL_FAST_FLAG, 'Exit immediately following the first failing scenario') { set_option :fail_fast }
Expand Down Expand Up @@ -271,6 +273,13 @@ def retry_msg
['Specify the number of times to retry failing tests (default: 0)']
end

def retry_total_msg
[
'The total number of failing test after which retrying of tests is suspended.',
'Example: --retry-total 10 -> Will stop retrying tests after 10 failing tests.'
]
end

def name_msg
[
'Only execute the feature elements which match part of the given name.',
Expand Down Expand Up @@ -543,6 +552,7 @@ def reverse_merge(other_options) # rubocop:disable Metrics/AbcSize
end

@options[:retry] = other_options[:retry] if @options[:retry].zero?
@options[:retry_total] = other_options[:retry_total] if @options[:retry_total].infinite?

self
end
Expand Down Expand Up @@ -616,7 +626,8 @@ def default_options
snippets: true,
source: true,
duration: true,
retry: 0
retry: 0,
retry_total: Float::INFINITY
}
end
end
Expand Down
7 changes: 6 additions & 1 deletion lib/cucumber/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ def retry_attempts
@options[:retry]
end

def retry_total_tests
@options[:retry_total]
end

def guess?
@options[:guess]
end
Expand Down Expand Up @@ -273,7 +277,8 @@ def default_options
snippets: true,
source: true,
duration: true,
event_bus: Cucumber::Events.make_event_bus
event_bus: Cucumber::Events.make_event_bus,
retry_total: Float::INFINITY
}
end

Expand Down
21 changes: 20 additions & 1 deletion lib/cucumber/filters/retry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +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)
Expand All @@ -21,7 +26,21 @@ def test_case(test_case)
private

def retry_required?(test_case, event)
event.test_case == test_case && event.result.failed? && test_case_counts[test_case] < configuration.retry_attempts
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
true
else
# test failed after max. attempts
@total_permanently_failed += 1
false
end
end

def test_case_counts
Expand Down
34 changes: 34 additions & 0 deletions spec/cucumber/cli/options_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,26 @@ def with_env(name, value)
end
end
end

context '--retry-total TESTS' do
context '--retry-total option not defined on the command line' do
it 'uses the --retry-total option from the profile' do
given_cucumber_yml_defined_as('foo' => '--retry-total 2')
options.parse!(%w[-p foo])

expect(options[:retry_total]).to be 2
end
end

context '--retry-total option defined on the command line' do
it 'ignores the --retry-total option from the profile' do
given_cucumber_yml_defined_as('foo' => '--retry-total 2')
options.parse!(%w[--retry-total 1 -p foo])

expect(options[:retry_total]).to be 1
end
end
end
end

context '-P or --no-profile' do
Expand Down Expand Up @@ -441,6 +461,20 @@ def with_env(name, value)
end
end

context '--retry-total TESTS' do
it 'is INFINITY by default' do
after_parsing('') do
expect(options[:retry_total]).to eql Float::INFINITY
end
end

it 'sets the options[:retry_total] value' do
after_parsing('--retry 3 --retry-total 10') do
expect(options[:retry_total]).to eql 10
end
end
end

it 'assigns any extra arguments as paths to features' do
after_parsing('-f pretty my_feature.feature my_other_features') do
expect(options[:paths]).to eq ['my_feature.feature', 'my_other_features']
Expand Down
57 changes: 51 additions & 6 deletions spec/cucumber/filters/retry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
include Cucumber::Core
include Cucumber::Events

let(:configuration) { Cucumber::Configuration.new(retry: 2) }
let(:configuration) { Cucumber::Configuration.new(retry: 2, retry_total: retry_total) }
let(:retry_total) { Float::INFINITY }
let(:id) { double }
let(:name) { double }
let(:location) { double }
Expand Down Expand Up @@ -55,12 +56,32 @@
context 'consistently failing test case' do
let(:result) { Cucumber::Core::Test::Result::Failed.new(0, StandardError.new) }

it 'describes the test case the specified number of times' do
expect(receiver).to receive(:test_case) { |test_case|
configuration.notify :test_case_finished, test_case, result
}.exactly(3).times
shared_examples 'retries the test case the specified number of times' do |expected_nr_of_times|
it 'describes the test case the specified number of times' do
expect(receiver).to receive(:test_case) { |test_case|
configuration.notify :test_case_finished, test_case, result
}.exactly(expected_nr_of_times).times

filter.test_case(test_case)
filter.test_case(test_case)
end
end

context 'when retry_total infinit' do
let(:retry_total) { Float::INFINITY }

include_examples 'retries the test case the specified number of times', 3
end

context 'when retry_total 1' do
let(:retry_total) { 1 }

include_examples 'retries the test case the specified number of times', 3
end

context 'when retry_total 0' do
let(:retry_total) { 0 }

include_examples 'retries the test case the specified number of times', 1
end
end

Expand Down Expand Up @@ -100,4 +121,28 @@
end
end
end

context 'too many failing tests' do
let(:retry_total) { 1 }
let(:always_failing_test_case1) do
Cucumber::Core::Test::Case.new(id, name, [double('test steps')], 'test.rb:1', tags, language)
end
let(:always_failing_test_case2) do
Cucumber::Core::Test::Case.new(id, name, [double('test steps')], 'test.rb:9', tags, language)
end
let(:fail_result) { Cucumber::Core::Test::Result::Failed.new(0, StandardError.new) }

it 'stops retrying tests' do
expect(receiver).to receive(:test_case).with(always_failing_test_case1) { |test_case|
configuration.notify :test_case_finished, test_case, fail_result
}.ordered.exactly(3).times

expect(receiver).to receive(:test_case).with(always_failing_test_case2) { |test_case|
configuration.notify :test_case_finished, test_case, fail_result
}.ordered.exactly(1).times

filter.test_case(always_failing_test_case1)
filter.test_case(always_failing_test_case2)
end
end
end