Exceptions thrown in event subscribers affect how tests are run and/or how their outcome is evaluated

[cspray]

Q	A
PHPUnit version	10.0.9
PHP version	8.2.3
Installation Method	Composer

Summary

A PHPUnit\Event\Test\PreparationStartedSubscriber throws an Exception.

Current behavior

The test that has had preparation started is silently discarded. There are no indications that the test failed.

How to reproduce

1. Create an Extension.
2. Register a PreparationStartedSubscriber and throw an Exception.
3. Ensure the Extension is set to be loaded in your PHPUnit Configuration.
4. Execute PHPUnit and see that no tests are run.

I have also reproduced this in an isolated repo: https://github.com/cspray/phpunit10-bugs under the PreparationStartedException directory.

Expected behavior

I would expect a test that has an exception thrown in a PreparationStartedSubscriber to be properly marked as a failure/error with information on the Exception that was thrown.

Composer Info Output

myclabs/deep-copy                  1.11.0             Create deep copies (clones) of your objects
nikic/php-parser                   v4.15.3            A PHP parser written in PHP
phar-io/manifest                   2.0.3              Component for reading phar.io manifest information from a PHP Archive (PHAR)
phar-io/version                    3.2.1              Library for handling version information and constraints
phpunit/php-code-coverage          10.0.0             Library that provides collection, processing, and rendering functionality for PHP code coverage information.
phpunit/php-file-iterator          4.0.1              FilterIterator implementation that filters files based on a list of suffixes.
phpunit/php-invoker                4.0.0              Invoke callables with a timeout
phpunit/php-text-template          3.0.0              Simple template engine.
phpunit/php-timer                  6.0.0              Utility class for timing
phpunit/phpunit                    10.0.9             The PHP Unit Testing framework.
roave/security-advisories          dev-latest 9206ad5 Prevents installation of composer packages with known security vulnerabilities: no API, simply require it
sebastian/cli-parser               2.0.0              Library for parsing CLI options
sebastian/code-unit                2.0.0              Collection of value objects that represent the PHP code units
sebastian/code-unit-reverse-lookup 3.0.0              Looks up which function or method a line of code belongs to
sebastian/comparator               5.0.0              Provides the functionality to compare PHP values for equality
sebastian/complexity               3.0.0              Library for calculating the complexity of PHP code units
sebastian/diff                     5.0.0              Diff implementation
sebastian/environment              6.0.0              Provides functionality to handle HHVM/PHP environments
sebastian/exporter                 5.0.0              Provides the functionality to export PHP variables for visualization
sebastian/global-state             6.0.0              Snapshotting of global state
sebastian/lines-of-code            2.0.0              Library for counting the lines of code in PHP source code
sebastian/object-enumerator        5.0.0              Traverses array structures and object graphs to enumerate all referenced objects
sebastian/object-reflector         3.0.0              Allows reflection of object attributes, including inherited and non-public ones
sebastian/recursion-context        5.0.0              Provides functionality to recursively process PHP variables
sebastian/type                     4.0.0              Collection of value objects that represent the types of the PHP type system
sebastian/version                  4.0.1              Library that helps with managing the version number of Git-hosted PHP projects
theseer/tokenizer                  1.2.1              A small library for converting tokenized PHP source code into XML and potentially other formats

[sebastianbergmann]

I would expect a test that has an exception thrown in a PreparationStartedSubscriber to be properly marked as a failure/error with information on the Exception that was thrown.

An exception thrown in a subscriber must not have any effect on how tests are run.

[sebastianbergmann]

Here is what I get:

.
├── phpunit.xml
├── src
│   ├── autoload.php
│   ├── Extension.php
│   └── TestPreparedSubscriber.php
└── tests
    └── Issue5219Test.php

phpunit.xml

<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:noNamespaceSchemaLocation="../../../../phpunit.xsd"
         bootstrap="src/autoload.php">
    <testsuites>
        <testsuite name="default">
            <directory>tests</directory>
        </testsuite>
    </testsuites>

    <extensions>
        <bootstrap class="PHPUnit\TestFixture\Issue5219\Extension"/>
    </extensions>
</phpunit>

src/autoload.php

namespace PHPUnit\TestFixture\Issue5219;

require __DIR__ . '/Extension.php';
require __DIR__ . '/TestPreparedSubscriber.php';

src/Extension.php

<?php declare(strict_types=1);
namespace PHPUnit\TestFixture\Issue5219;

use PHPUnit\Runner\Extension\Extension as ExtensionInterface;
use PHPUnit\Runner\Extension\Facade;
use PHPUnit\Runner\Extension\ParameterCollection;
use PHPUnit\TextUI\Configuration\Configuration;

final class Extension implements ExtensionInterface
{
    public function bootstrap(Configuration $configuration, Facade $facade, ParameterCollection $parameters): void
    {
        $facade->registerSubscriber(new TestPreparedSubscriber);
    }
}

src/TestPreparedSubscriber.php

<?php declare(strict_types=1);
namespace PHPUnit\TestFixture\Issue5219;

use PHPUnit\Event\Test\Prepared;
use PHPUnit\Event\Test\PreparedSubscriber;
use RuntimeException;

final class TestPreparedSubscriber implements PreparedSubscriber
{
    public function notify(Prepared $event): void
    {
        throw new RuntimeException('message');
    }
}

tests/Issue5219Test.php

<?php declare(strict_types=1);
namespace PHPUnit\TestFixture\Issue5219;

use PHPUnit\Framework\TestCase;

final class Issue5219Test extends TestCase
{
    public function testOne(): void
    {
        $this->assertTrue(true);
    }
}

You wrote

I would expect a test that has an exception thrown in a PreparationStartedSubscriber to be properly marked as a failure/error with information on the Exception that was thrown.

And this is exactly what I observe:

PHPUnit 10.0.9 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.3
Configuration: /usr/local/src/phpunit/tests/end-to-end/regression/5219/phpunit.xml

E                                                                   1 / 1 (100%)

Time: 00:00.087, Memory: 6.00 MB

There was 1 error:

1) PHPUnit\TestFixture\Issue5219\Issue5219Test::testOne
RuntimeException: message

/usr/local/src/phpunit/tests/end-to-end/regression/5219/src/TestPreparedSubscriber.php:20

ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

However, as I already wrote in #5219 (comment), this is wrong. The exception in the subscriber should not have any effect on how tests are run and/or how their outcome is evaluated.

[sebastianbergmann]

I think this is what we are missing:

diff --git a/src/Event/Dispatcher/DirectDispatcher.php b/src/Event/Dispatcher/DirectDispatcher.php
index ca430ec71..a7169b9ac 100644
--- a/src/Event/Dispatcher/DirectDispatcher.php
+++ b/src/Event/Dispatcher/DirectDispatcher.php
@@ -11,6 +11,7 @@
 
 use function array_key_exists;
 use function sprintf;
+use Throwable;
 
 /**
  * @internal This class is not covered by the backward compatibility promise for PHPUnit
@@ -80,7 +81,10 @@ public function dispatch(Event $event): void
         }
 
         foreach ($this->tracers as $tracer) {
-            $tracer->trace($event);
+            try {
+                $tracer->trace($event);
+            } catch (Throwable) {
+            }
         }
 
         if (!array_key_exists($eventClassName, $this->subscribers)) {
@@ -88,7 +92,10 @@ public function dispatch(Event $event): void
         }
 
         foreach ($this->subscribers[$eventClassName] as $subscriber) {
-            $subscriber->notify($event);
+            try {
+                $subscriber->notify($event);
+            } catch (Throwable) {
+            }
         }
     }
 }

This would ignore all exceptions thrown in subscribers.

[sebastianbergmann]

I just verified that the patch shown in #5219 (comment) has the desired effect for your use case, too.

[sebastianbergmann]

@cspray Thank you for reporting this issue!

[cspray]

@sebastianbergmann I noticed in the example you replicated, the event is the PHPUnit\Event\Test\Prepared event. The event I originally submitted the issue about was the PHPUnit\Event\Test\PreprationStarted event. I copied your example to my local machine, updated the TestPreparedSubscriber to implement the PreparationStartedSubscriber and I see the same error I reported.

PHPUnit 10.0.9 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.3
Configuration: /home/cspray/Code/personal/phpunit-10-prepstarted-exception/Regression5219Emulation/phpunit.xml

No tests executed!

The changes I made to your TestPreparedSubscriber

<?php

namespace PHPUnit\TestFixture\Issue5219;

use PHPUnit\Event\Test\PreparationStarted;
use PHPUnit\Event\Test\PreparationStartedSubscriber;

class TestPreparedSubscriber implements PreparationStartedSubscriber {

    public function notify(PreparationStarted $event) : void {
        throw new \RuntimeException('message');
    }
}

[sebastianbergmann]

I am sorry, but I do not understand what you are trying to tell me.

As of 74ce5b6, all exceptions thrown in third-party subscribers are ignored (as they should be). For this, it does not matter which event a subscriber subscribes to.