AssertLegacyTrait::assertPattern() calls in functional tests still have a message passed in

Patch no longer applies.

Patch rerolled for 9.1.

The latest patch returns with 4 coding standards messages. See: https://www.drupal.org/pift-ci-job/1710147.

Hi @daffie,

solved returns with 4 coding standards messages, kindly check.

A quick review of the interdiff file:

+++ b/core/modules/user/tests/src/Functional/UserBlocksTest.php
@@ -89,7 +92,7 @@ public function testUserLoginBlock() {
-    $this->assertPattern('!<title.*?' . t('Compose tips') . '.*?</title>!');
+    $this->assertPattern('!<title.*Compose tips.*?</title>!');

This change is missing a comma character.

@daffie #13 has been changed.
Please review this patch.

The latest patch returns a coding standard violation. See: https://www.drupal.org/pift-ci-job/1711330

Hi @pavnish, You latest patch contains a coding standard issue, Fixed this issue and uploaded interdiff for the same.

Ignore the previous patch, Here is the updated patch which addresses #15 issues.

Patch looks clean and didn't find any other occurrence of assertPattern() with a message.
So moving it to RTBC.

Reroll is done correctly.
Back to RTBC.

I read over all the removed messages in the patch, and I think that the inline comments are being added in all the cases where existing comments don't already provide sufficient information. Nice work!

1. I did a broad search for places that might have the second parameter with a non-regexy grep:

   [ayrton:maintainer | Wed 14:54:49] $ grep -r "assertPattern" * | grep "," | grep -v "vendor" | grep -v "node_modules"
   core/tests/Drupal/KernelTests/AssertContentTrait.php:  protected function assertPattern($pattern, $message = '', $group = 'Other') {
   core/modules/system/tests/src/Functional/Menu/LocalActionTest.php:      $this->assertPattern('@<a [^>]*class="[^"]*button-action[^"]*"[^>]*>' . preg_quote($title, '@') . '</@');
   core/modules/system/tests/src/Functional/Menu/LocalTasksTest.php:    return $this->assertPattern('@<a [^>]*>' . preg_quote($title, '@') . '</a>@');
   core/modules/system/tests/src/Kernel/Common/AddFeedTest.php:      $this->assertPattern($this->urlToRSSLinkPattern($feed_info['url'], $feed_info['title']));
   core/modules/responsive_image/tests/src/Functional/ResponsiveImageFieldDisplayTest.php:    $this->assertPattern('/<a(.*?)href="' . preg_quote(file_url_transform_relative(file_create_url($image_uri)), '/') . '"(.*?)>\s*<picture/');
   core/modules/responsive_image/tests/src/Functional/ResponsiveImageFieldDisplayTest.php:      $this->assertPattern('/media="\(min-width: 560px\)".+?sizes="\(min-width: 700px\) 700px, 100vw"/');
   core/modules/responsive_image/tests/src/Functional/ResponsiveImageFieldDisplayTest.php:    $this->assertPattern('/srcset="' . preg_quote(file_url_transform_relative($thumbnail_style->buildUrl($image_uri)), '/') . ' 1x".+?media="\(min-width: 0px\)"/');
   core/modules/responsive_image/tests/src/Functional/ResponsiveImageFieldDisplayTest.php:        $this->assertPattern('/<a(.*?)href="' . preg_quote(file_url_transform_relative(file_create_url($image_uri)), '/') . '"(.*?)>\s*<picture/');
   core/modules/responsive_image/tests/src/Functional/ResponsiveImageFieldDisplayTest.php:        $this->assertPattern('/<a(.*?)href="' . preg_quote($node->toUrl()->toString(), '/') . '"(.*?)>\s*<picture/');

   Those are all false positives, except the method on AssertContentTrait itself. So, this isn't like the other issue where the message wasn't supported at all, is it?

   Is the message not being printed? Should we be deprecating passing a message to the method?

2. +++ b/core/modules/taxonomy/tests/src/Functional/TermLanguageTest.php
   @@ -140,9 +140,9 @@ public function testTermTranslatedOnOverviewPage() {
   -    // Overview page in the other language shows the translated term
   +    // Overview page in the other language shows the translated term.
   

   Technically out of scope, but OK. :)

3. +++ b/core/modules/user/tests/src/Functional/UserBlocksTest.php
   @@ -32,6 +32,9 @@ class UserBlocksTest extends BrowserTestBase {
   +  /**
   +   * {@inheritdoc}
   +   */
   

   Hmm, also out of scope, and less close to the issue scope.

I'm mostly interested in more information for point 1, but let's fix 2 and 3 while we're at it. Thanks!

reverted point 2 and 3.

Opened #3162193: Modernize AssertContentTrait.

Patch needs a re-roll.

Added a reroll of patch #24.

Committed and pushed 402567d128 to 9.1.x and 0c53f5b191 to 9.0.x and 2c1c4e601e to 8.9.x. Thanks!

Backported to 8.9.x to keep tests in-sync.

+++ b/core/modules/system/tests/src/Kernel/Common/AddFeedTest.php
@@ -2,7 +2,6 @@
-use Drupal\Component\Render\FormattableMarkup;
 use Drupal\Core\Url;
 use Drupal\KernelTests\KernelTestBase;
 
@@ -69,7 +68,7 @@ public function testBasicFeedAddNoTitle() {

@@ -69,7 +68,7 @@ public function testBasicFeedAddNoTitle() {
     $this->setRawContent($response->getContent());
     // Assert that the content contains the RSS links we specified.
     foreach ($urls as $description => $feed_info) {
-      $this->assertPattern($this->urlToRSSLinkPattern($feed_info['url'], $feed_info['title']), new FormattableMarkup('Found correct feed header for %description', ['%description' => $description]));
+      $this->assertPattern($this->urlToRSSLinkPattern($feed_info['url'], $feed_info['title']));
     }
   }
 

This change is tricky. This is a kernel test. It's using \Drupal\KernelTests\AssertContentTrait::assertPattern() which still supports $message. That said the default message added by AssertContentTrait::assertPattern() is okay so let's proceed.