As per http://drupal.org/simpletest-tutorial-drupal7#t , I scanned the tests in the contact module and removed t()s from assert messages, or changed them to format_string() where appropriate.

Patch follows.

Files: 
CommentFileSizeAuthor
#19 1742830-19-contact-t.patch13.69 KBdcam
PASSED: [[SimpleTest]]: [MySQL] 40,372 pass(es).
[ View ]
#12 1742830-12-t-contact.patch2.76 KBlazysoundsystem
PASSED: [[SimpleTest]]: [MySQL] 41,903 pass(es).
[ View ]
#9 1742830-9-t-contact.patch3.22 KBLars Toomre
PASSED: [[SimpleTest]]: [MySQL] 41,904 pass(es).
[ View ]
#1 remove-t-from-Contact-asserts-1742830-1.patch14.06 KBlazysoundsystem
PASSED: [[SimpleTest]]: [MySQL] 41,485 pass(es).
[ View ]

Comments

StatusFileSize
new14.06 KB
PASSED: [[SimpleTest]]: [MySQL] 41,485 pass(es).
[ View ]

Here's the patch.

Status:Active» Needs review

Changing to 'needs review'.

I started this before reading through #500866: [META] remove t() from assert message, where they've been trying to solve this since 2009. Best to wait for confirmation there before putting any more work into this.

Status:Needs review» Reviewed & tested by the community

All of the changes removing t() from assert messages are appropriate. The re-test is green too so RBTC.

Status:Reviewed & tested by the community» Fixed

Committed to 8.x. Thanks!

Status:Fixed» Patch (to be ported)
Issue tags:+needs backport to D7

We're backporting these. :)

Status:Patch (to be ported)» Needs work

There's a some strings are still there, also I recommend to remove a group (Contact) from asserts. Probably $this->assertEqual(value1, value2) is enough....

+++ b/core/modules/contact/lib/Drupal/contact/Tests/ContactSitewideTest.php
@@ -206,14 +206,14 @@ class ContactSitewideTest extends WebTestBase {
+    $this->assertEqual(count($captured_emails), 1, t('Auto-reply e-mail was sent to the sender for category "bar".'), 'Contact');
+    $this->assertEqual($captured_emails[0]['body'], drupal_html_to_text($bar_autoreply), t('Auto-reply e-mail body is correct for category "bar".'), 'Contact');
...
-    $this->assertEqual(count($captured_emails), 0, t('No auto-reply e-mail was sent to the sender for category "no-autoreply".'), t('Contact'));
+    $this->assertEqual(count($captured_emails), 0, t('No auto-reply e-mail was sent to the sender for category "no-autoreply".'), 'Contact');

this t() should not be there

Status:Needs work» Needs review
StatusFileSize
new3.22 KB
PASSED: [[SimpleTest]]: [MySQL] 41,904 pass(es).
[ View ]

Atached is a patch which addresses the missing t() removals from messages.

@andypost - Can you open up a separate issue to remove the group parameter?

@Lars Toomre, I see no reason in new issue - this one is enough.
Please, re-roll a patch without group - this lines is one used this approach and this issue is about clean-up so let's make it here

I will leave the call about whether your request is appropriate to @xjm and @jhodgdon. I am tired of being accussed of scope creap.

This isssue is primarily about removing t() which slows down tests. Whether there are or not group parameters are another issue.

StatusFileSize
new2.76 KB
PASSED: [[SimpleTest]]: [MySQL] 41,903 pass(es).
[ View ]

Thanks for picking these up - I noticed I'd missed them when rolling the D7 backport.

I couldn't find any documentation about how they should be used, beyond the doxygen comment that they could be 'Browser' or 'Php', so I'm not sure if 'Contact' isn't an appropriate group for these, but it is inconsistent with all of the other module tests I've been through, so here's a patch without them.

I've opened another issue to make the use of groups more clear:
#1799326: Clean up $group usage in simpletests

Status:Needs review» Reviewed & tested by the community

Great!

Assigned:Unassigned» jhodgdon

Since this concerns coding standards, moving to Jennifer's queue.

Status:Reviewed & tested by the community» Needs work

Ummm... why is the group "contact" being *added* to these tests? We shouldn't be adding groups to tests.

Assigned:jhodgdon» Unassigned

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

The t() functions shown in #12 were removed by http://drupal.org/node/1588422#comment-6587438. Moving to D7 for backport.

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

Backported #1 to D7.

Issue tags:-needs backport to D7

#19: 1742830-19-contact-t.patch queued for re-testing.

Issue tags:+needs backport to D7

#19: 1742830-19-contact-t.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

seems good.
Thanks !

Status:Reviewed & tested by the community» Fixed

Thanks all! Committed to 7.x.

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