Issue #2189345 by benjy, sanduhrs, Mile23, joshtaylor, jbekker, klausi, sun,...

Issue #2189345 by benjy, sanduhrs, Mile23, joshtaylor, jbekker, klausi, sun, zaporylie, pfrenssen, jsacksick: run-tests.sh should exit with a failure code if any tests failed

core/modules/simpletest/simpletest.module

@@ -170,17 +170,20 @@ function simpletest_run_tests($test_list) {
  * @param $unescaped_test_classnames
  *   An array of test class names, including full namespaces, to be passed as
  *   a regular expression to PHPUnit's --filter option.
+ * @param int $status
+ *   (optional) The exit status code of the PHPUnit process will be assigned to
+ *   this variable.
  *
  * @return array
  *   The parsed results of PHPUnit's JUnit XML output, in the format of
  *   {simpletest}'s schema.
  */
-function simpletest_run_phpunit_tests($test_id, array $unescaped_test_classnames) {
+function simpletest_run_phpunit_tests($test_id, array $unescaped_test_classnames, &$status = NULL) {
   $phpunit_file = simpletest_phpunit_xml_filepath($test_id);
-  $ret = simpletest_phpunit_run_command($unescaped_test_classnames, $phpunit_file);
-  // A return value of 0 = passed test, 1 = failed test, > 1 indicates segfault
+  simpletest_phpunit_run_command($unescaped_test_classnames, $phpunit_file, $status);
+  // A $status of 0 = passed test, 1 = failed test, > 1 indicates segfault
   // timeout, or other type of failure.
-  if ($ret > 1) {
+  if ($status > 1) {
     // Something broke during the execution of phpunit.
     // Return an error record of all failed classes.
     $rows[] = [
@@ -251,11 +254,14 @@ function simpletest_phpunit_configuration_filepath() {
  *   a regular expression to PHPUnit's --filter option.
  * @param string $phpunit_file
  *   A filepath to use for PHPUnit's --log-junit option.
+ * @param int $status
+ *   (optional) The exit status code of the PHPUnit process will be assigned to
+ *   this variable.
  *
  * @return string
  *  The results as returned by exec().
  */
-function simpletest_phpunit_run_command(array $unescaped_test_classnames, $phpunit_file) {
+function simpletest_phpunit_run_command(array $unescaped_test_classnames, $phpunit_file, &$status = NULL) {
   // Setup an environment variable containing the database connection so that
   // functional tests can connect to the database.
   putenv('SIMPLETEST_DB=' . Database::getConnectionInfoAsUrl());
@@ -292,7 +298,8 @@ function simpletest_phpunit_run_command(array $unescaped_test_classnames, $phpun
 
   // exec in a subshell so that the environment is isolated when running tests
   // via the simpletest UI.
-  exec(join($command, " "), $output, $ret);
+  $ret = exec(join($command, " "), $output, $status);
+
   chdir($old_cwd);
   putenv('SIMPLETEST_DB=');
   return $ret;

core/scripts/run-tests.sh

@@ -24,46 +24,68 @@
 // Restricting the chunk of queries prevents memory exhaustion.
 const SIMPLETEST_SCRIPT_SQLITE_VARIABLE_LIMIT = 350;
 
+const SIMPLETEST_SCRIPT_EXIT_SUCCESS = 0;
+const SIMPLETEST_SCRIPT_EXIT_FAILURE = 1;
+const SIMPLETEST_SCRIPT_EXIT_EXCEPTION = 2;
+
 // Set defaults and get overrides.
 list($args, $count) = simpletest_script_parse_args();
 
 if ($args['help'] || $count == 0) {
   simpletest_script_help();
-  exit;
+  exit(($count == 0) ? SIMPLETEST_SCRIPT_EXIT_FAILURE : SIMPLETEST_SCRIPT_EXIT_SUCCESS);
 }
 
 simpletest_script_init();
 
-$request = Request::createFromGlobals();
-$kernel = TestRunnerKernel::createFromRequest($request, $autoloader);
-$kernel->prepareLegacyRequest($request);
+try {
+  $request = Request::createFromGlobals();
+  $kernel = TestRunnerKernel::createFromRequest($request, $autoloader);
+  $kernel->prepareLegacyRequest($request);
+}
+catch (Exception $e) {
+  echo (string) $e;
+  exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+}
 
 if ($args['execute-test']) {
   simpletest_script_setup_database();
   simpletest_script_run_one_test($args['test-id'], $args['execute-test']);
   // Sub-process exited already; this is just for clarity.
-  exit;
+  exit(SIMPLETEST_SCRIPT_EXIT_SUCCESS);
 }
 
 if ($args['list']) {
   // Display all available tests.
   echo "\nAvailable test groups & classes\n";
   echo   "-------------------------------\n\n";
-  $groups = simpletest_test_get_all($args['module']);
+  try {
+    $groups = simpletest_test_get_all($args['module']);
+  }
+  catch (Exception $e) {
+    echo (string) $e;
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+  }
   foreach ($groups as $group => $tests) {
     echo $group . "\n";
     foreach ($tests as $class => $info) {
       echo " - $class\n";
     }
   }
-  exit;
+  exit(SIMPLETEST_SCRIPT_EXIT_SUCCESS);
 }
 
 simpletest_script_setup_database(TRUE);
 
 if ($args['clean']) {
   // Clean up left-over tables and directories.
-  simpletest_clean_environment();
+  try {
+    simpletest_clean_environment();
+  }
+  catch (Exception $e) {
+    echo (string) $e;
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+  }
   echo "\nEnvironment cleaned.\n";
 
   // Get the status messages and print them.
@@ -71,7 +93,7 @@
   foreach ($messages['status'] as $text) {
     echo " - " . $text . "\n";
   }
-  exit;
+  exit(SIMPLETEST_SCRIPT_EXIT_SUCCESS);
 }
 
 $test_list = simpletest_script_get_test_list();
@@ -86,7 +108,7 @@
 }
 
 // Execute tests.
-simpletest_script_execute_batch($tests_to_run);
+$status = simpletest_script_execute_batch($tests_to_run);
 
 // Stop the timer.
 simpletest_script_reporter_timer_stop();
@@ -105,11 +127,17 @@
 
 // Clean up all test results.
 if (!$args['keep-results']) {
-  simpletest_clean_results_table();
+  try {
+    simpletest_clean_results_table();
+  }
+  catch (Exception $e) {
+    echo (string) $e;
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+  }
 }
 
 // Test complete, exit.
-exit;
+exit($status);
 
 /**
  * Print help text.
@@ -296,7 +324,7 @@ function simpletest_script_parse_args() {
       else {
         // Argument not found in list.
         simpletest_script_print_error("Unknown argument '$arg'.");
-        exit;
+        exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
       }
     }
     else {
@@ -309,7 +337,7 @@ function simpletest_script_parse_args() {
   // Validate the concurrency argument
   if (!is_numeric($args['concurrency']) || $args['concurrency'] <= 0) {
     simpletest_script_print_error("--concurrency must be a strictly positive integer.");
-    exit;
+    exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
   }
 
   if ($args['browser']) {
@@ -344,7 +372,7 @@ function simpletest_script_init() {
   else {
     simpletest_script_print_error('Unable to automatically determine the path to the PHP interpreter. Supply the --php command line argument.');
     simpletest_script_help();
-    exit();
+    exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
   }
 
   // Get URL from arguments.
@@ -442,7 +470,7 @@ function simpletest_script_setup_database($new = FALSE) {
     }
     catch (\InvalidArgumentException $e) {
       simpletest_script_print_error('Invalid --dburl. Reason: ' . $e->getMessage());
-      exit(1);
+      exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
     }
   }
   // Otherwise, use the default database connection from settings.php.
@@ -453,7 +481,7 @@ function simpletest_script_setup_database($new = FALSE) {
   // If there is no default database connection for tests, we cannot continue.
   if (!isset($databases['default']['default'])) {
     simpletest_script_print_error('Missing default database connection for tests. Use --dburl to specify one.');
-    exit(1);
+    exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
   }
   Database::addConnectionInfo('default', 'default', $databases['default']['default']);
 
@@ -497,21 +525,33 @@ function simpletest_script_setup_database($new = FALSE) {
   }
   catch (\PDOException $e) {
     simpletest_script_print_error($databases['test-runner']['default']['driver'] . ': ' . $e->getMessage());
-    exit(1);
+    exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
   }
   if ($new && $sqlite) {
     require_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'simpletest') . '/simpletest.install';
     foreach (simpletest_schema() as $name => $table_spec) {
-      if ($schema->tableExists($name)) {
-        $schema->dropTable($name);
+      try {
+        if ($schema->tableExists($name)) {
+          $schema->dropTable($name);
+        }
+        $schema->createTable($name, $table_spec);
+      }
+      catch (Exception $e) {
+        echo (string) $e;
+        exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
       }
-      $schema->createTable($name, $table_spec);
     }
   }
   // Verify that the Simpletest database schema exists by checking one table.
-  if (!$schema->tableExists('simpletest')) {
-    simpletest_script_print_error('Missing Simpletest database schema. Either install Simpletest module or use the --sqlite parameter.');
-    exit(1);
+  try {
+    if (!$schema->tableExists('simpletest')) {
+      simpletest_script_print_error('Missing Simpletest database schema. Either install Simpletest module or use the --sqlite parameter.');
+      exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
+    }
+  }
+  catch (Exception $e) {
+    echo (string) $e;
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
   }
 }
 
@@ -521,6 +561,8 @@ function simpletest_script_setup_database($new = FALSE) {
 function simpletest_script_execute_batch($test_classes) {
   global $args, $test_ids;
 
+  $total_status = SIMPLETEST_SCRIPT_EXIT_SUCCESS;
+
   // Multi-process execution.
   $children = array();
   while (!empty($test_classes) || !empty($children)) {
@@ -529,8 +571,16 @@ function simpletest_script_execute_batch($test_classes) {
         break;
       }
 
-      $test_id = Database::getConnection('default', 'test-runner')
-        ->insert('simpletest_test_id')->useDefaults(array('test_id'))->execute();
+      try {
+        $test_id = Database::getConnection('default', 'test-runner')
+          ->insert('simpletest_test_id')
+          ->useDefaults(array('test_id'))
+          ->execute();
+      }
+      catch (Exception $e) {
+        echo (string) $e;
+        exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+      }
       $test_ids[] = $test_id;
 
       $test_class = array_shift($test_classes);
@@ -540,7 +590,7 @@ function simpletest_script_execute_batch($test_classes) {
 
       if (!is_resource($process)) {
         echo "Unable to fork test process. Aborting.\n";
-        exit;
+        exit(SIMPLETEST_SCRIPT_EXIT_SUCCESS);
       }
 
       // Register our new child.
@@ -561,7 +611,10 @@ function simpletest_script_execute_batch($test_classes) {
       if (empty($status['running'])) {
         // The child exited, unregister it.
         proc_close($child['process']);
-        if ($status['exitcode']) {
+        if ($status['exitcode'] === SIMPLETEST_SCRIPT_EXIT_FAILURE) {
+          $total_status = max($status['exitcode'], $total_status);
+        }
+        elseif ($status['exitcode']) {
           $message = 'FATAL ' . $child['class'] . ': test runner returned a non-zero error code (' . $status['exitcode'] . ').';
           echo $message . "\n";
           // Insert a fail for xml results.
@@ -587,19 +640,19 @@ function simpletest_script_execute_batch($test_classes) {
       }
     }
   }
+  return $total_status;
 }
 
 /**
  * Run a group of phpunit tests.
  */
 function simpletest_script_run_phpunit($test_id, $class) {
-
   $reflection = new \ReflectionClass($class);
   if ($reflection->hasProperty('runLimit')) {
     set_time_limit($reflection->getStaticPropertyValue('runLimit'));
   }
 
-  $results = simpletest_run_phpunit_tests($test_id, array($class));
+  $results = simpletest_run_phpunit_tests($test_id, array($class), $status);
   simpletest_process_phpunit_results($results);
 
   // Map phpunit results to a data structure we can pass to
@@ -634,6 +687,7 @@ function simpletest_script_run_phpunit($test_id, $class) {
   foreach ($summaries as $class => $summary) {
     simpletest_script_reporter_display_summary($class, $summary);
   }
+  return $status;
 }
 
 /**
@@ -654,23 +708,28 @@ function simpletest_script_run_one_test($test_id, $test_class) {
     }
     $test = new $class_name($test_id);
     if (is_subclass_of($test_class, '\PHPUnit_Framework_TestCase')) {
-      simpletest_script_run_phpunit($test_id, $test_class);
+      $status = simpletest_script_run_phpunit($test_id, $test_class);
     }
     else {
       $test->dieOnFail = (bool) $args['die-on-fail'];
       $test->verbose = (bool) $args['verbose'];
       $test->run($methods);
       simpletest_script_reporter_display_summary($test_class, $test->results);
+
+      $status = SIMPLETEST_SCRIPT_EXIT_SUCCESS;
+      // Finished, kill this runner.
+      if ($test->results['#fail'] || $test->results['#exception']) {
+        $status = SIMPLETEST_SCRIPT_EXIT_FAILURE;
+      }
     }
 
-    // Finished, kill this runner.
-    exit(0);
+    exit($status);
   }
   // DrupalTestCase::run() catches exceptions already, so this is only reached
   // when an exception is thrown in the wrapping test runner environment.
   catch (Exception $e) {
     echo (string) $e;
-    exit(1);
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
   }
 }
 
@@ -732,7 +791,13 @@ function simpletest_script_cleanup($test_id, $test_class, $exitcode) {
     return;
   }
   // Retrieve the last database prefix used for testing.
-  list($db_prefix, ) = simpletest_last_test_get($test_id);
+  try {
+    list($db_prefix,) = simpletest_last_test_get($test_id);
+  }
+  catch (Exception $e) {
+    echo (string) $e;
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+  }
 
   // If no database prefix was found, then the test was not set up correctly.
   if (empty($db_prefix)) {
@@ -747,7 +812,13 @@ function simpletest_script_cleanup($test_id, $test_class, $exitcode) {
   $messages[] = "- Found database prefix '$db_prefix' for test ID $test_id.";
 
   // Read the log file in case any fatal errors caused the test to crash.
-  simpletest_log_read($test_id, $db_prefix, $test_class);
+  try {
+    simpletest_log_read($test_id, $db_prefix, $test_class);
+  }
+  catch (Exception $e) {
+    echo (string) $e;
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+  }
 
   // Check whether a test site directory was setup already.
   // @see \Drupal\simpletest\TestBase::prepareEnvironment()
@@ -769,12 +840,19 @@ function simpletest_script_cleanup($test_id, $test_class, $exitcode) {
   }
 
   // Clear out all database tables from the test.
-  $schema = Database::getConnection('default', 'default')->schema();
-  $count = 0;
-  foreach ($schema->findTables($db_prefix . '%') as $table) {
-    $schema->dropTable($table);
-    $count++;
+  try {
+    $schema = Database::getConnection('default', 'default')->schema();
+    $count = 0;
+    foreach ($schema->findTables($db_prefix . '%') as $table) {
+      $schema->dropTable($table);
+      $count++;
+    }
   }
+  catch (Exception $e) {
+    echo (string) $e;
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+  }
+
   if ($count) {
     $messages[] = "- Removed $count leftover tables.";
   }
@@ -798,7 +876,13 @@ function simpletest_script_get_test_list() {
 
   $test_list = array();
   if ($args['all'] || $args['module']) {
-    $groups = simpletest_test_get_all($args['module']);
+    try {
+      $groups = simpletest_test_get_all($args['module']);
+    }
+    catch (Exception $e) {
+      echo (string) $e;
+      exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+    }
     $all_tests = array();
     foreach ($groups as $group => $tests) {
       $all_tests = array_merge($all_tests, array_keys($tests));
@@ -814,14 +898,20 @@ function simpletest_script_get_test_list() {
           $test_list[] = $test_class;
         }
         else {
-          $groups = simpletest_test_get_all();
+          try {
+            $groups = simpletest_test_get_all();
+          }
+          catch (Exception $e) {
+            echo (string) $e;
+            exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+          }
           $all_classes = array();
           foreach ($groups as $group) {
             $all_classes = array_merge($all_classes, array_keys($group));
           }
           simpletest_script_print_error('Test class not found: ' . $class_name);
           simpletest_script_print_alternatives($class_name, $all_classes, 6);
-          exit(1);
+          exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
         }
       }
     }
@@ -830,7 +920,7 @@ function simpletest_script_get_test_list() {
       foreach ($args['test_names'] as $file) {
         if (!file_exists($file)) {
           simpletest_script_print_error('File not found: ' . $file);
-          exit;
+          exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
         }
         $content = file_get_contents($file);
         // Extract a potential namespace.
@@ -909,7 +999,13 @@ function simpletest_script_get_test_list() {
       }
     }
     else {
-      $groups = simpletest_test_get_all();
+      try {
+        $groups = simpletest_test_get_all();
+      }
+      catch (Exception $e) {
+        echo (string) $e;
+        exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+      }
       foreach ($args['test_names'] as $group_name) {
         if (isset($groups[$group_name])) {
           $test_list = array_merge($test_list, array_keys($groups[$group_name]));
@@ -917,7 +1013,7 @@ function simpletest_script_get_test_list() {
         else {
           simpletest_script_print_error('Test group not found: ' . $group_name);
           simpletest_script_print_alternatives($group_name, array_keys($groups));
-          exit(1);
+          exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
         }
       }
     }
@@ -925,7 +1021,7 @@ function simpletest_script_get_test_list() {
 
   if (empty($test_list)) {
     simpletest_script_print_error('No valid tests were specified.');
-    exit;
+    exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
   }
   return $test_list;
 }
@@ -999,7 +1095,13 @@ function simpletest_script_reporter_display_summary($class, $results) {
 function simpletest_script_reporter_write_xml_results() {
   global $args, $test_ids, $results_map;
 
-  $results = simpletest_script_load_messages_by_test_id($test_ids);
+  try {
+    $results = simpletest_script_load_messages_by_test_id($test_ids);
+  }
+  catch (Exception $e) {
+    echo (string) $e;
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+  }
 
   $test_class = '';
   $xml_files = array();
@@ -1089,7 +1191,13 @@ function simpletest_script_reporter_display_results() {
     echo "Detailed test results\n";
     echo "---------------------\n";
 
-    $results = simpletest_script_load_messages_by_test_id($test_ids);
+    try {
+      $results = simpletest_script_load_messages_by_test_id($test_ids);
+    }
+    catch (Exception $e) {
+      echo (string) $e;
+      exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+    }
     $test_class = '';
     foreach ($results as $result) {
       if (isset($results_map[$result->status])) {
@@ -1237,10 +1345,16 @@ function simpletest_script_load_messages_by_test_id($test_ids) {
   }
 
   foreach ($test_id_chunks as $test_id_chunk) {
-    $result_chunk = Database::getConnection('default', 'test-runner')
-      ->query("SELECT * FROM {simpletest} WHERE test_id IN ( :test_ids[] ) ORDER BY test_class, message_id", array(
-        ':test_ids[]' => $test_id_chunk,
-      ))->fetchAll();
+    try {
+      $result_chunk = Database::getConnection('default', 'test-runner')
+        ->query("SELECT * FROM {simpletest} WHERE test_id IN ( :test_ids[] ) ORDER BY test_class, message_id", array(
+          ':test_ids[]' => $test_id_chunk,
+        ))->fetchAll();
+    }
+    catch (Exception $e) {
+      echo (string) $e;
+      exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+    }
     if ($result_chunk) {
       $results = array_merge($results, $result_chunk);
     }
@@ -1255,14 +1369,20 @@ function simpletest_script_load_messages_by_test_id($test_ids) {
 function simpletest_script_open_browser() {
   global $test_ids;
 
-  $connection = Database::getConnection('default', 'test-runner');
-  $results = $connection->select('simpletest')
-    ->fields('simpletest')
-    ->condition('test_id', $test_ids, 'IN')
-    ->orderBy('test_class')
-    ->orderBy('message_id')
-    ->execute()
-    ->fetchAll();
+  try {
+    $connection = Database::getConnection('default', 'test-runner');
+    $results = $connection->select('simpletest')
+      ->fields('simpletest')
+      ->condition('test_id', $test_ids, 'IN')
+      ->orderBy('test_class')
+      ->orderBy('message_id')
+      ->execute()
+      ->fetchAll();
+  }
+  catch (Exception $e) {
+    echo (string) $e;
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+  }
 
   // Get the results form.
   $form = array();