-
Notifications
You must be signed in to change notification settings - Fork 54
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
Ensure Azure/cli action fail when executed on a non-Linux-based OS #123
Conversation
@@ -16,8 +16,7 @@ export async function main() { | |||
const CONTAINER_NAME = `MICROSOFT_AZURE_CLI_${getCurrentTime()}_CONTAINER`; | |||
try { | |||
if (process.env.RUNNER_OS != 'Linux') { | |||
core.setFailed('Please use Linux based OS as a runner.'); |
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.
Why doesn't core.setFailed
cause failure?
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.
The description of core.setFailed
is "Sets the action status to failed. When the action exits it will be with an exit code of 1.". See https://docs.github.com/en/actions/creating-actions/setting-exit-codes-for-actions.
However, in the previous code, it returned after setFailed.
Lines 18 to 20 in 1828f1c
if (process.env.RUNNER_OS != 'Linux') { | |
core.setFailed('Please use Linux based OS as a runner.'); | |
return; |
This would cause
main()
didn't catch the error.Lines 77 to 80 in 1828f1c
catch (error) { | |
core.error(error); | |
throw error; | |
} |
Then,
main()
exited with 0.Lines 4 to 10 in 1828f1c
main() | |
.then(() => { | |
process.exit(0) | |
}) | |
.catch((err: Error) => { | |
setFailed(err.message); | |
process.exit(1); |
This is why
core.setFailed
doesn't cause failure in previous code.
@@ -16,8 +16,7 @@ export async function main() { | |||
const CONTAINER_NAME = `MICROSOFT_AZURE_CLI_${getCurrentTime()}_CONTAINER`; | |||
try { | |||
if (process.env.RUNNER_OS != 'Linux') { | |||
core.setFailed('Please use Linux based OS as a runner.'); |
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.
Description
Azure/cli should terminate with a failure on a non-Linux-based OS; however, it currently doesn't exit with code 1 but merely outputs an error message.
Former:
Expected: