Problem/Motivation

In #3145418-47: [November 9, 2020] Remove uses of t() in assertText() calls we found more cases of assertNoText called with an object argument.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#32 3176200-32.patch1.66 KBadityasingh
#29 3176200-29.patch813 bytesPooja Ganjage
#27 3176200-27.patch1.17 KBPooja Ganjage
#26 interdiff_24-26.txt1015 bytesadityasingh
#26 3176200-26.patch2.81 KBadityasingh
#24 interdiff_22-24.txt1.2 KBadityasingh
#24 3176200-24.patch1.66 KBadityasingh
#22 3176200-22.patch3.1 KBadityasingh
#19 3176200-19.patch6.27 KBPooja Ganjage
#15 interdiff-3176200-11-15.txt1.4 KBmsuthars
#15 deprecated-assertions-3176200-15.patch6.3 KBmsuthars
#14 interdiff-3176200-11-14.txt1.4 KBmsuthars
#14 deprecated-assertions-3176200-14.patch12.6 KBmsuthars
#12 3176200-12.patch6.26 KBPooja Ganjage
#11 interdiff-3176200-8-11.txt3.76 KBmsuthars
#11 deprecated-assertions-3176200-11.patch6.45 KBmsuthars
#8 interdiff-3176200-6-8.txt2.62 KBmsuthars
#8 deprecated-assertions-3176200-8.patch9.28 KBmsuthars
#6 interdiff_2-6.txt5.34 KBravi.shankar
#6 3176200-6.patch6.35 KBravi.shankar
#2 3176200-2.patch1.02 KBmondrake
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mondrake created an issue. See original summary.

mondrake’s picture

mondrake’s picture

Issue tags: +Deprecated assertions

Status: Needs review » Needs work

The last submitted patch, 2: 3176200-2.patch, failed testing. View results

ravi.shankar’s picture

Assigned: Unassigned » ravi.shankar
ravi.shankar’s picture

Assigned: ravi.shankar » Unassigned
FileSize
6.35 KB
5.34 KB

Given a start to this issue.

msuthars’s picture

Assigned: Unassigned » msuthars
msuthars’s picture

Assigned: msuthars » Unassigned
Status: Needs work » Needs review
FileSize
9.28 KB
2.62 KB

Fix the test case issue. Updated the patch.

mondrake’s picture

Status: Needs review » Needs work

Thank you - to clarify: the intent here is not to convert all the instances, just those that are reasonable. With this perspective:

  1. +++ b/core/modules/system/tests/src/Functional/Form/ValidationTest.php
    @@ -240,11 +240,11 @@ public function testCustomRequiredError() {
    -        $this->assertNoText($form[$key]['#required_error']);
    +        $this->assertNoText($form[$key]['#required_error']->render());
    ...
    -        $this->assertNoText($form[$key]['#form_test_required_error']);
    +        $this->assertNoText($form[$key]['#form_test_required_error']->render());
    

    This can be reverted.

  2. +++ b/core/modules/system/tests/src/Functional/Module/UninstallTest.php
    @@ -86,7 +86,7 @@ public function testUninstallPage() {
    -    $this->assertNoText(\Drupal::translation()->translate('Configuration deletions'), 'No configuration deletions listed on the module install confirmation page.');
    +    $this->assertNoText(\Drupal::translation()->translate('Configuration deletions')->render(), 'No configuration deletions listed on the module install confirmation page.');
    
    @@ -99,7 +99,7 @@ public function testUninstallPage() {
    -    $this->assertNoText(\Drupal::translation()->translate('Configuration updates'), 'No configuration updates listed on the module install confirmation page.');
    +    $this->assertNoText(\Drupal::translation()->translate('Configuration updates')->render(), 'No configuration updates listed on the module install confirmation page.');
    

    I think this can be just $this->assertNoText('Configuration deletions'); since it does not look like in this test the translation layer is involved at all.

  3. +++ b/core/modules/tracker/tests/src/Functional/TrackerNodeAccessTest.php
    @@ -59,7 +59,7 @@ public function testTrackerNodeAccess() {
    -      'title' => t('Private node test'),
    +      'title' => 'Private node test',
    

    This can be reverted, see #3145418-61: [November 9, 2020] Remove uses of t() in assertText() calls and later comments

  4. +++ b/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php
    @@ -113,6 +113,9 @@ protected function assertText($text) {
    +    if (is_object($text)) {
    +      @trigger_error('Passing MarkupInterface objects to AssertLegacyTrait::assertNoText() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Pass plain strings instead. See https://www.drupal.org/node/xxxxxxx', E_USER_DEPRECATED);
    +    }
    

    this can be removed now as it has served its purpose

msuthars’s picture

Assigned: Unassigned » msuthars
msuthars’s picture

Assigned: msuthars » Unassigned
Status: Needs work » Needs review
FileSize
6.45 KB
3.76 KB

Updated the patch as suggested in #9. Please review.

Pooja Ganjage’s picture

Hi,

As #9 comment in point 2 mentioned as to be $this->assertNoText('Configuration deletions'); statement.

I am creating updated patch as cover all points and removed what it points.

Please review the patch.

Thanks.

msuthars’s picture

@Pooja Ganjage The patch in #12 is not applying so I'm adding the updated patch.

msuthars’s picture

msuthars’s picture

mondrake’s picture

Status: Needs review » Reviewed & tested by the community

Thank you.

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

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll
Pooja Ganjage’s picture

Hi,

Creating a reroll patch for latest version of drupal 9.2.x-dev.

Please review the patch.

Thanks.

Pooja Ganjage’s picture

Status: Needs work » Needs review
longwave’s picture

Status: Needs review » Needs work

Needs another reroll :(

adityasingh’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
3.1 KB

Reroll for 9.2.x

mondrake’s picture

+++ b/core/modules/system/tests/src/Functional/Module/UninstallTest.php
@@ -86,7 +86,7 @@ public function testUninstallPage() {
     $edit = [];
     $edit['uninstall[module_test]'] = TRUE;
     $this->drupalPostForm('admin/modules/uninstall', $edit, t('Uninstall'));
-    $this->assertNoText('Configuration deletions', 'No configuration deletions listed on the module install confirmation page.');
+    $this->assertNoText('Configuration deletions');
     $this->assertText('Configuration updates', 'Configuration updates listed on the module install confirmation page.');
     $this->assertText($node_type->label());
     $this->drupalPostForm(NULL, [], t('Uninstall'));
@@ -99,7 +99,7 @@ public function testUninstallPage() {

@@ -99,7 +99,7 @@ public function testUninstallPage() {
     $edit['uninstall[node]'] = TRUE;
     $this->drupalPostForm('admin/modules/uninstall', $edit, t('Uninstall'));
     $this->assertText('Configuration deletions', 'Configuration deletions listed on the module install confirmation page.');
-    $this->assertNoText('Configuration updates', 'No configuration updates listed on the module install confirmation page.');
+    $this->assertNoText('Configuration updates');
 

These should be reverted as they are out of scope here at this point.

adityasingh’s picture

Status: Needs work » Needs review
FileSize
1.66 KB
1.2 KB

Updated patch as suggested in #23.

longwave’s picture

Status: Needs review » Needs work

While we're here let's fix this easy one:

core/modules/views_ui/tests/src/Functional/FieldUITest.php
35:    $this->assertNoText('Views test: Name [' . t('hidden') . ']');
adityasingh’s picture

Updated patch as suggested in #25.

Pooja Ganjage’s picture

mondrake’s picture

Pooja Ganjage’s picture

mondrake’s picture

Hi, we are missing the changes from #24 in the last patches - we need them as well :)

longwave’s picture

Sorry, I think I should spin off #25 to another issue as there are other similar cases, and we should go back to #24.

adityasingh’s picture

Status: Needs work » Needs review
FileSize
1.66 KB

Reuploading the patch #24. As discussed in #31. Please review.

Thanks...

mondrake’s picture

Status: Needs review » Reviewed & tested by the community

Thanks

  • catch committed e0b3795 on 9.2.x
    Issue #3176200 by msuthars, adityasingh, Pooja Ganjage, ravi.shankar,...

  • catch committed 193f744 on 9.1.x
    Issue #3176200 by msuthars, adityasingh, Pooja Ganjage, ravi.shankar,...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!

longwave’s picture

Status: Fixed » Closed (fixed)

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