Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
All update path tests should be legacy tests because they are testing upgrading data that might trigger deprecated code paths. However the new BrowserTestBase variants based on \Drupal\FunctionalTests\Update\UpdatePathTestBase don't trigger deprecation warnings. We need to find out why.
Proposed resolution
Update PHPUnit Bridge once https://github.com/symfony/symfony/pull/25685 is in.
Remaining tasks
User interface changes
None
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#9 | 2934336-9.patch | 2.35 KB | alexpott |
#7 | 2934336-7.patch | 4.78 KB | alexpott |
#7 | 3-7-interdiff.txt | 2.79 KB | alexpott |
#3 | 2934336-3.patch | 3.13 KB | alexpott |
#3 | 2-3-interdiff.txt | 716 bytes | alexpott |
Comments
Comment #2
alexpottHere's a test that is green when it shouldn't be. A similar test
\Drupal\FunctionalTests\Core\Test\PhpUnitBridgeTest
shows that BrowserTestBase supports deprecation testing. So something is special about \Drupal\FunctionalTests\Update\UpdatePathTestBase().Comment #3
alexpottSo this looks like symfony fun... because changing the SYMFONY_DEPRECATIONS_HELPER to strict makes update path tests test deprecations again...
\Symfony\Bridge\PhpUnit\DeprecationErrorHandler::register() is determining that the deprecation error came from vendor and not from core :(
Comment #4
alexpottThe problem is the interaction between weak vendors and how Symfony's deprecation helper works for tests that run in a separate process.
Comment #6
alexpottCreated a PR to fix this in Symfony - https://github.com/symfony/symfony/pull/25685
Comment #7
alexpottNew symfony with the PR in has been released. It's working so this should fail. Hopefully there is only 1 fail.
Comment #9
alexpottSo yay that works... I think we should just make this issue about updating
symfony/phpunit-bridge
since that is where the bug lies. And then once #2916740: Add generic entity actions we fix all the update tests to be legacy tests and expected all the deprecations in #2934340: Remove action deprecations from DeprecationListenerTrait::getSkippedDeprecationsComment #10
alexpottComment #11
chr.fritschShouldn't we add the failing test from #7 to the current patch plus a setExpectedException?
Comment #12
alexpott@chr.fritsch nope - because if you set the expected deprecation it would work in HEAD. This is because the deprecation would be caught by yet another error handler (
\Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::handleError()
). So it would prove nothing :( And we can't add a failing test.Comment #13
dawehnerThanks a lot to the symfony community having such a fast process to fix issues, seriously!
This is interesting ...
Comment #14
Wim Leers@alexpott: wow — thank you so much for your continued diligence and upstream PRs to fix problems before they even become problems for the Drupal community!
:)
Comment #16
catchCommitted d1e91cb and pushed to 8.5.x. Thanks!
Comment #17
Wim Leers#2858482: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent was green and RTBC for >1 week. It failed just now. It looks like this change probably caused it to start failing… any help would be greatly appreciated :(
Comment #18
alexpott#17 was just a missing fullstop in the skipped deprecation messages added by the patch. This patch correctly fixed deprecation error handling.
Comment #19
Wim Leers… and @alexpott filed two PRs due to this:
🎉
Thank you so much, @alexpott!
EDIT: looks like both of those PRs will ship with Symfony 3.4!