Make WordPress Core

Opened 5 weeks ago

Last modified 2 weeks ago

#62004 accepted task (blessed)

Test suite: update the tests for PHPUnit 10/11 and get ready for PHPUnit 12

Reported by: jrf's profile jrf Owned by: hellofromtonya's profile hellofromTonya
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description

Setting the scene

At this moment, the WP Core PHPUnit tests are run on PHPUnit 8 and 9 with the PHPUnit Polyfills 1.x.

PHPUnit 10 was released in February 2023, PHPUnit 11 in February 2024 and PHPUnit 12 is expected in February 2025.

PHPUnit 10 and 11 are the only actively supported versions of PHPUnit at this time, though PHPUnit 8 and 9 still have "life" support, which means they are being made compatible with new PHP versions for as long as feasible.

This means that, while it is not currently urgent to make the test suite compatible with PHPUnit 10/11, it is desirable, as the further we fall behind, the bigger the mountain of work to get through to allow for an update becomes.

PHPUnit 10 and 11 introduced a LOT of changes, quite a few of which are problematic for the WP test suite.
Having said that, some of those problems have, in the mean time, been mitigated in later PHPUnit 10.x/11.x versions (for more details, keep reading).

The aim of this ticket is to give a detailed overview of the various problems (and their potential solutions) and to get the work started to prepare the WP test suite for an upgrade to support PHPUnit 11 and, once released, PHPUnit 12.

Why is this ticket being opened now ?

As PHPUnit 11.0 now includes expectUserDeprecation*() methods, which are polyfilled via the PHPUnit Polyfills 3.0 release, PHPUnit 11.1 includes a CoversMethod attribute and PHPUnit 10.5.32+/11.3.3+ now suppress PHPUnit native deprecations by default, it has now, finally, become viable for the WordPress test suite to be updated to be compatible with PHPUnit 11 and eventually PHPUnit 12, while still keeping compatibility with PHPUnit 8 and 9 to test against PHP 7.2 - 8.1. (Yes, we will be "skipping"/excluding PHPUnit 10, more about this below.)

While, with the changes in PHPUnit 10.5.32/11.3.3, we may be able to get onto PHPUnit 11 in the foreseeable future, making the test suite compatible with PHPUnit 12 will require quite a momenteous amount of work, which should not be underestimated and this work will need to be started now if we ever want to get onto PHPUnit 12.

Consider this a "meta"-ticket which will never be addressed in one WP version. Sub-tickets can/should be opened for individual sub-tasks for this ticket and the status of those reported back to this one to keep a grip on the total progress of this project.

Below is an analysis of the problems which will need to be addressed.

For those who follow the PHPUnit Polyfills and noticed that the 3.0 release does not support PHPUnit 10:
That does not mean WP couldn't support PHPUnit 10 by using a ^2.0 | ^3.0 requirement for the polyfills, so the below analysis is still applicable and relevant.

Outline of the problems we need to address

Problem 1: configuration file

PHPUnit 9.3 and later 10.0 and 10.1 introduced changes in the PHPUnit XML configuration file.

While there is a --migrate-configuration option available, this is not sufficient for our needs, as it doesn't migrate settings on the top-level phpunit element, so we'll need to introduce separate PHPUnit configuration files for PHPUnit < 10 (current config) and PHPUnit 10.1+.

This also means that in CI, we'll need extra steps to figure out the PHPUnit version being used in a build, to determine which PHPUnit config file should be used to run the tests.

We may also need to have a conditional (and continue-on-error) step to --migrate-configuration to mitigate any other small changes in the configuration files between PHPUnit versions for PHPUnit 10+.

Also note: the fact that we'll need different configuration files will make it more complex for contributors to run the tests, so this needs good documentation via Composer scripts + in the Make Core handbook.

Problems 2: PHPunit deprecations fail the test suite

The WordPress test suite should fail when PHP native deprecations/notices/warnings are encountered and by default, this functionality is turned off in PHPUnit 10 and higher.

Additionally, even when the related PHPUnit configuration options are turned on, this functionality has been implemented differently in PHPUnit 10+ vs PHPUnit < 10, with the most problematic difference being that in PHPUnit 9 and lower, when requesting a test suite to fail on deprecation notices, it would only fail on PHP deprecation notices, while in PHPUnit 10 and higher, it will fail on both PHP deprecation notices as well as PHPUnit deprecation notices, which, in effect, means that any new PHPUnit release can now make the test suite fail, as we do still want the test suite to fail on newly introduced PHP deprecation notices.

[Update] Now, luckily I've managed to convince PHPUnit of the undesirable side-effects of this change.

This means that, as of PHPUnit 10.5.32 and 11.3.3 (released this week), PHPUnit native deprecation notices will be suppressed by default and PHPUnit native deprecations will no longer affect the exit code of a test run.

The deprecation notices can still be seen/fail a test run using the new --display-phpunit-deprecations --fail-on-phpunit-deprecation flags and these options should be used when working on upgrading the test suite for a new (future) PHPUnit version.

For WP, this sets the minimum viable PHPUnit versions to run the tests on PHPUnit 10/11 to PHPUnit 10.5.32 and 11.3.3.

Problem 3: annotations are deprecated

PHPUnit 10 introduced attributes to replace annotations, like @covers. As of PHPUnit 11 using annotations is hard deprecated and the use of annotations will no longer be supported in PHPUnit 12.

As of PHPUnit 11, PHPUnit throws deprecation notices about the use of annotations when they are not accompanied by an attribute.

As per problem 2, these deprecation notices will be suppressed on PHPUnit 10.5.32+ and 11.3.3+.
This means that we don't necessarily need to address this deprecation yet, but we still will need to get ready to fix these before we can even consider supporting PHPUnit 12.

It is expected that some PHP_CodeSniffer sniffs will become available to help with auto-fixing annotations to attributes at some point in the future (in time for WP to use these).

I recommend doing the complete conversion in one go for the whole test suite once the sniffs are available.

However, the attributes are not always a 1-on-1 replacement, which brings us to problem 4....

Problem 4: covers attributes are only supported at class level (x2)

There are actually two problems with this:

  1. A test class can only have Covers* attributes at class level, so each test class should only contain tests targetting the same "code under test".
  2. Prior to PHPUnit 11.1, Covers* attributes only allowed for either covering a global function or a complete class. Covers* attributes did not allow for tests only targetting one method in a class (or a few select methods).

For the first part, this effectively means that each test class should only test one "unit" of code (one global function or one class) as otherwise we will not be able to get the correct code coverage recorded.

This means that the essence of ticket #53010 - splitting up a lot of the test classes to smaller test classes each containing tests for only one specific unit of code - now needs to be addressed with higher priority and needs to be addressed in full before supporting PHPUnit 12 can even be considered.

For the second part, we have a whole other problem and that is that a LOT of the code in WordPress is not SOLID, meaning that classes are not limited to one responsibility and do too much.

This means that the CoversClass attribute is insufficient for our needs as it would lead to a LOT of code being unintentionally marked as covered, while there are no dedicated tests for this code. So we'll need PHPUnit 11.1 with the CoversMethod attribute as a minimum to be able to set the code coverage markers correctly for tests targetting methods.

Yes, this also means that we don't just need to split the test classes up by class, in actual fact, we'll need to split them up to individual test classes for each method in a class to still be able to measure code coverage correctly. (that is, unless the class is SOLID, but that will be rare and an exception in the WP context)

And to be extra clear: this also means that @coversDefaultClass at class level with @covers tags at method level, is also no longer supported.

Explainer: why does this mean we need to skip PHPUnit 10.x and 11.0 ?

We need to use annotations on PHPUnit 8 + 9, both for code coverage, but also for things like @requires, @ticket etc.
Annotations will still be respected in PHPUnit 10 and 11 as long as no attributes are being used on the same test class/method.
So on PHPUnit 10 and 11 we can still get away with using annotations.

With the removal of support for annotations in PHPUnit 12 (see problem 3), that is no longer possible, so we need to duplicate all annotations as attributes.
Yes, duplicate as we need the annotations for PHPUnit 8 + 9 and the attributes for PHPUnit 10, 11 and 12.

We can't do this only partially, this has to be done in one go for the complete test code base as PHPUnit will not read annotations when attributes are found on a test class, so the duplication needs to be done in one go for all annotations.

However, not all attributes we need are available in PHPUnit 10.x and 11.0, so we can't add all the attributes we need unless we set the PHPUnit requirement to ^8.0 || ^9.0 || ^11.1 (well, ^11.3.3 as we also need the change in PHPUnit native deprecation notice handling).

Problem 5: none of the test classes comply with PHPUnit naming conventions

PHPUnit 9.0.0 stopped support for multiple test case classes (classes that extend TestCase) that are declared in a single source-code file.

Note: combining a non-test class (test fixture) and a test class in one file is still allowed.

PHPUnit 9.1.0 deprecated support for test case class names not matching the file name.

As things are, none of the WordPress test classes comply with the naming conventions expected by PHPUnit and while tests will still run on PHPUnit 9 (and 10 for that matter), this is no longer the case on PHPUnit 11 where any and all tests which do not comply with the naming conventions will no longer run.

For automatic recognition of the tests, test classes have to follow the following naming conventions:

  1. Class name [SomethingUnderTest]Test - take note of the Test suffix.
  2. The file name for the test class MUST match the class name EXACTLY in a case-sensitive manner.

In other words, test file names have to comply largely with PSR4 file name regulations, though underscores in the names are still allowed.

With some config tweaks, the first rule could be bypassed (for a limited set of possible name patterns), however, the second rule cannot be bypassed.

As all test classes will need to be touched/changed/split to address problem 4 anyway, I would strongly recommend for the new test classes to be made fully compatible with the PHPUnit naming conventions.
This will prevent having to re-do this exercise yet again in the future if the bypass for the first rule would be made impossible.

What this means in practice (example):

In the current situation, the test class Tests_Compat_ArrayKeyLast is in a file called tests/compat/arrayKeyLast.php.
This class will need to be renamed to Compat_ArrayKeyLast_Test and the file will need to be renamed to tests/compat/Compat_ArrayKeyLast_Test.php.

Or, even better, we could choose to use namespaces in the tests (also see ticket #53010) and to rename the class to WordPress\Tests\Compat\ArrayKeyLastTest and the file to tests/compat/ArrayKeyLastTest.php.
Optionally, this can then be combined with a PSR4 test autoload directive in the composer.json file.

Problem 6: expecting PHP native deprecations/notices/warnings

While PHPUnit 11.0 re-introduces functionality to expect our "own" deprecations (E_USER_DEPRECATED) again via the expectUserDeprecation*() methods, it still does not allow for expecting PHP native deprecations/notices/warnings, nor does it allow for expecting our "own" notices and warnings.

This will need to be handled for select tests via a custom error handler, though it should be strictly evaluated which tests using those expectations should use the custom error handler and for which tests the underlying problem (if it is a PHP native deprecation/notice/warning) should be addressed instead.

Also note that all test methods which use expectUserDeprecation*() will need the #[IgnoreDeprecations] attribute and all tests which use a custom error handler, will need the #[WithoutErrorHandler] attribute and the PHPUnit native error handling will be disabled completely for that test method.
Using these attributes may have unintended side-effects, so these should only be used if it can't be avoided!

Problem 7: expecting WP native deprecations and "doing it wrong" notices

The WP_UnitTestCase_Base contains functionality to test WP native deprecations and notices created via function calls to the _deprecated_*() functions and _doing_it_wrong().

This functionality largely works via a custom WP native @expectedDeprecated annotation.

I expect that this functionality will no longer work on PHPUnit 11 for two reasons:

  1. It is annotation based, while annotations are deprecated.
  2. It hooks into PHPUnit internal functionality - \PHPUnit\Util\Test::parseTestMethodAnnotations() and this functionality no longer exists in PHPUnit 11.

The most likely solution direction will be to use Events on PHPUnit 10 and up, but this would not be cross-version compatible with PHPUnit 8 and 9.

Further investigation is needed to determine how this can be solved in a cross-version compatible manner.

Problem 8: open tickets with patches adding tests will be invalidated

All currently open patches which add tests will need to get the same updates as listed in this ticket before the patch can be committed to Core (once this ticket has been addressed).

This should not be considered a blocker, but committers are recommended to take the problems outlined in this ticket into account when reviewing new test patches to prevent compounding the problems listed in this ticket.

In that respect, it would be most opportune to yes, take steps to prepare for PHPUnit 11/12 now, especially by addressing problem 4 and 5 (splitting of the test classes and file renames), but no, don't allow for PHPUnit 11/12 yet until there are sniffs available which will flag a number of the listed problems as otherwise a lot of the issues outlined in this ticket can be re-introduced a little too easily without the test suite failing on it.

Additionally, a detailed Make post will be needed once this ticket has been fully addressed to a) inform Core contributors, b) inform maintainers of test suites build on top of the WP native test suite, like plugin integration test suites.
This Make post will also need to include a call to contributors with open patches which include tests, to update their patches to comply with the new reality.

Other tasks

Aside from the above, the "normal" upgrade tasks for updating a test suite to be compatible with PHPUnit 11/12 will also need to be executed.

Keep in mind, the PHPUnit Polyfills provide forward-compatibility for test suites, not backward compatibility.
This means that functionality which is no longer supported by PHPUnit itself, will also not be supported by the PHPUnit Polyfills.

Both PHPUnit 10, as well as PHPUnit 11, remove various features which are in use in the WordPress Core test suite.
PHPUnit 12 is expected to do so again, but the exact specs are not yet known.

For each of this removed functionality, it will need to be considered if the functionality should get a polyfill in the WP test suite or (preferable) if the tests should be refactored to no longer use that functionality.

Preliminary list of "Normal" upgrade tasks

Disclaimer: this list may not be complete.

  • All data provider methods will need to be declared as static. There will likely be a sniff available in time which can handle this upgrade.
  • All data provider methods will need to be declared as public. There will likely be a sniff available in time which can handle this upgrade (in as far as necessary as I don't expect there to be any non-public data providers).
  • Data provider methods can no longer take parameters. I doubt this will cause issues for the WP test suite, but should be checked anyway. There will likely be a sniff available in time which can flag this.
  • Replace $backupGlobals and $backupStaticAttributes properties, which may be declared in test classes with annotations, combined with their corresponding attributes.
  • Replace all uses of Assert::assertObjectHasAttribute() and Assert::assertObjectNotHasAttribute() with Assert::assertObjectHasProperty() and Assert::assertObjectNotHasProperty() respectively.
  • Replace some WP native custom assertion methods with the new, PHPUnit native (and polyfilled) Assert::assertStringEqualsStringIgnoringLineEndings() and Assert::assertStringContainsStringIgnoringLineEndings() assertions.
  • Check if any tests were refactored/work arounds were introduced for the PHPUnit 9 removed assertArraySubset() functionality. This can now likely be replaced with the new PHPUnit 11 (and polyfilled) assertArrayIs[Equal|Identical]ToArray[OnlyConsidering|Ignoring}ListOfKeys() assertions.
  • Verify if any of the below removed functionality is in use in the WP test suite and either refactor the affected tests or provide a polyfill for this functionality:
    • assertStringNotMatchesFormat() and assertStringNotMatchesFormatFile()
    • MockObject::withConsecutive()
    • TestCase::setOutputCallback()
    • TestCase::returnValue(), TestCase::onConsecutiveCalls(), TestCase::returnValueMap(), TestCase::returnArgument(), TestCase::returnSelf(), and TestCase::returnCallback()
    • expect[Deprecation|Notice|Warning|Error]*() (see problem 4 above)
    • MockBuilder::setMethods(), MockBuilder::setMethodsExcept(), MockBuilder::addMethods(). These can, in most cases, be replaced with use of MockBuilder::onlyMethods() (PHPUnit 8+).
    • TestCase::getMockClass(), TestCase::getMockForAbstractClass(), TestCase::getMockFromWsdl(), TestCase::getMockForTrait(), TestCase::getObjectForTrait(), TestCase::iniSet(), TestCase::setLocale(), TestCase::createTestProxy()
    • MockBuilder::getMockForAbstractClass(), MockBuilder::getMockForTrait(), MockBuilder::enableProxyingToOriginalMethods(), MockBuilder::disableProxyingToOriginalMethods(), MockBuilder::setProxyTarget(), MockBuilder::allowMockingUnknownTypes(), MockBuilder::disallowMockingUnknownTypes(), MockBuilder::enableAutoload(), MockBuilder::disableAutoload(), MockBuilder::enableArgumentCloning(), MockBuilder::disableArgumentCloning()
  • Investigate the impact on the test suite configuration of the below PHPUnit 11 change:

    A test can no longer be part of multiple test suites that are configured in the XML configuration file

  • PHPUnit commands used in CI and documentation using any of the below listed arguments will need to be updated/have a toggle for PHPUnit 11. Previously those arguments accepted a comma-separated value. As of PHPUnit 11.1, these commands need to be repeated for each value.
    • --group
    • --exclude-group
    • --covers
    • --uses
    • --test-suffix

Refs:

Planning/Steps to get there

Steps which will need to be taken to make the test suite compatible with PHPUnit 11 and 12:

Prerequisite for PHPUnit 12

Step 1 to 10.000: Split and rename most test classes (above listed problem 4 + 5 / Trac ticket #53010). (yes, do not underestimate this step and it is a 100% required before the test suite can become compatible with PHPUnit 12).

Upgrade tasks

  • Make all the necessary upgrades to the tests to handle other deprecated/removed functionality, as per the "normal" upgrade task list.
  • Add the PHPUnit attributes (while leaving the annotations in place for PHPUnit < 10).
  • Start using PHPUnit Polyfills 3 or 4, depending on the timing.
    • Version 3 of the PHPUnit Polyfills has a minimum PHP requirement of PHP 7.0 and supports PHPUnit 6.x - 9.x + 11.x, which is sufficient for our needs for now.
    • Version 4 of the PHPUnit Polyfills is expected to have a PHP 7.1 minimum and to support PHPUnit 7.x - 9.x + 11.x - 12.x.
  • Explicitly require-dev PHPUnit in the Composer file again. This needs to be done for the following reason:
    • While the PHPUnit Polyfills 3.x support "phpunit/phpunit": "^6.4.4 || ^7.0 || ^8.0 || ^9.0 || ^11.0", WordPress will need to support a subset of this due to the CoversMethod functionality missing on PHPUnit < 11.1 and deprecation suppression not being available on PHPUnit < 11.3.3.
  • Introduce a second PHPUnit configuration file to handle the differences in the XML for PHPUnit <= 9 vs PHPUnit 11/12.
    • I'd recommend making the default phpunit.xml.dist file, the configuration file for PHPUnit 11/12 and introducing a new phpunit89.xml.dist file for PHPUnit 8 - 9. This will keep us forward-compatible.
    • Having a second configuration file also means changes will need to be made in the Composer scripts and the GitHub Actions scripts (and possibly in more places).
  • Write a Make post outlining the changes made and the upgrade path for external test suites build on top of the WP native test suite (like plugin integration tests).

I'd be happy to do the initial pass to get these upgrade tasks done, but I think it makes most sense to only do this once the prerequisite for PHPUnit 12 has been fullfilled.

Final notes

As a final note, I'd like to point out that due to the huge amount of work which is needed to address the prerequisite of splitting the test classes, it is likely that by the time this ticket can be addressed, PHPUnit 12 (and possibly even PHPUnit 13) will have been released.

If that's so, this does not need to be a blocker and the tasks outlined in this ticket can still be addressed and continued with as outlined above.

Depending on what will be included/removed in PHPUnit 12/13, it will need to be evaluated whether support for PHPUnit 12/13 can be included in the final update work for this ticket or if this needs to be addressed separately at a later point in time in its own ticket.

N.B.: This ticket replaces the earlier ticket #59486 about this topic.

Change History (10)

This ticket was mentioned in Slack in #core-php by jrf. View the logs.


5 weeks ago

This ticket was mentioned in Slack in #core-test by jrf. View the logs.


5 weeks ago

This ticket was mentioned in Slack in #core-committers by jrf. View the logs.


5 weeks ago

#4 @desrosj
4 weeks ago

  • Milestone set to Future Release

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


4 weeks ago

This ticket was mentioned in Slack in #hosting by javier. View the logs.


4 weeks ago

#7 follow-up: @hellofromTonya
3 weeks ago

  • Owner set to hellofromTonya
  • Status changed from new to accepted

As I've been working closely with @jrf in PHPUnit Polyfills and this epic ticket's scope, self-assigning to help co-lead this massive effort.

Why is it not assigned to a specific milestone?

It will span multiple WordPress releases (milestones). Thus, it does make sense to not assign it to current milestone.

That said, work will commence with commits landing in whatever the current milestone is.

#8 @pbearne
3 weeks ago

Let me loose on this :-)

#9 @pbearne
3 weeks ago

Hi All

I would like to propose that we create a new folder (wp) in PHPUnit/tests and put the updated and namespace test into that folder ( PHPUnit/tests/wp ) so that we can tell what is done and test to be done.

We can remove it once finished if needed.

paul

#10 in reply to: ↑ 7 @azaozz
2 weeks ago

Replying to hellofromTonya:

I've been working closely with @jrf in PHPUnit Polyfills and this epic ticket's scope, self-assigning to help co-lead this massive effort.

Yea, seems another "massive effort" is needed to update/make it possible to use PHPUnit in WP :(
Wondering if this will ever end? Seems it won't unless we fork it, perhaps?

Also see https://core.trac.wordpress.org/ticket/53010#comment:47.

Note: See TracTickets for help on using tickets.