jenkins-bot has submitted this change and it was merged.
Change subject: Message: Clean up unit tests and improve code coverage
......................................................................
Message: Clean up unit tests and improve code coverage
* Remove unnecessary use of ReflectionClass. It was testing
internal properties that aren't part of the API. Using the
getters instead.
* Remove need for func_get_args that was making the test more
complex and the data provider hard to read. Simply maintain
it as array of expected params and array of variadic arguments.
* Rename tests to more closely match tested methods.
* Rename data providers to provide*, and make them static.
* Reorder tests to more closely match logical order of the class.
* Improve line coverage from 31% to 67%.
Also:
* Remove testParams (dupes testConstructorParams).
* Add tests for RawMessage class.
* Add tests for transformation and parsing.
* Add tests for wfMessage().
* Add tests for Message::newFrom*.
* Add tests for "$*" replacement.
* Add tests for __toString.
Change-Id: I2b183a66f9e9f51bd800088e174b1ae4d3284d8d
---
M includes/Message.php
M tests/phpunit/includes/MessageTest.php
2 files changed, 244 insertions(+), 116 deletions(-)
Approvals:
Nikerabbit: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/Message.php b/includes/Message.php
index 49437f4..134af0e 100644
--- a/includes/Message.php
+++ b/includes/Message.php
@@ -249,7 +249,7 @@
$this->key = reset( $this->keysToTry );
$this->parameters = array_values( $params );
- $this->language = $language ? $language : $wgLang;
+ $this->language = $language ?: $wgLang;
}
/**
diff --git a/tests/phpunit/includes/MessageTest.php
b/tests/phpunit/includes/MessageTest.php
index 4c5424c..99ec2e4 100644
--- a/tests/phpunit/includes/MessageTest.php
+++ b/tests/phpunit/includes/MessageTest.php
@@ -16,22 +16,11 @@
* @dataProvider provideConstructor
*/
public function testConstructor( $expectedLang, $key, $params,
$language ) {
- $reflection = new ReflectionClass( 'Message' );
-
- $keyProperty = $reflection->getProperty( 'key' );
- $keyProperty->setAccessible( true );
-
- $paramsProperty = $reflection->getProperty( 'parameters' );
- $paramsProperty->setAccessible( true );
-
- $langProperty = $reflection->getProperty( 'language' );
- $langProperty->setAccessible( true );
-
$message = new Message( $key, $params, $language );
- $this->assertEquals( $key, $keyProperty->getValue( $message ) );
- $this->assertEquals( $params, $paramsProperty->getValue(
$message ) );
- $this->assertEquals( $expectedLang, $langProperty->getValue(
$message ) );
+ $this->assertEquals( $key, $message->getKey() );
+ $this->assertEquals( $params, $message->getParams() );
+ $this->assertEquals( $expectedLang, $message->getLanguage() );
}
public static function provideConstructor() {
@@ -45,21 +34,62 @@
);
}
- public static function provideTestParams() {
+ public static function provideConstructorParams() {
return array(
- array( array() ),
- array( array( 'foo' ), 'foo' ),
- array( array( 'foo', 'bar' ), 'foo', 'bar' ),
- array( array( 'baz' ), array( 'baz' ) ),
- array( array( 'baz', 'foo' ), array( 'baz', 'foo' ) ),
- array( array( 'baz', 'foo' ), array( 'baz', 'foo' ),
'hhh' ),
- array( array( 'baz', 'foo' ), array( 'baz', 'foo' ),
'hhh', array( 'ahahahahha' ) ),
- array( array( 'baz', 'foo' ), array( 'baz', 'foo' ),
array( 'ahahahahha' ) ),
- array( array( 'baz' ), array( 'baz' ), array(
'ahahahahha' ) ),
+ array(
+ array(),
+ array(),
+ ),
+ array(
+ array( 'foo' ),
+ array( 'foo' ),
+ ),
+ array(
+ array( 'foo', 'bar' ),
+ array( 'foo', 'bar' ),
+ ),
+ array(
+ array( 'baz' ),
+ array( array( 'baz' ) ),
+ ),
+ array(
+ array( 'baz', 'foo' ),
+ array( array( 'baz', 'foo' ) ),
+ ),
+ array(
+ array( 'baz', 'foo' ),
+ array( array( 'baz', 'foo' ), 'hhh' ),
+ ),
+ array(
+ array( 'baz', 'foo' ),
+ array( array( 'baz', 'foo' ), 'hhh', array(
'ahahahahha' ) ),
+ ),
+ array(
+ array( 'baz', 'foo' ),
+ array( array( 'baz', 'foo' ), array(
'ahahahahha' ) ),
+ ),
+ array(
+ array( 'baz' ),
+ array( array( 'baz' ), array( 'ahahahahha' ) ),
+ ),
);
}
- public function getLanguageProvider() {
+ /**
+ * @covers Message::__construct
+ * @covers Message::getParams
+ * @dataProvider provideConstructorParams
+ */
+ public function testConstructorParams( $expected, $args ) {
+ $msg = new Message( 'imasomething' );
+
+ $returned = call_user_func_array( array( $msg, 'params' ),
$args );
+
+ $this->assertSame( $msg, $returned );
+ $this->assertEquals( $expected, $msg->getParams() );
+ }
+
+ public static function provideConstructorLanguage() {
return array(
array( 'foo', array( 'bar' ), 'en' ),
array( 'foo', array( 'bar' ), 'de' )
@@ -67,27 +97,98 @@
}
/**
+ * @covers Message::__construct
* @covers Message::getLanguage
- * @dataProvider getLanguageProvider
+ * @dataProvider provideConstructorLanguage
*/
- public function testGetLanguageCode( $key, $params, $languageCode ) {
+ public function testConstructorLanguage( $key, $params, $languageCode )
{
$language = Language::factory( $languageCode );
$message = new Message( $key, $params, $language );
$this->assertEquals( $language, $message->getLanguage() );
}
+ public static function provideKeys() {
+ return array(
+ 'string' => array(
+ 'key' => 'mainpage',
+ 'expected' => array( 'mainpage' ),
+ ),
+ 'single' => array(
+ 'key' => array( 'mainpage' ),
+ 'expected' => array( 'mainpage' ),
+ ),
+ 'multi' => array(
+ 'key' => array( 'mainpage-foo', 'mainpage-bar',
'mainpage' ),
+ 'expected' => array( 'mainpage-foo',
'mainpage-bar', 'mainpage' ),
+ ),
+ 'empty' => array(
+ 'key' => array(),
+ 'expected' => null,
+ 'exception' => 'InvalidArgumentException',
+ ),
+ 'null' => array(
+ 'key' => null,
+ 'expected' => null,
+ 'exception' => 'InvalidArgumentException',
+ ),
+ 'bad type' => array(
+ 'key' => 123,
+ 'expected' => null,
+ 'exception' => 'InvalidArgumentException',
+ ),
+ );
+ }
+
/**
- * @covers Message::params
- * @dataProvider provideTestParams
+ * @covers Message::__construct
+ * @covers Message::getKey
+ * @covers Message::isMultiKey
+ * @covers Message::getKeysToTry
+ * @dataProvider provideKeys
*/
- public function testParams( $expected ) {
- $msg = new Message( 'imasomething' );
+ public function testKeys( $key, $expected, $exception = null ) {
+ if ( $exception ) {
+ $this->setExpectedException( $exception );
+ }
- $returned = call_user_func_array( array( $msg, 'params' ),
array_slice( func_get_args(), 1 ) );
+ $msg = new Message( $key );
+ $this->assertContains( $msg->getKey(), $expected );
+ $this->assertEquals( $expected, $msg->getKeysToTry() );
+ $this->assertEquals( count( $expected ) > 1, $msg->isMultiKey()
);
+ }
- $this->assertSame( $msg, $returned );
- $this->assertEquals( $expected, $msg->getParams() );
+ /**
+ * @covers ::wfMessage
+ */
+ public function testWfMessage() {
+ $this->assertInstanceOf( 'Message', wfMessage( 'mainpage' ) );
+ $this->assertInstanceOf( 'Message', wfMessage(
'i-dont-exist-evar' ) );
+ }
+
+ /**
+ * @covers Message::newFromKey
+ */
+ public function testNewFromKey() {
+ $this->assertInstanceOf( 'Message', Message::newFromKey(
'mainpage' ) );
+ $this->assertInstanceOf( 'Message', Message::newFromKey(
'i-dont-exist-evar' ) );
+ }
+
+ /**
+ * @covers ::wfMessage
+ * @covers Message::__construct
+ */
+ public function testWfMessageParams() {
+ $this->assertEquals( 'Return to $1.', wfMessage( 'returnto'
)->text() );
+ $this->assertEquals( 'Return to $1.', wfMessage( 'returnto',
array() )->text() );
+ $this->assertEquals(
+ 'You have foo (bar).',
+ wfMessage( 'youhavenewmessages', 'foo', 'bar' )->text()
+ );
+ $this->assertEquals(
+ 'You have foo (bar).',
+ wfMessage( 'youhavenewmessages', array( 'foo', 'bar' )
)->text()
+ );
}
/**
@@ -104,10 +205,12 @@
/**
* @covers Message::__construct
+ * @covers Message::text
+ * @covers Message::plain
+ * @covers Message::escaped
+ * @covers Message::toString
*/
- public function testKey() {
- $this->assertInstanceOf( 'Message', wfMessage( 'mainpage' ) );
- $this->assertInstanceOf( 'Message', wfMessage(
'i-dont-exist-evar' ) );
+ public function testToStringKey() {
$this->assertEquals( 'Main Page', wfMessage( 'mainpage'
)->text() );
$this->assertEquals( '<i-dont-exist-evar>', wfMessage(
'i-dont-exist-evar' )->text() );
$this->assertEquals( '<i<dont>exist-evar>', wfMessage(
'i<dont>exist-evar' )->text() );
@@ -118,6 +221,26 @@
'<i<dont>exist-evar>',
wfMessage( 'i<dont>exist-evar' )->escaped()
);
+ }
+
+ public static function provideToString() {
+ return array(
+ array( 'mainpage', 'Main Page' ),
+ array( 'i-dont-exist-evar', '<i-dont-exist-evar>' ),
+ array( 'i-dont-exist-evar',
'<i-dont-exist-evar>', 'escaped' ),
+ );
+ }
+
+ /**
+ * @covers Message::toString
+ * @covers Message::__toString
+ * @dataProvider provideToString
+ */
+ public function testToString( $key, $expect, $format = 'plain' ) {
+ $msg = new Message( $key );
+ $msg->$format();
+ $this->assertEquals( $expect, $msg->toString() );
+ $this->assertEquals( $expect, $msg->__toString() );
}
/**
@@ -138,26 +261,10 @@
}
/**
- * @covers Message::__construct
- */
- public function testMessageParams() {
- $this->assertEquals( 'Return to $1.', wfMessage( 'returnto'
)->text() );
- $this->assertEquals( 'Return to $1.', wfMessage( 'returnto',
array() )->text() );
- $this->assertEquals(
- 'You have foo (bar).',
- wfMessage( 'youhavenewmessages', 'foo', 'bar' )->text()
- );
- $this->assertEquals(
- 'You have foo (bar).',
- wfMessage( 'youhavenewmessages', array( 'foo', 'bar' )
)->text()
- );
- }
-
- /**
- * @covers Message::__construct
+ * @covers Message::rawParam
* @covers Message::rawParams
*/
- public function testMessageParamSubstitution() {
+ public function testRawParams() {
$this->assertEquals(
'(Заглавная страница)',
wfMessage( 'parentheses', 'Заглавная страница'
)->plain()
@@ -177,10 +284,21 @@
}
/**
- * @covers Message::__construct
- * @covers Message::params
+ * @covers RawMessage::__construct
+ * @covers RawMessage::fetchMessage
*/
- public function testDeliciouslyManyParams() {
+ public function testRawMessage() {
+ $msg = new RawMessage( 'example &' );
+ $this->assertEquals( 'example &', $msg->plain() );
+ $this->assertEquals( 'example &', $msg->escaped() );
+ }
+
+ /**
+ * @covers Message::params
+ * @covers Message::toString
+ * @covers Message::replaceParameters
+ */
+ public function testReplaceManyParams() {
$msg = new RawMessage( '$1$2$3$4$5$6$7$8$9$10$11$12' );
// One less than above has placeholders
$params = array( 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i',
'j', 'k' );
@@ -189,12 +307,20 @@
$msg->params( $params )->plain(),
'Params > 9 are replaced correctly'
);
+
+ $msg = new RawMessage( 'Params$*' );
+ $params = array( 'ab', 'bc', 'cd' );
+ $this->assertEquals(
+ 'Params: ab, bc, cd',
+ $msg->params( $params )->text()
+ );
}
/**
+ * @covers Message::numParam
* @covers Message::numParams
*/
- public function testMessageNumParams() {
+ public function testNumParams() {
$lang = Language::factory( 'en' );
$msg = new RawMessage( '$1' );
@@ -206,9 +332,10 @@
}
/**
+ * @covers Message::durationParam
* @covers Message::durationParams
*/
- public function testMessageDurationParams() {
+ public function testDurationParams() {
$lang = Language::factory( 'en' );
$msg = new RawMessage( '$1' );
@@ -222,9 +349,10 @@
/**
* FIXME: This should not need database, but Language#formatExpiry does
(bug 55912)
* @group Database
+ * @covers Message::expiryParam
* @covers Message::expiryParams
*/
- public function testMessageExpiryParams() {
+ public function testExpiryParams() {
$lang = Language::factory( 'en' );
$msg = new RawMessage( '$1' );
@@ -236,9 +364,10 @@
}
/**
+ * @covers Message::timeperiodParam
* @covers Message::timeperiodParams
*/
- public function testMessageTimeperiodParams() {
+ public function testTimeperiodParams() {
$lang = Language::factory( 'en' );
$msg = new RawMessage( '$1' );
@@ -250,9 +379,10 @@
}
/**
+ * @covers Message::sizeParam
* @covers Message::sizeParams
*/
- public function testMessageSizeParams() {
+ public function testSizeParams() {
$lang = Language::factory( 'en' );
$msg = new RawMessage( '$1' );
@@ -264,9 +394,10 @@
}
/**
+ * @covers Message::bitrateParam
* @covers Message::bitrateParams
*/
- public function testMessageBitrateParams() {
+ public function testBitrateParams() {
$lang = Language::factory( 'en' );
$msg = new RawMessage( '$1' );
@@ -277,7 +408,7 @@
);
}
- public function messagePlaintextParamsProvider() {
+ public static function providePlaintextParams() {
return array(
array(
'one $2 <div>foo</div> [[Bar]] {{Baz}} <',
@@ -308,10 +439,15 @@
}
/**
- * @dataProvider messagePlaintextParamsProvider
+ * @covers Message::plaintextParam
* @covers Message::plaintextParams
+ * @covers Message::formatPlaintext
+ * @covers Message::toString
+ * @covers Message::parse
+ * @covers Message::parseAsBlock
+ * @dataProvider providePlaintextParams
*/
- public function testMessagePlaintextParams( $expect, $format ) {
+ public function testPlaintextParams( $expect, $format ) {
$lang = Language::factory( 'en' );
$msg = new RawMessage( '$1 $2' );
@@ -323,6 +459,46 @@
$expect,
$msg->inLanguage( $lang )->plaintextParams( $params
)->$format(),
"Fail formatting for $format"
+ );
+ }
+
+ public static function provideParser() {
+ return array(
+ array(
+ "''&'' <x><!-- x -->",
+ 'plain',
+ ),
+
+ array(
+ "''&'' <x><!-- x -->",
+ 'text',
+ ),
+ array(
+ '<i>&</i> <x>',
+ 'parse',
+ ),
+
+ array(
+ "<p><i>&</i> <x>\n</p>",
+ 'parseAsBlock',
+ ),
+ );
+ }
+
+ /**
+ * @covers Message::text
+ * @covers Message::parse
+ * @covers Message::parseAsBlock
+ * @covers Message::toString
+ * @covers Message::transformText
+ * @covers Message::parseText
+ * @dataProvider provideParser
+ */
+ public function testParser( $expect, $format ) {
+ $msg = new RawMessage( "''&'' <x><!-- x -->" );
+ $this->assertEquals(
+ $expect,
+ $msg->inLanguage( 'en' )->$format()
);
}
@@ -371,53 +547,5 @@
*/
public function testInLanguageThrows() {
wfMessage( 'foo' )->inLanguage( 123 );
- }
-
- public function keyProvider() {
- return array(
- 'string' => array(
- 'key' => 'mainpage',
- 'expected' => array( 'mainpage' ),
- ),
- 'single' => array(
- 'key' => array( 'mainpage' ),
- 'expected' => array( 'mainpage' ),
- ),
- 'multi' => array(
- 'key' => array( 'mainpage-foo', 'mainpage-bar',
'mainpage' ),
- 'expected' => array( 'mainpage-foo',
'mainpage-bar', 'mainpage' ),
- ),
- 'empty' => array(
- 'key' => array(),
- 'expected' => null,
- 'exception' => 'InvalidArgumentException',
- ),
- 'null' => array(
- 'key' => null,
- 'expected' => null,
- 'exception' => 'InvalidArgumentException',
- ),
- 'bad type' => array(
- 'key' => 17,
- 'expected' => null,
- 'exception' => 'InvalidArgumentException',
- ),
- );
- }
-
- /**
- * @dataProvider keyProvider()
- *
- * @covers Message::getKey
- */
- public function testGetKey( $key, $expected, $exception = null ) {
- if ( $exception ) {
- $this->setExpectedException( $exception );
- }
-
- $msg = new Message( $key );
- $this->assertEquals( $expected, $msg->getKeysToTry() );
- $this->assertEquals( count( $expected ) > 1, $msg->isMultiKey()
);
- $this->assertContains( $msg->getKey(), $expected );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/201422
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2b183a66f9e9f51bd800088e174b1ae4d3284d8d
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Nikerabbit <niklas.laxst...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
