In 279fd16bab, 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 3d92cb2f82, 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
7d9ca602db