Issue #2913819 by Mile23, Wim Leers, catch: run-tests.sh ignores final classes

core/lib/Drupal/Core/Test/RunTests/TestFileParser.php

+<?php
+
+namespace Drupal\Core\Test\RunTests;
+
+use Drupal\simpletest\TestBase;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Parses class names from PHP files without loading them.
+ *
+ * @internal
+ */
+class TestFileParser {
+
+  /**
+   * Gets the classes from a PHP file.
+   *
+   * @param string $file
+   *   The path to the file to parse.
+   *
+   * @return string[]
+   *   Array of fully qualified class names within the PHP file.
+   */
+  public function getTestListFromFile($file) {
+    $test_list = $this->parseContents(file_get_contents($file));
+    return array_filter($test_list, function ($class) {
+      return (is_subclass_of($class, TestCase::class) || is_subclass_of($class, TestBase::class));
+    });
+  }
+
+  /**
+   * Parse class names out of PHP file contents.
+   *
+   * @param string $contents
+   *   The contents of a PHP file.
+   *
+   * @return string[]
+   *   Array of fully qualified class names within the PHP file contents.
+   */
+  protected function parseContents($contents) {
+    // Extract a potential namespace.
+    $namespace = FALSE;
+    if (preg_match('@^\s*namespace ([^ ;]+)@m', $contents, $matches)) {
+      $namespace = $matches[1];
+    }
+    $test_list = [];
+    // Extract all class names. Abstract classes are excluded on purpose.
+    preg_match_all('@^\s*(?!abstract\s+)(?:final\s+|\s*)class ([^ ]+)@m', $contents, $matches);
+    if (!$namespace) {
+      $test_list = $matches[1];
+    }
+    else {
+      foreach ($matches[1] as $class_name) {
+        $namespace_class = $namespace . '\\' . $class_name;
+        $test_list[] = $namespace_class;
+      }
+    }
+    return $test_list;
+  }
+
+}

core/scripts/run-tests.sh

@@ -15,6 +15,7 @@
 use Drupal\Core\File\Exception\FileException;
 use Drupal\Core\File\FileSystemInterface;
 use Drupal\Core\StreamWrapper\PublicStream;
+use Drupal\Core\Test\RunTests\TestFileParser;
 use Drupal\Core\Test\TestDatabase;
 use Drupal\Core\Test\TestRunnerKernel;
 use Drupal\simpletest\Form\SimpletestResultsForm;
@@ -1063,31 +1064,13 @@ function simpletest_script_get_test_list() {
     }
     elseif ($args['file']) {
       // Extract test case class names from specified files.
+      $parser = new TestFileParser();
       foreach ($args['test_names'] as $file) {
         if (!file_exists($file)) {
           simpletest_script_print_error('File not found: ' . $file);
           exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
         }
-        $content = file_get_contents($file);
-        // Extract a potential namespace.
-        $namespace = FALSE;
-        if (preg_match('@^namespace ([^ ;]+)@m', $content, $matches)) {
-          $namespace = $matches[1];
-        }
-        // Extract all class names.
-        // Abstract classes are excluded on purpose.
-        preg_match_all('@^class ([^ ]+)@m', $content, $matches);
-        if (!$namespace) {
-          $test_list = array_merge($test_list, $matches[1]);
-        }
-        else {
-          foreach ($matches[1] as $class_name) {
-            $namespace_class = $namespace . '\\' . $class_name;
-            if (is_subclass_of($namespace_class, '\Drupal\simpletest\TestBase') || is_subclass_of($namespace_class, TestCase::class)) {
-              $test_list[] = $namespace_class;
-            }
-          }
-        }
+        $test_list = array_merge($test_list, $parser->getTestListFromFile($file));
       }
     }
     elseif ($args['directory']) {
@@ -1121,27 +1104,9 @@ function simpletest_script_get_test_list() {
           $files[$filename] = $filename;
         }
       }
+      $parser = new TestFileParser();
       foreach ($files as $file) {
-        $content = file_get_contents($file);
-        // Extract a potential namespace.
-        $namespace = FALSE;
-        if (preg_match('@^\s*namespace ([^ ;]+)@m', $content, $matches)) {
-          $namespace = $matches[1];
-        }
-        // Extract all class names.
-        // Abstract classes are excluded on purpose.
-        preg_match_all('@^\s*class ([^ ]+)@m', $content, $matches);
-        if (!$namespace) {
-          $test_list = array_merge($test_list, $matches[1]);
-        }
-        else {
-          foreach ($matches[1] as $class_name) {
-            $namespace_class = $namespace . '\\' . $class_name;
-            if (is_subclass_of($namespace_class, '\Drupal\simpletest\TestBase') || is_subclass_of($namespace_class, TestCase::class)) {
-              $test_list[] = $namespace_class;
-            }
-          }
-        }
+        $test_list = array_merge($test_list, $parser->getTestListFromFile($file));
       }
     }
     else {

core/tests/Drupal/Tests/Core/Test/RunTests/TestFileParserTest.php

+<?php
+
+namespace Drupal\Tests\Core\Test\RunTests;
+
+use Drupal\Core\Test\RunTests\TestFileParser;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Test\RunTests\TestFileParser
+ * @group Test
+ * @group RunTests
+ */
+class TestFileParserTest extends UnitTestCase {
+
+  public function provideTestFileContents() {
+    return [
+      'empty' => [[], ''],
+      'no-namespace' => [['ConcreteClass'],
+       <<< 'NO_NAMESPACE'
+<?php
+
+class ConcreteClass {}
+NO_NAMESPACE
+      ],
+      'concrete' => [['Namespace\Is\Complex\ConcreteClass'],
+       <<< 'CONCRETE_CLASS'
+<?php
+
+namespace Namespace\Is\Complex;
+
+class ConcreteClass {}
+CONCRETE_CLASS
+      ],
+      'abstract' => [[],
+       <<< 'ABSTRACT_CLASS'
+<?php
+namespace Namespace\Is\Complex;
+
+abstract class AbstractClass {}
+ABSTRACT_CLASS
+      ],
+      'final' => [['Namespace\Is\Complex\FinalClass'],
+       <<< 'FINAL_CLASS'
+<?php
+namespace Namespace\Is\Complex;
+
+final class FinalClass {}
+FINAL_CLASS
+      ],
+      'compound_declarations' => [[
+        'Namespace\Is\Complex\FinalClass',
+        'Namespace\Is\Complex\AnotherClass',
+      ],
+       <<< 'COMPOUND'
+<?php
+namespace Namespace\Is\Complex;
+
+final class FinalClass {}
+
+class AnotherClass {}
+COMPOUND
+      ],
+    ];
+  }
+
+  /**
+   * @covers ::parseContents
+   * @dataProvider provideTestFileContents
+   */
+  public function testParseContents($expected, $contents) {
+    $parser = new TestFileParser();
+
+    $ref_parse = new \ReflectionMethod($parser, 'parseContents');
+    $ref_parse->setAccessible(TRUE);
+
+    $this->assertSame($expected, $ref_parse->invoke($parser, $contents));
+  }
+
+  /**
+   * @covers ::getTestListFromFile
+   */
+  public function testGetTestListFromFile() {
+    $parser = new TestFileParser();
+    $this->assertArrayEquals(
+      ['Drupal\Tests\Core\Test\RunTests\TestFileParserTest'],
+      $parser->getTestListFromFile(__FILE__)
+    );
+    // This WebTestBase test will eventually move, so we'll need to update it.
+    $this->assertArrayEquals(
+      ['Drupal\simpletest\Tests\TimeZoneTest'],
+      $parser->getTestListFromFile(__DIR__ . '/../../../../../../modules/simpletest/src/Tests/TimeZoneTest.php')
+    );
+    // Not a test.
+    $this->assertEmpty(
+      $parser->getTestListFromFile(__DIR__ . '/../../../AssertHelperTrait.php')
+    );
+  }
+
+}