Problem/Motivation
From #3139412-9: Replace usages of deprecated AssertLegacyTrait::assertTitle()
Cleanup the arguments of calls to WebAssert::titleEquals and AssertLegacyTrait::assertTitle: change in 1st argument so that a string is passed directly (instead of casting to string a call to t() or a FormattableMarkup object), and no more arguments are passed in 2nd (being a $message) and 3rd (being a $group).
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | 3143339-23.patch | 33.17 KB | bunty badgujar |
| #14 | interdiff_12-14.txt | 543 bytes | mohrerao |
| #14 | 3143339-14.patch | 33.74 KB | mohrerao |
Comments
Comment #2
mondrakeComment #3
mohrerao commentedOn it
Comment #4
mohrerao commentedAdding a patch
Comment #5
longwaveI think it might be safe to replace
$this->config('system.site')->get('name')with the hardcoded string "Drupal"?Comment #7
mohrerao commented@longwave, That makes sense. But if someone wants to test this on local with a different site name, then it would fail. I see tests using both hardcoded text 'Drupal' as well as $this->config('system.site')->get('name'). I have now replaced it to Drupal.
Comment #8
mondrakeThe changes to assertText calls are out of scope here.
This is ending up assigning $title_source twice, and not using it also.
No, it won't, because for every functional test a default Drupal installation is prepared and the test run on that, to avoid that the local site info leak in the test.
Comment #9
mohrerao commentedThanks for the comments @mondrake.
Thanks for the info. Did not know that Drupal uses a fresh installation for test runs. That answers lot of time taken for running tests!!
Comment #10
mondrakeThanks @mohrerao, this looks good, just there's a stray use statement left causing CS check failure.
Nit:
This could be just
$this->assertTitle('0 | Drupal');, probably.Comment #11
mondrakeComment #12
mohrerao commentedThanks for that. Updated.
Comment #13
mondrakeThanks. This is outstanding:
Comment #14
mohrerao commentedAah missed it. Updating patch.
Comment #16
mondrakeThanks! This patch is cleaning up Simpletest cruft from calls to AssertLegacyTrait::assertTitle and WebAssert::titleEquals - ensuring the first argument is always a string and no further arguments are passed in.
Prep for #3139412: Replace usages of deprecated AssertLegacyTrait::assertTitle().
Comment #17
xjmComment #21
xjmSo much cleaner, and also actually better test coverage in a couple places. Thanks for the fast turnaround!
The test author was feeling a bit existentialist, apparently.
(* In case someone needs to say something about how Samuel Beckett was not actually an existentialist, OK, fine.)
I've committed this to 9.1.x and cherry-picked it to 9.0.x and 8.9.x. It does not cherry-pick cleanly to 8.8.x, so if it's straightforward, we should create an 8.8.x version. (8.8.x will go into security-only mode starting with the code freeze on June 2, FYI, but we may be able to get this and the issue it's blocking backported by then.) Thanks!
Comment #22
bunty badgujar commentedComment #23
bunty badgujar commentedbackport to 8.8.x version.
Comment #24
mohrerao commented3143339-23.patch looks fine. Moving to RTBC again for commiting against 8.8.x
Comment #25
bunty badgujar commentedComment #27
xjmCommitted the backport to 8.8.x. Thanks!