-
Notifications
You must be signed in to change notification settings - Fork 0
ROX-32966: Add disabling label for the konflux retests #81
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
base: main
Are you sure you want to change the base?
Conversation
| github.event.check_run.conclusion == 'failure' && | ||
| startsWith(github.event.check_run.name, 'Red Hat Konflux') | ||
| startsWith(github.event.check_run.name, 'Red Hat Konflux') && | ||
| !contains(github.event.check_run.pull_requests[0].labels.*.name, 'disable-konflux-auto-retest') |
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.
we know that there is a least one PR associated with the check (see check above). My understanding is that the Konflux check might have multiple PRs only when the same commit is used in those PRs. I assume it's quite rare and unique scenario so I believe [0] is correct instead of * (we don't want to disable if at least one PR has label)
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.
Above you have github.event.check_run.pull_requests[0] != null. How does it evaluate when there are no pull requests? MintMaker does this when we configure automerge: branch, i.e. in https://github.com/stackrox/operator-index/blob/master/.github/renovate.json5#L51
Does it evaluate as "false" or does it crash GHA with sort-of a NullPointerException or is the result of github.event.check_run.pull_requests[0] equal to undefined and the expression github.event.check_run.pull_requests[0] != null weirdly evaluates to true?
Wouldn't it be more robust to check length(github.event.check_run.pull_requests) == 1 if such thing is possible?
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.
My bad, I didn't reflect the last changes. So the latest approach (which you have already tested in the https://github.com/stackrox/test-gh-actions/pull/240) is to pull actual label value directly from GH API instead of relying on event data. Your previous testing proofed that event data (github.event.check_run.pull_requests[0].labels.*.name) might be outdated. So I've updated the PR and also changed null check to github.event.check_run.pull_requests.length > 0
msugakov
left a comment
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.
Regarding
No description provided.
Is there any way you can test this before merging to see if it really works?
I planned to test it with this PR, but looks like konflux is not starting for that repo ATM. https://github.com/stackrox/test-gh-actions/pull/236. Looks like this testing Konflux app was deleted. I will add a new one |
msugakov
left a comment
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.
Given now it passes testing, I've a couple of things to clarify.
| if: github.event_name == 'pull_request' | ||
| if: | | ||
| github.event_name == 'pull_request' && | ||
| !contains(github.event.pull_request.labels.*.name, 'disable-konflux-auto-retest') |
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.
Is that right and you don't want to clean /retest comments when the label is present?
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.
I've changed my mind and now bot's retest command will be removed regardless disabling label
| github.event.check_run.conclusion == 'failure' && | ||
| startsWith(github.event.check_run.name, 'Red Hat Konflux') | ||
| startsWith(github.event.check_run.name, 'Red Hat Konflux') && | ||
| !contains(github.event.check_run.pull_requests[0].labels.*.name, 'disable-konflux-auto-retest') |
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.
Above you have github.event.check_run.pull_requests[0] != null. How does it evaluate when there are no pull requests? MintMaker does this when we configure automerge: branch, i.e. in https://github.com/stackrox/operator-index/blob/master/.github/renovate.json5#L51
Does it evaluate as "false" or does it crash GHA with sort-of a NullPointerException or is the result of github.event.check_run.pull_requests[0] equal to undefined and the expression github.event.check_run.pull_requests[0] != null weirdly evaluates to true?
Wouldn't it be more robust to check length(github.event.check_run.pull_requests) == 1 if such thing is possible?
Testing: https://github.com/stackrox/test-gh-actions/pull/240