Issue #1029606 follow-up by catch, David_Rothstein: Fixed Regression: Not loading the .module file causes a fatal error when uninstalling some modules (as does loading it).

8 files changed, 236 insertions, 64 deletions

diff --git a/modules/shortcut/shortcut.install b/modules/shortcut/shortcut.install
index 209a90754..9dbab806d 100644
--- a/modules/shortcut/shortcut.install
+++ b/modules/shortcut/shortcut.install
@@ -32,6 +32,7 @@ function shortcut_install() {
  * Implements hook_uninstall().
  */
 function shortcut_uninstall() {
+  drupal_load('module', 'shortcut');
   // Delete the menu links associated with each shortcut set.
   foreach (shortcut_sets() as $shortcut_set) {
     menu_delete_links($shortcut_set->set_name);
diff --git a/modules/simpletest/simpletest.install b/modules/simpletest/simpletest.install
index 0f017e75f..ea847f4ea 100644
--- a/modules/simpletest/simpletest.install
+++ b/modules/simpletest/simpletest.install
@@ -167,7 +167,8 @@ function simpletest_schema() {
  * Implements hook_uninstall().
  */
 function simpletest_uninstall() {
-  simpletest_clean_environment();
+  drupal_load('module', 'simpletest');
+  simpletest_clean_database();
 
   // Remove settings variables.
   variable_del('simpletest_httpauth_method');
diff --git a/modules/simpletest/simpletest.module b/modules/simpletest/simpletest.module
index b992fd2a0..586b23ae7 100644
--- a/modules/simpletest/simpletest.module
+++ b/modules/simpletest/simpletest.module
@@ -452,13 +452,15 @@ function simpletest_clean_database() {
  * Find all leftover temporary directories and remove them.
  */
 function simpletest_clean_temporary_directories() {
-  $files = scandir('public://simpletest');
   $count = 0;
-  foreach ($files as $file) {
-    $path = 'public://simpletest/' . $file;
-    if (is_dir($path) && is_numeric($file)) {
-      file_unmanaged_delete_recursive($path);
-      $count++;
+  if (is_dir('public://simpletest')) {
+    $files = scandir('public://simpletest');
+    foreach ($files as $file) {
+      $path = 'public://simpletest/' . $file;
+      if (is_dir($path) && is_numeric($file)) {
+        file_unmanaged_delete_recursive($path);
+        $count++;
+      }
     }
   }
 
diff --git a/modules/simpletest/tests/requirements1_test.info b/modules/simpletest/tests/requirements1_test.info
index ef3953517..b659b21c7 100644
--- a/modules/simpletest/tests/requirements1_test.info
+++ b/modules/simpletest/tests/requirements1_test.info
@@ -1,6 +1,6 @@
 name = Requirements 1 Test
 description = "Tests that a module is not installed when it fails hook_requirements('install')."
-package = Core
+package = Testing
 version = VERSION
 core = 7.x
 hidden = TRUE
diff --git a/modules/simpletest/tests/requirements2_test.info b/modules/simpletest/tests/requirements2_test.info
index 0cf86478e..a66e04be9 100644
--- a/modules/simpletest/tests/requirements2_test.info
+++ b/modules/simpletest/tests/requirements2_test.info
@@ -2,7 +2,7 @@ name = Requirements 2 Test
 description = "Tests that a module is not installed when the one it depends on fails hook_requirements('install)."
 dependencies[] = requirements1_test
 dependencies[] = comment
-package = Core
+package = Testing
 version = VERSION
 core = 7.x
 hidden = TRUE
diff --git a/modules/simpletest/tests/system_test.module b/modules/simpletest/tests/system_test.module
index 76841fb6b..fbe81fa75 100644
--- a/modules/simpletest/tests/system_test.module
+++ b/modules/simpletest/tests/system_test.module
@@ -146,8 +146,10 @@ function system_test_redirect_invalid_scheme() {
  * Implements hook_modules_installed().
  */
 function system_test_modules_installed($modules) {
-  if (in_array('aggregator', $modules)) {
-    drupal_set_message(t('hook_modules_installed fired for aggregator'));
+  if (variable_get('test_verbose_module_hooks')) {
+    foreach ($modules as $module) {
+      drupal_set_message(t('hook_modules_installed fired for @module', array('@module' => $module)));
+    }
   }
 }
 
@@ -155,8 +157,10 @@ function system_test_modules_installed($modules) {
  * Implements hook_modules_enabled().
  */
 function system_test_modules_enabled($modules) {
-  if (in_array('aggregator', $modules)) {
-    drupal_set_message(t('hook_modules_enabled fired for aggregator'));
+  if (variable_get('test_verbose_module_hooks')) {
+    foreach ($modules as $module) {
+      drupal_set_message(t('hook_modules_enabled fired for @module', array('@module' => $module)));
+    }
   }
 }
 
@@ -164,8 +168,10 @@ function system_test_modules_enabled($modules) {
  * Implements hook_modules_disabled().
  */
 function system_test_modules_disabled($modules) {
-  if (in_array('aggregator', $modules)) {
-    drupal_set_message(t('hook_modules_disabled fired for aggregator'));
+  if (variable_get('test_verbose_module_hooks')) {
+    foreach ($modules as $module) {
+      drupal_set_message(t('hook_modules_disabled fired for @module', array('@module' => $module)));
+    }
   }
 }
 
@@ -173,8 +179,10 @@ function system_test_modules_disabled($modules) {
  * Implements hook_modules_uninstalled().
  */
 function system_test_modules_uninstalled($modules) {
-  if (in_array('aggregator', $modules)) {
-    drupal_set_message(t('hook_modules_uninstalled fired for aggregator'));
+  if (variable_get('test_verbose_module_hooks')) {
+    foreach ($modules as $module) {
+      drupal_set_message(t('hook_modules_uninstalled fired for @module', array('@module' => $module)));
+    }
   }
 }
 
diff --git a/modules/system/system.api.php b/modules/system/system.api.php
index 584d4a8d7..f32433c08 100644
--- a/modules/system/system.api.php
+++ b/modules/system/system.api.php
@@ -3267,6 +3267,14 @@ function hook_update_last_removed() {
  * module's database tables are removed, allowing your module to query its own
  * tables during this routine.
  *
+ * When hook_uninstall() is called, your module will already be disabled, so
+ * its .module file will not be automatically included. If you need to call API
+ * functions from your .module file in this hook, use drupal_load() to make
+ * them available. (Keep this usage to a minimum, though, especially when
+ * calling API functions that invoke hooks, or API functions from modules
+ * listed as dependencies, since these may not be available or work as expected
+ * when the module is disabled.)
+ *
  * @see hook_install()
  * @see hook_schema()
  * @see hook_disable()
diff --git a/modules/system/system.test b/modules/system/system.test
index be4e36698..fee187f5b 100644
--- a/modules/system/system.test
+++ b/modules/system/system.test
@@ -37,6 +37,40 @@ class ModuleTestCase extends DrupalWebTestCase {
   }
 
   /**
+   * Assert that all tables defined in a module's hook_schema() exist.
+   *
+   * @param $module
+   *   The name of the module.
+   */
+  function assertModuleTablesExist($module) {
+    $tables = array_keys(drupal_get_schema_unprocessed($module));
+    $tables_exist = TRUE;
+    foreach ($tables as $table) {
+      if (!db_table_exists($table)) {
+        $tables_exist = FALSE;
+      }
+    }
+    return $this->assertTrue($tables_exist, t('All database tables defined by the @module module exist.', array('@module' => $module)));
+  }
+
+  /**
+   * Assert that none of the tables defined in a module's hook_schema() exist.
+   *
+   * @param $module
+   *   The name of the module.
+   */
+  function assertModuleTablesDoNotExist($module) {
+    $tables = array_keys(drupal_get_schema_unprocessed($module));
+    $tables_exist = FALSE;
+    foreach ($tables as $table) {
+      if (db_table_exists($table)) {
+        $tables_exist = TRUE;
+      }
+    }
+    return $this->assertFalse($tables_exist, t('None of the database tables defined by the @module module exist.', array('@module' => $module)));
+  }
+
+  /**
    * Assert the list of modules are enabled or disabled.
    *
    * @param $modules
@@ -96,6 +130,8 @@ class ModuleTestCase extends DrupalWebTestCase {
  * Test module enabling/disabling functionality.
  */
 class EnableDisableTestCase extends ModuleTestCase {
+  protected $profile = 'testing';
+
   public static function getInfo() {
     return array(
       'name' => 'Enable/disable modules',
@@ -105,59 +141,132 @@ class EnableDisableTestCase extends ModuleTestCase {
   }
 
   /**
-   * Enable a module, check the database for related tables, disable module,
-   * check for related tables, uninstall module, check for related tables.
-   * Also check for invocation of the hook_module_action hook.
+   * Test that all core modules can be enabled, disabled and uninstalled.
    */
   function testEnableDisable() {
-    // Enable aggregator, and check tables.
-    $this->assertModules(array('aggregator'), FALSE);
-    $this->assertTableCount('aggregator', FALSE);
-
-    // Install (and enable) aggregator module.
-    $edit = array();
-    $edit['modules[Core][aggregator][enable]'] = 'aggregator';
-    $edit['modules[Core][forum][enable]'] = 'forum';
-    $this->drupalPost('admin/modules', $edit, t('Save configuration'));
-    $this->assertText(t('The configuration options have been saved.'), t('Modules status has been updated.'));
-
-    // Check that hook_modules_installed and hook_modules_enabled hooks were invoked and check tables.
-    $this->assertText(t('hook_modules_installed fired for aggregator'), t('hook_modules_installed fired.'));
-    $this->assertText(t('hook_modules_enabled fired for aggregator'), t('hook_modules_enabled fired.'));
-    $this->assertModules(array('aggregator'), TRUE);
-    $this->assertTableCount('aggregator', TRUE);
-    $this->assertLogMessage('system', "%module module installed.", array('%module' => 'aggregator'), WATCHDOG_INFO);
-    $this->assertLogMessage('system', "%module module enabled.", array('%module' => 'aggregator'), WATCHDOG_INFO);
+    // Try to enable, disable and uninstall all core modules, unless they are
+    // hidden or required.
+    $modules = system_rebuild_module_data();
+    foreach ($modules as $name => $module) {
+      if ($module->info['package'] != 'Core' || !empty($module->info['hidden']) || !empty($module->info['required'])) {
+        unset($modules[$name]);
+      }
+    }
+    $this->assertTrue(count($modules), t('Found @count core modules that we can try to enable in this test.', array('@count' => count($modules))));
+
+    // Enable the dblog module first, since we will be asserting the presence
+    // of log messages throughout the test.
+   if (isset($modules['dblog'])) {
+     $modules = array('dblog' => $modules['dblog']) + $modules;
+   }
+
+   // Set a variable so that the hook implementations in system_test.module
+   // will display messages via drupal_set_message().
+   variable_set('test_verbose_module_hooks', TRUE);
+
+    // Throughout this test, some modules may be automatically enabled (due to
+    // dependencies). We'll keep track of them in an array, so we can handle
+    // them separately.
+    $automatically_enabled = array();
+
+    // Go through each module in the list and try to enable it (unless it was
+    // already enabled automatically due to a dependency).
+    foreach ($modules as $name => $module) {
+      if (empty($automatically_enabled[$name])) {
+        // Start a list of modules that we expect to be enabled this time.
+        $modules_to_enable = array($name);
+
+        // Find out if the module has any dependencies that aren't enabled yet;
+        // if so, add them to the list of modules we expect to be automatically
+        // enabled.
+        foreach (array_keys($module->requires) as $dependency) {
+          if (isset($modules[$dependency]) && empty($automatically_enabled[$dependency])) {
+            $modules_to_enable[] = $dependency;
+            $automatically_enabled[$dependency] = TRUE;
+          }
+        }
 
-    // Disable aggregator, check tables, uninstall aggregator, check tables.
-    $edit = array();
-    $edit['modules[Core][aggregator][enable]'] = FALSE;
-    $this->drupalPost('admin/modules', $edit, t('Save configuration'));
-    $this->assertText(t('The configuration options have been saved.'), t('Modules status has been updated.'));
+        // Check that each module is not yet enabled and does not have any
+        // database tables yet.
+        foreach ($modules_to_enable as $module_to_enable) {
+          $this->assertModules(array($module_to_enable), FALSE);
+          $this->assertModuleTablesDoNotExist($module_to_enable);
+        }
 
-    // Check that hook_modules_disabled hook was invoked and check tables.
-    $this->assertText(t('hook_modules_disabled fired for aggregator'), t('hook_modules_disabled fired.'));
-    $this->assertModules(array('aggregator'), FALSE);
-    $this->assertTableCount('aggregator', TRUE);
-    $this->assertLogMessage('system', "%module module disabled.", array('%module' => 'aggregator'), WATCHDOG_INFO);
+        // Install and enable the module.
+        $edit = array();
+        $edit['modules[Core][' . $name . '][enable]'] = $name;
+        $this->drupalPost('admin/modules', $edit, t('Save configuration'));
+        // Handle the case where modules were installed along with this one and
+        // where we therefore hit a confirmation screen.
+        if (count($modules_to_enable) > 1) {
+          $this->drupalPost(NULL, array(), t('Continue'));
+        }
+        $this->assertText(t('The configuration options have been saved.'), t('Modules status has been updated.'));
+
+        // Check that hook_modules_installed() and hook_modules_enabled() were
+        // invoked with the expected list of modules, that each module's
+        // database tables now exist, and that appropriate messages appear in
+        // the logs.
+        foreach ($modules_to_enable as $module_to_enable) {
+          $this->assertText(t('hook_modules_installed fired for @module', array('@module' => $module_to_enable)));
+          $this->assertText(t('hook_modules_enabled fired for @module', array('@module' => $module_to_enable)));
+          $this->assertModules(array($module_to_enable), TRUE);
+          $this->assertModuleTablesExist($module_to_enable);
+          $this->assertLogMessage('system', "%module module installed.", array('%module' => $module_to_enable), WATCHDOG_INFO);
+          $this->assertLogMessage('system', "%module module enabled.", array('%module' => $module_to_enable), WATCHDOG_INFO);
+        }
 
-    // Uninstall the module.
-    $edit = array();
-    $edit['uninstall[aggregator]'] = 'aggregator';
-    $this->drupalPost('admin/modules/uninstall', $edit, t('Uninstall'));
+        // Disable and uninstall the original module, and check appropriate
+        // hooks, tables, and log messages. (Later, we'll go back and do the
+        // same thing for modules that were enabled automatically.) Skip this
+        // for the dblog module, because that is needed for the test; we'll go
+        // back and do that one at the end also.
+        if ($name != 'dblog') {
+          $this->assertSuccessfulDisableAndUninstall($name);
+        }
+      }
+    }
 
-    $this->drupalPost(NULL, NULL, t('Uninstall'));
-    $this->assertText(t('The selected modules have been uninstalled.'), t('Modules status has been updated.'));
+    // Go through all modules that were automatically enabled, and try to
+    // disable and uninstall them one by one.
+    while (!empty($automatically_enabled)) {
+      $initial_count = count($automatically_enabled);
+      foreach (array_keys($automatically_enabled) as $name) {
+        // If the module can't be disabled due to dependencies, skip it and try
+        // again the next time. Otherwise, try to disable it.
+        $this->drupalGet('admin/modules');
+        $disabled_checkbox = $this->xpath('//input[@type="checkbox" and @disabled="disabled" and @name="modules[Core][' . $name . '][enable]"]');
+        if (empty($disabled_checkbox) && $name != 'dblog') {
+          unset($automatically_enabled[$name]);
+          $this->assertSuccessfulDisableAndUninstall($name);
+        }
+      }
+      $final_count = count($automatically_enabled);
+      // If all checkboxes were disabled, something is really wrong with the
+      // test. Throw a failure and avoid an infinite loop.
+      if ($initial_count == $final_count) {
+        $this->fail(t('Remaining modules could not be disabled.'));
+        break;
+      }
+    }
 
-    // Check that hook_modules_uninstalled hook was invoked and check tables.
-    $this->assertText(t('hook_modules_uninstalled fired for aggregator'), t('hook_modules_uninstalled fired.'));
-    $this->assertModules(array('aggregator'), FALSE);
-    $this->assertTableCount('aggregator', FALSE);
-    $this->assertLogMessage('system', "%module module uninstalled.", array('%module' => 'aggregator'), WATCHDOG_INFO);
+    // Disable and uninstall the dblog module last, since we needed it for
+    // assertions in all the above tests.
+    if (isset($modules['dblog'])) {
+      $this->assertSuccessfulDisableAndUninstall('dblog');
+    }
 
-    // Reinstall (and enable) aggregator module.
+    // Now that all modules have been tested, go back and try to enable them
+    // all again at once. This tests two things:
+    // - That each module can be successfully enabled again after being
+    //   uninstalled.
+    // - That enabling more than one module at the same time does not lead to
+    //   any errors.
     $edit = array();
-    $edit['modules[Core][aggregator][enable]'] = 'aggregator';
+    foreach (array_keys($modules) as $name) {
+      $edit['modules[Core][' . $name . '][enable]'] = $name;
+    }
     $this->drupalPost('admin/modules', $edit, t('Save configuration'));
     $this->assertText(t('The configuration options have been saved.'), t('Modules status has been updated.'));
   }
@@ -174,6 +283,49 @@ class EnableDisableTestCase extends ModuleTestCase {
     $this->assertEqual($info['label'], 'Entity Cache Test', 'Entity info label is correct.');
     $this->assertEqual($info['controller class'], 'DrupalDefaultEntityController', 'Entity controller class info is correct.');
   }
+
+  /**
+   * Disables and uninstalls a module and asserts that it was done correctly.
+   *
+   * @param $module
+   *   The name of the module to disable and uninstall.
+   */
+  function assertSuccessfulDisableAndUninstall($module) {
+    // Disable the module.
+    $edit = array();
+    $edit['modules[Core][' . $module . '][enable]'] = FALSE;
+    $this->drupalPost('admin/modules', $edit, t('Save configuration'));
+    $this->assertText(t('The configuration options have been saved.'), t('Modules status has been updated.'));
+    $this->assertModules(array($module), FALSE);
+
+    // Check that the appropriate hook was fired and the appropriate log
+    // message appears.
+    $this->assertText(t('hook_modules_disabled fired for @module', array('@module' => $module)));
+    $this->assertLogMessage('system', "%module module disabled.", array('%module' => $module), WATCHDOG_INFO);
+
+    //  Check that the module's database tables still exist.
+    $this->assertModuleTablesExist($module);
+
+    // Uninstall the module.
+    $edit = array();
+    $edit['uninstall[' . $module . ']'] = $module;
+    $this->drupalPost('admin/modules/uninstall', $edit, t('Uninstall'));
+    $this->drupalPost(NULL, NULL, t('Uninstall'));
+    $this->assertText(t('The selected modules have been uninstalled.'), t('Modules status has been updated.'));
+    $this->assertModules(array($module), FALSE);
+
+    // Check that the appropriate hook was fired and the appropriate log
+    // message appears. (But don't check for the log message if the dblog
+    // module was just uninstalled, since the {watchdog} table won't be there
+    // anymore.)
+    $this->assertText(t('hook_modules_uninstalled fired for @module', array('@module' => $module)));
+    if ($module != 'dblog') {
+      $this->assertLogMessage('system', "%module module uninstalled.", array('%module' => $module), WATCHDOG_INFO);
+    }
+
+    // Check that the module's database tables no longer exist.
+    $this->assertModuleTablesDoNotExist($module);
+  }
 }
 
 /**
@@ -196,7 +348,7 @@ class HookRequirementsTestCase extends ModuleTestCase {
 
     // Attempt to install the requirements1_test module.
     $edit = array();
-    $edit['modules[Core][requirements1_test][enable]'] = 'requirements1_test';
+    $edit['modules[Testing][requirements1_test][enable]'] = 'requirements1_test';
     $this->drupalPost('admin/modules', $edit, t('Save configuration'));
 
     // Makes sure the module was NOT installed.
@@ -278,8 +430,8 @@ class ModuleDependencyTestCase extends ModuleTestCase {
 
     // Attempt to install both modules at the same time.
     $edit = array();
-    $edit['modules[Core][requirements1_test][enable]'] = 'requirements1_test';
-    $edit['modules[Core][requirements2_test][enable]'] = 'requirements2_test';
+    $edit['modules[Testing][requirements1_test][enable]'] = 'requirements1_test';
+    $edit['modules[Testing][requirements2_test][enable]'] = 'requirements2_test';
     $this->drupalPost('admin/modules', $edit, t('Save configuration'));
 
     // Makes sure the modules were NOT installed.