I want to remove t() from all the documented example code in BrowserTestBase.

This best describes why.

https://www.drupal.org/docs/8/phpunit/phpunit-browser-test-tutorial

When to use t() in browser tests

Never! Nope, not in assertion messages, not for button labels, not for text you assert on the page. You always want to test the literal string on the page, you don't want to test the Drupal translation system.

------

It is too big a job to remove t('example') from our code ... it will go away slowly in years to come.

But we should stop documenting it in our examples now.

CommentFileSizeAuthor
#10 charm-2875148-9.patch3 KBmartin107
#9 interdiff-2875148-9.txt2.49 KBmartin107
#9 charm-2875148-9.txt3 KBmartin107
#5 interdiff-2875148-2-5.txt595 bytesmartin107
#5 top-2875148-5.patch2.73 KBmartin107
#2 t-2875148-2.patch2.38 KBmartin107

Comments

martin107 created an issue. See original summary.

martin107’s picture

martin107 commented
Assigned: martin107 » Unassigned
Status: Active » Needs review
StatusFileSize
new t-2875148-2.patch2.38 KB

My fellow Bostonians, I humbly suggest the first step is for us to throw the tea in the harbour.

martin107’s picture

martin107 commented
Issue tags: +PHPUnit
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Maybe we should explitely mention to not use the t() function, what do you think?

martin107’s picture

martin107 commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new top-2875148-5.patch2.73 KB
new interdiff-2875148-2-5.txt595 bytes

Maybe we should explicitly mention to not use the t() function, what do you think?

I agree, added at the top of the class.

PS made issue summary more readable.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Thank you @martin107!

martin107’s picture

martin107 commented
Title: BrowserTestBase: Steer new code development away from translation » BrowserTestBase: Steer new test development away from translation
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/tests/Drupal/Tests/BrowserTestBase.php
@@ -40,6 +40,9 @@
+ * Tests extending this base class should not use the t() translate function
+ * unless directly testing translation functionality.

    I think we should say something like Tests extending this base class should only translate text when testing translation functionality. For example, avoid wrapping test text with t() or TranslatableMarkup().

    Since t() is deprecated and on the way out. 

  2. +++ b/core/tests/Drupal/Tests/BrowserTestBase.php
@@ -761,9 +764,9 @@ protected function drupalLogout() {
+   *   'Save'. The processing of the request depends on this value. For
+   *   example, a form may have one button with the value 'Save' and another
+   *   button with the value 'Delete', and execute different code depending
    *   on which one is clicked.

@@ -871,9 +874,9 @@ protected function submitForm(array $edit, $submit, $form_html_id = NULL) {
+   *   'Save'. The processing of the request depends on this value. For
+   *   example, a form may have one button with the value 'Save' and another
+   *   button with the value 'Delete', and execute different code depending
    *   on which one is clicked.

    These comments can re-flowed

martin107’s picture

martin107 commented
Status: Needs work » Needs review
StatusFileSize
new charm-2875148-9.txt3 KB
new interdiff-2875148-9.txt2.49 KB

8.1) Sure, adopting a more informative tone, is better.

8.2) Reflowed.

martin107’s picture

martin107 commented
StatusFileSize
new charm-2875148-9.patch3 KB

I knew making quark jokes would get me into trouble.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

This patch still applies, it's a great idea to improve this documentation. It changed the changes requested in #8.

  • catch committed 8626704 on 8.5.x 
    Issue #2875148 by martin107, dawehner: BrowserTestBase: Steer new test...
catch’s picture

catch
he/him
Primary language English
commented
Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

  • catch committed 3530739 on 8.4.x 
    Issue #2875148 by martin107, dawehner: BrowserTestBase: Steer new test...
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Anyone want to improve Examples on this? #2924223: Remove t() from tests

Status: Fixed » Closed (fixed)

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