Make the test suite pass on Drupal10/PHP8.1

• Changed all the old assertions where needed
• Removed all the message logic, since these were producing old style Simpletest messages (which are possitive messages, there PHPUnit expect negative messages), we should remove all the messages in method calls too, but wanted to see if others agree if this was a good place to change this
• testDrupal8ShippedConfiguration gives an additional error now, so added that error to the test to make it green, not sure if modifying the test is right here or that this error shouldn't be raised and we need to fix this in the code under test

I think the YAML parsing error is new in Drupal 10 due to the more strict parsing? Or is that our own error?

Re changing the message handling, are our assert*() methods used with message arguments? Sounds like an API change if we are invoking it with explicit messages and now ignoring them?

The new error comes from Symfony, see #3055189: [Symfony 5] Support for mapping keys in multi-line blocks is deprecated since Symfony 4.3 and will throw a ParseException in 5.0. → for the core issue. So that sounds like it would be ok to add the new error in the test.

The ignored $messages are all in private methods, so nothing outside this test can access them, so don't think this would be an API change. But I can totally get that we would want to make the change as small as possible here. The $group parameter is not something we can support though, cause that was a Simpletest specific thing in assertions.

+++ b/tests/src/Kernel/PotxTest.php
@@ -748,70 +753,49 @@ class TestConstraint {
   /**
    * Assert the lack of a msgid with context in the .po file.
    */
   private function assertNoMsgIdContext($string, $context, $message = '', $group = 'Other') {
-    if (!$message) {
-      $message = new FormattableMarkup('No MsgID "@raw" in context "@context" found', ['@raw' => $string, '@context' => $context]);
-    }
-    $this->assert(strpos($this->potx_output, 'msgid "' . _potx_format_quoted_string('"' . $string . '"') . '"' . "\nmsgctxt \"" . _potx_format_quoted_string('"' . $context . '"') . '"') === FALSE, $message, $group);
+    $this->assertFalse(strpos($this->potx_output, 'msgid "' . _potx_format_quoted_string('"' . $string . '"') . '"' . "\nmsgctxt \"" . _potx_format_quoted_string('"' . $context . '"') . '"'));
   }

Well, what I meant is that before this method may have been invoked with a $message which is now ignored. So any uses of it internal in this class would not effectively use that message.

On top of that the reason we had the custom messages was easier debugging, you got an exact message about which string was not found or found but should not have been there. Now this requires a debug backtrace to find as you only get an "anonymous" assert result.

If phpunit wants us to swap the positivity of the message, should we adapt to that instead?

Agreed, let's keep the messages in, think we should just do minimal cleanup here to make it green.

So this just changes the assertions to use the correct ones, it does remove the $group since that isn't supported in the new assertions.
Also still removes outputScreenContents since that isn't used anymore and was providing a workaround for a simpletest bug

Didn't change the messages here, they have been "wrong" since this was converted to phpunit and even with the wrong positivity they are still better than 'failed asserting true is false'. If we care about getting those right we could probably do it in a follow up right?

Agreed, this looks great, thanks!

Automatically closed - issue fixed for 2 weeks with no activity.