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

Fixes trigger_all_tests.yml triggering on every issue comment #4114

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

JayShortway
Copy link
Member

Bug

The trigger_all_tests.yml workflow triggers for every issue comment. It doesn't check that the comment body matches "@RCGitBot please test".

You can see this if you check the latest workflow runs, and click a few of the comments in that list.

For example, this run was triggered by this comment:

This does add noise to the overall schemes in Xcode, let me know if that feels like a bit too much

Also, these workflow runs all fail.

Fix

Checking rhysd.github.io/actionlint for the trigger_all_tests.yml file at 68ac257 (HEAD of main at the time of writing) shows this error:

if: condition "${{ github.event.issue.pull_request }} &&\ngithub.event.comment.body == '@RCGitBot please test' &&\ngithub.repository == 'RevenueCat/purchases-ios'\n" is always evaluated to true because extra characters are around ${{ }}

The fix is to remove the curly braces altogether. Omitting the curly braces is okay in if conditions, according to the GitHub Actions docs:

The exception to this rule is when you are using expressions in an if clause, where, optionally, you can usually omit ${{ and }}.

(I tried moving the closing curly braces (}}) to the end of the last condition, but that didn't please the linter.)

@JayShortway JayShortway added the ci label Jul 25, 2024
@JayShortway JayShortway requested a review from a team July 25, 2024 09:53
@JayShortway JayShortway self-assigned this Jul 25, 2024
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

@JayShortway JayShortway merged commit 8d7443c into main Jul 25, 2024
4 checks passed
@JayShortway JayShortway deleted the fixes-trigger-all-tests-trigger branch July 25, 2024 10:04
@aboedo
Copy link
Member

aboedo commented Jul 25, 2024

+100, thanks for taking care of it!

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

Successfully merging this pull request may close these issues.

4 participants