Problem/Motivation

Replace annotation-based test metadata with PHP attributes.

Please DO NOT change the MR here manually. The patch is automatically generated by the MR at #3446380: [no-commit] Define a Rector rule to convert test annotations to attributes.

Done:

Proposed resolution

In this issue, convert lib Component unit tests, with the exception of:

  • core/tests/Drupal/Tests/Component/Plugin/PluginBaseTest.php

that is needed to keep Drupal\Tests\Core\Test\TestDiscoveryTest running.

  • core/tests/Drupal/Tests/Component/Annotation/Doctrine/Ticket/DCOM58Test.php
  • core/tests/Drupal/Tests/Component/Annotation/Doctrine/DocParserTest.php
  • core/tests/Drupal/Tests/Component/Datetime/TimeTest.php
  • core/tests/Drupal/Tests/Component/DependencyInjection/Dumper/OptimizedPhpArrayDumperTest.php
  • core/tests/Drupal/Tests/Component/DependencyInjection/Dumper/PhpArrayDumperTest.php

that rector is failing to convert correctly for FQCN.

Fixing those will require dedicated follow up issues.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 3446693-nr-bot.txt90 bytesneeds-review-queue-bot

Issue fork drupal-3446693

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

mondrake created an issue. See original summary.

mondrake’s picture

Issue summary: View changes

mondrake’s picture

Status: Active » Needs work

The @medium annotations can no longer be associated to methods, as attributes.
Only to classes.

mondrake’s picture

Issue summary: View changes
mondrake’s picture

mondrake’s picture

Issue summary: View changes
mondrake’s picture

Issue summary: View changes
mondrake’s picture

Title: Convert test annotation to attributes in Drupal/Test/Component » [PP-1] Convert test annotation to attributes in Drupal/Test/Component
Status: Needs work » Needs review

This is blocked by #3446705: TestDiscovery expects @group annotations, fails with #[Group()] attributes, tests with only attributes break the test discovery.

However, the MR is the pure output of the rector script indicated in the IS, and passes PHPCS and PHPStan checks. So a review would be welcome - to incorporate feedback to the script.

mondrake’s picture

Issue summary: View changes
needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new90 bytes

The 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.

mondrake’s picture

Status: Needs work » Postponed
mondrake’s picture

Keeping the @group annotations in for the moment, tests are passing.

mondrake changed the visibility of the branch 3446693-convert-test-annotation to hidden.

mondrake changed the visibility of the branch 3446693-convert-test-annotation to active.

mondrake’s picture

Issue summary: View changes
mondrake’s picture

Title: [PP-1] Convert test annotation to attributes in Drupal/Test/Component » Convert test annotation to attributes in Drupal/Test/Component
Issue summary: View changes
Status: Postponed » Needs review
mondrake’s picture

Title: Convert test annotation to attributes in Drupal/Test/Component » Convert test annotations to attributes in Drupal/Test/Component
mondrake’s picture

Issue summary: View changes
mondrake’s picture

Status: Needs review » Postponed
mondrake’s picture

Status: Postponed » Needs work

It seems PHPUnit does not like files where multiple namespaces are defined (e.g. the test namespace + test class + a fixture namespace to work around SUT).

BTW, neither Rector does: it fails to determine the ‘main’ namespace and therefore forces the use of FQCN while adding attributes, instead of adding an use import + attribute short class name.

Maybe for now we just skip converting those cases.

Weird that we get this only while moving to attributes.

mondrake’s picture

Issue summary: View changes
Status: Needs work » Needs review
mondrake’s picture

mondrake’s picture

Issue summary: View changes
Status: Postponed » Needs review
mondrake’s picture

rebased

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Having reviewed #3526490: Convert test annotations to attributes in Drupal/BuildTests that was already committed this is inline with those changes too, so therefore believe these are good to go.

catch’s picture

Status: Reviewed & tested by the community » Needs work

One question on the MR.

mondrake’s picture

Status: Needs work » Needs review

Replied inline.

catch’s picture

Status: Needs review » Needs work

Sorry but I'd go for #2 here - the forked Doctrine stuff is already weird, and we can't get rid of it until Drupal 13. Doesn't seem any more work to fix annotation -> attribute manually than to fix fqcn to use statements manually.

mondrake’s picture

Issue summary: View changes
mondrake’s picture

Issue summary: View changes
mondrake’s picture

Status: Needs work » Reviewed & tested by the community

Skipped in the patch generation script the files that lead to erroneous conversions, and applied the latest patch.

Self-RTBC as the patch was RTBC before and we just removed some hunks that will have to be converted manually.

Started a thread in Slack about how to scope further conversion issues https://drupal.slack.com/archives/C079NQPQUEN/p1751533999848609

  • catch committed 6221eb3a on 11.x
    Issue #3446693 by mondrake: Convert test annotations to attributes in...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

Status: Fixed » Closed (fixed)

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