From: Thiemo Mättig <thiemo.maettig@wikimedia.de>
Date: Thu, 6 Mar 2014 20:16:27 +0000 (+0100)
Subject: Fix Status::getMessage accidentially returning string instead of Message
X-Git-Tag: 1.31.0-rc.0~16582^2
X-Git-Url: http://git.cyclocoop.org/data/%24oldEdit?a=commitdiff_plain;h=2dd32c82ffe5677dec64933c06e0ddbfa2d7c476;p=lhc%2Fweb%2Fwiklou.git

Fix Status::getMessage accidentially returning string instead of Message

This mistake was introduced in commit
92e284d3fa62f45e20fed34c4359c575481d583c and the reason for the two
disabled tests. I did not enabled the second test because of an
unrelated problem. The first enabled test already covers the fix.

The method should return Message objects only but did return a
string in that special case (multiple warnings set but no context
message key).

Unfortunatelly Status::getMessage does have many, many more
problems but I understand it's not a good idea to address them all
in a single confusing patch.

Change-Id: I0dc37e248f407019d5921aaaca3eabba338b0fd3
---

diff --git a/includes/Status.php b/includes/Status.php
index e11ba03bdf..795fd4572a 100644
--- a/includes/Status.php
+++ b/includes/Status.php
@@ -224,9 +224,11 @@ class Status {
 	/**
 	 * Get the error list as a Message object
 	 *
-	 * @param string $shortContext a short enclosing context message name, to
-	 *        be used when there is a single error
-	 * @param string $longContext a long enclosing context message name, for a list
+	 * @param string|string[] $shortContext A short enclosing context message name (or an array of
+	 * message names), to be used when there is a single error.
+	 * @param string|string[] $longContext A long enclosing context message name (or an array of
+	 * message names), for a list.
+	 *
 	 * @return Message
 	 */
 	public function getMessage( $shortContext = false, $longContext = false ) {
@@ -256,13 +258,13 @@ class Status {
 				$msgCount++;
 			}
 
-			$wrapper = new RawMessage( '* $' . implode( "\n* \$", range( 1, $msgCount ) ) );
-			$s = $wrapper->params( $msgs )->parse();
+			$s = new RawMessage( '* $' . implode( "\n* \$", range( 1, $msgCount ) ) );
+			$s->params( $msgs )->parse();
 
 			if ( $longContext ) {
-				$s = wfMessage( $longContext, $wrapper );
+				$s = wfMessage( $longContext, $s );
 			} elseif ( $shortContext ) {
-				$wrapper = new RawMessage( "\n\$1\n", $wrapper );
+				$wrapper = new RawMessage( "\n\$1\n", $s );
 				$wrapper->parse();
 				$s = wfMessage( $shortContext, $wrapper );
 			}
diff --git a/tests/phpunit/includes/StatusTest.php b/tests/phpunit/includes/StatusTest.php
index 209b54c3e6..b0e6d20bfa 100644
--- a/tests/phpunit/includes/StatusTest.php
+++ b/tests/phpunit/includes/StatusTest.php
@@ -376,14 +376,15 @@ class StatusTest extends MediaWikiLangTestCase {
 	public function testGetMessage( Status $status, $expectedParams = array(), $expectedKey ) {
 		$message = $status->getMessage();
 		$this->assertInstanceOf( 'Message', $message );
-		$this->assertEquals( $expectedParams, $message->getParams() );
-		$this->assertEquals( $expectedKey, $message->getKey() );
+		$this->assertEquals( $expectedParams, $message->getParams(), 'Message::getParams' );
+		$this->assertEquals( $expectedKey, $message->getKey(), 'Message::getKey' );
 	}
 
 	/**
 	 * @return array of arrays with values;
 	 *    0 => status object
-	 *    1 => expected Message Params (with no context)
+	 *    1 => expected Message parameters (with no context)
+	 *    2 => expected Message key
 	 */
 	public static function provideGetMessage() {
 		$testCases = array();
@@ -407,17 +408,21 @@ class StatusTest extends MediaWikiLangTestCase {
 		$testCases[ '1StringWarning' ] = array(
 			$status,
 			array(),
-			"fooBar!"
+			'fooBar!'
 		);
 
-		//NOTE: this seems to return a string instead of a Message object...
+		// FIXME: Assertion tries to compare a StubUserLang with a Language object, because
+		// "data providers are executed before both the call to the setUpBeforeClass static method
+		// and the first call to the setUp method. Because of that you can't access any variables
+		// you create there from within a data provider."
+		// http://phpunit.de/manual/3.7/en/writing-tests-for-phpunit.html
 //		$status = new Status();
 //		$status->warning( 'fooBar!' );
 //		$status->warning( 'fooBar2!' );
 //		$testCases[ '2StringWarnings' ] = array(
 //			$status,
-//			array(),
-//			''
+//			array( new Message( 'fooBar!' ), new Message( 'fooBar2!' ) ),
+//			"* \$1\n* \$2"
 //		);
 
 		$status = new Status();
@@ -425,18 +430,17 @@ class StatusTest extends MediaWikiLangTestCase {
 		$testCases[ '1MessageWarning' ] = array(
 			$status,
 			array( 'foo', 'bar' ),
-			"fooBar!",
+			'fooBar!'
 		);
 
-		//NOTE: this seems to return a string instead of a Message object...
-//		$status = new Status();
-//		$status->warning( new Message( 'fooBar!', array( 'foo', 'bar' ) ) );
-//		$status->warning( new Message( 'fooBar2!' ) );
-//		$testCases[ '2MessageWarnings' ] = array(
-//			$status,
-//			array(),
-//			"",
-//		);
+		$status = new Status();
+		$status->warning( new Message( 'fooBar!', array( 'foo', 'bar' ) ) );
+		$status->warning( new Message( 'fooBar2!' ) );
+		$testCases[ '2MessageWarnings' ] = array(
+			$status,
+			array( new Message( 'fooBar!', array( 'foo', 'bar' ) ), new Message( 'fooBar2!' ) ),
+			"* \$1\n* \$2"
+		);
 
 		return $testCases;
 	}