-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[Core Bug Fix] Fixed runtime exit status for ambiguous scenarios #1342
[Core Bug Fix] Fixed runtime exit status for ambiguous scenarios #1342
Conversation
… option 'strict' is true.
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.
Hey thanks for reporting and trying to fix this. Much appreciated!
Can you make it so ambiguousScenarios
is treated the same as failedScenarios
with respect to isStrict
?
@@ -94,7 +94,7 @@ public void setEventPublisher(EventPublisher publisher) { | |||
|
|||
public byte exitStatus(boolean isStrict) { | |||
byte result = 0x0; | |||
if (!failedScenarios.isEmpty() || (isStrict && (!pendingScenarios.isEmpty() || !undefinedScenarios.isEmpty()))) { | |||
if (!failedScenarios.isEmpty() || (isStrict && (!pendingScenarios.isEmpty() || !undefinedScenarios.isEmpty() || !ambiguousScenarios.isEmpty()))) { |
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.
Looking at printNonZeroResultScenarios I think ambiguousScenarios
should be treated the same as failedScenarios
and always return a non zero exit status.
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.
@brasmusson This section feels like it's duplicating some functionality we have elsewhere already. I think we can improve this by using Result.SEVERITY from #1323 to find the worst the results and use Result.isOk(strict) to determine the exit code status.
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.
Yes, it sure feels as duplication. Somehow, both here and in printNonZeroResultScenarios
, Result.isOk
should be used (instead of duplicating its logic).
Hi @mpkorstanje ambiguousScenarios are now treated as failedScenarios. |
Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾 In return for this generous offer we hope you will:
On behalf of the Cucumber core team, |
Thanks! |
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. |
Summary
Fixed an issue where a successful runtime exit status (0x0) is returned for ambiguous scenarios when runtime option 'strict' is set to true.
Details
When using the Cucumber CLI and an AmbiguousStepDefinitionsException is raised, the runtime exit status is 0x0 even though runtime option 'strict' is set to true.
The runtime exit status should be 0x1 when isStrict
Motivation and Context
How Has This Been Tested?
Yes - 2 new tests added.
Screenshots (if appropriate):
Types of changes
Checklist: