Make WordPress Core

Opened 5 months ago

Last modified 25 hours ago

#62221 assigned task (blessed)

GitHub Actions updates and improvements for 6.8

Reported by: desrosj's profile desrosj Owned by: desrosj's profile desrosj
Milestone: 6.8 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

This ticket is for various updates and improvements for Core's GitHub Actions workflows.

Previously:

Attachments (1)

Screenshot 2025-01-22 10.41.13 AM.png (138.6 KB) - added by flixos90 2 months ago.
Screenshot of partial Code Vitals dashboard API response showing the same one commit twice

Download all attachments as: .zip

Change History (147)

This ticket was mentioned in PR #7565 on WordPress/wordpress-develop by @ayeshrajans.


5 months ago
#1

  • Keywords has-patch added

#2 @desrosj
5 months ago

In 59276:

Build/Test Tools: Run upgrade tests against 6.7-RC1.

The 6.7 version tag does not exist yet. This results in the upgrade tests failing when trying to update from 6.7.

Since 6.7-RC1 exists, this should be used instead.

Follow up to [59275].

See #62221.

#3 @desrosj
5 months ago

In 59277:

Build/Test Tools: Add support for MySQL 8.4 to the Docker environment.

See #62221.

#4 @desrosj
5 months ago

In 59281:

Build/Test Tools: Split up upgrade test matrix.

GitHub Actions caps the number of jobs that can be spawned from a single matrix at 256.

The changes in [59280] pushed the WordPress 6.x job over this limit. This splits that matrix into two following established pattern for older branches in the workflow.

See #61218, #62221.

This ticket was mentioned in PR #7729 on WordPress/wordpress-develop by @desrosj.


5 months ago
#5

Dependabot will never open a Trac ticket, so it's pointless to ask for one.

Dependeabot is also only configured to watch GitHub Actions for updates. So changes being made by the bot do not require the use of Playground for verification and testing.

Trac ticket: https://core.trac.wordpress.org/ticket/62221.

#6 @desrosj
5 months ago

In 59354:

Build/Test Tools: Update 3rd-party GitHub Actions.

This updates the following GitHub Actions to their latest versions:

  • wow-actions/welcome
  • actions/setup-node
  • actions/cache

See #62221.

#7 @desrosj
5 months ago

In 59355:

Build/Test Tools: Run test coverage when PHPunit workflow changes.

[59287] updated the test coverage workflow to make use of the reusable PHPUnit workflow logic to prevent having duplicate code. The workflow should be run when the reusable file is updated to confirm any changes made work as expected.

See #62221.

This ticket was mentioned in PR #7738 on WordPress/wordpress-develop by @desrosj.


4 months ago
#8

The ubuntu-latest runner will soon be updated to point to Ubuntu 24, which has a different set of available tools.

This PR is meant to test Core's workflows against this runner image to check for potential problems.

See https://github.blog/changelog/2024-11-05-notice-of-breaking-changes-for-github-actions/.

Trac ticket: https://core.trac.wordpress.org/ticket/62221.

@desrosj commented on PR #7738:


4 months ago
#9

Looks like everything is in working order after the upgrade. There should be no issues when this change is rolled out. 👍

#10 @desrosj
4 months ago

In 59370:

Build/Test Tools: Skip pull request comments for Dependabot.

Currently, Dependabot is configured to open pull requests when updates to 3rd-party GitHub Actions become available.

It does a great job at this. Thank you very much, 🤖 Mr. Dependabot Roboto.

Some of the automated comments for pull requests are not relevant to PRs opened by Dependabot. Despite how good of a robot it is, Dependabot will never open a Trac ticket, so it's pointless to ask for one.

Also, since it’s currently only configured to watch GitHub Actions for updates, there will never be a need to test Dependabot PRs in Playground. If instructed to monitor npm dependencies in the future, this comment can be added back as those packages can directly affect the built software that is distributed.

Props johnbillion.
See #62221.

#12 @desrosj
4 months ago

In 59397:

Build/Test Tools: Run upgrade tests against 6.7.

Now that 6.7 is generally available, the upgrade tests no longer need to be run against a pre-release version.

See #62221.

This ticket was mentioned in PR #7795 on WordPress/wordpress-develop by @desrosj.


4 months ago
#13

The database container is initialized when services are set up. The additional systemctl start is not required.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

#14 @desrosj
4 months ago

In 59402:

Build/Test Tools: Avoid starting the database twice.

The database container is started when the services are initially set up. Having a separate step for this sometimes introduces unexpected failures for an unknown reason.

Props johnbillion.
See #62221.

#16 @desrosj
4 months ago

In 59403:

Build/Test Tools: Correct upgrade testing workflow name.

The reusable upgrade testing workflow was renamed in [58165], but the event paths filters were not updated accordingly.

See #62221.

@desrosj commented on PR #7729:


4 months ago
#17

Seems that this did not work based on the latest PR opened by Dependabot: #7806.

@desrosj commented on PR #7729:


4 months ago
#18

Looks like the correct value to use is dependabot[bot].

#19 @desrosj
4 months ago

In 59441:

Build/Test Tools: Correctly check for Dependabot.

This updates the conditions added in [59370] to skip unnecessary pull request comments when Dependabot is the opening contributor to check for the correct github.actor value.

Follow up to [59380].

See #62221.

This ticket was mentioned in PR #7630 on WordPress/wordpress-develop by @desrosj.


4 months ago
#20

This adds the latest MySQL innovation release to the testing matrix: 9.1. It also adds the newest LTS version of MariaDB (11.4).

Additionally, it makes some adjustments to the MariaDB matrix to improve the overall coverage a bit.

Technically, back to MariaDB 5.5 is still supported by WordPress, even though <= 10.4 has reached EoL. However, 5.5 is equivalent to MySQL 5.5, which is not currently tested due to the fact that the Docker containers are not compatible with modern machines and do not start.

With a small change to the docker-compose.yml file, the MariaDB 5.5 containers do start successfully. Testing against this version essentially gives us MySQL 5.5 testing. The matrix has been adjusted to include all MariaDB versions with greater than 1% of usage with WordPress in the wild according to the wordpress.org Stats page.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

This ticket was mentioned in PR #7873 on WordPress/wordpress-develop by @desrosj.


4 months ago
#21

This extracts the workflow logic that parses the .version-support-(php|mysql).json files from the Installation testing workflow into a reusable one. This makes it accessible to other workflows that may want to automatically build a test matrix from these files.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

#22 @desrosj
4 months ago

In 59452:

Build/Test Tools: Create reusable workflow for parsing .version-support-*.json files.

This extracts the logic responsible for parsing the .version-support-*.json files and returning a list of supported PHP and MySQL versions for a given branch of WordPress into a reusable workflow so that other workflows can make use of the functionality without repeating code.

See #62221.

#24 @desrosj
4 months ago

In 59483:

Build/Test Tools: Add repository input to support JSON reading workflow.

actions/checkout will always checkout the current repository unless the repository input is specified. This updates the reusable-support-json-reader-v1.yml workflow to always default to reading the JSON files from wordpress-develop.

A repository has also been added to the workflow to allow a different set of JSON files to be read if desired.

See #62221.

#25 @desrosj
4 months ago

In 59484:

Build/Test Tools: Support older MariaDB versions in local Docker environment.

Older versions of MariaDB did not contain the mariadb-admin command. This command is configured as the healthcheck used by the local Docker environment to confirm that the database container has successfully started and is reporting as “healthy”. The current result is a failure when starting the environment while using one of the affected older versions.

For MariaDB versions 10.3 and earlier, the mysqladmin command was used instead. Since WordPress still technically supports back to MariaDB 5.5, the local environment should support running these versions. This updates the environment configuration to take this into account when performing a healthcheck test.

The README file is also updated to reflect that the same workaround added in [57568] for MySQL <= 5.7 is required when using MariaDB 5.5 on an Apple silicon machine.

Props johnbillion.
See #62221.

#26 @desrosj
4 months ago

In 59485:

Build/Test Tools: Run install tests when JSON reading workflow is changed.

Because the installation testing workflow relies on the reusable workflow that reads the JSON support files, it should be run when that file is changed to confirm there are no issues.

This is currently only configured for pull_request events, but should also be true for push.

See #62221.

This ticket was mentioned in PR #7950 on WordPress/wordpress-develop by @desrosj.


4 months ago
#27

This updates the jobs specifically specified in the includes list to use more modern versions of PHP and MySQL/MariaDB.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

#28 @afercia
4 months ago

@desrosj Looks like [59484] broke my environment, it fails to start since this morning. Console error:

> WordPress@6.8.0 env:start
> node ./tools/local-env/scripts/start.js && node ./tools/local-env/scripts/docker.js run -T --rm php composer update -W

invalid interpolation format for services.mysql.healthcheck.test.[]: "if [ \"$LOCAL_DB_TYPE\" = \"mariadb\" ]; then case \"$LOCAL_DB_VERSION\" in 5.5|10.0|10.1|10.2|10.3) mysqladmin ping -h localhost || exit $?;; *) mariadb-admin ping -h localhost || exit $?;; esac; else mysqladmin ping -h localhost || exit $?; fi". You may need to escape any $ with another $.
node:child_process:965
    throw err;
    ^

Error: Command failed: docker compose -f docker-compose.yml up -d wordpress-develop
    at genericNodeError (node:internal/errors:984:15)
    at wrappedFn (node:internal/errors:538:14)
    at checkExecSyncError (node:child_process:890:11)
    at execSync (node:child_process:962:15)
    at Object.<anonymous> (/Users/andrea/Projects/wordpress-develop/tools/local-env/scripts/start.js:34:1)
    at Module._compile (node:internal/modules/cjs/loader:1369:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1427:10)
    at Module.load (node:internal/modules/cjs/loader:1206:32)
    at Module._load (node:internal/modules/cjs/loader:1022:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:135:12) {
  status: 15,
  signal: null,
  output: [ null, null, null ],
  pid: 42399,
  stdout: null,
  stderr: null
}

Node.js v20.12.2

Looking at the part You may need to escape any $ with another $. of the error message, I just changed the three 'exit $?' to exit $$? on this line, and the start script worked again for me. Could you please have a look when you have a chance? Thanks.

#30 @desrosj
4 months ago

Thanks @afercia! I've opened a PR with this change, and it looks like everything works as expected. Could you confirm that it fixes the issue for you?

@afercia commented on PR #7960:


4 months ago
#31

@desrosj thanks for your quick fix. Yes it fixes the issue for me.

#32 @desrosj
4 months ago

In 59489:

Build/Test Tools: Properly escape $ characters in Docker compose file.

This fixes an invalid interpolation format error that can be encountered in the mysql container’s healthcheck test command.

Follow up to [59484].

Props afercia.
See #62221.

#34 @desrosj
4 months ago

In 59490:

Build/Test Tools: Use newer versions for include jobs.

The include part of the strategy for the PHPUnit testing workflow defines a few testing configurations outside of the matrix. The versions of PHP and MySQL used in these have not been updated for some time. This was mostly due to various incompatibilities that have since been resolved.

Props peterwilsoncc, johnbillion.
See #62221.

#36 @desrosj
4 months ago

In 59491:

Build/Test Tools: Support trunk as a version.

trunk is used interchangeably with nightly, so should be an accepted value when determining which version of WordPress is being tested.

Follow up to [59452], [59483].

Props johnbillion.
See #62221.

#37 @desrosj
4 months ago

In 59492:

Build/Test Tools: Introduce workflow for testing the local Docker environment.

While the PHPUnit workflow currently relies on the local Docker environment and provides some safety checks that the environment works as expected, this may not always be true and does not test all of the available commands related to the environment.

This introduces a basic workflow for testing the related scripts for the various supported combinations of PHP and database software with the environment to confirm everything is working as expected.

Ideally this would also be run on Windows and MacOS to catch platform specific bugs. Unfortunately, Docker is not supported within the GitHub Action runner images, so not all bugs will be caught by this workflow.

Props johnbillion, Clorith.
See #62221.

This ticket was mentioned in PR #7966 on WordPress/wordpress-develop by @desrosj.


3 months ago
#38

As it stands today, the upgrade testing workflow is currently at ~978 jobs created. While it's great to test all possible combinations, GitHub's UI cannot keep up with tracking that number of jobs, and that is 2x the total number of allowed jobs for the organization (which slows everything down even more).

This PR trims down the number of combinations included in the testing matrices to be a bit more thoughtful following the following methodology:

  • The last two releases of WordPress are tested against all PHP and MySQL LTS version combinations and the most recent innovation release.
  • The next 6 oldest versions of WordPress are tested against both the oldest and newest releases of PHP currently supported for both PHP 7 & 8 along with the oldest and newest MySQL LTS versions currently supported (no innovation releases).
  • For the remaining versions of WordPress receiving security updates, they are only included if the database version was different that the previous major release.
  • The oldest version of WordPress receiving security updates should always be tested.

Some additional notes about the chosen MySQL versions:

  • Only the most recent innovation release should be included in testing.
  • Even though MySQL >= 5.5.5 is currently supported, there are no 5.5.x Docker containers available that work on modern architectures.
  • 5.6.x Docker containers are available and work, but 5.6 only accounts for ~2.3% of installs as of 12/6/2024.defaults:
  • 5.7.x accounts for ~2-% of installs, so this is used below instead.

The contents of this PR description are included as inline documentation in the workflow to help guide contributors when updating this workflow in the future.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

This ticket was mentioned in PR #7971 on WordPress/wordpress-develop by @johnbillion.


3 months ago
#39

#40 @desrosj
3 months ago

In 59507:

Build/Test Tools: Remove repository specific logic from callable workflows.

Because reusable workflows could be called from any other repository in a variety of contexts, repository specific if conditions should not be present.

Instead, this logic should be included in the calling workflows only.

Props johnbillion.
See #62221.

@desrosj commented on PR #7966:


3 months ago
#41

I've made one adjustment to the strategy as described above.

I've separated out the oldest version still receiving security updates and expanded the matrix to ensure it's tested against the same range of releases as the last 2 releases. I think that it makes sense to run the oldest version against the full set of combinations.

#42 @desrosj
3 months ago

In 59508:

Build/Test Tools: Trim down the upgrade testing matrix.

The upgrade testing workflow is currently at ~978 jobs spawned from the strategy matrix. While it's great to test all possible combinations, GitHub's UI cannot keep up with tracking that number of jobs, often taking 30-45 minutes to accurately report the outcome even though the jobs themselves all complete in under 5 minutes.

This is 2x the total number of concurrent jobs allowed for the entire organization (which creates a backlog and slows everything down even more).

This trims down the number of combinations included in the testing matrices to be a bit more thoughtful following the following methodology:

  • The last two releases of WordPress are tested against all PHP and MySQL LTS version combinations and the most recent innovation release.
  • The next 6 oldest versions of WordPress are tested against both the oldest and newest releases of PHP currently supported for both PHP 7 & 8 along with the oldest and newest MySQL LTS versions currently supported (no innovation releases).
  • For the remaining versions of WordPress receiving security updates, they are only included if the database version was different that the previous major release.
  • The oldest version of WordPress receiving security updates should always be tested against the same full list of combinations as the last two releases.

When choosing which MySQL versions to test against:

  • Only the most recent innovation release should be included in testing.
  • Even though MySQL >= 5.5.5 is currently supported, there are no 5.5.x Docker containers available that work on modern architectures.
  • 5.6.x Docker containers are available and work, but 5.6 only accounts for ~2.3% of installs as of 12/6/2024.defaults:
  • 5.7.x accounts for ~20% of installs, so this is used below instead.

With these changes, the total number of jobs is reduced by ~58%.

Props johnbillion, mukesh27.
See #62221.

This ticket was mentioned in PR #8007 on WordPress/wordpress-develop by @johnbillion.


3 months ago
#44

#45 @desrosj
3 months ago

In 59519:

Build/Test Tools: Update slackapi/slack-github-action.

This makes the necessary changes to update the Slack GitHub Action to the latest version, currently 2.0.0.

Most notably this update provides more control over how attempts re retried when rate limiting is encountered.

Reverts [59209].

See #61701, #62221.

#46 @desrosj
3 months ago

In 59520:

Build/Test Tools: Support manual runs for the test old branch workflow.

This is the only workflow that does not currently support manually running.

See #62221.

#48 @desrosj
3 months ago

In 59521:

Build/Test Tools: Update the Codecov GitHub Action.

This updates the codecov/codecov-action to from version 4.6.0 to 5.1.1.

See #62221.

#52 @desrosj
3 months ago

In 59527:

Build/Test Tools: Use MySQL 8.4 as the default.

MySQL 8.4 is the latest LTS.

See #62221.

#53 @desrosj
3 months ago

In 59529:

Build/Test Tools: Allow more control when testing older branches.

This adds an input to the Test Old Branches workflow that allows a specific branch to be specified or all to run all old branches.

The default behavior is to only test the currently supported version of WordPress as defined in the CURRENTLY_SUPPORTED_BRANCH environment variable.

Follow up to [59520].

See #62221.

#54 @desrosj
3 months ago

In 59531:

Build/Test Tools: Document every matrix exclusion.

There should be inline documentation anytime a strategy matrix has an exclude combination configured so that contributors have proper context as to why it’s there.

See #62221.

#55 @johnbillion
3 months ago

In 59534:

Build/Test Tools: Remove an unnecessary call to svn in a debugging step.

None of the steps in any of the workflows use svn, so this debugging step is unnecessary, and svn has been removed in the ubuntu-24.04 runner which will be rolling out to GitHub Actions imminently.

See #62221

#56 @desrosj
2 months ago

In 59585:

Build/Test Tools: Test against MySQL 9.1.

This is the latest innovation release from MySQL.

Props johnbillion, jorbin.
See #62221.

#57 @desrosj
2 months ago

In 59586:

Build/Test Tools: Test MariaDB innovation releases.

MariaDB also follows the innovation release model. This adds testing for these releases to the test matrix and moves innovation versions to a new job in order to more clearly differentiate from LTS ones.

The current innovation release for MariaDB is 11.6.

Props johnbillion, jorbin.
See #62221.

#58 @desrosj
2 months ago

In 59587:

Build/Test Tools: Expand and improve MariaDB test matrix.

The latest LTS version of MariaDB is 11.4, which is now included in the test matrix.

This changeset also expands the test matrix to include all LTS versions of MariaDB with > 1% of usage on WordPress sites in the wild as reported by the stats page on WordPress.org. Though a few of these are unsupported upstream, they are still supported in WordPress itself.

MariaDB 5.5 is also included in the new matrix. Because it was intended as a drop-in replacement to MySQL at the time, this also brings some MySQL 5.5 testing into the matrix. This has not been regularly tested against since specific database versions were included due to the lack of a working Docker container.

Props johnbillion, jorbin.
See #62221.

This ticket was mentioned in PR #8162 on WordPress/wordpress-develop by @desrosj.


2 months ago
#61

The matchdep dependency has not been updated in over 7 years. The upstream repository has not run automated testing on a Node.js version higher than 10.x. This replaces the package with a custom Grunt function.

Previously introduced in Core-25243.

Trac ticket: https://core.trac.wordpress.org/ticket/62221.

This ticket was mentioned in PR #8164 on WordPress/wordpress-develop by @desrosj.


2 months ago
#62

The E2E and Performance testing workflows have started failing recently for branches using Playwright. This seems to be due to the change for ubuntu-latest to now point to ubuntu-24 instead of ubuntu-22.

Updating Playwright to the latest version seems to fix the issue with no obvious side effects.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

This ticket was mentioned in PR #8163 on WordPress/wordpress-develop by @desrosj.


2 months ago
#64

The E2E and Performance testing workflows have started failing recently for branches using Playwright. This seems to be due to the change for ubuntu-latest to now point to ubuntu-24 instead of ubuntu-22.

Updating Playwright to the latest version seems to fix the issue with no obvious side effects.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

This ticket was mentioned in PR #8163 on WordPress/wordpress-develop by @desrosj.


2 months ago
#63

The E2E and Performance testing workflows have started failing recently for branches using Playwright. This seems to be due to the change for ubuntu-latest to now point to ubuntu-24 instead of ubuntu-22.

Updating Playwright to the latest version seems to fix the issue with no obvious side effects.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

This ticket was mentioned in PR #8169 on WordPress/wordpress-develop by @desrosj.


2 months ago
#64

The E2E and Performance testing workflows have started failing recently for branches using Playwright. This seems to be due to the change for ubuntu-latest to now point to ubuntu-24 instead of ubuntu-22.

Updating Playwright to the latest version seems to fix the issue with no obvious side effects. Since this only effects build tooling and not the built software, this seems like a reasonable change to make in a somewhat older branch.

This updates Playwright to the latest version in trunk so that older versions are not more up to date than trunk.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

This ticket was mentioned in PR #8170 on WordPress/wordpress-develop by @desrosj.


2 months ago
#65

  • Keywords has-unit-tests added

The E2E and Performance testing workflows have started failing recently for branches using Playwright. This seems to be due to the change for ubuntu-latest to now point to ubuntu-24 instead of ubuntu-22.

Updating Playwright to the latest version seems to fix the issue with no obvious side effects. Since this only effects build tooling and not the built software, this seems like a reasonable change to make in a somewhat older branch.

This updates the 6.6 branch so that older versions are not more up to date.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

See also #8169, #8163, #8164.

#66 @johnbillion
2 months ago

In 59679:

Build/Test Tools: Improve the security and correctness of the GitHub Actions workflows files.

This includes removing use of dangerous inline GitHub Actions expressions, preventing word splitting, further tightening permissions, and generally improving many aspects of the workflows.

This also introduces a new workflow that runs Actionlint to detect incorrect and insecure code and configuration in workflow files.

Props johnbillion, swissspidy, flixos90, desrosj.

See #62221

This ticket was mentioned in PR #8171 on WordPress/wordpress-develop by @desrosj.


2 months ago
#67

The E2E and Performance testing workflows have started failing recently for branches using Playwright. This seems to be due to the change for ubuntu-latest to now point to ubuntu-24 instead of ubuntu-22.

Updating Playwright to the latest version seems to fix the issue with no obvious side effects. Since this only effects build tooling and not the built software, this seems like a reasonable change to make in a somewhat older branch.

This updates the 6.7 branch so that older versions are not more up to date.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

See also #8169, #8163, #8164, #8170.

#68 @desrosj
2 months ago

In 59681:

Build/Test Tools: Fix Slack message payload generation.

The JSON string set as an output for the Slack message payload needs to be one line to prevent causing errors. This ensures jq returns a compact JSON string.

Follow up to [59679].

Props johnbillion.
See #62221.

#75 follow-up: @flixos90
2 months ago

@johnbillion @desrosj Not a huge issue, but I just checked the commit data in https://www.codevitals.run/project/wordpress/ after [59679], and for some reason that particular commit (hash 8209135dfc327dc24a274efb0d3f01381e48c739) shows up twice.

All data looks right, and all other commits show once as expected, just this one shows twice (see screenshot which I'll upload). You can also see that in the graph itself that has two points for that same commit (towards the very end).

Again, as long as this was a once-off hiccup due to something particular with this change, it's not an issue, but it would be great to know why it happened, to rule out that there's an actual problem. Like for instance, did one of us rerun one of the workflows that sends the data for that commit, and it actually sent the data both times?

@flixos90
2 months ago

Screenshot of partial Code Vitals dashboard API response showing the same one commit twice

This ticket was mentioned in PR #8174 on WordPress/wordpress-develop by @johnbillion.


2 months ago
#76

context.runId is an integer, which doesn't seem to be accepted for the run_id input in the failed-workflow.yml workflow. This fixes that, and also updates two missed instances of inline expressions.

#77 in reply to: ↑ 75 @desrosj
2 months ago

Replying to flixos90:

Again, as long as this was a once-off hiccup due to something particular with this change, it's not an issue, but it would be great to know why it happened, to rule out that there's an actual problem. Like for instance, did one of us rerun one of the workflows that sends the data for that commit, and it actually sent the data both times?

After [59679], every workflow that ran as a result failed because of a bug in the Slack workflow. After it was fixed in [59681], I reran all of them. So seems we don't handle reruns properly (or we do and just don't account for two records)? Not sure what the preferred behavior is. But because the very last step responsible for sending the Slack message failed and not the testing job, each run submitted a report.

#78 @flixos90
2 months ago

@desrosj Thanks, that makes sense!

Ideally we would fix this so that the workflow would first check if there's already a record for the commit and then either not send it or overwrite the original record. But since this only happens in very specific situations like this (rerunning an already partially successful workflow), it's probably not the highest priority.

I created a tracking issue #62844 for it in any case.

#79 @johnbillion
2 months ago

In 59687:

Build/Test Tools: Coerce the run_id input to a string before passing it to the "Failed Workflow" workflow.

Follow-up to [59679].

See #62221

This ticket was mentioned in PR #8176 on WordPress/wordpress-develop by @johnbillion.


2 months ago
#81

#83 @johnbillion
8 weeks ago

Commit [59693] missed this ticket because I made a mistake with the ticket number.

This ticket was mentioned in PR #8190 on WordPress/wordpress-develop by @johnbillion.


8 weeks ago
#84

This ticket was mentioned in PR #8197 on WordPress/wordpress-develop by @desrosj.


8 weeks ago
#85

For some reason, Dependabot is not reporting these updates.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

#86 @desrosj
8 weeks ago

In 59716:

Build/Test Tools: Update 3rd-party GitHub Actions.

This updates the following GitHub Actions to their latest versions:

  • actions/cache
  • actions/checkout
  • actions/setup-node
  • actions/upload-artifact
  • codecov/codecov-action
  • shivammathur/setup-php

See #62221.

#88 @desrosj
7 weeks ago

In 59717:

Build/Test Tools: Correct input name for Code Coverage reports.

The input for providing files to the codecov/codecov-action was changed from file to files in version 5.0.0.

See #62221.

This ticket was mentioned in PR #8201 on WordPress/wordpress-develop by @desrosj.


7 weeks ago
#89

While it would be nice to always use *-latest images, it's proven to be problematic enough to consider pinning specific versions of runner images.

  • Because images are deployed over week long (sometimes month long) periods, it's not uncommon for workflows to have warning notices that GH adds for awareness that do not have any impact on the workflow's result. This makes it hard to separate legitimate failures/problems from these warnings.
  • As images slowly roll out, things can suddenly start to fail. See Core-62843 and Core-62808 as examples.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

#90 @desrosj
7 weeks ago

In 59720:

Build/Test Tools: Avoid using *-latest tags for runner images.

While using the ubuntu-latest, macos-latest, and windows-latest runner image tags is convenient, it has proven to be problematic in a number of instances as the runners are slowly updated (see #62808 and #62843).

This switches all workflows to using specific version tags representing the latest non-preview versions, which currently are as follows:

  • ubuntu-24.04
  • windows-2022
  • macos-14

Props swissspidy, johnbillion.
See #62221.

@johnbillion commented on PR #8190:


7 weeks ago
#92

@swissspidy @desrosj @sirreal @joemcgill What are your thoughts on this approach? The PR still needs some tweaks but the bulk of it is ready.

Latest results here: https://github.com/WordPress/wordpress-develop/actions/runs/13012122443

#93 @desrosj
7 weeks ago

In 59722:

Build/Test Tools: Adjust the check for runner type when creating a ZIP file.

Because the build process test workflow accepts an input for runner image, older workflows still use ubuntu-latest. This adjusts a conditional check to be more broad, allowing any ubuntu- image to match.

Follow up to [59720].

See #62221.

@swissspidy commented on PR #8190:


7 weeks ago
#94

The original intention of performing the tests sequentially in the same job was to reduce the chance that an environmental factor on GitHub Actions affects the comparison between the "current" and "before" tests that gets reported in a PR. I think in practice this is unlikely and uncommon

IIRC @dmsnell looked into that before and there can actually be quite some fluctuation, even depending on the time of day.

This ticket was mentioned in PR #8204 on WordPress/wordpress-develop by @desrosj.


7 weeks ago
#95

Dependabot updates have proven useful for tracking and updating 3rd-party GitHub Actions.

Without these updates configured, someone needs to notice there is a new release for a dependency, create a PR, create a Trac ticket, etc.

Previously, we have avoided managing npm dependencies through Dependabot for fear of too much noise. However with 52 devDependencies and 24 non-@wordpress/ dependencies (which are managed through the grunt sync-gutenberg-packages script), this is not sustainable or a good use of time when it can be _mostly_ automated.

To test this PR, I've gone and pushed this dependabot.yml file to my desrosj/wordpress-develop fork, enabled Dependabot updates, and downgraded every dependency in order to trigger an update PR. You can find the alerts in the open PR list.

I've organized all dependencies into groups of related functionality.

There are a few considerations though:

  • The Action workflows for some of the groups will almost always fail without contributor interaction in the current state. This is due to one of two things:
  • - The built script file that must have committed.
  • - The need to bump the version specified for the script in script-loader.php.
  • ` The need to change the expected version of a dependency in a test file.

For now, I think this is OK because someone can pick up the PR as a starting point and make the necessary final changes. In the future, we could add a GHA that detects when a built script file needs to be updated and automatically commit the difference for review. But once changes are made to a PR, Dependabot ignores it going forward. So that would need to be considered carefully.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

@desrosj commented on PR #8204:


7 weeks ago
#96

Also, ignore the Composer group in the dependabot.yml file. This was to test a group for #8137.

@swissspidy commented on PR #8204:


7 weeks ago
#97

Thanks for the thorough work looking into this!

Definitely a good opportunity to think about how to automate the rest too, like script-loader.php changes. Is there for example a Grunt script that updates the version based on package.json?

But once changes are made to a PR, Dependabot ignores it going forward. So that would need to be considered carefully.

Note that you can add [dependabot skip] to a commit message to continue having Dependabot rebase PRs. See https://docs.github.com/en/code-security/dependabot/working-with-dependabot/managing-pull-requests-for-dependency-updates#allowing-dependabot-to-rebase-and-force-push-over-extra-commits

@dmsnell commented on PR #8190:


7 weeks ago
#98

Thanks for the note, @swissspidy — I did in fact observe consistent variation when attempting to answer the question, “Does this release exhibit the same response times for the home page as the old release does?”

That variation existed on the order of _hours_ and while I’m sure there was an explanation, it was on an isolated test runner off Github and I did’t dive into figuring out what caused it. The impact of this variation was massive, however, significantly higher than any code changes, so my recommendation for performance testing is that run tests over the course of at least six to ten hours and interleave runs of the control and test groups so that we could avoid this long-running bias, in case our tests happen to run near the point where they appear or disappear.

For CI jobs where the results are more like suggestions or questions about performance than official or reliable reporting, I don’t see why this should be a blocker. The tests can be re-run. In fact, at some point I think I also considered running the tests concurrently since they only maxed out a single CPU core. Running on two or more cores would end up being a more realistic test environment, even though it introduces more uncertainty.

@desrosj commented on PR #8204:


7 weeks ago
#99

Definitely a good opportunity to think about how to automate the rest too, like script-loader.php changes. Is there for example a Grunt script that updates the version based on package.json?

There are several replace tasks configured for updating or replacing strings in PHP files. The tricky part is that the package names don't always map exactly to the script handle. For example, the polyfill-library dependency is used to build wp-polyfill-dom-rect and wp-polyfill-node-contains.

Note that you can add [dependabot skip] to a commit message to continue having Dependabot rebase PRs. See https://docs.github.com/en/code-security/dependabot/working-with-dependabot/managing-pull-requests-for-dependency-updates#allowing-dependabot-to-rebase-and-force-push-over-extra-commits

TIL! This would still be easy to forget. So worth keeping in mind when considering the contributor flow here. I think automation is the better option anyway.

@johnbillion commented on PR #8190:


7 weeks ago
#100

Thanks for the info Dennis! In that case I'll carry on with this PR (as it still needs work) because it seems that running the current/before/base tests in parallel won't affect the overall concern about the accuracy of the comparisons compared to what we have now.

There's a chance that they'll actually be more accurate because all three tests will start from a fresh installation, but looking back at the workflow runs on this PR it doesn't seem to make a measurable difference. But I'll continue looking at the numbers.

@joemcgill commented on PR #8190:


7 weeks ago
#101

The original intention of performing the tests sequentially in the same job was to reduce the chance that an environmental factor on GitHub Actions affects the comparison between the "current" and "before" tests that gets reported in a PR.

Originally, we only had two runs: "current" and "base", where the baseline is only run on commits to trunk (e.g., if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/trunk' }}) in order to have a consistent control measurement to send to the Code Vitals dashboard to normalize these fluctuations over time. Reducing side-effects from the test runner was the main reason these are being run in the same environment.

However, your observations about the way these are already potentially affecting each other by reusing the same DB is valid, i.e.:

This change removes the potential for to tests to interfere with one another.

In most cases, these tests are just being used as a spot check for folks reviewing the code if they suspect there might be a performance concern with a PR before it's committed, and for that use case, speeding up these runs seems much more valuable than isolating environmental side-effects. So running the "before" tests in parallel at minimum seems like a good idea.

For the base tests, I suspect that any commits that consistently lead to a major regression would still be visible in the dashboard if these are run in parallel, so it's probably worth the experiment to parallelize those as well, so I'm ok with us giving it a try and observing whether it negatively affects our ability to identify performance regressions during a release.

#102 @johnbillion
7 weeks ago

In 59749:

Build/Test Tools: Parallelise the performance tests.

This change introduces a job matrix for the "current", "before", and "base" performance tests to replace the current behaviour of running them sequentially in a single job. This speeds up the overall performance testing workflow and also reduces the chance of any given test interfering with another, for example by making a change to data in the database that affects a subsequent test.

Props johnbillion, swissspidy, dmsnell, joemcgill.

See #62221

This ticket was mentioned in PR #8242 on WordPress/wordpress-develop by @johnbillion.


7 weeks ago
#104

The npm run env:install command runs the install using the code in src regardless of whether LOCAL_DIR is set to build. 99% of the time this isn't a problem because the two codebases are usually functionally identical, but in the case of the performance tests on GitHub Actions the installation needs to run from the build directory because a different version of WordPress is downloaded into build.

This has always been the case but the effect was not noticed until the performance tests were split up in [59749]. This bug is preventing the performance tests for #7333 from running correctly.

#105 @johnbillion
7 weeks ago

In 59752:

Build/Test Tools: Fix the source code path handling when installing the local development environment.

This ensures the correct code is used to run the installation depending on whether it should be running from the src or build directory.

Props swissspidy, johnbillion

See #62221

This ticket was mentioned in PR #8252 on WordPress/wordpress-develop by @johnbillion.


6 weeks ago
#107

@audrasjb commented on PR #8252:


6 weeks ago
#108

Thanks for the ping @johnbillion. While I'm not the best person to checkout this proposal, I just wanted to ask a question about the "a new workflow which tests the upgrade process from the three latest major versions to the current branch" part: if this proposal is accepted, does it mean that we'll have to manually bump the related versions on each major release? This is not a blocker, but definitely something that will need some specific documentation on the major release handbook page :)

@johnbillion commented on PR #8252:


6 weeks ago
#109

@audrasjb Yes that's correct. We already have that in several other workflows, eg the performance tests, old branch tests, and upgrade tests. I'll make a note to get the major release handbook page udpated.

@johnbillion commented on PR #8252:


6 weeks ago
#110

we already generate a ZIP of the code in the current branch during the build test workflow

Yeah, if you take a look at the first few commits in this branch you'll see that I was initially trying to use that build, but it requires using the workflow_run trigger in order to make use of the build artifact once the workflow completes, and that trigger can't be tested in a PR because it only functions when the workflow with the trigger exists in the default branch. I tried workflow_dispatch but similarly this can't be tested in a PR from a fork. So I just went with another build.

I think this could be accomplished with some conditional checks, but we'd likely need to add a 3rd-party action like changed-files to know when to run each job

I understand your point but I'm not overly keen on adding further third party actions. Is a bunch of conditionals and an extra third party action better than an additional workflow? I don't think there's much between them.

@desrosj commented on PR #8204:


6 weeks ago
#111

I've opened #8260 to address some of the changes to built files required as a result of updating dependencies. It does not change the script versions in the unit tests or script-loader.php, but is a start.

@desrosj commented on PR #8204:


6 weeks ago
#112

I'm currently leaning towards #8260 combined with monitoring devDependencies only in dependabot.yml as a first iteration. That way it won't create PRs that will always fail, but it allows _some_ dependencies to be tracked more effectively.

This ticket was mentioned in PR #8284 on WordPress/wordpress-develop by @johnbillion.


6 weeks ago
#113

This ticket was mentioned in PR #8285 on WordPress/wordpress-develop by @johnbillion.


6 weeks ago
#114

WIP

#115 @desrosj
6 weeks ago

In 59797:

Build/Test Tools: Remove matchdep as a dependency.

matchdep was introduced in [25243] to more easily manage grunt-* dependencies. The package has effectively been abandoned upstream, and the functionality can be replaced with a simple loop.

Props desrosj, spacedmonkey, swissspidy.
See #62221.

@joemcgill commented on PR #8252:


5 weeks ago
#117

Thanks, @johnbillion. I think this is a good idea and things are looking good to me. Just to confirm one of my assumptions here, in addition to needing to bump the WP versions in the matrix passed for the reusable upgrade testing workflow, we'll also need to make sure the matrix stays up to date with the supported PHP and DB versions as that support changes over time, correct?

I also notice that the current matrix doesn't include any object cache tests. Is that intentional or do you think there would be some value in including at least one object cache test in the matrix?

@johnbillion commented on PR #8252:


5 weeks ago
#118

@joemcgill That's correct. We have that case already with the other upgrade tests. It's unfortunate, and perhaps going forward we could populate the matrix from .version-support-php.json and .version-support-mysql.json, but that would be better proposed in a separate PR I think.

Good spot on the object caching. Looking at the Installation Tests workflow there's a matrix entry for memcached but it's not actually used. The Upgrade Tests workflow doesn't include any handling for memcached.

Adding testing with and without memcached would require a change to the way these installation and upgrade workflow files work because they all use PHP directly on the host runner instead of using the containers. I'll open a follow-up ticket.

#119 @johnbillion
5 weeks ago

In 59815:

Build/Test Tools: Add a workflow that tests the process of upgrading WordPress to a build of the current branch.

This complements the existing workflow which tests upgrading to an already released version.

Props johnbillion, peterwilsoncc, mukesh27, desrosj, audrasjb, joemcgill.

See #62221

@joemcgill commented on PR #8204:


5 weeks ago
#122

We should definitely try to automate this as much as possible, so a big +1 from me. #8260 does look like a good start here. We should also consider ways that we could eliminate some of these manual tasks (rather than automating). Do you have an example of a test file that expects a specific dependency version to be defined?

@desrosj commented on PR #8204:


5 weeks ago
#123

We should also consider ways that we could eliminate some of these manual tasks (rather than automating).

Definitely agree the fewer tasks the better. Removing the script-loader-packages.php file from version control specifically came up in Trac-50460, but there has not been any traction on that yet.

Do you have an example of a test file that expects a specific dependency version to be defined?

Yes, the test_vendor_script_versions_registered_manually() test method will fail when script-loader.php is not updated to correctly reflect the right dependency version.

#124 @desrosj
5 weeks ago

In 59833:

Build/Test Tools: Update 3rd party actions in new workflow files.

Follow up to [59749].

See #62221.

@johnbillion commented on PR #7565:


4 weeks ago
#125

These actions were brought up to date in https://core.trac.wordpress.org/changeset/59716. Cheers!

This ticket was mentioned in PR #8436 on WordPress/wordpress-develop by @johnbillion.


3 weeks ago
#126

Prior to r59679 the commit message that was passed to the Slack API needed to be escaped so that backticks, double quotes, and dollar symbols were not interpreted by the shell within the GitHub Actions workflows. This was needed because the commit message was passed between jobs as output but used directly in the script using GitHub Actions expressions. In r59679 all instances of inline expressions were removed but the escaping was unnecessarily retained. This has resulted in backslashes preceding backticks, double quotes, and dollar symbols in messages sent to the Slack API and appearing in the test status messages that are shown in Slack.

This fixes that.

#127 @johnbillion
3 weeks ago

In 59920:

Build/Test Tools: Remove redundant escaping from the commit message that's passed to the Slack API.

This escaping is no longer needed since all instances of inline expressions were removed from workflows in r59679. The commit message can now be treated as a plain text string in an environment variable throughout the workflow.

Props johnbillion, swissspidy

Unprops johnblackbourn

See #62221

This ticket was mentioned in PR #8448 on WordPress/wordpress-develop by @johnbillion.


3 weeks ago
#129

Follow up to r59920. The commit message is now passed to this step as a plain string, not a JSON encoded string.

#130 @peterwilsoncc
3 weeks ago

In 59922:

Build/Test Tools: Remove redundant decoding from the commit message that's passed to the Slack API.

This JSON decoding is no longer needed since the commit message was converted to a plain string in r59920. The commit message can now be treated as a plain text string in an environment variable throughout the workflow.

Props johnbillion.
See #62221

#132 @desrosj
2 weeks ago

  • Owner set to desrosj
  • Status changed from new to assigned

This ticket was mentioned in PR #8492 on WordPress/wordpress-develop by @desrosj.


10 days ago
#133

This updates the concurrency:group to differentiate between runs using different input values.

This prevents the previous one from being cancelled when running upgrade tests for multiple versions. For example, 6.7.2, 6.6.3, etc. Because the workflow runs from trunk, the current configuration would cancel all ongoing workflows when a new one is created.

Trac ticket: https://core.trac.wordpress.org/ticket/62221

#134 @desrosj
10 days ago

In 59973:

Build/Test Tools: Allow upgrade tests for multiple versions.

This adds the new-version input to the concurrency group name to prevent manually dispatched workflow runs from cancelling each other when different versions are being tested.

See #62221.

This ticket was mentioned in PR #8260 on WordPress/wordpress-develop by @desrosj.


7 days ago
#136

This adds a workflow that automatically commits missing changes to versioned files managed through the build process when they are missed.

This was created primarily to help ensure necessary changes from Dependabot PRs are included so that various workflows pass, but it also helps contributors make sure they are not forgetting something.

The use of a GitHub App is required because of how permissions are limited for Dependabot PRs. Pushing to a branch created by Dependabot (or another fork) without the use of an app results in no GitHub Actions being triggered and defeats the purpose.

I have been testing this on my fork where Dependabot is currently configured to monitor all direct npm dependencies. View any Dependabot PR to see this workflow committing back changes to built files: https://github.com/desrosj/wordpress-develop/pulls

Trac ticket: https://core.trac.wordpress.org/ticket/62221

#137 @desrosj
7 days ago

In 59983:

Build/Test Tools: Introduce a workflow for checking built files.

There are several files generated and updated by the build process that are under version control. Including changes to these files is a common missed step for contributors regardless of experience level.

This introduces a workflow that checks for changes to versioned files as a result of other changes in pull requests and commits them back to the head branch. Because this workflow requires the pull_request_target event instead of pull_request, local references to reusable workflows should never be used.

In addition to improving the contributor experience, this also opens the door to use Dependabot for monitoring npm dependencies, many of which produce changes to built files when updating.

Props desrosj, johnbillion, joemcgill, swissspidy.
See #62221.

#138 @desrosj
7 days ago

In 59984:

Build/Test Tools: Use more specific (and correct) names for secrets.

Follow up to [59983].

See #62221.

@desrosj commented on PR #8260:


7 days ago
#139

Hmm, there's something different between my fork where I tested and wordpress-develop. I've configured everything the same way, but the token is unable to checkout the repository and I'm not sure why.

I've gone and disabled the workflow until someone is able to figure out why.

#140 @desrosj
4 days ago

In 60004:

Build/Test Tools: Correct E2E workflow path filtering.

The reusable E2E workflow does not yet follow the -v# pattern because there is only one version.

The current pattern is not matching the reusable-end-to-end-tests.yml file.

See #62221.

@desrosj commented on PR #8260:


4 days ago
#141

So this is an issue with PRs coming from forks, which I couldn't test in my own fork.

There are two issues:

  • The workflow is checking out WordPress/wordpress-develop, not the repository the PR originates from.
  • The ref is set to trunk because pull_request_target workflows originate from the base branch.

Adjusting the inputs for the actions/checkout step to the following _should_ fix the issue, but there's no way of knowing until it's committed.

repository: ${{ github.event.pull_request.head.repo.full_name }}
ref: ${{ github.event.pull_request.head.ref }}

The only unknown here is whether the GitHub App will be able to act in the same manner as maintainers, given the PR author has checked "Allow edits and access to secrets by maintainers".

#142 @desrosj
2 days ago

In 60051:

Build/Test Tools: Update 3rd party actions.

This updates the following 3rd party actions:

  • actions/setup-node from 4.2.0 to 4.3.0
  • actions/upload-artifact from 4.6.0 to 4.6.1
  • ramsey/composer-install from 3.0.0 to 3.1.0
  • actions/cache from 4.2.0 to 4.2.2
  • actions/download-artifact from 4.1.8 to 4.1.9
  • codecov/codecov-action from 5.3.1 to 5.4.0

See #62221.

#143 @desrosj
2 days ago

In 60052:

Build/Test Tools: Use ‘pull_request.head.ref` when checking built files.

Because pull_request_target happens in the context of the base branch, attempting to checkout github.head_ref results in a failure when the workflow comes from a fork.

This adjusts the options passed to actions/checkout to use the repository that actually contains the test branch.

See #62221.

#144 @desrosj
2 days ago

In 60053:

Build/Test Tools: Increase timeout value for Slack message jobs.

Because of how dependent jobs and queuing works in GitHub Actions, it’s common for workflows to be completed with the exception of sending Slack notifications.

“The waiting is the hardest part”, and occasionally these jobs hit the 10 minute timeout value before their turn comes up.

See #62221.

#145 @desrosj
25 hours ago

In 60059:

Build/Test Tools: Use the wordpress-develop-pr-bot app for changes.

This switches to using the wordpress-develop-pr-bot GitHub app when authoring commits instead of masquerading as Dependabot.

Follow up to [59983], [60052].

See #62221.

Note: See TracTickets for help on using tickets.