Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Make --fail-fast configurable #2065

Merged
merged 6 commits into from
Nov 1, 2015
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
25 changes: 21 additions & 4 deletions features/command_line/fail_fast.feature
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,29 @@ Feature: `--fail-fast` option
Use the `--fail-fast` option to tell RSpec to stop running the test suite on
the first failed test.

You may also specify `--no-fail-fast` to turn it off (default behaviour).
You may add a parameter to tell RSpec to stop running the test suite after N
failed tests, for example: `--fail-fast=3`.
Copy link
Member

Choose a reason for hiding this comment

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

If you make my suggested change you'll have to drop these = of course, but you should also mention the default is 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I won't be able to concatenate any other command if I drop the =. Maybe I am doing something wrong but if I drop it, it will see anything following fail-fast as a parameter for fail-fast itself, therefore if I add another command it will error.

For example we have --bisect that requires an =.
What do you suggest?


You can also specify `--no-fail-fast` to turn it off (default behaviour).

Background:
Given a file named "fail_fast_spec.rb" with:
"""ruby
RSpec.describe "fail fast" do
it "passing test" do; end
it "failing test" do
it "1st failing test" do
fail
end
it "2nd failing test" do
fail
end
it "3rd failing test" do
fail
end
it "this should not be run" do; end
it "4th failing test" do
fail
end
it "passing test" do; end
end
"""

Expand All @@ -22,6 +34,11 @@ Feature: `--fail-fast` option
Then the output should contain ".F"
Then the output should not contain ".F."

Scenario: Using `--fail-fast=3`
When I run `rspec . --fail-fast=3`
Then the output should contain ".FFF"
Then the output should not contain ".FFFF."

Scenario: Using `--no-fail-fast`
When I run `rspec . --no-fail-fast`
Then the output should contain ".F."
Then the output should contain ".FFFF."
54 changes: 42 additions & 12 deletions features/configuration/fail_fast.feature
Original file line number Diff line number Diff line change
@@ -1,19 +1,13 @@
Feature: fail fast

Use the `fail_fast` option to tell RSpec to abort the run on first failure:
Use the `fail_fast` option to tell RSpec to abort the run on after N failures:

```ruby
RSpec.configure { |c| c.fail_fast = true }
```

Background:
Scenario: `fail_fast` with no failures (runs all examples)
Given a file named "spec/spec_helper.rb" with:
"""ruby
Copy link
Member

Choose a reason for hiding this comment

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

This is documentation and should be restored (although you can add a comment that true means 1)

Copy link
Member

Choose a reason for hiding this comment

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

Do we still have an example of just a bare --fail-fast?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RSpec.configure {|c| c.fail_fast = true}
RSpec.configure {|c| c.fail_fast = 1}
"""

Scenario: `fail_fast` with no failures (runs all examples)
Given a file named "spec/example_spec.rb" with:
And a file named "spec/example_spec.rb" with:
"""ruby
RSpec.describe "something" do
it "passes" do
Expand All @@ -27,7 +21,11 @@ Feature: fail fast
Then the examples should all pass

Scenario: `fail_fast` with first example failing (only runs the one example)
Given a file named "spec/example_spec.rb" with:
Given a file named "spec/spec_helper.rb" with:
"""ruby
RSpec.configure {|c| c.fail_fast = 1}
"""
And a file named "spec/example_spec.rb" with:
"""ruby
require "spec_helper"
RSpec.describe "something" do
Expand All @@ -43,7 +41,11 @@ Feature: fail fast
Then the output should contain "1 example, 1 failure"

Scenario: `fail_fast` with multiple files, second example failing (only runs the first two examples)
Given a file named "spec/example_1_spec.rb" with:
Given a file named "spec/spec_helper.rb" with:
"""ruby
RSpec.configure {|c| c.fail_fast = 1}
"""
And a file named "spec/example_1_spec.rb" with:
"""ruby
require "spec_helper"
RSpec.describe "something" do
Expand Down Expand Up @@ -77,3 +79,31 @@ Feature: fail fast
"""
When I run `rspec spec`
Then the output should contain "2 examples, 1 failure"


Scenario: `fail_fast 2` with 1st and 3rd examples failing (only runs the first 3 examples)
Given a file named "spec/spec_helper.rb" with:
"""ruby
RSpec.configure {|c| c.fail_fast = 2}
"""
And a file named "spec/example_spec.rb" with:
"""ruby
require "spec_helper"
RSpec.describe "something" do
it "fails once" do
fail
end

it "passes once" do
end

it "fails twice" do
fail
end

it "passes" do
end
end
"""
When I run `rspec spec/example_spec.rb -fd`
Then the output should contain "3 examples, 2 failures"
3 changes: 2 additions & 1 deletion lib/rspec/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@ def only_failures_but_not_configured?
end

# @macro add_setting
# Clean up and exit after the first failure (default: `false`).
# If specified, indicates the number of failures required before cleaning
# up and exit (default: `false`).
add_setting :fail_fast

# @macro add_setting
Expand Down
4 changes: 3 additions & 1 deletion lib/rspec/core/example_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,9 @@ def self.run_examples(reporter)
instance = new(example.inspect_output)
set_ivars(instance, before_context_ivars)
succeeded = example.run(instance, reporter)
RSpec.world.wants_to_quit = true if fail_fast? && !succeeded
if !succeeded && fail_fast? && reporter.fail_fast_limit_met?
RSpec.world.wants_to_quit = true
end
Copy link
Member

Choose a reason for hiding this comment

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

It seems odd to do this:

if some_condition
  do_something if some_other_condition
end

...rather than:

if some_condition && some_other_condtion
  do_something
end

Mind refactoring to the latter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, you're completely right. Will do!

Copy link
Member

Choose a reason for hiding this comment

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

I'd still like to see the two if statements here combined into one. Nesting them seems messier and unnecessary to me.

succeeded
end.all?
end
Expand Down
15 changes: 13 additions & 2 deletions lib/rspec/core/option_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,18 @@ def parser(options)
bisect_and_exit(argument)
end

parser.on('--[no-]fail-fast', 'Abort the run on first failure.') do |value|
parser.on('--[no-]fail-fast[=COUNT]', 'Abort the run after a certain number of failures (1 by default).') do |argument|
Copy link
Member

Choose a reason for hiding this comment

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

I think the = in this string is what forces the use of = on the cli, did you try dropping it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added the "=" intentionally because if I drop it, the parser will believe that any other following string is a parameter.

ie
--fail-fast -profile

won't work because it will believe that -profile is the parameter for --fail-fast. Am I doing something wrong?

Copy link
Member

Choose a reason for hiding this comment

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

Well it should be --profile to be considered seperate but if that's a typo the square brackets are what should make it optional

Copy link
Member

Choose a reason for hiding this comment

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

Changing this string to '--[no-]fail-fast[ COUNT]' drops the need for the =, I double checked.

Copy link
Member

Choose a reason for hiding this comment

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

@JonRowe when I added the bisect feature and wanted to support --bisect verbose I couldn't get that to work and had to add the = in the middle. I couldn't figure out why. Do you want to take a look at that?

if argument == true
value = 1
elsif argument == false || argument == 0
value = false
else
begin
value = Integer(argument)
rescue ArgumentError
RSpec.warning "Non integer specified as fail count."
end
end
set_fail_fast(options, value)
end

Expand Down Expand Up @@ -176,7 +187,7 @@ def parser(options)
parser.on("--next-failure", "Apply `--only-failures` and abort after one failure.",
" (Equivalent to `--only-failures --fail-fast --order defined`)") do
configure_only_failures(options)
set_fail_fast(options, true)
set_fail_fast(options, 1)
options[:order] ||= 'defined'
end

Expand Down
10 changes: 10 additions & 0 deletions lib/rspec/core/reporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,16 @@ def abort_with(msg, exit_status)
exit!(exit_status)
end

# @private
def fail_fast_limit_met?
failures_required <= @failed_examples.size
end

# @private
def failures_required
@configuration.fail_fast == true ? 1 : @configuration.fail_fast
end

private

def close
Expand Down
62 changes: 52 additions & 10 deletions spec/rspec/core/example_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1342,34 +1342,76 @@ def extract_execution_results(group)
end

describe "#run" do
let(:reporter) { double("reporter").as_null_object }

context "with fail_fast? => true" do
context "with fail_fast and failures_required == 1" do
let(:group) do
group = RSpec.describe
allow(group).to receive(:fail_fast?) { true }
group
end
let(:config) { Configuration.new }
let(:reporter) do
the_reporter = Reporter.new config
allow(the_reporter).to receive(:failures_required) { 1 }
the_reporter
end

it "does not run examples after the failed example" do
examples_run = []
self.group.example('example 1') { examples_run << self }
self.group.example('example 2') { examples_run << self; fail; }
self.group.example('example 3') { examples_run << self }
group().example('example 1') { examples_run << self }
group().example('example 2') { examples_run << self; fail; }
group().example('example 3') { examples_run << self }

self.group.run
group().run(reporter)

expect(examples_run.length).to eq(2)
end

it "sets RSpec.world.wants_to_quit flag if encountering an exception in before(:all)" do
self.group.before(:all) { raise "error in before all" }
self.group.example("equality") { expect(1).to eq(2) }
expect(self.group.run).to be_falsey
group().before(:all) { raise "error in before all" }
group().example("equality") { expect(1).to eq(2) }
expect(group().run).to be_falsey
expect(RSpec.world.wants_to_quit).to be_truthy
end
end

context "with fail_fast and failures_required = 3" do
let(:group) do
group = RSpec.describe
allow(group).to receive(:fail_fast?) { true }
group
end
let(:config) { Configuration.new }

let(:reporter) do
the_reporter = Reporter.new config
allow(the_reporter).to receive(:failures_required) { 3 }
the_reporter
end

it "does not run examples after 3 failed examples" do
examples_run = []
group().example('example 1') { examples_run << self }
group().example('example 2') { examples_run << self; fail; }
group().example('example 3') { examples_run << self; fail; }
group().example('example 4') { examples_run << self; fail; }
group().example('example 5') { examples_run << self }

group().run(reporter)

expect(examples_run.length).to eq(4)
end

it "sets RSpec.world.wants_to_quit flag if encountering an exception in before(:all)" do
group().before(:all) { raise "error in before all" }
group().example("equality") { expect(1).to eq(2) }
expect(group().run).to be_falsey
expect(RSpec.world.wants_to_quit).to be_truthy
end
end

let(:reporter) { double("reporter").as_null_object }

context "with RSpec.world.wants_to_quit=true" do
let(:group) { RSpec.describe }

Expand All @@ -1379,7 +1421,7 @@ def extract_execution_results(group)

it "returns without starting the group" do
expect(reporter).not_to receive(:example_group_started)
self.group.run(reporter)
group().run(reporter)
end
end

Expand Down
9 changes: 9 additions & 0 deletions spec/rspec/core/option_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,15 @@ def generate_help_text
end
end

describe '--fail-fast' do
it 'warns when a non-integer is specified as fail count' do
expect(::Kernel).to receive(:warn) do |message|
expect(message).to match "Non integer specified as fail count"
end
Parser.parse(%w[--fail-fast=three])
end
end

describe '--warning' do
around do |ex|
verbose = $VERBOSE
Expand Down
17 changes: 17 additions & 0 deletions spec/rspec/core/reporter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -279,5 +279,22 @@ module RSpec::Core
reporter.finish
end
end

describe "#failures_required" do
it "returns 1 when RSpec.configuration.fail_fast == true" do
config.fail_fast = true
expect(reporter.failures_required).to eq 1
end

it "returns 1 when RSpec.configuration.fail_fast == 1" do
config.fail_fast = 1
expect(reporter.failures_required).to eq 1
end

it "returns RSpec.configuration.fail_fast when RSpec.configuration.fail_fast > 1" do
config.fail_fast = 3
expect(reporter.failures_required).to eq 3
end
end
end
end