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 assertFieldByName(), assertFieldByXpath() and assertNoFieldByXPath() 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 assertFieldByName(), assertFieldByXpath() and assertNoFieldByXPath(), except those used by translation-related code (if any).
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comments
Comment #2
hardik_patel_12 commentedKindly review the patch.
Comment #3
mondrakeComment #4
mohrerao commentedReroll # 2 as patch failed to apply
Comment #5
siddhant.bhosale commentedHi, I have tested the issue and the patch applies cleanly. Attaching the test result.
Comment #6
siddhant.bhosale commentedComment #8
mohrerao commentedFailing test passes in local. Looks like a random javascript test failure. Queuing for test again.
Comment #9
mondrake#4 does not seem to have anything in common with #2.
Also it would be good to temporarily add a conditional deprecation so that when a
t()object is passed in, a deprecation error is thrown. This would give us confidence that there are no more cases left out. When confirmed, then, we can remove the temporary deprecation from the patch before RTBC.Comment #10
snehalgaikwad commentedRemoved uses of t() from assertFieldByName(), assertFieldByXpath() and assertNoFieldByXPath() functions.
Comment #11
longwaveAdded the suggested test deprecation, found several missing cases, increased scope to all
assert[No]FieldBy*methods (as they all eventually call the XPath methods, and that is where the test deprecation lives)Comment #12
longwaveForgot the interdiff
Comment #14
ravi.shankar commentedWorking on this.
Comment #15
ravi.shankar commentedHere I have tried to fix the failed test of patch #11.
Comment #17
hardik_patel_12 commentedAssertLegacyTrait::assertFieldByXPath() is deprecated in drupal:9.1.0 , kindly see https://www.drupal.org/node/3129738
Comment #18
snehalgaikwad commentedReplaced deprecated AssertLegacyTrait::assertFieldByXPath() and AssertLegacyTrait::assertNoFieldByXPath() functions.
Comment #19
longwave#18 doesn't apply.
Comment #20
snehalgaikwad commentedComment #21
snehalgaikwad commentedComment #22
longwave#21 doesn't apply.
Comment #23
neslee canil pintoComment #25
nikitagupta commentedComment #26
nikitagupta commentedComment #28
megha_kundar commentedComment #29
megha_kundar commentedComment #31
megha_kundar commentedComment #32
longwave@Megha_kundar please upload interdiffs when working on patches, and comment to say what you have changed, as it helps reviewers figure out what has been done.
Comment #33
mondrakeI am not sure about the scope here - most of the assert[No]FieldBy*() calls have been converted to WebAssert already, and converrsion of assert[No]FieldByXPath() is ready for review at #3139429: Replace usages of AssertLegacyTrait::assert(No)FieldByXPath, that is deprecated - if that goes in first this issue will have no more material.
How about postponing on that, and then rescope this issue to fix
assertSession()->fieldValue[Not]Equalsinstead?Comment #36
longwaveClosing as duplicate, there are no instances remaining if I grep for
assert(No)?FieldBy.*t\(