From 27ef1a024cf3b37e12aa2f0fb4c704b969093e9a Mon Sep 17 00:00:00 2001 From: drugan Date: Sat, 17 Dec 2016 12:35:13 +0200 Subject: [PATCH] Issue #2745123 by Mile23, slasher13, blazey, drunken monkey, Yogesh Pawar, drugan: Simpletest module crashes on cleanup, uninstall --- core/modules/simpletest/simpletest.module | 43 +++++++++++++++++++++------ core/modules/simpletest/src/TestBase.php | 19 ++---------- core/scripts/run-tests.sh | 12 ++++---- core/tests/Drupal/Tests/BrowserTestBase.php | 22 ++------------ 4 files changed, 43 insertions(+), 53 deletions(-) diff --git a/core/modules/simpletest/simpletest.module b/core/modules/simpletest/simpletest.module index f72ca5c..09cc23e 100644 --- a/core/modules/simpletest/simpletest.module +++ b/core/modules/simpletest/simpletest.module @@ -674,24 +674,49 @@ function simpletest_clean_database() { /** * Finds all leftover temporary directories and removes them. */ -function simpletest_clean_temporary_directories() { +function simpletest_clean_temporary_directories($directory = NULL) { $count = 0; - if (is_dir(DRUPAL_ROOT . '/sites/simpletest')) { - $files = scandir(DRUPAL_ROOT . '/sites/simpletest'); - foreach ($files as $file) { + $paths = []; + $root = $directory ?: \Drupal::root() . '/sites/simpletest'; + if (is_dir($root)) { + $files = scandir($root); + + // First, chmod files and dirs recusively. + while ($file = current($files)) { if ($file[0] != '.') { - $path = DRUPAL_ROOT . '/sites/simpletest/' . $file; - file_unmanaged_delete_recursive($path, array('Drupal\simpletest\TestBase', 'filePreDeleteCallback')); - $count++; + $paths[] = $root . '/' . explode('/', $file)[0]; + $path = $root . '/' . $file; + // When the webserver runs with the same system user as the test runner, + // we can make read-only files writable again. If not, chmod will fail + // while the file deletion still works if file permissions have been + // configured correctly. Thus, we ignore any errors while running chmod. + @chmod($path, 0700); + + if (is_dir($path)){ + $dir = dir($path); + while (($entry = $dir->read()) !== FALSE) { + if ($entry[0] == '.') { + continue; + } + array_push($files, $file . '/' . $entry); + } + $dir->close(); + } } + next($files); + } + + foreach (array_unique($paths) as $path) { + file_unmanaged_delete_recursive($path); + $count++; } } if ($count > 0) { - drupal_set_message(\Drupal::translation()->formatPlural($count, 'Removed 1 temporary directory.', 'Removed @count temporary directories.')); + drupal_set_message(\Drupal::translation()->formatPlural($count, 'Removed 1 temporary directory in @root.', 'Removed @count temporary directories in @root.', ['@root' => $root])); } else { - drupal_set_message(t('No temporary directories to remove.')); + drupal_set_message(t('No temporary directories to remove in @root.', ['@root' => $root])); } } diff --git a/core/modules/simpletest/src/TestBase.php b/core/modules/simpletest/src/TestBase.php index f4a23e7..68aae02 100644 --- a/core/modules/simpletest/src/TestBase.php +++ b/core/modules/simpletest/src/TestBase.php @@ -1363,8 +1363,8 @@ private function restoreEnvironment() { $this->container = $this->originalContainer; \Drupal::setContainer($this->originalContainer); - // Delete test site directory. - file_unmanaged_delete_recursive($this->siteDirectory, array($this, 'filePreDeleteCallback')); + // Delete the current test site directory. + simpletest_clean_temporary_directories(\Drupal::root() . '/' . $this->siteDirectory); // Restore original database connection. Database::removeConnection('default'); @@ -1525,21 +1525,6 @@ public static function generatePermutations($parameters) { } /** - * Ensures test files are deletable within file_unmanaged_delete_recursive(). - * - * Some tests chmod generated files to be read only. During - * TestBase::restoreEnvironment() and other cleanup operations, these files - * need to get deleted too. - */ - public static function filePreDeleteCallback($path) { - // When the webserver runs with the same system user as the test runner, we - // can make read-only files writable again. If not, chmod will fail while - // the file deletion still works if file permissions have been configured - // correctly. Thus, we ignore any problems while running chmod. - @chmod($path, 0700); - } - - /** * Configuration accessor for tests. Returns non-overridden configuration. * * @param $name diff --git a/core/scripts/run-tests.sh b/core/scripts/run-tests.sh index 2936723..dfd2d7d 100755 --- a/core/scripts/run-tests.sh +++ b/core/scripts/run-tests.sh @@ -862,7 +862,7 @@ function simpletest_script_cleanup($test_id, $test_class, $exitcode) { // Check whether a test site directory was setup already. // @see \Drupal\simpletest\TestBase::prepareEnvironment() $test_db = new TestDatabase($db_prefix); - $test_directory = DRUPAL_ROOT . '/' . $test_db->getTestSitePath(); + $test_directory = \Drupal::root() . '/' . $test_db->getTestSitePath(); if (is_dir($test_directory)) { // Output the error_log. if (is_file($test_directory . '/error.log')) { @@ -871,12 +871,10 @@ function simpletest_script_cleanup($test_id, $test_class, $exitcode) { $messages[] = $errors; } } - // Delete the test site directory. - // simpletest_clean_temporary_directories() cannot be used here, since it - // would also delete file directories of other tests that are potentially - // running concurrently. - file_unmanaged_delete_recursive($test_directory, array('Drupal\simpletest\TestBase', 'filePreDeleteCallback')); - $messages[] = "- Removed test site directory."; + // Delete the current test site directory leaving alone other tests that + // are potentially running concurrently. + simpletest_clean_temporary_directories($test_directory); + $messages[] = "- Removed test site directory in {$test_directory}."; } // Clear out all database tables from the test. diff --git a/core/tests/Drupal/Tests/BrowserTestBase.php b/core/tests/Drupal/Tests/BrowserTestBase.php index 1306489..5e6e17e 100644 --- a/core/tests/Drupal/Tests/BrowserTestBase.php +++ b/core/tests/Drupal/Tests/BrowserTestBase.php @@ -520,24 +520,6 @@ protected function setUp() { } /** - * Ensures test files are deletable within file_unmanaged_delete_recursive(). - * - * Some tests chmod generated files to be read only. During - * BrowserTestBase::cleanupEnvironment() and other cleanup operations, - * these files need to get deleted too. - * - * @param string $path - * The file path. - */ - public static function filePreDeleteCallback($path) { - // When the webserver runs with the same system user as phpunit, we can - // make read-only files writable again. If not, chmod will fail while the - // file deletion still works if file permissions have been configured - // correctly. Thus, we ignore any problems while running chmod. - @chmod($path, 0700); - } - - /** * Clean up the Simpletest environment. */ protected function cleanupEnvironment() { @@ -555,8 +537,8 @@ protected function cleanupEnvironment() { } } - // Delete test site directory. - file_unmanaged_delete_recursive($this->siteDirectory, array($this, 'filePreDeleteCallback')); + // Delete the current test site directory. + simpletest_clean_temporary_directories(\Drupal::root() . '/' . $this->siteDirectory); } /** -- 1.7.9.5