Fix PHP 8 unit test GHA job; remove PHPUnit dependency and instead use PHPUnit polyfills package #6525
Conversation
…hRegularExpression()`
pierlon
force-pushed
the
enhancement/phpunit-polyfills
branch
from
15ce05f to
ebfd6e9
Compare
pierlon
changed the title
pierlon
added
dependencies
| @@ -16,7 +16,6 @@ public function setUp() { | |||
| $this->instance = $this->injector->make( PairedUrl::class ); | |||
| } | |||
|
|
|||
| /** @covers ::__construct() */ | |||
There was a problem hiding this comment.
Does this just not do anything?
There was a problem hiding this comment.
When running PHPUnit with coverage it complained that __construct() didn't exist in the class, so in short yes.
| @@ -2037,7 +2036,7 @@ public function get_external_stylesheet_data() { | |||
| ], | |||
| 'expected_styles' => [], | |||
| 'expected_errors' => [ AMP_Style_Sanitizer::STYLESHEET_FETCH_ERROR ], | |||
| 'cached_data' => new CachedResponse( | |||
| 'expected_cached_response' => new CachedResponse( | |||
There was a problem hiding this comment.
Does TestCase map associative arrays to named positional args?
| @@ -125,7 +126,7 @@ public function get_data_for_test_amp_get_customizer_url() { | |||
| Option::THEME_SUPPORT => AMP_Theme_Support::READER_MODE_SLUG, | |||
| Option::READER_THEME => ReaderThemes::DEFAULT_READER_THEME, | |||
| ], | |||
| function () { | |||
| 'assert' => function () { | |||
There was a problem hiding this comment.
Was this causing an error before or is it just a bug? It's strange that there's an associative array mixed with a numerical index.
There was a problem hiding this comment.
In PHP 8 it maps to a named parameter.
|
Plugin builds for 0c545ac are ready 🛎️!
|
Codecov Report
@@ Coverage Diff @@
## develop #6525 +/- ##
=============================================
- Coverage 75.52% 75.49% -0.04%
Complexity 5983 5983
=============================================
Files 190 190
Lines 18013 18013
=============================================
- Hits 13605 13599 -6
- Misses 4408 4414 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…mposer.lock after b9c7a40
|
I'd recommend using a plugin-specific See https://github.com/ampproject/amp-toolbox-php/blob/main/tests/src/TestCase.php for a simple example. This decouples the tests from whatever compatibility layer is needed. |
Summary
Note: Each commit in this PR represents an individual set of changes and so it's recommended to go through each commit instead of viewing all the files that changed.
This PR fixes the PHP 8 unit test GHA, along with some other changes that will allow us to easily adopt newer versions of PHPUnit when the time comes.
Changes
Replace deprecated PHPUnit assertions with their successors. This will allow us to easily switch to newer versions of PHPUnit in the future without hassle. (d846856, d1438ad, c828932, bf34961, cce7f32)
Remove
AssertContainsCompatibilitytrait as it is now unnecessary (0ec6eaf)Remove the
phpunit/phpunitComposer package and useyoast/wp-test-utilsto manage it instead. This allows us to remove the hard PHPUnit 5.7 requirement and delegate that responsibility towp-test-utilswhich will install the most recent compatible version of PHPUnit depending on the active PHP version (47a1e6b)Remove no longer necessary
composer.jsonpatch used to run PHPUnit tests on PHP 8 (25ae209)Update all tests to use the
Yoast\WPTestUtils\WPIntegration\TestCaseinstead ofWP_UnitTestCase. This will still allow us to use all the WP Core test utilities as it extendsWP_UnitTestCaseand will load the correct PHPUnit polyfills when necessary (ebfd6e9)Patch the
phpunit/phpunit-mock-objectspackage to prevent PHP warnings from occurring on PHP 8, which without it would cause some tests to fail (ffdcc78)Checklist