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

Handle default return correctly when returnStatus is used #267

Merged
merged 3 commits into from
Aug 18, 2020

Conversation

Alex-Vol-SV
Copy link
Contributor

The original default behavior for the sh step was undefined and fell
through the inaccurate handling of the resturnStdout behavior. This will
result in a success returnStatus when requested instead of a string
return.

The original default behavior for the `sh` step was undefined and fell
through the inaccurate handling of the resturnStdout behavior. This will
result in a success returnStatus when requested instead of a string
return.
Copy link
Contributor

@nre-ableton nre-ableton left a comment

Choose a reason for hiding this comment

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

First, thanks for the PR. 👍 I didn't account for this particular case, because the codepath of the original code that this feature was ported from would raise an exception if no mock output was registered. That was removed in order to avoid a breaking change when merging that PR.

This PR looks fine except for the empty return statement, which I left a line comment about.

…groovy


Make correction to final return for clarity and to avoid unnecessary return trigger from IntelliJ

Co-authored-by: Nik Reiman <[email protected]>
@Alex-Vol-SV
Copy link
Contributor Author

For what it is worth, the original code also returns a multiline string as output when returnStdout=false which failed to pass when returnStatus=true because of the missing condition. Additionally, the default behavior of returnStdout=false and returnStatus=false is not implemented accurately in this block of code as it will still fall through to the multiline string return.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants