scaffold-command icon indicating copy to clipboard operation
scaffold-command copied to clipboard

Improve PHPUnit integration tests

Open swissspidy opened this issue 2 years ago • 10 comments

Feature Request

Describe your use case and the problem you are facing

#297 sort of fixed testing on WP trunk, but it requires setting the WP_TESTS_PHPUNIT_POLYFILLS_PATH environment variable.

Forcing devs to run a command like this is not ideal:

WP_TESTS_DIR=/path/to/wordpress-tests-lib WP_TESTS_PHPUNIT_POLYFILLS_PATH=/path/to/wordpress-tests-lib/vendor/yoast/phpunit-polyfills ./phpunit -c phpunit.xml.dist

Describe the solution you'd like

Should the PHPUnit polyfills automatically be included in the scaffolded plugin? If so, how? Introducing composer would be quite the change.

Should we perhaps hardcode the polyfills path in the bootstrap file instead of checking an env var?

swissspidy avatar Sep 24 '21 10:09 swissspidy

Introducing composer would be quite the change.

As this is for testing only, I don't think it is that big of a deal, tbh, and it would solve a lot of issues with the current setup.

We could even go so far as to have a package be pulled in that manages all of the files that would normally be scaffolded, which then makes the entire thing composer updateable, instead of being stuck at whatever state the WP-CLI scaffold command was at the time...

schlessera avatar Sep 24 '21 16:09 schlessera

As this is for testing only, I don't think it is that big of a deal, tbh, and it would solve a lot of issues with the current setup.

+1 to this idea.

It's worth mentioning that as of WP 5.8.2 it looks like the current testing setup is broken due to the missing polyfills. 5.8.1 still works.

squelchdesign avatar Nov 11 '21 10:11 squelchdesign

It's worth mentioning that as of WP 5.8.2 it looks like the current testing setup is broken due to the missing polyfills. 5.8.1 still works.

Correct: the test changes from trunk have been backported to WP 5.2 - 5.8 and for 5.8, the first tag which included this backport is 5.8.2.

jrfnl avatar Nov 11 '21 11:11 jrfnl

Is there documentation on how the polyfill should be setup? I looked at WP_TESTS_PHPUNIT_POLYFILLS_PATH and expected to see it defined in install-wp-tests.sh, but it was not.

I ended up running composer require yoast/phpunit-polyfills --dev and adding require dirname( dirname( __FILE__ ) ) . '/vendor/yoast/phpunit-polyfills/phpunitpolyfills-autoload.php'; to tests/bootstrap.php.

grappler avatar Mar 16 '22 11:03 grappler

As per the description, the script currently expects WP_TESTS_PHPUNIT_POLYFILLS_PATH to be set as an environment variable. This issue is about improving that, which is why you don't see it yet in install-wp-tests.sh or bootstrap.php.

swissspidy avatar Mar 16 '22 11:03 swissspidy

@grappler Are you by chance looking for this write-up about it ? https://make.wordpress.org/core/2021/09/27/changes-to-the-wordpress-core-php-test-suite/ (In particular the "What does this mean for plugins/themes running integration tests based on the WP Core test suite?" section - bullet 4)

I ended up running composer require yoast/phpunit-polyfills --dev and adding require dirname( dirname( FILE ) ) . '/vendor/yoast/phpunit-polyfills/phpunitpolyfills-autoload.php'; to tests/bootstrap.php.

That is the right way to do it. If you can't (or don't want to) add a test/bootstrap.php file, then the environment variable is a fall-back alternative.

You can also have a look at the WP Test Utils repo which can handle the bootstrapping for you and plays nice with the install-wp-tests.sh script.

jrfnl avatar Mar 16 '22 11:03 jrfnl

Thanks @jrfnl! It is clearer now. I knew about the write-up, but missed the section regarding the plugins.

grappler avatar Mar 16 '22 11:03 grappler

@grappler Glad it helped ;-)

jrfnl avatar Mar 16 '22 12:03 jrfnl

@swissspidy Want to put together a pull request for this?

danielbachhuber avatar Jan 30 '23 18:01 danielbachhuber

Not sure I have bandwidth for that right now unfortunately

swissspidy avatar Jan 30 '23 20:01 swissspidy