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

error code 0 on successful --retry #1121

Closed
wants to merge 4 commits into from

Conversation

gondalez
Copy link

Summary

A basic working version of --retry based on work by @streetlogics, in the time I had available.

The logging could use some work and it would be nice to get feedback from the maintainers.

I figured I would put this here in case it helps someone else out and maybe it can go part-way towards a fix for #1044 :)

Details

I cherry picked the commits master...streetlogics:issue/retry-success-exit-code-0-1044 on top of master.
See #1044 (comment).

Motivation and Context

I tried using @streetlogics branch but I got an ArgumentError when a feature is retried.
This seems to have been fixed by 31968ae, that was not yet in the remote branch.

How Has This Been Tested?

I tested quickly locally and can confirm the error code is now 0 if a step succeeds on subsequent retry.

@danascheider
Copy link
Contributor

Thanks @gondalez! If you can rebase this off of master I'll help figure out what's breaking in jruby.

Brad Wedell added 2 commits July 11, 2017 11:56
-  Updated configuration to count the base number of cases being run
-  Updated failure? method to use cases.total_passed and total_cases to determine if the run was successful
@gondalez gondalez force-pushed the retry-return-code-fix branch from 87b8662 to 8174b73 Compare July 11, 2017 04:00
@gondalez
Copy link
Author

@danascheider no probs; rebase is done and the jruby build passed this time 🎉

@@ -228,6 +228,12 @@ def legacy_formatter?(factory)
def failure?
if @configuration.wip?
summary_report.test_cases.total_passed > 0
elsif @configuration.retry_attempts > 0
unless summary_report.test_cases.total_passed != @configuration.total_cases
Copy link
Contributor

Choose a reason for hiding this comment

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

Golfing the code:

  • an unless with != could become an if with a ==

Would that read better?

Copy link
Author

Choose a reason for hiding this comment

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

@olleolleolle 👍 fixed :)

@brasmusson
Copy link
Contributor

As I commented on the original PR, I think the exit code should be non-zero in --strict mode also when all scenarios are passed on rerun, that the summary should list the number of scenarios passed on rerun in the new flakey category, and that the exit code should be based on the summary (zero in --strict mode if all scenarios are passed or skipped, zero in non-strict mode if no scenarios are failed or ambiguous). I can't see that an implementation adding Configuration#total_cases is the implementation we want.

@gondalez
Copy link
Author

@brasmusson That makes sense. I like the idea of a new category.
I'm happy to try my hand at an implementation over the coming weeks. Can you give me any helpful pointers?

@brasmusson
Copy link
Contributor

Superseded by cucumber/cucumber-ruby-core#141 and #1159.

@lock
Copy link

lock bot commented Oct 25, 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 25, 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 this pull request may close these issues.

4 participants