Opened 6 years ago
Closed 3 years ago
#46149 closed task (blessed) (fixed)
PHPUnit 8.x support
Reported by: | SergeyBiryukov | Owned by: | jrf |
---|---|---|---|
Milestone: | 5.9 | Priority: | high |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | has-patch has-unit-tests early php8 has-dev-note |
Focuses: | Cc: |
Description (last modified by )
Background: #43218
PHPUnit 8.0.0 is scheduled for February 2019. The changelog includes some backwards compatibility breaks.
From the PHPUnit 7 announcement (February 2, 2018):
Looking Forward
The following methods do not have a return value and should therefore have a
void
return type declaration:
PHPUnit\Framework\TestCase::setUpBeforeClass()
PHPUnit\Framework\TestCase::setUp()
PHPUnit\Framework\TestCase::assertPreConditions()
PHPUnit\Framework\TestCase::assertPostConditions()
PHPUnit\Framework\TestCase::tearDown()
PHPUnit\Framework\TestCase::tearDownAfterClass()
PHPUnit\Framework\TestCase::onNotSuccessfulTest()
These methods will have a
void
return type declaration in PHPUnit 8. Please declare your methods that overwrite the above mentioned methodsvoid
now so you are not affected by this backward compatibility break.
This is now implemented in PHPUnit 8, so our method signatures are no longer compatible and cause fatal errors:
Fatal error: Declaration of WP_UnitTestCase_Base::setUpBeforeClass() must be compatible with PHPUnit\Framework\TestCase::setUpBeforeClass(): void in tests/phpunit/includes/abstract-testcase.php on line 1110
We could look into using different signatures for different PHPUnit versions in the same way as in [44701]. However, given the large number of tests extending the ::setUp()
and ::tearDown()
methods, it's probably easier to address this when we bump the required PHP version to 7.1.
Attachments (3)
Change History (144)
#1
@
6 years ago
- Milestone changed from Awaiting Review to Future Release
- Type changed from defect (bug) to task (blessed)
#4
@
6 years ago
PHPUnit 8.0 and 8.0.1 have officially been released.
#5
@
6 years ago
Noticed today that the PHP nightly build is now failing from the changes in #43218. This should be fixed when the needed changes are made for this ticket.
https://travis-ci.org/WordPress/wordpress-develop/jobs/490350822
#6
@
6 years ago
Nightly switched from PHP 7.4 to PHP 8.0. #46235 is for adding PHP 7.4 back to the test matrix.
#7
@
5 years ago
Please consider upgrading. I ran into this issue today trying to setup pcov:
https://github.com/krakjoe/pcov
Requires phpunit v8. Improves code coverage performance significantly.
#8
@
5 years ago
In February, PHPUnit 7 support will end and only PHPUnit 8 and 9 (to be released on February 7) will be supported.
The tickets says "it's probably easier to address this when we bump the required PHP version to 7."
Is there any news related to that?
#10
@
5 years ago
- Keywords needs-patch added
There isn't one. If someone wants to work on this, that would be great.
#11
follow-ups:
↓ 13
↓ 15
@
5 years ago
Note that this still depends on bumping the required PHP version to 7.
As of this time, the decision has not been made yet.
#12
@
5 years ago
- Owner set to netweb
- Status changed from new to assigned
I've partially implemented this for a client, will work on bringing those changes here
#13
in reply to:
↑ 11
@
5 years ago
Replying to SergeyBiryukov:
Note that this still depends on bumping the required PHP version to 7.
As of this time, the decision has not been made yet.
There has been 0 news on this front since the initial announcement. That announcement said "Depending upon feedback and effectiveness, we can follow up by bumping the required PHP version to 7 as early as December 2019." but that timeframe has clearly passed.
Can the WP team please share some news on this?
This ticket was mentioned in Slack in #core by sergey. View the logs.
4 years ago
#15
in reply to:
↑ 11
@
4 years ago
- Description modified (diff)
Replying to SergeyBiryukov:
Note that this still depends on bumping the required PHP version to 7.
Just to clarify, bumping the required version to PHP 7.0 is apparently not enough, as the void
return type declarations only appeared in PHP 7.1. So to support PHPUnit 8.x, we'd need PHP 7.1 as the minimum required version.
Updating the ticket description accordingly.
#16
follow-up:
↓ 17
@
4 years ago
As per #50902 - we will never be able to get proper insight into the issues which need fixing on PHP 8 without making the testsuite compatible with PHPUnit 8 + 9.
PHPUnit 9.3.x was released a week ago and is the first PHPUnit version which is officially claiming support for PHP 8.0.
As per the WP 5.6 release planning support for PHP 5.6 will be dropped in WP 5.6.
Is there an official decision yet that PHP 7.1 (or higher) will be the new minimum ? If so, I'd willing to create an initial patch to fix the test suite.
#17
in reply to:
↑ 16
@
4 years ago
- Milestone changed from Future Release to 5.6
Replying to jrf:
Is there an official decision yet that PHP 7.1 (or higher) will be the new minimum ? If so, I'd willing to create an initial patch to fix the test suite.
No official decision yet, but I'm pretty sure that is the goal that was agreed upon in the May 11th Site Health meeting (Slack logs).
Since PHP 8 support is also one of the WP 5.6 focuses, and PHPUnit 8+ is a prerequisite for that, let's proceed with this.
This ticket was mentioned in PR #483 on WordPress/wordpress-develop by jrfnl.
4 years ago
#18
- Keywords has-patch has-unit-tests added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/46149
The commits in this PR combined:
- Bump the minimum supported PHP version for WordPress to PHP 7.1.26.
The "patch" part of the version number -
7.1.patch
- is open for discussion, please see commit https://github.com/WordPress/wordpress-develop/commit/e913a6889a05cee5dab9a4eb2f3727029199076a for my argumentation for this specific version.
- Applies the proposed patch for trac #50902 to allow the unit tests to run on PHP 8.0/nightly.
- Bumps the supported PHPUnit versions to
^7.5 || ^8.0 || ^9.0
. - Removes now redundant backfills for PHPUnit native functionality.
- Adds a few polyfills for new PHPUnit functionality in PHPunit-version based
trait
s. - Comprehensively applies any and all fixes needed to make the complete test suite compatible with PHPUnit 8.x and 9.x.
This PR will be easiest to review by looking at the individual commits and their detailed commit messages.
Once the patch from ticket 50902 and the patches from this PR are merged, we will be able to get proper insight into the problems we need to solve to make WordPress compatible with PHP 8.0 based on the existing tests.
Trac ticket 50913 already contains a couple of patches to make a start with PHP 8.0 compatibility.
Expanding the test suite, strict type checking in the test suite (Trac 38266) and code coverage tracking (Trac 46373) combined with adding @covers
annotations (Trac 39265) to get better insight into which parts of the code base need more tests, is strongly recommended all the same.
This ticket was mentioned in PR #484 on WordPress/wordpress-develop by jrfnl.
4 years ago
#20
Trac ticket: https://core.trac.wordpress.org/ticket/46149
The commits in this PR combined:
- Bump the minimum supported PHP version for WordPress to PHP 7.1.26.
The "patch" part of the version number -
7.1.patch
- is open for discussion, please see commit https://github.com/WordPress/wordpress-develop/commit/e913a6889a05cee5dab9a4eb2f3727029199076a for my argumentation for this specific version.
- Applies the proposed patch for Trac #50902 to allow the unit tests to run on PHP 8.0/nightly.
- Bumps the supported PHPUnit versions to
^7.5 || ^8.0 || ^9.0
. - Removes now redundant backfills for PHPUnit native functionality.
- Adds a few polyfills for new PHPUnit functionality in PHPunit-version based
trait
s. - Comprehensively applies any and all fixes needed to make the complete test suite compatible with PHPUnit 8.x and 9.x.
This PR will be easiest to review by looking at the individual commits and their detailed commit messages.
Once the patch from ticket #50902 and the patches from this PR are merged, we will be able to get proper insight into the problems we need to solve to make WordPress compatible with PHP 8.0 based on the existing tests.
Trac ticket #50913 already contains a couple of patches to make a start with PHP 8.0 compatibility.
Expanding the test suite, strict type checking in the test suite (Trac #38266) and code coverage tracking (Trac #46373)) combined with adding @covers
annotations (Trac #39265) to get better insight into which parts of the code base need more tests, is strongly recommended all the same.
#21
@
4 years ago
Please see the GitHub PR for a comprehensive set of patches to address this ticket.
#23
@
4 years ago
Before I forget to mention it: the "WP version bump" commit does not have to be included in the commits for this ticket, however, as the patches for this ticket require to stop testing against PHP < 7.1, it seemed like a logical commit to include.
After this ticket has been addressed, I would like to suggest follow-up tickets to address the following additional points in the test suite:
- Review any
@requires
andversion_compare()
's et al in the test suite to see if any of these can be removed now the requirements have changed. - Review any disabled tests (like commented out tests) to see if they were disabled due to version requirements not being met.
- Review if the
includes/phpunit6/compat.php
file can be removed completely.
Similarly, after the version bump patch has been committed, follow-up tickets should be opened for the following additional points:
- Removing the random_compat library. This library is no longer needed when the minimum PHP requirement is PHP 7.1. (Similar to #47699 for the PHP < 5.6 drop)
- Go through the
compat.php
file to see if there are other PHP polyfills which can be removed. (Similar to #47698 for the PHP < 5.6 drop) - Go through the code base to review any code using
version_compare()
,function_exists()
,defined()
,extension_loaded()
et al to verify whether these kind of toggles are still needed and remove them whenever possible. (Similar to #48074 for the PHP < 5.6 drop) - Updating external PHP dependencies if an update was so far blocked by a minimum PHP version mismatch.
4 years ago
#24
Thanks @jrfnl , a few more thoughts:
### PHP 5.6 support:
- https://wordpress.org/about/stats/
- PHP 5.6 = 15.1%
- This pretty much rules out PHP 5.6 being dropped in WP 5.6, that usage is far too high to consider dropping it, the considerations historically have been to consider if usage drops below 5%
### PHPUnit test matrix
- PHPUnit 8.x supports PHP 7.2, 7.3
- PHPUnit 9.x supports PHP 7.3, 7.4 & 8.0
Would it be worth setting up or changing the existing jobs to use newer versions of PHPUnit?
- PHP 7.2 using PHPUnit 8.x
- PHP 7.3 and/or 7.4 using PHPUnit 9.x
4 years ago
#25
Taking a read of the #site-health chat logs
The numbers for PHP 5.6 are similar to those we had for < 5.6 when we last bumped the version for WordPress 5.2, so it seems like we could bump the minimum to PHP 7.0 by the end of year as is. We do want to bump to PHP 7.1, though (as that would unblock using PHPUnit 8+ in core and testing WordPress on PHP 8), and we need additional efforts for that.
Given that we're still releasing security updates for WP 3.7 (released almost 7 years ago), it's not like we're leaving PHP 5.6 or 7.0 users without security updates, they just won't have some latest and greatest features of WP 5.6+, which seems fair.
A good first step might be to highlight PHP 7.0 as no longer "acceptable" in Browse Happy dashboard widget, same as we currently do for 5.6.
That's some reassuring news
Just noted the RECOMMENDED_PHP value in servehappy-config.php is still at 7.3. It was bumped 18 months ago in https://meta.trac.wordpress.org/changeset/7990.
Per the comment, it's supposed to be "The latest branch of PHP which WordPress.org recommends".
Given that 7.3 support ends in 5 months, should this be updated to 7.4?
Some things to note:
- It's mentioned on https://wordpress.org/download/ and https://wordpress.org/about/requirements/.
- Bumping this also means that WP core would display a "Your site is running an older version of PHP" message for <= 7.3 in Site Health. However, this does not trigger the Dashboard nag, as that widget depends on the ACCEPTABLE_PHP value.
Ok, that's also some good steps
I'm somewhat more optimistic now having read the entire site #site-health Slack channel history since May 11 🤞🏼
#26
@
4 years ago
Hmm.. something weird going on with the GH bot as my response which should be between those last two comments did not get cross-posted.
Quoting it here now for completeness:
This pretty much rules out PHP 5.6 being dropped in WP 5.6, that usage is far too high to consider dropping it, the considerations historically have been to consider if usage drops below 5%
@ntwb Dropping PHP 5.6 support is on the roadmap for WP 5.6, so IMO this is a foregone conclusion.
I did ask for confirmation before working on this patch and wouldn't have bothered spending this much time on getting it set up if that intention hadn't been clarified.
Would it be worth setting up or changing the existing jobs to use newer versions of PHPUnit?
No, that is not an option because of the
void
return type introduced for all fixtures in PHPUnit 8.
Return types were only introduced in PHP 7.0, thevoid
return type in PHP 7.1, so it is not easy to make the test suite cross-version compatible with both PHPUnit 7 as well as PHPUnit 8 if the test suite also still has to be compatible with PHP 5.6/7.0.
There is a way to do it, but I would strongly recommend against it if it can be avoided in any conceivable way. IMO dropping support for PHP 5.6 and 7.0 is the much better choice.
#27
@
4 years ago
- Keywords early added
- Priority changed from normal to high
Ticket #51043 has been created to handle the version bump specifically.
#28
follow-up:
↓ 35
@
4 years ago
FWIW after looking through this and doing some non-trivial testing, I think it'd be best to get PHP8 unit tests running ASAP, with the assumption that PHP 5.6 support is going to stick around for a bit longer, probably until at least until the end of the year - and even if it doesn't, getting unit tests running on a not-too-far-away PHP8 is more important than that decision.
That means that migrating to using :void
doesn't appear to be possible quite yet.
After talking through things with an unnamed committer, I suspect the best option might be to shim the functions in question.
- Move from
setUp()
to something prefixed like_setUp()
in all the actual Unit Tests (plus all the other newly void functions) - Conditionally load a
WP_UnitTestCase
class which definessetUp()
as either non-void for older PHPUnits+PHPs or with Void for newer PHPUnits, which calls said_setUp()
method instead. - Use Traits (PHP 5.4+) as per PR 484 to polyfill newer PHPUnit functionality
- Migrate all the tests to the newer test syntax, as with PR 484.
That might mean that we'd ultimately end up with a phpunit-[5-9]/testcase.php
file set, but that seems like a more reasonable framework to ensure that going forward we'll have a framework in place to support future PHPUnit/PHP syntax changes.
Related to that.. I've PR'd the wpdev images to add a PHPUnit9 image and use it for PHP 8 Since it's the only version that is supported there.
#29
@
4 years ago
I think the approach @dd32 is suggesting is going to be the best route for us. Supporting multiple versions of PHPunit is an unfortunate reality, but one that will help us not just now but also possibly farther into the future. While the ultimate goal of WordPress supporting only the versions of PHP that the PHP core team supports (which is the policy in place for PHPUnit) is admirable, until hosts are updating PHP versions with more regularity, it isn't something that I see becoming a reality any time soon.
#30
@
4 years ago
I doubt Symfony's PHPUnit bridge will be able to make WordPress's test suite compatible across 5 PHPUnit versions, having a single test suite that works with multiple PHPUnit versions is one if its goals: https://symfony.com/doc/current/components/phpunit_bridge.html
#31
@
4 years ago
While problably not "production-ready" for Core, you can take https://github.com/mundschenk-at/phpunit-cross-version as a proof of concept for cross-version PHPUnit tests based on the Symfony phpunit-bridge. Caveat: Newer PHPUnit versions require their particular file naming conventions for individual test classes for separate execution (complete test suite works fine even when you use WordPress conventions).
#32
follow-ups:
↓ 33
↓ 38
@
4 years ago
I understand the direction you all want to take, but cannot in good conscience support it for the following reasons:
- It really isn't as straight forward as outlined in the comments above and I say so based on both my experience with other test code bases as well as on the work I did for this ticket.
- It will raise the barrier for entry to new contributors by a factor five as they will need to learn new (undocumented) names for fixtures, assertions etc The PHPUnit knowledge they already have becomes next to useless, the PHPUnit manual can no longer be used as reference etc.
- It will alienate existing contributors to the tests.
- I truly believe our time would be better spend on making WP compatible with newer PHP versions, than on a huge undertaking of making the unit test suite compatible with PHPUnit 5 - 9.
So good luck to you. I wish you well.
#33
in reply to:
↑ 32
@
4 years ago
Replying to jrf:
I understand the direction you all want to take, but cannot in good conscience support it for the following reasons:
I totally understand your position here, and I want to state that your input is incredibly valuable and the work you've done so far in adding support is tremendous.
- It really isn't as straight forward as outlined in the comments above and I say so based on both my experience with other test code bases as well as on the work I did for this ticket.
I totally agree, this isn't super straight forward (Implementation vs Trac comments never match 1:1!), and will be messy, but it's the unfortunate reality when building with tools that have a mismatch in PHP distributions and user focus. If we only had to support one version of PHP, one version of PHPUnit, and could enforce that our lives would be so much easier, but WordPress would mostly be a developer-only tool at that point.
WordPress has been relatively lucky in that up until now, there hasn't been any significant changes like this language syntax change that cause such problems.
- It will raise the barrier for entry to new contributors by a factor five as they will need to learn new (undocumented) names for fixtures, assertions etc The PHPUnit knowledge they already have becomes next to useless, the PHPUnit manual can no longer be used as reference etc.
They will still be able to use their own knowledge, it's just that come PR time / patch time, whomever merges/commits would just need to check that the correct functions are in use.
Developers writing WordPress PHPUnit tests will still need to figure out all of our unit test factories, custom assertion methods, etc, this is far from the only change/knowledge they'd need to get up to speed on.
A unit test could even scan the tests looking for non-prefixed cases to make it easier for all.
- It will alienate existing contributors to the tests.
This has always been a struggle with WordPress, be it PHP 5.2 vs 5.3 OOP, PHP 5.2 vs 7.0, and now PHP 5.6 vs 7.1. If done correctly, I would hope that the narrow functionality of PHPUnit that we do use would just work and most contributors wouldn't notice a thing.
- I truly believe our time would be better spend on making WP compatible with newer PHP versions, than on a huge undertaking of making the unit test suite compatible with PHPUnit 5 - 9.
I do believe that too - WordPress should be, and has to be, compatible with newer PHP versions, but that shouldn't have to mean dropping support for older versions of PHP.
There's an alternative option we have here too, we could standardise on PHP8 + PHPUnit9 for *all* tests and rely upon a PHP transpile process to run it on older PHP versions / PHPUnit versions. I attempted that with PHP7.4/PHPUnit7 as a base and a transpile (let's be honest, regex..) to PHP8 which would work for travis unit testing, but falls over because we can't force everyone to use PHP7.4 for unit test development, hopefully PHP8 would be common soon.
This ticket was mentioned in PR #489 on WordPress/wordpress-develop by dd32.
4 years ago
#34
Trac ticket: https://core.trac.wordpress.org/ticket/46149
#35
in reply to:
↑ 28
@
4 years ago
Replying to dd32:
I suspect the best option might be to shim the functions in question.
I went ahead and put something together that does just that, it's not super pretty, but it does work on PHP 5.6 -> 8.0 with PHPUnit 5 -> 9.
Most importantly, it introduces a way to shim functionality from newer PHPUnits for older PHPUnit versions, so the tests can immediately use the new functionality from PHPUnit 9 - it just has to include a compat method for the older versions.
Unfortunately it did require ceasing using the setUp()
(and friends) methods directly, as there's simply no way to have those functions defined AND work on all current supported PHPs.
For now, I've simply prefixed those methods with an underscore, we do have custom wpSetUpBeforeClass()
and wpTearDownAfterClass()
methods though, we could probably do the same for the setUp()
/etc.
I think this will affect plugins in some way or another, but if they're affected they'd have had to change their testing to work with PHP8 anyway.
My primary goal here was to get unit testing for PHP 5.6-8.0 running for core, so that there was a possibility to actually see the PHP 8 Unit test status..
To reduce the diff size for review, all of the code changes to the tests are done through a bash script, mostly based on @jrf's PR, only the changes to the test runner framework is in the PR as a code change.
If you want to see the complete changes to the tests, you can run the ./tools/migrate-test-syntax.sh
script locally or look around line 200 of the Travis tests for a 16,000 line diff.
Here's the PR and hopefully, all the Travis tests will pass.. except for PHP8.. which well you'll see :)
This ticket was mentioned in Slack in #core by jorbin. View the logs.
4 years ago
This ticket was mentioned in PR #491 on WordPress/wordpress-develop by dd32.
4 years ago
#37
Trac ticket: https://core.trac.wordpress.org/ticket/46149
#38
in reply to:
↑ 32
;
follow-up:
↓ 40
@
4 years ago
Replying to jrf:
- It really isn't as straight forward as outlined in the comments above and I say so based on both my experience with other test code bases as well as on the work I did for this ticket.
@jrf I'd like to bow down to you and agree with you. I apologise for doubting you. It's not realistic to follow what I outlined above. It works, but it's not worth it, and it's a support burden that can't be carried forward.
I've taken my above PR and forked it into a new PR 491 which does two things, which I think is a reasonable middleground
- Standardises on PHPUnit8/9 assert calls, requires PHP 7.2+ to run. (edit: 7.1+ but lets say 7.2 for future compat)
- Includes a bash script for use on Travis to remove the void return type hinting so that Travis can still run the Unit tests for PHP 5.6, 7.0 , and 7.1 (edit: Not needed for 7.1 yet)
There are two bash scripts included, one of which will be required to be bundled for Travis:
- tools/migrate-test-syntax.sh Responsible for doing some automated changes to the codebase that's needed for PHPUnit8/9 support - @jrf did a better job than this script in PR484 but this let me test that PHP8 support was working without making the PR diff huge.
- tools/tests-old-php.sh It's responsible for removing the PHP 7.2+ syntax (Right now, just the void return type hinting) so that the tests will run on PHP 5.6-7.1 with PHPUnit 5-6. This one is going to be needed on Travis, It could be done just be done inline in
.travis.yml
but I'm assuming that we might end up adding other things when PHPUnit 10 comes out next year, so it seems worth while including it separately, which also allows developers to run it locally if needed.
The PHPUnit Compat traits that I've got in there are super minimal and just pass it through to other assertion methods that should exist. I did it slightly different to PR484 which makes it far easier and simpler as we don't have to use matching function definitions, nor check if the native function exists. PR484 has better PHPDocs though.
There's a bunch of other cleanups that can be done, but I didn't look into, such as using the PHPUnit namespaces and the SpeedTrap changes.
all of the unit tests run with only PHP8 failing due to some Core changes needed and some test alterations that I couldn't easily automate.
#40
in reply to:
↑ 38
@
4 years ago
Replying to dd32:
I've taken my above PR and forked it into a new PR 491 which does two things, which I think is a reasonable middleground
- Standardises on PHPUnit8/9 assert calls, requires PHP 7.2+ to run.
- Includes a bash script for use on Travis to remove the void return type hinting so that Travis can still run the Unit tests for PHP 5.6, 7.0, and 7.1.
Just noting that the void
return type hinting was introduced in PHP 7.1, so unless I'm missing something, PHP 7.1 should be able to run the newer syntax without any modifications, I think they are only needed for 5.6 and 7.0. Is that assumption wrong?
#41
follow-up:
↓ 42
@
4 years ago
Just noting that the void return type hinting was introduced in PHP 7.1, so unless I'm missing something, PHP 7.1 should be able to run the newer syntax without any modifications, I think they are only needed for 5.6 and 7.0. Is that assumption wrong?
Yes and no. Void is not the problem for PHP 7.1. The problem there is that PHPUnit 7 is still needed - PHPUnit 8 has a minimum requirement of PHP 7.2 - and if per @dd32 comment above, the test suite will be standardized on PHPUnit8/9 assert calls (which I support as the sensible thing to do), then some of those assertions will either need wrappers, like the traits I created in the original PR, or the proposed bash script will need to run to "rewrite" some of those assertions to their old name/ to the old way of doing that assertion.
#42
in reply to:
↑ 41
@
4 years ago
Replying to jrf:
Yes and no. Void is not the problem for PHP 7.1. The problem there is that PHPUnit 7 is still needed
Ah, that makes sense. Thanks for the clarification!
#43
@
4 years ago
I did indeed mix PHP 7.1/7.2 and PHPUnit 8/9 requirements up.
The bash script isn't currently needed for PHP 7.1 as the void return type hints work there.
However, as @jrf suggested, if we're going to take this route (Which in all honesty, is the only way forward I can see) it makes zero sense to recommend PHPUnit 7 / PHP 7.1 be natively supported by the tests - It may still just work, but we shouldn't advocate it's use, and we have to assume that it won't always run without run-time modifications in the future.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#47
@
4 years ago
- Keywords needs-dev-note added
Adding needs-dev-note
. Detailing things to be aware of related to unit testing and PHP 8 should be outlined.
#49
@
4 years ago
- Milestone changed from 5.6 to Future Release
PHPUnit 8.x/9.x support would be great to have, but is complicated if we also have to support PHP 5.6 and 7.0 at the same time.
Currently, we have the tests running on PHP 8 with PHPUnit 7.5.x, as result of the changes in #50902, specifically [48957] and [49037]. It's not pretty, but does the job for now. So I think the best way forward at the moment would be to continue with PHPUnit 7.5.x, as that already works.
Since core no longer requires PHPUnit 9.x support for PHP 8 testing, I'm moving this to a Future Release to be explored separately. Once the minimum required PHP version is bumped to 7.1 or later, this should be easier to address.
#50
follow-up:
↓ 53
@
3 years ago
The first alpha of PHP 8.1 has been released yesterday and brings with it new challenges for getting the tests running on PHP 8.1, so this ticket needs to be revived again.
I know I'm biased, but I would strongly recommend using the PHPUnit Polyfills to solve the majority of issues.
The Polyfills allow for using the expectations and assertions as available in PHPUnit 9.x and to still run the tests on PHPUnit 4.8 - 9.x.
It also includes a TestCase
solution solving the void
problem.
Additionally, I'd like to draw attention to the fact that PHPUnit 9.1.0 deprecated support for test file names which do not match the class name of the test class within.
Ref: https://github.com/sebastianbergmann/phpunit/pull/4109/
This means that the test file naming conventions will also need revisiting when addressing this ticket.
Loosely related to #53010 and https://github.com/WordPress/WordPress-Coding-Standards/issues/1995.
This ticket was mentioned in Slack in #core-test by mkaz. View the logs.
3 years ago
#53
in reply to:
↑ 50
@
3 years ago
Replying to jrf:
I know I'm biased, but I would strongly recommend using the PHPUnit Polyfills to solve the majority of issues.
The Polyfills allow for using the expectations and assertions as available in PHPUnit 9.x and to still run the tests on PHPUnit 4.8 - 9.x.
It also includes aTestCase
solution solving thevoid
problem.
Just voicing my support for this as it seems to be the most sensible approach.
Per discussion with Juliette, I think we can take things one step at a time for clarity and modernize the assertions first, then tackle the void
return type issue separately.
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
This ticket was mentioned in Slack in #hosting-community by javier. View the logs.
3 years ago
This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.
3 years ago
#59
@
3 years ago
During today's working session with @johnbillion @SergeyBiryukov @jrf @netweb and me (see the live stream here), we discussed and reached consensus on adopting the practice of building tests to the latest PHPUnit version:
- Adopt PHPUnit Polyfills to shim backwards for cross-version compatibility
- Add an adapter layer between the tooling and Core's test cases for future flexibility
- Change the inheritance order
- Remove the copied PHPUnit files and PHPUnit assertions
- Adopt Polyfills snakecase fixture methods (as PHPUnit 8 method signatures changed) -> this is a breaking change for extenders who are using fixture methods in their tests
- Help extenders with this breaking change (ticket #53904)
- Get the atomic commits (listed here) committed (in order) ASAP
- Implement a very very early communication plan for outreach to the extender community including publishing a Dev Note on Make Core (i.e. to give the extender community awareness and guidance on how to prepare their test suites)
This plan will add PHPUnit 8 and 9 support to Core. Plus, it sets up the test suites to adopt future PHPUnit versions.
#60
@
3 years ago
To add to @hellofromTonya's comment, the commit workflow for these changes will be as follows:
- Commit the patch for ticket #47381, which is a prerequisite. The patch has been pulled to GitHub, been reviewed by various committers and is now ready for commit.
- Once that patch has been committed a new PR on GitHub will be opened against this ticket with the first set of commits referenced by @hellofromTonya above. This first set of commits will introduce the PHPUnit Polyfills package and sort out all assertion and expectation method changes.
- Once that patch has also been committed a next PR on GitHub will be opened against this ticket with the second set of commits referenced above. This second set of commits will address the
void
return type and make the test suite cross-version compatible with PHPUnit 5.7.x up to PHPUnit 9.5.x (current) and will enable running of the tests against PHP 8.1 in CI.
#61
@
3 years ago
Couple extra points not mentioned so far:
SpeedTrapListener
will be removed, see 6:ticket:38237 for details.- Some degree of backporting will happen in order to allow a plugin or theme test suite to use the snakecase methods in all test classes so they run on older WP versions too. Ticket(s) to be opened in due course.
This ticket was mentioned in PR #1545 on WordPress/wordpress-develop by jrfnl.
3 years ago
#62
Note: this patch/these commits were reviewed and discussed extensively during the livestreamed mobprogramming session of July 30 with @johnbillion @SergeyBiryukov @hellofromtonya and me.
The livestream can be watched back as video on demand.
### Composer: add dependency on the PHPUnit Polyfills package
The PHPUnit Polyfills package is an add-on for PHPUnit, which works around common issues for writing PHPUnit cross-version compatible tests.
Features:
- It offers a full set of polyfills for assertions and expectations introduced in PHPUnit since PHPUnit 4.8.
- It offers two generic
TestCase
s which include these polyfills, but also solve thevoid
return type issue for the fixtures methods. - It offers a PHPUnit cross-version solution for the changes to the PHPUnit
TestListener
implementation. - Supports PHPUnit 4.8 - current.
- Supports and is compatible with PHP 5.4 - current.
The package has no outside dependencies, other than PHPUnit, is actively maintained and endorsed by the maintainer of PHPUnit itself (the only package of its kind which has ever been endorsed).
### Build/Tests: unify the PHPUnit adapter TestCases
This commit:
- Removes the PHPUnit 7 specific
TestCase
. - Removes all existing polyfills from the PHPUnit 5.x
TestCase
. - Imports all polyfill traits from the PHPUnit Polyfills package into the
WP_UnitTestCase
class and updates the docblock to reflect the actual function of the class.
Note: the list of polyfills needs to be verified and updated after each new release of the PHPUnit Polyfills package.
Alternatively (recommended), one of the build-inTestCase
classes from the PHPUnit Polyfills package can be used instead.
- Moves the
require
for the WPabstract-testcase.php
to thebootstrap.php
file. - Adds a
require_once
for the PHPUnit Polyfills autoloader to thebootstrap.php
file.
Note: while this isn't _strictly_ necessary when the tests are run via Composer, having the include in the bootstrap allows for the tests to also be run via a PHPUnit Phar, providing contributors with more flexibility.
### Build/Tests: change the inheritance order of the abstract test classes
As things were, the inheritance order of the abstract test classes was as follows:
WP_UnitTestCase
(PHPUnit adapter layer)
extends
WP_UnitTestCase_Base
(base test class)
extends
PHPUnit\Framework\TestCase
(PHPUnit native class.
Concrete (child) test classes, as well as more specific abstract TestCase
s are/were expected to extend the WP_UnitTestCase
.
This order is not optimal as it means that the WP_UnitTestCase_Base
class would not be able to benefit from any polyfills and/or shims in the PHPUnit adapter layer.
With that in mind, this commit changes the inheritance to:
WP_UnitTestCase
(empty class, left in place to not break BC for plugin/theme integration tests)
extends
WP_UnitTestCase_Base
(base test class)
extends
PHPUnit_Adapter_TestCase
(PHPUnit adapter layer)
extends
PHPUnit\Framework\TestCase
(PHPUnit native class.
The new order allow for the WP_UnitTestCase_Base
to also benefit from the PHPUnit adapter layer.
For BC reasons the WP_UnitTestCase
, which all test classes are (were) expected to extend is left in place, though it is now an empty class and explicitly abstract
.
### WP_UnitTestCase_Base::setExpectedException(): simplify redundant PHPUnit shim
PHPUnit 6 deprecated the setExpectedException()
method in favour of the expectException()
, expectExceptionMessage()
and expectExceptionCode()
methods.
The WP_UnitTestCase_Base::setExpectedException()
backfilled the old method.
As the PHPUnit Polyfills polyfill the _new_ method, this backfill can now be simplified.
This backfill _should_ be removed in a future iteration, but is, for now, left in place so as not to break BC for plugin/theme test suites which extend the WP native test suite for their integration tests.
Follow up on [48996], [48997].
See #51344
### Tests: replace expectException() for PHP native errors
... with calls to the dedicated PHPUnit 8.4.0+ methods.
The old manner of testing these is soft deprecated as of PHPUnit 8.4, hard deprecated as of PHPUnit 9.0 and will be removed in PHPUnit 10.0.0.
The dedicated expectDeprecation()
, expectDeprecationMessage()
, expectDeprecationMessageMatches()
, expectNotice()
, expectNoticeMessage()
, expectNoticeMessageMatches()
, expectWarning()
, expectWarningMessage()
, expectWarningMessageMatches()
, expectError()
, expectErrorMessage()
, expectErrorMessageMatches()
methods were introduced as replacement in PHPUnit 8.4 and should be used instead.
These new PHPUnit methods are all polyfilled by the PHPUnit Polyfills and switching to these will future proof the tests some more.
Ref:
- https://github.com/sebastianbergmann/phpunit/blob/8.4.3/ChangeLog-8.4.md#840---2019-10-04
- sebastianbergmann/phpunit#3775
### Tests: replace assertFileNotExists() with assertFileDoesNotExist()
The assertFileNotExists()
method was hard deprecated in PHPUnit 9.1 and the functionality will be removed in PHPUnit 10.0.0.
The assertFileDoesNotExist()
method was introduced as a replacement in PHPUnit 9.1.
This new PHPUnit method is polyfilled by the PHPUnit Polyfills and switching to it will future proof the tests some more.
Ref:
- https://github.com/sebastianbergmann/phpunit/blob/9.1.5/ChangeLog-9.1.md#910---2020-04-03
- sebastianbergmann/phpunit#4076
### Tests: replace assertRegExp() with assertMatchesRegularExpression()
The assertRegExp()
and assertNotRegExp()
methods were hard deprecated in PHPUnit 9.1 and the functionality will be removed in PHPUnit 10.0.0.
The assertMatchesRegularExpression()
and assertDoesNotMatchRegularExpression()
methods were introduced as a replacement in PHPUnit 9.1.
This new PHPUnit method is polyfilled by the PHPUnit Polyfills and switching to it will future proof the tests some more.
Ref:
- https://github.com/sebastianbergmann/phpunit/blob/9.1.5/ChangeLog-9.1.md#910---2020-04-03
- sebastianbergmann/phpunit#4085
- sebastianbergmann/phpunit#4088
### Tests: replace assertNotRegExp() with assertDoesNotMatchRegularExpression()
The assertRegExp()
and assertNotRegExp()
methods were hard deprecated in PHPUnit 9.1 and the functionality will be removed in PHPUnit 10.0.0.
The assertMatchesRegularExpression()
and assertDoesNotMatchRegularExpression()
methods were introduced as a replacement in PHPUnit 9.1.
This new PHPUnit method is polyfilled by the PHPUnit Polyfills and switching to it will future proof the tests some more.
Ref:
- https://github.com/sebastianbergmann/phpunit/blob/9.1.5/ChangeLog-9.1.md#910---2020-04-03
- sebastianbergmann/phpunit#4085
- sebastianbergmann/phpunit#4088
Trac ticket: https://core.trac.wordpress.org/ticket/46149
#66
follow-ups:
↓ 67
↓ 70
@
3 years ago
Soo...looks like r51560 breaking a lot of plugins' test suites which are using scripts like install-wp-tests.sh
(which is quite common) to get the test suite. At least in my plugin I now get You need to run
composer update before running the tests.
errors when trying to run tests.
Any thoughts on how this can be fixed?
#67
in reply to:
↑ 66
@
3 years ago
Replying to swissspidy:
Soo...looks like r51560 breaking a lot of plugins' test suites which are using scripts like
install-wp-tests.sh
(which is quite common) to get the test suite. At least in my plugin I now getYou need to run
composer updatebefore running the tests.
errors when trying to run tests.
Any thoughts on how this can be fixed?
As per https://core.trac.wordpress.org/ticket/46149#comment:59, a dev-note will be published soonish, once all the relevant commits for the changes being made here have been committed.
The dev-note will contain information on how to make plugin/theme integration test setups compatible.
In the mean time: have you tried doing what the message says ?
#68
follow-up:
↓ 69
@
3 years ago
In the mean time: have you tried doing what the message says ?
The `install-wp-tests.sh` script typically downloads the tests/phpunit/includes
and tests/phpunit/data
folders for running the test suite. No other folders and no compoer.json
file, so one can't run composer update
in such a setup.
I appreciate that you all are planning on writing a dev-note soon-ish and I hope that you take this scenario into account, so that ideally we can update that shell script as well.
#69
in reply to:
↑ 68
@
3 years ago
Replying to swissspidy:
Soo...looks like r51560 breaking a lot of plugins' test suites which are using scripts like
install-wp-tests.sh
(which is quite common) to get the test suite. At least in my plugin I now getYou need to run
composer updatebefore running the tests.
errors when trying to run tests.
Just in case we need some examples, I'll jump in to say we're running into that exact same issue with tests in the Jetpack plugin, with the same setup you described in your follow-up comment:
The `install-wp-tests.sh` script typically downloads the
tests/phpunit/includes
andtests/phpunit/data
folders for running the test suite. No other folders and nocompoer.json
file, so one can't runcomposer update
in such a setup.
#70
in reply to:
↑ 66
@
3 years ago
My test suites running on PHP8 for my plugin utilize tests lib on the trunk/master branch in order to work around compatibility issues with building our plugin test suites off the top of the WP core test suite.... Maybe this was short sighted but I needed php8 unit tests working and this was the fastest way to get it working.
The PHPUnit polyfills works wonderfully. Thank you for all this hard work.
After suites started failing yesterday I started looking into what's going on and found my way here.
I've added the https://github.com/Yoast/PHPUnit-Polyfills as a dependency for my own project (since I install via install-wp-tests.sh (as noted above). However. I've chosen to install my test stuff in a separate directory that gets ignored from version control. As a result the reference to the autoloader assumes installation in the root directory and I can't get it working without forking the bootstrap file.
I'd like to propose a configurable vendor directory constant in the bootstrap file (similar to how the wp-tests-config.php file can be configured with a constant (https://github.com/WordPress/wordpress-develop/commit/1e88432e34a5c8405c75e5a56977362766b4cf95)
This would allow me to add a custom vendor directory location in my phpunit.xml
(or elsewhere)
I've written (and tested) a patch that will allow this kind of setup and doesn't break the core tests lib (attached)
Would this be an acceptable addition to help someone in my position. Maybe I'm the only silly guy to install in a different directory...
#71
@
3 years ago
@thomasplevy The changes now committed are only the first three commits of a much larger PR, with a second follow-up PR still waiting to be pulled. Please be patient and wait until everything from those PRs has been committed.
We'll sort out a solution for the polyfill autoloader, but please have a few days/weeks patience.
#72
@
3 years ago
And just as a general note for anyone now panicking:
WP 5.9 is currently in early development.
As a general best practice, plugins/themes should be tested against stable versions of WP and against the alpha/beta/RC of an upcoming version, with this latter build potentially being allowed to fail.
We're nowhere near WP 5.9-alpha yet, so please give us a little time to finish these changes.
Things like the scaffold command will be taken into account and if needs be, small changes will be made to accommodate, but let's first get the bulk of the changes to fix the PHPUnit cross-version compatibility for WordPress Core committed.
If you're interested in what more is upcoming, please watch the live stream as linked above, which is available as video on demand, in which the changes are discussed in detail and/or peruse the PRs when they are opened.
#73
@
3 years ago
@jrf Don't mistake this as impatience. I didn't intend it that way. I saw a problem and it was unclear to me what other changes were coming and I saw a problem with my (possibly weird) setup and instead of pointing out a problem I thought I'd try to help.
Thanks for the update and for whatever it's worth I don't care how far away a release is, I test nightlies automatically. I have a small team with limited resources and advanced warning of whatever is coming is very helpful. If I wait until an RC is available and then find a problem it might be too late for my team and I to get compatibility ready before it's stable.
I appreciate the effort and work being done here and I don't intend to come off as rude or impatient.
This ticket was mentioned in PR #1551 on WordPress/wordpress-develop by jrfnl.
3 years ago
#80
### Build/Tests: use the PHPUnit Polyfill Testcase as void
work-around
PHPUnit 8.0.0 introduced a
void
return type declaration to the "fixture" methods -setUpBeforeClass()
,setUp()
,tearDown()
andtearDownAfterClass()
. As thevoid
return type was not introduced until PHP 7.1, this makes it more difficult to create cross-version compatible tests when using fixtures, due to signature mismatches.
The
Yoast\PHPUnitPolyfills\TestCases\TestCase
overcomes the signature mismatch by having two versions. The correct one will be loaded depending on the PHPUnit version being used.
When using this
TestCase
, if an individual test, or anotherTestCase
which extends thisTestCase
, needs to overload any of the "fixture" methods, it should do so by using a snake_case variant of the original fixture method name, i.e.set_up_before_class()
,set_up()
,assert_pre_conditions()
,assert_post_conditions()
,tear_down()
andtear_down_after_class()
.
The snake_case methods will automatically be called by PHPUnit.
IMPORTANT: The snake_case methods should not call the PHPUnit parent, i.e. do not use
parent::setUp()
from within an overloadedset_up()
method. If necessary, DO callparent::set_up()
.
Source: https://github.com/Yoast/PHPUnit-Polyfills#testcases
This commit:
- Lets the
PHPUnit_Adapter_TestCase
extend theYoast\PHPUnitPolyfills\TestCases\TestCase
, which makes this solution for thevoid
return type available to the WordPress test suite. - Removes the individual import and trait
use
statements for the Polyfill traits.
These are no longer necessary as the
Yoast\PHPUnitPolyfills\TestCases\TestCase
already includes those.
### Build/Tests: implement use of the void
solution
... by renaming all declared fixture methods, and calls to parent versions of those fixture methods, from camelCase to snake_case.
### Test_WP_Widget_Media::test_constructor(): remove use of assertArraySubset()
The assertArraySubset()
method has been deprecated in PHPUnit 8 and removed in PHPUnit 9.
Replacing the assertions with looping through the array and testing both the key and the value individually.
Refs:
- https://github.com/sebastianbergmann/phpunit/blob/8.0.6/ChangeLog-8.0.md#800---2019-02-01
- sebastianbergmann/phpunit#3494
Note: there is a polyfill package available for the removed assertion - dms/phpunit-arraysubset-asserts
-, but as the assertion was only used in this one test method, adding this seems redundant.
### Build/Tests: alias the Getopt class conditionally
... as the class no longer exists in PHPUnit 9.x.
To be honest, most of the aliasing in this compat.php
file is redundant as PHPUnit 5.7.21+ contains a forward compatibility layer for these classes anyway (= PHPUnit provides both the namespaced and underscore named versions of these classes in PHPUnit 5.7.21+).
All the same, I'm choosing to leave the file (and the aliases) in place for the time being, as plugins/themes using the WP test suite as the basis for their integration tests may rely on it, though WP itself should not really need it anymore, save for maybe one or two classes.
### Tests: fix tests failing due to assertContains() using strict checking
Since PHPUnit 8.0.2, the assertContains()
method when checking whether a value exists in an array will do a strict type comparison of the values.
This caused a couple of tests to fail.
Using the correct data type in the test fixes that.
Refs:
- https://github.com/sebastianbergmann/phpunit/blob/8.0.6/ChangeLog-8.0.md#802---2019-02-07
- sebastianbergmann/phpunit#3511
- sebastianbergmann/phpunit@6205f33
### Build/Tests: handle removal of TestCase::getAnnotations()
The PHPUnit native TestCase::getAnnotations()
method is used to check for WP flavoured deprecation notices, however, this method was not covered by the backward compatibility promise for PHPUnit (and was annotated as excluded).
The method has been removed as part of an internal refactor in commit sebastianbergmann/phpunit@6858204, which is included in PHPUnit 9.5.0.
For now, I've put a work-around in place, but I'd recommend that the WP expectDeprecated()
method be re-evaluated in a future iteration.
### Build/Tests: remove SpeedTrapListener
Now the tests can run PHPUnit cross-version and Composer will be used to install the test suite in CI, we could switch out the local copies of the PHPUnit speedtrap package in favour of using the Composer package, which would prevent us having to make the WP local copies of the class compatible with later PHPUnit versions.
The SpeedTrap test listener was introduced to identify slow tests and take action on these to make them faster.
In practice, however, no notable action was ever taken based on the output of the testlistener in all the years the test listener was in place.
With that in mind, it was decided to remove the SpeedTrap testlisteners without replacement.
If - at a future date - contributors would want to take action to speed up slow tests anyway, they can:
- Either add the package to their local install and use the output they receive locally to identify slow tests.
- Or use the PHPUnit native
@small
annotations in combination with the PHPUnitPHP_Invoker
package as described in the PHPUnit documentation to run tests with time limits: https://phpunit.readthedocs.io/en/stable/risky-tests.html#test-execution-timeout
### Build/Tests: loosen the PHPUnit restriction
composer.json
:
Remove the PHPUnit dependency in favour of allowing the PHPUnit Polyfills library to manage the supported PHPUnit version.
This automatically now widens the supported PHPUnit versions to 5.7.21 to 9.5.8 (current).
Letting the PHPUnit Polyfills handle the version constraints for PHPUnit prevents potential version conflicts in the future as well as allows WP to benefit straight away when a new PHPUnit version would be released and the PHPUnit Polyfills package adds support for that PHPUnit version.
Test Bootstrap
Update the supported version number for PHPUnit 5.x, as the minimum PHPUnit 5.x version supported by the PHPUnit Polyfills is PHPUnit 5.7.21 and remove the PHPUnit maximum.
.gitignore:
Add the PHPUnit cache file to the list of files to be ignored.
Since PHPUnit 8, PHPUnit has a build in caching feature which creates a .phpunit.result.cache
file. This file should not be committed.
Note: This same change needs to be made to the .svnignore
file.
### Build/Tests: remove the copied in PHPUnit 9.x MockObject files
As the version constraints for PHPUnit now allow the tests to be run on PHPUnit 8.x and 9.x, these files are no longer needed.
### Tests: replace expectException() for PHP native errors
... with calls to the dedicated PHPUnit 8.4.0+ methods.
The old manner of testing these is soft deprecated as of PHPUnit 8.4, hard deprecated as of PHPUnit 9.0 and will be removed in PHPUnit 10.0.0.
Most calls like this were already replaced in the patch which introduced the PHPUnit Polyfills, however, this particular one could not be changed yet due to the mismatch between the PHPUnit version and the PHP version on which the tests were being run.
Fixed now anyway.
Ref:
- https://github.com/sebastianbergmann/phpunit/blob/8.4.3/ChangeLog-8.4.md#840---2019-10-04
- sebastianbergmann/phpunit#3775
### Build/Tests: remove redundant @requires tags
As the minimum supported PHPUnit version has been upped to PHPUnit 5.7.21, these @requires
tags are now redundant.
---
Trac ticket: https://core.trac.wordpress.org/ticket/46149
#81
@
3 years ago
Thank you @SergeyBiryukov for getting the first PR fully committed. 💖
I've now opened the second PR which addresses the void
issue and various other PHPUnit cross-version compatibility issues.
We are fully aware that this will cause plugin/theme integration tests against trunk
to fail (even more) for now. This was discussed in depth and we cannot avoid a breaking change here, but intend to backport select commits to older WP branches to make this less painful for plugins/themes supporting multiple WP versions.
Please bear with us while we work through all the tasks and commits.
Also note that this PR does not yet turn on the build for PHP 8.1 (even though the tests can run fine on PHP 8.1 now, though there are still a lot of failures to address), as the CI build would not run due to issues with the NPM/Docker scripts.
See this test build for details and take note that the actual failure is not so much in the test run, but in the "Install WordPress" step before it.
#82
@
3 years ago
FYI: tickets #52825 and #53858 are both prerequisites before the test runs against PHP 8.1 in CI can be turned on.
With patches for those two tickets committed, the tests will run, though there will still be warnings all over the place in earlier steps and lots of test failures/errors, but at least they will run.
#95
@
3 years ago
- Owner changed from netweb to jrf
Reassigning the ownership per discussion with @jrf as the author of most of the PRs here.
#96
@
3 years ago
FYI: The turning on of test runs against PHP 8.1 in CI will be handled in ticket #53891
This ticket was mentioned in PR #1563 on WordPress/wordpress-develop by jrfnl.
3 years ago
#98
The PHPUnit Polyfills are, since [51559], a required dependency for the WP test suite and, by extension, for plugin/theme integration test suites which are based on and use (parts of) the WP Core test suite.
However, plugin/theme integration test suites may not use a full WordPress installation.
This commit:
- Removes the presumption that a full WP install, including
vendor
directory, will be available when the testbootstrap.php
file is run. - Makes the loading of the PHPUnit Polyfills autoload file more flexible by:
- Checking if the autoload class contained within the autoload file is already available before attempting to load the file.
This allows for plugin/theme integration test suites to load the
phpunitpolyfills-autoload.php
file from any location, as long as it is loaded before the WP Core testbootstrap.php
file is run.
- Allowing for the path to an arbitrary installation location for the PHPUnit polyfills to be passed as a constant.
As long as the provided location is a valid file path and the
phpunitpolyfills-autoload.php
file exists in the provided location, that file will be loaded.
The constant can be declared in a plugin/theme integration test suite native test bootstrap file, in thewp-tests-config.php
file or even in aphpunit.xml[.dist]
file via<php><const name="WP_TESTS_PHPUNIT_POLYFILLS_PATH" value="path/to/yoast/phpunit-polyfills"/></php>
.
- Adds a version check for the PHPUnit Polyfills to prevent a mismatch between the version of the package expected by WordPress and the version used by plugins/themes.
The version this checks for should be in line with the minimum version requirement for the PHPUnit Polyfills as declared in the
composer.json
file.
This version number should only be updated when new features added in later PHPUnit Polyfills releases are actually used in the WP Core tests suite.
- Adds appropriate error messages for every possible error condition.
- Upgrades the PHPUnit Polyfills to version 1.0.1, which now includes a version constant.
Trac ticket: https://core.trac.wordpress.org/ticket/46149
This ticket was mentioned in PR #1563 on WordPress/wordpress-develop by jrfnl.
3 years ago
#99
The PHPUnit Polyfills are, since [51559], a required dependency for the WP test suite and, by extension, for plugin/theme integration test suites which are based on and use (parts of) the WP Core test suite.
However, plugin/theme integration test suites may not use a full WordPress installation.
This commit:
- Removes the presumption that a full WP install, including
vendor
directory, will be available when the testbootstrap.php
file is run. - Makes the loading of the PHPUnit Polyfills autoload file more flexible by:
- Checking if the autoload class contained within the autoload file is already available before attempting to load the file.
This allows for plugin/theme integration test suites to load the
phpunitpolyfills-autoload.php
file from any location, as long as it is loaded before the WP Core testbootstrap.php
file is run.
- Allowing for the path to an arbitrary installation location for the PHPUnit polyfills to be passed as a constant.
As long as the provided location is a valid file path and the
phpunitpolyfills-autoload.php
file exists in the provided location, that file will be loaded.
The constant can be declared in a plugin/theme integration test suite native test bootstrap file, in thewp-tests-config.php
file or even in aphpunit.xml[.dist]
file via<php><const name="WP_TESTS_PHPUNIT_POLYFILLS_PATH" value="path/to/yoast/phpunit-polyfills"/></php>
.
- Adds a version check for the PHPUnit Polyfills to prevent a mismatch between the version of the package expected by WordPress and the version used by plugins/themes.
The version this checks for should be in line with the minimum version requirement for the PHPUnit Polyfills as declared in the
composer.json
file.
This version number should only be updated when new features added in later PHPUnit Polyfills releases are actually used in the WP Core tests suite.
- Adds appropriate error messages for every possible error condition.
- Upgrades the PHPUnit Polyfills to version 1.0.1, which now includes a version constant.
Trac ticket: https://core.trac.wordpress.org/ticket/46149
#100
follow-up:
↓ 121
@
3 years ago
@swissspidy @jeherve @thomasplevy Now all the commits to get the WP Core test suite running on PHP 8.1 are in, I've spend some time on addressing your concerns.
I've just opened a PR on GitHub with a proposal to make the loading of the PHPUnit Polyfills package more flexible.
I'd really appreciate it if you could look it over, test it and would give your opinion.
Aside from the above, the next steps for this "project" are:
- Prepare a proposal for which parts of these commits to backport.
- Once consensus is reached about that proposal, apply the backports, probably to WP 5.2 to 5.8.
- Prepare the dev-note.
- Possibly prepare one or two plugin integration test suites for compatibility with these changes as "proof of concept". The WP-CLI scaffold command and WP Test Utils are the likely candidates to look at as a start.
swissspidy commented on PR #1563:
3 years ago
#101
I've successfully tested this patch together with a plugin that already uses PHPUnit-Polyfills and uses a shell script as added by https://github.com/wp-cli/scaffold-command to set up the integration tests.
Setting WP_TESTS_PHPUNIT_POLYFILLS_PATH
works as expected and allows running tests just fine. The version comparison works as expected too.
So overall LGTM 👍
hellofromtonya commented on PR #1563:
3 years ago
#102
When running the test suite without first doing composer install
, there are 2 different experiences:
Scenario 1:
- No
vendor
folder ascomposer install
not ran WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant not defined
Results:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. You need to run `composer update` before running the tests. You can still use a PHPUnit phar to run them, but the dependencies do need to be installed.
Scenario 2:
- No
vendor
folder ascomposer install
not ran WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant defined to a valid directory that does not yet have thevendor
folder in it ascomposer install
has not yet been run
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', './valid_dir/vendor/yoast/phpunit-polyfills/' );
}}}
Results:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. The PHPUnit Polyfills autoload file was not found in "./valid_dir/vendor/yoast/phpunit-polyfills/" Please verify that the file path provided in the WP_TESTS_PHPUNIT_POLYFILLS_PATH constant is correct.
Notice how Scenario 2 does not guide the extender to run composer install
or composer update
.
@jrfnl What do you think? Should it be more descriptive?
hellofromtonya commented on PR #1563:
3 years ago
#103
When running the test suite without first doing composer install
, there are 2 different experiences:
Scenario 1:
- No
vendor
folder ascomposer install
not ran WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant not defined
Results:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. You need to run `composer update` before running the tests. You can still use a PHPUnit phar to run them, but the dependencies do need to be installed.
Scenario 2:
- No
vendor
folder ascomposer install
not ran WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant defined to a valid directory that does not yet have thevendor
folder in it ascomposer install
has not yet been run
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', './valid_dir/vendor/yoast/phpunit-polyfills/' );
}}}
Results:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. The PHPUnit Polyfills autoload file was not found in "./valid_dir/vendor/yoast/phpunit-polyfills/" Please verify that the file path provided in the WP_TESTS_PHPUNIT_POLYFILLS_PATH constant is correct.
Notice how Scenario 2 does not guide the extender to run composer install
or composer update
.
@jrfnl What do you think? Should it be more descriptive?
hellofromtonya commented on PR #1563:
3 years ago
#104
## Testing Results
### Composer Install not Run
- No
vendor
folder ascomposer install
not ran WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant not defined
Results:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. You need to run `composer update` before running the tests. You can still use a PHPUnit phar to run them, but the dependencies do need to be installed.
Expected error message ✅
#### With Constant Defined
- No
vendor
folder ascomposer install
not ran WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant defined to a valid directory that does not yet have thevendor
folder in it ascomposer install
has not yet been run
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', './valid_dir/vendor/yoast/phpunit-polyfills/' );
}}}
Results:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. The PHPUnit Polyfills autoload file was not found in "./valid_dir/vendor/yoast/phpunit-polyfills/" Please verify that the file path provided in the WP_TESTS_PHPUNIT_POLYFILLS_PATH constant is correct.
Expected error message ✅
### Constant Defined: Empty
Defined an empty constant in wp-tests-config.php
:
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', );
}}}
Results:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. The PHPUnit Polyfills autoload file was not found in "" Please verify that the file path provided in the WP_TESTS_PHPUNIT_POLYFILLS_PATH constant is correct.
Expected error message ✅
### Constant Defined: Invalid Path
Defined the constant in wp-tests-config.php
:
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', './doesnotexist/vendor/yoast/phpunit-polyfills/' );
}}}
Results:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. The PHPUnit Polyfills autoload file was not found in "./doesnotexist/vendor/yoast/phpunit-polyfills/" Please verify that the file path provided in the WP_TESTS_PHPUNIT_POLYFILLS_PATH constant is correct.
Expected error message ✅
### Constant Defined: Valid Path
Defined the constant in wp-tests-config.php
:
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', './valid_dir/vendor/yoast/phpunit-polyfills/' );
}}}
and copied the vendor
folder into this directory.
Results:
No error messages and tests run ✅
### Older Version of PHPUnit Polyfills Installed
Installed 1.0.0 of polyfills
Results:
Error: Version mismatch detected for the PHPUnit Polyfills. Please ensure that PHPUnit Polyfills 1.0.1 or higher is loaded.
Expected error message ✅
#### With Path Constant Defined to Valid Path
Defined the constant in wp-tests-config.php
to a valid path:
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', './valid_dir/vendor/yoast/phpunit-polyfills/' );
}}}
Results:
Error: Version mismatch detected for the PHPUnit Polyfills. Please ensure that PHPUnit Polyfills 1.0.1 or higher is loaded.
Expected error message ✅
#### With Path Constant Defined to Invalid Path
Defined the constant in wp-tests-config.php
to a invalid path:
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', './doesnotexist/vendor/yoast/phpunit-polyfills/' );
}}}
Results:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. The PHPUnit Polyfills autoload file was not found in "./doesnotexist/vendor/yoast/phpunit-polyfills/" Please verify that the file path provided in the WP_TESTS_PHPUNIT_POLYFILLS_PATH constant is correct.
Expected error message ✅
3 years ago
#105
For visibility (on Trac) re-posting my inline reply to @hellofromtonya's earlier question:
I did it like this _on purpose_ as, in the case the constant is used, WordPress has no awareness of the type of installation (and shouldn't care either), so we cannot presume that it will be a Composer install.
The constant is expected to be pointed to the _installation directory_ for the PHPUnit Polyfills.
This can be - and most likely will be - a Composervendor/...
directory, but it can also be a stand-alone install of the PHPUnit Polyfills (tested and works just fine), a directory containing a copied in version of the Polyfills (not recommended, violates copyright, but will work), a custom install directory or even a Composer global install directory.
It is not for WordPress to dictate how a user who chooses to use the
WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant organizes their development environment, so no assumptions about this should be made in the message either.
Also notice that there is still a third scenario for which the same message from scenario 2 will be shown:
Scenario 3:
vendor
folder exist,composer install
orupdate
has been run.WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant defined, but contains a typo or otherwise points to the wrong directory, like when it would be pointed to thevendor
directory without the subpath.
Scenario 3 has since been tested.
@hellofromtonya There's still one (two) scenario untested in case you want to be complete...
Scenario: Polyfills autoload file already loaded before the WordPress bootstrap file has been run.
Variant: Polyfills autoload file already loaded before the WordPress bootstrap file has been run with a version lower than the minimum required version.
Both should also work as expected and a cursory test I did for this seemed to work fine, but confirmation by a second person testing this would, of course, be welcome.
hellofromtonya commented on PR #1563:
3 years ago
#106
Scenario: Polyfills autoload file already loaded before the WordPress bootstrap file has been run.
Variant: Polyfills autoload file already loaded before the WordPress bootstrap file has been run with a version lower than the minimum required version.
## More Testing
### * Scenario: Polyfills autoload file already loaded before the WordPress bootstrap file runs
How:
Added the following code to the wp-tests-config.php
file:
{{{php
require_once DIR . '/vendor/yoast/phpunit-polyfills/phpunitpolyfills-autoload.php';
}}}
Results:
No error messages and tests run as expected ✅
### * Scenario: Older version of Polyfills autoload file already loaded before the WordPress bootstrap file runs
How:
- Install polyfills
1.0.0
- Keep the same code from above in the
wp-tests-config.php
file
Results:
Error: Version mismatch detected for the PHPUnit Polyfills. Please ensure that PHPUnit Polyfills 1.0.1 or higher is loaded.
Expected error message ✅
SergeyBiryukov commented on PR #1563:
3 years ago
#108
I got this message (as expected) when testing the PR with the Polyfills 1.0:
Error: Version mismatch detected for the PHPUnit Polyfills. Please ensure that PHPUnit Polyfills 1.0.1 or higher is loaded.
However, I don't have the WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant defined, and just forgot to run composer update :slightly_smiling_face:
If the constant is not defined, would it make sense to add a sentence like "Please run composer update
to install the latest version"?
3 years ago
#109
If the constant is not defined, would it make sense to add a sentence like "Please run
composer update
to install the latest version"?
Great suggestion. Applied.
hellofromtonya commented on PR #1563:
3 years ago
#111
Closing via changeset 51598.
#113
@
3 years ago
Patch 46149-hard-deprecate-checkRequirements.patch
is a follow-up to the previous round of changes where this method, which is not used by WP Core was missed.
Full description for the patch:
Build/Test Tools: hard deprecate WP_UnitTestCase_Base::checkRequirements().
The WP_UnitTestCase_Base::checkRequirements()
method calls the parent::checkRequirements()
method, but this method became private
in PHPUnit 7.0.0 via commit sebastianbergmann/phpunit@932238a.
Aside from that the TestCase::getAnnotations()
method which is called next is now also removed in PHPUnit 9.5.0.
WP Core doesn't use the method anymore and the method only remains to prevent potentially breaking external integration tests relying on the method.
However, in effect, the method is not functional anymore knowing the above.
So, I'm proposing to hard deprecate the method, effectively rendering it useless, except to prevent breaking test suites which still called the method.
#114
@
3 years ago
Looking at 46149-hard-deprecate-checkRequirements.patch, I was wondering if we still need something to handle the ms-required
and ms-excluded
groups, but then realized they are already handled in configuration files:
ms-required
is excluded in single-sitephpunit.xml.dist
.ms-excluded
is excluded intests/phpunit/multisite.xml
.
So the patch should be good to go.
#115
@
3 years ago
Some history on the ms-required
and ms-excluded
groups:
It looks like handling of these groups was added to WP_UnitTestCase_Base::checkRequirements()
to avoid fatal errors in case they are not properly excluded in plugin/theme PHPUnit configuration files, or in case plugin/theme test suites don't have a multisite-specific configuration file.
In practice, as the ::checkRequirements()
method has silently stopped being called with the update to PHPUnit 7 in [44701] / #43218, it seems safe to deprecate it.
#117
follow-up:
↓ 122
@
3 years ago
@jrf Looks like [51602] broke the tests for PHP 5.6 and 7.0 (PHPUnit 5.7.27 and PHPUnit 6.5.14, respectively):
There was 1 error: 1) Tests_oEmbed_HTTP_Headers::test_rest_pre_serve_request_headers Error: Call to undefined function xdebug_get_headers() /var/www/tests/phpunit/tests/oembed/headers.php:32
I guess we'll still need to call parent::checkRequirements()
there? That fixes the tests for me.
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
#121
in reply to:
↑ 100
;
follow-up:
↓ 123
@
3 years ago
Replying to jrf:
I've just opened a PR on GitHub with a proposal to make the loading of the PHPUnit Polyfills package more flexible.
I'd really appreciate it if you could look it over, test it and would give your opinion.
I know I'm probably a bit late as the changes already appear to be merged in but this *works perfectly* for my needs. I've been able to switch my test utils framework, drop phpunit as a dependency and include the PHPUnit Polyfills and it all loads as expected (since I'm including my composer autoloader (that makes PHPUnit Polyfills autoloader available) in my own bootstrap file loaded via my phpunit.xml
config file.
I can't exactly switch today (but I also *don't have to*) as I have quite a bit of other changes to make (namely switching incompatible fixture usage over to snake case methods) and then there's some deprecations in PHP Unit 8+ that we're still using that we'll have to switch off of. But that's for me to deal with.
Thanks so much for all the hard work here @jrf -- I'm very excited to see the version compatibility matrix nightmare being handled by the WP Core so that I can run my testing matrix with a relatively small number of headaches. A million thank yous <3
#122
in reply to:
↑ 117
@
3 years ago
Replying to SergeyBiryukov:
@jrf Looks like [51602] broke the tests for PHP 5.6 and 7.0 (PHPUnit 5.7.27 and PHPUnit 6.5.14, respectively):
There was 1 error: 1) Tests_oEmbed_HTTP_Headers::test_rest_pre_serve_request_headers Error: Call to undefined function xdebug_get_headers() /var/www/tests/phpunit/tests/oembed/headers.php:32I guess we'll still need to call
parent::checkRequirements()
there? That fixes the tests for me.
Correct. As the method in PHPUnit 5 and 6 is still an overload of the PHPUnit native method, the @requires function xdebug_get_headers
tags in the test method docblocks were not being correctly handled (for the two tests which are normally only run in the xdebug
group).
Let's prevent the fatal error on PHPUnit >= 7.0 though, just in case people would call the WP version of the method in their tests.
I've uploaded a fixed version of the patch and ran a test build which is now passing:
https://github.com/jrfnl/wordpress-develop-official/actions/runs/1121782154
#123
in reply to:
↑ 121
@
3 years ago
Replying to thomasplevy:
I know I'm probably a bit late as the changes already appear to be merged in but this *works perfectly* for my needs.
...
I'm very excited to see the version compatibility matrix nightmare being handled by the WP Core so that I can run my testing matrix with a relatively small number of headaches. A million thank yous <3
Thanks Thomas! Very happy to hear that the solution now committed works for you and yes, I'm really hoping the setup now adopted will sustain WP Core for quite a few years to come with PHPUnit Polyfills adding compatibility for future versions of PHPUnit. Hoping it will make life a lot easier in the long run, both for Core as well as for extenders.
This ticket was mentioned in PR #1587 on WordPress/wordpress-develop by jrfnl.
3 years ago
#125
@schlessera gave me some really good further feedback on the messages shown when running the WP Core test bootstrap file in a plugin/theme integration test situation, most notably about when the plugin/theme test suite has not been adjusted yet for the changes in WP 5.9.
This PR attempts to address these comments.
There is one comment @schlessera made which I haven't addressed, which is to do with the use of composer update
in the messages versus composer install
.
Alain correctly stated that running composer install
when there is no composer.lock
file in place, is effectively the same as running composer update
and will prevent accidental updates to other dependencies.
To address this, I have made sure that the composer update
messages are now only shown in a WP Core test run context.
I have, however, not changed the message from composer update
to composer install
as WP Core, as of recently, no longer has a composer.lock
file and if a user running the core tests has already previously run composer install
, they will have a local-only composer.lock
file in place.
In that situation running composer install
will not update the dependency, it will verify the existing installation against the lock
file and if necessary, (re-)install the dependencies as per the composer.lock
file. It will also show a message that the lock
file is out of date, but a next test run would in still be blocked by the outdated dependency.
@schlessera I can't tag you as a reviewer, but would appreciate it if you would look this PR over as well.
## Commit details
### Tests/bootstrap: make constant setting more flexible
The constant WP_TESTS_PHPUNIT_POLYFILLS_PATH
is intended to contain the path to the root directory of the PHPUnit Polyfills library without trailing slash.
The code already took into account that the value could potentially include a trailing slash.
Now it will also take into account if it is accidentally set to point to the autoload file instead of the path.
### Tests/bootstrap: improve messaging when polyfills can not be found
Previously, two situations were taken in to account:
- The
WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant is defined => show message specific to that constant not being set correctly.
This message would typically be shown for plugin/theme integration tests which are already aware of the changes in WP 5.9.
- The constant is not defined => show a message to run
composer update
.
This message is intended for people trying to run the WP Core tests.
This left two situations unaccounted for:
- Someone trying to run the WP Core tests, but not having set the
WP_RUN_CORE_TESTS
constant or not having set it to1
. - Someone trying to run plugin/theme integration tests without the new
WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant being defined as they are not (yet) aware of the changes made in WP 5.9.
The changes made in this commit, are intended to improve the error messages displayed in those situations.
### Tests/bootstrap: improve messaging when polyfills do not comply with version requirements
Previously, two situations were taken into account:
- The
WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant is defined => just show a message about the version mismatch. - The constant is not defined => show a message to run
composer update
.
This message is intended for people trying to run the WP Core tests.
This could lead to an unclear situation for people trying to run plugin/theme integration tests without the new WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant being defined.
They could be shown the message to run composer update
while if they would do so for their local install without adding the Polyfills, the message would still display the next time they would attempt to run the tests.
This commit:
- Provides more information about the PHPUnit Polyfills version detected vs the version expected.
- Shows a more specific message to guide users which have the
WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant declared. - Only shows the message to run
composer update
when theWP_RUN_CORE_TESTS
constant is declared to prevent confusing people more.
Trac ticket: https://core.trac.wordpress.org/ticket/46149
#126
@
3 years ago
FYI: I've opened a new PR with a further iteration on the test bootstrap messages around the Polyfills requirement. Feedback and reviews welcome.
hellofromtonya commented on PR #1587:
3 years ago
#127
## Test Report
### Scenario 1: Someone trying to run the WP Core tests, but not having set the WP_RUN_CORE_TESTS
constant or not having set it to 1
define( 'WP_RUN_CORE_TESTS', false );
- The constant
WP_TESTS_PHPUNIT_POLYFILLS_PATH
is not defined - did not run
composer install
orcomposer update
- ran tests
Results:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. If you are trying to run plugin/theme integration tests, make sure the PHPUnit Polyfills library is available and either load the autoload file of this library in your own test bootstrap before calling the WP Core test bootstrap file; or set the path to the PHPUnit Polyfills library in a "WP_TESTS_PHPUNIT_POLYFILLS_PATH" constant to allow the WP Core bootstrap to load the Polyfills. If you are trying to run the WP Core tests, make sure to set the "WP_RUN_CORE_TESTS" constant to 1 and run `composer update` before running the tests. Once the dependencies are installed, you can run the tests using the Composer-installed version of PHPUnit or using a PHPUnit phar file, but the dependencies do need to be installed whichever way the tests are run.
Works as expected ✅
### Scenario 2: Someone trying to run plugin/theme integration tests without the new WP_TESTS_PHPUNIT_POLYFILLS_PATH
constant being defined as they are not (yet) aware of the changes made in WP 5.9
Steps to test:
- Run
composer install
- Create a new directory in the root of the project called `my-vendors'
- Move the
yoast
folder fromvendor
folder and put it into the newmy-vendors
folder - Add the following code to the
wp-test-config.php
file:
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', './my-vendors/yoast/phpunit-polyfills/phpunitpolyfills-autoload.php' );
}}}
- Run the tests
Results:
- Tests are successfully ✅
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
hellofromtonya commented on PR #1587:
3 years ago
#129
# Test Report
## Plugin/theme integration tests
### Scenario 1: Constant not defined and no Composer packages installed
WP_RUN_CORE_TESTS
is0
- The constant
WP_TESTS_PHPUNIT_POLYFILLS_PATH
is not defined - Composer packages not installed
- Run the tests
Results:
- Tests do not run
- Error message in command-line:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. If you are trying to run plugin/theme integration tests, make sure the PHPUnit Polyfills library (https://github.com/Yoast/PHPUnit-Polyfills) is available and either load the autoload file of this library in your own test bootstrap before calling the WP Core test bootstrap file; or set the absolute path to the PHPUnit Polyfills library in a "WP_TESTS_PHPUNIT_POLYFILLS_PATH" constant to allow the WP Core bootstrap to load the Polyfills. If you are trying to run the WP Core tests, make sure to set the "WP_RUN_CORE_TESTS" constant to 1 and run `composer update` before running the tests. Once the dependencies are installed, you can run the tests using the Composer-installed version of PHPUnit or using a PHPUnit phar file, but the dependencies do need to be installed whichever way the tests are run.
Works as expected ✅
### Scenario 2: Constant defined as relative path with Composer packages installed
WP_RUN_CORE_TESTS
is0
- Composer packages are installed, but in different folder than
vendor
. This can be set in the plugin/theme'scomposer.json
file, for example:"config": { "vendor-dir": "./plugin-vendors" }
- Constant is defined as a relative path. For example:
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', '/plugin-vendors/yoast/phpunit-polyfills' );
}}}
- Run the tests
Results:
- Tests do not run
- Error message in command-line:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. The PHPUnit Polyfills autoload file was not found in "/plugin-vendors/yoast/phpunit-polyfills" Please verify that the file path provided in the WP_TESTS_PHPUNIT_POLYFILLS_PATH constant is correct. The WP_TESTS_PHPUNIT_POLYFILLS_PATH constant should contain an absolute path to the root directory of the PHPUnit Polyfills library.
Works as expected ✅
### Scenario 3: Constant defined as absolute path without the autoload file with Composer packages installed.
- Repeat previous scenario
- Change the constant to an absolute path, e.g.:
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', DIR . '/plugin-vendors/yoast/phpunit-polyfills' );
}}}
- Run the tests
Results:
- No error messages ✅
- Tests run ✅
Works as expected ✅
### Scenario 4: Constant defined as absolute path with the autoload file.
- Repeat previous scenario
- Change constant to include autoload file, e.g.:
{{{php
define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', DIR . '/plugin-vendors/yoast/phpunit-polyfills/phpunitpolyfills-autoload.php' );
}}}
- Run the tests
Results:
- No error messages ✅
- Tests run ✅
Works as expected ✅
### Scenario 3: Version mismatch
- Repeat previous scenario
- Wrong version of Polyfills package is installed, e.g.:
composer require yoast/phpunit-polyfills:1.0.0
- Run the tests
Results:
- Tests do not run
- Error message in command-line:
Error: Version mismatch detected for the PHPUnit Polyfills. Please ensure that PHPUnit Polyfills 1.0.1 or higher is loaded. Found version: 1.0.0 or lower Please run `composer update` to install the latest version.
Works as expected ✅
## Core Tests
### Scenario 1: WP_RUN_CORE_TESTS
constant set it to 0
WP_RUN_CORE_TESTS
is0
- The constant
WP_TESTS_PHPUNIT_POLYFILLS_PATH
is not defined - Composer packages not installed
- Run the tests
Results:
- Tests do not run
- Error message in command-line:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. If you are trying to run plugin/theme integration tests, make sure the PHPUnit Polyfills library (https://github.com/Yoast/PHPUnit-Polyfills) is available and either load the autoload file of this library in your own test bootstrap before calling the WP Core test bootstrap file; or set the absolute path to the PHPUnit Polyfills library in a "WP_TESTS_PHPUNIT_POLYFILLS_PATH" constant to allow the WP Core bootstrap to load the Polyfills. If you are trying to run the WP Core tests, make sure to set the "WP_RUN_CORE_TESTS" constant to 1 and run `composer update` before running the tests. Once the dependencies are installed, you can run the tests using the Composer-installed version of PHPUnit or using a PHPUnit phar file, but the dependencies do need to be installed whichever way the tests are run.
Works as expected ✅
### Scenario 2: Composer packages not installed
WP_RUN_CORE_TESTS
is1
- The constant
WP_TESTS_PHPUNIT_POLYFILLS_PATH
is not defined - did not run
composer install
orcomposer update
- ran tests
Results:
- Tests do not run
- Error message in command-line:
Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. You need to run `composer update` before running the tests. Once the dependencies are installed, you can run the tests using the Composer-installed version of PHPUnit or using a PHPUnit phar file, but the dependencies do need to be installed whichever way the tests are run.
Works as expected ✅
### Scenario 3: Version mismatch
WP_RUN_CORE_TESTS
is1
- Composer packages are installed
- Wrong version of Polyfills package is installed, e.g.:
composer require yoast/phpunit-polyfills:1.0.0
- Run the tests
Results:
- Tests do not run
- Error message in command-line:
Error: Version mismatch detected for the PHPUnit Polyfills. Please ensure that PHPUnit Polyfills 1.0.1 or higher is loaded. Found version: 1.0.0 or lower Please run `composer update` to install the latest version.
Works as expected ✅
hellofromtonya commented on PR #1587:
3 years ago
#130
Per feedback and consensus, the latest changes require the Polyfills path to be absolute. In my testing, changes work as expected.
Next step plan: Tomorrow I'll commit this PR unless there are any objections or further feedback that requires action.
Why tomorrow? Gives more time for folks to test and review.
hellofromtonya commented on PR #1587:
3 years ago
#136
Committed in the following changesets: 51810-51813.
3 years ago
#137
FYI: PR #1588 (backporting of the test changes to WP 5.2 - 5.8) has been updated to be in sync with the additional changes which were proposed in this PR as they have now been committed.
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
#141
@
3 years ago
- Keywords has-dev-note added; needs-dev-note removed
- Resolution set to fixed
- Status changed from assigned to closed
With the publication of the dev note, all work for the scope of this ticket is complete. Closing this ticket.
Thank you everyone for collaborating together to make this happen!
We could also look into displaying a friendly message instead of a fatal error. Something like: