Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix php8.2 test error, xdebug default off on testing, allow php version switching #1511

Merged
merged 11 commits into from
Jul 24, 2024

Conversation

tharsheblows
Copy link
Contributor

@tharsheblows tharsheblows commented Jul 18, 2024

Fixes #1278

  • adds npm script for switching php versions easily
  • does not start xdebug by default
  • adds in npm script for running phpunit tests with xdebug
  • sets WP_DEBUG to false by default in the tests but allows an override with an environment variable
  • updates Test_WP_Stream_Connector_Media so that test_callback_wp_save_image_editor_file does not error

Checklist

  • Project documentation has been updated to reflect the changes in this pull request, if applicable.
  • I have tested the changes in the local development environment (see contributing.md).
  • I have added phpunit tests.

Release Changelog

  • Fix: Test error in PHP 8.2
  • New: Local environment enhancements including PHP version switching

@tharsheblows tharsheblows requested a review from delawski July 18, 2024 20:13
Copy link
Contributor

@delawski delawski left a comment

Choose a reason for hiding this comment

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

I like where this is going! 😄 I tested it locally and ran into some issues.

When I switched to PHP 8.2, I got dozens of deprecation notices coming from Jetpack, EDD and ACF. It turns out the plugin versions are pinned to some very old releases incomposer.json:

stream/composer.json

Lines 30 to 32 in ffdaa20

"wpackagist-plugin/advanced-custom-fields": "5.8.12",
"wpackagist-plugin/easy-digital-downloads": "2.9.23",
"wpackagist-plugin/jetpack": "10.0",

Those versions do not support for PHP 8.2. I updated them to the latest versions and while the deprecations went away, the EDD tests started to fail.

Here you'll find the PHPUnit errors returned
There were 7 errors:

1) WP_Stream\Test_Alerts::test_load_alerts_settings
Error: Class 'EDD_Stripe_Admin_Notices_Registry' not found

/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/functions.php:442
/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/payment-methods/apple-pay.php:17
/var/www/html/wp/wp-includes/class-wp-hook.php:324
/var/www/html/wp/wp-includes/class-wp-hook.php:348
/var/www/html/wp/wp-includes/plugin.php:517
/var/www/html/wp-content/plugins/stream-src/vendor/wp-phpunit/wp-phpunit/includes/testcase-ajax.php:264
/var/www/html/wp-content/plugins/stream-src/tests/tests/test-class-alerts.php:238
phpvfscomposer:///var/www/html/wp-content/plugins/stream-src/vendor/phpunit/phpunit/phpunit:106

2) WP_Stream\Test_Alerts::test_load_alerts_settings_bad_alert_type
Error: Class 'EDD_Stripe_Admin_Notices_Registry' not found

/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/functions.php:442
/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/payment-methods/apple-pay.php:17
/var/www/html/wp/wp-includes/class-wp-hook.php:324
/var/www/html/wp/wp-includes/class-wp-hook.php:348
/var/www/html/wp/wp-includes/plugin.php:517
/var/www/html/wp-content/plugins/stream-src/vendor/wp-phpunit/wp-phpunit/includes/testcase-ajax.php:264
/var/www/html/wp-content/plugins/stream-src/tests/tests/test-class-alerts.php:266
phpvfscomposer:///var/www/html/wp-content/plugins/stream-src/vendor/phpunit/phpunit/phpunit:106

3) WP_Stream\Test_Alerts::test_load_alerts_settings_missing_caps
Error: Class 'EDD_Stripe_Admin_Notices_Registry' not found

/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/functions.php:442
/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/payment-methods/apple-pay.php:17
/var/www/html/wp/wp-includes/class-wp-hook.php:324
/var/www/html/wp/wp-includes/class-wp-hook.php:348
/var/www/html/wp/wp-includes/plugin.php:517
/var/www/html/wp-content/plugins/stream-src/vendor/wp-phpunit/wp-phpunit/includes/testcase-ajax.php:264
/var/www/html/wp-content/plugins/stream-src/tests/tests/test-class-alerts.php:296
phpvfscomposer:///var/www/html/wp-content/plugins/stream-src/vendor/phpunit/phpunit/phpunit:106

4) WP_Stream\Test_Alerts::test_get_actions
Error: Class 'EDD_Stripe_Admin_Notices_Registry' not found

/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/functions.php:442
/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/payment-methods/apple-pay.php:17
/var/www/html/wp/wp-includes/class-wp-hook.php:324
/var/www/html/wp/wp-includes/class-wp-hook.php:348
/var/www/html/wp/wp-includes/plugin.php:517
/var/www/html/wp-content/plugins/stream-src/vendor/wp-phpunit/wp-phpunit/includes/testcase-ajax.php:264
/var/www/html/wp-content/plugins/stream-src/tests/tests/test-class-alerts.php:368
phpvfscomposer:///var/www/html/wp-content/plugins/stream-src/vendor/phpunit/phpunit/phpunit:106

5) WP_Stream\Test_Alerts::test_save_new_alert_with_parent_context
Error: Class 'EDD_Stripe_Admin_Notices_Registry' not found

/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/functions.php:442
/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/payment-methods/apple-pay.php:17
/var/www/html/wp/wp-includes/class-wp-hook.php:324
/var/www/html/wp/wp-includes/class-wp-hook.php:348
/var/www/html/wp/wp-includes/plugin.php:517
/var/www/html/wp-content/plugins/stream-src/vendor/wp-phpunit/wp-phpunit/includes/testcase-ajax.php:264
/var/www/html/wp-content/plugins/stream-src/tests/tests/test-class-alerts.php:391
phpvfscomposer:///var/www/html/wp-content/plugins/stream-src/vendor/phpunit/phpunit/phpunit:106

6) WP_Stream\Test_Alerts::test_save_new_alert_with_child_context
Error: Class 'EDD_Stripe_Admin_Notices_Registry' not found

/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/functions.php:442
/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/payment-methods/apple-pay.php:17
/var/www/html/wp/wp-includes/class-wp-hook.php:324
/var/www/html/wp/wp-includes/class-wp-hook.php:348
/var/www/html/wp/wp-includes/plugin.php:517
/var/www/html/wp-content/plugins/stream-src/vendor/wp-phpunit/wp-phpunit/includes/testcase-ajax.php:264
/var/www/html/wp-content/plugins/stream-src/tests/tests/test-class-alerts.php:413
phpvfscomposer:///var/www/html/wp-content/plugins/stream-src/vendor/phpunit/phpunit/phpunit:106

7) WP_Stream\Test_Alerts::test_get_new_alert_triggers_notifications
Error: Class 'EDD_Stripe_Admin_Notices_Registry' not found

/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/functions.php:442
/var/www/html/wp-content/plugins/easy-digital-downloads/includes/gateways/stripe/includes/payment-methods/apple-pay.php:17
/var/www/html/wp/wp-includes/class-wp-hook.php:324
/var/www/html/wp/wp-includes/class-wp-hook.php:348
/var/www/html/wp/wp-includes/plugin.php:517
/var/www/html/wp-content/plugins/stream-src/vendor/wp-phpunit/wp-phpunit/includes/testcase-ajax.php:264
/var/www/html/wp-content/plugins/stream-src/tests/tests/test-class-alerts.php:555
phpvfscomposer:///var/www/html/wp-content/plugins/stream-src/vendor/phpunit/phpunit/phpunit:106

The PHPUnit output is preceded with a wall of DB-related errors like this one:

[19-Jul-2024 08:51:26 UTC] WordPress database error Table 'wordpress_test.wptests_edd_customer_email_addresses' doesn't exist for query SELECT * FROM wptests_edd_customer_email_addresses WHERE email = '[email protected]' made by include('phpvfscomposer:///plugins/stream-src/vendor/phpunit/phpunit/phpunit'), PHPUnit\TextUI\Command::main, PHPUnit\TextUI\Command->run, PHPUnit\TextUI\TestRunner->run, PHPUnit\Framework\TestSuite->run, PHPUnit\Framework\TestSuite->run, PHPUnit\Framework\TestSuite->run, PHPUnit\Framework\TestCase->run, PHPUnit\Framework\TestResult->run, PHPUnit\Framework\TestCase->runBare, WP_Stream\Test_Author->setUp, WP_UnitTest_Factory_For_Thing->create, WP_UnitTest_Factory_For_User->create_object, wp_insert_user, do_action('user_register'), WP_Hook->do_action, WP_Hook->apply_filters, edd_connect_existing_customer_to_new_user, EDD_Customer->__construct, edd_get_customer_by, EDD\Database\Query->get_item_by, EDD\Database\Query->get_item_raw

It all looks like Stream is not compatible with the latest version of EDD. I'm not sure if the fix would be something easy or not.

Did you get anything like that when testing on PHP 8.2? Maybe it's something with my local setup?

package.json Show resolved Hide resolved
tests/data/images/icon-128x128.png Outdated Show resolved Hide resolved
@delawski delawski added this to the 4.0.1 milestone Jul 19, 2024
@delawski delawski added the tooling Development and deployment tooling improvements label Jul 19, 2024
This does not do any refactoring or fixing of the actions used in the tests, in particular EDD discounts are not consistent within that plugin as of version 3.0.
This keeps the image in git from being updated
Switching blogs in the Blogs connector ended up causing issues in WP SEO which in turn caused issues in EDD. Maybe try switching back to the original blog in the cleanup for that test.
@tharsheblows tharsheblows requested a review from delawski July 23, 2024 19:43
Copy link
Contributor

@delawski delawski left a comment

Choose a reason for hiding this comment

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

@tharsheblows This looks great. Thank you!

I ran the tests on my local using both PHP 7.4 and PHP 8.2 and everything went well. I'm approving and merging now.

@delawski delawski merged commit fd3a560 into develop Jul 24, 2024
2 checks passed
@delawski delawski deleted the update/1278-phpunit-php8-compat branch July 24, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling Development and deployment tooling improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix PHP unit tests with PHP 8
2 participants