This is a sub-task of #500866: [META] remove t() from assert message focused on the P-S include files from the system module sub-systems (excluding system sub-system itself).

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 Pager, Path, Queue and Session sub-system tests for the above. There are approximately 84 changes needed spread out across 7 test files. This issue includes conversion of all format_patch() occurances as well.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lars Toomre’s picture

Status: Active » Needs review
FileSize
30.92 KB

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

As in other of my patches for this initiative, this patch also includes some conversion of double quoted assert message text strings to single quoted strings to conform with Drupal's best practices.

dcam’s picture

Status: Needs review » Reviewed & tested by the community

#1 looks good. I didn't find any additional t()'s around assert messages.

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs work

Amazingly, this patch still applies. Committed to 8.x.

I noticed when doing my final review that some of the $group parameters still have t() on them. Maybe we should follow-up patch to remove those? Thanks!

dcam’s picture

Status: Needs work » Needs review
FileSize
6.64 KB

I removed the additional t()'s from around the group parameters where I found them.

dcam’s picture

Issue tags: +Novice

Tagging as Novice.

izus’s picture

#4: t-assert-P-S-1797914-4.patch queued for re-testing.

izus’s picture

Status: Needs review » Reviewed & tested by the community

HI,
#4 looks good for me
Thanks

jhodgdon’s picture

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

Thanks! Committed #4 to 8.x. We now need to port a combination of #1/#4 to 7.x.

dcam’s picture

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

Here's a backport of #1 and #4.

Status: Needs review » Needs work
Issue tags: -Novice

The last submitted patch, 1797914-9-P-S-assert-t.patch, failed testing.

dcam’s picture

Status: Needs work » Needs review

#9: 1797914-9-P-S-assert-t.patch queued for re-testing.

jhodgdon’s picture

Issue tags: +Novice

#9: 1797914-9-P-S-assert-t.patch queued for re-testing.

jhodgdon’s picture

bump! We only have a couple of these left. Can someone review this patch?

lazysoundsystem’s picture

Status: Needs review » Reviewed & tested by the community

This one applies cleanly, and passes all the tests.

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.

Anonymous’s picture

Issue summary: View changes

Added change counts for this issue.