From 6e2cff2d81451b68887d32195b186c8ea64a6b86 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Thu, 5 Apr 2012 19:03:31 +0200 Subject: [PATCH] MDL-32323 minor phpunit related improvements and cleanup --- admin/tool/phpunit/cli/util.php | 2 +- admin/tool/phpunit/index.php | 2 - admin/tool/phpunit/settings.php | 8 ++-- admin/tool/phpunit/version.php | 4 +- admin/tool/phpunit/webrunner.php | 10 ++--- lib/phpunit/bootstrap.php | 33 +++++++++------- lib/phpunit/bootstraplib.php | 23 +++++++++-- lib/phpunit/lib.php | 65 +++++++++++++++----------------- 8 files changed, 82 insertions(+), 65 deletions(-) diff --git a/admin/tool/phpunit/cli/util.php b/admin/tool/phpunit/cli/util.php index 0e8cf2976cf..1e0a670920f 100644 --- a/admin/tool/phpunit/cli/util.php +++ b/admin/tool/phpunit/cli/util.php @@ -20,7 +20,7 @@ * Exit codes: * 0 - success * 1 - general error - * 130 - missing PHPUnit error + * 130 - missing PHPUnit library error * 131 - configuration problem * 132 - install new test database * 133 - drop existing data before installing diff --git a/admin/tool/phpunit/index.php b/admin/tool/phpunit/index.php index 1dbaded3c6c..489a910134b 100644 --- a/admin/tool/phpunit/index.php +++ b/admin/tool/phpunit/index.php @@ -22,8 +22,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -define('NO_OUTPUT_BUFFERING', true); - require(dirname(__FILE__) . '/../../../config.php'); require_once($CFG->libdir.'/adminlib.php'); diff --git a/admin/tool/phpunit/settings.php b/admin/tool/phpunit/settings.php index 2e280659ab8..efefd32dfab 100644 --- a/admin/tool/phpunit/settings.php +++ b/admin/tool/phpunit/settings.php @@ -25,6 +25,8 @@ defined('MOODLE_INTERNAL') || die; -$ADMIN->add('development', new admin_externalpage('toolphpunit', get_string('pluginname', 'tool_phpunit'), "$CFG->wwwroot/$CFG->admin/tool/phpunit/index.php")); -$ADMIN->add('development', new admin_externalpage('toolphpunitwebrunner', get_string('pluginname', 'tool_phpunit'), "$CFG->wwwroot/$CFG->admin/tool/phpunit/webrunner.php", - 'moodle/site:config', true)); +if ($hassiteconfig) { + $ADMIN->add('development', new admin_externalpage('toolphpunit', get_string('pluginname', 'tool_phpunit'), "$CFG->wwwroot/$CFG->admin/tool/phpunit/index.php")); + $ADMIN->add('development', new admin_externalpage('toolphpunitwebrunner', get_string('pluginname', 'tool_phpunit'), "$CFG->wwwroot/$CFG->admin/tool/phpunit/webrunner.php", + 'moodle/site:config', true)); +} diff --git a/admin/tool/phpunit/version.php b/admin/tool/phpunit/version.php index e745026f858..9e594347907 100644 --- a/admin/tool/phpunit/version.php +++ b/admin/tool/phpunit/version.php @@ -24,7 +24,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2012030800; // The current plugin version (Date: YYYYMMDDXX) -$plugin->requires = 2012030100; // Requires this Moodle version +$plugin->version = 2012040500; // The current plugin version (Date: YYYYMMDDXX) +$plugin->requires = 2012040500; // Requires this Moodle version $plugin->component = 'tool_phpunit'; // Full name of the plugin (used for diagnostics) diff --git a/admin/tool/phpunit/webrunner.php b/admin/tool/phpunit/webrunner.php index 35c2b31a84b..2e0cf68c4e2 100644 --- a/admin/tool/phpunit/webrunner.php +++ b/admin/tool/phpunit/webrunner.php @@ -30,7 +30,6 @@ require_once($CFG->libdir.'/adminlib.php'); $path = optional_param('path', null, PARAM_PATH); $execute = optional_param('execute', 0, PARAM_BOOL); - navigation_node::override_active_url(new moodle_url('/admin/tool/phpunit/index.php')); admin_externalpage_setup('toolphpunitwebrunner'); @@ -64,7 +63,6 @@ if ($code != 0) { } if ($execute) { - require_sesskey(); chdir($CFG->dirroot); @@ -95,7 +93,6 @@ if ($execute) { } else if ($code == 133) { tool_phpunit_header(); - ignore_user_abort(true); echo $OUTPUT->box_start('generalbox'); echo '
';
         echo "Reinitialising test database:\n\n";
@@ -130,7 +127,6 @@ if ($execute) {
     echo $OUTPUT->box_start('generalbox');
     echo '
';
 
-    chdir($CFG->dirroot);
     // use the dataroot file
     $configdir = "$CFG->phpunit_dataroot/phpunit/webrunner.xml";
     if (!file_exists($configdir)) {
@@ -143,6 +139,7 @@ if ($execute) {
     // cleanup the path - this is tricky because we can not use quotes for escaping
     $path = escapeshellcmd($path);
     $path = str_replace('\*', '*', $path);
+    chdir($CFG->dirroot);
     passthru("phpunit -c $configdir $path", $code);
     chdir($oldcwd);
 
@@ -173,7 +170,8 @@ die;
 //========================================
 
 /**
- * Print headers and warning
+ * Print headers and experimental warning
+ * @return void
  */
 function tool_phpunit_header() {
     global $OUTPUT;
@@ -188,7 +186,7 @@ function tool_phpunit_header() {
  * @return void
  */
 function tool_phpunit_problem($message) {
-    global $OUTPUT, $PAGE;
+    global $PAGE;
     if (!$PAGE->headerprinted) {
         tool_phpunit_header();
     }
diff --git a/lib/phpunit/bootstrap.php b/lib/phpunit/bootstrap.php
index 470dddbff5f..99b32ddbae5 100644
--- a/lib/phpunit/bootstrap.php
+++ b/lib/phpunit/bootstrap.php
@@ -15,12 +15,13 @@
 // along with Moodle.  If not, see .
 
 /**
- * Prepares PHPUnit environment, it is called automatically.
+ * Prepares PHPUnit environment, the phpunit.xml configuration
+ * must specify this file as bootstrap.
  *
  * Exit codes:
  *  0   - success
  *  1   - general error
- *  130 - missing PHPUnit error
+ *  130 - missing PHPUnit library error
  *  131 - configuration problem
  *  132 - install new test database
  *  133 - drop existing data before installing
@@ -49,7 +50,7 @@ if (defined('PHPUNIT_TEST')) {
 define('PHPUNIT_TEST', true);
 
 if (!defined('PHPUNIT_UTIL')) {
-    /** Identifies utility scripts */
+    /** Identifies utility scripts - the database does not need to be initialised */
     define('PHPUNIT_UTIL', false);
 }
 
@@ -60,7 +61,7 @@ define('CLI_SCRIPT', true);
 
 define('NO_OUTPUT_BUFFERING', true);
 
-// only load CFG from config.php
+// only load CFG from config.php, stop ASAP in lib/setup.php
 define('ABORT_AFTER_CONFIG', true);
 require(__DIR__ . '/../../config.php');
 
@@ -97,7 +98,7 @@ if (!is_dir($CFG->phpunit_dataroot)) {
 }
 
 if (!is_writable($CFG->phpunit_dataroot)) {
-    // try to fix premissions if possible
+    // try to fix permissions if possible
     if (function_exists('posix_getuid')) {
         $chmod = fileperms($CFG->phpunit_dataroot);
         if (fileowner($CFG->phpunit_dataroot) == posix_getuid()) {
@@ -138,10 +139,17 @@ if (isset($CFG->prefix) and $CFG->prefix === $CFG->phpunit_prefix) {
     phpunit_bootstrap_error(131, '$CFG->prefix and $CFG->phpunit_prefix must not be identical, can not run tests!');
 }
 
-// throw away standard CFG settings
-
-$CFG->dataroot = $CFG->phpunit_dataroot;
-$CFG->prefix = $CFG->phpunit_prefix;
+// override CFG settings if necessary nad throw away extra CFG settings
+$CFG->dataroot  = $CFG->phpunit_dataroot;
+$CFG->prefix    = $CFG->phpunit_prefix;
+$CFG->dbtype    = isset($CFG->phpunit_dbtype) ? $CFG->phpunit_dbtype : $CFG->dbtype;
+$CFG->dblibrary = isset($CFG->phpunit_dblibrary) ? $CFG->phpunit_dblibrary : $CFG->dblibrary;
+$CFG->dbhost    = isset($CFG->phpunit_dbhost) ? $CFG->phpunit_dbhost : $CFG->dbhost;
+$CFG->dbname    = isset($CFG->phpunit_dbname) ? $CFG->phpunit_dbname : $CFG->dbname;
+$CFG->dbuser    = isset($CFG->phpunit_dbuser) ? $CFG->phpunit_dbuser : $CFG->dbuser;
+$CFG->dbpass    = isset($CFG->phpunit_dbpass) ? $CFG->phpunit_dbpass : $CFG->dbpass;
+$CFG->prefix    = isset($CFG->phpunit_prefix) ? $CFG->phpunit_prefix : $CFG->prefix;
+$CFG->dboptions = isset($CFG->phpunit_dboptions) ? $CFG->phpunit_dboptions : $CFG->dboptions;
 
 $allowed = array('wwwroot', 'dataroot', 'dirroot', 'admin', 'directorypermissions', 'filepermissions',
                  'dbtype', 'dblibrary', 'dbhost', 'dbname', 'dbuser', 'dbpass', 'prefix', 'dboptions');
@@ -164,7 +172,7 @@ $CFG->debug = (E_ALL | E_STRICT); // can not use DEBUG_DEVELOPER yet
 $CFG->debugdisplay = 1;
 error_reporting($CFG->debug);
 ini_set('display_errors', '1');
-ini_set('log_errors', '0');
+ini_set('log_errors', '1');
 
 $CFG->passwordsaltmain = 'phpunit'; // makes login via normal UI impossible
 
@@ -185,7 +193,7 @@ require("$CFG->dirroot/lib/setup.php");
 raise_memory_limit(MEMORY_EXTRA);
 
 if (PHPUNIT_UTIL) {
-    // we are not going to do testing, this is 'true' in utility scripts that init database usually
+    // we are not going to do testing, this is 'true' in utility scripts that only init database
     return;
 }
 
@@ -195,6 +203,5 @@ if ($errorcode) {
     phpunit_bootstrap_error($errorcode, $message);
 }
 
-// prepare for the first test run - store fresh globals, reset dataroot, etc.
+// prepare for the first test run - store fresh globals, reset database and dataroot, etc.
 phpunit_util::bootstrap_init();
-
diff --git a/lib/phpunit/bootstraplib.php b/lib/phpunit/bootstraplib.php
index 1efefe2e5a7..cd9bcce6c98 100644
--- a/lib/phpunit/bootstraplib.php
+++ b/lib/phpunit/bootstraplib.php
@@ -48,7 +48,7 @@ function phpunit_bootstrap_error($errorcode, $text = '') {
             $text = 'Database was initialised for different version, please use "php admin/tool/phpunit/cli/util.php --drop; php admin/tool/phpunit/cli/util.php --install"';
             break;
         case 134:
-            $text = 'Can not create main configuration file, please verify dirroot permissions."';
+            $text = 'Can not create main configuration file, please verify dirroot permissions.';
             break;
         default:
             $text = empty($text) ? '' : ': '.$text;
@@ -75,10 +75,25 @@ function phpunit_bootstrap_initdataroot($dataroot) {
     if (!file_exists("$dataroot/phpunittestdir.txt")) {
         file_put_contents("$dataroot/phpunittestdir.txt", 'Contents of this directory are used during tests only, do not delete this file!');
     }
-    if (fileperms("$dataroot/phpunittestdir.txt") & $CFG->filepermissions != $CFG->filepermissions) {
-        chmod("$dataroot/phpunittestdir.txt", $CFG->filepermissions);
-    }
+    phpunit_boostrap_fix_file_permissions("$dataroot/phpunittestdir.txt");
     if (!file_exists("$CFG->phpunit_dataroot/phpunit")) {
         mkdir("$CFG->phpunit_dataroot/phpunit", $CFG->directorypermissions);
     }
 }
+
+/**
+ * Try to change permissions to $CFG->dirroot or $CFG->dataroot if possible
+ * @param string $file
+ * @return bool success
+ */
+function phpunit_boostrap_fix_file_permissions($file) {
+    global $CFG;
+
+    $permissions = fileperms($file);
+    if ($permissions & $CFG->filepermissions != $CFG->filepermissions) {
+        $permissions = $permissions | $CFG->filepermissions;
+        return chmod($file, $permissions);
+    }
+
+    return true;
+}
\ No newline at end of file
diff --git a/lib/phpunit/lib.php b/lib/phpunit/lib.php
index cf97da72f85..c3d9de0c605 100644
--- a/lib/phpunit/lib.php
+++ b/lib/phpunit/lib.php
@@ -23,7 +23,6 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-
 require_once 'PHPUnit/Autoload.php';
 
 
@@ -228,39 +227,37 @@ class phpunit_util {
 
         if ($logchanges) {
             if (self::$lastdbwrites != $DB->perf_get_writes()) {
-                $warnings[] = 'warning: unexpected database modification, resetting DB state';
+                $warnings[] = 'Warning: unexpected database modification, resetting DB state';
             }
 
             $oldcfg = self::get_global_backup('CFG');
             $oldsite = self::get_global_backup('SITE');
             foreach($CFG as $k=>$v) {
                 if (!property_exists($oldcfg, $k)) {
-                    $warnings[] = 'warning: unexpected new $CFG->'.$k.' value';
+                    $warnings[] = 'Warning: unexpected new $CFG->'.$k.' value';
                 } else if ($oldcfg->$k !== $CFG->$k) {
-                    $warnings[] = 'warning: unexpected change of $CFG->'.$k.' value';
+                    $warnings[] = 'Warning: unexpected change of $CFG->'.$k.' value';
                 }
                 unset($oldcfg->$k);
 
             }
             if ($oldcfg) {
                 foreach($oldcfg as $k=>$v) {
-                    $warnings[] = 'warning: unexpected removal of $CFG->'.$k;
+                    $warnings[] = 'Warning: unexpected removal of $CFG->'.$k;
                 }
             }
 
             if ($USER->id != 0) {
-                $warnings[] = 'warning: unexpected change of $USER';
+                $warnings[] = 'Warning: unexpected change of $USER';
             }
 
             if ($COURSE->id != $oldsite->id) {
-                $warnings[] = 'warning: unexpected change of $COURSE';
+                $warnings[] = 'Warning: unexpected change of $COURSE';
             }
         }
 
-        // restore _SERVER
-        unset($_SERVER['HTTP_USER_AGENT']);
-
         // restore original config
+        $_SERVER = self::get_global_backup('_SERVER');
         $CFG = self::get_global_backup('CFG');
         $SITE = self::get_global_backup('SITE');
         $COURSE = $SITE;
@@ -314,6 +311,7 @@ class phpunit_util {
         global $CFG, $SITE;
 
         // backup the globals
+        self::$globals['_SERVER'] = $_SERVER;
         self::$globals['CFG'] = clone($CFG);
         self::$globals['SITE'] = clone($SITE);
 
@@ -385,7 +383,7 @@ class phpunit_util {
         }
 
         if (!file_exists("$CFG->dataroot/phpunit/tabledata.ser")) {
-            if (empty($table)) {
+            if (empty($tables)) {
                 return array(132, '');
             } else {
                 return array(133, '');
@@ -393,7 +391,7 @@ class phpunit_util {
         }
 
         if (!file_exists("$CFG->dataroot/phpunit/versionshash.txt")) {
-            if (empty($table)) {
+            if (empty($tables)) {
                 return array(132, '');
             } else {
                 return array(133, '');
@@ -463,7 +461,7 @@ class phpunit_util {
         }
 
         $options = array();
-        $options['adminpass'] = 'admin'; // removed later
+        $options['adminpass'] = 'admin';
         $options['shortname'] = 'phpunit';
         $options['fullname'] = 'PHPUnit test site';
 
@@ -489,19 +487,13 @@ class phpunit_util {
             }
         }
         $data = serialize($data);
-        @unlink("$CFG->dataroot/phpunit/tabledata.ser");
         file_put_contents("$CFG->dataroot/phpunit/tabledata.ser", $data);
-        if (fileperms("$CFG->dataroot/phpunit/tabledata.ser") & $CFG->filepermissions != $CFG->filepermissions) {
-            chmod("$CFG->dataroot/phpunit/tabledata.ser", $CFG->filepermissions);
-        }
+        phpunit_boostrap_fix_file_permissions("$CFG->dataroot/phpunit/tabledata.ser");
 
         // hash all plugin versions - helps with very fast detection of db structure changes
         $hash = phpunit_util::get_version_hash();
-        @unlink("$CFG->dataroot/phpunit/versionshash.txt");
         file_put_contents("$CFG->dataroot/phpunit/versionshash.txt", $hash);
-        if (fileperms("$CFG->dataroot/phpunit/versionshash.txt") & $CFG->filepermissions != $CFG->filepermissions) {
-            chmod("$CFG->dataroot/phpunit/versionshash.txt", $CFG->filepermissions);
-        }
+        phpunit_boostrap_fix_file_permissions("$CFG->dataroot/phpunit/versionshash.txt", $hash);
     }
 
     /**
@@ -590,18 +582,14 @@ class phpunit_util {
         $result = false;
         if (is_writable($CFG->dirroot)) {
             if ($result = file_put_contents("$CFG->dirroot/phpunit.xml", $data)) {
-                if (fileperms("$CFG->dirroot/phpunit.xml") & $CFG->filepermissions != $CFG->filepermissions) {
-                    chmod("$CFG->dirroot/phpunit.xml", $CFG->filepermissions);
-                }
+                phpunit_boostrap_fix_file_permissions("$CFG->dirroot/phpunit.xml");
             }
         }
-        // relink - it seems that xml:base does not work in phpunit xml files
+        // relink - it seems that xml:base does not work in phpunit xml files, remove this nasty hack if you find a way to set xml base for relative refs
         $data = str_replace('lib/phpunit/', "$CFG->dirroot/lib/phpunit/", $data);
         $data = preg_replace('|([^<]+)|', ''.$CFG->dirroot.'/$1', $data);
         file_put_contents("$CFG->dataroot/phpunit/webrunner.xml", $data);
-        if (fileperms("$CFG->dirroot/phpunit.xml") & $CFG->filepermissions != $CFG->filepermissions) {
-            chmod("$CFG->dirroot/phpunit.xml", $CFG->filepermissions);
-        }
+        phpunit_boostrap_fix_file_permissions("$CFG->dataroot/phpunit/webrunner.xml");
 
         return (bool)$result;
     }
@@ -729,7 +717,11 @@ class UnitTestCase extends PHPUnit_Framework_TestCase {
      * @return void
      */
     public static function assertIsA($actual, $expected, $message = '') {
-        parent::assertInstanceOf($expected, $actual, $message);
+        if ($expected === 'array') {
+            parent::assertEquals(gettype($actual), 'array', $message);
+        } else {
+            parent::assertInstanceOf($expected, $actual, $message);
+        }
     }
 }
 
@@ -892,10 +884,10 @@ class advanced_testcase extends PHPUnit_Framework_TestCase {
      * Recursively visit all the files in the source tree. Calls the callback
      * function with the pathname of each file found.
      *
-     * @param $path the folder to start searching from.
-     * @param $callback the method of this class to call with the name of each file found.
-     * @param $fileregexp a regexp used to filter the search (optional).
-     * @param $exclude If true, pathnames that match the regexp will be ignored. If false,
+     * @param string $path the folder to start searching from.
+     * @param string $callback the method of this class to call with the name of each file found.
+     * @param string $fileregexp a regexp used to filter the search (optional).
+     * @param bool $exclude If true, pathnames that match the regexp will be ignored. If false,
      *     only files that match the regexp will be included. (default false).
      * @param array $ignorefolders will not go into any of these folders (optional).
      * @return void
@@ -931,9 +923,10 @@ class advanced_testcase extends PHPUnit_Framework_TestCase {
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class database_driver_testcase extends PHPUnit_Framework_TestCase {
+    /** @var moodle_database connection to extra database */
     protected static $extradb = null;
 
-    /** @var moodle_database */
+    /** @var moodle_database used in these tests*/
     protected $tdb;
 
     /**
@@ -953,6 +946,7 @@ class database_driver_testcase extends PHPUnit_Framework_TestCase {
 
     public static function setUpBeforeClass() {
         global $CFG;
+        parent::setUpBeforeClass();
 
         if (!defined('PHPUNIT_TEST_DRIVER')) {
             // use normal $DB
@@ -986,6 +980,7 @@ class database_driver_testcase extends PHPUnit_Framework_TestCase {
 
     protected function setUp() {
         global $DB;
+        parent::setUp();
 
         if (self::$extradb) {
             $this->tdb = self::$extradb;
@@ -1004,6 +999,7 @@ class database_driver_testcase extends PHPUnit_Framework_TestCase {
                 $dbman->drop_table($table);
             }
         }
+        parent::tearDown();
     }
 
     public static function tearDownAfterClass() {
@@ -1012,5 +1008,6 @@ class database_driver_testcase extends PHPUnit_Framework_TestCase {
             self::$extradb = null;
         }
         phpunit_util::reset_all_data();
+        parent::tearDownAfterClass();
     }
 }