Problem/Motivation

Follow up to #3114617-40: Properly deprecate methods in Drupal\FunctionalTests\AssertLegacyTrait and Drupal\KernelTests\AssertLegacyTrait, while still keeping deprecations silenced. Change the link from the issue to the CR.

Proposed resolution

-   * @expectedDeprecation AssertLegacyTrait::getAllOptions() is deprecated in drupal:8.5.0 and is removed from drupal:10.0.0. Use $element->findAll('xpath', 'option') instead. See http://drupal.org/node/2735045
+   * @expectedDeprecation AssertLegacyTrait::getAllOptions() is deprecated in drupal:8.5.0 and is removed from drupal:10.0.0. Use $element->findAll('xpath', 'option') instead. See https://www.drupal.org/node/3129738

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#4 3137455-4.patch7.34 KBsja112
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sja112 created an issue. See original summary.

sja112’s picture

Issue summary: View changes
sja112’s picture

Issue summary: View changes
sja112’s picture

Status: Active » Needs review
FileSize
7.34 KB

Created the patch add/update the CR link in the deprecated code messages.

mondrake’s picture

Issue summary: View changes
mondrake’s picture

Issue summary: View changes
mondrake’s picture

Status: Needs review » Reviewed & tested by the community

Thanks

longwave’s picture

Title: Change the link in @trigger_error and other related palces from the issue to the CR » Change links in @trigger_error deprecations to point the change record

Improving the title a bit.

Is this the only cases where the message points to an issue instead of a CR? Can we check this automatically somehow?

mondrake’s picture

Title: Change links in @trigger_error deprecations to point the change record » AssertLegacyTrait - change links in @trigger_error deprecations to point the relevant change record

  • xjm committed e0c1781 on 9.1.x
    Issue #3137455 by sja112, mondrake, longwave: AssertLegacyTrait - change...

  • xjm committed e4f670b on 9.0.x
    Issue #3137455 by sja112, mondrake, longwave: AssertLegacyTrait - change...

  • xjm committed 75b35f0 on 8.9.x
    Issue #3137455 by sja112, mondrake, longwave: AssertLegacyTrait - change...
xjm’s picture

Version: 9.1.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: +Needs followup

Thanks for splitting this out. Saving credits for work from the other issue as well.

Reviewed locally with git diff --color-words and confirmed that this only changes node IDs (and in some cases, for the same URLs, adds the canonical https:// or www. There are three URLs replaced with https://www.drupal.org/node/3129738:

  1. #2735045: Convert StandardTest to BrowserTestBase by introducing a AssertLegacyTrait and new assertions on the WebAssert -- an issue, which should never have been linked in the first place
  2. https://www.drupal.org/node/2864262, a CR for a method covered in the above overall CR
  3. https://www.drupal.org/node/2864029, similarly a CR for a single method covered in the above

Confirmed that there are no remaining references to the above in core as well.

#8 is a good question. I think it's good to keep the scope here limited to this one CR, since it is relevant for so many deprecations, but it should be straightforward-ish to write a script that generates a list of CRs in deprecations in the codebase, curls down those URLs, and parses the source for what kind of node it is. (I'd put limits on such a script when testing it to not get on d.o's naughty list.) Tagging "Needs followup" to check for that. We should at least fix it in 9.1.x if not the older branches.

Committed to 9.1.x, and cherry-picked to 9.0.x and 8.9.x since these links are wrong there too, and it's not new deprecations, just correcting existing ones. I didn't backport it to 8.8.x in case there were specific checks against the URLs in the deprecation messages in contrib.

Thanks!

jungle’s picture

Status: Fixed » Needs work
Issue tags: +Deprecated assertions

Adding the "Deprecated assertions" tag to add this into the kanban board https://contribkanban.com/board/Deprecatedassertions

Setting back NW for Needs followup

longwave’s picture

Status: Fixed » Closed (fixed)

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