From 4cacea3b72c0f0ddc7fe83d60f98111f85bd3c32 Mon Sep 17 00:00:00 2001 From: nicolasconnault Date: Wed, 21 May 2008 06:34:56 +0000 Subject: [PATCH] MDL-14905 Completed all core functional tests for database_manager (ddl). Found some API design issues and commented on them using @TODO comments. --- lib/ddl/database_manager.php | 14 ++++- lib/ddl/simpletest/fixtures/invalid.xml | 19 +++++++ lib/ddl/simpletest/fixtures/xmldb_table.xml | 19 +++++++ lib/ddl/simpletest/testddllib.php | 61 ++++++++++++++++++++- 4 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 lib/ddl/simpletest/fixtures/invalid.xml create mode 100644 lib/ddl/simpletest/fixtures/xmldb_table.xml diff --git a/lib/ddl/database_manager.php b/lib/ddl/database_manager.php index cb0d9ec7f5..da7f2830fa 100644 --- a/lib/ddl/database_manager.php +++ b/lib/ddl/database_manager.php @@ -364,7 +364,7 @@ class database_manager { } /** - * This function will all tables found in XMLDB file from db + * This function will delete all tables found in XMLDB file from db * * @param $file full path to the XML file to be used * @param $feedback @@ -411,6 +411,10 @@ class database_manager { * @param boolean continue to specify if must continue on error (true) or stop (false) * @param boolean feedback to specify to show status info (true) or not (false) * @return boolean true on success, false on error + * @TODO I don't think returning TRUE when trying to drop a non-existing table is a good idea. + * the point is, the method name is drop_table, and if it doesn't drop a table, + * it should be obvious to the code calling this method, and not rely on the visual + * feedback of debugging(). Exception handling may solve this. */ public function drop_table($xmldb_table, $continue=true, $feedback=true) { if (!($xmldb_table instanceof xmldb_table)) { @@ -483,6 +487,10 @@ class database_manager { * @param boolean continue to specify if must continue on error (true) or stop (false) * @param boolean feedback to specify to show status info (true) or not (false) * @return boolean true on success, false on error + * @TODO I don't think returning TRUE when trying to create an existing table is a good idea. + * the point is, the method name is create_table, and if it doesn't create a table, + * it should be obvious to the code calling this method, and not rely on the visual + * feedback of debugging(). Exception handling may solve this. */ public function create_table($xmldb_table, $continue=true, $feedback=true) { if (!($xmldb_table instanceof xmldb_table)) { @@ -515,6 +523,10 @@ class database_manager { * NOTE: The return value is the tablename - some DBs (MSSQL at least) use special * names for temp tables. * + * @TODO There is no way to know, from the return value alone, whether a table was actually created + * or not: if an existing table is given as param, its name will be returned, but no DB action + * will have occurred. This should be remedied using an Exception + * * @param xmldb_table table object (full specs are required) * @param boolean continue to specify if must continue on error (true) or stop (false) * @param boolean feedback to specify to show status info (true) or not (false) diff --git a/lib/ddl/simpletest/fixtures/invalid.xml b/lib/ddl/simpletest/fixtures/invalid.xml new file mode 100644 index 0000000000..7360410405 --- /dev/null +++ b/lib/ddl/simpletest/fixtures/invalid.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + +
+ + diff --git a/lib/ddl/simpletest/fixtures/xmldb_table.xml b/lib/ddl/simpletest/fixtures/xmldb_table.xml new file mode 100644 index 0000000000..1d1b224e3c --- /dev/null +++ b/lib/ddl/simpletest/fixtures/xmldb_table.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + +
+ + diff --git a/lib/ddl/simpletest/testddllib.php b/lib/ddl/simpletest/testddllib.php index 7893616f3c..ee1732dde8 100755 --- a/lib/ddl/simpletest/testddllib.php +++ b/lib/ddl/simpletest/testddllib.php @@ -94,12 +94,29 @@ class ddllib_test extends UnitTestCase { $this->assertTrue($this->dbmanager->create_table($table)); $this->assertTrue($this->dbmanager->table_exists("other_test_table")); $this->dbmanager->drop_table($table); + + // Give existing table as argument + $table = $this->tables[1]; + $this->assertFalse($this->dbmanager->create_table($table)); + + // Give a wrong table param + $table = 'string'; + $this->assertFalse($this->dbmanager->create_table($table)); + } public function testDropTable() { $table = $this->tables[0]; $this->assertTrue($this->dbmanager->drop_table($table, true, false)); $this->assertFalse($this->dbmanager->table_exists("testtable")); + + // Try dropping non-existent table + $table = new xmldb_table('nonexistenttable'); + $this->assertFalse($this->dbmanager->drop_table($table, true, false)); + + // Give a wrong table param + $table = 'string'; + $this->assertFalse($this->dbmanager->drop_table($table, true, false)); } public function testAddEnumField() { @@ -429,16 +446,58 @@ class ddllib_test extends UnitTestCase { } public function testDeleteTablesFromXmldbFile() { + global $CFG; + $this->assertTrue($this->dbmanager->table_exists('anothertest')); + + // feed nonexistent file + $this->assertFalse($this->dbmanager->delete_tables_from_xmldb_file('fpsoiudfposui', false)); + + // Real file but invalid xml file + $this->assertFalse($this->dbmanager->delete_tables_from_xmldb_file($CFG->libdir . '/ddl/simpletest/fixtures/invalid.xml', false)); + + // Check that the table has not been deleted from DB + $this->assertTrue($this->dbmanager->table_exists('anothertest')); + + // Real and valid xml file + $this->assertTrue($this->dbmanager->delete_tables_from_xmldb_file($CFG->libdir . '/ddl/simpletest/fixtures/xmldb_table.xml', false)); + // Check that the table has been deleted from DB + $this->assertFalse($this->dbmanager->table_exists('anothertest')); } public function testInstallFromXmldbFile() { + global $CFG; + // First delete existing test table to make room for new one + $table = $this->tables[1]; + $this->dbmanager->drop_table($table); + $this->assertFalse($this->dbmanager->table_exists('anothertest')); + + // feed nonexistent file + $this->assertFalse($this->dbmanager->install_from_xmldb_file('fpsoiudfposui', false)); + + // Real but invalid xml file + $this->assertFalse($this->dbmanager->install_from_xmldb_file($CFG->libdir . '/ddl/simpletest/fixtures/invalid.xml', false)); + // Check that the table has not yet been created in DB + $this->assertFalse($this->dbmanager->table_exists('anothertest')); + + // Real and valid xml file + $this->assertTrue($this->dbmanager->install_from_xmldb_file($CFG->libdir . '/ddl/simpletest/fixtures/xmldb_table.xml', false)); + $this->assertTrue($this->dbmanager->table_exists('anothertest')); } public function testCreateTempTable() { + // Feed incorrect table param + $this->assertFalse($this->dbmanager->create_temp_table('anothertest')); + + // Correct table but with existing name + $table = $this->tables[0]; + $this->assertEqual('testtable', $this->dbmanager->create_temp_table($table)); + + // New table + $this->dbmanager->drop_table($this->tables[0]); + $this->assertEqual('testtable', $this->dbmanager->create_temp_table($table)); } } - ?> -- 2.39.5