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

Improve PHPUnit integration tests #299

Open
1 task done
swissspidy opened this issue Sep 24, 2021 · 10 comments
Open
1 task done

Improve PHPUnit integration tests #299

swissspidy opened this issue Sep 24, 2021 · 10 comments

Comments

@swissspidy
Copy link
Member

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?

@schlessera
Copy link
Member

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...

@squelchdesign
Copy link

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.

@jrfnl
Copy link
Contributor

jrfnl commented Nov 11, 2021

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.

@grappler
Copy link

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.

@swissspidy
Copy link
Member Author

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.

@jrfnl
Copy link
Contributor

jrfnl commented Mar 16, 2022

@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.

@grappler
Copy link

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

@jrfnl
Copy link
Contributor

jrfnl commented Mar 16, 2022

@grappler Glad it helped ;-)

@danielbachhuber
Copy link
Member

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

@swissspidy
Copy link
Member Author

Not sure I have bandwidth for that right now unfortunately

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants