Configuration Merger lead to unduplicated extensionBootstrappers entries

[llaville]

Q	A
PHPUnit version	11.5.36
PHP version	8.3.6
Installation Method	Composer v2

Summary

Performance and side effect issues with Configuration::extensionBootstrappers() (https://github.com/sebastianbergmann/phpunit/blob/11.5/src/TextUI/Configuration/Configuration.php#L869)

Context: I'm working on a PHPUnit extension and play with all events available when I detect this issue

Current behavior

Nothing are visible because only PHAR extensions are displayed on the PHPUnit header

For example :

PHPUnit 11.5.36 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.6
Configuration: /home/devilbox/data/sarif-php-sdk/phpunit.xml
Extension:     ergebnis/phpunit-slow-test-detector 2.20.0

How to reproduce

Invoke PHPUnit with both an extension loaded by XML configuration and from CLI arguments

For example (I've reduced XML config file, only to show extension context usage)

The "phpunit-extensions" folder contains a copy of https://github.com/ergebnis/phpunit-slow-test-detector/releases/download/2.20.0/phpunit-slow-test-detector.phar

<phpunit
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/11.5/phpunit.xsd"
    bootstrap="vendor/autoload.php"
    extensionsDirectory="../phpunit-extensions"
>
    <extensions>
        <bootstrap class="Ergebnis\PHPUnit\SlowTestDetector\Extension">
            <parameter name="maximum-count" value="50"/>
        </bootstrap>
    </extensions>
</phpunit>

Execute command : vendor/bin/phpunit --extension Ergebnis\\PHPUnit\\SlowTestDetector\\Extension that use the previous XML config declaration

Expected behavior

De-dupplicated entries in https://github.com/sebastianbergmann/phpunit/blob/11.5/src/TextUI/Configuration/Merger.php#L316-L332

I propose this patch

diff --git a/src/TextUI/Configuration/Merger.php b/src/TextUI/Configuration/Merger.php
index 578031402..176a6c2ab 100644
--- a/src/TextUI/Configuration/Merger.php
+++ b/src/TextUI/Configuration/Merger.php
@@ -329,7 +329,7 @@ public function merge(CliConfiguration $cliConfiguration, XmlConfiguration $xmlC

         if ($cliConfiguration->hasExtensions()) {
             foreach ($cliConfiguration->extensions() as $extension) {
-                $extensionBootstrappers[] = [
+                $extensionBootstrappers[$extension] = [
                     'className'  => $extension,
                     'parameters' => [],
                 ];
@@ -337,7 +337,7 @@ public function merge(CliConfiguration $cliConfiguration, XmlConfiguration $xmlC
         }

         foreach ($xmlConfiguration->extensions() as $extension) {
-            $extensionBootstrappers[] = [
+            $extensionBootstrappers[$extension->className()] = [
                 'className'  => $extension->className(),
                 'parameters' => $extension->parameters(),
             ];

Performance side effects : avoid to bootstrap twice the same extension at Application::bootstrapExtensions() (https://github.com/sebastianbergmann/phpunit/blob/11.5/src/TextUI/Application.php#L430-L435)

[sebastianbergmann]

You just can't stop finding new ways to use PHPUnit incorrectly, can you? 😄 I mean no disrespect in saying that; quite the opposite, in fact, as it helps us to identify areas where we can improve the way we inform users about mistakes they make.