Problem/Motivation
Whilst working on a project I want to be able to have tests trigger deprecation errors so that I can confirm that custom code is not using any deprecated core code. However, contrib code is likely to still be using deprecated APIs that trigger silenced deprecations.
Currently in order to do this the project patches the getSkippedDeprecations() in order to run deprecation tests and keep a record of our current technical debt (at this point it is all contribs technical debt).
Giving projects a better way to do this helps them prepare for new major version of Drupal.
Proposed resolution
Allow projects to set an environment variable that points to a file containing a list of addition deprecations to skip.
Projects can define a DRUPAL_DEPRECATION_SKIP_FILE that points to a file. The file contains a JSON encode array of deprecation messages to skip.
Remaining tasks
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
@todo
| Comment | File | Size | Author |
|---|---|---|---|
| #40 | 3143388-40.patch | 3.1 KB | heddn |
| #37 | 3143388-10.0.x-37.patch | 3.3 KB | alexpott |
| #37 | 3143388-9.4.x-37.patch | 3.05 KB | alexpott |
| #31 | 3143388-10.0.x-31.patch | 3.3 KB | alexpott |
| #31 | 3143388-9.4.x-31.patch | 3.01 KB | alexpott |
Comments
Comment #2
alexpottSomething like this.
Comment #3
alexpottComment #4
xjmWhy can't they just override the method?
Comment #5
larowlanWebtestBase is calling that method directly, without using the trait it seems. We'd need to sort that.
But I think that static method overridding in traits is a bit odd see https://3v4l.org/1GFNa
Comment #6
alexpottIt's not really possible. It's called statically and this method is part of the test infrastructure. It is not part of each test class. Symfony handles this situation a bit more elegantly because they can detect whether it's your deprecation or a vendor deprecation. So you can set SYMFONY_DEPRECATIONS_HELPER to different values to determine with deprecations cause fails. But with Drupal our contrib modules and core are not in vendor so it's more complicated. And also in this case I wouldn't want to ignore new deprecations triggered by core. I'd want to investigate a find out if it was our custom code or contrib that was causing the new deprecation to be triggered.
Comment #7
alexpottAlso overriding this on a per test basis is not what a project wants to do. A project wants to maintain a list of its technical debt and then it can say to a dev work on removing this deprecation from the list. So having this in a json file on the project level is helpful because all your technical debt is listed in one place.
Thinking about this some more. I'm not sure I like the env var. It's better to check for a dot file in project root. Easy to document and less fragile.
Comment #8
alexpottUgh... that's not going to be simple to get Drupal root in there. Env var it is.
Comment #9
alexpottHere's a Drupal 8 version since this patch is more useful there...
Comment #10
alexpottStill do + when I mean array_merge()... ho hum. Real world testing FTW.
Comment #11
alexpottI'm successfully using #10 on a project and now we can maintain a list of skipped deprecations in a file which is much easier to manage than a patch.
Comment #12
heddnComment #13
heddnNo interdiff as just a re-roll to apply on 9.2.x.
Comment #14
anmolgoyal74 commentedRe-Rolled for 8.9.x
Comment #16
alexpottReroll for 9.3.x / 9.4.x
Comment #18
paulocsAttaching the correct reroll.
Comment #19
alexpott@paulocs++ thanks
Comment #20
alexpottI think the patch in #18 is for 9.4.x only. Here's a 9.3.x patch for those that need it.
Comment #21
heddnI've been using this patch on several sites long enough, we should just do the right thing. LGTM.
Comment #22
paulocsBtw yes, patch #18 is for drupal 9.4.x branch only.
Comment #25
heddnComment #26
mondrakeSee also #3180042-35: Add baseline for deprecation testing and https://github.com/symfony/symfony/issues/45223 trying to get a solution from upstream.
Comment #27
quietone commentedThis still needs a change record and the 9.3.x patch is being retested which no longer applies.
Comment #28
heddnComment #29
heddnAdded a change notice.
Comment #30
edysmpWorks great!
Comment #31
alexpottThe test will trigger a PHP 8.1 deprecation and the patch does not apply to 10.0.x.
Patches attached fix all of this.
Comment #32
mondrakeSymfony 6.1 will provide upstream [PhpUnitBridge] Add option ignoreFile to configure a file that lists deprecation messages to ignore, #3272779: [Symfony 6.1] Replace deprecationListenerTrait with PHPUnitBridge ignoreFile will look at implementing that for Drupal.
Comment #37
alexpottRerolled for 9.4.x
Comment #40
heddnReroll for 9.5