Problem/Motivation

AssertLegacyTrait::assertText() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->responseContains() or $this->assertSession()->pageTextContains() instead. See https://www.drupal.org/node/3129738

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3139404

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mondrake created an issue. See original summary.

jungle’s picture

Issue tags: +Deprecated assertions

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

munish.kumar’s picture

Assigned: Unassigned » munish.kumar
munish.kumar’s picture

Assigned: munish.kumar » Unassigned
Status: Active » Needs review
StatusFileSize
new1.03 MB
munish.kumar’s picture

StatusFileSize
new1.01 MB

Fixed linting issues.

Status: Needs review » Needs work

The last submitted patch, 5: 3139404-5.patch, failed testing. View results

pavnish’s picture

Assigned: Unassigned » pavnish
pavnish’s picture

Status: Needs work » Needs review
StatusFileSize
new1.01 MB

This is the testing patch only

Status: Needs review » Needs work

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

pavnish’s picture

mondrake’s picture

Thanks for working on this

This is a very big elephant, that will be probably have to be sliced for eating. IMHO we should wait for most of the #3027952: [Plan] Remove the usage of deprecated methods in tests sub-issues to be done before, maybe also for #2549805: [Meta] Remove all usage of FormattableMarkup in tests apart from explicit tests of that API. Then find ways to break this down into smaller patches (e.g. separate assertText from assertNoText, split base classes+traits from concrete test, etc).

mondrake’s picture

Title: Replace usages of AssertLegacyTrait::assert(No)Text, that is deprecated » [PP-2] Replace usages of AssertLegacyTrait::assert(No)Text, that is deprecated
Status: Needs work » Postponed

Similarly to #3139409: Replace usages of AssertLegacyTrait::assertRaw, that is deprecated, I would suggest splitting this issue in 3:

1) a first issue to remove the $message parameter from calls to assertText and assertNoText. This is already redundant as in the legacy trait these methods are defined as protected function assertText($text). Suggest following #3131946: [policy] Remove PHPUnit assertion messages when possible, and standardize remaining messages for guidelines when a $message should be converted to an inline comment.
2) a second issue to remove the calls to t() from calls to assertText, unless we are testing the translation layer.
3) finally this one that at that point will be simply a find and replace exercise.

pavnish’s picture

Assigned: pavnish » Unassigned

It's postponed so i am unassign this issue

mondrake’s picture

Title: [PP-2] Replace usages of AssertLegacyTrait::assert(No)Text, that is deprecated » [PP-3] Replace usages of AssertLegacyTrait::assert(No)Text, that is deprecated
Issue summary: View changes
Related issues: +#3145418: [November 9, 2020] Remove uses of t() in assertText() calls, +#3166349: Remove uses of t() in assertNoText(), +#3159788: AssertLegacyTrait::assert(No)Text() in functional tests still have a message passed in
mondrake’s picture

Title: [PP-3] Replace usages of AssertLegacyTrait::assert(No)Text, that is deprecated » [PP-2] Replace usages of AssertLegacyTrait::assert(No)Text, that is deprecated
Issue summary: View changes

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

mondrake’s picture

Title: [PP-2] Replace usages of AssertLegacyTrait::assert(No)Text, that is deprecated » [PP-1] Replace usages of AssertLegacyTrait::assert(No)Text, that is deprecated
Issue summary: View changes
mondrake’s picture

Title: [PP-1] Replace usages of AssertLegacyTrait::assert(No)Text, that is deprecated » [PP-1] Replace usages of AssertLegacyTrait::assertText, that is deprecated

I am splitting this in two, one for assertText and one for assertNoText, since the conversions are not as straightforward as it may seem.

mondrake’s picture

Title: [PP-1] Replace usages of AssertLegacyTrait::assertText, that is deprecated » Replace usages of AssertLegacyTrait::assertText, that is deprecated
Assigned: Unassigned » mondrake
Issue summary: View changes
Status: Postponed » Active
mondrake’s picture

Status: Active » Needs review
mondrake’s picture

Assigned: mondrake » Unassigned

Green! Ready for review.

daffie’s picture

@mondrake: When I download the MR as a patchfile it is 850 KB. Should we schedule this issue in the same way as #3186661: Remove usage of drupalPostForm?

mondrake’s picture

@daffie re. #24 obviously, this is a big patch. I'd leave that decision to committers. Personally, I respect the need to minimise the impact of large changes. OTOH, I see that having only a 10 days window every 6 months to get those done, the risk is that it will take a long time to do them especially if you need to start doing a big one only after another big one is committed. For example, these remaining Simpletest deprecations

      // Simpletest's legacy assertion methods.
      'UiHelperTrait::drupalPostForm() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use $this->submitForm() instead. See https://www.drupal.org/node/3168858',
      'AssertLegacyTrait::assertEqual() is deprecated in drupal:8.0.0 and is removed from drupal:10.0.0. Use $this->assertEquals() instead. See https://www.drupal.org/node/3129738',
      'AssertLegacyTrait::assertNotEqual() is deprecated in drupal:8.0.0 and is removed from drupal:10.0.0. Use $this->assertNotEquals() instead. See https://www.drupal.org/node/3129738',
      'AssertLegacyTrait::assertIdentical() is deprecated in drupal:8.0.0 and is removed from drupal:10.0.0. Use $this->assertSame() instead. See https://www.drupal.org/node/3129738',
      'AssertLegacyTrait::assertText() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->responseContains() or $this->assertSession()->pageTextContains() instead. See https://www.drupal.org/node/3129738',
      'AssertLegacyTrait::assertNoText() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->responseNotContains() or $this->assertSession()->pageTextNotContains() instead. See https://www.drupal.org/node/3129738',
      'AssertLegacyTrait::assertRaw() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->responseContains() instead. See https://www.drupal.org/node/3129738',
      'AssertLegacyTrait::assertNoRaw() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->responseNotContains() instead. See https://www.drupal.org/node/3129738',

are all pretty large, interdependent one to the other, and non-trivial (it's not just renaming the method, we need to check order of arguments, select the appropriate method in some cases, etc...).

daffie’s picture

@mondrake: If you have a good idea how we can get this done in another way, then please say so. Because I do not.

mondrake’s picture

Same here, @daffie :)

daffie’s picture

Title: Replace usages of AssertLegacyTrait::assertText, that is deprecated » [May 25, 2021] Replace usages of AssertLegacyTrait::assertText, that is deprecated
mondrake’s picture

Rerolled

mondrake’s picture

Status: Needs review » Postponed
longwave’s picture

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mondrake’s picture

Version: 9.3.x-dev » 9.2.x-dev

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

Version: 9.3.x-dev » 9.2.x-dev
Status: Postponed » Needs review
mondrake’s picture

Rerolled to solve conflicts introduced by commit of #3131281: Replace assertEqual() with assertEquals().

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Agreed with @mondrake in Slack that we both think each other's commits here are good, so we are joint-RTBC-ing this patch together :)

mondrake’s picture

alexpott’s picture

Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed and pushed 3872f1a8ed to 9.3.x and 01d96560b5 to 9.2.x. Thanks!

Let's backport this without the changes to skipped deprecations.

  • alexpott committed 3872f1a on 9.3.x
    Issue #3139404 by mondrake, munish.kumar, pavnish, longwave: [May 25,...

  • alexpott committed 01d9656 on 9.2.x
    Issue #3139404 by mondrake, munish.kumar, pavnish, longwave: [May 25,...

longwave’s picture

Status: Patch (to be ported) » Needs review
mondrake’s picture

Title: [May 25, 2021] Replace usages of AssertLegacyTrait::assertText, that is deprecated » [backport to 9.1] Replace usages of AssertLegacyTrait::assertText, that is deprecated
daffie’s picture

Assigned: Unassigned » daffie

I am reviewing

alexpott’s picture

Status: Needs review » Needs work

This one needs a re-roll - the other disruptive patches have landed in 9.1.x

daffie’s picture

Assigned: daffie » Unassigned

I shall wait for the reroll.

longwave’s picture

Status: Needs work » Needs review

Merged in 9.1.x and fixed conflicts.

mondrake’s picture

Status: Needs review » Reviewed & tested by the community

RTBC; as far as I can see, this is only removing lines with assertText(...) and adding lines with pageTextContains(...) or responseContains(...).

Working on #3191935: Replace usages of AssertLegacyTrait::assertNoText, which is deprecated I now realize that this conversion may be impacted by #3175718: Random fails due to drupal-settings-json being counted as page text in the sense that if an expected text is found in the drupal-settings-json but not in the user visible page, still the assertion would pass.

alexpott’s picture

Title: [backport to 9.1] Replace usages of AssertLegacyTrait::assertText, that is deprecated » Replace usages of AssertLegacyTrait::assertText, that is deprecated
Status: Reviewed & tested by the community » Fixed

Committed 5a6777a and pushed to 9.1.x. Thanks!

  • alexpott committed 5a6777a on 9.1.x
    Issue #3139404 by mondrake, longwave, munish.kumar, pavnish: Replace...

Status: Fixed » Closed (fixed)

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