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

Incorrect logic for if for success() || failure() #367

Open
andrewvaughan opened this issue Oct 19, 2023 · 3 comments
Open

Incorrect logic for if for success() || failure() #367

andrewvaughan opened this issue Oct 19, 2023 · 3 comments

Comments

@andrewvaughan
Copy link

if: condition "${{ success() }} || ${{ failure() }}" is always evaluated to true because extra characters are around ${{ }} [if-cond]
    |
184 |         if: ${{ success() }} || ${{ failure() }}
    |             ^~~

I don't believe this is correct - if a workflow is canceled, this will not evaluate to true.

@andrewvaughan
Copy link
Author

Note that this causes all megalinter configurations to fail currently.

@ChristopherHX
Copy link

I don't believe that your statement is correct, just tested this scenario and the job run after a the workflow has been cancelled. Even while it should be skipped.

if: ${{ success() }} || ${{ failure() }}

Also known as

if: ${{ format('{0} || {1}', success(), failure()) }}

Is always a non empty string, non empty strings are truthy.

Due to the usage of success and failure you don't get success() && (yourcond) applied.

There is no evidence that GitHub tried to fix this

Good ways to write the intended meaning is to use one of

# ok no implicit format to merge multiple scalar expressions
if: ${{ success() || failure() }}
# ok no expression notation, implicit expression
if: success() || failure()

@ChristopherHX
Copy link

Here is the relevant source code of the actions/runner template engine:

https://github.com/actions/runner/blob/9e3e57ff90c089641a3a5833c2211841da1a37f8/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateReader.cs#L589-L619'

Single vs. multiple Segments of ${{ }} are relevant for this GitHub Actions Bug.

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

No branches or pull requests

2 participants