Add fail fast switch

[danascheider]

Here are the changes I've made for the fail-fast switch in #879, @mattwynne. I ended up just setting Cucumber.wants_to_quit in the FailFast report.

The problem I haven't figured out how to solve is, when I run the fail_fast features, the instance of Cucumber running the test also gets its @wants_to_quit set to true, with the result that running the passing fail_fast.feature invariably causes the entire thing to exit (with status 2, no less). I'll keep hammering away at this, so just let me know if any solutions spring readily to mind.

Thanks!

[brasmusson]

The annotation @spawn needs to be added to this scenario so that the cucumber instance started by the When step is not executed in the same process as the cucumber instance that rake has started. This problem reminds me of "singletons considered harmful" and a quote from one of the pages return from a search of that topic: "everything that can be done with Singletons, can be done better with dependency injection". @mattwynne Cucumber.wants_to_quit really?

[mattwynne]

Yeah totally agreed about the use of that global state @brasmusson - it's a legacy we're living with for now but I'd love to see a PR that gets rid of it. Should be much easier now that the behaviour is all centred in the QuitFilter.

[danascheider]

Thank you, @brasmusson. I went ahead and added that tag.

[danascheider]

Just an update - I'm working on fixing this but am unable to reproduce the failures locally (and it seems they are not consistent in Travis either). It looks like something I changed in runtime is interfering with some of the WireSupport functionality. This hasn't fallen off my radar, don't worry!

[mattwynne]

The wire support tests can be a bit flakey, so it might be nothing to do with you. I've been very busy lately but one of us should be able to take a good look at this soon. Thanks @danascheider!

[mattwynne]

Merged, this should be released in 2.1, hopefully later today.

Thanks again @danascheider.

[mattwynne]

I don't understand this line. Is it a typo? If I remove the @configuration.strict? bit all the tests pass.

[brasmusson]

That because there are no test with a test case with an undefined step, then the test case result is "not ok" only in strict mode, and only then shall the execution be stopped.

[mattwynne]

Oh silly me! I'm old-school and I couldn't read the Ruby without parentheses.

[danascheider]

That's OK, I just remembered recently that you didn't have to use them, so I stopped using them just for the sake of novelty, really. I meant to use them in this code.

[mattwynne]

Having given this some focus today as I merged it in, I thought of a couple of new ways to implement this feature. @tooky @brasmusson @danascheider I'm interested in your thoughts here.

1. We could now use the new Events and listen for the AfterTestStep event. But that would still mean we'd need to use the global Cucumber.wants_to_quit to tell the runner to stop. It's just a bit neater than using a formatter.

2. We could use a Filter to add (only in fail-fast mode) what are effectively AfterStep hooks that check for failure of the preceding step and raise a new type of exception. We can update the runner to respond to that type of exception by going into a new state that skips everything from then on.

I like (2) better. WDYT?

[mattwynne]

I've gone ahead and implemented (1) #couldnthelpmyself.

[mattwynne]

But I'd still like to consider doing (2)

[danascheider]

@mattwynne I actually considered implementing something like (2) to begin with. I think it's a good idea because that way the exit status could be 1 instead of 2, which has been bugging me a little about the way I wrote it. I did it the way I did mainly because I didn't want to block on doing it the most elegant way possible - figured it's better to get something that works and then tweak.

[brasmusson]

@mattwynne You cannot use actual AfterStep hooks to do (2), since also hooks can fail. After hooks does not work either, since Around hook can still fail after the After hooks have executed. I think the idea of using exceptions to tell the runner that a test case has failed, is to complex - I mean the runner (I am talking about Cucumber::Core::Test::Runner) knows perfectly well if a test case has failed, it is actually passing the after_test_case message that the formatters will receive, so if the runner should be used to stop running test case, then it could be configured to a fail-fast mode before starting the execution of test cases.

I do not see the problem with the quit filter (except that is uses the global Cucumber.wants_to_quit), but why not use the event bus not only to enable formatters and plugins to receive event, but also to enable them to send events like wants_to_quit?

[mattwynne]

Question for @danascheider - do you think it makes more sense to stop as soon as one test case fails, or should we continue but skip all the remaining ones (so we still get the same total number of scenarios)?

[danascheider]

@mattwynne - I actually like the idea of skipping all the remaining ones. I often wish RSpec did that with its --fail-fast option.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.