This is a sub-task of #500866: [META] remove t() from assert message focused on the T-Z 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 Theme, Update, Upgrade and Uuid sub-system tests for the above. There are approximately 98 changes needed spread out across 16 test files. This issue includes conversion of all format_patch() occurances as well.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 1797920-15-t-assert-T-Z.patch | 59.84 KB | dcam |
| #9 | 1797920-9-t-assert-T-Z.patch | 59.74 KB | dcam |
| #5 | 1797920-5-t-assert-T-Z.patch | 46.31 KB | lars toomre |
| #3 | 1797920-3-t-assert-T-Z.patch | 45.46 KB | lars toomre |
| #1 | t-assert-T-Z-1797920-1.patch | 46.06 KB | lars toomre |
Comments
Comment #1
lars toomre commentedHere is an initial untested patch for this issue. This patch includes format_string() conversions as well.
Per #1797318-5: Remove t() from asserts from Form sub-system tests, I was under the impression that escaped strings should use format_string(). @xjm thought it was not required. Hence, in this issue, I have simply removed the t() and not changed them to format_string(). Let's see what the bot thinks.
Those assert messages in this patch also might be worthy of a closer look in the review process. We also could convert some of them to double quoted strings and remove the escape character.
Comment #2
dcam commented#1 no longer applies due to changes in UpdateScriptTest.php, LanguageUpgradePathTest.php, and UpgradePathTestBase.php.
Comment #3
lars toomre commentedAttached is a re-roll of #1.
Comment #4
dcam commentedI tested #3. I found one extra t() around an assert message in Upgrade/UuidUpgradePathTest.php, line 40. If that can be added, then I'll re-test and it can be RTBC.
Comment #5
lars toomre commentedHere is an updated patch that includes what looks like an added assert since this was first rolled weeks ago.
Thanks for the review @dcam.
Comment #6
dcam commentedI tested #5 and didn't find any more t()'s around assert messages. It looks good to me.
Comment #7
webchickComment #8
jhodgdonThanks, committed to 8.x and ready for backport.
Comment #9
dcam commentedBackported #5 to D7.
Comment #10
dcam commented#9: 1797920-9-t-assert-T-Z.patch queued for re-testing.
Comment #11
dcam commented#9: 1797920-9-t-assert-T-Z.patch queued for re-testing.
Comment #12
dcam commentedTagging as Novice.
Comment #13
izus commented#9: 1797920-9-t-assert-T-Z.patch queued for re-testing.
Comment #15
dcam commentedRerolled #9.
Comment #16
dcam commentedStatus...
Comment #17
izus commentedHi,
#15 seems good for me.
Thanks
Comment #18
jhodgdonThanks again! Committed to 7.x.
Comment #19.0
(not verified) commentedUpdated initial change counts.