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

In D8 per, 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 database sub-system tests for the above. There are approximately 350 changes needed.

#10 database-1794012-10.patch104.12 KBdcam
PASSED: [[SimpleTest]]: [MySQL] 39,782 pass(es).
[ View ]
#8 database-1794012-8.patch104.09 KBdcam
FAILED: [[SimpleTest]]: [MySQL] 39,437 pass(es), 0 fail(s), and 2 exception(s).
[ View ]
#1 Remove-t-1794012-1.patch114.8 KBLars Toomre
PASSED: [[SimpleTest]]: [MySQL] 41,483 pass(es).
[ View ]


Status:Active» Needs review
new114.8 KB
PASSED: [[SimpleTest]]: [MySQL] 41,483 pass(es).
[ View ]

Here is an initial untested patch for this issue.

Status:Needs review» Reviewed & tested by the community

This looks good on visual spot check, and the bot approves, so yay! Thanks, Lars.

Issue tags:+coding standards

Tagging with coding standards so Jennifer can get to this if she has time.

Assigned:Lars Toomre» jhodgdon

Might as well make that explicit.

Assigned:jhodgdon» Unassigned
Status:Reviewed & tested by the community» Fixed

Wow. Committed to 8.x. Should this be backported to 7.x too? And are there issues for the other tests, since this one just covers the database system tests?

Thanks @jhodgdon. I think this was my first core commit.

The main issue #500866: [META] remove t() from assert message is stalled. Correction for the comment modules needs to be rerolled in #1742602: Removing t() from asserts in simpletests in comment module. To my knowledge, there are no other active patches about removing t() from assert messages. You might want to check with @xjm though.

I can continue to roll patches like this one. However, as I asked in #500866-237: [META] remove t() from assert message, I would like some guidance on number of changes in one patch. On Monday, when locally fixing tests for Common, there were about 235 changes. Do you want that as one big group or as smaller sub-sets? Previously, @xjm was worried that large patches like this one might conflict with other active patches.

Let me know. Thanks.

Version:8.x-dev» 7.x-dev
Status:Fixed» Patch (to be ported)

We now have a tag called "avoid commit conflicts" for issues
and my mandate is to avoid conflicts with those issues when I commit. Other than that, patches about the size of the one that was on this issue are fine, and you don't need to worry about the conflicts (I'll check when I do the commit). So far I haven't had any complaints about big commits messing up other patches since we introduced this tag.

Anyway, based on that base issue (which should be mentioned in this issue summary by the way!), I guess this needs a backport to D7.

Status:Patch (to be ported)» Needs review
new104.09 KB
FAILED: [[SimpleTest]]: [MySQL] 39,437 pass(es), 0 fail(s), and 2 exception(s).
[ View ]

Backported #1 to D7.

Status:Needs review» Needs work

The last submitted patch, database-1794012-8.patch, failed testing.

Status:Needs work» Needs review
new104.12 KB
PASSED: [[SimpleTest]]: [MySQL] 39,782 pass(es).
[ View ]

I mistakenly deleted a couple of t() functions that should have been changed to format_string().

#10: database-1794012-10.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

I was in shock that this still applied, until I realized it's the backport patch so the code isn't really changing. :-) Bot's good with it, looks good on visual read through.

Status:Reviewed & tested by the community» Fixed

Whew, 345 lines changed! Thank goodness for diff --color-words... makes the scanning much more readable.

Thanks for the patch & review. Committed to 7.x.

Sweet. We've finally gotten one of these committed for 7.x and closed out. Thanks for the review, Crell. =)

jhodgdon, do you think these issues are good candidates for being tagged as "Novice" issues so we can maybe get some more eyes on them? They seem easy to review, although maybe a bit tedious.

Many of them are already tagged Novice. Yes, definitely good novice material! We don't require Novice projects to be extremely quick, just straightforward and doable. :)

Ok, thanks. I'll check them and tag any that haven't been tagged already.

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

Issue summary:View changes

Updated issue summary.