GH Actions: enable running the tests on PHP 8.1

[jrf]

PHP 8.1 is expected to be released end of November 2021.
Enabling the tests to run in CI on PHP 8.1 will allow us to get WordPress ready in time.

To allow for enabling the tests to run on PHP 8.1, quite some prep work needs/needed to be done ahead of the actual CI change.

Preparation work already committed:

• #47381 - removing of composer.lock file and switching to running the tests via Composer.
• #46149 - making the test suite cross-version compatible with PHPUnit 5.x to 9.x
• #53635 - [51517] patch JsonSerializable_Object::jsonSerialize() for compatibility with PHP 8.1

Preparation work in progress:

• #52825 - fix mysqli error handling for PHP 8.1.
• #53858 - fix parse error in PHP 8.1 due to readonly

Patches for these last two tickets need to be committed before PHP 8.1 in CI can be turned on as these are both causing fatal errors in the "Install WordPress" step before the actual test run.

I'll be attaching a patch to this ticket for the CI change, but this patch can/should only be committed once the patches for the above two tickets are in.

Once the tests are running, the application of fixes for PHP 8.1 will be handled in #53635 and related tickets.

PHP 8.1 is expected to be released end of November. Enabling the tests to run in CI on PHP 8.1 will allow us to get ready in time.

These builds are, for now, still allowed to fail.

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

[jrf]

I've opened ​PR 1553 with the patch as uploaded and (partial) patches from the two tickets which still need to be committed to demonstrate/verify the CI on PHP 8.1 will work with those two changes.

The fact that the tests fail is only to be expected for now.

Once the patch is committed, all builds will show as "failed" until all PHP 8.1 issues have been fixed, which has the risk of people not noticing new failures on PHP < 8.1.

As an interim measure, while working through the PHP 8.1 issues, we could add separate conditional steps to run the tests on PHP 8.1 and add continue-on-error to those. That would allow the test builds to show as "successful" if all non-PHP 8.1 test runs pass.

Opinions ? /cc @desrosj

As an interim measure, while working through the PHP 8.1 issues, we could add separate conditional steps to run the tests on PHP 8.1 and add continue-on-error to those. That would allow the test builds to show as "successful" if all non-PHP 8.1 test runs pass.

That makes sense to me 👍

As an interim measure, while working through the PHP 8.1 issues, we could add separate conditional steps to run the tests on PHP 8.1 and add continue-on-error to those. That would allow the test builds to show as "successful" if all non-PHP 8.1 test runs pass.

One other consideration I've just learned from @aristath is that if WordPress core tests fail, Gutenberg contributors cannot merge any PRs, so definitely let's do that :)

• Rebased after the patches for the blockers had been merged.
• Updated to apply my earlier suggestion.

You can see that the builds against PHP 8.1 now show as 🟢 , even though when you open them up, you will see that all test runs have failures.
The failures are also listed on the CI "Summary" overview of the "PHPUnit Tests" workflow.
See: ​https://github.com/WordPress/wordpress-develop/actions/runs/1113845520

[SergeyBiryukov]

In 51588:

Build/Test Tools: Enable running the tests on PHP 8.1.

PHP 8.1 is expected to be released at the end of November 2021.

Enabling the tests to run in CI on PHP 8.1 allows us to get WordPress ready in time.

As an interim measure, while working through the PHP 8.1 issues, separate conditional steps are added to run the tests on PHP 8.1 with the continue-on-error option. That allows the test builds to show as "successful" if all non-PHP 8.1 test runs pass.

Follow-up to [51517], [51543], [51545], [51574], [51582], [51586].

Props jrf.
Fixes #53891. See #53635.

Closing as committed in 51588

[swissspidy]

Regarding r51588, couldn't the test workflow be simplified by just using continue-on-error: ${{ matrix.php == '8.1' }} instead of duplicating all steps?

[jrf]

Replying to swissspidy:

Regarding r51588, couldn't the test workflow be simplified by just using continue-on-error: ${{ matrix.php == '8.1' }} instead of duplicating all steps?

@swissspidy Please see the above discussion. With that (which was exactly what the initial commit on GH did), builds/PRs would still show as failed, even though it is only the PHP 8.1 builds which are failing.

Trac ticket:

[desrosj]

Replying to swissspidy:

Regarding r51588, couldn't the test workflow be simplified by just using continue-on-error: ${{ matrix.php == '8.1' }} instead of duplicating all steps?

I also came here to suggest this.

I did some testing, and I was able to get this to work. It looks like 53891-ghactions-change.patch​ added continue-on-error at the job level. I did not retry this approach, though ​it is documented as supported. Instead I included continue-on-error inline for each step that should be allowed to fail for PHP 8.1. This makes what can fail more specific, and could help us debug other parts of the workflow on PHP 8.1.

Thanks @jrfnl! I removed the PHP 8.1 check, but I did not add the continue-on-error for that step.

Slow tests are currently only split out for PHP 5.6. If PHP 8.1 started running slow tests, that is something that should get flagged.

@desrosj Understood, though as split_slow is a matrix condition which _could_ be added anywhere, that means there's an undocumented assumption in the workflow that it will not be added for the PHP 8.1 workflows. So, in that sense, as that step does not have a PHP version based condition, it would still be _safer_ to add the continue-on-error to it, just in case.

Either way, with a bit of luck, these conditions will only be in the workflow for a month or two at the most, so no biggie.

I don't have a strong preference. @SergeyBiryukov care to be the tie breaker on this if you're able to commit?

I don't have a strong preference either, but as the split_slow condition is specific to PHP 5.6 per [50444], it seems like a safe assumption that it will not be added for the PHP 8.1 workflows. So the PR as is seems good to me :)

[SergeyBiryukov]

In 51604:

Build/Test Tools: Simplify the PHPUnit test workflow.

This removes the previously duplicated set of test runs specifically for PHP 8.1 in favor of combining if conditions for individual test runs with the continue-on-error option.

Follow-up to [51588].

Props desrosj, swissspidy, jrf.
Fixes #53891.

Merged in https://core.trac.wordpress.org/changeset/51604.