Inline trivial object creation helpers in tests
authorThiemo Kreuz <thiemo.kreuz@wikimedia.de>
Tue, 28 May 2019 14:11:30 +0000 (16:11 +0200)
committerThiemo Kreuz <thiemo.kreuz@wikimedia.de>
Tue, 28 May 2019 14:12:28 +0000 (16:12 +0200)
I would like to argue that these named helper functions don't make the
code easier to read. It reads the same as before, just with less code.

Change-Id: I375e970b00ac126a47d5f1b49d4e45ff9f9309fb

tests/phpunit/includes/ActorMigrationTest.php
tests/phpunit/includes/TitleArrayFromResultTest.php
tests/phpunit/includes/import/ImportTest.php
tests/phpunit/includes/user/UserArrayFromResultTest.php

index de70f26..40c45dc 100644 (file)
@@ -18,15 +18,6 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
                'actor',
        ];
 
-       /**
-        * Create an ActorMigration for a particular stage
-        * @param int $stage
-        * @return ActorMigration
-        */
-       protected function makeMigration( $stage ) {
-               return new ActorMigration( $stage );
-       }
-
        /**
         * @dataProvider provideConstructor
         * @param int $stage
@@ -81,7 +72,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
         * @param array $expect
         */
        public function testGetJoin( $stage, $key, $expect ) {
-               $m = $this->makeMigration( $stage );
+               $m = new ActorMigration( $stage );
                $result = $m->getJoin( $key );
                $this->assertEquals( $expect, $result );
        }
@@ -260,7 +251,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
                        $users = reset( $users );
                }
 
-               $m = $this->makeMigration( $stage );
+               $m = new ActorMigration( $stage );
                $result = $m->getWhere( $this->db, $key, $users, $useId );
                $this->assertEquals( $expect, $result );
        }
@@ -510,7 +501,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
                                $extraFields['ipb_address'] = __CLASS__ . "#{$stageNames[$writeStage]}";
                        }
 
-                       $w = $this->makeMigration( $writeStage );
+                       $w = new ActorMigration( $writeStage );
                        $usesTemp = $key === 'rev_user';
 
                        if ( $usesTemp ) {
@@ -543,7 +534,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
                        }
 
                        foreach ( $possibleReadStages as $readStage ) {
-                               $r = $this->makeMigration( $readStage );
+                               $r = new ActorMigration( $readStage );
 
                                $queryInfo = $r->getJoin( $key );
                                $row = $this->db->selectRow(
@@ -615,7 +606,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
         * @expectedExceptionMessage Must use getInsertValuesWithTempTable() for rev_user
         */
        public function testInsertWrong( $stage ) {
-               $m = $this->makeMigration( $stage );
+               $m = new ActorMigration( $stage );
                $m->getInsertValues( $this->db, 'rev_user', $this->getTestUser()->getUser() );
        }
 
@@ -626,7 +617,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
         * @expectedExceptionMessage Must use getInsertValues() for rc_user
         */
        public function testInsertWithTempTableWrong( $stage ) {
-               $m = $this->makeMigration( $stage );
+               $m = new ActorMigration( $stage );
                $m->getInsertValuesWithTempTable( $this->db, 'rc_user', $this->getTestUser()->getUser() );
        }
 
@@ -639,7 +630,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
                $wrap->formerTempTables += [ 'rc_user' => '1.30' ];
 
                $this->hideDeprecated( 'ActorMigration::getInsertValuesWithTempTable for rc_user' );
-               $m = $this->makeMigration( $stage );
+               $m = new ActorMigration( $stage );
                list( $fields, $callback )
                        = $m->getInsertValuesWithTempTable( $this->db, 'rc_user', $this->getTestUser()->getUser() );
                $this->assertTrue( is_callable( $callback ) );
@@ -652,7 +643,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
         * @expectedExceptionMessage $extra[rev_timestamp] is not provided
         */
        public function testInsertWithTempTableCallbackMissingFields( $stage ) {
-               $m = $this->makeMigration( $stage );
+               $m = new ActorMigration( $stage );
                list( $fields, $callback )
                        = $m->getInsertValuesWithTempTable( $this->db, 'rev_user', $this->getTestUser()->getUser() );
                $callback( 1, [] );
@@ -677,7 +668,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
 
                list( $cFields, $cCallback ) = MediaWikiServices::getInstance()->getCommentStore()
                        ->insertWithTempTable( $this->db, 'rev_comment', '' );
-               $m = $this->makeMigration( $stage );
+               $m = new ActorMigration( $stage );
                list( $fields, $callback ) =
                        $m->getInsertValuesWithTempTable( $this->db, 'rev_user', $userIdentity );
                $extraFields = [
@@ -701,7 +692,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
                        (int)$row->rev_actor
                );
 
-               $m = $this->makeMigration( $stage );
+               $m = new ActorMigration( $stage );
                $fields = $m->getInsertValues( $this->db, 'dummy_user', $userIdentity );
                if ( $stage & SCHEMA_COMPAT_WRITE_OLD ) {
                        $this->assertSame( $user->getId(), $fields['dummy_user'] );
@@ -730,7 +721,7 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
         * @param string $isNotAnon
         */
        public function testIsAnon( $stage, $isAnon, $isNotAnon ) {
-               $m = $this->makeMigration( $stage );
+               $m = new ActorMigration( $stage );
                $this->assertSame( $isAnon, $m->isAnon( 'foo' ) );
                $this->assertSame( $isNotAnon, $m->isNotAnon( 'foo' ) );
        }
index af49ecf..32c7571 100644 (file)
@@ -30,10 +30,6 @@ class TitleArrayFromResultTest extends PHPUnit\Framework\TestCase {
                return $row;
        }
 
-       private function getTitleArrayFromResult( $resultWrapper ) {
-               return new TitleArrayFromResult( $resultWrapper );
-       }
-
        /**
         * @covers TitleArrayFromResult::__construct
         */
@@ -41,7 +37,7 @@ class TitleArrayFromResultTest extends PHPUnit\Framework\TestCase {
                $row = false;
                $resultWrapper = $this->getMockResultWrapper( $row );
 
-               $object = $this->getTitleArrayFromResult( $resultWrapper );
+               $object = new TitleArrayFromResult( $resultWrapper );
 
                $this->assertEquals( $resultWrapper, $object->res );
                $this->assertSame( 0, $object->key );
@@ -57,7 +53,7 @@ class TitleArrayFromResultTest extends PHPUnit\Framework\TestCase {
                $row = $this->getRowWithTitle( $namespace, $title );
                $resultWrapper = $this->getMockResultWrapper( $row );
 
-               $object = $this->getTitleArrayFromResult( $resultWrapper );
+               $object = new TitleArrayFromResult( $resultWrapper );
 
                $this->assertEquals( $resultWrapper, $object->res );
                $this->assertSame( 0, $object->key );
@@ -79,7 +75,7 @@ class TitleArrayFromResultTest extends PHPUnit\Framework\TestCase {
         * @covers TitleArrayFromResult::count
         */
        public function testCountWithVaryingValues( $numRows ) {
-               $object = $this->getTitleArrayFromResult( $this->getMockResultWrapper(
+               $object = new TitleArrayFromResult( $this->getMockResultWrapper(
                        $this->getRowWithTitle(),
                        $numRows
                ) );
@@ -93,7 +89,7 @@ class TitleArrayFromResultTest extends PHPUnit\Framework\TestCase {
                $namespace = 0;
                $title = 'foo';
                $row = $this->getRowWithTitle( $namespace, $title );
-               $object = $this->getTitleArrayFromResult( $this->getMockResultWrapper( $row ) );
+               $object = new TitleArrayFromResult( $this->getMockResultWrapper( $row ) );
                $this->assertInstanceOf( Title::class, $object->current() );
                $this->assertEquals( $namespace, $object->current->mNamespace );
                $this->assertEquals( $title, $object->current->mTextform );
@@ -111,7 +107,7 @@ class TitleArrayFromResultTest extends PHPUnit\Framework\TestCase {
         * @covers TitleArrayFromResult::valid
         */
        public function testValid( $input, $expected ) {
-               $object = $this->getTitleArrayFromResult( $this->getMockResultWrapper( $input ) );
+               $object = new TitleArrayFromResult( $this->getMockResultWrapper( $input ) );
                $this->assertEquals( $expected, $object->valid() );
        }
 
index 2b81222..abe91ea 100644 (file)
@@ -10,10 +10,6 @@ use MediaWiki\MediaWikiServices;
  */
 class ImportTest extends MediaWikiLangTestCase {
 
-       private function getDataSource( $xml ) {
-               return new ImportStringSource( $xml );
-       }
-
        /**
         * @covers WikiImporter
         * @dataProvider getUnknownTagsXML
@@ -22,7 +18,7 @@ class ImportTest extends MediaWikiLangTestCase {
         * @param string $title
         */
        public function testUnknownXMLTags( $xml, $text, $title ) {
-               $source = $this->getDataSource( $xml );
+               $source = new ImportStringSource( $xml );
 
                $importer = new WikiImporter(
                        $source,
@@ -81,7 +77,7 @@ EOF
         * @param string|null $redirectTitle
         */
        public function testHandlePageContainsRedirect( $xml, $redirectTitle ) {
-               $source = $this->getDataSource( $xml );
+               $source = new ImportStringSource( $xml );
 
                $redirect = null;
                $callback = function ( Title $title, ForeignTitle $foreignTitle, $revCount,
@@ -167,7 +163,7 @@ EOF
         * @param array|null $namespaces
         */
        public function testSiteInfoContainsNamespaces( $xml, $namespaces ) {
-               $source = $this->getDataSource( $xml );
+               $source = new ImportStringSource( $xml );
 
                $importNamespaces = null;
                $callback = function ( array $siteinfo, $innerImporter ) use ( &$importNamespaces ) {
@@ -252,7 +248,7 @@ EOF
                $n = ( $assign ? 1 : 0 ) + ( $create ? 2 : 0 );
 
                // phpcs:disable Generic.Files.LineLength
-               $source = $this->getDataSource( <<<EOF
+               $source = new ImportStringSource( <<<EOF
 <mediawiki xmlns="http://www.mediawiki.org/xml/export-0.10/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.mediawiki.org/xml/export-0.10/ http://www.mediawiki.org/xml/export-0.10.xsd" version="0.10" xml:lang="en">
   <page>
     <title>TestImportPage</title>
index beaacec..4cbfe46 100644 (file)
@@ -27,10 +27,6 @@ class UserArrayFromResultTest extends MediaWikiTestCase {
                return $row;
        }
 
-       private function getUserArrayFromResult( $resultWrapper ) {
-               return new UserArrayFromResult( $resultWrapper );
-       }
-
        /**
         * @covers UserArrayFromResult::__construct
         */
@@ -38,7 +34,7 @@ class UserArrayFromResultTest extends MediaWikiTestCase {
                $row = false;
                $resultWrapper = $this->getMockResultWrapper( $row );
 
-               $object = $this->getUserArrayFromResult( $resultWrapper );
+               $object = new UserArrayFromResult( $resultWrapper );
 
                $this->assertEquals( $resultWrapper, $object->res );
                $this->assertSame( 0, $object->key );
@@ -53,7 +49,7 @@ class UserArrayFromResultTest extends MediaWikiTestCase {
                $row = $this->getRowWithUsername( $username );
                $resultWrapper = $this->getMockResultWrapper( $row );
 
-               $object = $this->getUserArrayFromResult( $resultWrapper );
+               $object = new UserArrayFromResult( $resultWrapper );
 
                $this->assertEquals( $resultWrapper, $object->res );
                $this->assertSame( 0, $object->key );
@@ -74,7 +70,7 @@ class UserArrayFromResultTest extends MediaWikiTestCase {
         * @covers UserArrayFromResult::count
         */
        public function testCountWithVaryingValues( $numRows ) {
-               $object = $this->getUserArrayFromResult( $this->getMockResultWrapper(
+               $object = new UserArrayFromResult( $this->getMockResultWrapper(
                        $this->getRowWithUsername(),
                        $numRows
                ) );
@@ -87,7 +83,7 @@ class UserArrayFromResultTest extends MediaWikiTestCase {
        public function testCurrentAfterConstruction() {
                $username = 'addshore';
                $userRow = $this->getRowWithUsername( $username );
-               $object = $this->getUserArrayFromResult( $this->getMockResultWrapper( $userRow ) );
+               $object = new UserArrayFromResult( $this->getMockResultWrapper( $userRow ) );
                $this->assertInstanceOf( User::class, $object->current() );
                $this->assertEquals( $username, $object->current()->mName );
        }
@@ -104,7 +100,7 @@ class UserArrayFromResultTest extends MediaWikiTestCase {
         * @covers UserArrayFromResult::valid
         */
        public function testValid( $input, $expected ) {
-               $object = $this->getUserArrayFromResult( $this->getMockResultWrapper( $input ) );
+               $object = new UserArrayFromResult( $this->getMockResultWrapper( $input ) );
                $this->assertEquals( $expected, $object->valid() );
        }