As noted on:

http://drupal.org/node/265828

asserts should not have calls to t().

When working on:

#1483662: URLs for drupal.org module help pages need updates

we noticed that there are calls to t() inside asserts in help.test.

CommentFileSizeAuthor
#4 Remove_t_from_help_test-1497240-4.patch2.1 KBrealityloop
#1 Remove_t_from_help_test-1497240-1.patch2.98 KBmojolama

Comments

mojolama’s picture

mojolama commented
Status: Active » Needs review
StatusFileSize
new Remove_t_from_help_test-1497240-1.patch2.98 KB

Changed about 8 lines in help.test for 8.x.

Did a search on "assert" and removed all instances of t() from those lines.

tayzlor’s picture

tayzlor commented

Tried patch out, applies cleanly and tests ran successfully.

robloach’s picture

robloach
he/him
commented
Status: Needs review » Needs work

Needs an update though.

realityloop’s picture

realityloop commented
Status: Needs work » Needs review
StatusFileSize
new Remove_t_from_help_test-1497240-4.patch2.1 KB

rerolled

realityloop’s picture

realityloop commented
Assigned: mojolama » realityloop
realityloop’s picture

realityloop commented
Assigned: realityloop » Unassigned
dawehner’s picture

dawehner
Primary language German
commented

Just in general we should think whether it is okay to also remove the t() from the expected value,
though for the actual message this is perfect.

manumilou’s picture

manumilou commented
Status: Needs review » Reviewed & tested by the community

Patch #4 applies nicely, tests passed.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue tags: +Coding standards

I don't think these should be changed, it's assertion messages that we don't want in t(), like @dawehner said.

+++ b/core/modules/help/lib/Drupal/help/Tests/HelpTest.php
@@ -70,10 +70,10 @@ public function testHelp() {
+    $this->assertRaw(format_string('For more information, refer to the specific topics listed in the next section or to the <a href="@drupal">online Drupal handbooks</a>.', array('@drupal' => 'http://drupal.org/documentation')), 'Help intro text correctly appears.');
...
+    $this->assertRaw('<h2>' . 'Help topics' . '</h2><p>' . 'Help is available on the following items:' . '</p>', 'Help topics text correctly appears.');

Assertion messages: 'Help intro text correctly appears.', 'Help topics text correctly appears.'.

See #500866: [META] remove t() from assert message. Leaving status for now but this should probably be closed.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Closed (works as designed)

Yes t() is OK in those situations - it's comparing a translated string with a translated string, not an assertion message.