Bootstrap HtmlOutputLogger from phpunit.xml

Created on 8 June 2024, 18 days ago
Updated 18 June 2024, 7 days ago

Problem/Motivation

When removing the HtmlOutputPrinter listener in PHPUnit 10, we replaced it with the HtmlOutputLogger extension, and we initialize it from the bootstrap.php file. However, the canonical way to bootstrap PHPUnit extensions is by including it in the phpunit.xml, https://docs.phpunit.de/en/11.0/configuration.html#the-extensions-element

See https://docs.phpunit.de/en/11.0/extending-phpunit.html#implementing-an-e...

Also see https://drupal.slack.com/archives/C1BMUQ9U6/p1717659911747249

Proposed resolution

  • Change the extension bootstrap to phpunit.xml
  • Return the default to print out the list of all the links generated (verbose=true)

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

📌 Task
Status

RTBC

Version

11.0 🔥

Component
PHPUnit 

Last updated about 9 hours ago

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 !8343Closes #3453341 → (Open) created by mondrake
  • Pipeline finished with Failed
    18 days ago
    Total: 183s
    #194319
  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Failed
    17 days ago
    Total: 640s
    #194324
  • Status changed to Needs review 17 days ago
  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Success
    17 days ago
    Total: 597s
    #194357
  • 🇬🇧United Kingdom longwave UK

    We will need a change record as anyone with a custom phpunit.xml will need to add this.

    I wonder if we should configure it with the extension configuration instead of using environment variables, or perhaps support both methods?

    Let's also let @alexpott confirm that this solves the problem he found with paratest.

  • 🇮🇹Italy mondrake 🇮🇹

    +1 on adding config parameters in the xml, and deprecating the two environment variables.

  • 🇬🇧United Kingdom longwave UK

    Re: the change record and custom phpunit.xml, I wonder if we can detect from bootstrap.php or elsewhere that the extension is not loaded, and print some kind of notice.

  • 🇮🇹Italy mondrake 🇮🇹

    I've tried #7 for deprecating BROWSERTEST_OUTPUT_VERBOSE, although one may argue this is not really a deprecation since there is no point release using it yet.

    Deprecating BROWSERTEST_OUTPUT_DIRECTORY is somehow more complicated, since its value is set dynamically in code by PhpUnitTestRunner - for testing purposes and to determine realpath. I'd rather try do that in a follow-up.

  • Pipeline finished with Success
    17 days ago
    Total: 614s
    #194730
  • Pipeline finished with Failed
    16 days ago
    Total: 237s
    #194842
  • Pipeline finished with Success
    16 days ago
    Total: 624s
    #194856
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    ++ this works great and fixes running Drupal tests using paratest. We should definitely do this. Thanks @mondrake.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    I've applied this fix to #3452524: Fix tests on 11.x due to PHPUnit 10 and it works great. See https://git.drupalcode.org/project/distributions_recipes/-/jobs/1822778 for a successful run using paratest.

  • Assigned to mondrake
  • Status changed to Needs work 15 days ago
  • 🇮🇹Italy mondrake 🇮🇹

    Let's see if I can please everyone with the following:
    1. Let's have xml parameters for output directory and verbose, with defaults. Verbose = true
    2. Let's keep the two env variables as overrides of the xml parameters, if they exist (i.e. no more deprecation)

    This way we combine both worlds, and on CI we do not have to tamper with the xml file.

    On that.

  • 🇮🇹Italy mondrake 🇮🇹
  • Issue was unassigned.
  • Status changed to Needs review 15 days ago
  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Success
    15 days ago
    Total: 665s
    #195663
  • Pipeline finished with Failed
    15 days ago
    Total: 543s
    #195735
  • Pipeline finished with Failed
    15 days ago
    Total: 540s
    #195752
  • Pipeline finished with Success
    15 days ago
    Total: 509s
    #195783
  • 🇮🇹Italy mondrake 🇮🇹

    Added a draft CR.

  • Status changed to RTBC 15 days ago
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    This looks fantastic - nice work @mondrake

    Raising this to major as it fixes a regression in Drupal 11. The regression is the ability to use paratest - which works great on D10. I think with this change and the move of deprecation reporting to PHPUnit it might be finally possible to consider removing run-tests.sh in favour of paratest which would be quite amazing.

  • 🇮🇹Italy mondrake 🇮🇹

    Once this is in, the main CR for PHPUnit 10 will need adjustment.

  • Status changed to Needs work 7 days ago
  • 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.

  • Status changed to RTBC 7 days ago
  • 🇮🇹Italy mondrake 🇮🇹

    Rebased, only context changes

  • Pipeline finished with Success
    7 days ago
    Total: 682s
    #202356
Production build 0.69.0 2024