Disallow overriding services that were set

Otherwise setService() calls (including indirect, like via
setContentLang() ) will be silently overridden.  This bit me several
times already while working on tests.  If someone actually wants to do
this, they should probably split their test in two, because it evidently
has two separate phases that they're testing (with/without services
set).

Depends-On: I9acb81c0de95eb5a6bed543d757ae62523ea6041
Change-Id: I8c60e37c179320e61684cbc11281c509e525e8fb

diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php
index d675e85..15e9847 100644 (file)
--- a/tests/phpunit/MediaWikiTestCase.php
+++ b/tests/phpunit/MediaWikiTestCase.php
@@ -112,6 +112,13 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
         */
        private $cliArgs = [];
 
+       /**
+        * Holds a list of services that were overridden with setService().  Used for printing an error
+        * if overrideMwServices() overrides a service that was previously set.
+        * @var string[]
+        */
+       private $overriddenServices = [];
+
        /**
         * Table name prefixes. Oracle likes it shorter.
         */
@@ -408,6 +415,9 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
 
                parent::run( $result );
 
+               // We don't mind if we override already-overridden services during cleanup
+               $this->overriddenServices = [];
+
                if ( $needsResetDB ) {
                        $this->resetDB( $this->db, $this->tablesUsed );
                }
@@ -486,6 +496,8 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
 
                $this->phpErrorLevel = intval( ini_get( 'error_reporting' ) );
 
+               $this->overriddenServices = [];
+
                // Cleaning up temporary files
                foreach ( $this->tmpFiles as $fileName ) {
                        if ( is_file( $fileName ) || ( is_link( $fileName ) ) ) {
@@ -630,6 +642,8 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                                . 'instance has been replaced by test code.' );
                }
 
+               $this->overriddenServices[] = $name;
+
                $this->localServices->disableService( $name );
                $this->localServices->redefineService(
                        $name,
@@ -891,6 +905,12 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
        protected function overrideMwServices(
                Config $configOverrides = null, array $services = []
        ) {
+               if ( $this->overriddenServices ) {
+                       throw new MWException(
+                               'The following services were set and are now being unset by overrideMwServices: ' .
+                                       implode( ', ', $this->overriddenServices )
+                       );
+               }
                $newInstance = self::installMockMwServices( $configOverrides );
 
                if ( $this->localServices ) {
@@ -1013,14 +1033,17 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
         */
        public function setContentLang( $lang ) {
                if ( $lang instanceof Language ) {
-                       $langCode = $lang->getCode();
-                       $langObj = $lang;
+                       $this->setMwGlobals( 'wgLanguageCode', $lang->getCode() );
+                       // Set to the exact object requested
+                       $this->setService( 'ContentLanguage', $lang );
                } else {
-                       $langCode = $lang;
-                       $langObj = Language::factory( $langCode );
+                       $this->setMwGlobals( 'wgLanguageCode', $lang );
+                       // Let the service handler make up the object.  Avoid calling setService(), because if
+                       // we do, overrideMwServices() will complain if it's called later on.
+                       $services = MediaWikiServices::getInstance();
+                       $services->resetServiceForTesting( 'ContentLanguage' );
+                       $this->doSetMwGlobals( [ 'wgContLang' => $services->getContentLanguage() ] );
                }
-               $this->setMwGlobals( 'wgLanguageCode', $langCode );
-               $this->setService( 'ContentLanguage', $langObj );
        }
 
        /**