Small follow-up to #2005166: Create simple file listing under admin/content/file related to meta #500866: [META] remove t() from assert message.

A couple of these snuck in with this recent commit, let's remove them :)

The offending assertions are in \Drupal\file\Tests\FileListingTest.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim-e’s picture

tim-e’s picture

Assigned: Unassigned » tim-e
Status: Active » Needs review

Changing status

star-szr’s picture

Status: Needs review » Needs work

Thanks for working on this @tim-e! Good start, needs to be dialed back a bit though. The top section of #500866: [META] remove t() from assert message gives a rough guide to what we are changing.

+++ b/core/modules/file/lib/Drupal/file/Tests/FileListingTest.php
@@ -81,7 +81,7 @@ function testFileListingPages() {
-      $this->drupalPost(NULL, $edit, t('Save'));
+      $this->drupalPost(NULL, $edit, 'Save');

@@ -111,7 +111,7 @@ function testFileListingPages() {
-    $result = $this->xpath("//td[contains(@class, 'views-field-status') and contains(text(), :value)]", array(':value' => t('Temporary')));
...
+    $result = $this->xpath("//td[contains(@class, 'views-field-status') and contains(text(), :value)]", array(':value' => 'Temporary'));

Only assertion messages should have t() removed, so $this->assert____ calls. Both of these lines need to be rolled back.

+++ b/core/modules/file/lib/Drupal/file/Tests/FileListingTest.php
@@ -94,7 +94,7 @@ function testFileListingPages() {
-    $this->assertFalse(preg_match('/views-field-status priority-low\">\s*' . t('Temporary') . '/', $this->drupalGetContent()), t('All files are stored as permanent.'));
+    $this->assertFalse(preg_match('/views-field-status priority-low\">\s*' . 'Temporary' . '/', $this->drupalGetContent()), 'All files are stored as permanent.');

This one changes too much - the 'Temporary' string should still be wrapped in t().

+++ b/core/modules/file/lib/Drupal/file/Tests/FileListingTest.php
@@ -111,7 +111,7 @@ function testFileListingPages() {
-    $this->assertEqual(1, count($result), t('Unused file marked as temporary.'));
...
+    $this->assertEqual(1, count($result), 'Unused file marked as temporary.');

This one looks great because only the assertion message (last parameter) is changing.

tim-e’s picture

Status: Needs work » Needs review
FileSize
1.37 KB

Ok, I think Ive got it now. Only remove t() from assert message param.

Here's another pass at it.

larowlan’s picture

Status: Needs review » Reviewed & tested by the community

Looking good

star-szr’s picture

Agreed :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for catching these!

Committed and pushed to 8.x.

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