Merge "Reset services before every test"

diff --git a/tests/common/TestSetup.php b/tests/common/TestSetup.php
index c176a67..2feb438 100644 (file)
--- a/tests/common/TestSetup.php
+++ b/tests/common/TestSetup.php
@@ -104,10 +104,6 @@ class TestSetup {
                // may break testing against floating point values
                // treated with PHP's serialize()
                ini_set( 'serialize_precision', 17 );
-
-               // TODO: we should call MediaWikiTestCase::prepareServices( new GlobalVarConfig() ) here.
-               // But PHPUnit may not be loaded yet, so we have to wait until just
-               // before PHPUnit_TextUI_Command::main() is executed.
        }
 
 }

diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php
index 5cc45f5..17147eb 100644 (file)
--- a/tests/phpunit/MediaWikiTestCase.php
+++ b/tests/phpunit/MediaWikiTestCase.php
@@ -21,13 +21,16 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
        use PHPUnit4And6Compat;
 
        /**
-        * The service locator created by prepareServices(). This service locator will
-        * be restored after each test. Tests that pollute the global service locator
-        * instance should use overrideMwServices() to isolate the test.
+        * The original service locator. This is overridden during setUp().
         *
         * @var MediaWikiServices|null
         */
-       private static $serviceLocator = null;
+       private static $originalServices;
+
+       /**
+        * The local service locator, created during setUp().
+        */
+       private $localServices;
 
        /**
         * $called tracks whether the setUp and tearDown method has been called.
@@ -155,8 +158,10 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
        public static function setUpBeforeClass() {
                parent::setUpBeforeClass();
 
-               // Get the service locator, and reset services if it's not done already
-               self::$serviceLocator = self::prepareServices( new GlobalVarConfig() );
+               // Get the original service locator
+               if ( !self::$originalServices ) {
+                       self::$originalServices = MediaWikiServices::getInstance();
+               }
        }
 
        /**
@@ -245,63 +250,9 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
        }
 
        /**
-        * Prepare service configuration for unit testing.
-        *
-        * This calls MediaWikiServices::resetGlobalInstance() to allow some critical services
-        * to be overridden for testing.
-        *
-        * prepareServices() only needs to be called once, but should be called as early as possible,
-        * before any class has a chance to grab a reference to any of the global services
-        * instances that get discarded by prepareServices(). Only the first call has any effect,
-        * later calls are ignored.
-        *
-        * @note This is called by PHPUnitMaintClass::finalSetup.
-        *
-        * @see MediaWikiServices::resetGlobalInstance()
-        *
-        * @param Config $bootstrapConfig The bootstrap config to use with the new
-        *        MediaWikiServices. Only used for the first call to this method.
-        * @return MediaWikiServices
+        * @deprecated since 1.32
         */
        public static function prepareServices( Config $bootstrapConfig ) {
-               static $services = null;
-
-               if ( !$services ) {
-                       $services = self::resetGlobalServices( $bootstrapConfig );
-               }
-               return $services;
-       }
-
-       /**
-        * Reset global services, and install testing environment.
-        * This is the testing equivalent of MediaWikiServices::resetGlobalInstance().
-        * This should only be used to set up the testing environment, not when
-        * running unit tests. Use MediaWikiTestCase::overrideMwServices() for that.
-        *
-        * @see MediaWikiServices::resetGlobalInstance()
-        * @see prepareServices()
-        * @see MediaWikiTestCase::overrideMwServices()
-        *
-        * @param Config|null $bootstrapConfig The bootstrap config to use with the new
-        *        MediaWikiServices.
-        * @return MediaWikiServices
-        */
-       private static function resetGlobalServices( Config $bootstrapConfig = null ) {
-               $oldServices = MediaWikiServices::getInstance();
-               $oldConfigFactory = $oldServices->getConfigFactory();
-               $oldLoadBalancerFactory = $oldServices->getDBLoadBalancerFactory();
-
-               $testConfig = self::makeTestConfig( $bootstrapConfig );
-
-               MediaWikiServices::resetGlobalInstance( $testConfig );
-
-               $serviceLocator = MediaWikiServices::getInstance();
-               self::installTestServices(
-                       $oldConfigFactory,
-                       $oldLoadBalancerFactory,
-                       $serviceLocator
-               );
-               return $serviceLocator;
        }
 
        /**
@@ -320,7 +271,7 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                $defaultOverrides = new HashConfig();
 
                if ( !$baseConfig ) {
-                       $baseConfig = MediaWikiServices::getInstance()->getBootstrapConfig();
+                       $baseConfig = self::$originalServices->getBootstrapConfig();
                }
 
                /* Some functions require some kind of caching, and will end up using the db,
@@ -415,17 +366,10 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
        }
 
        /**
-        * Resets some well known services that typically have state that may interfere with unit tests.
-        * This is a lightweight alternative to resetGlobalServices().
-        *
-        * @note There is no guarantee that no references remain to stale service instances destroyed
-        * by a call to doLightweightServiceReset().
-        *
-        * @throws MWException if called outside of PHPUnit tests.
-        *
-        * @see resetGlobalServices()
+        * Resets some non-service singleton instances and other static caches. It's not necessary to
+        * reset services here.
         */
-       private function doLightweightServiceReset() {
+       private function resetNonServiceCaches() {
                global $wgRequest, $wgJobClasses;
 
                foreach ( $wgJobClasses as $type => $class ) {
@@ -434,10 +378,6 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                JobQueueGroup::destroySingletons();
 
                ObjectCache::clear();
-               $services = MediaWikiServices::getInstance();
-               $services->resetServiceForTesting( 'MainObjectStash' );
-               $services->resetServiceForTesting( 'LocalServerObjectCache' );
-               $services->getMainWANObjectCache()->clearProcessCache();
                FileBackendGroup::destroySingleton();
                DeferredUpdates::clearPendingUpdates();
 
@@ -453,6 +393,8 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
        }
 
        public function run( PHPUnit_Framework_TestResult $result = null ) {
+               $this->overrideMwServices();
+
                $needsResetDB = false;
 
                if ( !self::$dbSetup || $this->needsDB() ) {
@@ -492,6 +434,10 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                        $this->testLogger->info( "Resetting DB" );
                        $this->resetDB( $this->db, $this->tablesUsed );
                }
+
+               $this->localServices->destroy();
+               $this->localServices = null;
+               MediaWikiServices::forceGlobalInstance( self::$originalServices );
        }
 
        /**
@@ -591,13 +537,12 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                }
 
                // Reset all caches between tests.
-               $this->doLightweightServiceReset();
+               $this->resetNonServiceCaches();
 
                // XXX: reset maintenance triggers
                // Hook into period lag checks which often happen in long-running scripts
-               $services = MediaWikiServices::getInstance();
-               $lbFactory = $services->getDBLoadBalancerFactory();
-               Maintenance::setLBFactoryTriggers( $lbFactory, $services->getMainConfig() );
+               $lbFactory = $this->localServices->getDBLoadBalancerFactory();
+               Maintenance::setLBFactoryTriggers( $lbFactory, $this->localServices->getMainConfig() );
 
                ob_start( 'MediaWikiTestCase::wfResetOutputBuffersBarrier' );
        }
@@ -654,10 +599,6 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                $this->mwGlobalsToUnset = [];
                $this->restoreLoggers();
 
-               if ( self::$serviceLocator && MediaWikiServices::getInstance() !== self::$serviceLocator ) {
-                       MediaWikiServices::forceGlobalInstance( self::$serviceLocator );
-               }
-
                // TODO: move global state into MediaWikiServices
                RequestContext::resetMain();
                if ( session_id() !== '' ) {
@@ -708,13 +649,12 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
         * @param object $object
         */
        protected function setService( $name, $object ) {
-               // If we did not yet override the service locator, so so now.
-               if ( MediaWikiServices::getInstance() === self::$serviceLocator ) {
-                       $this->overrideMwServices();
+               if ( !$this->localServices ) {
+                       throw new Exception( __METHOD__ . ' must be called after MediaWikiTestCase::run()' );
                }
 
-               MediaWikiServices::getInstance()->disableService( $name );
-               MediaWikiServices::getInstance()->redefineService(
+               $this->localServices->disableService( $name );
+               $this->localServices->redefineService(
                        $name,
                        function () use ( $object ) {
                                return $object;
@@ -796,15 +736,17 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
         * Otherwise old namespace data will lurk and cause bugs.
         */
        private function resetNamespaces() {
+               if ( !$this->localServices ) {
+                       throw new Exception( __METHOD__ . ' must be called after MediaWikiTestCase::run()' );
+               }
                MWNamespace::clearCaches();
                Language::clearCaches();
 
                // We can't have the TitleFormatter holding on to an old Language object either
                // @todo We shouldn't need to reset all the aliases here.
-               $services = MediaWikiServices::getInstance();
-               $services->resetServiceForTesting( 'TitleFormatter' );
-               $services->resetServiceForTesting( 'TitleParser' );
-               $services->resetServiceForTesting( '_MediaWikiTitleCodec' );
+               $this->localServices->resetServiceForTesting( 'TitleFormatter' );
+               $this->localServices->resetServiceForTesting( 'TitleParser' );
+               $this->localServices->resetServiceForTesting( '_MediaWikiTitleCodec' );
        }
 
        /**
@@ -963,16 +905,15 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
         * @return MediaWikiServices
         * @throws MWException
         */
-       protected static function overrideMwServices(
+       protected function overrideMwServices(
                Config $configOverrides = null, array $services = []
        ) {
                if ( !$configOverrides ) {
                        $configOverrides = new HashConfig();
                }
 
-               $oldInstance = MediaWikiServices::getInstance();
-               $oldConfigFactory = $oldInstance->getConfigFactory();
-               $oldLoadBalancerFactory = $oldInstance->getDBLoadBalancerFactory();
+               $oldConfigFactory = self::$originalServices->getConfigFactory();
+               $oldLoadBalancerFactory = self::$originalServices->getDBLoadBalancerFactory();
 
                $testConfig = self::makeTestConfig( null, $configOverrides );
                $newInstance = new MediaWikiServices( $testConfig );
@@ -994,7 +935,13 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                        $oldLoadBalancerFactory,
                        $newInstance
                );
+
+               if ( $this->localServices ) {
+                       $this->localServices->destroy();
+               }
+
                MediaWikiServices::forceGlobalInstance( $newInstance );
+               $this->localServices = $newInstance;
 
                return $newInstance;
        }
@@ -1682,12 +1629,6 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                foreach ( $tables as $tbl ) {
                        $tbl = $db->tableName( $tbl );
                        $db->query( "DROP TABLE IF EXISTS $tbl", __METHOD__ );
-
-                       if ( $tbl === 'page' ) {
-                               // Forget about the pages since they don't
-                               // exist in the DB.
-                               MediaWikiServices::getInstance()->getLinkCache()->clear();
-                       }
                }
        }
 
@@ -1835,12 +1776,6 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                                __METHOD__
                        );
                }
-
-               if ( $tableName === 'page' ) {
-                       // Forget about the pages since they don't
-                       // exist in the DB.
-                       MediaWikiServices::getInstance()->getLinkCache()->clear();
-               }
        }
 
        private static function unprefixTable( &$tableName, $ind, $prefix ) {

diff --git a/tests/phpunit/includes/PrefixSearchTest.php b/tests/phpunit/includes/PrefixSearchTest.php
index 0e35767..685cb40 100644 (file)
--- a/tests/phpunit/includes/PrefixSearchTest.php
+++ b/tests/phpunit/includes/PrefixSearchTest.php
@@ -68,8 +68,6 @@ class PrefixSearchTest extends MediaWikiLangTestCase {
                parent::tearDown();
 
                TestingAccessWrapper::newFromClass( Hooks::class )->handlers = $this->originalHandlers;
-
-               $this->overrideMwServices();
        }
 
        protected function searchProvision( array $results = null ) {

diff --git a/tests/phpunit/includes/Storage/PageUpdaterTest.php b/tests/phpunit/includes/Storage/PageUpdaterTest.php
index 2805ea8..517e7c6 100644 (file)
--- a/tests/phpunit/includes/Storage/PageUpdaterTest.php
+++ b/tests/phpunit/includes/Storage/PageUpdaterTest.php
@@ -19,14 +19,6 @@ use WikiPage;
  * @group Database
  */
 class PageUpdaterTest extends MediaWikiTestCase {
-
-       public static function setUpBeforeClass() {
-               parent::setUpBeforeClass();
-
-               // force service reset!
-               MediaWikiServices::getInstance()->resetServiceForTesting( 'RevisionStore' );
-       }
-
        private function getDummyTitle( $method ) {
                return Title::newFromText( $method, $this->getDefaultWikitextNS() );
        }

diff --git a/tests/phpunit/includes/search/SearchEnginePrefixTest.php b/tests/phpunit/includes/search/SearchEnginePrefixTest.php
index 41c1218..ee272b9 100644 (file)
--- a/tests/phpunit/includes/search/SearchEnginePrefixTest.php
+++ b/tests/phpunit/includes/search/SearchEnginePrefixTest.php
@@ -76,8 +76,6 @@ class SearchEnginePrefixTest extends MediaWikiLangTestCase {
                parent::tearDown();
 
                TestingAccessWrapper::newFromClass( Hooks::class )->handlers = $this->originalHandlers;
-
-               $this->overrideMwServices();
        }
 
        protected function searchProvision( array $results = null ) {

diff --git a/tests/phpunit/phpunit.php b/tests/phpunit/phpunit.php
index d83dedb..b1cca4a 100755 (executable)
--- a/tests/phpunit/phpunit.php
+++ b/tests/phpunit/phpunit.php
@@ -129,9 +129,6 @@ class PHPUnitMaintClass extends Maintenance {
                        'Using HHVM ' . HHVM_VERSION . ' (' . PHP_VERSION . ")\n" :
                        'Using PHP ' . PHP_VERSION . "\n";
 
-               // Prepare global services for unit tests.
-               MediaWikiTestCase::prepareServices( new GlobalVarConfig() );
-
                $phpUnitClass::main();
        }
 

diff --git a/tests/phpunit/structure/SpecialPageFatalTest.php b/tests/phpunit/structure/SpecialPageFatalTest.php
index 9d85fde..a6bc5a7 100644 (file)
--- a/tests/phpunit/structure/SpecialPageFatalTest.php
+++ b/tests/phpunit/structure/SpecialPageFatalTest.php
@@ -13,17 +13,6 @@ use MediaWiki\MediaWikiServices;
  * @author Addshore
  */
 class SpecialPageFatalTest extends MediaWikiTestCase {
-
-       public static function setUpBeforeClass() {
-               parent::setUpBeforeClass();
-               self::overrideMwServices();
-       }
-
-       public static function tearDownAfterClass() {
-               self::overrideMwServices();
-               parent::tearDownAfterClass();
-       }
-
        public function provideSpecialPages() {
                $specialPages = [];
                $spf = MediaWikiServices::getInstance()->getSpecialPageFactory();

diff --git a/tests/phpunit/tests/MediaWikiTestCaseTest.php b/tests/phpunit/tests/MediaWikiTestCaseTest.php
index 1850f6f..d2fca72 100644 (file)
--- a/tests/phpunit/tests/MediaWikiTestCaseTest.php
+++ b/tests/phpunit/tests/MediaWikiTestCaseTest.php
@@ -114,9 +114,6 @@ class MediaWikiTestCaseTest extends MediaWikiTestCase {
 
                $this->overrideMwServices();
                $this->assertNotSame( $initialServices, MediaWikiServices::getInstance() );
-
-               $this->tearDown();
-               $this->assertSame( $initialServices, MediaWikiServices::getInstance() );
        }
 
        public function testSetService() {
@@ -126,17 +123,11 @@ class MediaWikiTestCaseTest extends MediaWikiTestCase {
                        ->disableOriginalConstructor()->getMock();
 
                $this->setService( 'DBLoadBalancer', $mockService );
-               $this->assertNotSame( $initialServices, MediaWikiServices::getInstance() );
                $this->assertNotSame(
                        $initialService,
                        MediaWikiServices::getInstance()->getDBLoadBalancer()
                );
                $this->assertSame( $mockService, MediaWikiServices::getInstance()->getDBLoadBalancer() );
-
-               $this->tearDown();
-               $this->assertSame( $initialServices, MediaWikiServices::getInstance() );
-               $this->assertNotSame( $mockService, MediaWikiServices::getInstance()->getDBLoadBalancer() );
-               $this->assertSame( $initialService, MediaWikiServices::getInstance()->getDBLoadBalancer() );
        }
 
        /**