From: Aryeh Gregor Date: Sun, 7 Oct 2018 12:26:18 +0000 (+0300) Subject: Deprecate MediaWikiTestCase::stashMwGlobals X-Git-Tag: 1.34.0-rc.0~3868^2 X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=09eee138e1d725a599fc5953eb1d88173c643701;p=lhc%2Fweb%2Fwiklou.git Deprecate MediaWikiTestCase::stashMwGlobals This method encourages directly editing configuration variables. It's a better idea to use setMwGlobals() (or other set wrappers) so that we can be intelligent in the future, for instance resetting services after the config change. Plus, a lot of the callers come out cleaner this way anyway. Depends-On: I8a1e81acc5c42a8d7f30938a72cface0acea4a70 Depends-On: I4105dbcf9c5399fe7239478c460ec57c015a98d4 Depends-On: I1b220996acf2f66cf7b0f092b341584663df32f9 Depends-On: Ie2d1ea65c0cb334bbde1666d00781474b7ac4dab Change-Id: I23d77398e401f4986b1d5bd1c9e11a8a40da16f8 --- diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php index dfd430990c..feab0df451 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiTestCase.php @@ -711,7 +711,7 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { * touch. */ private function doSetMwGlobals( $pairs, $value = null ) { - $this->stashMwGlobals( array_keys( $pairs ) ); + $this->doStashMwGlobals( array_keys( $pairs ) ); foreach ( $pairs as $key => $value ) { $GLOBALS[$key] = $value; @@ -785,8 +785,14 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { * call overrideMwServices(). * * @since 1.23 + * @deprecated since 1.32, use setMwGlobals() and don't alter globals directly */ protected function stashMwGlobals( $globalKeys ) { + wfDeprecated( __METHOD__, '1.32' ); + $this->doStashMwGlobals( $globalKeys ); + } + + private function doStashMwGlobals( $globalKeys ) { if ( is_string( $globalKeys ) ) { $globalKeys = [ $globalKeys ]; } @@ -1063,17 +1069,18 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { public function setGroupPermissions( $newPerms, $newKey = null, $newValue = null ) { global $wgGroupPermissions; - $this->stashMwGlobals( 'wgGroupPermissions' ); - if ( is_string( $newPerms ) ) { $newPerms = [ $newPerms => [ $newKey => $newValue ] ]; } + $newPermissions = $wgGroupPermissions; foreach ( $newPerms as $group => $permissions ) { foreach ( $permissions as $key => $value ) { - $wgGroupPermissions[$group][$key] = $value; + $newPermissions[$group][$key] = $value; } } + + $this->setMwGlobals( 'wgGroupPermissions', $newPermissions ); } /** diff --git a/tests/phpunit/includes/api/ApiMoveTest.php b/tests/phpunit/includes/api/ApiMoveTest.php index fb697ffd6a..3fa85394c1 100644 --- a/tests/phpunit/includes/api/ApiMoveTest.php +++ b/tests/phpunit/includes/api/ApiMoveTest.php @@ -133,8 +133,6 @@ class ApiMoveTest extends ApiTestCase { // @todo File moving public function testPingLimiter() { - global $wgRateLimits; - $this->setExpectedException( ApiUsageException::class, "You've exceeded your rate limit. Please wait some time and try again." ); @@ -142,8 +140,8 @@ class ApiMoveTest extends ApiTestCase { $this->setMwGlobals( 'wgMainCacheType', 'hash' ); - $this->stashMwGlobals( 'wgRateLimits' ); - $wgRateLimits['move'] = [ '&can-bypass' => false, 'user' => [ 1, 60 ] ]; + $this->mergeMwGlobalArrayValue( 'wgRateLimits', + [ 'move' => [ '&can-bypass' => false, 'user' => [ 1, 60 ] ] ] ); $id = $this->createPage( $name ); @@ -257,12 +255,9 @@ class ApiMoveTest extends ApiTestCase { } public function testMoveSubpages() { - global $wgNamespacesWithSubpages; - $name = ucfirst( __FUNCTION__ ); - $this->stashMwGlobals( 'wgNamespacesWithSubpages' ); - $wgNamespacesWithSubpages[NS_MAIN] = true; + $this->mergeMwGlobalArrayValue( 'wgNamespacesWithSubpages', [ NS_MAIN => true ] ); $pages = [ $name, "$name/1", "$name/2", "Talk:$name", "Talk:$name/1", "Talk:$name/3" ]; $ids = []; diff --git a/tests/phpunit/includes/api/ApiQuerySiteinfoTest.php b/tests/phpunit/includes/api/ApiQuerySiteinfoTest.php index 2d7f8a4b50..9587a763f1 100644 --- a/tests/phpunit/includes/api/ApiQuerySiteinfoTest.php +++ b/tests/phpunit/includes/api/ApiQuerySiteinfoTest.php @@ -325,9 +325,8 @@ class ApiQuerySiteinfoTest extends ApiTestCase { public function testFileExtensions() { global $wgFileExtensions; - $this->stashMwGlobals( 'wgFileExtensions' ); // Add duplicate - $wgFileExtensions[] = 'png'; + $this->setMwGlobals( 'wgFileExtensions', array_merge( $wgFileExtensions, [ 'png' ] ) ); $expected = array_map( function ( $val ) { diff --git a/tests/phpunit/includes/api/ApiStashEditTest.php b/tests/phpunit/includes/api/ApiStashEditTest.php index d5d33fbc8e..61512d5c35 100644 --- a/tests/phpunit/includes/api/ApiStashEditTest.php +++ b/tests/phpunit/includes/api/ApiStashEditTest.php @@ -254,10 +254,8 @@ class ApiStashEditTest extends ApiTestCase { } public function testPingLimiter() { - global $wgRateLimits; - - $this->stashMwGlobals( 'wgRateLimits' ); - $wgRateLimits['stashedit'] = [ '&can-bypass' => false, 'user' => [ 1, 60 ] ]; + $this->mergeMwGlobalArrayValue( 'wgRateLimits', + [ 'stashedit' => [ '&can-bypass' => false, 'user' => [ 1, 60 ] ] ] ); $this->doStash( [ 'text' => 'A' ] ); diff --git a/tests/phpunit/includes/api/ApiUserrightsTest.php b/tests/phpunit/includes/api/ApiUserrightsTest.php index c8ecda2173..2534ad3979 100644 --- a/tests/phpunit/includes/api/ApiUserrightsTest.php +++ b/tests/phpunit/includes/api/ApiUserrightsTest.php @@ -26,17 +26,13 @@ class ApiUserrightsTest extends ApiTestCase { * @param array|bool $remove Groups bureaucrats should be allowed to remove, true for all */ protected function setPermissions( $add = [], $remove = [] ) { - global $wgAddGroups, $wgRemoveGroups; - $this->setGroupPermissions( 'bureaucrat', 'userrights', false ); if ( $add ) { - $this->stashMwGlobals( 'wgAddGroups' ); - $wgAddGroups['bureaucrat'] = $add; + $this->mergeMwGlobalArrayValue( 'wgAddGroups', [ 'bureaucrat' => $add ] ); } if ( $remove ) { - $this->stashMwGlobals( 'wgRemoveGroups' ); - $wgRemoveGroups['bureaucrat'] = $remove; + $this->mergeMwGlobalArrayValue( 'wgRemoveGroups', [ 'bureaucrat' => $remove ] ); } } @@ -241,12 +237,9 @@ class ApiUserrightsTest extends ApiTestCase { } public function testWithoutTagPermission() { - global $wgGroupPermissions; - ChangeTags::defineTag( 'custom tag' ); - $this->stashMwGlobals( 'wgGroupPermissions' ); - $wgGroupPermissions['user']['applychangetags'] = false; + $this->setGroupPermissions( 'user', 'applychangetags', false ); $this->doFailedRightsChange( 'You do not have permission to apply change tags along with your changes.', diff --git a/tests/phpunit/includes/auth/AuthManagerTest.php b/tests/phpunit/includes/auth/AuthManagerTest.php index 0c5d02671c..6bdc5695d9 100644 --- a/tests/phpunit/includes/auth/AuthManagerTest.php +++ b/tests/phpunit/includes/auth/AuthManagerTest.php @@ -1408,13 +1408,9 @@ class AuthManagerTest extends \MediaWikiTestCase { } public function testCheckAccountCreatePermissions() { - global $wgGroupPermissions; - - $this->stashMwGlobals( [ 'wgGroupPermissions' ] ); - $this->initializeManager( true ); - $wgGroupPermissions['*']['createaccount'] = true; + $this->setGroupPermissions( '*', 'createaccount', true ); $this->assertEquals( \Status::newGood(), $this->manager->checkAccountCreatePermissions( new \User ) @@ -1428,11 +1424,11 @@ class AuthManagerTest extends \MediaWikiTestCase { ); $readOnlyMode->setReason( false ); - $wgGroupPermissions['*']['createaccount'] = false; + $this->setGroupPermissions( '*', 'createaccount', false ); $status = $this->manager->checkAccountCreatePermissions( new \User ); $this->assertFalse( $status->isOK() ); $this->assertTrue( $status->hasMessage( 'badaccess-groups' ) ); - $wgGroupPermissions['*']['createaccount'] = true; + $this->setGroupPermissions( '*', 'createaccount', true ); $user = \User::newFromName( 'UTBlockee' ); if ( $user->getID() == 0 ) { @@ -2356,7 +2352,7 @@ class AuthManagerTest extends \MediaWikiTestCase { } public function testAutoAccountCreation() { - global $wgGroupPermissions, $wgHooks; + global $wgHooks; // PHPUnit seems to have a bug where it will call the ->with() // callbacks for our hooks again after the test is run (WTF?), which @@ -2367,9 +2363,8 @@ class AuthManagerTest extends \MediaWikiTestCase { $username = self::usernameForCreation(); $this->initializeManager(); - $this->stashMwGlobals( [ 'wgGroupPermissions' ] ); - $wgGroupPermissions['*']['createaccount'] = true; - $wgGroupPermissions['*']['autocreateaccount'] = false; + $this->setGroupPermissions( '*', 'createaccount', true ); + $this->setGroupPermissions( '*', 'autocreateaccount', false ); $this->mergeMwGlobalArrayValue( 'wgObjectCaches', [ __METHOD__ => [ 'class' => 'HashBagOStuff' ] ] ); @@ -2550,8 +2545,8 @@ class AuthManagerTest extends \MediaWikiTestCase { $this->assertSame( 'noname', $session->get( 'AuthManager::AutoCreateBlacklist' ) ); // IP unable to create accounts - $wgGroupPermissions['*']['createaccount'] = false; - $wgGroupPermissions['*']['autocreateaccount'] = false; + $this->setGroupPermissions( '*', 'createaccount', false ); + $this->setGroupPermissions( '*', 'autocreateaccount', false ); $session->clear(); $user = \User::newFromName( $username ); $this->hook( 'LocalUserCreated', $this->never() ); @@ -2571,8 +2566,8 @@ class AuthManagerTest extends \MediaWikiTestCase { // Test that both permutations of permissions are allowed // (this hits the two "ok" entries in $mocks['pre']) - $wgGroupPermissions['*']['createaccount'] = false; - $wgGroupPermissions['*']['autocreateaccount'] = true; + $this->setGroupPermissions( '*', 'createaccount', false ); + $this->setGroupPermissions( '*', 'autocreateaccount', true ); $session->clear(); $user = \User::newFromName( $username ); $this->hook( 'LocalUserCreated', $this->never() ); @@ -2580,8 +2575,8 @@ class AuthManagerTest extends \MediaWikiTestCase { $this->unhook( 'LocalUserCreated' ); $this->assertEquals( \Status::newFatal( 'ok' ), $ret ); - $wgGroupPermissions['*']['createaccount'] = true; - $wgGroupPermissions['*']['autocreateaccount'] = false; + $this->setGroupPermissions( '*', 'createaccount', true ); + $this->setGroupPermissions( '*', 'autocreateaccount', false ); $session->clear(); $user = \User::newFromName( $username ); $this->hook( 'LocalUserCreated', $this->never() ); diff --git a/tests/phpunit/includes/auth/TemporaryPasswordAuthenticationRequestTest.php b/tests/phpunit/includes/auth/TemporaryPasswordAuthenticationRequestTest.php index ab4a174eb9..54538edb61 100644 --- a/tests/phpunit/includes/auth/TemporaryPasswordAuthenticationRequestTest.php +++ b/tests/phpunit/includes/auth/TemporaryPasswordAuthenticationRequestTest.php @@ -25,12 +25,14 @@ class TemporaryPasswordAuthenticationRequestTest extends AuthenticationRequestTe public function testNewRandom() { global $wgPasswordPolicy; - $this->stashMwGlobals( 'wgPasswordPolicy' ); - $wgPasswordPolicy['policies']['default'] += [ + $policy = $wgPasswordPolicy; + $policy['policies']['default'] += [ 'MinimalPasswordLength' => 1, 'MinimalPasswordLengthToLogin' => 1, ]; + $this->setMwGlobals( 'wgPasswordPolicy', $policy ); + $ret1 = TemporaryPasswordAuthenticationRequest::newRandom(); $ret2 = TemporaryPasswordAuthenticationRequest::newRandom(); $this->assertNotSame( '', $ret1->password ); diff --git a/tests/phpunit/includes/config/GlobalVarConfigTest.php b/tests/phpunit/includes/config/GlobalVarConfigTest.php index db5f73d41e..ec443e7a8a 100644 --- a/tests/phpunit/includes/config/GlobalVarConfigTest.php +++ b/tests/phpunit/includes/config/GlobalVarConfigTest.php @@ -8,8 +8,7 @@ class GlobalVarConfigTest extends MediaWikiTestCase { public function testNewInstance() { $config = GlobalVarConfig::newInstance(); $this->assertInstanceOf( GlobalVarConfig::class, $config ); - $this->maybeStashGlobal( 'wgBaz' ); - $GLOBALS['wgBaz'] = 'somevalue'; + $this->setMwGlobals( 'wgBaz', 'somevalue' ); // Check prefix is set to 'wg' $this->assertEquals( 'somevalue', $config->get( 'Baz' ) ); } @@ -21,8 +20,7 @@ class GlobalVarConfigTest extends MediaWikiTestCase { public function testConstructor( $prefix ) { $var = $prefix . 'GlobalVarConfigTest'; $rand = wfRandomString(); - $this->maybeStashGlobal( $var ); - $GLOBALS[$var] = $rand; + $this->setMwGlobals( $var, $rand ); $config = new GlobalVarConfig( $prefix ); $this->assertInstanceOf( GlobalVarConfig::class, $config ); $this->assertEquals( $rand, $config->get( 'GlobalVarConfigTest' ) ); @@ -43,9 +41,7 @@ class GlobalVarConfigTest extends MediaWikiTestCase { * @covers GlobalVarConfig::hasWithPrefix */ public function testHas() { - $this->maybeStashGlobal( 'wgGlobalVarConfigTestHas' ); - $GLOBALS['wgGlobalVarConfigTestHas'] = wfRandomString(); - $this->maybeStashGlobal( 'wgGlobalVarConfigTestNotHas' ); + $this->setMwGlobals( 'wgGlobalVarConfigTestHas', wfRandomString() ); $config = new GlobalVarConfig(); $this->assertTrue( $config->has( 'GlobalVarConfigTestHas' ) ); $this->assertFalse( $config->has( 'GlobalVarConfigTestNotHas' ) ); @@ -87,11 +83,4 @@ class GlobalVarConfigTest extends MediaWikiTestCase { } $this->assertEquals( $config->get( $name ), $expected ); } - - private function maybeStashGlobal( $var ) { - if ( array_key_exists( $var, $GLOBALS ) ) { - // Will be reset after this test is over - $this->stashMwGlobals( $var ); - } - } } diff --git a/tests/phpunit/includes/user/LocalIdLookupTest.php b/tests/phpunit/includes/user/LocalIdLookupTest.php index c91d8e0cb5..6ce01ca885 100644 --- a/tests/phpunit/includes/user/LocalIdLookupTest.php +++ b/tests/phpunit/includes/user/LocalIdLookupTest.php @@ -8,12 +8,9 @@ class LocalIdLookupTest extends MediaWikiTestCase { private $localUsers = []; protected function setUp() { - global $wgGroupPermissions; - parent::setUp(); - $this->stashMwGlobals( [ 'wgGroupPermissions' ] ); - $wgGroupPermissions['local-id-lookup-test']['hideuser'] = true; + $this->setGroupPermissions( 'local-id-lookup-test', 'hideuser', true ); } public function addDBData() { diff --git a/tests/phpunit/tests/MediaWikiTestCaseTest.php b/tests/phpunit/tests/MediaWikiTestCaseTest.php index d2fca72b10..599b733e05 100644 --- a/tests/phpunit/tests/MediaWikiTestCaseTest.php +++ b/tests/phpunit/tests/MediaWikiTestCaseTest.php @@ -72,6 +72,7 @@ class MediaWikiTestCaseTest extends MediaWikiTestCase { * @covers MediaWikiTestCase::tearDown */ public function testStashedGlobalsAreRestoredOnTearDown( $globalKey, $newValue ) { + $this->hideDeprecated( 'MediaWikiTestCase::stashMwGlobals' ); $this->stashMwGlobals( $globalKey ); $GLOBALS[$globalKey] = $newValue; $this->assertEquals(