Problem/Motivation
There are several uses of deprecated methods in the test classes, like assertText(), assertNoText(), assertRaw(), assertNoRaw() and assertTitle(), all of which will be removed in Drupal 10. There are other deprecated methods in use which will also be removed in Drupal 10, but I thought the scope of this issue would maybe become too big to fix them all in one go, in a suitable manner. If the maintainers or the community think it's doable, we could increase the scope of this issue to those other methods too.
Proposed resolution
Replace the assertText(), assertNoText(), assertRaw(), assertNoRaw() and assertTitle() methods with the ones suggested in their respective deprecation notices.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | Simplenews_assertText_result.txt | 1.28 KB | marcos_lima |
Issue fork simplenews-3250848
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:
Comments
Comment #2
marcos_lima commentedComment #4
marcos_lima commentedI replaced uses of
assertText()withassertSession()->pageTextContains();assertNoText()withassertSession()->pageTextNotContains();assertRaw()withassertSession()->responseContains();assertNoRaw()withassertSession()->responseNotContains(); andassertTitle()withassertSession()->titleEquals(). Then committed the changes and opened an MR. Please kindly review.Comment #5
beatrizrodriguesI'll do the review
Comment #6
beatrizrodriguesI checked, and all the uses of assertText(), assertNoText(), assertRaw(), assertNoRaw() and assertTitle() where replaced for its respective methods suggested in the deprecation notice. So, I'm changing to RTBC.
Comment #7
adamps commentedMany thanks for taking on this one. I made some comments, just minor.
Comment #8
marcos_lima commentedThank you kindly for your review @beatrizrodrigues and @AdamPS. Here are some comments about @AdamPS notes on the MR.
1. I removed the Html::escape() in
SimplenewsPersonalizationFormsTest.phpbecause, unlike the deprecatedassertText(), inpageTextContains()the passed text should be HTML decoded, exactly as a human sees it in the browser. So the Html::escape was being used to sanitize/encode therandomStringgenerated (like converting ‘<’ to ‘<’) and then asserting if it was in the page, sinceassertText()filters the page Html. WithpageTextContains()that was not needed anymore.To try and make it more clear, I attached a .txt with the output of the test with
assertText()if I remove the Html::escape. TherandomStringgenerated was ”z_{\^A7V7x>&=2))A0]” and the output theassertText()looked for was the encoded version of it, “z_{\^A7V7x>&=2))A0]”. WithpageTextContains()we assert using the decoded Html directly, so no need to use Html::escape anymore. If it doesn’t make sense and you think I need to change something, feel free to correct me.2. I was not able to remove the t() from some of the
assertRaw()methods initially because, like inSimplenewsPersonalizationFormsTest.php, the page is returning“The email address <em class="placeholder">s88x@example.com</em> belongs to a blocked user.".Without the t() the tests fail with the following error:
The string "The email address nctb@example.com belongs to a blocked user." was not found anywhere in the HTML response of the current page.Also this happens with some others
assertRaw()that were in use. InSimplenewsSubscribeTest.phpI get"Are you sure you want to confirm your subscription for <em class="placeholder">n*****@e*****.com</em>?"from the page.I researched about it and found that the placeholder tag used in some files of the codebase is the % one, like on line 199 of
SubscriptionsBlockForm.php:$message = $this->t('The email address %mail belongs to a blocked user.', ['%mail' => $mail]);so the replacement value is wrapped in
<em>tags on the page. I guess the t() function was keeping the tag from being sanitized. I think we could add the tags to the test assertion like in the commit I just made, which removed all t() calls in the methods I replaced, or change the placeholder tag (to @) in the code itself.3. Thanks for the tip about string interpolation, I tried to use it in some places where I saw it fit better, if you think there are any others I can change them too.
I think I addressed all the comments, if I missed any or if something is wrong I will gladly try to fix it again.
Comment #9
marcos_lima commentedComment #10
adamps commentedComment #12
adamps commentedGreat thanks