Problem/Motivation

PHPUnit 12 was released in Feb 2025.

The most complex prep for Drupal to support it is converting all tests from using annotations to use attributes for the test metadata. [done]

The current effort is in rationalizing usage of stubs vs mocks in the test codebase - lately deprecated in PHPUnit 12.5. See #3561671: [meta] Refactor tests to use stubs instead of mocks where mocks do not configure expectations.

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3527936

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mondrake created an issue. See original summary.

mondrake opened merge request !12659

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
Parent issue: #3497431: Deprecate TestDiscovery test file scanning, use PHPUnit API instead »
Related issues: +#3445240: [meta] Add #[RunTestsInSeparateProcesses] attribute to all Kernel and Functional tests
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: Introduce support for PHPUnit 12 » [D12] Introduce support for PHPUnit 12
Issue tags: +Major version only
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented

Merge MR with MRs from #3530453: Remove deprecated use of Assert::isType() and #3550292: Test cleanup for PHPUnit 12 - round 2

mondrake’s picture

mondrake
Location 🇮🇹
commented

Green! The MR now includes #3530453: Remove deprecated use of Assert::isType(), #3550292: Test cleanup for PHPUnit 12 - round 2.

mstrelan’s picture

mstrelan commented

Regarding #[RunTestsInSeparateProcesses] and requiring it for contrib, I noticed you've used rector for the core conversions, but I think there is a lower barrier to entry for phpcs and phpcbf since it's already installed in drupal/core-dev. I had AI generate a fixable phpcs rule, tested it on a contrib module and it worked. Should we consider opening a coder issue for this?

mondrake’s picture

mondrake
Location 🇮🇹
commented

#8 sure it would make sense. Can PHPCS selectively detect that the test class inherits from one of KernelTestBase or BrowserTestBase? Only those should have the attribute, Unit and Build tests do not need it, at least by default.

mstrelan’s picture

mstrelan commented

Yes the poc I did checked the parent class(es).

mondrake’s picture

mondrake
Location 🇮🇹
commented
Version: 11.x-dev » main
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
Related issues: +#3561671: [meta] Refactor tests to use stubs instead of mocks where mocks do not configure expectations
quietone’s picture

quietone commented
Title: [D12] Introduce support for PHPUnit 12 » Introduce support for PHPUnit 12
Parent issue: » #3449808: [12.x] [meta] Requirements for tagging 12.0.0-beta1

We are trying to use tags to identify version, so removing from title. The 'Major version only' seems sufficient?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

What is this currently postponed on? This is not clear to me. Being a Drupal 12 requirement, would be good to know :)

mondrake’s picture

mondrake
Location 🇮🇹
commented

#14 the main blocker is #3561671: [meta] Refactor tests to use stubs instead of mocks where mocks do not configure expectations at the moment.

catch’s picture

catch
he/him
Primary language English
commented

@gabor #3561671: [meta] Refactor tests to use stubs instead of mocks where mocks do not configure expectations which is linked from the issue summary is the main blocker.

Annotations to attributes is fully done at this point (as far as we know).

Not sure if there are other smaller changes too but probably easier to refresh the MR once the mock/stub issues are in for any stragglers.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes

Thanks, adding postponed-on issue to the top of the issue summary to make this clear :)

quietone’s picture

quietone commented
Issue tags: -Major version only +12.0.0 beta blocker
dcam’s picture

dcam commented

I've been working on fixing the mock object notices in the core library tests. I think there were over 1500 notices when I started. I'm down to under 300 at the moment. My plan is to start opening more issues and committing some of the work in the next day or two.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Getting close.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Postponed » Needs review

There's only one failing test once #3581058: Convert expectation-less test mocks to stubs - cleanup is included here. I think this MR is reviewable now.

mondrake’s picture

mondrake
Location 🇮🇹
commented

All tests green on all db platforms.

dcam’s picture

dcam commented

The last mock object fix issue has been committed.

dcam’s picture

dcam commented

There's a to-do in phpstan.neon.dist to remove the disallowed method call for any() since it will be detected as deprecated by normal PHPStan checks. I can remove it, if you'd prefer for me to do it. But I didn't want to jump in and make changes while you're actively pushing commits, @mondrake.

mondrake’s picture

mondrake
Location 🇮🇹
commented

@dcam I will be afk for next 3 days so if you want to move this forward, feel free! Thanks for checking.

smustgrave’s picture

smustgrave commented

Left a few comments/questions on the MR.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

All feedback appears to be addressed! I resolved the threads I opened and I believe this one is good to go!!!

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Reviewed & tested by the community » Needs work

IMHO we should get ahead and just remove support for PHPUnit 11 here. It’s been painful to keep concurrent 10/11 support within D11 lifecycle, can’t see a reason for such a pain when going into a new Drupal major. Especially if we target even another bump to PHPUnit 13.

dcam’s picture

dcam commented
Issue tags: +Needs change record

Makes sense to me. I can start the work on it, though I don't know off-hand where all the BC layers are at. I'll do my best.

It will need a change record. Tagging.

mondrake’s picture

mondrake
Location 🇮🇹
commented

I think we also need a CR, tagging

mondrake’s picture

mondrake
Location 🇮🇹
commented

#29 #30 small (or was great? 😀) minds think alike - xpost

smustgrave’s picture

smustgrave commented

Bummer thought we were close but makes sense to drop 11.

Ping me if you want a quick review to help keep moving it

mondrake’s picture

mondrake
Location 🇮🇹
commented

#29 OTOMH it should be sufficient to remove the version dependent compatibility trait, and check if there are occurrences of runnerversion::getMajor() that target 11 or below.

Then update the composer.json constraints and run composer update —lock to get a freah composer.lock

dcam’s picture

dcam commented

Ok. I will do that.

I came back to ask if we want to remove the custom PHPStan rules forbidding PHPUnit annotations. I didn't know if the preference would be to purge everything or to leave those rules so any code in violation gets caught early by automated tests. I'd lean toward keeping them, at least until Drupal 13.

smustgrave’s picture

smustgrave commented

Could it be a follow up? Guess it depends how much it would blow the MR up.

mondrake’s picture

mondrake
Location 🇮🇹
commented

#34 good point. Annotations in PHPUnit 12 would just be no-ops, or a relic of the past if you wish. But PHPStan would flag that for older MRs in the pipeline, and for contrib if they use the rules in their config, which I agree would be a good thing for a bit longer. I think we should keep them for D12 cycle to smoothen the transition, I feel like contrib will take a while to comply.

dcam’s picture

dcam commented

Yes, there's no reason we couldn't defer removing them.

dcam’s picture

dcam commented

I think we should keep them for D12 cycle to smoothen the transition, I feel like contrib will take a while to comply.

Exactly. If we leave them in, then there's less chance of something going wrong for someone somewhere.

dcam’s picture

dcam commented
Status: Needs work » Needs review
Issue tags: -Needs change record

There's one test failure that I think is random.

I added the change record. I opted to link to the original CR about adding PHPUnit attribute support instead of copying and pasting all that information. That way there's a single source for that information. I also decided to mention that we're leaving the PHPStan rules in place to encourage people to use them for detecting the obsolete annotations.

smustgrave’s picture

smustgrave commented

Started a new pipeline since we can't re-run

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Had to re-run like 3 times but finally all green. Kept just triggering a new random each time.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Thanks. How about a follow up to remove the PHPUnit compatibilty trait? We have been having that completely empty for PHPUnit 11, now 12, and likely in 13 in the future. We may readd it in case in the future is needed, but for now it is just unnecessary maintenance burden.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main, thanks!

A follow-up to remove the compatibility trait sounds good.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !12659

  • catch committed 35fcf3d6 on main 
    task: #3527936 Introduce support for PHPUnit 12

By: mondrake
By:...
smustgrave’s picture

smustgrave commented

I published the CR

mondrake’s picture

mondrake
Location 🇮🇹
commented
Related issues: +#3583606: Fix usage of with*() without expects() in mocks - round 1

Filed #3583606: Fix usage of with*() without expects() in mocks - round 1

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.