Problem/Motivation

There are a lot of deprecations, according to PHPStorm we are talking about ~1500 detected deprecations, mostly test asserts, which are fairly easy to resolve but it is quite lot of work.

To keep it reviewable, we need to split it up. This is enough that we actually might want to do it per method, at least for the most common ones. however, this means that we should _not_ work on multiple of those issues at the same time, due to high conflict potential

Recommended steps and groups:

Exclusive steps, only start the next if the previous one is done:

  1. setUp void return, $modules definition and update all .info.yml files to require Drupal 9.2
    1. setUp void return - seems to have been fixed in #3221963: Fix Drupal 9 test fails
    2. #3270461: Tests $modules property should be declared protected
    3. #3270501: Bump minimum required Drupal version to 9.2
  2. assertEqual() (460 usages, argument order needs to be reviewed
    1. #3271301: assertEqual() and assertNotEqual() is deprecated and will be removed from Drupal 10
  3. drupalPostForm() (300 usages)
    1. #3271901: drupalPostForm() is deprecated and will be removed in Drupal 10
  4. assertText() (350 usages, but should be a single global search & replace)
    1. #3272563: assertText() and assertNoText() is deprecated and will be removed in Drupal 10
  5. Remaining assert methods (after all the above are done, most should be search & replace too)
    1. #3273183: assertRaw and assertNoRaw is deprecated and will be removed in Drupal 10
    2. #3273185: assertResponse is deprecated and will be removed in Drupal 10
    3. #3273187: assertLink and assertNoLink is deprecated and will be removed in Drupal 10
    4. #3273366: assertUrl is deprecated and will be removed in Drupal 10
    5. #3273368: assertFieldByName and assertNoFieldByName is deprecated and will be removed in Drupal 10
    6. #3273371: assertTitle is deprecated and will be removed in Drupal 10
    7. #3273166: Refactor remaining deprecated assert methods
  6. drupal_get_path() + file_create_url(), often used together, so makes sense to group. Requires Drupal 9.3.
    1. #3274495: Drupal 10 compatibility
  7. #3274484: Add explicit accessCheck(TRUE/FALSE) to content entity queries

Then we can use this issue to mop up the reminders and test against D10. For that, our test dependencies will need to either be removed in a first step or be compatible too: metatag, paragraphs, pathauto, webform

If you want to help work on this, pick one of the topics, create an issue if there is none yet and if there is no other active one yet. If there is an active issue, check if you can help there.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

Berdir created an issue. See original summary.

marcusml’s picture

marcusml commented
Issue summary: View changes

Updated the issue summary with formatting and link to related issues.

setUp void return

looks like it got fixed by #3221963: Fix Drupal 9 test fails.

$modules definition

Created #3270461: Tests $modules property should be declared protected as child issue.

marcusml’s picture

marcusml commented
Issue summary: View changes

Adding another child issue for raising minimum required Drupal version to 9.2.

marcusml’s picture

marcusml commented
Issue summary: View changes

Phase 1 completed! I just created a new issue to replace assertEqual and assertNotEqual which I'll be submitting an MR for soon.

marcusml’s picture

marcusml commented
Issue summary: View changes

#3271301: assertEqual() and assertNotEqual() is deprecated and will be removed from Drupal 10 is done.

Created a new issue for drupalPostForm #3271901: drupalPostForm() is deprecated and will be removed in Drupal 10

marcusml’s picture

marcusml commented
Issue summary: View changes

#3271901: drupalPostForm() is deprecated and will be removed in Drupal 10 is done. Continuing with #3272563: assertText() and assertNoText() is deprecated and will be removed in Drupal 10.

marcusml’s picture

marcusml commented
Issue summary: View changes

#3272563: assertText() and assertNoText() is deprecated and will be removed in Drupal 10 is done. Created #3273166: Refactor remaining deprecated assert methods.

marcusml’s picture

marcusml commented
Issue summary: View changes

I split the remainder issue up a bit and linked the new issues.

marcusml’s picture

marcusml commented
Issue summary: View changes

Adding more child issues for deprecated assertions.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Another topic that makes sense in a separate issue: entity query access. there are ~35 calls to entityQuery() and getQuery() calls, all will need an explicit accessCheck(TRUE/FALSE).

marcusml’s picture

marcusml commented
Issue summary: View changes

@Berdir is it save to assume that all entityQuery() and getQuery() calls (without an accessCheck) should use accessCheck(TRUE)? That is/was the default behaviour before the accessCheck became mandatory right? Or might there be a reason to look more closely into each entity query?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

That's a fairly safe assumption, I might think about that while reviewing the patch, but it's at least not a regression. Yes, the default was TRUE before.

marcusml’s picture

marcusml commented
Issue summary: View changes

Sounds good.! Thank you! I added the entity query thing to the list of tasks in the issue summary.

marcusml’s picture

marcusml commented
Issue summary: View changes

Linked a new issue about raising minimum required core version to 9.3.

marcusml’s picture

marcusml commented
Issue summary: View changes

Added #3274484: Add explicit accessCheck(TRUE/FALSE) to content entity queries to issue summary.

marcusml’s picture

marcusml commented
Issue summary: View changes

Linked to new issues: #3274495: Drupal 10 compatibility and #3274496: drupal_get_path() is deprecated and will be removed in Drupal 10

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Re #16: See my comment about combining the version change in that issue. Not quite sure yet about that. However, I would recommend to _not_ split drupal_get_path() and file_create_url(). they are very often used together, on the same line. If we split it, we will have to touch all those lines twice. I'd suggest we combine it together.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

In fact, maybe try setting the drupal core version to ^9.3 || ^10 in that same issue, then we can do test runs against D10 to already to identify other issues. Might end up doing the remaining changes all in a single issue or we can split out specific changes if they do not require 9.3. To actually run the test, you'll likely need to temporarily remove the require-dev definitions for modules that are not yet D10 compatible, as until they all are, the composer require will fail. Possibly as a separate patch or MR, see how I did that for #3248874: Drupal 10 compatibility.

marcusml’s picture

marcusml commented
Issue summary: View changes

Thanks for the guidance @berdir! I was struggling a bit on how to organise it.. I'll close the 9.3 and drupal_get_path issues and focus on the one active issue instead.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Fixed

All known remaining deprecations have been dealt with in #3274495: Drupal 10 compatibility

Status: Fixed » Closed (fixed)

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

google01’s picture

google01 commented

In the latest versions, functions continue to appear in the code without explicit accessCheck(TRUE/FALSE) to content entity queries.

For example in the function "tmgmt_entity_delete" in the code file "tmgmt.module". This case is really worrying because it causes errors in the removal of any content, leaving Drupal 10 websites completely inoperative.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

There is no such function this module. You might be using patches that have not been updated.

google01’s picture

google01 commented

Effectively the patch https://www.drupal.org/project/tmgmt/issues/2998143 "Delete job item when corresponding entity is being deleted" is adding code without explicit accessCheck(TRUE/FALSE) to content entity queries.

Thank you very much for the quick response!