Problem/Motivation

Test::parseTestMethodAnnotations is removed from PHPUnit 10. Needs finding an alternative for

Drupal\Tests\migrate\Kernel\MigrateSourceTestBase
Drupal\BuildTests\Framework\ExternalCommandRequirementsTrait
Drupal\Tests\TestRequirementsTrait

that rely on it.

Done

Proposed resolution

Introduce a light ::getTestClassCovers() shim in the PhpUnit TestCompatibilityTrait that abstracts fetching the @cover annotations via Test::parseTestMethodAnnotations() in PHPUnit 9; when shifting to PHPUnit 10 change the implementation to the new $this->valueObjectForEvents()->metadata()->isCovers() methods (see prototype in https://git.drupalcode.org/project/drupal/-/blame/904e0e97f562e88b06dcf0...)

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3355795

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’s picture

mondrake
Location 🇮🇹
commented
Parent issue: » #3217904: [meta] Support PHPUnit 10 in Drupal 11

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mondrake’s picture

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

mondrake
Location 🇮🇹
commented
Issue summary: View changes
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: -PHPUnit 11
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [PHPUnit 10] Test::parseTestMethodAnnotations is removed in PHPUnit 10 » [PP-1] Test::parseTestMethodAnnotations is removed in PHPUnit 10
Status: Active » Postponed

Better do this once #3405353: Convert use of TestCase::getName() to forward compatible TestCase::name() is in, that revives the PhpUnitCompatibilityTrait.

mondrake opened merge request !6409

mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [PP-1] Test::parseTestMethodAnnotations is removed in PHPUnit 10 » [PP-2] Test::parseTestMethodAnnotations is removed in PHPUnit 10
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [PP-2] Test::parseTestMethodAnnotations is removed in PHPUnit 10 » [PP-1] Test::parseTestMethodAnnotations is removed in PHPUnit 10
Issue summary: View changes
mondrake’s picture

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

mondrake
Location 🇮🇹
commented
Title: [PP-1] Test::parseTestMethodAnnotations is removed in PHPUnit 10 » Test::parseTestMethodAnnotations is removed in PHPUnit 10
Status: Postponed » Active
mondrake’s picture

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

mondrake
Location 🇮🇹
commented
Issue summary: View changes
Status: Active » Needs review
smustgrave’s picture

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

Searched for parseTestMethodAnnotations and found TestRequirementsTrait but that appears to be testing the deprecation of parseTestMethodAnnotations.

Other instance is replaced here.

LGTM

spokje’s picture

spokje
Primary language Dutch
commented

Pondered about this being nowadays a "misuse" of the @covers-tag (although currently nifty and when implemented probably The-Only-Way) and if this should have a follow-up to make this into a whole new annotation.

I'm unsure, maybe core committers have an idea about this?

No matter what, this has to get in asap to get use closer to the PHPUnit 10 hurdle.

longwave’s picture

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

I think we can defer that decision until the PHPUnit 10 upgrade. It seems a relatively neat way of doing it, we need metadata about which class we are testing and conveniently we can borrow that from PHPUnit already. As long as the PHPUnit 10 metadata method handles both attributes and annotations then I don't see an issue, and we should be able to upgrade to PHPUnit 10 first and then migrate test annotations to attributes separately if we need to.

Committed 0e38c5f and pushed to 11.x. Thanks!

  • longwave committed 0e38c5f4 on 11.x 
    Issue #3355795 by mondrake, smustgrave, Spokje: Test::...

longwave closed merge request !6409

smustgrave’s picture

smustgrave commented

Updating attribution

mondrake’s picture

mondrake
Location 🇮🇹
commented

This https://git.drupalcode.org/project/drupal/-/blob/6ba264f34cc45511333ef47... is how the trait will change to support PHPUnit 10.

As long as the PHPUnit 10 metadata method handles both attributes and annotations then I don't see an issue, and we should be able to upgrade to PHPUnit 10 first and then migrate test annotations to attributes separately if we need to.

PHPUnit 10 will support both. PHPUnit 11 will deprecate annotations. PHPUnit 12 will remove them. We potentially need to convert annotations to attributes before Drupal 11 if we want to target PHPUnit 11 for it AND we do not want deprecation notices.

Status: Fixed » Closed (fixed)

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