[Symfony 6.1] Replace deprecationListenerTrait with PHPUnitBridge ignoreFile

Nice work @mondrake - it's great to have this functionality pushed upstream. I always hoped the baseline generation would replace this but moving this functionality upstream makes sense as well.

Good that this is all pushed upstream instead, some minor questions but overall this is great work.

Great work. We are removing a whole bunch of custom code and replacing it with a few lines that defer to upstream, along with a separate config file that's easier to use than what we had before.

...although this only works for people who use run-tests.sh. If you run phpunit directly, deprecations are now shown in the CLI.

Before:

$ ddev exec vendor/bin/phpunit -c core core/tests/Drupal/Tests/Core/StringTranslation/TranslationManagerTest.php
PHPUnit 9.5.20 #StandWithUkraine

Testing Drupal\Tests\Core\StringTranslation\TranslationManagerTest
..........                                                        10 / 10 (100%)

Time: 00:00.039, Memory: 4.00 MB

OK (10 tests, 26 assertions)

After:

$ ddev exec vendor/bin/phpunit -c core core/tests/Drupal/Tests/Core/StringTranslation/TranslationManagerTest.php
#PHPUnit 9.5.20 #StandWithUkraine

Testing Drupal\Tests\Core\StringTranslation\TranslationManagerTest
..........                                                        10 / 10 (100%)

Time: 00:00.052, Memory: 6.00 MB

OK (10 tests, 26 assertions)

Remaining self deprecation notices (2)

  2x: Method "Behat\Mink\Element\ElementInterface::getText()" might add "string" as a native return type declaration in the future. Do the same in implementation "Drupal\Tests\DocumentElement" now to avoid errors or add an explicit @return annotation to suppress this message.
    2x in DrupalListener::endTest from Drupal\Tests\Listeners

Remaining direct deprecation notices (3)

  1x: The "PHPUnit\TextUI\DefaultResultPrinter" class is considered internal This class is not covered by the backward compatibility promise for PHPUnit. It may change without further notice. You should not use it from "Drupal\Tests\Listeners\HtmlOutputPrinter".
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

  1x: The "Drupal\Tests\Listeners\DrupalListener" class implements "PHPUnit\Framework\TestListener" that is deprecated.
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

  1x: The "Drupal\Tests\Listeners\DrupalListener" class uses "PHPUnit\Framework\TestListenerDefaultImplementation" that is deprecated The `TestListener` interface is deprecated.
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

Other deprecation notices (1)

  1x: The "PHPUnit\Framework\TestCase::addWarning()" method is considered internal This method is not covered by the backward compatibility promise for PHPUnit. It may change without further notice. You should not extend it from "Drupal\Tests\UnitTestCase".
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

Failed to execute command vendor/bin/phpunit -c core core/tests/Drupal/Tests/Core/StringTranslation/TranslationManagerTest.php: exit status 1

How about setting the default in phpunit.xml.dist?

    <!-- To use an alternative deprecation ignore file change the next line. -->
    <env name="SYMFONY_DEPRECATIONS_HELPER" value="ignoreFile=core/.deprecation-ignore.txt"/>
    <!-- To disable deprecation testing completely use the next line. -->
    <!-- <env name="SYMFONY_DEPRECATIONS_HELPER" value="disabled"/> -->

Annoyingly though it depends on which directory you run PHPUnit from, so maybe we should just document it there but left commented out.

I think that is a fine way of explaining it.

I suspect this might need a release note snippet as it affects users who run tests, but will leave that to core committers to decide.

This is borderline for release notes for me since it won't affect site owners as such just people running tests. However we should put instructions in the change record which currently just says @todo.

We can also consider updating the phpunit bridge to 6.1 on Drupal 9 - that would allow us to get this in earlier. FWIW maybe we should fix the CWD inconsistency as that would make this simpler and have other benefits - most notably consistency and the principle of least surprise.

@alexpott unfortunately the path entirely depends on how you run PHPUnit. The following are equivalent:

SYMFONY_DEPRECATIONS_HELPER=ignoreFile=core/.deprecation-ignore.txt vendor/bin/phpunit -c core core/modules/SomeTest.php

cd core
SYMFONY_DEPRECATIONS_HELPER=ignoreFile=.deprecation-ignore.txt ../vendor/bin/phpunit modules/SomeTest.php

so I'm not sure how we can make this consistent.

Maybe upstream can fix so ignoreFile is relative to the PHPUnit configuration directory instead of cwd?

Actually I think we can make this simpler. We can set the SYMFONY_DEPRECATIONS_HELPER envvar in core/tests/bootstrap.php if it is not set instead of run-tests.sh. That way we provide todays behaviour without any change.

Perfect - this works for me locally running vendor/bin/phpunit without having to set an env var at all, but if I do set the variable then it overrides the core default.

However the change record needs updating again as this has all been simplified and we don't need to tell people to update their setups, only if they want custom deprecations now.

Thanks, the documentation changes all look good to me.

Re backport I believe we cannot add new deprecations to 9.4.x now or removals in 10.0.x, now but we might want to add some to 9.5.x and 10.0.x for removal in 11.0.0, so maybe it is worth it? Not sure.

We'll end up with a handful of 9.5.x deprecations for removal in 10.0.0 or 11.0.0, but I don't think we'd need to add them to the skipped deprecation list - we'd be removing the usages too at the same time. It's possible we might need to use it for PHP 8.2 stuff though as we have with EasyRDF - but we only have one of those currently for PHP 8.1, so not sure it's worth trying to backport for that either.

Committed d77a68d and pushed to 10.0.x. Thanks!