DependencySerializationTrait depends on removed __PHPUNIT_BOOTSTRAP global

Maybe we should try removing this hack entirely on PHPUnit 9 and seeing how badly things break?

Needs rebase and understanding why the tests failed

Tests were failing due to missing @group annotation on the newly introduced test.

Wonder if IS could be updated to included proposed solution. Not sure if this counts as API change?

Small comments on MR. Probably good to self RTBC after.

done

Believe this is is good now

Difficult to decide whether to backport this one or whether we need a change record. This seems somewhat unlikely to affect contrib or custom tests unless they are specifically checking dependency serialization so decided to backport to 10.3.x to keep things in sync, but not 10.2.x, and that we don't need a change record as it is an edge case for tests only; testing serialization of dependencies should have been done in a kernel test anyway.

Committed and pushed 24f8f3c961 to 11.x and dfa7222ae5 to 10.3.x. Thanks!

Reverted, after commit this broke DrupalDateTimeTest::testSleep.

$ ddev test core/tests/Drupal/Tests/Core/Datetime/DrupalDateTimeTest.php 
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\Core\Datetime\DrupalDateTimeTest
................E                                                 17 / 17 (100%)

Time: 00:00.116, Memory: 6.00 MB

There was 1 error:

1) Drupal\Tests\Core\Datetime\DrupalDateTimeTest::testSleep
Drupal\Core\DependencyInjection\ContainerNotInitializedException: \Drupal::$container is not initialized yet. \Drupal::setContainer() must be called with a real container.

/var/www/html/drupal/core/lib/Drupal.php:169
/var/www/html/drupal/core/lib/Drupal/Core/DependencyInjection/DependencySerializationTrait.php:75
/var/www/html/drupal/core/tests/Drupal/Tests/Core/Datetime/DrupalDateTimeTest.php:289
/var/www/html/drupal/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
/var/www/html/drupal/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/var/www/html/drupal/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
/var/www/html/drupal/vendor/phpunit/phpunit/src/TextUI/Command.php:144
/var/www/html/drupal/vendor/phpunit/phpunit/src/TextUI/Command.php:97

This was added recently in #3187004: DrupalDateTime serialization issue

Moved DateTimeTest::testSleep from UnitTest to KernelTest.

Tests appear green again so remarking.

Given that I broke HEAD once here, changed my mind on #16. Let's add a short change record that explains that if you were testing dependency serialization in a unit test you will need to switch to a kernel test.

For review: https://www.drupal.org/node/3425462

CR reads fine. So this change is for 11.x only correct?

Technically, yes, but we can also make it in 10.3.x if we want, with the minor risk of breaking some contrib or custom code tests - but as there are only two in core that are affected the chances of any others feels quite slim.

Gotcha, thanks for that.

If I have to work this out when a contrib or custom test suddenly fails on me I'm going to be a little bit frustrated. This change will break someone's contrib or custom tests somewhere. In fact I think it's more like than core as some contrib and custom much prefer to wirte unit tests over other types of tests because they are lighter and easier to manage.

An alternate fix would be use the constant PHPUNIT_COMPOSER_INSTALL - we ensure it is set it in core/tests/bootstrap.php and it is still used in PHPUnit 11 - see https://github.com/sebastianbergmann/phpunit/blob/2a6d51d23035bfcd0fe6c4...

So if do the one line change

    // Tests in isolation potentially unserialize in the parent process.
    $phpunit_bootstrap = defined('PHPUNIT_COMPOSER_INSTALL');

Then everything continues to work and you can unit test serialization of objects that use the DependencySerializationTrait

The problem is that we are calling \Drupal::getContainer() when there is no container. But also in the unit test case there are no service IDs to restore in wakeup either, so we can just skip calling getContainer() in that case. MR!6914 does this and passes locally without changing any tests.

So according to #2553655: Convert ViewKernelTestBase to use KernelTestBaseTNG we added this code largely to prevent deserialization errors in PHPUnit on failure.

I've tested @longwave's new approach with \Drupal\Tests\user\Kernel\Condition\UserRoleConditionTest::testConditions and made it fail. We don't get serialisations issues anymore! Yay! We think this is due to changes in PHPUnit include the golden oldie https://github.com/sebastianbergmann/phpunit/issues/4983

Updated the IS and unpublished the change record as we no longer need it.

Let's try again. Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

hmm but isn't that why it was moved into the loop - so that it only runs if there are actual service IDs to operate on, so it doesn't run when there's no container.

But... we could probably check if _serviceIds has anything in/whether there's a container, before getting the container to achieve the same thing.

> \Drupal::container to Drupal::service() in a loop. Could that potentially add a bunch of extra function calls to a wakeup?

To me this is a micro-optimisation and not worth worrying about; we are adding one extra function call for each service that is being reinitialised on wakeup.

The tests were already testing object serialization without a container, and we aren't changing that here; they aren't testing the trait itself, just other features of their objects.

Looks good to me now!

Committed and pushed 1707169f2b to 11.x and a20d399646 to 10.3.x. Thanks!