-
Notifications
You must be signed in to change notification settings - Fork 72
Add GitHub action for easy remote test execution via GitHub Actions #245
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: master
Are you sure you want to change the base?
Add GitHub action for easy remote test execution via GitHub Actions #245
Conversation
@@ -0,0 +1,137 @@ | |||
name: WordPress Host Test |
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.
Possible follow-up: Since the pipeline in .github/workflows/tests.yml
also appears to be intended as a template to be adopted by hosting providers in their own pipelines, that might possibly be dropped when/if this PR gets merged.
Define a re-usable GitHub action based on the existing workflow file. This action can be used in other repositories (running schedules pipelines) to repeatedly and regularly run the WordPress test suite on remote hosts.
29495fe
to
6fce3b4
Compare
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.
Thanks for this, @martin-helmich!
I have some suggestions that I've left in context. But I have one main push back to merging this here: if we are going to go the composite action route, then I think we should publish this as an action on the GitHub marketplace. However, that would require a new repository. Not a deal breaker, but that would allow us to properly support releasing and versioning.
If we want the action to remain in this repository, I personally prefer the reusable pattern for a few reasons:
- It displays each step of the workflow as a separate step in the workflow run UI instead of collapsing them all into the same step. The latter can sometimes make it difficult to debug where exactly errors are encountered.
- It allows you to specify the runner type to use. While this is likely not important today, it could be very useful if a host is running their own self-hosted runners.
- Reusable workflows can run multiple jobs, which in the future could mean easily supporting other types of testing (E2E, Performance, etc.) in addition to PHPUnit tests within the same workflow in parallel with each showing as their own job. With composite actions, this would require separate actions.
The pattern has worked well in https://github.com/wordpress/wordpress-develop, but the situation is a bit different and I don't believe that those workflows are used outside of that repository.
@@ -0,0 +1,139 @@ | |||
name: WordPress Host Test | |||
description: > |
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.
description: > | |
author: WordPress.org | |
description: > |
@@ -0,0 +1,139 @@ | |||
name: WordPress Host Test | |||
description: > | |||
Run the WordPress Host Test on a remote system reachable via SSH |
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.
Run the WordPress Host Test on a remote system reachable via SSH | |
Runs the WordPress PHPUnit test suite on a remote system reachable via SSH and reports the results. |
description: > | ||
Run the WordPress Host Test on a remote system reachable via SSH | ||
|
||
inputs: |
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.
Could we list all required
inputs first?
- name: Set up PHP | ||
uses: shivammathur/setup-php@cf4cade2721270509d5b1c766ab3549210a39a2a # v2.33.0 | ||
with: | ||
php-version: '8.4' |
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 think this should be configurable through an optional input
. This would ensure that the version of PHP on configured on the runner matches the one being tested so that Composer will install the correct dependencies.
I think that 8.4
is a good default. We should note in the input description that this will default to the latest version of PHP officially supported by WordPress in wordpress-develop@trunk
.
php-version: '8.4' | ||
coverage: none | ||
|
||
- name: Install NodeJS |
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 there any reason not to pass cache: 'npm'
here?
required: true | ||
ssh-private-key: | ||
description: > | ||
A base64-encoded SSH private key. |
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.
The description for each input
passing sensitive values should explicitly note that the values should be configured as GitHub Actions repository or organization secrets to ensure that they are properly obfuscated in the logs.
using: "composite" | ||
steps: | ||
- name: Checkout repository | ||
uses: actions/checkout@v4 |
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.
uses: actions/checkout@v4 | |
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 |
Third-party actions should always be pinned to a specific SHA value, which is currently the only immutable release supported (🤞 GH public roadmap).
with: | ||
node-version: 20 | ||
|
||
- name: Prepare SSH private key |
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.
Curious if there was a reason why you decided to configure the key here instead of relying on the prepare.php
script to take care of it? I don't think this is an issue, but just wanted to know if there was a specific reason.
|
||
- name: Prepare environment | ||
env: | ||
WPT_PREPARE_DIR: /tmp/wp-tests |
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.
Would the ${{ runner.temp }}
context work here? It may be better to ensure that the directory is reliably accessible/writable.
shell: bash | ||
working-directory: ./runner | ||
|
||
- name: Run unit tests |
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.
- name: Run unit tests | |
- name: Run PHPUnit tests |
This PR introduces an actual, re-usable GitHub action definition.
Hosting providers can use this action in their own GitHub actions (which could for example be running on a schedule) to remotely run the test suite on their infrastructure (which needs to be accessible via SSH).