Make WordPress Core

Opened 4 years ago

Closed 18 months ago

#50482 closed defect (bug) (invalid)

Uncaught Exception: Serialization of 'Closure' is not allowed when run PHPUnit in plugin development

Reported by: yshinoda's profile yshinoda Owned by:
Milestone: Priority: normal
Severity: critical Version: 5.5
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description

Following error message is presented when I run PHPUnit in plugin development with latest trunk of WordPress:

root@bdc07b68467e:/plugin# phpunit
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

..
Fatal error: Uncaught Exception: Serialization of 'Closure' is not allowed in /root/.composer/vendor/phpunit/phpunit/src/Util/GlobalState.php:187
Stack trace:
#0 /root/.composer/vendor/phpunit/phpunit/src/Util/GlobalState.php(187): serialize(Array)
#1 /root/.composer/vendor/phpunit/phpunit/src/Util/GlobalState.php(156): PHPUnit_Util_GlobalState::exportVariable(Array)
#2 /root/.composer/vendor/phpunit/phpunit/src/Framework/TestCase.php(783): PHPUnit_Util_GlobalState::getGlobalsAsString()
#3 /root/.composer/vendor/phpunit/phpunit/src/Framework/TestSuite.php(733): PHPUnit_Framework_TestCase->run(Object(PHPUnit_Framework_TestResult))
#4 /root/.composer/vendor/phpunit/phpunit/src/Framework/TestSuite.php(733): PHPUnit_Framework_TestSuite->run(Object(PHPUnit_Framework_TestResult))
#5 /root/.composer/vendor/phpunit/phpunit/src/TextUI/TestRunner.php(517): PHPUnit_Framework_TestSuite->run(Object(PHPUnit_Framework_TestResult))
#6 /root/.composer/vendor/phpunit/phpunit/src/TextUI/Command.php(186): PHPUnit_TextUI_TestRunner->doRun(Obj in /root/.composer/vendor/phpunit/phpunit/src/Util/GlobalState.php on line 187

Last succeed in CI
2020-06-17 17:17
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/builds/699221099

First failed in CI
2020-06-26 22:01
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/builds/702342495

I'm using @runInSeparateProcess annotation to test function including die().
When I comment out all tests using @runInSeparateProcess,
PHPUnit worked well.

I'm perfectly following standard steps for PHPUnit:
https://make.wordpress.org/cli/handbook/misc/plugin-unit-tests/

Travis CI is standard environment to run PHPUnit for WordPress plugin.

If you want to build environment to reproduce,
This Docker Image may be your help:

https://github.com/yukihiko-shinoda/dockerfile-phpunit-wordpress-plugin

docker build --build-arg PHP_VERSION=7.3 --build-arg WORDPRESS_VERSION=trunk .

VSCode integration also available:

https://github.com/yukihiko-shinoda/docker-compose-phpunit-wordpress-plugin

Attachments (1)

50482.diff (1.1 KB) - added by tharsheblows 3 years ago.
move closure to its own function

Download all attachments as: .zip

Change History (25)

#1 @swissspidy
4 years ago

  • Component changed from General to Build/Test Tools
  • Keywords close added

In your tests that use @runInSeparateProcess, are you using anonymous functions (closures) anywhere?

Because PHP cannot serialize anonmyous functions. The @runInSeparateProcess feature requires serialization, for instance of the data from a data provider.

So this error message is expected behavior, and is not an issue with the WordPress test suite, but just how PHPUnit works.

See https://github.com/sebastianbergmann/phpunit/issues/2739

#2 @SergeyBiryukov
4 years ago

  • Keywords close removed
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Hi there welcome to WordPress Trac! Thanks for the report.

Closing per comment:1, as this appears to be expected PHPUnit behavior, not specific to WordPress.

#3 @yshinoda
4 years ago

Hi,
I had not understood mechanism why error message is presented,
Thanks to you for explaining so I got understand it.

After remove anonymous function from test code, PHPUnit worked fine.

By the way, I report here again since I'm curious that only test with trunk of WordPress failed:
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/builds/702342495

#4 @yshinoda
4 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

I mistaked how to check PHPUnit result in last post,
I got same error even though I removed all of anonymous function that was included in my code.

This reproduces only when test with trunk version of WordPress.
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/builds/702971274

Using dependencies is perfectly same.
Fail:
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/jobs/702971277
Success:
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/jobs/702971276

#5 @SergeyBiryukov
4 years ago

  • Milestone set to Awaiting Review

#6 follow-up: @yshinoda
4 years ago

  • Severity changed from normal to critical

3 days ago, the almost test excluding with trunk was green:
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/builds/702971274

Today, finally the almost test excluding legacy version have been red:
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/builds/718609866

How should I guarantee safety to users?

@runInSeparateProcess is an important function.

Would you at least raise the supported version of PHPUnit from 5.* ?
https://make.wordpress.org/cli/handbook/misc/plugin-unit-tests/#running-tests-locally

#7 follow-up: @thomasplevy
4 years ago

@yshinoda

I've run into a similar problems in some of my WP plugin test cases (built off the wp develop unit testing lib) when running tests with @runInSeparateProcess

This error is only presenting itself for me when I run tests against WP 5.5. On 5.4 or lower it doesn't have any issues. I haven yet to determine what's changed to cause this but for my issues I've been able to resolve this by adding an additional annotation to my @runInSeparateProcess tests which disables the global state preservation which happens when running tests in a separate process:

@preserveGlobalState disabled

I came to this "solution" via the following sources:

https://stackoverflow.com/a/14514753/400568

And instead of overwriting the run() method I decided to add an annotation to the specific test cases: https://phpunit.readthedocs.io/en/9.2/annotations.html?highlight=preserve%20global%20state#preserveglobalstate

I can't quite figure out what's causing this from 5.5, as I said, but wanted to share what's helped me out in the meantime.

#8 in reply to: ↑ 6 @SergeyBiryukov
4 years ago

Replying to yshinoda:

Would you at least raise the supported version of PHPUnit from 5.* ?
https://make.wordpress.org/cli/handbook/misc/plugin-unit-tests/#running-tests-locally

As far as I'm aware, PHPUnit 7.x is the latest supported version, see [44701] / #43218.

PHPUnit 8.x support is currently blocked by moving to PHP 7.1 as the required version, see #46149 and #51043.

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

#9 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.5.1

Last succeed in CI
2020-06-17 17:17
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/builds/699221099

First failed in CI
2020-06-26 22:01
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/builds/702342495

Looking at this again, apparently the issue was introduced somewhere between those dates:
https://core.trac.wordpress.org/log/trunk?rev=48186&stop_rev=48067

In that timeframe, there were several commits introducing a closure: [47943], [48069], [48171].

Additionally, one more closure was added later in [48645].

So one of these commits could be a potential culprit here.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


4 years ago

#11 in reply to: ↑ 7 @yshinoda
4 years ago

Replying to thomasplevy:

Thank you very much, I could suppress this error by following your advice👍🎉🚀
https://travis-ci.org/github/yukihiko-shinoda/staticpress2019/builds/720426136

#12 @SergeyBiryukov
4 years ago

  • Milestone changed from 5.5.1 to 5.5.2

Moving to milestone 5.5.2 as we are close to 5.5.1 RC and this ticket still needs a patch.

#13 @auth0josh
3 years ago

I'm having this problem as well. Only a few of our tests annotated with @runInSeparateProcess and adding @preserveGlobalState as an annotating to those or globally with the phpunit configuration file did not make a difference. Running tests against 5.4.2 passes but 5.5 or latest errors for all tests:

EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE  63 / 416 ( 15%)
EEEEEEEEEEE
Fatal error: Uncaught Exception: Serialization of 'Closure' is not allowed in /Users/josh-cunningham/Sites/wp-auth0/wp-content/plugins/auth0/vendor/phpunit/phpunit/src/Util/GlobalState.php on line 175

Exception: Serialization of 'Closure' is not allowed in /Users/josh-cunningham/Sites/wp-auth0/wp-content/plugins/auth0/vendor/phpunit/phpunit/src/Util/GlobalState.php on line 175

We're going to release an update marked as tested up to 5.4.2 and adjust our CI for now but it would be good to know if there's something we can do differently or if this needs to be changed in core.

Plugin repo is below, happy to help test!

https://github.com/auth0/wp-auth0

#14 @whyisjake
3 years ago

  • Milestone changed from 5.5.2 to 5.5.3

Moving to 5.5.3, still looking for a patch.

#15 @JeffPaul
3 years ago

  • Milestone changed from 5.5.3 to 5.6

We're working on a short cycle 5.5.3 release in the very near future and have no further minor releases planned so I'm punting this to 5.6 in hopes it'll have a patch ready for that major release.

This ticket was mentioned in Slack in #core by helen. View the logs.


3 years ago

#17 follow-up: @TimothyBlynJacobs
3 years ago

IMO we should close this as wontfix. I don't think Core should be forced to forego any use of closures that may wind up in global state somewhere.

For the use case outlined in the original ticket description, I'd recommend using wp_die instead of die() in your code. wp_die supports multiple handlers and during test suite execution throws WPDieException instead of killing the request.

#18 @helen
3 years ago

  • Milestone changed from 5.6 to Awaiting Review

Moving out from 5.6 and back into Awaiting Review because it's not clear to me we even have a full diagnosis here.

#19 in reply to: ↑ 17 @SergeyBiryukov
3 years ago

Replying to TimothyBlynJacobs:

IMO we should close this as wontfix. I don't think Core should be forced to forego any use of closures that may wind up in global state somewhere.

It's not quite clear why this used to work in 5.4.2 but not in 5.5 though, would be great to figure out what exactly is causing the fatal error and maybe display a more appropriate message if possible.

comment:9 aims to track down the relevant changes, needs more investigation though.

#20 @TimothyBlynJacobs
3 years ago

I think it'd most likely be the theme registration for post-formats.

#21 @SergeyBiryukov
3 years ago

Thanks, that would be [48171] then.

#22 @tharsheblows
3 years ago

@TimothyBlynJacobs @SergeyBiryukov Updating the theme registration for post-formats works for me -- attached is a diff that takes out the closure from the post formats theme registration and puts it in a function. I'm not sure of the naming or things like that but this is fixes it for me on the Auth0 plugin @auth0josh mentioned at least.

@tharsheblows
3 years ago

move closure to its own function

#23 @jrf
3 years ago

  • Keywords close added

In my experience, these type of errors are typical on older PHPUnit versions being run on incompatible, more recent PHP versions, when the @runInSeparateProcess annotation is used and there is an error somewhere in the code being run via the test.

In this case, using PHPUnit 5.x on PHP 7.3 is the first thing to fix. Use PHPUnit 7.x instead when running tests on PHP 7.3.

I concur with @thomasplevy that using @preserveGlobalState disabled can sometimes help.
@TimothyBlynJacobs suggestion about using wp_die() instead of die() is also very valid.

Additionally, declaring that particular test class in a separate testsuite in the PHPUnit configuration can also often be used as a work-around.

As a debugging method, I'd generally recommend isolating the problem test to their own file, removing the annotation and then running just and only that test, for instance by using --filter TestClass::testName on the command line.

That should then show the underlying error and once that it solved, the test can be brought back to its normal state.

Also now the patches for #46149 have been committed, more recent versions of PHPUnit can now be used with the Core tests and in my experience, those should no longer display this particular error, but should display an error for the real, underlying issue.

So if this is a Core test, please use PHPUnit 9.x when testing on PHP 7.3 as of now. You should get this automatically when running composer update on PHP 7.3.

If the issue is in a (integration) test suite for a plugin/theme, a Make post will be published over the next couple of weeks with information on how to update your tests to also make them cross-version compatible with a wider range of PHPUnit versions.

As this is not a problem with Core directly, but rather with an incompatible test setup, possibly combined with a bug in the code under test in a plugin/theme, I'd like to suggest closing this issue.

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

#24 @desrosj
18 months ago

  • Keywords close removed
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from reopened to closed

It's been 14 months without any further activity. I'm going to close this one out again.

If anyone feels this is incorrect, please provide some more details. Discussion can always continue on closed tickets.

Note: See TracTickets for help on using tickets.