This is a sub-task of #500866: [META] remove t() from assert message focused on the File 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 File sub-system tests for the above. There are approximately 118 changes needed (including format_string() changes).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lars Toomre’s picture

Status: Active » Needs review
FileSize
42.43 KB

Here is an initial untested patch for this issue. This patch includes format_string() conversions as well.

xjm’s picture

Assigned: Unassigned » jhodgdon
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs backport to D7

I reviewed all the changes locally; looks good. Sorry @jhodgdon for another email. :)

jhodgdon’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

This one's in, thanks!

jhodgdon’s picture

Assigned: jhodgdon » Unassigned
Lars Toomre’s picture

Title: Remove t() from asserts from File sub-system tests » Remove t() from asserts from File system tests
Version: 7.x-dev » 8.x-dev
Status: Patch (to be ported) » Needs review
FileSize
8.26 KB

I was double checking all of the tests in the File system and see that a bunch were not included from one file: DirectoryTest.php. The attached patch converts those as well. I have confirmed that all other tests in File system are correct after this gets in.

dcam’s picture

Status: Needs review » Needs work

All of the changes in #5 look good, but I found a couple of stray t()'s around assert messages in UnmanagedCopyTest.php, lines 42, 86. Maybe they were added by a recent commit, I don't know. If they can get taken care of, then I'll make sure to re-test.

Lars Toomre’s picture

Status: Needs work » Needs review
FileSize
9.41 KB

Here is a patch that builds on #5 and adds the two fixes from #6 as well. I am pretty sure both of those lines were changed in the past three weeks or so.

dcam’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, @Lars Toomre! #7 looks good. I don't see any other t()'s in the system file tests.

jhodgdon’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Thanks! This is committed. I guess back to 7.x for porting of one or more of the above 8.x patches?

dcam’s picture

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

Backported #1 and #7 to D7.

dcam’s picture

#10: 1797296-10-t-file.patch queued for re-testing.

dcam’s picture

Issue tags: +Novice

Tagging as Novice.

izus’s picture

#10: 1797296-10-t-file.patch queued for re-testing.

izus’s picture

Status: Needs review » Reviewed & tested by the community

Hi,
patch in #10 seems good for me
Thanks

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Thanks all! Committed to 7.x.

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