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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lars Toomre’s picture

Status: Active » Needs review
FileSize
51.18 KB

Here is an initial untested patch for this issue.

xjm’s picture

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

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

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

Lars Toomre’s picture

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

Status: Needs work » Needs review
FileSize
49.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

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

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

Assigned: jhodgdon » Unassigned
dcam’s picture

Status: Patch (to be ported) » Needs review
FileSize
51.29 KB

Backported #6 to D7.

dcam’s picture

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

dcam’s picture

Issue tags: +Novice

Tagging as Novice.

izus’s picture

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

izus’s picture

Status: Needs review » Reviewed & tested by the community

Hi,
the patch seems ok
Thanks

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Thanks again -- committed to 7.x.

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