Replace usages of deprecated AssertLegacyTrait::assert(No)Link()

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

@jungle The $this->assertSession() is not a part of use Drupal\KernelTests\KernelTestBase;
This is the part of use Drupal\Tests\BrowserTestBase;

Please suggest what we can do?

@mondrake thanks for suggesting me
I am reverting the kernel test changes
Thanks

Hi @mondrake
Changes has been done it would be a great help for me if you can review this patch.
Thanks
Pavnish

1. All occurrences of $this->assert(No)Link(XXX) have been replaced by $this->assertSession()->linkExists(XXX) except from Kernel tests,
to be specific from
core/modules/comment/tests/src/Kernel/Views/CommentUserNameTest.php and
core/modules/field/tests/src/Kernel/String/StringFormatterTest.php

core/tests/Drupal/Tests/Core/Utility/LinkGeneratorTest.php contain its own $this->assertLink() method.
This is also not changed.

2. The test has been added for deprecation testing.

3. The suppression of the deprecation message has been removed.

All code changes look good to me.
For me it is RTBC.

Oops, looks like it needs a reroll already. Thanks!

Rerolling patch

Fixed php lint errors.

The patch needs to be rerolled again.

Added re-roll of patch #17.

There a few thing that need to be done in the patch:

1. In the file core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php the 2 lines supressing the deprecationg messages for AssertLegacyTrait::assertLink() and AssertLegacyTrait::assertNoLink() need to be removed.
2. All occurrences of $this->assertLink() and $this->assertNoLink() need to be replaced. There a few of those in the patch, only there are a lot more of them.

All occurrences of $this->assertLink() and $this->assertNoLink() need to be replaced.

Careful - this applies to calls to AssertLegacyTrait::assertLink(), which are in functional tests. There is also AssertContentTrait::assertLink() which is used in kernel tests and which isn't marked as deprecated. Fortunately, kernel tests are all in namespaces/filepaths containing "Kernel" and can be excluded that way.

Replacing all occurrences in files whose paths don't contain "Kernel" of ->assertLink( with ->assertSession()->linkExists( and ->assertNoLink( with ->assertSession()->linkNotExists(, and removing the two suppressions.

RandomGeneratorTrait::randomStringValidate() has an inline comment that still mentions WebTestBase::assertLink(), but that class doesn't even exist anymore, and it's not clear to me that the problem it mentions also exists in WebAssert::linkExists(). Not sure what to do with that one.

Missed the LinkGeneratorTest special case that was mentioned further upthread. Those changes need to be reverted.

RandomGeneratorTrait::randomStringValidate() has an inline comment that still mentions WebTestBase::assertLink(), but that class doesn't even exist anymore, and it's not clear to me that the problem it mentions also exists in WebAssert::linkExists(). Not sure what to do with that one.

Created an issue for that one. See: #3144331: Update comment in Drupal\Tests\RandomGeneratorTrait::randomStringValidate().

Careful - this applies to calls to AssertLegacyTrait::assertLink(), which are in functional tests. There is also AssertContentTrait::assertLink() which is used in kernel tests and which isn't marked as deprecated. Fortunately, kernel tests are all in namespaces/filepaths containing "Kernel" and can be excluded that way.

@cburschka: You are right about that.

Most code changes are right, only there are a few that are wrong:

+++ b/core/modules/views_ui/tests/src/Functional/DefaultViewsTest.php
@@ -53,7 +53,7 @@ public function testDefaultViews() {
-    // $this->assertNoLink(t('Revert'));
+    // $this->assertSession()->linkExists(t('Revert'));

+++ b/core/modules/views_ui/tests/src/Functional/DisplayCRUDTest.php
@@ -47,8 +47,8 @@ public function testAddDisplay() {
-    $this->assertNoLink('Master*', 'Make sure the master display is not marked as changed.');
...
+    $this->assertSession()->linkExists('Master*', 'Make sure the master display is not marked as changed.');

+++ b/core/modules/views_ui/tests/src/Functional/DisplayPathTest.php
@@ -57,7 +57,7 @@ protected function doBasicPathUITest() {
-    $this->assertNoLink(t('View @display', ['@display' => 'page']), 'No view page link found on the page.');
+    $this->assertSession()->linkExists(t('View @display', ['@display' => 'page']), 'No view page link found on the page.');

+++ b/core/modules/views_ui/tests/src/Functional/SettingsTest.php
@@ -76,7 +76,7 @@ public function testEditUI() {
-    $this->assertNoLink(t('Master'));
+    $this->assertSession()->linkExists(t('Master'));

+++ b/core/modules/views_ui/tests/src/Functional/ViewEditTest.php
@@ -136,7 +136,7 @@ public function testEditFormLanguageOptions() {
-      $this->assertNoLink(t('Content language of view row'));
+      $this->assertSession()->linkExists(t('Content language of view row'));

@@ -153,12 +153,12 @@ public function testEditFormLanguageOptions() {
-        $this->assertNoLink(t('Content language of view row'));
+        $this->assertSession()->linkExists(t('Content language of view row'));

Oops, I messed up my replacement pattern there apparently :)

The deprecation message suppression for AssertLegacyTrait::assert(No)Link() have been removed.
All occurrences of $this->assert(No)Link() have been replaced by $this->assertSession()->link(Not)Exists().
All changes look good to me.
The testbot is happy.
For me it is RTBC.

Needs a reroll. Unfortunately.

Brushed up too close to a change in #3142752: AssertLegacyTrait::assert(No)Escaped() in functional tests still have a message passed in; merged.

Looks like a random failure, re-queued

The deprecation message suppression for AssertLegacyTrait::assert(No)Link() have been removed.
All occurrences of $this->assert(No)Link() have been replaced by $this->assertSession()->link(Not)Exists().
All changes look good to me.
For me it is RTBC.

Committed 0f9cecb and pushed to 9.1.x. Thanks!

Backported 12fb3d8 and pushed to 9.0.x, and 8130d88 and pushed to 8.9.x. Thanks!

I did this by ignoring these conflicts:
error: patch failed: core/modules/tracker/tests/src/Functional/TrackerTest.php:208
error: core/modules/tracker/tests/src/Functional/TrackerTest.php: patch does not apply
error: patch failed: core/modules/user/tests/src/Functional/UserPasswordResetTest.php:109
error: core/modules/user/tests/src/Functional/UserPasswordResetTest.php: patch does not apply
error: patch failed: core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php:185
error: core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php: patch does not apply

So we're probably still using these methods in 9.0.x and 8.9.x but the branches are largely the same.

We can now prepare a backport patch for core/modules/tracker/tests/src/Functional/TrackerTest.php and core/modules/user/tests/src/Functional/UserPasswordResetTest.php

The block of skipped AssertLegacyTrait deprecations in DeprecationListenerTrait (of which this patch removes two lines) seems to be entirely absent in 9.0.x.

It appears that in 9.0.x the AssertLegacyTrait methods are "fake"-deprecated (no @deprecated and no @trigger_error), so I guess that's why we don't need to listen for these deprecations.

  /**
   * Passes if a link with the specified label is found.
   *
   * An optional link index may be passed.
   *
   * @param string|\Drupal\Component\Render\MarkupInterface $label
   *   Text between the anchor tags.
   * @param int $index
   *   Link position counting from zero.
   *
   * Deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use
   *   $this->assertSession()->linkExists() instead.
   */
  protected function assertLink($label, $index = 0) {
    return $this->assertSession()->linkExists($label, $index);
  }

  /**
   * Passes if a link with the specified label is not found.
   *
   * @param string|\Drupal\Component\Render\MarkupInterface $label
   *   Text between the anchor tags.
   *
   * Deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use
   *   $this->assertSession()->linkNotExists() instead.
   */
  protected function assertNoLink($label) {
    return $this->assertSession()->linkNotExists($label);
  }

One thing I noticed was that a lot of these assertions have a $message argument which they didn't remove, even though the new method doesn't support it (see also #3143398: WebAssert methods do not have a $message argument like AssertLegacyTrait methods), so removing that may be a follow-up task.

Oops - I accidentally diffed against 9.1.x instead of 9.0.x and created a monster patch

Removing two assertTitle -> assertSession()->titleEquals changes that should not be in this patch.

Verified backport patch for core/modules/tracker/tests/src/Functional/TrackerTest.php and core/modules/user/tests/src/Functional/UserPasswordResetTest.php

All occurrences of $this->assert(No)Link() have been replaced by $this->assertSession()->link(Not)Exists() in these two files.
All changes look good to me.
For me it is RTBC.

In future can #40 be filed as a separate follow-up issue. Multiple commits on the same issue are not good AND we already have follow-up issues committed so this just confuses people.

Committed 82979c9 and pushed to 9.0.x and 8.9.x. Thanks!

Backported to 8.9.x as this is a test only fix.