From 40b2e059c002ce46b9764ba11503ad6a07dbdc09 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 13 Apr 2018 01:39:22 -0700 Subject: [PATCH] Reset table sequences and skip some test assertions for sqlite Various revision storage tests assume the sequences are reset and fail otherwise. Also disable some failing tests that are not applicable to sqlite as well as postgres. Change-Id: Ibdb034121a44e16bb35059a92baafb1867951ea8 --- .../libs/rdbms/database/DatabaseSqlite.php | 22 ++++-- tests/phpunit/MediaWikiTestCase.php | 2 +- tests/phpunit/includes/db/LBFactoryTest.php | 75 ++++++++++--------- .../phpunit/includes/db/LoadBalancerTest.php | 56 ++++++++------ 4 files changed, 87 insertions(+), 68 deletions(-) diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php index 601a62fef7..a6a153a387 100644 --- a/includes/libs/rdbms/database/DatabaseSqlite.php +++ b/includes/libs/rdbms/database/DatabaseSqlite.php @@ -120,7 +120,7 @@ class DatabaseSqlite extends Database { protected function doInitConnection() { if ( $this->dbPath !== null ) { // Standalone .sqlite file mode. - $this->openFile( $this->dbPath ); + $this->openFile( $this->dbPath, $this->connectionParams['dbname'] ); } elseif ( $this->dbDir !== null ) { // Stock wiki mode using standard file names per DB if ( strlen( $this->connectionParams['dbname'] ) ) { @@ -173,11 +173,7 @@ class DatabaseSqlite extends Database { $this->conn = false; throw new DBConnectionError( $this, "SQLite database not accessible" ); } - $this->openFile( $fileName ); - - if ( $this->conn ) { - $this->dbName = $dbName; - } + $this->openFile( $fileName, $dbName ); return (bool)$this->conn; } @@ -186,10 +182,11 @@ class DatabaseSqlite extends Database { * Opens a database file * * @param string $fileName + * @param string $dbName * @throws DBConnectionError * @return PDO|bool SQL connection or false if failed */ - protected function openFile( $fileName ) { + protected function openFile( $fileName, $dbName ) { $err = false; $this->dbPath = $fileName; @@ -211,6 +208,7 @@ class DatabaseSqlite extends Database { $this->opened = is_object( $this->conn ); if ( $this->opened ) { + $this->dbName = $dbName; # Set error codes only, don't raise exceptions $this->conn->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT ); # Enforce LIKE to be case sensitive, just like MySQL @@ -1081,6 +1079,16 @@ class DatabaseSqlite extends Database { } } + public function resetSequenceForTable( $table, $fname = __METHOD__ ) { + $encTable = $this->addIdentifierQuotes( 'sqlite_sequence' ); + $encName = $this->addQuotes( $this->tableName( $table, 'raw' ) ); + $this->query( "DELETE FROM $encTable WHERE name = $encName", $fname ); + } + + public function databasesAreIndependent() { + return true; + } + /** * @return string */ diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php index 8b2d099227..87ca918138 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiTestCase.php @@ -1570,7 +1570,7 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { $db->delete( $tbl, '*', __METHOD__ ); } - if ( $db->getType() === 'postgres' ) { + if ( in_array( $db->getType(), [ 'postgres', 'sqlite' ], true ) ) { // Reset the table's sequence too. $db->resetSequenceForTable( $tbl, __METHOD__ ); } diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php index 2892283a19..ed4c977f9f 100644 --- a/tests/phpunit/includes/db/LBFactoryTest.php +++ b/tests/phpunit/includes/db/LBFactoryTest.php @@ -345,32 +345,25 @@ class LBFactoryTest extends MediaWikiTestCase { } public function testNiceDomains() { - global $wgDBname, $wgDBtype; - - if ( $wgDBtype === 'sqlite' ) { - $tmpDir = $this->getNewTempDirectory(); - $dbPath = "$tmpDir/unit_test_db.sqlite"; - file_put_contents( $dbPath, '' ); - $tempFsFile = new TempFSFile( $dbPath ); - $tempFsFile->autocollect(); - } else { - $dbPath = null; + global $wgDBname; + + if ( wfGetDB( DB_MASTER )->databasesAreIndependent() ) { + self::markTestSkipped( "Skipping tests about selecting DBs: not applicable" ); + return; } $factory = $this->newLBFactoryMulti( [], - [ 'dbFilePath' => $dbPath ] + [] ); $lb = $factory->getMainLB(); - if ( $wgDBtype !== 'sqlite' ) { - $db = $lb->getConnectionRef( DB_MASTER ); - $this->assertEquals( - wfWikiID(), - $db->getDomainID() - ); - unset( $db ); - } + $db = $lb->getConnectionRef( DB_MASTER ); + $this->assertEquals( + wfWikiID(), + $db->getDomainID() + ); + unset( $db ); /** @var Database $db */ $db = $lb->getConnection( DB_MASTER, [], '' ); @@ -411,6 +404,7 @@ class LBFactoryTest extends MediaWikiTestCase { $db = $lb->getConnection( DB_MASTER ); // local domain connection $factory->setDomainPrefix( 'my_' ); + $this->assertEquals( $wgDBname, $db->getDBname() ); $this->assertEquals( "$wgDBname-my_", $db->getDomainID() @@ -431,23 +425,17 @@ class LBFactoryTest extends MediaWikiTestCase { } public function testTrickyDomain() { - global $wgDBtype, $wgDBname; - - if ( $wgDBtype === 'sqlite' ) { - $tmpDir = $this->getNewTempDirectory(); - $dbPath = "$tmpDir/unit_test_db.sqlite"; - file_put_contents( $dbPath, '' ); - $tempFsFile = new TempFSFile( $dbPath ); - $tempFsFile->autocollect(); - } else { - $dbPath = null; + global $wgDBname; + + if ( wfGetDB( DB_MASTER )->databasesAreIndependent() ) { + self::markTestSkipped( "Skipping tests about selecting DBs: not applicable" ); + return; } $dbname = 'unittest-domain'; // explodes if DB is selected $factory = $this->newLBFactoryMulti( [ 'localDomain' => ( new DatabaseDomain( $dbname, null, '' ) )->getId() ], [ - 'dbFilePath' => $dbPath, 'dbName' => 'do_not_select_me' // explodes if DB is selected ] ); @@ -496,7 +484,27 @@ class LBFactoryTest extends MediaWikiTestCase { "Correct full table name" ); - if ( $db->databasesAreIndependent() ) { + $lb->reuseConnection( $db ); // don't care + + $factory->closeAll(); + $factory->destroy(); + } + + public function testInvalidSelectDB() { + $dbname = 'unittest-domain'; // explodes if DB is selected + $factory = $this->newLBFactoryMulti( + [ 'localDomain' => ( new DatabaseDomain( $dbname, null, '' ) )->getId() ], + [ + 'dbName' => 'do_not_select_me' // explodes if DB is selected + ] + ); + $lb = $factory->getMainLB(); + /** @var Database $db */ + $db = $lb->getConnection( DB_MASTER, [], '' ); + + if ( $db->getType() === 'sqlite' ) { + $this->assertFalse( $db->selectDB( 'garbage-db' ) ); + } elseif ( $db->databasesAreIndependent() ) { try { $e = null; $db->selectDB( 'garbage-db' ); @@ -510,11 +518,6 @@ class LBFactoryTest extends MediaWikiTestCase { $this->assertFalse( $db->selectDB( 'garbage-db' ) ); \Wikimedia\restoreWarnings(); } - - $lb->reuseConnection( $db ); // don't care - - $factory->closeAll(); - $factory->destroy(); } private function quoteTable( Database $db, $table ) { diff --git a/tests/phpunit/includes/db/LoadBalancerTest.php b/tests/phpunit/includes/db/LoadBalancerTest.php index cd48d90fa2..cb29975214 100644 --- a/tests/phpunit/includes/db/LoadBalancerTest.php +++ b/tests/phpunit/includes/db/LoadBalancerTest.php @@ -67,20 +67,24 @@ class LoadBalancerTest extends MediaWikiTestCase { $this->assertTrue( $dbr->getLBInfo( 'master' ), 'DB_REPLICA also gets the master' ); $this->assertTrue( $dbr->getFlag( $dbw::DBO_TRX ), "DBO_TRX set on replica" ); - $dbwAuto = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTOCOMMIT ); - $this->assertFalse( - $dbwAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTOCOMMIT" ); - $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on master" ); - $this->assertNotEquals( $dbw, $dbwAuto, "CONN_TRX_AUTOCOMMIT uses separate connection" ); + if ( !$lb->getServerAttributes( $lb->getWriterIndex() )[$dbw::ATTR_DB_LEVEL_LOCKING] ) { + $dbwAuto = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTOCOMMIT ); + $this->assertFalse( + $dbwAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTOCOMMIT" ); + $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on master" ); + $this->assertNotEquals( + $dbw, $dbwAuto, "CONN_TRX_AUTOCOMMIT uses separate connection" ); - $dbrAuto = $lb->getConnection( DB_REPLICA, [], false, $lb::CONN_TRX_AUTOCOMMIT ); - $this->assertFalse( - $dbrAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTOCOMMIT" ); - $this->assertTrue( $dbr->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on replica" ); - $this->assertNotEquals( $dbr, $dbrAuto, "CONN_TRX_AUTOCOMMIT uses separate connection" ); + $dbrAuto = $lb->getConnection( DB_REPLICA, [], false, $lb::CONN_TRX_AUTOCOMMIT ); + $this->assertFalse( + $dbrAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTOCOMMIT" ); + $this->assertTrue( $dbr->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on replica" ); + $this->assertNotEquals( + $dbr, $dbrAuto, "CONN_TRX_AUTOCOMMIT uses separate connection" ); - $dbwAuto2 = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTOCOMMIT ); - $this->assertEquals( $dbwAuto2, $dbwAuto, "CONN_TRX_AUTOCOMMIT reuses connections" ); + $dbwAuto2 = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTOCOMMIT ); + $this->assertEquals( $dbwAuto2, $dbwAuto, "CONN_TRX_AUTOCOMMIT reuses connections" ); + } $lb->closeAll(); } @@ -137,20 +141,24 @@ class LoadBalancerTest extends MediaWikiTestCase { $this->assertTrue( $dbr->getFlag( $dbw::DBO_TRX ), "DBO_TRX set on replica" ); $this->assertWriteForbidden( $dbr ); - $dbwAuto = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTOCOMMIT ); - $this->assertFalse( - $dbwAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTOCOMMIT" ); - $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on master" ); - $this->assertNotEquals( $dbw, $dbwAuto, "CONN_TRX_AUTOCOMMIT uses separate connection" ); + if ( !$lb->getServerAttributes( $lb->getWriterIndex() )[$dbw::ATTR_DB_LEVEL_LOCKING] ) { + $dbwAuto = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTOCOMMIT ); + $this->assertFalse( + $dbwAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTOCOMMIT" ); + $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on master" ); + $this->assertNotEquals( + $dbw, $dbwAuto, "CONN_TRX_AUTOCOMMIT uses separate connection" ); - $dbrAuto = $lb->getConnection( DB_REPLICA, [], false, $lb::CONN_TRX_AUTOCOMMIT ); - $this->assertFalse( - $dbrAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTOCOMMIT" ); - $this->assertTrue( $dbr->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on replica" ); - $this->assertNotEquals( $dbr, $dbrAuto, "CONN_TRX_AUTOCOMMIT uses separate connection" ); + $dbrAuto = $lb->getConnection( DB_REPLICA, [], false, $lb::CONN_TRX_AUTOCOMMIT ); + $this->assertFalse( + $dbrAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTOCOMMIT" ); + $this->assertTrue( $dbr->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on replica" ); + $this->assertNotEquals( + $dbr, $dbrAuto, "CONN_TRX_AUTOCOMMIT uses separate connection" ); - $dbwAuto2 = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTOCOMMIT ); - $this->assertEquals( $dbwAuto2, $dbwAuto, "CONN_TRX_AUTOCOMMIT reuses connections" ); + $dbwAuto2 = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTOCOMMIT ); + $this->assertEquals( $dbwAuto2, $dbwAuto, "CONN_TRX_AUTOCOMMIT reuses connections" ); + } $lb->closeAll(); } -- 2.20.1