From cad7b8368d6cb08cb2310c8da86a713a992a0350 Mon Sep 17 00:00:00 2001 From: addshore Date: Fri, 15 Nov 2013 15:32:12 +0100 Subject: [PATCH] General Cleanup of some Tests Style Fixes, Comment fixes Change-Id: I675d3f098e81709d5dfd928af6ca54589d3d5fad --- .../phpunit/includes/api/ApiEditPageTest.php | 7 ++++ tests/phpunit/includes/api/ApiOptionsTest.php | 6 +++ .../includes/api/ApiTestCaseUpload.php | 27 +++++++++---- tests/phpunit/includes/api/ApiUploadTest.php | 4 ++ .../includes/api/RandomImageGenerator.php | 40 ++++++++++++------- .../includes/api/query/ApiQueryBasicTest.php | 34 +++++++++------- .../api/query/ApiQueryContinue2Test.php | 2 + .../api/query/ApiQueryContinueTest.php | 2 + .../api/query/ApiQueryContinueTestBase.php | 3 ++ 9 files changed, 89 insertions(+), 36 deletions(-) diff --git a/tests/phpunit/includes/api/ApiEditPageTest.php b/tests/phpunit/includes/api/ApiEditPageTest.php index 8fe08e10b6..c0bf1b7e3e 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 6e5edbf6e4..6659414cb5 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 7e18b6ede0..42f02faea8 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 7bd7207843..ba7fb2561c 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 13b751b5dd..1d534ee4d4 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( '' ); @@ -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 a68c8309bd..9c5b3ca2ad 100644 --- a/tests/phpunit/includes/api/query/ApiQueryBasicTest.php +++ b/tests/phpunit/includes/api/query/ApiQueryBasicTest.php @@ -1,6 +1,5 @@ '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 2116cd396d..347cd6f810 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 7797522fbe..0379790148 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 3b55b13b1b..54f35988d4 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(); -- 2.20.1