Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
When we started using PHPUnit 10, we dropped the dependency to symfony/phpunit-bridge, and added our own custom DeprecationHandler to replace it.
Quite a few parts of the Symfony code were included in the replacement, to allow BC, including the expectDeprecation() method to be used in tests to check deprecations messages are actually triggered.
In the meantime, PHPUnit evolved and in version 11.0, the expectUserDeprecationMessage() and expectUserDeprecationMessageMatches() methods were introduced with basically the same purpose.
https://github.com/sebastianbergmann/phpunit/blob/11.0.10/ChangeLog-11.0...
Proposed resolution
- Deprecate
expectDeprecation(pun moderately intended), and replace its usage withexpectUserDeprecationMessage* - Deprecate the entire
ExpectDeprecationTraittrait - Cleanup
DeprecationHandlerfrom no longer necessary code
NOTE: differently from expectDeprecation, there is no check in PHPUnit that expectUserDeprecationMessage* are only called within deprecation tests (those marked @group legacy or #[IgnoreDeprecations]). Need to be careful about this.
Remaining tasks
None
Follow ups
See child issues.
| Comment | File | Size | Author |
|---|
Issue fork drupal-3497124
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:
- 3497124-deprecate-expectdeprecation-use
changes, plain diff MR !10871
Comments
Comment #2
mondrakeComment #4
mondrakeBlocked on upstream expectUserDeprecationMessage*() fails when test is run in separate process
Comment #5
mondrakeComment #6
mondrakeComment #7
mondrakehttps://github.com/sebastianbergmann/phpunit/issues/6102 is fixed upstream, so as soon as a release 11.5.33 hits the road this is actionable again
Comment #8
mondrakeWith PHPUnit 11.5.34, the facade sealed exception no longer shows, but now we need to complete conversion of all tests to use metadata attributes instead of annotations.
Comment #9
mondrakeComment #10
mondrake#3537104: Remove support for PHPUnit 10 must happen before committing this, but the MR can be reviewed now.
Comment #11
mondrakeRebased post commit of #3537104: Remove support for PHPUnit 10
Comment #12
mondrakeComment #13
mondrakeComment #14
longwaveIf PHPUnit doesn't check IgnoreDeprecations and ours does, and that is important to us, I wonder if this is worth keeping just as a wrapper? Would also save the busy work of making all the changes here and in contrib.
Comment #15
mondrakeWe may not deprecate
expectDeprecation(), even if I do not see the benefit of it, longer term. But I think we should definitely remove the collection mechanism at this point, it's a duplicate of PHPUnit's one and already there are are differences between the two, see the draft CR. Developers from outside will get confused if we keep living in our own island.Re. deprecation ignore file, that remains after this. It's operating on an higher level i.e. before triggered errors are even passed to PHPUnit. In #3558105: [experimental] Remove DeprecationHandler I've started exploring removing it entirely, but it would require upstream changes and we would need to build a strong case for it.
Comment #16
mondrakeBTW, 99% of the changes will only be renaming
expectDeprecation(), toexpectUserDeprecationMessage(). That would be low impact if done in a major beta once most deprecated code is removed.Comment #17
mondrakeAlso, here we would finally get rid of stray
#[Group('legacy')]attributes instead of#[IgnoreDeprecations], that are still leaking in. See last commit to MR.Comment #18
mondrakeComment #19
dcam commentedI don't feel like I'm the right person to review this, but I gave it my best shot since it's been lingering for a while.
The only thing I noticed was that the deprecation of the entire
ExpectDeprecationTraitisn't mentioned in the change record. I feel like it isn't enough to only say thatexpectDeprecation()is deprecated, if for no other reason than to ensure it pops up in the CR search if someone searches for the trait name.That's all the feedback I have. Either the MR is pretty solid or I just don't know enough about the test system to judge it.
Comment #20
mondrakeThanks @dcam. I've added your point to the CR and the IS.
Comment #21
dcam commentedYeah, that looks good. Although the trait is already used by the base classes the fact that several Core tests were using it directly made me think that we needed to mention the deprecation.
Let's see what the committers think.
Comment #22
mondrake#21 only the component tests were importing the trait explicitly, which is fine as components should not rely on core code. In the MR I removed all such cases to use directly the now-native methods. #3552827: Replace all expectDeprecation() calls with expectUserDeprecationMessage() will take care of adjusting core tests.
Comment #23
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #24
mondrakerebased
Comment #25
catchThis looks like good clean-up, but there's a typo in the JavaScript deprecation handling and surprised that's not causing test failures here.
Comment #26
mondrakeComment #27
mondrakeComment #28
catchSorry no idea what happened there maybe my monitor had a speck on it or something.
Comment #29
mondrake:) I know what you mean, I see commas everywhere
Comment #31
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #32
mondrakebot fluke
Comment #33
longwaveAdded a question, if there is no overhead (or reasonable workaround) then set this back to RTBC.
Comment #34
mondrakeHad to think to remember why this.... deprecations triggered during tearDown are no longer captured, so I did not find a better way to check this.
The CR reflects this point:
Happy to hear about alternatives.
Comment #35
longwaveDoes a #[PostCondition] method work?
https://docs.phpunit.de/en/12.5/attributes.html#postcondition
Comment #36
mondrakeLet's try, thanks
Comment #37
mondrake#35 unfortunately not (would have been a great solution), apparently the deprecation capture has already stopped at that point.
Reverting the last two commits.
Comment #38
mondrakeBack to RTBC
Comment #39
mondrakeComment #40
mondrakeThis latest fix requires setting back to NR. It's becoming more difficult to fix, as deprecation handling under native PHPUnit is diverging from our implementation... IMHO a sign that it's time to get this in and we adopt standard.
Comment #41
smustgrave commentedAre all instances of expectDeprecation() suppose to be replaced?
Comment #42
mondrakeYes, in a follow up. See #22.
Comment #43
mondrakeThe fix that was necessary for WebDriverTestBaseTest is no longer needed as that test was removed by #3572325: Remove non-W3C compliant browser tests.
This sets back the MR to a stage when it was RTBC, so I'll self-RTBC this.
Comment #44
mondrakeNeeded another reroll for misusage of #[Group('legacy')], in DefaultHtmlRouteProviderTest this time.
Comment #47
catchCommitted/pushed to main and 11.x, thanks! Made some very minor edits to the CR which I've also published.