From c70c2e4714ac5186b69319dafd0ace3a0d2d00ab Mon Sep 17 00:00:00 2001
From: Aryeh Gregor <ayg@aryeh.name>
Date: Mon, 6 Aug 2018 19:01:31 +0300
Subject: [PATCH] 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
---
 tests/phpunit/MediaWikiTestCase.php | 35 ++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php
index d675e8518b..15e9847a67 100644
--- 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 );
 	}
 
 	/**
-- 
2.20.1