Cast objects implementing SafeStringInterface to string in TestBase / WebTestBase

comments from @dawehner in the parent issue:

+++ b/core/modules/simpletest/src/AssertContentTrait.php
@@ -178,6 +179,10 @@ protected function getUrl() {
+ // @todo fix this
+ if ($value instanceof SafeStringInterface) {
+ $value = (string) $value;
+ }

IMHO for xpath we can always cast to string, right? anything else would not pack anyway, given how xpath works
+++ b/core/modules/simpletest/src/TestBase.php
@@ -654,6 +655,18 @@ protected function assertNotNull($value, $message = '', $group = 'Other') {
+ if (is_array($first)) {
+ array_walk_recursive($first, [$this, 'translationWrapperToString']);
+ }
+ if (is_array($second)) {
+ array_walk_recursive($second, [$this, 'translationWrapperToString']);
+ }

@@ -683,6 +696,15 @@ protected function assertNotEqual($first, $second, $message = '', $group = 'Othe
+ * @todo remove this
+ */
+ public function translationWrapperToString(&$value) {
+ if ($value instanceof SafeStringInterface) {
+ $value = (string) $value;
+ }
+ }
+

What about using strval for iteration?

Upgrading to major as these were split out of a Major issue

Seems we already do something similar in AssertContentTrait::assertThemeOutput() as well:

    // The string cast is necessary because theme functions return
    // SafeStringInterface objects. This means we can assert that $expected
    // matches the theme output without having to worry about 0 == ''.
    $output = (string) $renderer->executeInRenderContext(new RenderContext(), function() use ($callback, $variables) {
      return \Drupal::theme()->render($callback, $variables);
    });

Sadly this can't really be tested, so a few manual reviews would be good.

As to #3, I had rather it keep failing on anything other than SafeStringInterface... And strval the same concern -- I'd rather not blindly cast everything to string.

Agreed...

Jut gave this another look and not much to add here.

We already use SafeStringInterface in various places interchangeably with strings and in the renderer we do quite a bit of string casting as well so it only makes sense for us to cater to this in tests. SafeMarkup::format() may soon only output safe strings anymore for instance.

This is all a bit magical and it will make the equality checks and assertions more lenient but I don't really see where this could currently pose any serious problem. Maybe in the future we (or contrib) will have some wonky objects implementing SafeStringInterface, but it should only be used for value objects anyway, and TranslationWrappers seem like the only possible place where casting is a bit more involved than printing the value. And that is where we actuallly /want/ the string casting as when we make t() return a TranslationWrapper, the string casting in tests will make the BC break less severe as it will make t() behave the way it used to historically.

In #2557113: Make t() return a TranslationWrapper object to remove reliance on a static, unpredictable safe list most of the work went into fixing test fails rather than actual code that broke - without a patch like tere the work would be quite a bit more involved, so this will definitely be a help to contrib as well in having less refactoring to do in dealing with the BC break.

Added comments to clarify - I think we want to keep the string cast in assertEquals() just to be sure, rather than relying on PHP silently casting compared TranslationWrappers to string.

I'd rather err on the side of being more specific in these cases rather than blindly casting - we don't want false positives/negatives or hide any errors when passing in NULL/FALSE or such.

Anyway let's see if this passes tests anyway, these seem like they should be fine.

Converted the ones that were possible -- turned one into an isset (as it could also be NULL) and two into a more lenient "is_object" as they could also be NULL or an array.

Removing the cast from the identical check, we'll have to run $this->castSafeStrings() in the individual tests themselves before identical checks that include output from t()