Opened 5 months ago
Last modified 25 hours ago
#62221 assigned task (blessed)
GitHub Actions updates and improvements for 6.8
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.8 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Attachments (1)
Change History (147)
This ticket was mentioned in PR #7565 on WordPress/wordpress-develop by @ayeshrajans.
5 months ago
#1
- Keywords has-patch added
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.
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.
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. 👍
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
4 months ago
#18
Looks like the correct value to use is dependabot[bot]
.
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
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
@
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.
This ticket was mentioned in PR #7960 on WordPress/wordpress-develop by @desrosj.
4 months ago
#29
Follow up to Core-59484.
Trac ticket: https://core.trac.wordpress.org/ticket/62221
#30
@
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?
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
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.
This ticket was mentioned in PR #8007 on WordPress/wordpress-develop by @johnbillion.
3 months ago
#44
This ticket was mentioned in PR #8010 on WordPress/wordpress-develop by @desrosj.
3 months ago
#47
Trac ticket: https://core.trac.wordpress.org/ticket/62221
This ticket was mentioned in PR #8011 on WordPress/wordpress-develop by @desrosj.
3 months ago
#49
Trac ticket: https://core.trac.wordpress.org/ticket/62221
3 months ago
#50
Merged into trunk
in https://core.trac.wordpress.org/changeset/59519.
2 months ago
#59
Merged in Core changes 59583,59585-59587
@johnbillion commented on PR #7971:
2 months ago
#60
Closing in favour of https://github.com/WordPress/wordpress-develop/pull/8007
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
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
@johnbillion commented on PR #8007:
2 months ago
#69
Committed in: https://core.trac.wordpress.org/changeset/59679
Follow-up commit: https://core.trac.wordpress.org/changeset/59681
#75
follow-up:
↓ 77
@
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?
@
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
@
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
@
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.
@johnbillion commented on PR #8174:
2 months ago
#80
Committed in https://core.trac.wordpress.org/changeset/59687
This ticket was mentioned in PR #8176 on WordPress/wordpress-develop by @johnbillion.
2 months ago
#81
@johnbillion commented on PR #8176:
8 weeks ago
#82
Committed in https://core.trac.wordpress.org/changeset/59693
#83
@
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
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
@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
@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
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
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.
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 onpackage.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.
@johnbillion commented on PR #8190:
7 weeks ago
#103
Committed in https://core.trac.wordpress.org/changeset/59749
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.
@johnbillion commented on PR #8242:
7 weeks ago
#106
Committed in https://core.trac.wordpress.org/changeset/59752
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.
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.
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
@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.
@johnbillion commented on PR #8252:
5 weeks ago
#120
Committed in https://core.trac.wordpress.org/changeset/59815
@johnbillion commented on PR #8252:
5 weeks ago
#121
@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?
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.
@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.
@johnbillion commented on PR #8436:
3 weeks ago
#128
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.
@peterwilsoncc commented on PR #8448:
3 weeks ago
#131
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
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
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.
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 totrunk
becausepull_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".
Trac ticket: https://core.trac.wordpress.org/ticket/62221