Merge "Fix guarding of MySQL's numRows()"

diff --git a/includes/api/ApiFormatJson.php b/includes/api/ApiFormatJson.php
index 2f63faf..9dcde8f 100644 (file)
--- a/includes/api/ApiFormatJson.php
+++ b/includes/api/ApiFormatJson.php
@@ -81,7 +81,9 @@ class ApiFormatJson extends ApiFormatBase {
 
                                default:
                                        // Should have been caught during parameter validation
+                                       // @codeCoverageIgnoreStart
                                        $this->dieDebug( __METHOD__, 'Unknown value for \'formatversion\'' );
+                                       // @codeCoverageIgnoreEnd
                        }
                }
                $data = $this->getResult()->getResultData( null, $transform );

diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php
index dfd4309..feab0df 100644 (file)
--- 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 fb697ff..3fa8539 100644 (file)
--- 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 2d7f8a4..9587a76 100644 (file)
--- 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 d5d33fb..61512d5 100644 (file)
--- 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 c8ecda2..2534ad3 100644 (file)
--- 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/api/format/ApiFormatJsonTest.php b/tests/phpunit/includes/api/format/ApiFormatJsonTest.php
index 7eb2a35..2408518 100644 (file)
--- a/tests/phpunit/includes/api/format/ApiFormatJsonTest.php
+++ b/tests/phpunit/includes/api/format/ApiFormatJsonTest.php
@@ -9,14 +9,20 @@ class ApiFormatJsonTest extends ApiFormatTestBase {
        protected $printerName = 'json';
 
        private static function addFormatVersion( $format, $arr ) {
-               foreach ( $arr as &$p ) {
-                       if ( !isset( $p[2] ) ) {
-                               $p[2] = [ 'formatversion' => $format ];
-                       } else {
-                               $p[2]['formatversion'] = $format;
+               $ret = [];
+               foreach ( $arr as $val ) {
+                       if ( !isset( $val[2] ) ) {
+                               $val[2] = [];
+                       }
+                       $val[2]['formatversion'] = $format;
+                       $ret[] = $val;
+                       if ( $format === 2 ) {
+                               // Add a test for 'latest' as well
+                               $val[2]['formatversion'] = 'latest';
+                               $ret[] = $val;
                        }
                }
-               return $arr;
+               return $ret;
        }
 
        public static function provideGeneralEncoding() {
@@ -123,6 +129,7 @@ class ApiFormatJsonTest extends ApiFormatTestBase {
                                // Cross-domain mangling
                                [ [ '< Cross-Domain-Policy >' ], '["\u003C Cross-Domain-Policy >"]' ],
                        ] )
+                       // @todo Test rawfm
                );
        }
 

diff --git a/tests/phpunit/includes/auth/AuthManagerTest.php b/tests/phpunit/includes/auth/AuthManagerTest.php
index 0c5d026..6bdc569 100644 (file)
--- 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 ab4a174..54538ed 100644 (file)
--- 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 db5f73d..ec443e7 100644 (file)
--- 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/session/PHPSessionHandlerTest.php b/tests/phpunit/includes/session/PHPSessionHandlerTest.php
index b001eb9..b191a2f 100644 (file)
--- a/tests/phpunit/includes/session/PHPSessionHandlerTest.php
+++ b/tests/phpunit/includes/session/PHPSessionHandlerTest.php
@@ -17,7 +17,7 @@ class PHPSessionHandlerTest extends MediaWikiTestCase {
 
                // Ignore "headers already sent" warnings during this test
                set_error_handler( function ( $errno, $errstr ) use ( &$warnings ) {
-                       if ( preg_match( '/headers already sent/', $errstr ) ) {
+                       if ( preg_match( '/[hH]eaders already sent/', $errstr ) ) {
                                return true;
                        }
                        return false;

diff --git a/tests/phpunit/includes/user/LocalIdLookupTest.php b/tests/phpunit/includes/user/LocalIdLookupTest.php
index c91d8e0..6ce01ca 100644 (file)
--- 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 d2fca72..599b733 100644 (file)
--- 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(