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

Cucumber --retry option status 1 with strict mode causes --retry option to not be useful #1160

Closed
streetlogics opened this issue Jul 17, 2017 · 8 comments · Fixed by #1169
Closed
Milestone

Comments

@streetlogics
Copy link

streetlogics commented Jul 17, 2017

@brasmusson - re:
cucumber/cucumber-ruby-core#141 Tying this to --strict mode is overloading what --strict mode means and how developers are going to want to use the --retry option.

Personally speaking I am trying to tie this in with CI because I don't want to have to rebuild 30+ minutes of feature files for 1 scenario that happened to randomly fail. The only way I see that happening is by turning off strict mode since CI relies on a 0 or 1 status code to report success/failure. However, I also don't want someone to be able to put pending or worse yet forget to define a step in one of their new scenarios and have CI report a "pass" because --strict mode was disabled, which is the whole reason I enabled --strict mode in CI to begin with.

Since --retry isn't a default option, if I pass --retry and --strict, that means I know there's a chance something may need to be retried but I want --strict enforcement of rules around undefined or pending tests. Backing up to a higher level, since --retry isn't a default option, if I wanted a single "failure" to fail my whole test suite and report a non-zero status code, I would just leave off the --retry option all together since 1 fail is the same as 1 fail and 1 pass retry in the way it's currently designed.

If the current functionality is absolutely desired (although I honestly can't think of a case where I would want a non-zero exit code if retries were successful), maybe we could use a new option instead, something like --retry-strict

@mattwynne @garie @garside @gondalez @danascheider @rishi-freshbooks - would love any thoughts on this as well. Was just trying to get this latest code updated in my CI and couldn't figure out why it wasn't passing until I realized I put --strict in our cucumber.yml.

references:
#1121
#1044
cucumber/cucumber-ruby-core#141

@brasmusson
Copy link
Contributor

Note, there is an ongoing diskussion if undefined steps always should give non-zero exit code cucumber/cucumber-js#867.

--retry and --strict is for The people that don't want to allow flickering scenarios, but still want to distinguish flickering from true failures, useful information when the scenario skall be fixed.

@brasmusson
Copy link
Contributor

Was just trying to get this latest code updated in my CI and couldn't figure out why it wasn't passing

As of #1158, the reson(s) why the exit code is not zero is listed in the console, the scenarios listed under "Failing scenarios" etc, are exactly the scenarios that (each) made the exit code non-zero.

@garside
Copy link

garside commented Jul 19, 2017

@brasmusson do you have a rough estimate of when this behavior will make it out to the ruby gem?

@streetlogics
Copy link
Author

useful information when the scenario skall be fixed

A "flaky" scenario isn't necessarily going to be fixed. Most often a scenario needs to be repeated because there was a driver issue with Capybara trying to communicate with the browser. I've seen this in practice many times, especially with very large test suites. The point of --retry (and the legacy "rerun profile" feature) is that I know a few tests will occasionally fail because of timings / browser issues, but what I want to know is if a test failed 3 successive attempts in a row before I spend the time troubleshooting and fixing and scenario.

Note, there is an ongoing diskussion if undefined steps always should give non-zero exit code cucumber/cucumber-js#867.

This would make sense, but to me doesn't justify overloading the current functionality of --strict to make flaky tests reported as failures. I'd prefer to see a --retry-strict or potentially an option to define with strict which types of tests should be reported as failures, ex: --strict='failure,flaky,undefined,pending' (this would be the default if no values were passed), then if someone like me doesn't want flaky reported, I could just pass failure,undefined,pending

@brasmusson
Copy link
Contributor

I find hard to believe that there is not even one shop in the whole world with the attitude "flickering scenarios should be identified, analyzed, and fixed" - I am well aware that not all shops have that attitude.

In my mind has the --strict option

@brasmusson
Copy link
Contributor

Sorry, on vacation - has only smartphone available

@brasmusson brasmusson reopened this Jul 19, 2017
@brasmusson
Copy link
Contributor

In my mind has the meaning of the --strict option not changed, it has always had the meaning "be strict about the the result types that either can be handled strict or not so strict". Yes, the list of those result types has been extended, but that is natural as a product is evolved.

Every time a new result type is introduced, it has to be decided in which of the categories "always fails the build", "never fails the build", "can be handled strict or not so strict" it shall betong to. I just added "flaky" to the "strict or not so strict" category, and we are about to add "ambiguous" to the "always fails the build" category.

Twice have I in comments on issues and PRs about the retry functionality expressed the opinion that the --strict option should affect the exit code with flaky scenarios, and I have not seen any questionings or opposing opinions.

Yes I agree that it should be possible to control the strictness of each result type in the "strict or not so strict" category individually.

@brasmusson brasmusson added this to the 3.0 milestone Jul 25, 2017
@lock
Copy link

lock bot commented Oct 24, 2018

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.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants