-
-
Notifications
You must be signed in to change notification settings - Fork 753
Make --fail-fast configurable #2065
Changes from all commits
6caef8b
6b26f87
c6819f0
55f2e23
2c1cd3d
37cdf42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still have an example of just a bare There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, you're completely right. Will do! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd still like to see the two |
||
succeeded | ||
end.all? | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 won't work because it will believe that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well it should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing this string to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @JonRowe when I added the bisect feature and wanted to support |
||
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 | ||
|
||
|
@@ -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 | ||
|
||
|
There was a problem hiding this comment.
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 is1
.There was a problem hiding this comment.
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 followingfail-fast
as a parameter forfail-fast
itself, therefore if I add another command it will error.For example we have
--bisect
that requires an=
.What do you suggest?