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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

martin107 created an issue. See original summary.

martin107’s picture

Assigned: martin107 » Unassigned
Status: Active » Needs review
FileSize
2.38 KB

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

martin107’s picture

Issue tags: +PHPUnit
dawehner’s picture

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

Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
FileSize
2.73 KB
595 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

Status: Needs review » Reviewed & tested by the community

Thank you @martin107!

martin107’s picture

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

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

Status: Needs work » Needs review
FileSize
3 KB
2.49 KB

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

8.2) Reflowed.

martin107’s picture

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

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

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

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.