Build/Tests: modernize assertions used in PHPUnit using PHPUnit Polyfills (trac 46149) #1545
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 the `void` 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).
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-in `TestCase` classes from the PHPUnit Polyfills package can be used instead.
* Moves the `require` for the WP `abstract-testcase.php` to the `bootstrap.php` file.
* Adds a `require_once` for the PHPUnit Polyfills autoloader to the `bootstrap.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.
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`.
…nit 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
jrfnl
changed the title
... 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
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
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
…ression()` 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
jrfnl
force-pushed
the
trac-46149/implement-phpunit-polyfills-4-assertions
branch
from
4f94d5b
to
ee80981
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
TestCases which include these polyfills, but also solve thevoidreturn type issue for the fixtures methods.TestListenerimplementation.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:
TestCase.TestCase.WP_UnitTestCaseclass 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-in
TestCaseclasses from the PHPUnit Polyfills package can be used instead.requirefor the WPabstract-testcase.phpto thebootstrap.phpfile.require_oncefor the PHPUnit Polyfills autoloader to thebootstrap.phpfile.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
TestCases are/were expected to extend theWP_UnitTestCase.This order is not optimal as it means that the
WP_UnitTestCase_Baseclass 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_Baseto 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 explicitlyabstract.WP_UnitTestCase_Base::setExpectedException(): simplify redundant PHPUnit shim
PHPUnit 6 deprecated the
setExpectedException()method in favour of theexpectException(),expectExceptionMessage()andexpectExceptionCode()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:
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:
Tests: replace assertRegExp() with assertMatchesRegularExpression()
The
assertRegExp()andassertNotRegExp()methods were hard deprecated in PHPUnit 9.1 and the functionality will be removed in PHPUnit 10.0.0.The
assertMatchesRegularExpression()andassertDoesNotMatchRegularExpression()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:
Tests: replace assertNotRegExp() with
assertDoesNotMatchRegularExpression()The
assertRegExp()andassertNotRegExp()methods were hard deprecated in PHPUnit 9.1 and the functionality will be removed in PHPUnit 10.0.0.The
assertMatchesRegularExpression()andassertDoesNotMatchRegularExpression()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:
Trac ticket: https://core.trac.wordpress.org/ticket/46149
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.