This is a sub-task of #500866: Simpletest: remove t() from assert message focused on the Entity sub-system.

In D8 per http://drupal.org/simpletest-tutorial-drupal7#t, best practice is to remove t() from assert messages in tests. When necessary, t() should be replaced with format_string().

This issue is to correct the Entity sub-system tests for the above. There are approximately 120 changes needed. This is a smaller patch than others to faciliate backport to D7. It will be a bit tricky to review because many of the message strings appear as the second to last parameter rather than as the last one.

CommentFileSizeAuthor
#10 entity-1797220-10.patch51.29 KBdcam
#6 1797220-6-t-entity.patch49.73 KBlars toomre
#1 t-assert-entity-1797220-1.patch51.18 KBlars toomre

Comments

lars toomre’s picture

lars toomre commented
Status: Active » Needs review
StatusFileSize
new t-assert-entity-1797220-1.patch51.18 KB

Here is an initial untested patch for this issue.

xjm’s picture

xjm
she/her
Primary language English
commented

It will be a bit tricky to review because many of the message strings appear as the second to last parameter rather than as the last one.

Yes, very glad you caught this. It was this particular irregularity that kept us from finishing these in July.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs review » Needs work
Issue tags: +Needs backport to D7

Thanks @Lars Toomre. Two things:

  1. +++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityFieldQueryTest.phpundefined
@@ -1049,7 +1049,7 @@ class EntityFieldQueryTest extends WebTestBase {
-    $this->assertTrue($pass, t("Can't query the universe."));
+    $this->assertTrue($pass, 'Can not query the universe.');

    Really too bad, don't you think? ;) But, if we're changing can't to cannot, then cannot needs to be one word. Can not has a different meaning. Personally, I'd forego this word change, leave the message as it is, and leave the double quotes. 

  2. +++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityTranslationFormTest.phpundefined
@@ -98,18 +98,18 @@ class EntityTranslationFormTest extends WebTestBase {
-    $this->assertTrue($field['translatable'], "Field body is translatable.");
+    $this->assertTrue($field['translatable'], 'Field body is translatable.');

+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityTranslationTest.phpundefined
@@ -89,7 +89,7 @@ class EntityTranslationTest extends WebTestBase {
-    $message = "An exception is thrown when trying to set a field with an invalid language";
+    $message = 'An exception is thrown when trying to set a field with an invalid language';

@@ -139,7 +139,7 @@ class EntityTranslationTest extends WebTestBase {
-    $message = "An exception is thrown when trying to set an invalid translation.";
+    $message = 'An exception is thrown when trying to set an invalid translation.';

    These changes are out of scope and should be removed. Edit: Reference: http://webchick.net/please-stop-eating-baby-kittens

Also, could you clarify your point about backport from the summary?

Edit 2: Please include an interdiff when updating the patch, since everything aside from these things looked to be correct.

lars toomre’s picture

lars toomre commented

I will have to figure out how to the correct interdiff @xjm. I will try in the morning.

lars toomre’s picture

lars toomre commented

I am having git problems trying to get the patch in #1 to apply locally. My git skills are not the best so it probably is something I am doing wrong.

It appears that there has been a change to EntityTranslationTest.php in the interim. If not addressed by others, I will come back to this after rolling updates for other comments in this initiative.

lars toomre’s picture

lars toomre commented
Status: Needs work » Needs review
StatusFileSize
new 1797220-6-t-entity.patch49.73 KB

Here is the revised patch that addresses #3. I am having problems generating an interdiff because the $message lines noted in #3 were removed by an external patch in the interim.

xjm’s picture

xjm
she/her
Primary language English
commented
Assigned: Unassigned » jhodgdon
Status: Needs review » Reviewed & tested by the community

Reviewed this locally again. Looks good. Thanks @jhodgdon for suggesting adding --unified=0 to the diff command locally; that makes it even easier to scan.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Patch in #6 is committed to 8.x. Can someone review 8.x and make sure it's all taken care of? Meanwhile, moving to 7.x on the assumption that it is.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: jhodgdon » Unassigned
dcam’s picture

dcam commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new entity-1797220-10.patch51.29 KB

Backported #6 to D7.

dcam’s picture

dcam commented

#10: entity-1797220-10.patch queued for re-testing.

dcam’s picture

dcam commented
Issue tags: +Novice

Tagging as Novice.

izus’s picture

izus commented

#10: entity-1797220-10.patch queued for re-testing.

izus’s picture

izus commented
Status: Needs review » Reviewed & tested by the community

Hi,
the patch seems ok
Thanks

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks again -- committed to 7.x.

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