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:
In this issue, convert lib Component unit tests, with the exception of:
that is needed to keep Drupal\Tests\Core\Test\TestDiscoveryTest running.
that rector is failing to convert correctly for FQCN.
Fixing those will require dedicated follow up issues.
| Comment | File | Size | Author |
|---|
Show commandsStart 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:
changes, plain diff MR !8033
Comments
Comment #2
mondrakeComment #4
mondrakeThe @medium annotations can no longer be associated to methods, as attributes.
Only to classes.
Comment #5
mondrakeComment #6
mondrakeMR merged with #3446705: TestDiscovery expects @group annotations, fails with #[Group()] attributes to check with TestDiscovery suppoorting #[Group] attributes.
Comment #7
mondrakeComment #8
mondrakeComment #9
mondrakeThis 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.
Comment #10
mondrakeComment #11
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 #12
mondrakeComment #13
mondrakeKeeping the @group annotations in for the moment, tests are passing.
Comment #16
mondrakeComment #17
mondrakeComment #18
mondrakeComment #19
mondrakeComment #20
mondrakeComment #21
mondrakeComment #22
mondrakeIt 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
useimport + attribute short class name.Maybe for now we just skip converting those cases.
Weird that we get this only while moving to attributes.
Comment #23
mondrakeComment #24
mondrakePostpone on #3526499: PhpUnitApiGetTestClassesTest and PhpUnitApiFindAllClassFilesTest need to execute PHPUnit discovery before TestDiscovery
Comment #25
mondrakeComment #26
mondrakeComment #27
mondrakerebased
Comment #28
smustgrave commentedHaving 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.
Comment #29
catchOne question on the MR.
Comment #30
mondrakeReplied inline.
Comment #31
catchSorry 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.
Comment #32
mondrakeComment #33
mondrakeComment #34
mondrakeSkipped 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
Comment #36
catchCommitted/pushed to 11.x, thanks!
Comment #37
mondrake