r79109 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r79108‎ \| r79109 \| r79110 >
Date:	17:15, 28 December 2010
Author:	soxred93
Status:	resolved (Comments)
Tags:
Comment:	It's here! It's finally here! The skies are falling, pigs have sprouted wings, and it's pretty cold in hell! -Add a working PHPUnit test that instantiates a new DB, adds set data, and deleted when done. -Add listTables() to the Database classes (only MySql and SQLite use it, and only MySQL is tested)
Modified paths:	/trunk/phase3/includes/db/Database.php (modified) (history) /trunk/phase3/includes/db/DatabaseMysql.php (modified) (history) /trunk/phase3/includes/db/DatabaseSqlite.php (modified) (history) /trunk/phase3/tests/phpunit/bootstrap.php (modified) (history) /trunk/phase3/tests/phpunit/includes/NewDBTest.php (added) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/bootstrap.php
—	—	@@ -46,13 +46,141 @@
47	47	protected $suite;
48	48	public $regex = '';
49	49	public $runDisabled = false;
	50	+
	51	+ protected static $databaseSetupDone = false;
	52	+ protected $db;
	53	+ protected $dbClone;
	54	+ protected $oldTablePrefix;
	55	+ protected $useTemporaryTables = true;
50	56
51	57	function __construct( PHPUnit_Framework_TestSuite $suite = null ) {
52	58	if ( null !== $suite ) {
53	59	$this->suite = $suite;
54	60	}
	61	+ parent::__construct();
	62	+
	63	+ if( $this->needsDB() && !is_object( $this->dbClone ) ) {
	64	+ $this->initDB();
	65	+ $this->addCoreDBData();
	66	+ $this->addDBData();
	67	+ }
55	68	}
	69	+
	70	+ function __destruct() {
	71	+ if( $this->needsDB() && is_object( $this->dbClone ) && $this->dbClone instanceof CloneDatabase ) {
	72	+ $this->destroyDB();
	73	+ }
	74	+ }
	75	+
	76	+ function needsDB() { return false; }
	77	+
	78	+ function addDBData() {}
	79	+
	80	+ private function addCoreDBData() {
	81	+
	82	+ //Make sysop user
	83	+ $user = User::newFromName( 'UTSysop' );
	84	+ if ( $user->idForName() == 0 ) {
	85	+ $user->addToDatabase();
	86	+ $user->setPassword( 'UTSysopPassword' );
56	87
	88	+ $user->addGroup( 'sysop' );
	89	+ $user->addGroup( 'bureaucrat' );
	90	+ $user->saveSettings();
	91	+ }
	92	+
	93	+
	94	+ //Make 1 page with 1 revision
	95	+ $article = new Article( Title::newFromText( 'UTPage' ) );
	96	+ $article->doEdit( 'UTContent',
	97	+ 'UTPageSummary',
	98	+ EDIT_NEW,
	99	+ false,
	100	+ User::newFromName( 'UTSysop' ) );
	101	+ }
	102	+
	103	+ private function initDB() {
	104	+ global $wgDBprefix;
	105	+
	106	+ if ( self::$databaseSetupDone ) {
	107	+ return;
	108	+ }
	109	+
	110	+ $this->db = wfGetDB( DB_MASTER );
	111	+ $dbType = $this->db->getType();
	112	+
	113	+ if ( $wgDBprefix === 'unittest_' \|\| ( $dbType == 'oracle' && $wgDBprefix === 'ut_' ) ) {
	114	+ throw new MWException( 'Cannot run unit tests, the database prefix is already "unittest_"' );
	115	+ }
	116	+
	117	+ self::$databaseSetupDone = true;
	118	+ $this->oldTablePrefix = $wgDBprefix;
	119	+
	120	+ # SqlBagOStuff broke when using temporary tables on r40209 (bug 15892).
	121	+ # It seems to have been fixed since (r55079?).
	122	+ # If it fails, $wgCaches[CACHE_DB] = new HashBagOStuff(); should work around it.
	123	+
	124	+ # CREATE TEMPORARY TABLE breaks if there is more than one server
	125	+ if ( wfGetLB()->getServerCount() != 1 ) {
	126	+ $this->useTemporaryTables = false;
	127	+ }
	128	+
	129	+ $temporary = $this->useTemporaryTables \|\| $dbType == 'postgres';
	130	+
	131	+ $tables = $this->listTables();
	132	+
	133	+ $prefix = $dbType != 'oracle' ? 'unittest_' : 'ut_';
	134	+
	135	+ $this->dbClone = new CloneDatabase( $this->db, $tables, $prefix );
	136	+ $this->dbClone->useTemporaryTables( $temporary );
	137	+ $this->dbClone->cloneTableStructure();
	138	+
	139	+ if ( $dbType == 'oracle' )
	140	+ $this->db->query( 'BEGIN FILL_WIKI_INFO; END;' );
	141	+
	142	+ if ( $dbType == 'oracle' ) {
	143	+ # Insert 0 user to prevent FK violations
	144	+
	145	+ # Anonymous user
	146	+ $this->db->insert( 'user', array(
	147	+ 'user_id' => 0,
	148	+ 'user_name' => 'Anonymous' ) );
	149	+ }
	150	+
	151	+ }
	152	+
	153	+ private function destroyDB() {
	154	+ if ( !self::$databaseSetupDone ) {
	155	+ return;
	156	+ }
	157	+
	158	+ $this->dbClone->destroy();
	159	+ self::$databaseSetupDone = false;
	160	+
	161	+ if ( $this->useTemporaryTables ) {
	162	+ # Don't need to do anything
	163	+ //return;
	164	+ //Temporary tables seem to be broken ATM, delete anyway
	165	+ }
	166	+
	167	+ if( $this->db->getType() == 'oracle' ) {
	168	+ $tables = $this->db->listTables( 'ut_', __METHOD__ );
	169	+ }
	170	+ else {
	171	+ $tables = $this->db->listTables( 'unittest_', __METHOD__ );
	172	+ }
	173	+
	174	+ foreach ( $tables as $table ) {
	175	+ $sql = $this->db->getType() == 'oracle' ? "DROP TABLE $table DROP CONSTRAINTS" : "DROP TABLE `$table`";
	176	+ $this->db->query( $sql );
	177	+ }
	178	+
	179	+ if ( $this->db->getType() == 'oracle' )
	180	+ $this->db->query( 'BEGIN FILL_WIKI_INFO; END;' );
	181	+
	182	+
	183	+ }
	184	+
57	185	function __call( $func, $args ) {
58	186	if ( method_exists( $this->suite, $func ) ) {
59	187	return call_user_func_array( array( $this->suite, $func ), $args);
—	—	@@ -61,5 +189,14 @@
62	190	. get_class( $this ) );
63	191	}
64	192	}
	193	+
	194	+ protected function listTables() {
	195	+ global $wgDBprefix;
	196	+
	197	+ $tables = $this->db->listTables( $wgDBprefix, __METHOD__ );
	198	+ $tables = array_map( create_function( '$table', 'global $wgDBprefix; return substr( $table, strlen( $wgDBprefix ) );' ), $tables );
	199	+ return $tables;
	200	+
	201	+ }
65	202	}
66	203
Index: trunk/phase3/tests/phpunit/includes/NewDBTest.php
—	—	@@ -0,0 +1,37 @@
	2	+<?php
	3	+
	4	+class NewDBTest extends MediaWikiTestSetup {
	5	+
	6	+ function setUp() {
	7	+ }
	8	+
	9	+ function tearDown() {
	10	+ }
	11	+
	12	+ function addDBData() {
	13	+
	14	+ //Make a page
	15	+ $article = new Article( Title::newFromText( 'Foobar' ) );
	16	+ $article->doEdit( 'FoobarContent',
	17	+ '',
	18	+ EDIT_NEW,
	19	+ false,
	20	+ User::newFromName( 'UTSysop' ) );
	21	+ }
	22	+
	23	+ function needsDB() { return true; }
	24	+
	25	+ function testBootstrapCreation() {
	26	+
	27	+ $article = new Article( Title::newFromText("UTPage") );
	28	+
	29	+ $this->assertEquals("UTContent", $article->fetchContent(), "Automatic main page creation");
	30	+
	31	+ $article = new Article( Title::newFromText("Foobar") );
	32	+
	33	+ $this->assertEquals("FoobarContent", $article->fetchContent(), "addDBData() adds to the database");
	34	+
	35	+ }
	36	+
	37	+}
	38	+
Index: trunk/phase3/includes/db/DatabaseMysql.php
—	—	@@ -530,6 +530,29 @@
531	531	}
532	532	$this->query( $query, $fname );
533	533	}
	534	+
	535	+ /**
	536	+ * List all tables on the database
	537	+ *
	538	+ * @param $prefix Only show tables with this prefix, e.g. mw_
	539	+ * @param $fname String: calling function name
	540	+ */
	541	+ function listTables( $prefix = null, $fname = 'DatabaseMysql::listTables' ) {
	542	+ $result = $this->query( "SHOW TABLES", $fname);
	543	+
	544	+ $endArray = array();
	545	+
	546	+ foreach( $result as $table ) {
	547	+ $vars = get_object_vars($table);
	548	+ $table = array_pop( $vars );
	549	+
	550	+ if( strpos( $table, $prefix ) === 0 \|\| is_null( $prefix ) ) {
	551	+ $endArray[] = $table;
	552	+ }
	553	+ }
	554	+
	555	+ return $endArray;
	556	+ }
534	557
535	558	public function dropTable( $tableName, $fName = 'DatabaseMysql::dropTable' ) {
536	559	if( !$this->tableExists( $tableName ) ) {
Index: trunk/phase3/includes/db/Database.php
—	—	@@ -2288,6 +2288,16 @@
2289	2289	function duplicateTableStructure( $oldName, $newName, $temporary = false, $fname = 'DatabaseBase::duplicateTableStructure' ) {
2290	2290	throw new MWException( 'DatabaseBase::duplicateTableStructure is not implemented in descendant class' );
2291	2291	}
	2292	+
	2293	+ /**
	2294	+ * List all tables on the database
	2295	+ *
	2296	+ * @param $prefix Only show tables with this prefix, e.g. mw_
	2297	+ * @param $fname String: calling function name
	2298	+ */
	2299	+ function listTables( $prefix = null, $fname = 'DatabaseBase::listTables' ) {
	2300	+ throw new MWException( 'DatabaseBase::listTables is not implemented in descendant class' );
	2301	+ }
2292	2302
2293	2303	/**
2294	2304	* Return MW-style timestamp used for MySQL schema
Index: trunk/phase3/includes/db/DatabaseSqlite.php
—	—	@@ -608,6 +608,34 @@
609	609	$sql = preg_replace( '/\b' . preg_quote( $oldName ) . '\b/', $newName, $sql, 1 );
610	610	return $this->query( $sql, $fname );
611	611	}
	612	+
	613	+
	614	+ /**
	615	+ * List all tables on the database
	616	+ *
	617	+ * @param $prefix Only show tables with this prefix, e.g. mw_
	618	+ * @param $fname String: calling function name
	619	+ */
	620	+ function listTables( $prefix = null, $fname = 'DatabaseSqlite::listTables' ) {
	621	+ $result = $this->select(
	622	+ 'sqlite_master',
	623	+ 'name',
	624	+ "type='TABLE'"
	625	+ );
	626	+
	627	+ $endArray = array();
	628	+
	629	+ foreach( $result as $table ) {
	630	+ $vars = get_object_vars($table);
	631	+ $table = array_pop( $vars );
	632	+
	633	+ if( strpos( $table, $prefix ) === 0 \|\| is_null( $prefix ) ) {
	634	+ $endArray[] = $table;
	635	+ }
	636	+ }
	637	+
	638	+ return $endArray;
	639	+ }
612	640
613	641	} // end DatabaseSqlite class
614	642

Follow-up revisions

Revision	Commit summary	Author	Date
r79114	Followup to r79109: Use a clean DB by default for each test	soxred93	18:10, 28 December 2010
r79119	Fix r79109: listTables() does not work without a prefix, and verified that SQ...	soxred93	18:30, 28 December 2010
r79173	Follow up r79109....	platonides	15:01, 29 December 2010
r79257	Follow up r79109. Do not use create_function.	platonides	16:24, 30 December 2010
r91029	Remove superfluous NewDBTest, doesn't test anything afaict	demon	23:03, 28 June 2011

Comments

#Comment by MaxSem (talk | contribs) 17:29, 28 December 2010

+ "type='TABLE'" It should be lowercase.

+ if( strpos( $table, $prefix ) === 0 || is_null( $prefix ) ) { Use LIKE?

#Comment by X! (talk | contribs) 18:14, 28 December 2010

LIKE is ugly, and I don't think it would work if prefix is null. It is also probably faster this way.

#Comment by MaxSem (talk | contribs) 21:19, 28 December 2010

Then how about using it for filtering out system tables? :P

#Comment by 😂 (talk | contribs) 17:32, 28 December 2010

Having to manually specify the prefix to listTables() seems confusing. How about if prefix is null, it returns all the MW tables (eg: $this->tablePrefix()), empty string would return all tables.

#Comment by Platonides (talk | contribs) 12:35, 29 December 2010

Breaks databaseless tests. They no longer work without a db.

Status & tagging log

22:26, 25 May 2011 😂 (talk | contribs) changed the status of r79109 [removed: new added: resolved]
15:08, 29 December 2010 Platonides (talk | contribs) changed the status of r79109 [removed: fixme added: new]
12:35, 29 December 2010 Platonides (talk | contribs) changed the status of r79109 [removed: new added: fixme]
17:32, 28 December 2010 😂 (talk | contribs) changed the status of r79109 [removed: fixme added: new]
17:29, 28 December 2010 MaxSem (talk | contribs) changed the status of r79109 [removed: new added: fixme]