Issue #3450616 by catch, quietone, smustgrave: Optimize test order when --directory is used

(cherry picked from commit af883167)

core/lib/Drupal/Core/Test/TestDiscovery.php

@@ -124,7 +124,9 @@ public function registerTestNamespaces() {
    * @param string $extension
    *   (optional) The name of an extension to limit discovery to; e.g., 'node'.
    * @param string[] $types
-   *   An array of included test types.
+   *   (optional) An array of included test types.
+   * @param string|null $directory
+   *   (optional) Limit discovered tests to a specific directory.
    *
    * @return array
    *   An array of tests keyed by the group name. If a test is annotated to
@@ -145,7 +147,7 @@ public function registerTestNamespaces() {
    * @todo Remove singular grouping; retain list of groups in 'group' key.
    * @see https://www.drupal.org/node/2296615
    */
-  public function getTestClasses($extension = NULL, array $types = []) {
+  public function getTestClasses($extension = NULL, array $types = [], ?string $directory = NULL) {
     if (!isset($extension) && empty($types)) {
       if (!empty($this->testClasses)) {
         return $this->testClasses;
@@ -153,7 +155,7 @@ public function getTestClasses($extension = NULL, array $types = []) {
     }
     $list = [];
 
-    $classmap = $this->findAllClassFiles($extension);
+    $classmap = $this->findAllClassFiles($extension, $directory);
 
     // Prevent expensive class loader lookups for each reflected test class by
     // registering the complete classmap of test classes to the class loader.
@@ -205,12 +207,14 @@ public function getTestClasses($extension = NULL, array $types = []) {
    *
    * @param string $extension
    *   (optional) The name of an extension to limit discovery to; e.g., 'node'.
+   * @param string|null $directory
+   *   (optional) Limit discovered tests to a specific directory.
    *
    * @return array
    *   A classmap containing all discovered class files; i.e., a map of
    *   fully-qualified classnames to path names.
    */
-  public function findAllClassFiles($extension = NULL) {
+  public function findAllClassFiles($extension = NULL, ?string $directory = NULL) {
     $classmap = [];
     $namespaces = $this->registerTestNamespaces();
     if (isset($extension)) {
@@ -220,7 +224,7 @@ public function findAllClassFiles($extension = NULL) {
     }
     foreach ($namespaces as $namespace => $paths) {
       foreach ($paths as $path) {
-        if (!is_dir($path)) {
+        if (!is_dir($path) || (!is_null($directory) && !str_contains($path, $directory))) {
           continue;
         }
         $classmap += static::scanDirectory($namespace, $path);

core/scripts/run-tests.sh

@@ -907,15 +907,17 @@ function simpletest_script_get_test_list() {
   $types_processed = empty($args['types']);
   $test_list = [];
   $slow_tests = [];
-  if ($args['all'] || $args['module']) {
+  if ($args['all'] || $args['module'] || $args['directory']) {
     try {
-      $groups = $test_discovery->getTestClasses($args['module'], $args['types']);
+      $groups = $test_discovery->getTestClasses($args['module'], $args['types'], $args['directory']);
       $types_processed = TRUE;
     }
     catch (Exception $e) {
       echo (string) $e;
       exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
     }
+    // If the tests are run in parallel jobs, ensure that slow tests are
+    // distributed between each job.
     if ((int) $args['ci-parallel-node-total'] > 1) {
       if (key($groups) === '#slow') {
         $slow_tests = array_keys(array_shift($groups));
@@ -923,8 +925,29 @@ function simpletest_script_get_test_list() {
     }
     $all_tests = [];
     foreach ($groups as $group => $tests) {
+      if ($group === '#slow') {
+        $slow_group = $tests;
+      }
+      else {
         $all_tests = array_merge($all_tests, array_keys($tests));
       }
+    }
+    // If no type has been set, order the tests alphabetically by test namespace
+    // so that unit tests run last. This takes advantage of the fact that Build,
+    // Functional, Functional JavaScript, Kernel, Unit roughly corresponds to
+    // test time.
+    usort($all_tests, function ($a, $b) {
+      $slice = function ($class) {
+        $parts = explode('\\', $class);
+        return implode('\\', array_slice($parts, 3));
+      };
+      return $slice($a) > $slice($b) ? 1 : -1;
+    });
+    // If the tests are not being run in parallel, then ensure slow tests run all
+    // together first.
+    if ((int) $args['ci-parallel-node-total'] <= 1 && !empty($slow_group)) {
+      $all_tests = array_merge(array_keys($slow_group), $all_tests);
+    }
     $test_list = array_unique($all_tests);
     $test_list = array_diff($test_list, $slow_tests);
   }
@@ -965,42 +988,6 @@ function simpletest_script_get_test_list() {
         $test_list = array_merge($test_list, $parser->getTestListFromFile($file));
       }
     }
-    elseif ($args['directory']) {
-      // Extract test case class names from specified directory.
-      // Find all tests in the PSR-X structure; Drupal\$extension\Tests\*.php
-      // Since we do not want to hard-code too many structural file/directory
-      // assumptions about PSR-4 files and directories, we check for the
-      // minimal conditions only; i.e., a '*.php' file that has '/Tests/' in
-      // its path.
-      // Ignore anything from third party vendors.
-      $ignore = ['.', '..', 'vendor'];
-      $files = [];
-      if ($args['directory'][0] === '/') {
-        $directory = $args['directory'];
-      }
-      else {
-        $directory = DRUPAL_ROOT . "/" . $args['directory'];
-      }
-      foreach (\Drupal::service('file_system')->scanDirectory($directory, '/\.php$/', $ignore) as $file) {
-        // '/Tests/' can be contained anywhere in the file's path (there can be
-        // sub-directories below /Tests), but must be contained literally.
-        // Case-insensitive to match all Simpletest and PHPUnit tests:
-        // ./lib/Drupal/foo/Tests/Bar/Baz.php
-        // ./foo/src/Tests/Bar/Baz.php
-        // ./foo/tests/Drupal/foo/Tests/FooTest.php
-        // ./foo/tests/src/FooTest.php
-        // $file->filename doesn't give us a directory, so we use $file->uri
-        // Strip the drupal root directory and trailing slash off the URI.
-        $filename = substr($file->uri, strlen(DRUPAL_ROOT) + 1);
-        if (stripos($filename, '/Tests/')) {
-          $files[$filename] = $filename;
-        }
-      }
-      $parser = new TestFileParser();
-      foreach ($files as $file) {
-        $test_list = array_merge($test_list, $parser->getTestListFromFile($file));
-      }
-    }
     else {
       try {
         $groups = $test_discovery->getTestClasses(NULL, $args['types']);