-
Notifications
You must be signed in to change notification settings - Fork 2
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
PHPUnit tests with @runInSeparateProcess fail with PHP 8.0 and Windows #8
Comments
@pondermatic have you found anything actionalable here... I can't find anything online about a windows 10 conflict there Removing the annotation will cause issues because when using this annotation we generally test things that will affect the PHP process and cause other tests to behave in unexpected ways, usually this is done when testing things related to constants or singleton instances |
It appears that the current solution is to run LifterLMS unit tests with PHP 7.3 as that is the last version that is compatible with PHPUnit 7.5. Until WordPress and LifterLMS transition to a newer version of PHPUnit, it may be possible to use a modified version of PHPUnit 7.5 as @konamiman at Automattic has done, although that version does not fix this issue and it adds additional complexity to this project for probably very little gain. |
@pondermatic if that's the case then this trac ticket (on my immediate priority list) is going to help us resolve this: https://core.trac.wordpress.org/ticket/46149 The WP Core is making big steps towards reducing the enumerable issues inherent in trying to run PHP Unit with WordPress across the various WP versions that need supporting. I started working on switching us to use this library (which the WP Core is now using on the nightly branch) https://github.com/Yoast/PHPUnit-Polyfills I got stuck and a recent commit (two days ago) will make it possible for us to switch over to using this which I hope will resolve this too. If you can manage to deal with this for a bit I'll hopefully have us switched over in a week or two... I'll work on it today a bit and have a better idea on timelines. |
I can easily switch unit testing to use PHP 7.2 instead of PHP 8.0. There is one test with a trailing comma in the arguments of a function call, which breaks if PHP < 8.0. Otherwise all tests run. 3 are skipped and 1 is risky, but 0 failures. |
@pondermatic check this branch out, if you have a moment: https://github.com/gocodebox/lifterlms/tree/phpunit-polyfill
If the problem we're encountering on Windows is PHPUnit version compatibility with Windows we should be good to go. Also, for whatever it's worth, there already are existing solutions to run PHPUnit on PHP 8 despite version support, we run Travis Tests against PHP 8 this way. It's convoluted and complicated and it's going to stop working with PHP 8.1 which is why I've been following the core trac ticket and keeping my eye on getting this situated in a less insane way... Once the WP core changes are backported (they're talking about backporting this to 5.2) I'll restructure the composer scripts, travis tests, etc... and we'll switch to using this across the board (other add-ons, etc...) |
Reproduction Steps
The LifterLMS unit tests that use the
@runInSeparateProcess
annotation return an error and are not tested when run with PHP 8.0 and Windows 10.The tests are successful if run with PHP 7.2 or the
@runInSeparateProcess
annotation is removed.Expected Behavior
The tests pass.
Actual Behavior
The tests fail with a "The filename, directory name, or volume label syntax is incorrect." message.
Error Messages / Logs
"The filename, directory name, or volume label syntax is incorrect."
This issue has been recreated:
The text was updated successfully, but these errors were encountered: