Problem/Motivation

#3497431: Deprecate TestDiscovery test file scanning, use PHPUnit API instead added some new deprecations to the baseline. These were deprecated in https://github.com/sebastianbergmann/phpunit/issues/6052.

Proposed resolution

Replace Assert::isType calls with PHPUnit 11+ specific type assertions.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3530453

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:

Comments

mstrelan created an issue. See original summary.

mstrelan’s picture

Status: Active » Needs review
mondrake’s picture

I’ve closed #3497080: Assert::isType() is deprecated that is now a duplicate.

The only question I have here is whether we should wait for PHPUnit 10 support to be removed, or add PHPUnit 10 polyfills for the new methods. PHPUnit 10 lacks them and we might still have someone testing locally core with it ATM.

mstrelan’s picture

Oh I missed that one. A polyfill makes sense, but I wonder if it's worth it for 8 tests. Do we do still do test runs of 11.x with PHPUnit 10 or is it only 11 now? Do we need to keep BC for people running it in other environments?

mondrake’s picture

Do we do still do test runs of 11.x with PHPUnit 10 or is it only 11 now?

Core in GitLabCI is now consistently on PHPUnit 11 (has been so for just a couple of days though, very fresh), but we can’t say for contrib or people running core tests locally or via other CI tools.

mondrake’s picture

smustgrave’s picture

Status: Needs review » Needs work

Seems to be just 4 more instances inside ParamConverterManagerTest

mstrelan’s picture

Status: Needs work » Needs review

Updated ParamConverterManagerTest and rebased. Not sure who is best to answer #4.

mondrake’s picture

Status: Needs review » Reviewed & tested by the community

No more isType calls reported in the baseline. RTBC

Let’s see what core committers think of #4.

Now core is set on testing with PHPUnit 11 by default, so not sure it’s worth the effort.

mondrake’s picture

Issue tags: +PHPUnit 11
xjm’s picture

Title: Remove deprecated use of Assert::isType » Remove deprecated use of Assert::isType()

Re: #4: It's been suggested that it was incorrect of us to drop support for PHPUnit 10 in 11.2, and that we should re-broaden core's constraint to be ^10 || ^11 so as to be less disruptive to folks' CI. We also discussed changing one of the daily or weekly core environments back to testing on PHPUnit 10 for that (because if we don't run the test suite on PHPUnit 10 occasionally then the lower minimum will be a lie). That said, even if we do both those things, I'd still lean toward "no polyfill" given the low number of usages.

xjm’s picture

Correcting my comment from #12: The core constraint is still ^10 || ^11; it was dropping support for sebastian/diff 5 that accidentally made PHPUnit 11 the de facto core requirement. But same conclusion. 🙂

xjm’s picture

Technically the PHPUnit 10 support question is a release manager review thing, so getting a second opinion.

xjm’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -Needs release manager review +Needs change record, +Needs release note

Discussed with @catch. We are okay with no polyfill. However, that would mean officially dropping support for running the core test suite locally on PHPUnit 10 (currently impossible anyway due to the sebastian/diff constraint, but ideally that will be fixed before the Aug. patch release window). So, we will need a CR and a release note.

xjm credited catch.

xjm’s picture

mondrake’s picture

Issue summary: View changes
Status: Needs work » Postponed
Parent issue: » #3537104: Remove support for PHPUnit 10
mondrake’s picture

mondrake’s picture

Issue summary: View changes
Status: Postponed » Needs review
Issue tags: -Needs change record

I think the CR of #3537104: Remove support for PHPUnit 10 is enough. Not sure about a release note specifically for this issue, but leaving the tag.

mondrake’s picture

Issue summary: View changes
smustgrave’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Yea not sure either about the release note but does appear the instances have been replaced. Think it would be good to get into 11.3

longwave’s picture

Version: 11.x-dev » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs release note

Yeah we don't need a separate release note for this now we explicitly dropped support for PHPUnit 10. Good to keep chipping away at the baseline though!

Committed and pushed 35283e39ca0 to 11.x and 033a864f1c3 to 11.3.x. Thanks!

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.

  • longwave committed 033a864f on 11.3.x
    task: #3530453 Remove deprecated use of Assert::isType()
    
    By: mstrelan...

  • longwave committed 35283e39 on 11.x
    task: #3530453 Remove deprecated use of Assert::isType()
    
    By: mstrelan...

Status: Fixed » Closed (fixed)

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