Remove uses of t() and switch to pageTextContains() in assert(No)Raw() calls

Kick-off patch, with just a deprecation error triggered when an object is passed as a $raw argument. This would give us the compass to steer through the conversions.

For discussion: shall we leave the deprecation in, or only during the conversion to be removed before commit? While we are saying that calling t() is legitimate in translation tests, IMHO those cases are limited enough to expect that developers could cast the output of t() to a string before putting that in the assertion.

I'll start doing some.

#6 is a fair point in general, OTOH there is no comparator kicking in here specifically, since the assertion is eventually perfomed by WebAssert::response[Not]Contains which is just doing the casting to string itself.

#5 failure is telling us that we probably need to allow FormattableMarkup though:

+++ b/core/modules/action/tests/src/Functional/ConfigurationTest.php
@@ -83,7 +83,7 @@ public function testActionConfiguration() {
-    $this->assertRaw(t('The action %action has been deleted.', ['%action' => $new_action_label]));
+    $this->assertRaw("The action $new_action_label has been deleted.");

This is failing because the real string is

The action <em class="placeholder">$new_action_label</em> has been deleted.

but that seems to me too much to add all that boilerplate. So maybe here a reasonable compromise could be to switch to

-    $this->assertRaw(new FormattableMarkup('The action %action has been deleted.', ['%action' => $new_action_label]));

Thoughts?

Working on an updated patch.

#9 very good point... in fact I was also thinking lots of these assertions should use assertText (or more appropriately, WebAssert::pageTextContains).

Point is: we do that here straight, or we do an intermediate step to clean up here and then do the association to the appropriate WebAssert method in the parent issue?

Still trying to figure out the proper direction for this patch.

Here:

• ::assertRaw calls that were using t() to workaround the preparation of a matchable string with markup for use by assertRaw, but are really meant to check correctness of the human readable text, are converted to $this->assertSession()->pageTextContains() with concatenated strings as input (so that they can match the page text as returned by the getText Mink method)
• tests that are really using t() to translate an input string are kept as assertRaw calls, but with the argument explicitly cast to string to prevent the deprecation trigger
• ... just for a limited number of cases to get feedback ...

Some more.

@longwave sorry #12 was xposted with #11 and I did not notice until I came back for #13. IMHO #12 and #13 are in the right direction, because I don't see a purpose to test the <em> markup, the use of t() in many cases seems just a shortcut. Maybe we should ask for feedback to core committers in Slack?

Some progress. I am starting to think we should not have a permanent deprecation trigger here: FormattableMarkup may be returned legitimately from underlying methods tested. So leaving ATM the deprecation trigger only TranslatableMarkup, however, there are cases where instead of t() there is use of new FormattableMarkup with the same intent of formatting a string within the test for the purpose of using it in an assert, which is what we are trying to get rid of. OTOH I agree casting t() to string explicitly in translation related testing is annoying. A bit messy.

Trying to fix failures in the work done so far - which pertains to test groups starting with a-c. I'll pause here. We need feedback if this is the right direction and whether it would be ok to proceed with a single patch, which will be rather large eventually.

Some fixes.

#23 yes... maybe Postponed is a better status right now.

Changed issue to MR workflow.

Rerolled.

Rerolled. Don't think we need release manager review ATM.

Green. Ready for review

Thanks for review. Done the changes suggested, and removed the deprecation triggered when passing a Translatable object as the argument to assert(No)Raw() calls - that's not strictly necessary, it was useful to spot the changes to be done.

Few fixes needed.

fixed