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
Symfony's PHPUnit-bridge provides a couple of traits to polyfill lower-version-upwards methods that PHPUnit introduces in its latest versions, https://symfony.com/doc/current/components/phpunit_bridge.html#polyfills....
Proposed resolution
- Adjust
Drupal\TestTools\PhpUnitCompatibility\PhpUnit8\ClassWriter::mutateTestBase
so that it rewrites thePHPUnit\Framework\Assert
class as well, as we do forPHPUnit\Framework\TestCase
, and for each of them inject the polyfill trait as part of the rewrite. - Replace calls of
assertDirectoryNotExists
withassertDirectoryDoesNotExist
, and remove the deprecation silencer, to prove the model works properly.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#30 | 3174200-30.patch | 13.59 KB | mondrake |
Issue fork drupal-3174200
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3174200-use-phpunit-bridge-polyfills changes, plain diff MR !255
Comments
Comment #2
mondrakeOut of curiosity, a patch built on top of #3127141: Use PHPUnit 9 for PHP 7.4+, while keeping support for PHPUnit 8.4 in PHP 7.3 to replace
assertDirectoryNotExists
.Comment #3
mondrakeHuh, I'm afraid we'd have to rewrite the
PHPUnit\Framework\Assert
class as well, as we do forPHPUnit\Framework\TestCase
inDrupal\TestTools\PhpUnitCompatibility\PhpUnit8\ClassWriter::mutateTestBase
, in order to get this to work in PHPUnit8...Comment #4
mondrakeComment #5
mondrakeDoing #3.
Comment #6
longwaveHow does Symfony deal with the Assert class BC issue?
Comment #7
mondrake@longwave - same as here - they rewrite the class https://github.com/symfony/phpunit-bridge/blob/master/bin/simple-phpunit...
Comment #8
mondrakeComment #9
mondrakeComment #10
mondrakeComment #11
mondrakeComment #12
mondrakeComment #13
mondrakeReuploading patch in #9 that is still good after commit of #3127141: Use PHPUnit 9 for PHP 7.4+, while keeping support for PHPUnit 8.4 in PHP 7.3.
This approach would allow changing how we manage tests' codebase in relation to PHPUnit versions: instead of trying to fit latest versions of PHPUnit by providing BC backfills, we would now be able to convert methods deprecated in latest PHPUnit even before we adopt them, and the polyfills help us supporting also PHPUnit versions where the new methods are not yet available.
For this reason, if we move forward here I think we should backport this to 8.9 so that contrib too can leverage the polyfills for their branches that are currently supporting both D8 and D9.
Comment #16
mondrakeReroll and changed to MR workflow.
Comment #18
mondrakererolled; no longer sure this needs a backport with 6 months to go before D8 end of support.
Comment #19
longwaveCouple of comments on the MR, but otherwise I think this is a good idea - letting us get rid of PHPUnit deprecations early.
I did think we could merge the two methods into one common mutator and pass it configuration in the form of filenames and sets of regexps, but it seems overkill for two files - if we start mutating more than two maybe we should try that.
I notice the Symfony trait is marked @internal, but the TestCase one is too, and as this is in test code we can react fairly easily if Symfony changes something under us, so I think that's OK.
Comment #20
mondrakeOK, still maybe we can do a little in that sense.
Comment #21
longwaveOne little comment nit but otherwise this looks ready to go.
Comment #22
mondrakeThanks!
Comment #23
longwaveLooks good.
Comment #24
alexpottThis looks really smart. It's nice to be able to fix PHPUnit 10 deprecations in our code and will help people move to PHPUnit 9 too.
I've pondered whether we need a change record for this and I think it might be helpful for module developers to be informed about this and reminded that keeping tests up-to-date with the latest version of PHPUnit supported by core means they'll have less surprises.
Comment #25
mondrakeAdded draft CR https://www.drupal.org/node/3217694
Comment #26
alexpottCommitted 5be5048 and pushed to 9.3.x. Thanks!
Comment #28
alexpottDiscussed with @catch and @xjm, who agreed that we could consider this for 9.2 because:
Can someone create an MR or patch against 9.2.x so we can get a test run? Thanks.
Comment #30
mondrakeHere's a patch, the MR diff applied smoothly.
Comment #31
longwavePatch is identical, so RTBC if tests pass.
Comment #32
mondrakeComment #34
catchCommitted/pushed to 9.2.x, thanks!