Problem/Motivation

In #2973791: Fix deprecation messages related to deprecated comment Action plugins we added the legacy group to all update tests. We've also been converting tests to PHPUnit. It is likely that some deprecations no longer need to be skipped. Review the list and see what we can remove as in an ideal world none of these would be skipped.

Proposed resolution

Remove skipped deprecations that no longer need to be skipped. Not skipping deprecations helps contrib and custom keep up with the latest APIs.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#11 2991080-11.patch9.4 KBalexpott
#11 8-11.patch1.73 KBalexpott
#8 2991080-8.patch9.39 KBalexpott
#8 5-8-interdiff.txt1.95 KBalexpott
#5 2991080-5.patch9.16 KBalexpott
#2 2991080-2.patch11.93 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 2991080-2.patch11.93 KB

So let's remove them all and see...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Parent issue: » #2959269: [meta] Core should not trigger deprecated code except in tests and during updates

Status: Needs review » Needs work

The last submitted patch, 2: 2991080-2.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Try removing a few skipped deprecations because all update tests are now legacy » Remove skipped deprecations no longer triggered by tests
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new 2991080-5.patch9.16 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented 
+++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
@@ -150,17 +138,11 @@ public static function getSkippedDeprecations() {
-      'The Symfony\Component\ClassLoader\WinCacheClassLoader class is deprecated since Symfony 3.3 and will be removed in 4.0. Use `composer install --apcu-autoloader` instead.',

Hmmm this on is probably not right. We just don't test on windows. Will add a comment.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 5-8-interdiff.txt1.95 KB
new 2991080-8.patch9.39 KB
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

That's a nice patch :)

Reminded me of #2961861: Remove usage of WriteCheckSessionHandler, we should finish that one too, another 2 messages to get rid of..

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

The only one of the deprecations I had a question about was solved in #8, there's a typo in the comment but not setting it back to NW for that.

+++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
@@ -143,6 +143,10 @@ public static function getSkippedDeprecations() {
+      // is a Windows only deprecation. Only remove once we;ve removed core

/s/we;ve/we've/

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 8-11.patch1.73 KB
new 2991080-11.patch9.4 KB

Thanks for the reviews! Fixing the comment and making it less repetitive.

The last submitted patch, 11: 8-11.patch, failed testing. View results

  • catch committed cfeae61 on 8.7.x 
    Issue #2991080 by alexpott: Remove skipped deprecations no longer...

  • catch committed 30e9c00 on 8.6.x 
    Issue #2991080 by alexpott: Remove skipped deprecations no longer...
catch’s picture

catch
he/him
Primary language English
commented
Version: 8.7.x-dev » 8.6.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.7.x and cherry-picked to 8.6.x. Thanks!

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +8.6.0 release notes

This will cause contrib/custom tests to start failing if they exercise the deprecated stuff, so we should mention it in the release notes. It would probably have been better to have this in before beta1.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

contrib doesn't fail on deprecations by default, so this shouldn't cause anything to fail :)

xjm’s picture

xjm
she/her
Primary language English
commented

@Berdir Was that changed since last year?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Yes, don't remember when exactly, drupalci.yml has a "suppress-deprecations: false", which is false for core and by default for contrib is true.

So you can opt-in if you really want to, but by default you won't have fails because of it. I guess at some point with D9 getting closer, we'll switch the default for contrib as well..

xjm’s picture

xjm
she/her
Primary language English
commented

Cool, thanks @Berdir!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.