-
Notifications
You must be signed in to change notification settings - Fork 6
IOS-9864: Update record-screenshots to also pass the tests #342
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
Conversation
Launch the action by commenting on a PR. Execute the tests after recording the screenshots. Update the latest commit status and thus the PR with the tests result.
Screenshot tests report ✔️ All passing |
name: Execute tests | ||
needs: record-screenshots | ||
uses: ./.github/workflows/ci.yml | ||
secrets: inherit |
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.
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.
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.
Nice
runs-on: self-hosted-novum-mac | ||
steps: | ||
- name: Get branch of PR | ||
uses: xt0rted/pull-request-comment-branch@v2 |
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.
IMHO I would develop it ourselves instead of using third-party repositories
GitHub provides very useful variables, you'll need to process them a bit to get the values you desire:
GITHUB_SHA: The commit SHA that triggered the workflow.
GITHUB_REF: The branch or tag ref that triggered the workflow.
Or if you need to get the base branch of a PR you can just do: ${{ github.event.pull_request.base.ref }}
My concern is that these repositories don't seem to be very maintained. IMHO we could create our own actions in Telefonica/github-actions so everybody could use them
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.
What you reference would not work because the action, when triggered from a issue_comment trigger, runs on the main branch instead of the PR branch.
In the repository of that action it's explained:
The pull request head ref and sha are important because issue_comment workflows run against the repository's default branch (usually master) and not the pull request's branch. With this action you'll be able to pass the ref to [actions/checkout](https://github.com/actions/checkout) and work with the pull request's code.
There is an open discussion in the actions repository about this, but the solutions seem more hacky to me and prone to failing without warning than using an action that works well: actions/checkout#331
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 haven't done much research, if you think it's going to take a long time to do it on our own, okay, but if not, I think we should do it ourselves.
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.
IMHO It is good not being dependent on third party actions, but it is difficult and time consuming; we were already using one in this workflow and now two more, because it is not just this one you comment, we also need another one to set the commit status.
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.
Great work!!
🎉 This PR is included in version 29.5.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
IOS-9864
🥅 What's the goal?
Update the GitHub action to record the screenshots so it also executes the tests.
Also change the action so it is executed by inserting
/record-screenshots
as comment on a PR to avoid branch mistakes and to be able to set the commit and the PR status after passing the tests.🚧 How do we do it?