-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Build/Test Tools: Add a workflow for the HTML API's html5lib tests. #6546
Changes from all commits
86ca144
5cdf152
2b71fbd
36e9f49
a8f0aa8
9d7f20b
1bd3a1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,195 @@ | ||||||||||
name: HTML API html5lib Tests | ||||||||||
|
||||||||||
on: | ||||||||||
push: | ||||||||||
branches: | ||||||||||
- trunk | ||||||||||
# The html5lib tests were introduced in WordPress 6.6. | ||||||||||
- '6.[6-9]' | ||||||||||
- '[7-9].[0-9]' | ||||||||||
- '[1-9][0-9]+.[0-9]' | ||||||||||
tags: | ||||||||||
- '6.[6-9]' | ||||||||||
- '6.[6-9].[0-9]+' | ||||||||||
- '[7-9].[0-9]' | ||||||||||
- '[7-9].[0-9].[0-9]+' | ||||||||||
- '[1-9][0-9]+.[0-9]' | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, let's stop with the |
||||||||||
- '[1-9][0-9]+.[0-9].[0-9]+' | ||||||||||
pull_request: | ||||||||||
branches: | ||||||||||
- trunk | ||||||||||
- '6.[6-9]' | ||||||||||
- '[7-9].[0-9]' | ||||||||||
- '[1-9][0-9]+.[0-9]' | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here as well. |
||||||||||
paths: | ||||||||||
# Any change to a PHP file in the HTML API should run checks. | ||||||||||
- 'src/wp-includes/html-api/**.php' | ||||||||||
- 'tests/phpunit/tests/html-api/wpHtmlProcessorHtml5lib.php' | ||||||||||
- 'tests/phpunit/data/html5lib-tests/**.dat' | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changes to this file should also trigger it.
Suggested change
|
||||||||||
workflow_dispatch: | ||||||||||
|
||||||||||
# Cancels all previous workflow runs for pull requests that have not completed. | ||||||||||
concurrency: | ||||||||||
# The concurrency group contains the workflow name and the branch name for pull requests | ||||||||||
# or the commit hash for any other events. | ||||||||||
group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }} | ||||||||||
cancel-in-progress: true | ||||||||||
|
||||||||||
# Disable permissions for all available scopes by default. | ||||||||||
# Any needed permissions should be configured at the job level. | ||||||||||
permissions: {} | ||||||||||
|
||||||||||
jobs: | ||||||||||
# Runs the HTML API html5lib PHPUnit tests for WordPress. | ||||||||||
# | ||||||||||
# Performs the following steps: | ||||||||||
# - Sets environment variables. | ||||||||||
# - Checks out the repository. | ||||||||||
# - Sets up Node.js. | ||||||||||
# - Sets up PHP. | ||||||||||
# - Installs Composer dependencies. | ||||||||||
# - Installs npm dependencies | ||||||||||
# - Logs general debug information about the runner. | ||||||||||
# - Logs Docker debug information (about the Docker installation within the runner). | ||||||||||
# - Starts the WordPress Docker container. | ||||||||||
# - Logs the running Docker containers. | ||||||||||
# - Logs debug information about what's installed within the WordPress Docker containers. | ||||||||||
# - Install WordPress within the Docker container. | ||||||||||
# - Run the HTML API html5lib PHPUnit tests. | ||||||||||
# - Ensures version-controlled files are not modified or deleted. | ||||||||||
run-html-api-html5lib-tests: | ||||||||||
name: PHP ${{ matrix.php }} | ||||||||||
runs-on: ubuntu-latest | ||||||||||
permissions: | ||||||||||
contents: read | ||||||||||
if: ${{ github.repository == 'WordPress/wordpress-develop' || github.event_name == 'pull_request' }} | ||||||||||
strategy: | ||||||||||
fail-fast: false | ||||||||||
matrix: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PHP version is not being applied properly to the test environment. Every job in the workflow is running on the default version of PHP, which is currently 8.2.
|
||||||||||
php: [ '7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3' ] | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there reasons not to run against multisite? Or different database types? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think either would be valuable here since these tests are extremely close to actual unit tests. None of the HTML API code interacts with the database nor does it rely on any code which is different for multisite. |
||||||||||
|
||||||||||
steps: | ||||||||||
- name: Configure environment variables | ||||||||||
run: | | ||||||||||
echo "PHP_FPM_UID=$(id -u)" >> $GITHUB_ENV | ||||||||||
echo "PHP_FPM_GID=$(id -g)" >> $GITHUB_ENV | ||||||||||
|
||||||||||
- name: Checkout repository | ||||||||||
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 | ||||||||||
with: | ||||||||||
show-progress: ${{ runner.debug == '1' && 'true' || 'false' }} | ||||||||||
|
||||||||||
- name: Set up Node.js | ||||||||||
uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2 | ||||||||||
with: | ||||||||||
node-version-file: '.nvmrc' | ||||||||||
cache: npm | ||||||||||
Comment on lines
+82
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could probably be skipped. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tried removing Docker, Node and NPM in 36e9f49, but an error was thrown due to |
||||||||||
|
||||||||||
## | ||||||||||
# This allows Composer dependencies to be installed using a single step. | ||||||||||
# | ||||||||||
# Since the tests are currently run within the Docker containers where the PHP version varies, | ||||||||||
# the same PHP version needs to be configured for the action runner machine so that the correct | ||||||||||
# dependency versions are installed and cached. | ||||||||||
## | ||||||||||
- name: Set up PHP | ||||||||||
uses: shivammathur/setup-php@a4e22b60bbb9c1021113f2860347b0759f66fe5d # v2.30.0 | ||||||||||
with: | ||||||||||
php-version: '${{ matrix.php }}' | ||||||||||
coverage: none | ||||||||||
|
||||||||||
# Since Composer dependencies are installed using `composer update` and no lock file is in version control, | ||||||||||
# passing a custom cache suffix ensures that the cache is flushed at least once per week. | ||||||||||
- name: Install Composer dependencies | ||||||||||
uses: ramsey/composer-install@57532f8be5bda426838819c5ee9afb8af389d51a # v3.0.0 | ||||||||||
with: | ||||||||||
custom-cache-suffix: $(/bin/date -u --date='last Mon' "+%F") | ||||||||||
|
||||||||||
- name: Install npm dependencies | ||||||||||
run: npm ci | ||||||||||
Comment on lines
+108
to
+109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could probably be skipped. |
||||||||||
|
||||||||||
- name: General debug information | ||||||||||
run: | | ||||||||||
npm --version | ||||||||||
node --version | ||||||||||
curl --version | ||||||||||
git --version | ||||||||||
composer --version | ||||||||||
locale -a | ||||||||||
|
||||||||||
- name: Docker debug information | ||||||||||
run: | | ||||||||||
docker -v | ||||||||||
|
||||||||||
- name: Start Docker environment | ||||||||||
run: | | ||||||||||
npm run env:start | ||||||||||
|
||||||||||
- name: Log running Docker containers | ||||||||||
run: docker ps -a | ||||||||||
|
||||||||||
- name: WordPress Docker container debug information | ||||||||||
run: | | ||||||||||
docker compose run --rm mysql ${{ env.LOCAL_DB_TYPE }} --version | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This environment variable is not set within this workflow. The command ( |
||||||||||
docker compose run --rm php php --version | ||||||||||
docker compose run --rm php php -m | ||||||||||
docker compose run --rm php php -i | ||||||||||
docker compose run --rm php locale -a | ||||||||||
|
||||||||||
- name: Install WordPress | ||||||||||
run: npm run env:install | ||||||||||
|
||||||||||
- name: Run HTML API html5lib PHPUnit tests | ||||||||||
run: node ./tools/local-env/scripts/docker.js run php ./vendor/bin/phpunit --verbose -c phpunit.xml.dist --group html-api-html5lib-tests | ||||||||||
|
||||||||||
- name: Ensure version-controlled files are not modified or deleted | ||||||||||
run: git diff --exit-code | ||||||||||
|
||||||||||
slack-notifications: | ||||||||||
name: Slack Notifications | ||||||||||
uses: WordPress/wordpress-develop/.github/workflows/slack-notifications.yml@trunk | ||||||||||
permissions: | ||||||||||
actions: read | ||||||||||
contents: read | ||||||||||
needs: [ run-html-api-html5lib-tests ] | ||||||||||
if: ${{ github.repository == 'WordPress/wordpress-develop' && github.event_name != 'pull_request' && always() }} | ||||||||||
with: | ||||||||||
calling_status: ${{ contains( needs.*.result, 'cancelled' ) && 'cancelled' || contains( needs.*.result, 'failure' ) && 'failure' || 'success' }} | ||||||||||
secrets: | ||||||||||
SLACK_GHA_SUCCESS_WEBHOOK: ${{ secrets.SLACK_GHA_SUCCESS_WEBHOOK }} | ||||||||||
SLACK_GHA_CANCELLED_WEBHOOK: ${{ secrets.SLACK_GHA_CANCELLED_WEBHOOK }} | ||||||||||
SLACK_GHA_FIXED_WEBHOOK: ${{ secrets.SLACK_GHA_FIXED_WEBHOOK }} | ||||||||||
SLACK_GHA_FAILURE_WEBHOOK: ${{ secrets.SLACK_GHA_FAILURE_WEBHOOK }} | ||||||||||
|
||||||||||
failed-workflow: | ||||||||||
name: Failed workflow tasks | ||||||||||
runs-on: ubuntu-latest | ||||||||||
permissions: | ||||||||||
actions: write | ||||||||||
needs: [ slack-notifications ] | ||||||||||
if: | | ||||||||||
always() && | ||||||||||
github.repository == 'WordPress/wordpress-develop' && | ||||||||||
github.event_name != 'pull_request' && | ||||||||||
github.run_attempt < 2 && | ||||||||||
( | ||||||||||
contains( needs.*.result, 'cancelled' ) || | ||||||||||
contains( needs.*.result, 'failure' ) | ||||||||||
) | ||||||||||
|
||||||||||
steps: | ||||||||||
- name: Dispatch workflow run | ||||||||||
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 | ||||||||||
with: | ||||||||||
retries: 2 | ||||||||||
retry-exempt-status-codes: 418 | ||||||||||
script: | | ||||||||||
github.rest.actions.createWorkflowDispatch({ | ||||||||||
owner: context.repo.owner, | ||||||||||
repo: context.repo.repo, | ||||||||||
workflow_id: 'failed-workflow.yml', | ||||||||||
ref: 'trunk', | ||||||||||
inputs: { | ||||||||||
run_id: '${{ github.run_id }}' | ||||||||||
} | ||||||||||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ | |
* | ||
* See the README file at DIR_TESTDATA / html5lib-tests for details on the third-party suite. | ||
* | ||
* A change to test that the workflow triggers. | ||
* | ||
Comment on lines
+15
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flagging this, I expect the plan is to remove this before final commit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep 🙂 |
||
* @package WordPress | ||
* @subpackage HTML-API | ||
* | ||
|
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.
Let's remove this range. At our current 3 major versions a year pace, we won't reach 10.0 for 11 more years. 🙈
All other workflows will need to be updated as well, and I think it's preferable to just do them all at once.