Problem/Motivation
In PHPUnit 12, we have to replace test annotations with attributes. As noticed in #3417066-120: Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency,
we can't mix PHPUnit attributes and annotations [in a single file], we will have to convert them all at once unfortunately
with thousands of test classes in Drupal, we should try to automate the conversion, in a way that a script can be executed on a file and convert it end-to-end.
Rector can help, but we need an overall Rector script that will do all the changes necessary in a go - we should convert by files or group of files, and not by rule/annotation.
Listing some findings for the Rector rule, will document here along
@group legacyis a symfony/phpunit-bridge concept. It must be converted to the#[IgnoreDeprecations]attribute, however:it is still undocumented: https://github.com/sebastianbergmann/phpunit-documentation-english/issue...PHPStan does not identify this attribute as a deprecated scope https://github.com/phpstan/phpstan-deprecation-rules/issues/109@coversDefaultClassand@coversneeds complete rethinking as there is NO one-to-one conversion:- https://github.com/sebastianbergmann/phpunit/issues/4502
- https://github.com/rectorphp/rector-phpunit/issues/149
- https://github.com/sebastianbergmann/phpunit/issues/5175
- Rector rule
CoversAnnotationWithValueToAttributeRectormay not be fit for purpose because (a) does not manage short class names + use import and (b) it does not take into account the existence of [#CoversMethod] that was only (re)introduced in PHPUnit 11.1 - The #[CoversMethod] attribute only targets test classes and not individual test methods, this is being disputed upstream: https://github.com/sebastianbergmann/phpunit/issues/5837
- --> Since we will have to live with tests in shape for execution in PHPUnit 10 for a while, and only get later to a supported coverage of methods, the suggestion is to change
@coversinstances to@legacy-coversin the mean time, then decide what to do once fully on PHPUnit 11. @mediumcan no longer be associated to a method --> add#[Medium]to the class instead@usescan no longer be associated to a method --> add#[UsesClass()]to the class instead- more to be defined
Proposed resolution
The MR here contains the rector.php file with a custome rule to do the conversion, and a .gitlab-ci.yml CI configuration file with a script that executes the rector rule, then runs PBPCBF to cleanup the changed files, then produces a git diff patch file and finally stores the patch as an artifact.
The diff patch file can then be used in child issues to convert specific parts of the test codebase.
The MR itself is not meant to be committed.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Issue fork drupal-3446380
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
Comment #2
mondrakeComment #3
mondrakeComment #4
mondrakeCurrently developing a Rector script for this in https://github.com/mondrake/d8-unit/blob/test-rector/rector.php
Comment #5
mondrakeComment #6
mondrakeComment #7
mondrakeComment #8
mstrelan commentedUpdated IS to clarify this is required in PHPUnit 12, not 11.
Comment #9
mondrakeComment #10
mondrakeComment #12
mondrakeComment #13
mondrakeComment #14
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 #15
mondrakeComment #16
mondrake'Active' is more appropriate as a status, maybe.
Comment #17
nicxvan commentedThis could go into drupal rector too.
Comment #18
mondrakeComment #19
mondrakeThis issue has completed its purpose as all bulk tests conversions were done.
I'd close this 'Closed (works as designed)' but I thought to set to RTBC first to see if any comments.
Comment #20
nicxvan commentedCan we open an issue in drupal rector with your rector script?
Comment #21
alexpottAs this has been marked RTBC for over a month going to mark as 'Closed (works as designed)'.
I opened #3552124: Leverage #3446380's rector rule to convert PHPUnit annotations to attributes