Deprecations in data provider affect all tests

[nikophil]

Q	A
PHPUnit version	12.2
PHP version	any
Installation Method	Composer

Since #6165, when a deprecation is triggered in a data provider, it affects all tests:

This test:

class TriggersDeprecationInDataProviderTest extends TestCase
{
    #[Test]
    public function method1(): void
    {
        self::assertTrue(true);
    }

    #[Test]
    #[DataProvider('dataProvider')]
    public function method2(bool $value): void
    {
        self::assertTrue($value);
    }

    public static function dataProvider(): iterable
    {
        @trigger_error('some deprecation', \E_USER_DEPRECATED);

        yield [true];
    }

    #[Test]
    public function method3(): void
    {
        self::assertTrue(true);
    }
}

would display this output:

DDD                                                                 3 / 3 (100%)

Time: 00:00.001, Memory: 16.00 MB

3 tests triggered 1 deprecation:

1) /home/nicolas/works/github.com/sebastianbergmann/phpunit/tests/end-to-end/deprecation-trigger/_files/deprecation-in-data-provider/tests/TriggersDeprecationInDataProviderTest.php:26
some deprecation

Triggered by:

* PHPUnit\TestFixture\DeprecationInDataProvider\TriggersDeprecationInDataProviderTest::method1
  /home/nicolas/works/github.com/sebastianbergmann/phpunit/tests/end-to-end/deprecation-trigger/_files/deprecation-in-data-provider/tests/TriggersDeprecationInDataProviderTest.php:12

* PHPUnit\TestFixture\DeprecationInDataProvider\TriggersDeprecationInDataProviderTest::method2#0
  /home/nicolas/works/github.com/sebastianbergmann/phpunit/tests/end-to-end/deprecation-trigger/_files/deprecation-in-data-provider/tests/TriggersDeprecationInDataProviderTest.php:19

* PHPUnit\TestFixture\DeprecationInDataProvider\TriggersDeprecationInDataProviderTest::method3
  /home/nicolas/works/github.com/sebastianbergmann/phpunit/tests/end-to-end/deprecation-trigger/_files/deprecation-in-data-provider/tests/TriggersDeprecationInDataProviderTest.php:32

OK, but there were issues!
Tests: 3, Assertions: 3, Deprecations: 1.

I think the problem is because ErrorHandler::$globalDeprecations is not emptied in ErrorHandler::triggerGlobalDeprecations().

A straightforward fix would be:

    private function triggerGlobalDeprecations(): void
    {
        foreach ($this->globalDeprecations ?? [] as $d) {
            $this->__invoke(...$d);
        }

+        $this->globalDeprecations = [];
    }

But this fix seems brittle:

• all the "data provider deprecations" would be affected to the first test, although they are not related to this specific test
• if this test has #[IgnoreDeprecation] attribute, all those "data provider deprecations" would suddenly disappear

One cool thing to do would be to manage to connect the "data provider deprecations" to their related test method.
So that the above test would output:

.D.                                                                 3 / 3 (100%)

Time: 00:00.001, Memory: 16.00 MB

1 test triggered 1 deprecation:

Maybe in ErrorHandler::deprecationHandler() we could guess from which test we're coming from (something similar to https://github.com/sebastianbergmann/phpunit/blob/main/src/Runner/ErrorHandler.php#L101 or by leveraging DataProviderMethodCalled/DataProviderMethodFinished events), and defer the trigger of the deprecation only when we reach the test(s) connected to the data provider?

WDYT?

/cc @soyuka

[nikophil]

I just realized that this problem is very similar to this issue.

I assume that the related PR was never merged because of the involved complexity?

[soyuka]

Its a hard problem because of when the error handlers are registered and how errors are collected. I think that we could skip the data provider deprecation although I find it weird that a data provider triggers deprecation. It should be a function that provides data for a test, having deprecation there is probably an indicator that the provider does too much no?

For example you'd not want to expect a deprecation from a data provider, you would from a test and if the data provider triggers that deprecation I don't think that we should be able to expect it inside the test.

[nikophil]

if you use some external lib to generate objects in your data provider, let's say, for instance, if you use Foundry, you might have deprecations in your data provider. I'd rather those to be displayed to the user, but I understand that this is a complex topic.

For example you'd not want to expect a deprecation from a data provider, you would from a test and if the data provider triggers that deprecation I don't think that we should be able to expect it inside the test.

I'm not sure, wouldn't it be nice that the test output says "deprecation triggered in the data provider related to [test name]"?

Or add a totally new concept of "data provider deprecation", as in the work @sebastianbergmann started here but I do understand that it was way too complex.

Its a hard problem because of when the error handlers are registered and how errors are collected. I think that we could skip the data provider deprecation [...]

yeah, I have in mind a solution for this for which I can try to provide an implementation, but maybe that's also too complex.

Indeed another solution would be to completely ignore data provider deprecations, but don't we also need the error handler to know if we are in a data provider for this?

At the end, I'd be pleased with any solution, but I think we need to fix something somewhere. In Foundry's CI we need to test some behavior with PHPUnit 12 which triggers a deprecation in a data provider, and then we have this output:

 .DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD........DDDDDDDDDD.DDDDDDD  63 / 643 (  9%)
DD...DD...DDDDDDDDDDD.DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD 126 / 643 ( 19%)
DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD.................... 189 / 643 ( 29%)
...........................DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD 252 / 643 ( 39%)
DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD.............................. 315 / 643 ( 48%)
....................DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD. 378 / 643 ( 58%)
.........................................DDDDDDDDDDDDDDDDDDDDDD 441 / 643 ( 68%)
DDDDDDDDDDDDD...............................................DDD 504 / 643 ( 78%)
D...DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD.DD.DDDDDDDDD 567 / 643 ( 88%)
DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD.....DDDDDDDDDDDDDDD... 630 / 643 ( 97%)
........

Time: 00:38.290, Memory: 156.50 MB

414 tests triggered 4 deprecations:

notice the number of deprecations VS the number of tests 😅. (The tests which do not trigger the deprecations are flagged with #[IgnoreDeprecations] attribute)