From 1959b3eb5ac162cac0db4591173186dd4078aa1e Mon Sep 17 00:00:00 2001
From: aude <aude.wiki@gmail.com>
Date: Sun, 29 Dec 2013 22:00:39 +0100
Subject: [PATCH] Avoid interacting with LBFactory singleton in tests

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                   | 20 ++++----
 tests/phpunit/includes/db/LBFactoryTest.php | 53 +++++++--------------
 2 files changed, 27 insertions(+), 46 deletions(-)

diff --git a/includes/db/LBFactory.php b/includes/db/LBFactory.php
index c4e7976e04..0f3126fb47 100644
--- a/includes/db/LBFactory.php
+++ b/includes/db/LBFactory.php
@@ -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;
 	}
 
 	/**
diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php
index f64a784d4d..4c59f47481 100644
--- a/tests/phpunit/includes/db/LBFactoryTest.php
+++ b/tests/phpunit/includes/db/LBFactoryTest.php
@@ -18,59 +18,38 @@
  * 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' ),
-- 
2.20.1