General Cleanup of some Tests

Style Fixes, Comment fixes

Change-Id: I675d3f098e81709d5dfd928af6ca54589d3d5fad

9 files changed, 89 insertions, 36 deletions

diff --git a/tests/phpunit/includes/api/ApiEditPageTest.php b/tests/phpunit/includes/api/ApiEditPageTest.php
index 8fe08e10b651..c0bf1b7e3ee1 100644
--- a/tests/phpunit/includes/api/ApiEditPageTest.php
+++ b/tests/phpunit/includes/api/ApiEditPageTest.php
@@ -126,6 +126,9 @@ class ApiEditPageTest extends ApiTestCase {
 		$this->assertEquals( $data, $page->getContent()->serialize() );
 	}
 
+	/**
+	 * @return array
+	 */
 	public static function provideEditAppend() {
 		return array(
 			array( #0: append
@@ -407,6 +410,10 @@ class ApiEditPageTest extends ApiTestCase {
 			"no edit conflict expected here" );
 	}
 
+	/**
+	 * @param WikiPage $page
+	 * @param string|int $timestamp
+	 */
 	protected function forceRevisionDate( WikiPage $page, $timestamp ) {
 		$dbw = wfGetDB( DB_MASTER );
 
diff --git a/tests/phpunit/includes/api/ApiOptionsTest.php b/tests/phpunit/includes/api/ApiOptionsTest.php
index 6e5edbf6e4a6..6659414cb51e 100644
--- a/tests/phpunit/includes/api/ApiOptionsTest.php
+++ b/tests/phpunit/includes/api/ApiOptionsTest.php
@@ -99,6 +99,12 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 		return true;
 	}
 
+	/**
+	 * @param IContextSource $context
+	 * @param array|null $options
+	 *
+	 * @return array
+	 */
 	public function getOptionKinds( IContextSource $context, $options = null ) {
 		// Match with above.
 		$kinds = array(
diff --git a/tests/phpunit/includes/api/ApiTestCaseUpload.php b/tests/phpunit/includes/api/ApiTestCaseUpload.php
index 7e18b6ede0dc..42f02faea810 100644
--- a/tests/phpunit/includes/api/ApiTestCaseUpload.php
+++ b/tests/phpunit/includes/api/ApiTestCaseUpload.php
@@ -29,7 +29,10 @@ abstract class ApiTestCaseUpload extends ApiTestCase {
 
 	/**
 	 * Helper function -- remove files and associated articles by Title
-	 * @param $title Title: title to be removed
+	 *
+	 * @param Title $title title to be removed
+	 *
+	 * @return bool
 	 */
 	public function deleteFileByTitle( $title ) {
 		if ( $title->exists() ) {
@@ -53,7 +56,10 @@ abstract class ApiTestCaseUpload extends ApiTestCase {
 
 	/**
 	 * Helper function -- remove files and associated articles with a particular filename
-	 * @param $fileName String: filename to be removed
+	 *
+	 * @param string $fileName filename to be removed
+	 *
+	 * @return bool
 	 */
 	public function deleteFileByFileName( $fileName ) {
 		return $this->deleteFileByTitle( Title::newFromText( $fileName, NS_FILE ) );
@@ -61,7 +67,10 @@ abstract class ApiTestCaseUpload extends ApiTestCase {
 
 	/**
 	 * Helper function -- given a file on the filesystem, find matching content in the db (and associated articles) and remove them.
-	 * @param $filePath String: path to file on the filesystem
+	 *
+	 * @param string $filePath path to file on the filesystem
+	 *
+	 * @return bool
 	 */
 	public function deleteFileByContent( $filePath ) {
 		$hash = FSFile::getSha1Base36FromPath( $filePath );
@@ -77,10 +86,14 @@ abstract class ApiTestCaseUpload extends ApiTestCase {
 	/**
 	 * Fake an upload by dumping the file into temp space, and adding info to $_FILES.
 	 * (This is what PHP would normally do).
-	 * @param $fieldName String: name this would have in the upload form
-	 * @param $fileName String: name to title this
-	 * @param $type String: mime type
-	 * @param $filePath String: path where to find file contents
+	 *
+	 * @param string $fieldName name this would have in the upload form
+	 * @param string $fileName name to title this
+	 * @param string $type mime type
+	 * @param string $filePath path where to find file contents
+	 *
+	 * @throws Exception
+	 * @return bool
 	 */
 	function fakeUploadFile( $fieldName, $fileName, $type, $filePath ) {
 		$tmpName = tempnam( wfTempDir(), "" );
diff --git a/tests/phpunit/includes/api/ApiUploadTest.php b/tests/phpunit/includes/api/ApiUploadTest.php
index 7bd720784340..ba7fb2561c14 100644
--- a/tests/phpunit/includes/api/ApiUploadTest.php
+++ b/tests/phpunit/includes/api/ApiUploadTest.php
@@ -110,6 +110,7 @@ class ApiUploadTest extends ApiTestCaseUpload {
 			$this->markTestIncomplete( $e->getMessage() );
 		}
 
+		/** @var array $filePaths */
 		$filePath = $filePaths[0];
 		$fileSize = filesize( $filePath );
 		$fileName = basename( $filePath );
@@ -199,6 +200,7 @@ class ApiUploadTest extends ApiTestCaseUpload {
 		}
 
 		// we'll reuse this filename
+		/** @var array $filePaths */
 		$fileName = basename( $filePaths[0] );
 
 		// clear any other files with the same name
@@ -269,6 +271,7 @@ class ApiUploadTest extends ApiTestCaseUpload {
 			$this->markTestIncomplete( $e->getMessage() );
 		}
 
+		/** @var array $filePaths */
 		$fileNames[0] = basename( $filePaths[0] );
 		$fileNames[1] = "SameContentAs" . $fileNames[0];
 
@@ -353,6 +356,7 @@ class ApiUploadTest extends ApiTestCaseUpload {
 			$this->markTestIncomplete( $e->getMessage() );
 		}
 
+		/** @var array $filePaths */
 		$filePath = $filePaths[0];
 		$fileSize = filesize( $filePath );
 		$fileName = basename( $filePath );
diff --git a/tests/phpunit/includes/api/RandomImageGenerator.php b/tests/phpunit/includes/api/RandomImageGenerator.php
index 13b751b5ddd3..1d534ee4d4d4 100644
--- a/tests/phpunit/includes/api/RandomImageGenerator.php
+++ b/tests/phpunit/includes/api/RandomImageGenerator.php
@@ -113,7 +113,11 @@ class RandomImageGenerator {
 
 	/**
 	 * Figure out how we write images. This is a factor of both format and the local system
-	 * @param $format (a typical extension like 'svg', 'jpg', etc.)
+	 *
+	 * @param string $format (a typical extension like 'svg', 'jpg', etc.)
+	 *
+	 * @throws Exception
+	 * @return string
 	 */
 	function getImageWriteMethod( $format ) {
 		global $wgUseImageMagick, $wgImageMagickConvertCommand;
@@ -219,9 +223,12 @@ class RandomImageGenerator {
 	/**
 	 * Based on image specification, write a very simple SVG file to disk.
 	 * Ignores the background spec because transparency is cool. :)
-	 * @param $spec: spec describing background and shapes to draw
-	 * @param $format: file format to write (which is obviously always svg here)
-	 * @param $filename: filename to write to
+	 *
+	 * @param array $spec spec describing background and shapes to draw
+	 * @param string $format file format to write (which is obviously always svg here)
+	 * @param string $filename filename to write to
+	 *
+	 * @throws Exception
 	 */
 	public function writeSvg( $spec, $format, $filename ) {
 		$svg = new SimpleXmlElement( '<svg/>' );
@@ -247,9 +254,9 @@ class RandomImageGenerator {
 
 	/**
 	 * Based on an image specification, write such an image to disk, using Imagick PHP extension
-	 * @param $spec: spec describing background and circles to draw
-	 * @param $format: file format to write
-	 * @param $filename: filename to write to
+	 * @param array $spec spec describing background and circles to draw
+	 * @param string $format file format to write
+	 * @param string $filename filename to write to
 	 */
 	public function writeImageWithApi( $spec, $format, $filename ) {
 		// this is a hack because I can't get setImageOrientation() to work. See below.
@@ -304,7 +311,7 @@ class RandomImageGenerator {
 	 * This is used when simulating a rotated image capture with Exif orientation
 	 * @param $spec Object returned by getImageSpec
 	 * @param $matrix 2x2 transformation matrix
-	 * @return transformed Spec
+	 * @return array transformed Spec
 	 */
 	private static function rotateImageSpec( &$spec, $matrix ) {
 		$tSpec = array();
@@ -361,9 +368,12 @@ class RandomImageGenerator {
 	 *   -draw 'fill rgb(99,123,231) circle 59,39 56,57' \
 	 *   -draw 'fill rgb(240,12,32)  circle 50,21 50,3'  filename.png
 	 *
-	 * @param $spec: spec describing background and shapes to draw
-	 * @param $format: file format to write (unused by this method but kept so it has the same signature as writeImageWithApi)
-	 * @param $filename: filename to write to
+	 * @param array $spec spec describing background and shapes to draw
+	 * @param string $format file format to write (unused by this method but kept so it has the same signature as
+	 * writeImageWithApi)
+	 * @param string $filename filename to write to
+	 *
+	 * @return bool
 	 */
 	public function writeImageWithCommandLine( $spec, $format, $filename ) {
 		global $wgImageMagickConvertCommand;
@@ -388,7 +398,7 @@ class RandomImageGenerator {
 	/**
 	 * Generate a string of random colors for ImageMagick or SVG, like "rgb(12, 37, 98)"
 	 *
-	 * @return {String}
+	 * @return string
 	 */
 	public function getRandomColor() {
 		$components = array();
@@ -422,8 +432,10 @@ class RandomImageGenerator {
 	 *
 	 * Will throw exception if the file could not be read or if it had fewer lines than requested.
 	 *
-	 * @param $number_desired Integer: number of lines desired
-	 * @return Array: of exactly n elements, drawn randomly from lines the file
+	 * @param int $number_desired number of lines desired
+	 *
+	 * @throws Exception
+	 * @return array of exactly n elements, drawn randomly from lines the file
 	 */
 	private function getRandomLines( $number_desired ) {
 		$filepath = $this->dictionaryFile;
diff --git a/tests/phpunit/includes/api/query/ApiQueryBasicTest.php b/tests/phpunit/includes/api/query/ApiQueryBasicTest.php
index a68c8309bd13..9c5b3ca2ade1 100644
--- a/tests/phpunit/includes/api/query/ApiQueryBasicTest.php
+++ b/tests/phpunit/includes/api/query/ApiQueryBasicTest.php
@@ -1,7 +1,6 @@
 <?php
 /**
  *
- *
  * Created on Feb 6, 2013
  *
  * Copyright © 2013 Yuri Astrakhan "<Firstname><Lastname>@gmail.com"
@@ -35,6 +34,8 @@ require_once 'ApiQueryTestBase.php';
  * @covers ApiQuery
  */
 class ApiQueryBasicTest extends ApiQueryTestBase {
+	protected $exceptionFromAddDBData;
+
 	/**
 	 * Create a set of pages. These must not change, otherwise the tests might give wrong results.
 	 * @see MediaWikiTestCase::addDBData()
@@ -130,6 +131,7 @@ class ApiQueryBasicTest extends ApiQueryTestBase {
 		) )
 	);
 
+	// Although this appears to have no use it is used by testLists()
 	private static $allcategories = array(
 		array( 'list' => 'allcategories', 'acprefix' => 'AQBT-' ),
 		array( 'allcategories' => array(
@@ -236,6 +238,7 @@ class ApiQueryBasicTest extends ApiQueryTestBase {
 		$this->check( self::$alllinks );
 		$this->check( self::$alltransclusions );
 		// This test is temporarily disabled until a sqlite bug is fixed
+		// Confirmed still broken 15-nov-2013
 		// $this->check( self::$allcategories );
 		$this->check( self::$backlinks );
 		$this->check( self::$embeddedin );
@@ -367,29 +370,30 @@ class ApiQueryBasicTest extends ApiQueryTestBase {
 
 	/**
 	 * Recursively compare arrays, ignoring mismatches in numeric key and pageids.
-	 * @param $expected array expected values
-	 * @param $result array returned values
+	 *
+	 * @param $expectedArray array expected values
+	 * @param $resultArray array returned values
 	 */
-	private function assertQueryResults( $expected, $result ) {
-		reset( $expected );
-		reset( $result );
+	private function assertQueryResults( $expectedArray, $resultArray ) {
+		reset( $expectedArray );
+		reset( $resultArray );
 		while ( true ) {
-			$e = each( $expected );
-			$r = each( $result );
+			$expectedValue = each( $expectedArray );
+			$resultValue = each( $resultArray );
 			// If either of the arrays is shorter, abort. If both are done, success.
-			$this->assertEquals( (bool)$e, (bool)$r );
-			if ( !$e ) {
+			$this->assertEquals( (bool)$expectedValue, (bool)$resultValue );
+			if ( !$expectedValue ) {
 				break; // done
 			}
 			// continue only if keys are identical or both keys are numeric
-			$this->assertTrue( $e['key'] === $r['key'] || ( is_numeric( $e['key'] ) && is_numeric( $r['key'] ) ) );
+			$this->assertTrue( $expectedValue['key'] === $resultValue['key'] || ( is_numeric( $expectedValue['key'] ) && is_numeric( $resultValue['key'] ) ) );
 			// don't compare pageids
-			if ( $e['key'] !== 'pageid' ) {
+			if ( $expectedValue['key'] !== 'pageid' ) {
 				// If values are arrays, compare recursively, otherwise compare with ===
-				if ( is_array( $e['value'] ) && is_array( $r['value'] ) ) {
-					$this->assertQueryResults( $e['value'], $r['value'] );
+				if ( is_array( $expectedValue['value'] ) && is_array( $resultValue['value'] ) ) {
+					$this->assertQueryResults( $expectedValue['value'], $resultValue['value'] );
 				} else {
-					$this->assertEquals( $e['value'], $r['value'] );
+					$this->assertEquals( $expectedValue['value'], $resultValue['value'] );
 				}
 			}
 		}
diff --git a/tests/phpunit/includes/api/query/ApiQueryContinue2Test.php b/tests/phpunit/includes/api/query/ApiQueryContinue2Test.php
index 2116cd396dd4..347cd6f81067 100644
--- a/tests/phpunit/includes/api/query/ApiQueryContinue2Test.php
+++ b/tests/phpunit/includes/api/query/ApiQueryContinue2Test.php
@@ -27,6 +27,8 @@ require_once 'ApiQueryContinueTestBase.php';
  * @covers ApiQuery
  */
 class ApiQueryContinue2Test extends ApiQueryContinueTestBase {
+	protected $exceptionFromAddDBData;
+
 	/**
 	 * Create a set of pages. These must not change, otherwise the tests might give wrong results.
 	 * @see MediaWikiTestCase::addDBData()
diff --git a/tests/phpunit/includes/api/query/ApiQueryContinueTest.php b/tests/phpunit/includes/api/query/ApiQueryContinueTest.php
index 7797522fbe00..037979014891 100644
--- a/tests/phpunit/includes/api/query/ApiQueryContinueTest.php
+++ b/tests/phpunit/includes/api/query/ApiQueryContinueTest.php
@@ -31,6 +31,8 @@ require_once 'ApiQueryContinueTestBase.php';
  * @covers ApiQuery
  */
 class ApiQueryContinueTest extends ApiQueryContinueTestBase {
+	protected $exceptionFromAddDBData;
+
 	/**
 	 * Create a set of pages. These must not change, otherwise the tests might give wrong results.
 	 * @see MediaWikiTestCase::addDBData()
diff --git a/tests/phpunit/includes/api/query/ApiQueryContinueTestBase.php b/tests/phpunit/includes/api/query/ApiQueryContinueTestBase.php
index 3b55b13b1be3..54f35988d444 100644
--- a/tests/phpunit/includes/api/query/ApiQueryContinueTestBase.php
+++ b/tests/phpunit/includes/api/query/ApiQueryContinueTestBase.php
@@ -110,6 +110,9 @@ abstract class ApiQueryContinueTestBase extends ApiQueryTestBase {
 		} while ( true );
 	}
 
+	/**
+	 * @param array $data
+	 */
 	private function printResult( $data ) {
 		$q = $data['query'];
 		$print = array();