Posted by xjm
Part of meta issue #500866: [META] remove t() from assert message focused on the Search module.
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().
There are approximately XXX changes needed in XX Test files. This issue includes conversion of all format_patch() occurances as well.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 1797508-11-t-search.patch | 17.18 KB | dcam |
| #7 | 1797508-7-t-search.patch | 18.04 KB | lars toomre |
| #5 | 1797508-5-t-search.patch | 18.03 KB | lars toomre |
| #3 | 1797508-2-t-search.patch | 19.03 KB | lars toomre |
Comments
Comment #1
lars toomre commentedAn initial untest patch for this one is forthcoming shortly!
Comment #2
xjmOh, I had actually already generated part of the patch, which is why I'd assigned the issue to myself (as with the others). But it wasn't a lot of work, so no big deal.
Comment #3
lars toomre commentedHere is an initial untested patch for this issue. It includes a number of format_string() conversions.
It also includes some string concatenations that were turned into format_string() as well. Please take a closer look at those to make sure I did the conversions correctly.
Comment #5
lars toomre commentedA slight formatting problem should be fixed with this untested patch.
Comment #7
lars toomre commentedHelps if I remember to put the placeholders in an array!
Comment #8
dcam commented#7 looks good to me. I didn't find any remaining t()'s in the Search module test asserts. Marking as RTBC.
Comment #9
webchickTum te tum...
Comment #10
jhodgdonCommitted to 8.x, thanks all! Ready for port...
Comment #11
dcam commentedBackported #7 to D7.
Comment #12
dcam commented#11: 1797508-11-t-search.patch queued for re-testing.
Comment #13
dcam commentedTagging as Novice.
Comment #14
theladebug commentedMaybe I'm misunderstanding, but there are obviously t()'s in the patch in #11.
Comment #15
jhodgdonTo review this patch, what you want to do is:
- Apply the patch.
- Verify that after the patch is applied, there are no t() messages left around test assertion messages in the affected files.
- Verify by using the appropriate git diff command that the changes made are accurate.
The base issue has more information:
#500866: [META] remove t() from assert message
Comment #16
dcam commentedIf you would like information on reviewing these patches, I'm happy to help out. If you're sprinting in Portland today, come find me (dcam). I'm directing traffic out in front of the main core contributor sprint rooms, B113-119. Look for the guy in the green shirt and black hoodie.
Comment #17
jhodgdon#11: 1797508-11-t-search.patch queued for re-testing.
Comment #17.0
jhodgdonUpdated issue summary.
Comment #18
lazysoundsystem commentedThis too looks good. RTBC.
Comment #19
jhodgdonThanks all! Committed to 7.x.
Comment #20.0
(not verified) commentedRemoving myself from the author field so I can unfollow. --xjm