Ensure base test classes don't have phpunit annotations

Created on 30 June 2025, 3 months ago

Problem/Motivation

Follow-up from 🐛 Fix test classes with no *Test suffix Active - we found various base classes in core that had phpunit annotations that won't do anything, we should check if there are more.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component

phpunit

Created by

🇬🇧United Kingdom catch

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @catch
  • 🇮🇹Italy mondrake 🇮🇹

    I suggest to wait for conversions of annotations to attributes to be completed, then check on attributes instead. Probably a PHPStan rule should be written here.

  • Merge request !12705Closes #3533282 → (Closed) created by mondrake
  • Pipeline finished with Failed
    2 months ago
    Total: 153s
    #546012
  • 🇮🇹Italy mondrake 🇮🇹

    Changed mind and added a PHPStan rule checking for either annotations or attributes on abstract test base classes. We have 17. errors to be solved.

  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Failed
    2 months ago
    Total: 245s
    #546195
  • Pipeline finished with Failed
    2 months ago
    Total: 153s
    #546207
  • Pipeline finished with Success
    2 months ago
    Total: 690s
    #546210
  • 🇺🇸United States smustgrave

    Curious why did the baseline change?

  • 🇮🇹Italy mondrake 🇮🇹

    Because fixing the errors that this rule surfaces is not trivial. So better leave that to follow ups IMHO, while the rule prevents adding more leaks.

  • Pipeline finished with Success
    about 2 months ago
    Total: 643s
    #561899
  • 🇺🇸United States smustgrave

    Oh I was just curious not that they should be fixed

  • 🇮🇹Italy mondrake 🇮🇹

    Well, the new rule introduces new errors, so either we fix them or we baseline them :)

  • 🇺🇸United States smustgrave

    Only other question is should annotations be attributes or does it not matter

  • 🇮🇹Italy mondrake 🇮🇹

    A check to prevent the use of annotation is already done by PHPUnit itself when it is configured to fail on PHPUnit deprecations. See 📌 Complete test annotations to attributes conversion for Drupal/Test/Component Active for an example.

    IMHO we should not replicate such a check here.

  • 🇺🇸United States smustgrave

    In that case I'll go on a limb and say this one is probably ready.

  • 🇬🇧United Kingdom catch

    Because fixing the errors that this rule surfaces is not trivial.

    Can you explain why this isn't trivial? I would have thought it would only be necessary to delete the annotations from the base classes and nothing else.

  • 🇮🇹Italy mondrake 🇮🇹

    Because I think we should check if the extending classes have the metadata implemented as the base class, and if not see why not and possibly convert to attributes.

  • Pipeline finished with Success
    about 1 month ago
    Total: 642s
    #568902
  • Pipeline finished with Failed
    22 days ago
    Total: 654s
    #581651
  • Pipeline finished with Failed
    19 days ago
    #584317
  • 🇮🇹Italy mondrake 🇮🇹

    Let's simplify - we do not have to list all the attributes, we can just check that no attributes in the namespace PHPUnit\Framework\Attributes are present - this also leaves space to additional attributes introduced by PHPUnit w/out needing to come back to this rule.

  • Pipeline finished with Failed
    19 days ago
    Total: 185s
    #584476
  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Success
    19 days ago
    Total: 506s
    #584482
  • Pipeline finished with Failed
    18 days ago
    Total: 245s
    #585898
  • Pipeline finished with Success
    18 days ago
    Total: 512s
    #585905
  • Pipeline finished with Failed
    14 days ago
    Total: 160s
    #588194
  • Pipeline finished with Success
    14 days ago
    Total: 411s
    #588202
  • Pipeline finished with Failed
    8 days ago
    Total: 218s
    #593606
  • Pipeline finished with Success
    8 days ago
    Total: 566s
    #593621
  • Pipeline finished with Success
    4 days ago
    Total: 656s
    #597325
  • Status changed to Fixed 1 day ago
Production build 0.71.5 2024