Deprecate TestDiscovery test file scanning, use PHPUnit API instead

Created on 5 January 2025, 3 months ago

Problem/Motivation

Drupal is using its own code to discover tests to be executed, which along time lead to discrepancies in the tests executed betwen PHPUnit CLI and run-tests.sh.

PHPUnit 10 introduced an API to discover the test suites, that can be used instead.

Proposed resolution

  • Deprecate TestDiscovery::getTestClasses() and related methods.
  • Introduce a like-for-like replacement that uses PHPUnit's API for the discovery, and enriches the information returned by the API to provide same info as TestDiscovery::getTestInfor().
  • Make the new code ready for PHpUnit's 10+ attributes instead of annotations.
  • In the process, fix some bugs of TestDiscovery::getTestClasses() that were identified:
    • for each test class, it only reports groups present in the class-level annotations: this means that method level annotations do not qualify test classes for a group, potentially missing the execution of some tests. PHPUnit is more accurate in bubbling up method-level annotations/attributes to the class
    • it does not check duplication of @group annotations that specify the same group. PHPUnit make them unique.
    • when filtering by test suite, it reports some test groups with an empty array, since in any case it iterates all test directories and files. PHPUnit only scans the directories indicated in phpunit.xml for each test suite, providing more accurate results.

Remaining tasks

User interface changes

nope

API changes

nope

Data model changes

nope

Release notes snippet

Drupal tests now can use PHPUnit's 10+ attributes instead of the legacy annotations. New tests MUST use attributes.

📌 Task
Status

Active

Version

11.0 🔥

Component

phpunit

Created by

🇮🇹Italy mondrake 🇮🇹

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

Merge Requests

Comments & Activities

  • Issue created by @mondrake
  • Merge request !10809Closes #3497431 → (Open) created by mondrake
  • Pipeline finished with Failed
    3 months ago
    Total: 136s
    #386235
  • Pipeline finished with Failed
    3 months ago
    Total: 175s
    #386257
  • Pipeline finished with Failed
    3 months ago
    #386276
  • Pipeline finished with Failed
    3 months ago
    #386277
  • Pipeline finished with Failed
    3 months ago
    Total: 88s
    #386279
  • Pipeline finished with Failed
    3 months ago
    Total: 697s
    #386315
  • Pipeline finished with Failed
    3 months ago
    Total: 420s
    #386438
  • Pipeline finished with Failed
    3 months ago
    Total: 131s
    #386605
  • Pipeline finished with Failed
    3 months ago
    Total: 429s
    #386613
  • Pipeline finished with Failed
    3 months ago
    Total: 345s
    #386630
  • Pipeline finished with Failed
    3 months ago
    Total: 702s
    #386639
  • Pipeline finished with Failed
    3 months ago
    Total: 129s
    #386645
  • Pipeline finished with Failed
    3 months ago
    Total: 89s
    #386651
  • Pipeline finished with Failed
    3 months ago
    Total: 990s
    #386652
  • Pipeline finished with Failed
    3 months ago
    Total: 1222s
    #387009
  • Pipeline finished with Success
    3 months ago
    Total: 709s
    #387172
  • Pipeline finished with Failed
    3 months ago
    Total: 503s
    #387395
  • Pipeline finished with Failed
    3 months ago
    Total: 129s
    #387696
  • Pipeline finished with Success
    3 months ago
    Total: 495s
    #387707
  • Pipeline finished with Success
    3 months ago
    Total: 505s
    #387737
  • Pipeline finished with Canceled
    3 months ago
    Total: 117s
    #387746
  • Pipeline finished with Failed
    3 months ago
    Total: 111s
    #387750
  • Pipeline finished with Failed
    3 months ago
    Total: 381s
    #388227
  • Pipeline finished with Failed
    3 months ago
    Total: 112s
    #388331
  • Pipeline finished with Success
    3 months ago
    Total: 382s
    #388341
  • 🇮🇹Italy mondrake 🇮🇹
  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Failed
    3 months ago
    Total: 141s
    #390881
  • Pipeline finished with Failed
    3 months ago
    Total: 317s
    #390921
  • Pipeline finished with Failed
    3 months ago
    Total: 331s
    #390964
  • 🇮🇹Italy mondrake 🇮🇹

    We are hitting a PHPUnit bug here, The file attribute of node of XML test list can be wrong. So we still need to read the file of a test class from the classloader.

  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Success
    3 months ago
    Total: 1231s
    #392885
  • 🇮🇹Italy mondrake 🇮🇹

    I think we can still avoid the classloader. On it.

  • Pipeline finished with Canceled
    3 months ago
    Total: 411s
    #397047
  • Pipeline finished with Failed
    3 months ago
    Total: 5512s
    #397051
  • Pipeline finished with Failed
    3 months ago
    Total: 626s
    #397148
  • 🇮🇹Italy mondrake 🇮🇹

    Very interesting side effect of this - since PHPUnit discovery actually executes the dataproviders, and includes in the discovery list as many test cases as the number of datasets provided for a test method, we can now be precise in the number of test cases run for each test class, and use that information in run-tests.sh instead of the current approximate count.

  • Pipeline finished with Failed
    3 months ago
    Total: 654s
    #397523
  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Failed
    3 months ago
    Total: 1661s
    #397531
  • Pipeline finished with Failed
    3 months ago
    Total: 492s
    #397680
  • Pipeline finished with Failed
    3 months ago
    Total: 3578s
    #397725
  • Pipeline finished with Failed
    3 months ago
    Total: 560s
    #397804
  • Pipeline finished with Failed
    3 months ago
    Total: 125s
    #397813
  • Pipeline finished with Canceled
    3 months ago
    Total: 249s
    #398024
  • Pipeline finished with Success
    3 months ago
    Total: 700s
    #398029
  • Pipeline finished with Failed
    3 months ago
    Total: 208s
    #400143
  • Pipeline finished with Failed
    3 months ago
    Total: 179s
    #400161
  • Pipeline finished with Failed
    3 months ago
    Total: 497s
    #400164
  • Pipeline finished with Failed
    3 months ago
    Total: 1015s
    #400252
  • First commit to issue fork.
  • Pipeline finished with Success
    3 months ago
    Total: 840s
    #411054
  • 🇬🇧United Kingdom catch

    This looks really good and cleans up quite a lot of mess.

    I went ahead and added @group #slow to mainly kernel tests that are now ending up towards the end of the run. This is not a fault in the ordering logic at all, it's just a side effect that some tests that happened to run earlier in a test run now could run later either purely because other tests moved around, or subtle changes in the method counting logic.

    The debugging logic at the top of the test runs is great and I wonder if we should just keep that all the time.

  • 🇮🇹Italy mondrake 🇮🇹

    I was thinking in a follow up we could introduce a “weight“ to #slow (i.e. #slow300, #slow200, etc) and further tune the start sequence.

  • 🇬🇧United Kingdom catch

    @mondrake I think at the moment this is OK because we have less #slow tests than we run tests concurrently per-job (e.g. we might have 5 slow tests but start 8 at the same time), however it took a while for that to be the case and it might not be the case again, so definitely worth an issue. Even now it might save a few seconds which is worth having.

  • 🇮🇹Italy mondrake 🇮🇹

    We can leverage on PHPUnit API also to determine the test description, instead of doing separate parsing. On it.

  • Pipeline finished with Failed
    2 months ago
    Total: 428s
    #413389
  • Pipeline finished with Failed
    2 months ago
    Total: 179s
    #413442
  • Pipeline finished with Success
    2 months ago
    Total: 357s
    #413443
  • 🇮🇹Italy mondrake 🇮🇹

    Done #14.

  • 🇮🇹Italy mondrake 🇮🇹

    Filed 📌 Introduce a “weight“ to #slow tests Postponed re #12 and #13.

  • Pipeline finished with Canceled
    2 months ago
    Total: 106s
    #422211
  • Pipeline finished with Failed
    2 months ago
    Total: 553s
    #422212
  • 🇮🇹Italy mondrake 🇮🇹

    merged with HEAD after commit of 🐛 Extend PhpUnitTestDiscoveryTest to test also PHPUnit API Active

  • Pipeline finished with Success
    2 months ago
    Total: 4893s
    #424571
  • Pipeline finished with Failed
    about 2 months ago
    Total: 731s
    #434842
  • Pipeline finished with Failed
    about 2 months ago
    Total: 329s
    #435548
  • Pipeline finished with Success
    about 2 months ago
    Total: 439s
    #436786
  • Pipeline finished with Success
    about 2 months ago
    Total: 421s
    #439168
  • Pipeline finished with Success
    about 2 months ago
    Total: 1557s
    #439579
  • Pipeline finished with Failed
    about 1 month ago
    Total: 599s
    #441372
  • Pipeline finished with Failed
    about 1 month ago
    Total: 518s
    #447250
  • Pipeline finished with Success
    26 days ago
    Total: 624s
    #456038
  • Pipeline finished with Success
    24 days ago
    Total: 780s
    #457802
  • Pipeline finished with Failed
    12 days ago
    Total: 660s
    #466780
  • Status changed to Needs review 4 days ago
  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Success
    4 days ago
    Total: 2642s
    #473283
  • 🇮🇹Italy mondrake 🇮🇹

    Thanks for review @mstrelan, added some of your suggestions and replied inline to some other ones; NW to address the remaining comments.

  • Pipeline finished with Success
    4 days ago
    Total: 533s
    #473617
  • Pipeline finished with Success
    4 days ago
    Total: 731s
    #473627
  • Pipeline finished with Success
    4 days ago
    Total: 1934s
    #473734
  • Pipeline finished with Failed
    4 days ago
    Total: 219s
    #473786
  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Success
    3 days ago
    Total: 1135s
    #473931
  • Pipeline finished with Failed
    2 days ago
    Total: 120s
    #474982
  • Pipeline finished with Failed
    2 days ago
    Total: 120s
    #474985
  • Pipeline finished with Failed
    2 days ago
    Total: 532s
    #475047
Production build 0.71.5 2024