Closed (fixed)
Project:
Drupal core
Version:
9.2.x-dev
Component:
phpunit
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
30 Jul 2021 at 09:16 UTC
Updated:
23 Aug 2021 at 11:04 UTC
Jump to comment: Most recent
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.
In #3133726: [meta] Remove usage of t() in tests not testing translation we identified there are severals of calls to t() in calls to assertEqual() and assertEquals() and that removing all these in one go seems to be a suitable way of attacking this problem.
Proposed resolution
Identify and remove all calls to t() wrapped in calls to assertEquals() where:
- the message is a plain string that doesn't use placeholders - here t() can be removed
- the message is used as the $message argument in the assertion - here the entire argument can usually be removed
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3226008
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:
- 3226008-9.2.x
changes, plain diff MR !1033 - 3226008-remove-simple-uses changes, plain diff MR !987
Comments
Comment #3
longwaveFirst pass at this. I thought about removing
new FormattableMarkup()here as well but then remembered #2549805: [Meta] Remove all usage of FormattableMarkup in tests apart from explicit tests of that API, I think this should only covert().Comment #4
mondrakeNice. A few things can be improved, two changes that overlap with #3220255: Convert assertions involving use of xpath on links to WebAssert
Comment #5
longwaveThanks for the review. I fixed those and took the opportunity to improve some other assertions here as well.
Comment #6
mondrakeComment #7
longwaveComment #8
mondrakeFew more comments.
Comment #9
longwaveFixed two, disagreed with one :)
Comment #10
longwaveImproved the "Place block" case over at #3038594: WebAssert should return the found links
Comment #11
mondrakeThanks. LGTM now. Overlaps with #3220255: Convert assertions involving use of xpath on links to WebAssert so whatever goes in first will require the reroll of the other.
Comment #12
mondrakeComment #13
mondrakeComment #14
longwaveMerged 9.3.x.
Comment #15
mondrakeBack to Rtbc
Comment #16
catchCommitted/pushed to 9.3.x, thanks!
Needs a re-roll for 9.2.x.
Comment #17
mondrakeNot pushed.
Comment #18
longwaveRestoring old metadata until 9.3.x is pushed.
Comment #20
longwaveComment #22
longwaveRerolled, one conflict in DownloadTest where file_create_url() is $this->fileUrlGenerator->generateString() in 9.3.x
Comment #25
catchCommitted c4d7857 and pushed to 9.2.x. Thanks!