Issue #2422019 by alexpott, dawehner: Don't use reflection for parsing test annotations

core/modules/migrate/tests/src/Unit/Entity/MigrationTest.php

@@ -14,6 +14,7 @@ use Drupal\Tests\UnitTestCase;
  * Tests the migrate entity.
  *
  * @coversDefaultClass \Drupal\migrate\Entity\Migration
+ * @group migrate
  */
 class MigrationTest extends UnitTestCase {
 

core/modules/simpletest/simpletest.module

@@ -144,7 +144,7 @@ function simpletest_run_tests($test_list) {
 
   // Get the info for the first test being run.
   $first_test = reset($test_list);
-  $info = TestDiscovery::getTestInfo(new \ReflectionClass($first_test));
+  $info = TestDiscovery::getTestInfo($first_test);
 
   $batch = array(
     'title' => t('Running tests'),
@@ -320,7 +320,7 @@ function _simpletest_batch_operation($test_list_init, $test_id, &$context) {
   $test = new $test_class($test_id);
   $test->run();
   $size = count($test_list);
-  $info = TestDiscovery::getTestInfo(new \ReflectionClass($test));
+  $info = TestDiscovery::getTestInfo($test_class);
 
   \Drupal::moduleHandler()->invokeAll('test_finished', array($test->results));
 

core/modules/simpletest/src/Exception/MissingGroupException.php

@@ -0,0 +1,14 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\simpletest\Exception\MissingGroupException.
+ */
+
+namespace Drupal\simpletest\Exception;
+
+/**
+ * Exception thrown when a simpletest class is missing an @group annotation.
+ */
+class MissingGroupException extends \LogicException {
+}

core/modules/simpletest/src/Exception/MissingSummaryLineException.php

@@ -0,0 +1,14 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\simpletest\Exception\MissingSummaryLineException.
+ */
+
+namespace Drupal\simpletest\Exception;
+
+/**
+ * Exception thrown when a simpletest class is missing a summary line.
+ */
+class MissingSummaryLineException extends \LogicException {
+}

core/modules/simpletest/src/Form/SimpletestResultsForm.php

@@ -153,7 +153,7 @@ class SimpletestResultsForm extends FormBase {
     $form['result']['results'] = array();
     foreach ($results as $group => $assertions) {
       // Create group details with summary information.
-      $info = TestDiscovery::getTestInfo(new \ReflectionClass($group));
+      $info = TestDiscovery::getTestInfo($group);
       $form['result']['results'][$group] = array(
         '#type' => 'details',
         '#title' => $info['name'],

core/modules/simpletest/src/TestDiscovery.php

@@ -8,9 +8,14 @@
 namespace Drupal\simpletest;
 
 use Composer\Autoload\ClassLoader;
+use Doctrine\Common\Annotations\SimpleAnnotationReader;
+use Doctrine\Common\Reflection\StaticReflectionParser;
+use Drupal\Component\Annotation\Reflection\MockFileFinder;
+use Drupal\Component\Utility\Unicode;
 use Drupal\Core\Cache\CacheBackendInterface;
-use Drupal\Core\Extension\Extension;
 use Drupal\Core\Extension\ExtensionDiscovery;
+use Drupal\simpletest\Exception\MissingGroupException;
+use Drupal\simpletest\Exception\MissingSummaryLineException;
 use PHPUnit_Util_Test;
 
 /**
@@ -128,6 +133,9 @@ class TestDiscovery {
    * @todo Add base class groups 'Kernel' + 'Web', complementing 'PHPUnit'.
    */
   public function getTestClasses($extension = NULL) {
+    $reader = new SimpleAnnotationReader();
+    $reader->addNamespace('Drupal\\simpletest\\Annotation');
+
     if (!isset($extension)) {
       if ($this->cacheBackend && $cache = $this->cacheBackend->get('simpletest:discovery:classes')) {
         return $cache->data;
@@ -144,24 +152,16 @@ class TestDiscovery {
     $this->classLoader->addClassMap($classmap);
 
     foreach ($classmap as $classname => $pathname) {
+      $finder = MockFileFinder::create($pathname);
+      $parser = new StaticReflectionParser($classname, $finder, TRUE);
       try {
-        $class = new \ReflectionClass($classname);
+        $info = static::getTestInfo($classname, $parser->getDocComment());
       }
-      catch (\ReflectionException $e) {
+      catch (\LogicException $e) {
-        // Re-throw with expected pathname.
+        // If the class is missing a summary line or an @group annotation just
-        $message = $e->getMessage() . " in expected $pathname";
+        // skip it. Most likely it is an abstract class, trait or test fixture.
-        throw new \ReflectionException($message, $e->getCode(), $e);
-      }
-      // Skip interfaces, abstract classes, and traits.
-      if (!$class->isInstantiable()) {
         continue;
       }
-      // Skip non-test classes.
-      if (!$class->isSubclassOf('Drupal\simpletest\TestBase') && !$class->isSubclassOf('PHPUnit_Framework_TestCase')) {
-        continue;
-      }
-      $info = static::getTestInfo($class);
-
       // Skip this test class if it requires unavailable modules.
       // @todo PHPUnit skips tests with unmet requirements when executing a test
       //   (instead of excluding them upfront). Refactor test runner to follow
@@ -275,8 +275,11 @@ class TestDiscovery {
   /**
    * Retrieves information about a test class for UI purposes.
    *
-   * @param \ReflectionClass $class
+   * @param string $class
-   *   The reflected test class.
+   *   The test classname.
+   * @param string $doc_comment
+   *   (optional) The class PHPDoc comment. If not passed in reflection will be
+   *   used but this is very expensive when parsing all the test classes.
    *
    * @return array
    *   An associative array containing:
@@ -287,79 +290,56 @@ class TestDiscovery {
    *     PHPDoc annotations:
    *     - module: List of Drupal module extension names the test depends on.
    *
-   * @throws \LogicException
+   * @throws \Drupal\simpletest\Exception\MissingSummaryLineException
    *   If the class does not have a PHPDoc summary line or @coversDefaultClass
    *   annotation.
-   * @throws \LogicException
+   * @throws \Drupal\simpletest\Exception\MissingGroupException
    *   If the class does not have a @group annotation.
    */
-  public static function getTestInfo(\ReflectionClass $class) {
+  public static function getTestInfo($classname, $doc_comment = NULL) {
-    $classname = $class->getName();
+    if (!$doc_comment) {
+      $reflection = new \ReflectionClass($classname);
+      $doc_comment = $reflection->getDocComment();
+    }
     $info = array(
       'name' => $classname,
     );
-
+    $annotations = array();
-    // Automatically convert @coversDefaultClass into summary.
+    preg_match_all('/^ \* \@([^\s]*) (.*$)/m', $doc_comment, $matches);
-    $annotations = static::parseTestClassAnnotations($class);
+    if (isset($matches[1])) {
-    if (isset($annotations['coversDefaultClass'][0])) {
+      foreach ($matches[1] as $key => $annotation) {
-      $info['description'] = 'Tests ' . $annotations['coversDefaultClass'][0] . '.';
+        if (!empty($annotations[$annotation])) {
-    }
+          // Only have the first match per annotation. This deals with
-    elseif ($summary = static::parseTestClassSummary($class)) {
+          // multiple @group annotations.
-      $info['description'] = $summary;
+          continue;
-    }
-    else {
-      throw new \LogicException(sprintf('Missing PHPDoc summary line on %s in %s.', $classname, $class->getFileName()));
-    }
-
-    // Reduce to @group and @requires.
-    $info += array_intersect_key($annotations, array('group' => 1, 'requires' => 1));
-
-    // @todo Remove legacy getInfo() methods.
-    if (method_exists($classname, 'getInfo')) {
-      $legacy_info = $classname::getInfo();
-
-      // Derive the primary @group from the namespace to ensure that legacy
-      // tests are not located in different groups than converted tests.
-      $classparts = explode('\\', $classname);
-      if ($classparts[1] === 'Tests') {
-        if ($classparts[2] === 'Component' || $classparts[2] === 'Core') {
-          // Drupal\Tests\Component\{group}\...
-          $info['group'][] = $classparts[3];
         }
-        else {
+        $annotations[$annotation] = $matches[2][$key];
-          // Drupal\Tests\{group}\...
-          $info['group'][] = $classparts[2];
-        }
-      }
-      elseif ($classparts[1] === 'system' && $classparts[3] !== 'System') {
-        // Drupal\system\Tests\{group}\...
-        $info['group'][] = $classparts[3];
-      }
-      else {
-        // Drupal\{group}\Tests\...
-        $info['group'][] = $classparts[1];
-      }
-
-      if (isset($legacy_info['dependencies'])) {
-        $info += array('requires' => array());
-        $info['requires'] += array('module' => array());
-        $info['requires']['module'] = array_merge($info['requires']['module'], $legacy_info['dependencies']);
       }
     }
 
-    // Process @group information.
+    if (empty($annotations['group'])) {
-    // @todo Support multiple @groups + change UI to expose a group select
+      // Concrete tests must have a group.
-    //   dropdown to filter tests by group instead of collapsible table rows.
+      throw new MissingGroupException(sprintf('Missing @group annotation in %s', $classname));
-    // @see https://www.drupal.org/node/2296615
+    }
-    // @todo Replace single enforced PHPUnit group with base class groups.
+    // Force all PHPUnit tests into the same group.
-    if ($class->isSubclassOf('PHPUnit_Framework_TestCase')) {
+    if (strpos($classname, 'Drupal\\Tests\\') === 0) {
       $info['group'] = 'PHPUnit';
     }
     else {
-      if (empty($info['group'])) {
+      $info['group'] = $annotations['group'];
-        throw new \LogicException("Missing @group for $classname.");
+    }
+
+    if (!empty($annotations['coversDefaultClass'])) {
+      $info['description'] = 'Tests ' . $annotations['coversDefaultClass'] . '.';
+    }
+    else {
+      $info['description'] = static::parseTestClassSummary($doc_comment);
+      if (empty($info['description'])) {
+        throw new MissingSummaryLineException(sprintf('Missing PHPDoc summary line in %s', $classname));
       }
-      $info['group'] = reset($info['group']);
+    }
+    if (isset($annotations['dependencies'])) {
+      $info['requires']['module'] = array_map('trim', explode(',', $annotations['dependencies']));
     }
 
     return $info;
@@ -368,32 +348,27 @@ class TestDiscovery {
   /**
    * Parses the phpDoc summary line of a test class.
    *
-   * @param \ReflectionClass $class
+   * @param string $doc_comment.
-   *   The reflected test class.
    *
    * @return string
-   *   The parsed phpDoc summary line.
+   *   The parsed phpDoc summary line. An empty string is returned if no summary
+   *   line can be parsed.
    */
-  public static function parseTestClassSummary(\ReflectionClass $class) {
+  public static function parseTestClassSummary($doc_comment) {
-    $phpDoc = $class->getDocComment();
     // Normalize line endings.
-    $phpDoc = preg_replace('/\r\n|\r/', '\n', $phpDoc);
+    $doc_comment = preg_replace('/\r\n|\r/', '\n', $doc_comment);
     // Strip leading and trailing doc block lines.
-    //$phpDoc = trim($phpDoc, "* /\n");
+    $doc_comment = substr($doc_comment, 4, -4);
-    $phpDoc = substr($phpDoc, 4, -4);
 
-    // Extract actual phpDoc content.
+    $lines = explode("\n", $doc_comment);
-    $phpDoc = explode("\n", $phpDoc);
+    $summary = [];
-    array_walk($phpDoc, function (&$value) {
+    foreach ($lines as $line) {
-      $value = trim($value, "* /\n");
+      if ($line == ' *' || preg_match('/^ \* \@/', $line)) {
-    });
+        break;
-
+      }
-    // Extract summary; allowed to it wrap and continue on next line.
+      $summary[] = trim($line, ' *');
-    list($summary) = explode("\n\n", implode("\n", $phpDoc));
-    if ($summary === '') {
-      throw new \LogicException(sprintf('Missing phpDoc on %s.', $class->getName()));
     }
-    return $summary;
+    return implode(' ', $summary);
   }
 
   /**

core/modules/simpletest/src/Tests/MissingDependentModuleUnitTest.php

@@ -13,7 +13,7 @@ use Drupal\simpletest\KernelTestBase;
  * This test should not load since it requires a module that is not found.
  *
  * @group simpletest
- * @requires module simpletest_missing_module
+ * @dependencies simpletest_missing_module
  */
 class MissingDependentModuleUnitTest extends KernelTestBase {
 

core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php

@@ -0,0 +1,165 @@
+<?php
+
+namespace Drupal\Tests\simpletest\Unit;
+
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * @coversDefaultClass \Drupal\simpletest\TestDiscovery
+ * @group simpletest
+ */
+class TestInfoParsingTest extends UnitTestCase {
+
+  /**
+   * @covers ::getTestInfo
+   * @dataProvider infoParserProvider
+   */
+  public function testTestInfoParser($expected, $classname, $doc_comment = NULL) {
+    $info = \Drupal\simpletest\TestDiscovery::getTestInfo($classname, $doc_comment);
+    $this->assertEquals($expected, $info);
+  }
+
+  public function infoParserProvider() {
+    $tests[] = [
+      // Expected result.
+      [
+        'name' => 'Drupal\Tests\simpletest\Unit\TestInfoParsingTest',
+        'group' => 'PHPUnit',
+        'description' => 'Tests \Drupal\simpletest\TestDiscovery.',
+      ],
+      // Classname.
+      'Drupal\Tests\simpletest\Unit\TestInfoParsingTest',
+    ];
+
+    $tests[] = [
+      // Expected result.
+      [
+        'name' => 'Drupal\field\Tests\BulkDeleteTest',
+        'group' => 'field',
+        'description' => 'Bulk delete storages and fields, and clean up afterwards.',
+      ],
+      // Classname.
+      'Drupal\field\Tests\BulkDeleteTest',
+      // Doc block.
+      "/**
+ * Bulk delete storages and fields, and clean up afterwards.
+ *
+ * @group field
+ */
+ ",
+    ];
+
+    // Multiple @group annotations.
+    $tests[] = [
+      // Expected result.
+      [
+        'name' => 'Drupal\field\Tests\BulkDeleteTest',
+        'group' => 'Test',
+        'description' => 'Bulk delete storages and fields, and clean up afterwards.',
+      ],
+      // Classname.
+      'Drupal\field\Tests\BulkDeleteTest',
+      // Doc block.
+      "/**
+ * Bulk delete storages and fields, and clean up afterwards.
+ *
+ * @group Test
+ * @group field
+ */
+ ",
+    ];
+
+    // @dependencies annotation.
+    $tests[] = [
+      // Expected result.
+      [
+        'name' => 'Drupal\field\Tests\BulkDeleteTest',
+        'group' => 'field',
+        'description' => 'Bulk delete storages and fields, and clean up afterwards.',
+        'requires' => ['module' => ['test']],
+      ],
+      // Classname.
+      'Drupal\field\Tests\BulkDeleteTest',
+      // Doc block.
+      "/**
+ * Bulk delete storages and fields, and clean up afterwards.
+ *
+ * @dependencies test
+ * @group field
+ */
+ ",
+    ];
+
+    // Multiple @dependencies annotation.
+    $tests[] = [
+      // Expected result.
+      [
+        'name' => 'Drupal\field\Tests\BulkDeleteTest',
+        'group' => 'field',
+        'description' => 'Bulk delete storages and fields, and clean up afterwards.',
+        'requires' => ['module' => ['test', 'test1', 'test2']],
+      ],
+      // Classname.
+      'Drupal\field\Tests\BulkDeleteTest',
+      // Doc block.
+      "/**
+ * Bulk delete storages and fields, and clean up afterwards.
+ *
+ * @dependencies test, test1,test2
+ * @group field
+ */
+ ",
+    ];
+
+    // Multi-line summary line.
+    $tests[] = [
+      // Expected result.
+      [
+        'name' => 'Drupal\field\Tests\BulkDeleteTest',
+        'group' => 'field',
+        'description' => 'Bulk delete storages and fields, and clean up afterwards. And the summary line continues and there is no gap to the annotation.',
+      ],
+      // Classname.
+      'Drupal\field\Tests\BulkDeleteTest',
+      // Doc block.
+      "/**
+ * Bulk delete storages and fields, and clean up afterwards. And the summary
+ * line continues and there is no gap to the annotation.
+ * @group field
+ */
+ ",
+    ];
+    return $tests;
+  }
+
+  /**
+   * @covers ::getTestInfo
+   * @expectedException \Drupal\simpletest\Exception\MissingGroupException
+   * @expectedExceptionMessage Missing @group annotation in Drupal\field\Tests\BulkDeleteTest
+   */
+  public function testTestInfoParserMissingGroup() {
+    $classname = 'Drupal\field\Tests\BulkDeleteTest';
+    $doc_comment = <<<EOT
+/**
+ * Bulk delete storages and fields, and clean up afterwards.
+ */
+EOT;
+    \Drupal\simpletest\TestDiscovery::getTestInfo($classname, $doc_comment);
+  }
+
+  /**
+   * @covers ::getTestInfo
+   * @expectedException \Drupal\simpletest\Exception\MissingSummaryLineException
+   * @expectedExceptionMessage Missing PHPDoc summary line in Drupal\field\Tests\BulkDeleteTest
+   */
+  public function testTestInfoParserMissingSummary() {
+    $classname = 'Drupal\field\Tests\BulkDeleteTest';
+    $doc_comment = <<<EOT
+/**
+ * @group field
+ */
+EOT;
+    \Drupal\simpletest\TestDiscovery::getTestInfo($classname, $doc_comment);
+  }
+
+}

core/tests/Drupal/Tests/Component/EventDispatcher/ContainerAwareEventDispatcherTest.php

@@ -26,6 +26,8 @@ use Symfony\Component\EventDispatcher\Tests\TestEventListener;
  * synchronizations.
  *
  * @see https://github.com/symfony/symfony/pull/12521
+ *
+ * @group Symfony
  */
 class ContainerAwareEventDispatcherTest extends AbstractEventDispatcherTest
 {

core/tests/Drupal/Tests/Component/Plugin/DefaultFactoryTest.php

@@ -12,6 +12,7 @@ use Drupal\Tests\UnitTestCase;
 
 /**
  * @coversDefaultClass \Drupal\Component\Plugin\DefaultFactory
+ * @group Plugin
  */
 class DefaultFactoryTest extends UnitTestCase {