[wp-trac] [WordPress Trac] #62004: Test suite: update the tests for PHPUnit 10/11 and get ready for PHPUnit 12
WordPress Trac
noreply at wordpress.org
Sat Sep 7 01:29:48 UTC 2024
#62004: Test suite: update the tests for PHPUnit 10/11 and get ready for PHPUnit 12
------------------------------+--------------------
Reporter: jrf | Owner: (none)
Type: task (blessed) | Status: new
Priority: normal | Milestone:
Component: Build/Test Tools | Version:
Severity: normal | Keywords:
Focuses: |
------------------------------+--------------------
= 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 [https://phpunit.de/supported-versions.html
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 [https://github.com/Yoast/PHPUnit-
Polyfills/releases/tag/3.0.0 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
[https://github.com/sebastianbergmann/phpunit/issues/5937 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
[https://github.com/sebastianbergmann/phpunit/issues/5175#issuecomment-1966727421
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 [https://github.com/sebastianbergmann/phpunit/issues/4105
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:
* [https://phpunit.de/announcements/phpunit-10.html PHPUnit 10 release
post]
* [https://phpunit.de/announcements/phpunit-11.html PHPUnit 11 release
post]
= 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.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/62004>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list