Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

[Dont Merge Yet] Fix PHP unit errors with wp-env phpunit container removal #182

Merged
merged 5 commits into from
May 22, 2023

Conversation

johnstonphilip
Copy link
Contributor

@johnstonphilip johnstonphilip commented May 19, 2023

Tasks

Summary of changes

In the wp-env project (inside Gutenberg repo), the PHPUnit and Composer packages were removed.

This resulted in PHPunit tests failing to run using wpp-scripts with the message:
The 'phpunit' container has been removed. Please use 'wp-env run tests-cli --env-cwd=wp-content/path/to/plugin phpunit' instead.

In fixing it, I was getting this error:
PHP Fatal error: Declaration of PatternManager\PatternDataHandlers\PatternDataHandlersTest::setUp() must be compatible with Yoast\PHPUnitPolyfills\TestCases\TestCase::setUp(): void in /var/www/html/wp-content/plugins/pattern-manager/wp-modules/pattern-data-handlers/tests/PatternDataHandlersTest.php on line 23

The fix is to make sure that setUp and tearDown declarations have a void declared for their return type.

I was also seeing errors for Array to string conversion on test_register_pattern_post_type_meta_types. This is apparently the error message you get if you use array_diff but one of them is multi-dimentional.

How to test

  1. Make sure tests pass on circleci, or run them locally with npm run test:phpunit

@@ -54,11 +72,23 @@ public function test_register_pattern_post_type_meta_types() {
public function test_register_pattern_post_type_meta_defaults() {
register_pattern_post_type();

foreach ( array_diff( get_pattern_defaults(), [ 'title' => null ] ) as $meta_key => $default_value ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I had to change this was because it was failing with the error Fatal error: Array to string conversion. This is apparently the error message if you use array_diff with a multi-dimentional array. See:
https://stackoverflow.com/questions/19830585/array-to-string-conversion-error-when-calling-array-diff-assoc-with-a-multid

See lines 41 and 57:
Screenshot 2023-05-19 at 4 41 44 PM

I broke it out to be a bit easier for me to understand as well. I had a hard time fully understanding what was happening without breaking it out a bit.

Copy link
Contributor

@kienstra kienstra May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience with this.

My obsession with terse code gave you some nasty debugging.

@johnstonphilip johnstonphilip changed the title Fix PHP unit errors with wp-env phpunit container removal [Dont Merge Yet] Fix PHP unit errors with wp-env phpunit container removal May 19, 2023
@johnstonphilip
Copy link
Contributor Author

Before merging, remove the branch in package.json referring to this PR in wpps-scripts:
wp-plugin-sidekick/wpps-scripts#37

Copy link
Contributor

@kienstra kienstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @johnstonphilip,
Thanks a lot for figuring this out!

@johnstonphilip johnstonphilip merged commit a8cf26c into main May 22, 2023
@johnstonphilip johnstonphilip deleted the fix/phpunit branch May 22, 2023 18:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants