This is a sub-task of #500866: [META] remove t() from assert message focused on the System sub-system.

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 System sub-system tests for the above. There are approximately 127 changes needed (including format_string() changes) changing 22 Test files.

Files: 
CommentFileSizeAuthor
#14 1797926-14-t-assert-system.patch47.89 KBdcam
PASSED: [[SimpleTest]]: [MySQL] 40,373 pass(es).
[ View ]
#9 1797926-9-t-assert-system.patch56.09 KBLars Toomre
PASSED: [[SimpleTest]]: [MySQL] 42,802 pass(es).
[ View ]
#4 1797926-4-t-assert-system.patch55.82 KBLars Toomre
PASSED: [[SimpleTest]]: [MySQL] 42,807 pass(es).
[ View ]
#1 t-asset-system-1797926-1.patch55.75 KBLars Toomre
PASSED: [[SimpleTest]]: [MySQL] 41,696 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new55.75 KB
PASSED: [[SimpleTest]]: [MySQL] 41,696 pass(es).
[ View ]

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

The format_string() conversions appear to be correct, but are worthy of a closer look in the review process. A couple of them confused me at first and I had to convert some concatenations to escaped strings.

Looks like #1798756: Finish conversion of error_level to CMI added another t(). That will need to be removed.

Let's first see what reviewers of this patch have to say.

Status:Needs review» Needs work
Issue tags:+Needs reroll

#2 no longer applies due to changes in InfoAlterTest.php.

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new55.82 KB
PASSED: [[SimpleTest]]: [MySQL] 42,807 pass(es).
[ View ]

Here is a re-roll of the patch from #1. It also includes two additional fixes from the ErrorHandlerTest.php that were referenced in #2.

I did not go back and recheck that there were no other t() around assert messages. There might well have been some added since #1 was first rolled about three weeks ago. However, #1 was complete as of its creation time.

Thanks for trying to review this @dcam. Let me know what else you think needs to be changed.

Status:Needs review» Needs work

The last submitted patch, 1797926-4-t-assert-system.patch, failed testing.

Drupal\file\Tests\UsageTest test failure looks like it is a random test failure.

Status:Needs work» Needs review

#4: 1797926-4-t-assert-system.patch queued for re-testing.

Status:Needs review» Needs work

I tested #4. There's an addtional t() to eliminate in SiteMaintenanceTest.php, line 103.

Status:Needs work» Needs review
StatusFileSize
new56.09 KB
PASSED: [[SimpleTest]]: [MySQL] 42,802 pass(es).
[ View ]

Thanks for the review @dcam. Here is an updated patch with that one additional change.

Status:Needs review» Reviewed & tested by the community

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

Thanks again for the review @dcam! I glad that we are getting the D8 patches completed.

Assigned:Lars Toomre» jhodgdon

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

Thanks! Committed to 8.x.

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

Backported #9 to D7.

I tried to stick to backporting only the changes made in #9. There are other t()'s around assert messages in system.test, but they may be handled by other issues with patches that are being backported. system.test will probably need to be checked for remaining t()'s after all system-test-related issues have been committed.

Issue tags:+Novice

Tagging as Novice.

#14: 1797926-14-t-assert-system.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

Hi,
#14 seems good.
Thanks

Status:Reviewed & tested by the community» Fixed

Thanks! Committed to 7.x.

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

Issue summary:View changes

Updated initial counts.