Avoid interacting with LBFactory singleton in tests
authoraude <aude.wiki@gmail.com>
Sun, 29 Dec 2013 21:00:39 +0000 (22:00 +0100)
committeraude <aude.wiki@gmail.com>
Thu, 2 Jan 2014 13:27:19 +0000 (14:27 +0100)
Instead test that LBFactory::getLBFactoryClass() does the right thing.

In LBFactory::getLBFactoryClass(), the method now requires a configuration
array as a parameter, and returns the class name as a string.

LBFactory::singleton() now passes in $wgLBFactoryConf into getLBFactoryClass(),
while tests can use test configuration.

Only use of LBFactory::getLBFactoryClass() in core is in LBFactory::singleton().

No extension in gerrit uses getLBFactoryClass() and I can find no use of it
when searching places like Github and Google, so it should be safe to make
such change in LBFactory.

Bug: 59105
Change-Id: I71ae7df16bc5c214b9389125140bca5ce68d274c

includes/db/LBFactory.php
tests/phpunit/includes/db/LBFactoryTest.php

index c4e7976..0f3126f 100644 (file)
@@ -46,10 +46,12 @@ abstract class LBFactory {
         * @return LBFactory
         */
        static function &singleton() {
+               global $wgLBFactoryConf;
+
                if ( is_null( self::$instance ) ) {
-                       $LBFactoryConf = self::getLBFactoryClass();
+                       $class = self::getLBFactoryClass( $wgLBFactoryConf );
 
-                       self::$instance = new $LBFactoryConf[0]( $LBFactoryConf[1] );
+                       self::$instance = new $class( $wgLBFactoryConf );
                }
 
                return self::$instance;
@@ -58,11 +60,11 @@ abstract class LBFactory {
        /**
         * Returns the LBFactory class to use and the load balancer configuration.
         *
-        * @return array ( factory class, $wgLBFactoryConf )
+        * @param array $config (e.g. $wgLBFactoryConf)
+        *
+        * @return string class name
         */
-       static function getLBFactoryClass() {
-               global $wgLBFactoryConf;
-
+       public static function getLBFactoryClass( array $config ) {
                // For configuration backward compatibility after removing
                // underscores from class names in MediaWiki 1.23.
                $bcClasses = array(
@@ -72,9 +74,9 @@ abstract class LBFactory {
                        'LBFactory_Fake' => 'LBFactoryFake',
                );
 
-               $class = $wgLBFactoryConf['class'];
+               $class = $config['class'];
 
-               if ( in_array( $class, array_keys( $bcClasses ) ) ) {
+               if ( isset( $bcClasses[$class] ) ) {
                        $class = $bcClasses[$class];
                        wfDeprecated(
                                '$wgLBFactoryConf must be updated. See RELEASE-NOTES for details',
@@ -82,7 +84,7 @@ abstract class LBFactory {
                        );
                }
 
-               return array( $class, $wgLBFactoryConf );
+               return $class;
        }
 
        /**
index f64a784..4c59f47 100644 (file)
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
+ * @group Database
  * @file
  * @author Antoine Musso
  * @copyright © 2013 Antoine Musso
  * @copyright © 2013 Wikimedia Foundation Inc.
  */
-
-class FakeLBFactory extends LBFactory {
-       function __construct( $conf ) {}
-       function newMainLB( $wiki = false ) {}
-       function getMainLB( $wiki = false ) {}
-       function newExternalLB( $cluster, $wiki = false ) {}
-       function &getExternalLB( $cluster, $wiki = false ) {}
-       function forEachLB( $callback, $params = array() ) {}
-}
-
 class LBFactoryTest extends MediaWikiTestCase {
 
-       function setup() {
-               parent::setup();
-               FakeLBFactory::destroyInstance();
-       }
-
        /**
-        * @dataProvider provideDeprecatedLbfactoryClasses
+        * @dataProvider getLBFactoryClassProvider
         */
-       function testLbfactoryClassBackcompatibility( $expected, $deprecated ) {
+       public function testGetLBFactoryClass( $expected, $deprecated ) {
                $mockDB = $this->getMockBuilder( 'DatabaseMysql' )
-                       -> disableOriginalConstructor()
+                       ->disableOriginalConstructor()
                        ->getMock();
-               $this->setMwGlobals( 'wgLBFactoryConf',
-                       array(
-                               'class'          => $deprecated,
-                               'connection'     => $mockDB,
-                               # Various other parameters required:
-                               'sectionsByDB'   => array(),
-                               'sectionLoads'   => array(),
-                               'serverTemplate' => array(),
-                       )
-               );
 
-               global $wgLBFactoryConf;
-               $this->assertArrayHasKey( 'class', $wgLBFactoryConf );
-               $this->assertEquals( $wgLBFactoryConf['class'], $deprecated );
+               $config = array(
+                       'class'          => $deprecated,
+                       'connection'     => $mockDB,
+                       # Various other parameters required:
+                       'sectionsByDB'   => array(),
+                       'sectionLoads'   => array(),
+                       'serverTemplate' => array(),
+               );
 
-               # The point of this test is to call a deprecated interface and make
-               # sure it keeps back compatibility, so skip the deprecation warning.
                $this->hideDeprecated( '$wgLBFactoryConf must be updated. See RELEASE-NOTES for details' );
-               $lbfactory = FakeLBFactory::singleton();
-               $this->assertInstanceOf( $expected, $lbfactory,
-                       "LBFactory passed $deprecated should yield the new class $expected" );
+               $result = LBFactory::getLBFactoryClass( $config );
+
+               $this->assertEquals( $expected, $result );
        }
 
-       function provideDeprecatedLbfactoryClasses() {
+       public function getLBFactoryClassProvider() {
                return array(
                        # Format: new class, old class
                        array( 'LBFactorySimple', 'LBFactory_Simple' ),