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.

Files: 
CommentFileSizeAuthor
#11 1797508-11-t-search.patch17.18 KBdcam
PASSED: [[SimpleTest]]: [MySQL] 40,480 pass(es).
[ View ]
#7 1797508-7-t-search.patch18.04 KBLars Toomre
PASSED: [[SimpleTest]]: [MySQL] 41,894 pass(es).
[ View ]
#5 1797508-5-t-search.patch18.03 KBLars Toomre
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/search/lib/Drupal/search/Tests/SearchNumbersTest.php.
[ View ]
#3 1797508-2-t-search.patch19.03 KBLars Toomre
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/search/lib/Drupal/search/Tests/SearchNumberMatchingTest.php.
[ View ]

Comments

Assigned:xjm» Lars Toomre

An initial untest patch for this one is forthcoming shortly!

Oh, 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.

Title:Remove t() from assertion messages in tests for the search moduleRemove t() from assertion messages in tests for the Search module
Status:Active» Needs review
StatusFileSize
new19.03 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/search/lib/Drupal/search/Tests/SearchNumberMatchingTest.php.
[ View ]

Here 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.

Status:Needs review» Needs work

The last submitted patch, 1797508-2-t-search.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new18.03 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/search/lib/Drupal/search/Tests/SearchNumbersTest.php.
[ View ]

A slight formatting problem should be fixed with this untested patch.

Status:Needs review» Needs work

The last submitted patch, 1797508-5-t-search.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new18.04 KB
PASSED: [[SimpleTest]]: [MySQL] 41,894 pass(es).
[ View ]

Helps if I remember to put the placeholders in an array!

Status:Needs review» Reviewed & tested by the community

#7 looks good to me. I didn't find any remaining t()'s in the Search module test asserts. Marking as RTBC.

Assigned:Lars Toomre» jhodgdon

Tum te tum...

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

Committed to 8.x, thanks all! Ready for port...

Status:Patch (to be ported)» Needs review
StatusFileSize
new17.18 KB
PASSED: [[SimpleTest]]: [MySQL] 40,480 pass(es).
[ View ]

Backported #7 to D7.

#11: 1797508-11-t-search.patch queued for re-testing.

Issue tags:+Novice

Tagging as Novice.

Maybe I'm misunderstanding, but there are obviously t()'s in the patch in #11.

To 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

If 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.

#11: 1797508-11-t-search.patch queued for re-testing.

Issue summary:View changes

Updated issue summary.

Status:Needs review» Reviewed & tested by the community

This too looks good. RTBC.

Status:Reviewed & tested by the community» Fixed

Thanks all! Committed to 7.x.

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

Issue summary:View changes

Removing myself from the author field so I can unfollow. --xjm