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.
Comment | File | Size | Author |
---|---|---|---|
#9 | 1797914-9-P-S-assert-t.patch | 28.51 KB | dcam |
#4 | t-assert-P-S-1797914-4.patch | 6.64 KB | dcam |
#1 | t-assert-P-S-1797914-1.patch | 30.92 KB | Lars Toomre |
Comments
Comment #1
Lars Toomre CreditAttribution: Lars Toomre commentedHere 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.
Comment #2
dcam CreditAttribution: dcam commented#1 looks good. I didn't find any additional t()'s around assert messages.
Comment #3
jhodgdonAmazingly, 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!
Comment #4
dcam CreditAttribution: dcam commentedI removed the additional t()'s from around the group parameters where I found them.
Comment #5
dcam CreditAttribution: dcam commentedTagging as Novice.
Comment #6
izus CreditAttribution: izus commented#4: t-assert-P-S-1797914-4.patch queued for re-testing.
Comment #7
izus CreditAttribution: izus commentedHI,
#4 looks good for me
Thanks
Comment #8
jhodgdonThanks! Committed #4 to 8.x. We now need to port a combination of #1/#4 to 7.x.
Comment #9
dcam CreditAttribution: dcam commentedHere's a backport of #1 and #4.
Comment #11
dcam CreditAttribution: dcam commented#9: 1797914-9-P-S-assert-t.patch queued for re-testing.
Comment #12
jhodgdon#9: 1797914-9-P-S-assert-t.patch queued for re-testing.
Comment #13
jhodgdonbump! We only have a couple of these left. Can someone review this patch?
Comment #14
lazysoundsystem CreditAttribution: lazysoundsystem commentedThis one applies cleanly, and passes all the tests.
Comment #15
jhodgdonThanks all! Committed to 7.x.
Comment #16.0
(not verified) CreditAttribution: commentedAdded change counts for this issue.