Problem/Motivation

#2870194: Ensure that process-isolated tests can use Symfony's PHPunit bridge to catch usages of deprecated code added the ability to test deprecated code. In order to get that done we add to add \Drupal\Tests\Listeners\DeprecationListenerTrait::getSkippedDeprecations() because core is exercising lots of deprecated code. This is all technical debt that we need to clear up. This issue is going to coordinate all the issues that need to be filed in order to stop using deprecated code.

DONE:

Proposed resolution

Ensure each skipped deprecation has an issue to work on.

Remaining tasks

Create all the necessary issues

User interface changes

None

API changes

None

Data model changes

None

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes
alexpott’s picture

alexpott’s picture

Issue summary: View changes
Berdir’s picture

@alexpott+++++ :)

> The revision_user revision metadata key is not set.

We won't be able to get rid of these due to the upgrade path and we actually explicitly don't have them on one test class to test the BC layer as we broke it at some point in 8.5.x. Not sure how to deal with issues like that.

Mile23’s picture

alexpott’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes
alexpott’s picture

In order to be sure we're not triggering deprecated code we need to move back to strict checking - see #2961691: Change SYMFONY_DEPRECATIONS_HELPER back to strict

removing the drupal_set_message() issues from related issues. Duplicating the issue summary in the related issues is not going to be that useful.

alexpott’s picture

Issue summary: View changes
Berdir’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes
rteijeiro’s picture

Issue summary: View changes
marcel66’s picture

Issue summary: View changes
marcel66’s picture

Issue summary: View changes
mihai11’s picture

Issue summary: View changes
jmikii’s picture

Issue summary: View changes
szloredan’s picture

Issue summary: View changes
marcel66’s picture

Issue summary: View changes
jmikii’s picture

Issue summary: View changes
mihai11’s picture

Issue summary: View changes
mihai11’s picture

Issue summary: View changes
szloredan’s picture

Issue summary: View changes
jmikii’s picture

Issue summary: View changes
mihai11’s picture

Issue summary: View changes
szloredan’s picture

Issue summary: View changes
jmikii’s picture

Issue summary: View changes
marcel66’s picture

Issue summary: View changes
mihai11’s picture

Issue summary: View changes
marcel66’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes

Moving some things to the done section. Also there's no need to related the issues to this plan since we have the list in the issue summary.

Another thing is that we have plenty of deprecations that do not trigger a silence error because they we added before the policy. We need to fix that too. They are harder to identify. Plenty of stuff to be getting on will :)

alexpott’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes
Mile23’s picture

We have a problem with some issues such as #2970017-9: Fix "Drupal\system\Tests\Menu\AssertBreadcrumbTrait is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. Instead, use \Drupal\Tests\system\Functional\Menu\AssertBreadcrumbTrait"

We can't fix the deprecation because it's triggered during discovery, and some tests cause another discovery.

We could limit our discovery to include only *Test.php files, but this has BC problems. #2296635: Evaluate performance impact of limiting test discovery to *Test.php filename suffix

Another solution might be to remove @trigger_error() from deprecated test traits and base classes which are touched by TestDiscovery in these instances. So for instance, in #2970017: Fix "Drupal\system\Tests\Menu\AssertBreadcrumbTrait is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. Instead, use \Drupal\Tests\system\Functional\Menu\AssertBreadcrumbTrait" we'd remove @trigger_error() from Drupal\system\Tests\Menu\AssertBreadcrumbTrait and also remove that error from the whitelist. At some point in the future, if we scan only for *test.php files, we could change back to using @trigger_error().

We'll also eventually officially deprecate WebTestBase which will handle this use-case anyway. #2847678: Deprecate WebTestBase and its required classes in favour of BrowserTestBase

I'm offering this solution here because there are a number of trait errors we're dealing with in child issues.

Thoughts?

alexpott’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes
daniel.bosen’s picture

Issue tags: +DCScot18
mairi’s picture

Issue summary: View changes
mairi’s picture

Issue summary: View changes
alexpott’s picture

Title: [meta] Core should not trigger deprecated code » [meta] Core should not trigger deprecated code except in tests and during updates

Adding a clarification to the issue title. We are allowed to trigger deprecated code in tests marked with @group legacy and in whilst running update functions.

Mile23’s picture

Added this issue to CR: https://www.drupal.org/node/2937545

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

alexpott’s picture

alexpott’s picture

Issue summary: View changes

Updating todo list.
Removing the done list - not that helpful - we have revisions.

jibran’s picture

Is there a way for contrib modules to keep up with these removed deprecated warnings?

If Drupal CI starts failings the daily test runs of a module then that would be great. It would not be ideal to start failing all contrib test runs but to allow modules to opt in would be great. Is there a way to do that?

Mile23’s picture

You can add or modify your project's drupalci.yml file and say suppress-deprecations: true in the run_tests sections.

Docs: https://www.drupal.org/drupalorg/docs/drupal-ci/customizing-drupalci-tes...

It'd be ideal if we had different build target types in drupalci, but we don't yet. #2951375: Allow for build targets

Mile23’s picture

Mile23’s picture

Issue summary: View changes
Mile23’s picture

tim.plunkett’s picture

Berdir’s picture

Issue summary: View changes
Mile23’s picture

mikelutz’s picture

Issue summary: View changes
mikelutz’s picture

Issue summary: View changes
mikelutz’s picture

Issue summary: View changes

.

Gábor Hojtsy’s picture

voleger’s picture

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Berdir’s picture

Issue summary: View changes
Berdir’s picture

Issue summary: View changes
Berdir’s picture

Issue summary: View changes

Splitting the issue summary into todo and done, as it is very hard to scan.

Berdir’s picture

Issue summary: View changes

And removing one of the assert messages as that is no longer deprecated for 9.x

alexpott’s picture

Issue summary: View changes
Berdir’s picture

Issue summary: View changes
Berdir’s picture

Issue summary: View changes
Berdir’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Berdir’s picture

#3084983: Move all the code related to path aliases to a new (required) "path_alias" module added a bunch of new entries to this list, so we should open a D8 issue to clean that up as much as possible.

Beside those explicitly skipped deprecations, it also added @deprecated to various interfaces that aren't enforced with @trigger_error() because the new interfaces are actually just extending from those, that makes sense for BC, but it's going to be tricky to detect/enforce it.

plach’s picture

jonathan1055’s picture

DamienMcKenna’s picture

Tagging as a requirement for Drupal 9.0-beta1.

catch’s picture

Looks like we need an issue to remove these:

   'The "core/jquery.ui.checkboxradio" asset library is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. See https://www.drupal.org/node/3067969',
    'The "core/jquery.ui.controlgroup" asset library is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. See https://www.drupal.org/node/3067969',
catch’s picture

Issue summary: View changes

Found the issue with some help from longwave: #3098489: Remove deprecated jQuery UI library definitions

catch’s picture

Priority: Normal » Critical
Issue summary: View changes

Explicitly bumping this to critical, these are some of the stickier blockers to getting to zero deprecated code.

Gábor Hojtsy’s picture

Berdir’s picture

Issue summary: View changes
Gábor Hojtsy’s picture

catch’s picture

Issue summary: View changes

Removing #3020296: Remove Symfony's classloader as it does not exist in Symfony 4 from the issue summary, because it is also covered in #3009213: [META] Update / reconsider PHP dependencies for Drupal 9 (in which it is the only remaining blocking issue) and independently mentioned in the issue summary of #3079680: [META] Requirements for tagging Drupal 9.0.0-beta1. So I think we can get away with counting it three times instead of five...

We can't actually empty this method due to Symfony 4 deprecations so it's a case of removing the two that were incomplete deprecations we did ourselves, both of which have in-progress issues.

catch’s picture

Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Issue summary: View changes
xjm’s picture

Crediting contributors to organizing the meta.

Status: Fixed » Closed (fixed)

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