Also run unit tests on PHP 8.0 and 8.1 #119
Conversation
| npm run lint:pkg-json | ||
| if [ -n "$( git diff --diff-filter=d --staged --name-only | grep -E 'package.json' )" ] | ||
| then | ||
| npm run lint:pkg-json |
There was a problem hiding this comment.
If there's no change to package.json, no need to lint it in the pre-commit hook
| @@ -51,7 +51,7 @@ public function add_submenu_page() { | |||
| * Add submenu pages to the Genesis Custom Blocks menu. | |||
| */ | |||
| public function maybe_redirect() { | |||
| $page = filter_input( INPUT_GET, 'page', FILTER_SANITIZE_STRING ); | |||
| $page = filter_input( INPUT_GET, 'page' ); | |||
There was a problem hiding this comment.
FILTER_SANITIZE_STRING is deprecated in PHP 8.1.
By removing it here, it will use the default filter of FILTER_UNSAFE_RAW.
There was a problem hiding this comment.
Here's the deprecation notice:
Deprecated: Constant FILTER_SANITIZE_STRING is deprecated in /var/www/html/wp-content/plugins/project/php/Blocks/Loader.php on line 545
| @@ -27,8 +27,8 @@ class TestTemplateOutput extends AbstractAttribute { | |||
| * | |||
| * @inheritdoc | |||
| */ | |||
| public function setUp() { | |||
| parent::setUp(); | |||
| public function set_up() { | |||
There was a problem hiding this comment.
Changing setUp() to set_up(), like in Core, allows this to not have a :void return declaration, without causing a PHP error.
There was a problem hiding this comment.
It also prevents having an ugly phpcs:ignore on this line.
| @@ -148,14 +148,14 @@ public function test_block_template() { | |||
| $actual_template = str_replace( [ "\t", "\n" ], '', $rendered_template ); | |||
|
|
|||
| // The 'className' should be present. | |||
| $this->assertContains( | |||
| $this->assertStringContainsString( | |||
There was a problem hiding this comment.
assertContains() causes a PHPUnit error in PHP 8.1, as a string is not iterable
| @@ -11,27 +11,25 @@ references: | |||
| executors: | |||
| php: | |||
| docker: | |||
| - image: cimg/php:7.4.5-node | |||
| - image: cimg/php:7.4-node | |||
There was a problem hiding this comment.
The 5 isn't needed, any 7.4.* version should be fine
| description: "Ensure nodegit runs without error" | ||
| steps: | ||
| - run: sudo apt-get update && sudo apt-get install libkrb5-dev | ||
| - run: composer update |
There was a problem hiding this comment.
This'll ensure that the Composer packages are compatible with the PHP version, whether it be 5.6 or 8.1
|
|
||
| e2e-tests: | ||
| machine: | ||
| image: ubuntu-1604:202004-01 | ||
| image: ubuntu-2004:202111-02 |
There was a problem hiding this comment.
1604 is deprecated
| "squizlabs/php_codesniffer": "3.5.3", | ||
| "wp-coding-standards/wpcs": "2.2.0", | ||
| "phpunit/phpunit": "7.5.2" | ||
| "brain/monkey": "^2", |
There was a problem hiding this comment.
Uses a version of ^2 instead of 2.4.0, as it's fine for composer to get the latest 2.*
| @@ -132,7 +132,7 @@ abstract public function register_settings(); | |||
| /** | |||
| * Gets a JSON-serialized version of this object. | |||
| * | |||
| * @return array|mixed The JSON-serialized object. | |||
| * @return mixed The JSON-serialized object. | |||
There was a problem hiding this comment.
This was a PHP 8 notice:
Deprecated: Return type of Genesis\CustomBlocks\Blocks\Controls\ControlAbstract::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /var/www/html/wp-content/plugins/project/php/Blocks/Controls/ControlAbstract.php on line 137
| $abspath = ABSPATH; | ||
|
|
||
| // Workaround for weird hosting situations. | ||
| if ( trailingslashit( ABSPATH ) . 'wp-content' !== WP_CONTENT_DIR && isset( $_SERVER['DOCUMENT_ROOT'] ) ) { | ||
| $abspath = sanitize_text_field( wp_unslash( $_SERVER['DOCUMENT_ROOT'] ) ); | ||
| } | ||
|
|
||
| $stylesheet_url = str_replace( untrailingslashit( $abspath ), '', $path ); |
There was a problem hiding this comment.
This was a PHP 8 error in PHPUnit tests.
If $path is null, str_replace() can't accept null as the 3rd argument.
There was a problem hiding this comment.
So on line 212, this exits if $path is empty.
| <directory suffix=".php">./tests/php/Integration/Helpers/</directory> | ||
| <directory prefix="Test" suffix=".php">./tests/php/Integration/</directory> | ||
| </testsuite> | ||
| </testsuites> |
There was a problem hiding this comment.
Just changed the configuration to the new XML configuration, via:
composer test -- --migrate-configuration 
kienstra
requested review from
dreamwhisper,
johnstonphilip,
mikemcalister and
marksabbath
|
@dreamwhisper, thanks so much! |
Changes
8.0and8.1.8issues.Testing instructions
Not needed, just a sanity check would be great.