UserAuthority: Fix wikitext escaping for block errors (again)

In 279fd16bab170eba95158a7cf438cfac17ff37b5, I made what I thought
was a trivial change to UserAuthorityTest:

(diff slightly modified here for clarity)
-		$message = $permissionStatus->getErrors()[2]['message'];
+		$message = $permissionStatus->getMessages()[2];
 		$this->assertArrayEquals(
 			$this->getFakeBlockMessageParams(),
 			$message->getParams()
 		);

And in 3d92cb2f824b333eef876211ad257478d1e8aa21, I made what I thought
was also a trivial change to UserAuthority:

(diff slightly modified here for clarity, likewise)
-			foreach ( $errors as $err ) {
-				$status->fatal( wfMessage( ...$err ) );
-			}
+			$status->merge( $tempStatus );

However, it turns out these two pieces of code had vital roles:

* The code in UserAuthority ensured that the final status contains
  Message objects instead of key strings + parameter arrays, and thus
  does not trigger wikitext escaping in a legacy code path (T368821).

* The code in UserAuthorityTest accessed the internals of the same
  status with (now deprecated) getErrors() to check that it indeed
  contained a Message object, rather then a key string, which would
  cause a test failure due to a fatal error in the code below.
  getMessages() returns objects regardless of what's inside the
  status, so the test never fails.

Thus I managed to disarm the regression test, and then cause exactly
the regression it was supposed to prevent: block error messages on
Special:CreateAccount have parameters shown as wikitext (T306494).

Restore a foreach loop instead of `$status->merge()` to fix that, and
document why it is there. Change the test so that it actually runs
the code whose behavior it wants to verify, instead of a related but
different method, hopefully making it more resilient against future
developers.

(I found the bug because the test started failing with the refactoring
I'm trying to do in I625a48a6ecd3fad5c2ed76b23343a0fef91e1b83.)

Bug: T306494
Change-Id: I7601fc51702cb33ef9d2b341ea555dc230d31537

1 files changed, 12 insertions, 10 deletions

diff --git a/tests/phpunit/unit/includes/GlobalFunctions/WfEscapeWikiTextTest.php b/tests/phpunit/unit/includes/GlobalFunctions/WfEscapeWikiTextTest.php
index ed3b37fcf4ed..c9de634b8fe4 100644
--- a/tests/phpunit/unit/includes/GlobalFunctions/WfEscapeWikiTextTest.php
+++ b/tests/phpunit/unit/includes/GlobalFunctions/WfEscapeWikiTextTest.php
@@ -14,16 +14,18 @@ class WfEscapeWikiTextTest extends MediaWikiUnitTestCase {
 		$old = $wgEnableMagicLinks;
 		$wgEnableMagicLinks = [];
 
-		$actual = wfEscapeWikiText( $input );
-		// Sanity check that the output can be decoded back to the input
-		// input as well.
-		$decoded = html_entity_decode( $actual, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5 );
-		$this->assertEquals( $decoded, (string)$input );
-		// And that the output was what we expected
-		$this->assertEquals( $expected, $actual );
-
-		// restore global
-		$wgEnableMagicLinks = $old;
+		try {
+			$actual = wfEscapeWikiText( $input );
+			// Sanity check that the output can be decoded back to the input
+			// input as well.
+			$decoded = html_entity_decode( $actual, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5 );
+			$this->assertEquals( $decoded, (string)$input );
+			// And that the output was what we expected
+			$this->assertEquals( $expected, $actual );
+		} finally {
+			// restore global
+			$wgEnableMagicLinks = $old;
+		}
 	}
 
 	public function provideEscape() {