tests: Remove use of wfRandomString() for test fixtures
authorTimo Tijhof <krinklemail@gmail.com>
Sat, 4 May 2019 20:41:10 +0000 (21:41 +0100)
committerKrinkle <krinklemail@gmail.com>
Mon, 6 May 2019 17:22:03 +0000 (17:22 +0000)
This reduces confidence in the test. There is no guruantee that
it won't return the same value twice during the duration of a full
PHPUnit run of all test suites, whether twice in a row or 20 minutes
apart.

For a test that needs a string of any kind, use an explicit, consinstent
and cheap literal value.

For a test that specifically needs some kind of uniqueness compared to
something else within the same test case, do so explicitly.

Tests that require something globally unique (for some undefined/vague
definition of "global") were not found, and should not exist anyway.

Also, in libs/objectcache tests, fix order of parameters in some
assertions (expected first, then actual), and use assertFalse/assertSame
instead of assertEqual for cases where false is expected to remove
tolerance of other loosely equal values.

Change-Id: Ifc60e88178da471330b94bfbf12e2731d2efc77d

tests/phpunit/includes/GlobalFunctions/GlobalTest.php
tests/phpunit/includes/TestUserRegistry.php
tests/phpunit/includes/config/GlobalVarConfigTest.php
tests/phpunit/includes/filebackend/FileBackendTest.php
tests/phpunit/includes/jobqueue/JobQueueTest.php
tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php
tests/phpunit/includes/libs/objectcache/ReplicatedBagOStuffTest.php

index 9443b19..1210a50 100644 (file)
@@ -74,12 +74,8 @@ class GlobalTest extends MediaWikiTestCase {
                $this->assertFalse(
                        wfRandomString() == wfRandomString()
                );
-               $this->assertEquals(
-                       strlen( wfRandomString( 10 ) ), 10
-               );
-               $this->assertTrue(
-                       preg_match( '/^[0-9a-f]+$/i', wfRandomString() ) === 1
-               );
+               $this->assertSame( 10, strlen( wfRandomString( 10 ) ), 'length' );
+               $this->assertSame( 1, preg_match( '/^[0-9a-f]+$/i', wfRandomString() ), 'pattern' );
        }
 
        /**
index 3064a3d..40a5dc5 100644 (file)
@@ -33,7 +33,7 @@ class TestUserRegistry {
         */
        public static function getMutableTestUser( $testName, $groups = [] ) {
                $id = self::getNextId();
-               $password = wfRandomString( 20 );
+               $password = "password_for_test_user_id_{$id}";
                $testUser = new TestUser(
                        "TestUser $testName $id",  // username
                        "Name $id",                // real name
@@ -75,7 +75,7 @@ class TestUserRegistry {
                                $password = 'UTSysopPassword';
                        } else {
                                $username = "TestUser $id";
-                               $password = wfRandomString( 20 );
+                               $password = "password_for_test_user_id_{$id}";
                        }
                        self::$testUsers[$key] = $testUser = new TestUser(
                                $username,            // username
index ec443e7..591f27d 100644 (file)
@@ -19,11 +19,10 @@ class GlobalVarConfigTest extends MediaWikiTestCase {
         */
        public function testConstructor( $prefix ) {
                $var = $prefix . 'GlobalVarConfigTest';
-               $rand = wfRandomString();
-               $this->setMwGlobals( $var, $rand );
+               $this->setMwGlobals( $var, 'testvalue' );
                $config = new GlobalVarConfig( $prefix );
                $this->assertInstanceOf( GlobalVarConfig::class, $config );
-               $this->assertEquals( $rand, $config->get( 'GlobalVarConfigTest' ) );
+               $this->assertEquals( 'testvalue', $config->get( 'GlobalVarConfigTest' ) );
        }
 
        public static function provideConstructor() {
@@ -41,7 +40,7 @@ class GlobalVarConfigTest extends MediaWikiTestCase {
         * @covers GlobalVarConfig::hasWithPrefix
         */
        public function testHas() {
-               $this->setMwGlobals( 'wgGlobalVarConfigTestHas', wfRandomString() );
+               $this->setMwGlobals( 'wgGlobalVarConfigTestHas', 'testvalue' );
                $config = new GlobalVarConfig();
                $this->assertTrue( $config->has( 'GlobalVarConfigTestHas' ) );
                $this->assertFalse( $config->has( 'GlobalVarConfigTestNotHas' ) );
index 50696af..8548fde 100644 (file)
@@ -98,7 +98,7 @@ class FileBackendTest extends MediaWikiTestCase {
                        'name' => 'localtesting',
                        'lockManager' => LockManagerGroup::singleton()->get( 'fsLockManager' ),
                        'parallelize' => 'implicit',
-                       'wikiId' => wfWikiID() . wfRandomString(),
+                       'wikiId' => 'testdb',
                        'backends' => [
                                [
                                        'name' => 'localmultitesting1',
@@ -2569,11 +2569,9 @@ class FileBackendTest extends MediaWikiTestCase {
                        'wikiId' => wfWikiID()
                ] ) );
 
-               $name = wfRandomString( 300 );
-
                $input = [
                        'headers' => [
-                               'content-Disposition' => FileBackend::makeContentDisposition( 'inline', $name ),
+                               'content-Disposition' => FileBackend::makeContentDisposition( 'inline', 'name' ),
                                'Content-dUration' => 25.6,
                                'X-LONG-VALUE' => str_pad( '0', 300 ),
                                'CONTENT-LENGTH' => 855055,
@@ -2581,7 +2579,7 @@ class FileBackendTest extends MediaWikiTestCase {
                ];
                $expected = [
                        'headers' => [
-                               'content-disposition' => FileBackend::makeContentDisposition( 'inline', $name ),
+                               'content-disposition' => FileBackend::makeContentDisposition( 'inline', 'name' ),
                                'content-duration' => 25.6,
                                'content-length' => 855055
                        ]
index 1baaa54..ce07f78 100644 (file)
@@ -259,8 +259,7 @@ class JobQueueTest extends MediaWikiTestCase {
                $this->assertEquals( 0, $queue->getSize(), "Queue is empty ($desc)" );
                $this->assertEquals( 0, $queue->getAcquiredCount(), "Queue is empty ($desc)" );
 
-               $id = wfRandomString( 32 );
-               $root1 = Job::newRootJobParams( "nulljobspam:$id" ); // task ID/timestamp
+               $root1 = Job::newRootJobParams( "nulljobspam:testId" ); // task ID/timestamp
                for ( $i = 0; $i < 5; ++$i ) {
                        $this->assertNull( $queue->push( $this->newJob( 0, $root1 ) ), "Push worked ($desc)" );
                }
index 0376803..9f88474 100644 (file)
@@ -28,8 +28,8 @@ class MultiWriteBagOStuffTest extends MediaWikiTestCase {
         * @covers MultiWriteBagOStuff::doWrite
         */
        public function testSetImmediate() {
-               $key = wfRandomString();
-               $value = wfRandomString();
+               $key = 'key';
+               $value = 'value';
                $this->cache->set( $key, $value );
 
                // Set in tier 1
@@ -42,8 +42,8 @@ class MultiWriteBagOStuffTest extends MediaWikiTestCase {
         * @covers MultiWriteBagOStuff
         */
        public function testSyncMerge() {
-               $key = wfRandomString();
-               $value = wfRandomString();
+               $key = 'keyA';
+               $value = 'value';
                $func = function () use ( $value ) {
                        return $value;
                };
@@ -56,14 +56,14 @@ class MultiWriteBagOStuffTest extends MediaWikiTestCase {
                // Set in tier 1
                $this->assertEquals( $value, $this->cache1->get( $key ), 'Written to tier 1' );
                // Not yet set in tier 2
-               $this->assertEquals( false, $this->cache2->get( $key ), 'Not written to tier 2' );
+               $this->assertFalse( $this->cache2->get( $key ), 'Not written to tier 2' );
 
                $dbw->commit();
 
                // Set in tier 2
                $this->assertEquals( $value, $this->cache2->get( $key ), 'Written to tier 2' );
 
-               $key = wfRandomString();
+               $key = 'keyB';
 
                $dbw->begin();
                $this->cache->merge( $key, $func, 0, 1, BagOStuff::WRITE_SYNC );
@@ -80,8 +80,8 @@ class MultiWriteBagOStuffTest extends MediaWikiTestCase {
         * @covers MultiWriteBagOStuff::set
         */
        public function testSetDelayed() {
-               $key = wfRandomString();
-               $value = (object)[ 'v' => wfRandomString() ];
+               $key = 'key';
+               $value = (object)[ 'v' => 'saved value' ];
                $expectValue = clone $value;
 
                // XXX: DeferredUpdates bound to transactions in CLI mode
@@ -90,12 +90,12 @@ class MultiWriteBagOStuffTest extends MediaWikiTestCase {
                $this->cache->set( $key, $value );
 
                // Test that later changes to $value don't affect the saved value (e.g. T168040)
-               $value->v = 'bogus';
+               $value->v = 'other value';
 
                // Set in tier 1
                $this->assertEquals( $expectValue, $this->cache1->get( $key ), 'Written to tier 1' );
                // Not yet set in tier 2
-               $this->assertEquals( false, $this->cache2->get( $key ), 'Not written to tier 2' );
+               $this->assertFalse( $this->cache2->get( $key ), 'Not written to tier 2' );
 
                $dbw->commit();
 
index b7f22ec..550ec0b 100644 (file)
@@ -23,40 +23,40 @@ class ReplicatedBagOStuffTest extends MediaWikiTestCase {
         * @covers ReplicatedBagOStuff::set
         */
        public function testSet() {
-               $key = wfRandomString();
-               $value = wfRandomString();
+               $key = 'a key';
+               $value = 'a value';
                $this->cache->set( $key, $value );
 
                // Write to master.
-               $this->assertEquals( $this->writeCache->get( $key ), $value );
+               $this->assertEquals( $value, $this->writeCache->get( $key ) );
                // Don't write to replica. Replication is deferred to backend.
-               $this->assertEquals( $this->readCache->get( $key ), false );
+               $this->assertFalse( $this->readCache->get( $key ) );
        }
 
        /**
         * @covers ReplicatedBagOStuff::get
         */
        public function testGet() {
-               $key = wfRandomString();
+               $key = 'a key';
 
-               $write = wfRandomString();
+               $write = 'one value';
                $this->writeCache->set( $key, $write );
-               $read = wfRandomString();
+               $read = 'another value';
                $this->readCache->set( $key, $read );
 
                // Read from replica.
-               $this->assertEquals( $this->cache->get( $key ), $read );
+               $this->assertEquals( $read, $this->cache->get( $key ) );
        }
 
        /**
         * @covers ReplicatedBagOStuff::get
         */
        public function testGetAbsent() {
-               $key = wfRandomString();
-               $value = wfRandomString();
+               $key = 'a key';
+               $value = 'a value';
                $this->writeCache->set( $key, $value );
 
                // Don't read from master. No failover if value is absent.
-               $this->assertEquals( $this->cache->get( $key ), false );
+               $this->assertFalse( $this->cache->get( $key ) );
        }
 }