action #130940
closedcoordination #58184: [saga][epic][use case] full version control awareness within openQA
coordination #48641: [epic] Trigger openQA tests in pull requests of any product github pull request
coordination #130850: [epic] Use openqa-clone-custom-git-refspec to parse github description+comments and trigger openQA tests as part of CI
Trigger openQA tests mentioned in github comments as part of automatic testing as well - trusted group "tests-maintainer" only size:M
Description
Motivation¶
Follow-up to #130934. After having openQA CI integration which reads github pull request description and triggers according openQA clone jobs github comments created or updated should be considered the same.
Acceptance criteria¶
- AC1: openQA CI integration in any existing test distribution github project automatically runs openQA tests based on links to existing openQA jobs in the github comments
- AC2: Ensure openQA documentation covers that also github comments are parsed and how to use that
- AC3: An update to the PR code does not retrigger any openQA jobs mentioned in comments
- AC4: Comments created/edited by anyone not in the group "tests-maintainer" are ignored
Suggestions¶
- Wait for #130934
- Read what was done originally in #63712 and the according pull request to openQA https://github.com/os-autoinst/openQA/pull/2618 and also #130934
- Check if openqa-clone-custom-git-refspec with special comments also covers github comments or only the initial description
- Extend the existing approach where necessary
- Ensure that the same process is triggered also on comment updates
- Only look at comments created/edited by members of the group "tests-maintainer" for security reasons
Updated by okurz 9 months ago
- Subject changed from Trigger openQA tests mentioned in github comments as part of CI as well to Trigger openQA tests mentioned in github comments as part of automatic testing as well
- Description updated (diff)
- Assignee set to okurz
we couldn't agree on this yet during estimation and I will follow-up with the team in more detail about the general idea if it makes sense like that or not at all.
Updated by okurz 8 months ago
- Subject changed from Trigger openQA tests mentioned in github comments as part of automatic testing as well to Trigger openQA tests mentioned in github comments as part of automatic testing as well - trusted group "tests-maintainer" only
- Description updated (diff)
- Assignee deleted (
okurz) - Target version changed from Tools - Next to Ready
Updated by jbaier_cz 8 months ago
- Subject changed from Trigger openQA tests mentioned in github comments as part of automatic testing as well - trusted group "tests-maintainer" only to Trigger openQA tests mentioned in github comments as part of automatic testing as well - trusted group "tests-maintainer" only size:M
Updated by openqa_review 7 months ago
- Due date set to 2024-05-29
Setting due date based on mean cycle time of SUSE QE Tools
Updated by mkittler 7 months ago
I tried creating a workflow that triggers when comments are created. The triggering generally works but there are a few problems:
- The workflow isn't shown as part of the related PR (e.g. https://github.com/Martchus/os-autoinst-distri-opensuse/pull/3) but only under the "Actions" tab (e.g. https://github.com/Martchus/os-autoinst-distri-opensuse/actions). So we probably need to use the statuses API to get some in-progress/result checkmarks within the PR.
- The event meta-data doesn't contain the PRs ref so we need to query that from our action.
Those are the changes I have so far for our GitHub action: https://github.com/os-autoinst/scripts/compare/master...Martchus:scripts:clone-on-comment
Updated by mkittler 7 months ago
- Status changed from In Progress to Feedback
PR for the scripts repo covering 1 and 2: https://github.com/os-autoinst/scripts/pull/318
Updated by mkittler 7 months ago
The scripts PR was merged so now the next step would be to merge https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19305.
The workflow is not triggered before merging the changes so I guess we cannot easily test this on the main repo before merging. But it worked when I tested it on my fork (where I added the changes already on the master branch). In case the token GitHub provides to workflows by default is not sufficient I will have to create a new token just for that - but it won't be a big deal as I have already tested this on my fork.
Updated by mkittler 7 months ago
The PR was merged as well and I created https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19371 to supply a token with enough permissions. I hope that'll be the last step.
Updated by mkittler 7 months ago
Looks like it generally works now, e.g. jobs were cloned for https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19305#issuecomment-2127177246.
It failed to update the status and to clone the ref:
Unable to update status on GitHub via 'https://api.github.com/repos/Martchus/os-autoinst-distri-opensuse/statuses/acf5d851c6a6eead3605a324a14f3651117d54b0'.
{"message":"Not Found","documentation_url":"https://docs.github.com/rest/commits/statuses#create-a-commit-status"}
[2024-05-23T15:45:30.188302Z] [warn] [pid:74634] !!! main: Unable to clone Git repository 'https://github.com/https://github.com/Martchus/os-autoinst-distri-opensuse.git#acf5d851c6a6eead3605a324a14f3651117d54b0' specified via CASEDIR (see log for details) at /usr/lib/os-autoinst/OpenQA/Isotovideo/Utils.pm line 164.
Maybe the URL for updating the status needed to point to the repository under the original organization (os-autoinst) and not the fork (Martchus). And the URL of CASEDIR is just wrong in a very obvious way.
But there's another caveat: This will now spawn a GitHub action on every new/updated comment in PRs. This means setting up a container and doing some initial checks - even if no job is to be cloned. That's maybe a bit wasteful. Not sure whether this can be tweaked via the if:-condition in the workflow YAML.
Updated by livdywan 7 months ago
mkittler wrote in #note-15:
Looks like it generally works now, e.g. jobs were cloned for https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19305#issuecomment-2127177246.
Really cool. One thing I noticed, there is no way to see the PR from the job itself or at least not easily.
This is not in the AC so just a suggestion, but it would be nice to see the PR in the settings for example.
But there's another caveat: This will now spawn a GitHub action on every new/updated comment in PRs. This means setting up a container and doing some initial checks - even if no job is to be cloned. That's maybe a bit wasteful. Not sure whether this can be tweaked via the
if:-condition in the workflow YAML.
Maybe https://github.com/orgs/community/discussions/25389 (although I'd say it is fine as-is, too, if this is not straightforward to do)
Updated by mkittler 7 months ago
I was actually able to find the contains()-syntax myself (in the official documentation). It works so I created https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19379 to use it.
Updated by mkittler 7 months ago
- Status changed from In Progress to Feedback
Really cool. One thing I noticed, there is no way to see the PR from the job itself or at least not easily.
I added another commit to https://github.com/os-autoinst/scripts/pull/324 for that. It will add a job setting pointing to the specific PR comment the job was cloned by. I also created https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19380 to add a back reference to the relevant PR for jobs cloned via PR descriptions.
Updated by mkittler 7 months ago ยท Edited
All PRs merged, looks good so far: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19305#issuecomment-2133002967
EDIT: Unfortunately reporting the status back didn't work when I tested it on the open PR https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19378#issuecomment-2133222287.
The log contains the following error:
Unable to update status on GitHub via 'https://api.github.com/repos/os-autoinst/os-autoinst-distri-opensuse/statuses/32731e00569b74a04f4269c101da9d181cef8513'.
{"message":"Not Found","documentation_url":"https://docs.github.com/rest/commits/statuses#create-a-commit-status"}
I'm not sure why that's the case. The URL looks correct and the same worked on my fork.
Updated by mkittler 7 months ago
The long build numbers are disturbing the dashboard. I think this should be handled better in general by the dashboard so I created https://github.com/os-autoinst/openQA/pull/5663 to limit the size of build labels on the dashboard.
Updated by okurz 7 months ago
mkittler wrote in #note-21:
The long build numbers are disturbing the dashboard. I think this should be handled better in general by the dashboard so I created https://github.com/os-autoinst/openQA/pull/5663 to limit the size of build labels on the dashboard.
That only happens though if builds are created as part of job groups which shouldn't happen for those jobs triggered as part of verification jobs. Maybe you just need to make those jobs groupless?
Updated by mkittler 7 months ago
The jobs are intentionally in a specific job group.
Showing the status also works now after making the bot account an owner within the os-autoinst organization, see the green checkmark on https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19378. The back-reference also works, see CLONED_BY on https://openqa.opensuse.org/tests/4243617#settings.
PRs for documentation: