Problem/Motivation

There is no need to use t() in tests, unless we're testing translations, however in core we do not follow this consistently, which does not set a good example for new contributions.

#3153468: Strip HTML tags when using assertEquals() to compare markup exists to deal with some tricky cases in assertEquals() calls.

If we ignore those then there are just over 100 cases remaining in assertions, which seems suitable to deal with all in one go:

$ rg 'assert.*\Wt\(' core|grep -v assertEqual|wc -l
108

Steps to reproduce

Proposed resolution

Search for calls to t() in tests using the above regex and remove t() and/or replace the assertion with something more appropriate.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3227501

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

longwave’s picture

Title: Remove remaining uses of t() in tests » Remove remaining uses of t() in test assertions
Issue summary: View changes

longwave’s picture

Status: Active » Needs work

Started but didn't finish.

longwave’s picture

Status: Needs work » Needs review

Remaining cases are translation tests and MenuUiContentModerationTest where it is checking for a specific link embedded in a message and the t() version is significantly easier to read, to me anyway.

mondrake’s picture

Status: Needs review » Needs work

A few possible improvements, see comments in the MR

longwave’s picture

Status: Needs work » Needs review

Thanks for reviewing, feedback addressed (and added two more replacements of Link::fromTextAndUrl() in UpdateContribTest).

mondrake’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, looks good to me.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.3.x, thanks!

  • catch committed 406dfbc on 9.3.x
    Issue #3227501 by longwave, mondrake: Remove remaining uses of t() in...
catch’s picture

Doesn't cherry-pick to 9.2.x. While this touches a few files, it's not that much in each file, so we can probably survive without the backport - however would commit one if someone does one.

longwave’s picture

Status: Fixed » Patch (to be ported)

It will be easier to backport if #3139409: Replace usages of AssertLegacyTrait::assertRaw, that is deprecated gets in, as most of the conflicts are related to assertRaw().

longwave’s picture

Title: Remove remaining uses of t() in test assertions » [backport] Remove remaining uses of t() in test assertions

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

xjm’s picture

Version: 9.4.x-dev » 9.3.x-dev
Status: Patch (to be ported) » Fixed

Too late for 9.2.x now. :)

Status: Fixed » Closed (fixed)

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