Support PHPUnit 11 in Drupal 11

PHPUnit 11 dropped support for PHP 8.1, so with PHPUnit 10 already a major challenge for Drupal 10, not sure it makes sense to target also PHPUnit 11 for it.

By the time PHPUnit 10 is out of support, we'll be in a position to stop running tests on PHP 8.1, so theoretically we can update as it's only a dev dependency. Having said that, whether we eventually run Drupal 10 tests against PHPUnit 11 is very secondary to whether we can with Drupal 11, so dealing with the deprecations (after we've dealt with the PHPUnit 10 ones) is the main thing anyway.

Given test results, the only big thing to get us onto PHPUnit 11 is the replacement of test annotations with PHP attributes, that throw thousands of deprecations. Actual errors and failures are just a few (functional ones seem actually error-free)

Hmmm… but how to have a test that uses annotations compatible with D10 (on PHPUnit 9) and D11 (on PHPUnit 11) - just not care about the deprecations? I guess if you are going for D10 and D11 compatibility then you should be ignoring deprecations. So we just need to document that.

In #3445106-7: Replace @dataProvider annotations with #[DataProvider()] attributes @mondrake linked to an example where the file has both annotations (for PHPUnit 9) and attributes (for PHPUnit 10/11) at the same time: https://github.com/FileEye/MimeMap/blob/ba0b04e179976e7d6a487fdb166c5f1f...

Unsure if we want to repeat that pattern across core as it could be quite messy, but it looks like a possible option for contrib that wants to be compatible with both.

AFAICS we cannot ignore deprecations thrown by PHPUnit itself, anymore. PHPUnit is not using trigger_error for deprecations, but it's own event system. But we could apparently have both annotations and attributes and being compatible with PHPUnit all the way from 9 to 11 - see for instance https://github.com/FileEye/MimeMap/blob/master/tests/src/TypeTest.php#L7... in a separate project. AFAICS PHPUnit will throw a deprecation when no attributes exists AND annotation exist; if the test has attributes, PHPUnit 11 will rely on those and not check about existing annotations.

EDIT - xpost with #9

But we could apparently have both annotations and attributes and being compatible with PHPUnit all the way from 9 to 11

The attribute class would be missing in PHPUnit 9 though so that wouldn't work, or we'd have to alias them to some placeholder class.

Note sure about #11. It doesn't seem to be an issue in the project I referenced. Maybe if the class is missing yes, me might have a problem with PHPStan reporting it, but runtime the attribute is just handled like a comment?

Right, I thought the use statement to a non existent class would cause a problem but it seems it doesn't.

I agree with #9 btw, and think in core we should just remove the annotations, and document the possible double standard for contrib.

Using attributes in core but recommending both attributes and annotations for contrib seems good. We would need to work out the mechanics of it, but it would be good to run core tests on phpunit 11 by default for Drupal core (composer update maybe once we're compatible?) but lock to phpunit 10 for contrib (maybe with the option to upgrade too, or something like that). Thanks for doing the investigation here, really good to know it's not going to be as bad this time.

Rebased against 11.x and updated to the latest PHPUnit 11. If we can ignore the deprecations for now we are not too far off landing this.

PHPUnit expects parameter names to match in data providers, this was not strictly the case before and we had two mismatched, fixed here.

The remaining issues are mostly

Failed to open directory: Too many open files

which I think might be caused by triggering way too many deprecations, and a handful of strange mocking errors:

Error: Typed property MockObject_EntityBaseTest_2da9ae70::$__phpunit_state must not be accessed before initialization

Since PHPUnit 11.3.3, PHPUnit's own deprecations are only reported if the --fail-on-phpunit-deprecation toggle is passed in.

See https://github.com/sebastianbergmann/phpunit/blob/11.3.6/ChangeLog-11.3....

PHPUnit deprecations do not use trigger_error for being reported (see #10). So if we move on like this it's OK but we need to be aware and conscious that all our tests will silently execute with deprecated PHPUnit code, and that code will be dropped in PHPUnit 12 that is one month away.

The bigger part of PHPUnit 11 change - moving from using annotations to using attributes - still needs to be started in fact. #3446705: TestDiscovery expects @group annotations, fails with #[Group()] attributes is the first step in that sense, waiting for a review, and #3446380: [no-commit] Define a Rector rule to convert test annotations to attributes + #3446693: Convert test annotations to attributes in Drupal/Test/Component would be the beginning of the following steps.

I have added the --fail-on-phpunit-deprecation toggle to make the MR comparable to earlier results; we can remove that later if we decide to ignore PHPUnit's own deprecations for now. Or, alternatively, find a way to make PHPUnit's own deprecations reporting configurable in our CI.

The IS seems quite off, now.

I think PHPUnit deprecations are OK to deal with in a followup, especially as there is a switch for them now that means we can ignore them for the time being. The sooner that we get onto PHPUnit 11 at all the sooner contrib can start to trying to catch up, and we can figure out the plan for migrating to attributes in the meantime.

Not sure what's up with the "too many open files", debugging it is tricky.

Locally, DerivativeDiscoveryDecoratorTest::testInvalidDerivativeFetcher() does not fail but takes 18 seconds until I comment out the trigger_error in DebugClassLoader::checkClass():

            foreach ($deprecations as $message) {
                @trigger_error($message, \E_USER_DEPRECATED);
            }

After removing this it takes 16 milliseconds - over 1000 times quicker. There is only one message:

The "PHPUnit\Framework\TestCase::__construct()" method is considered final. It may change without further notice as of its next major version. You should not extend it from "Drupal\KernelTests\KernelTestBase".

I guess there is something happening in an error handler.

The issue is in PHPUnit's own error handler. For a given deprecation it tries to figure out whether it was caused in and/or by first party code as per the source map.

            if ($this->sourceFilter->includes($this->source, $trace[0]['file'])) {
                $triggeredInFirstPartyCode = true;
            }

SourceFilter::includes() creates a new SourceMap object which scans the entire filesystem as per the <source> tag in phpunit.xml.dist.

However it looks like this code has largely existed since PHPUnit 10.1 so not sure why it is problematic now.

Seems it's related to this feature in PHPUnit 11.1: https://github.com/sebastianbergmann/phpunit/issues/5689

For some reason it's excluded suffix checking that is the problem, at least for performance. If I remove

      <directory suffix=".api.php">./lib/**</directory>
      <directory suffix=".api.php">./modules/**</directory>
      <directory suffix=".api.php">../modules/**</directory>

from phpunit.xml.dist then DerivativeDiscoveryDecoratorTest::testInvalidDerivativeFetcher() takes 2 seconds instead of 18.

I'm not even sure why we exclude these, they should never be included for the purposes of code coverage anyway. Let's see what happens to the open files errors if we remove this.

How about adding a part to the SYMFONY_DEPRECATIONS_HELPER environment variable to indicate if PHPUnit's own deprecation should be reported or not? That variable name is legacy, but in fact it's only used by our own replacement of the PHPUnit-bridge, so IMHO we can change its syntax at our wish. Say something like

SYMFONY_DEPRECATIONS_HELPER='ignoreFile=.deprecation-ignore.txt&failOnPhpunitDeprecation=false'

with a default fallback value for failOnPhpunitDeprecation=TRUE if not indicated.

Or we could just add a separate environment variable?

Figured out three of the four fails. It feels like KeyValueEntityStorageTest is bending the rules a bit as - from what I can tell - we expect the entity storage to be able to create an instance of a mock class and it will just work, but I am not sure that is expected or should be allowed.

#27

could just add a separate environment variable?

OK, on it - I would like to keep PHPUnit deprecation configuration things centralised in DeprecationHandler::getConfiguration(), though.

Also, can #3476189: [CI] Use sudo -E to execute run-tests.sh be committed? It'd make life easier.

Re the remaining test, I think this is OK, we create a new mock and explicitly force the new flag, instead of trying to reuse the previously-created entity to test the insert hooks.

We have this deprecation error, that we can ignore for now, adding an entry in .deprecation-ignore.txt:

The "PHPUnit\Framework\TestCase::__construct()" method is considered final. It may change without further notice as of its next major version. You should not extend it from "Drupal\Tests\BrowserTestBase".

However this is painful as this means that in the future we will not be able to override the TestCase constructor to force Kernel and Functional* tests to run in isolation even if the #[RunTestsInSeparateProcesses] attribute is not specified.

Tests are green now.

Edit: PHPUnit changelog where TestCase:__construct is pre-declared final https://github.com/sebastianbergmann/phpunit/blob/11.1.3/ChangeLog-11.1....

Filed #3497080: Assert::isType() is deprecated to try and reduce the PHPStan baseline increase.

EDIT: no, PHPUnit 10 does not have the replacement methods so that will have to wait for this.

Re #32 I guess we have a little while to figure out a solution to overriding the constructor. I seem to remember that PHPUnit thought that our solution was okay when we asked for a way to enable isolation in a base class, maybe we have to follow that up again.

#34 unfortunately not. I tried https://github.com/sebastianbergmann/phpunit/pull/5981 but that was rejected and AFAICS there is no guidance in https://github.com/sebastianbergmann/phpunit/issues/5838.

What I was thinking instead is to add a #[RunTestsInSeparateProcesses] attribute to all Kernel and Functional* tests with the rector conversion of the tests, and supplement that wit a PHPStan custom rule that will fail if new tests are added without the attribute.

Filed #3497128: Fix static call to instance methods in PHPUnit as a child issue.

No longer a plan, this has a MR that can be merged.

Updated IS.

The test failure looks legit, and it'd unveil a PHPUnit 11 bug. Let's see if a workaround works, then I will file a bug upstream.

Strange bug, on it working locally.

OK, so it seems we have a type bug upstream when a @group annotation specifies a numeric string (first test failure), plus in PHPUnit 11 The format of the XML document generated using the `--list-tests-xml` CLI option has been changed.

For the bug upstream, filed --list-tests-xml is broken if a numeric @group annotation is specified in PHPUnit's issue queue.

Fixed.

PHPUnit 11.5.3, due shortly, fixes the upstream bug so IMHO it's worth waiting for it.

Merged #3497116: Remove redundant entries in phpunit.xml.dist in here.

https://github.com/sebastianbergmann/phpunit/releases/tag/11.5.3 is out.

Bumped PHPUnit to 11.5.3, reverted change to @group 44 annotation, ready for review again.

Bumped to PHPUnit 11.5.7 and adjusted PHPStan baseline after commit of #3497128: Fix static call to instance methods in PHPUnit

merged with HEAD after commit of #3497327: Extend PhpUnitTestDiscoveryTest to test also PHPUnit API

I read through this issue and reviewed the MR. I pulled down the MR and tested locally and it worked fine. I tried to apply it to a client project but ran in to too many problems with composer dependencies. I ran a few contrib tests locally and they worked fine too.

The issue summary needs an update with a release note and next steps for follow ups. We need a change record too, and possibly instructions for downgrading to PHPUnit 10. I think we still need to figure out #15, what happens in gitlab for contrib? Would it automatically get PHPUnit 11 or would that require a change in gitlab_templates?

Maybe for now it would be better to do the other way round, i.e. keep PHPUnit 10 in the lock file, with relaxed constraints in composer.json, and allow opt-in to PHPUnit 11? So core can bump to PHPUnit 11 in its test pipelines, while we do not force everyone to downgrade from 11 to 10.

#50 sounds like a good next step, can always be more aggressive later.

We already have the composer drupal-phpunit-upgrade command that should still work if the constraints are set correctly back from when we supported both PHPUnit 8 and 9.

Back then we forced PHPUnit 9 on PHP 7.4, perhaps we should do the same thing for PHPUnit 11 and PHP 8.5?

#52 blast from the past :)

I am working on this.

Done #52. Now PHPUnit 11 executes for Unit tests with PHP 8.5. It's a bit too conservative IMHO, but a first step.

Updated IS, added a release note snippet, tests are green. CR still missing but let's agree this is good to go before writing one?

not clear why ordering has changes https://git.drupalcode.org/project/drupal/-/commit/0b61d1dacd6d8026d5eb1...

and todo needs follow-up

#57 @andypost that's because in PHPUnit 10 the arguments coming from the DataProvider are passed to the test method as a list sequence (regardless of the array keys), whereas in PHPUnit 11 the array keys are used to pass each parameter to the corresponding named argument of the test method. So effectively PHPUnit 11 swaps arguments vis a vis PHPUnit 10 for the same test unless in PHPUnit 10 the sequence of passed-in arguments is aligned to the sequence of the arguments in the test method's signature.

See

• https://github.com/sebastianbergmann/phpunit/blob/11.0.0/ChangeLog-11.0....
• https://github.com/sebastianbergmann/phpunit/issues/5150
• https://github.com/sebastianbergmann/phpunit/pull/5225

From #3515706-5: [CI] Switch the default test environment to PHP 8.4 and MySQL 8.4 :

I wonder would it make sense to switch to PHPUnit 11 for 8.4 too? And let 8.3 and below onto PHPUnit 10? https://stitcher.io/blog/php-version-stats-january-2025

@mondrake it looks ready for 11.2 maybe add 11.2.0 release priority label and add change record?

I find it RTBC

@andypost I think that tag would be a call for a release manager, tagging.

Also #59 needs an answer, I will write a CR afterwards.

#59 makes sense to me, @longwave pointed out we did similar for phpunit 8/9. Removing the release manager review tag.

Updated IS

Added CR draft.

There are test failures under PHP 8.4.

I think the build test failures under PHP 8.4 are somehow connected with the composer script that updates PHPUnit to 11, and probably caused by a permission problem.

Anoyone familiar with build tests that can lend a hand?

Fixed jobs - should be green, looks like compsoer is executed from root intentionally to disallow tests to change files...
Maybe it needs follow-up to clean-up the hunk https://git.drupalcode.org/issue/drupal-3418267/-/blob/3418267-meta-supp...

Looks like #3515706: [CI] Switch the default test environment to PHP 8.4 and MySQL 8.4 has broken build tests for this MR.

It seems failures are now due to drupal/core-recommended not allowing to bump a dependency:

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires drupal/core-recommended ^11 -> satisfiable by drupal/core-recommended[11.2].
    - drupal/core-recommended 11.2 requires sebastian/diff ~5.1.1 -> found sebastian/diff[6.0.2] but it does not match the constraint.

yes I am reverting last commit now, but the failure then gets to the PHPStan job

I set path repos "canonical": false, according to docs but it sounds magic that it was working before

Root composer.json requires drupal/core 11.x-dev (exact version match), it is satisfiable by drupal/core[11.x-dev] from composer repo (https://repo.packagist.org) but drupal/core[dev-main] from path repo (core) has higher repository priority.

Still needs to fix

       ├   Problem 1                                                              
       ├     - Root composer.json requires drupal/core-recommended * -> satisfiable by drupal/core-recommended[dev-main].
       ├     - drupal/core-recommended dev-main requires sebastian/diff ~5.1.1 -> found sebastian/diff[6.0.2] but it does not match the constraint.

The revert of #3515706: [CI] Switch the default test environment to PHP 8.4 and MySQL 8.4 also fixed the build test failure... weird.

Looks good to me.. let's try to ship this with 11.2.

NW for the spelling issue here.

+1 for skipping spellcheck on that file as it's machine generated anyway. The issue was resolved, back to RTBC.

Committed to 11.x
Published change record
Added release note tag

PHPUnit 12 is unblocked https://github.com/phpspec/prophecy-phpunit/releases/tag/v2.4.0

Retroactively given credit for helpful discussion and original IS submission that helped get this in.