1 files changed, 87 insertions, 90 deletions

diff --git a/tests/phpunit/skins/SideBarTest.php b/tests/phpunit/skins/SideBarTest.php
index 912d7602..a385320f 100644
--- a/tests/phpunit/skins/SideBarTest.php
+++ b/tests/phpunit/skins/SideBarTest.php
@@ -5,15 +5,14 @@
  */
 class SideBarTest extends MediaWikiLangTestCase {
 
-	/** A skin template, reinitialized before each test */
+	/**
+	 * A skin template, reinitialized before each test
+	 * @var SkinTemplate
+	 */
 	private $skin;
 	/** Local cache for sidebar messages */
 	private $messages;
 
-	function __construct() {
-		parent::__construct();
-	}
-
 	/** Build $this->messages array */
 	private function initMessagesHref() {
 		# List of default messages for the sidebar:
@@ -26,29 +25,26 @@ class SideBarTest extends MediaWikiLangTestCase {
 			'helppage',
 		);
 
-		foreach( $URL_messages as $m ) {
-			$titleName = MessageCache::singleton()->get($m);
+		foreach ( $URL_messages as $m ) {
+			$titleName = MessageCache::singleton()->get( $m );
 			$title = Title::newFromText( $titleName );
 			$this->messages[$m]['href'] = $title->getLocalURL();
 		}
 	}
 
-	function setUp() {
+	protected function setUp() {
 		parent::setUp();
 		$this->initMessagesHref();
 		$this->skin = new SkinTemplate();
 		$this->skin->getContext()->setLanguage( Language::factory( 'en' ) );
 	}
-	function tearDown() {
-		parent::tearDown();
-		$this->skin = null;
-	}
 
 	/**
 	 * Internal helper to test the sidebar
 	 * @param $expected
 	 * @param $text
 	 * @param $message (Default: '')
+	 * @todo this assert method to should be converted to a test using a dataprovider..
 	 */
 	private function assertSideBar( $expected, $text, $message = '' ) {
 		$bar = array();
@@ -56,93 +52,101 @@ class SideBarTest extends MediaWikiLangTestCase {
 		$this->assertEquals( $expected, $bar, $message );
 	}
 
-	function testSidebarWithOnlyTwoTitles() {
+	/**
+	 * @covers SkinTemplate::addToSidebarPlain
+	 */
+	public function testSidebarWithOnlyTwoTitles() {
 		$this->assertSideBar(
-		array(
-			'Title1' => array(),
-			'Title2' => array(),
-		),
-'* Title1
+			array(
+				'Title1' => array(),
+				'Title2' => array(),
+			),
+			'* Title1
 * Title2
 '
 		);
 	}
 
-	function testExpandMessages() {
+	/**
+	 * @covers SkinTemplate::addToSidebarPlain
+	 */
+	public function testExpandMessages() {
 		$this->assertSidebar(
-		array( 'Title' => array(
-			array(
-				'text' => 'Help',
-				'href' => $this->messages['helppage']['href'],
-				'id' => 'n-help',
-				'active' => null
-			)
-		)),
-'* Title
+			array( 'Title' => array(
+				array(
+					'text' => 'Help',
+					'href' => $this->messages['helppage']['href'],
+					'id' => 'n-help',
+					'active' => null
+				)
+			) ),
+			'* Title
 ** helppage|help
 '
 		);
 	}
 
-	function testExternalUrlsRequireADescription() {
+	/**
+	 * @covers SkinTemplate::addToSidebarPlain
+	 */
+	public function testExternalUrlsRequireADescription() {
 		$this->assertSidebar(
-		array( 'Title' => array(
-			# ** http://www.mediawiki.org/| Home
-			array(
-				'text'   => 'Home',
-				'href'   => 'http://www.mediawiki.org/',
-				'id'     => 'n-Home',
-				'active' => null,
-				'rel'    => 'nofollow',
-			),
-			# ** http://valid.no.desc.org/
-			# ... skipped since it is missing a pipe with a description
-		)),
-'* Title
+			array( 'Title' => array(
+				# ** http://www.mediawiki.org/| Home
+				array(
+					'text' => 'Home',
+					'href' => 'http://www.mediawiki.org/',
+					'id' => 'n-Home',
+					'active' => null,
+					'rel' => 'nofollow',
+				),
+				# ** http://valid.no.desc.org/
+				# ... skipped since it is missing a pipe with a description
+			) ),
+			'* Title
 ** http://www.mediawiki.org/| Home
 ** http://valid.no.desc.org/
 '
-
 		);
-
 	}
+
 	/**
 	 * bug 33321 - Make sure there's a | after transforming.
 	 * @group Database
+	 * @covers SkinTemplate::addToSidebarPlain
 	 */
-	function testTrickyPipe() {
+	public function testTrickyPipe() {
 		$this->assertSidebar(
-		array( 'Title' => array(
-			# The first 2 are skipped
-			# Doesn't really test the url properly
-			# because it will vary with $wgArticlePath et al.
-			# ** Baz|Fred
-			array(
-				'text'   => 'Fred',
-				'href'   => Title::newFromText( 'Baz' )->getLocalUrl(),
-				'id'     => 'n-Fred',
-				'active' => null,
-			),
-			array(
-				'text'   => 'title-to-display',
-				'href'   => Title::newFromText( 'page-to-go-to' )->getLocalUrl(),
-				'id'     => 'n-title-to-display',
-				'active' => null,
-			),
-		)),
-'* Title
+			array( 'Title' => array(
+				# The first 2 are skipped
+				# Doesn't really test the url properly
+				# because it will vary with $wgArticlePath et al.
+				# ** Baz|Fred
+				array(
+					'text' => 'Fred',
+					'href' => Title::newFromText( 'Baz' )->getLocalURL(),
+					'id' => 'n-Fred',
+					'active' => null,
+				),
+				array(
+					'text' => 'title-to-display',
+					'href' => Title::newFromText( 'page-to-go-to' )->getLocalURL(),
+					'id' => 'n-title-to-display',
+					'active' => null,
+				),
+			) ),
+			'* Title
 ** {{PAGENAME|Foo}}
 ** Bar
 ** Baz|Fred
 ** {{PLURAL:1|page-to-go-to{{int:pipe-separator/en}}title-to-display|branch not taken}}
 '
 		);
-
 	}
 
 
 	#### Attributes for external links ##########################
-	private function getAttribs( ) {
+	private function getAttribs() {
 		# Sidebar text we will use everytime
 		$text = '* Title
 ** http://www.mediawiki.org/| Home';
@@ -155,11 +159,12 @@ class SideBarTest extends MediaWikiLangTestCase {
 
 	/**
 	 * Simple test to verify our helper assertAttribs() is functional
-	 * Please note this assume MediaWiki default settings:
-	 *   $wgNoFollowLinks = true
-	 *   $wgExternalLinkTarget = false
 	 */
-	function testTestAttributesAssertionHelper() {
+	public function testTestAttributesAssertionHelper() {
+		$this->setMwGlobals( array(
+			'wgNoFollowLinks' => true,
+			'wgExternalLinkTarget' => false,
+		) );
 		$attribs = $this->getAttribs();
 
 		$this->assertArrayHasKey( 'rel', $attribs );
@@ -171,39 +176,31 @@ class SideBarTest extends MediaWikiLangTestCase {
 	/**
 	 * Test $wgNoFollowLinks in sidebar
 	 */
-	function testRespectWgnofollowlinks() {
-		global $wgNoFollowLinks;
-		$saved = $wgNoFollowLinks;
-		$wgNoFollowLinks = false;
+	public function testRespectWgnofollowlinks() {
+		$this->setMwGlobals( 'wgNoFollowLinks', false );
 
 		$attribs = $this->getAttribs();
 		$this->assertArrayNotHasKey( 'rel', $attribs,
 			'External URL in sidebar do not have rel=nofollow when $wgNoFollowLinks = false'
 		);
-
-		// Restore global
-		$wgNoFollowLinks = $saved;
 	}
 
 	/**
 	 * Test $wgExternaLinkTarget in sidebar
+	 * @dataProvider dataRespectExternallinktarget
 	 */
-	function testRespectExternallinktarget() {
-		global $wgExternalLinkTarget;
-		$saved = $wgExternalLinkTarget;
-
-		$wgExternalLinkTarget = '_blank';
-		$attribs = $this->getAttribs();
-		$this->assertArrayHasKey( 'target', $attribs );
-		$this->assertEquals( $attribs['target'], '_blank' );
+	public function testRespectExternallinktarget( $externalLinkTarget ) {
+		$this->setMwGlobals( 'wgExternalLinkTarget', $externalLinkTarget );
 
-		$wgExternalLinkTarget = '_self';
 		$attribs = $this->getAttribs();
 		$this->assertArrayHasKey( 'target', $attribs );
-		$this->assertEquals( $attribs['target'], '_self' );
-
-		// Restore global
-		$wgExternalLinkTarget = $saved;
+		$this->assertEquals( $attribs['target'], $externalLinkTarget );
 	}
 
+	public static function dataRespectExternallinktarget() {
+		return array(
+			array( '_blank' ),
+			array( '_self' ),
+		);
+	}
 }