Issue #2296615 by Mile23, chr.fritsch, jibran, borisson_, dawehner, alexpott,...

Issue #2296615 by Mile23, chr.fritsch, jibran, borisson_, dawehner, alexpott, andypost: Accurately support multiple @groups per test class

Issue #2296615 by Mile23, chr.fritsch, jibran, borisson_, dawehner, alexpott,...
2af907ca · Alex Pott · 165ba7ec · 2af907ca · 2af907ca · 2af907ca
Unverified Commit 2af907ca authored 6 years ago by Alex Pott
--- a/core/modules/simpletest/src/TestDiscovery.php
+++ b/core/modules/simpletest/src/TestDiscovery.php
@@ -2,7 +2,6 @@
 namespace Drupal\simpletest;
-use Doctrine\Common\Annotations\SimpleAnnotationReader;
 use Doctrine\Common\Reflection\StaticReflectionParser;
 use Drupal\Component\Annotation\Reflection\MockFileFinder;
 use Drupal\Component\Utility\NestedArray;
@@ -137,13 +136,16 @@ public function registerTestNamespaces() {
   *   An array of included test types.
   *
   * @return array
-   *   An array of tests keyed by the the group name.
+   *   An array of tests keyed by the the group name. If a test is annotated to
+   *   belong to multiple groups, it will appear under all group keys it belongs
+   *   to.
   * @code
   *     $groups['block'] => array(
   *       'Drupal\Tests\block\Functional\BlockTest' => array(
   *         'name' => 'Drupal\Tests\block\Functional\BlockTest',
   *         'description' => 'Tests block UI CRUD functionality.',
   *         'group' => 'block',
+   *         'groups' => ['block', 'group2', 'group3'],
   *       ),
   *     );
   * @endcode
@@ -152,9 +154,6 @@ public function registerTestNamespaces() {
   * @see https://www.drupal.org/node/2296615
   */
  public function getTestClasses($extension = NULL, array $types = []) {
-    $reader = new SimpleAnnotationReader();
-    $reader->addNamespace('Drupal\\simpletest\\Annotation');
    if (!isset($extension) && empty($types)) {
      if (!empty($this->testClasses)) {
        return $this->testClasses;
@@ -199,7 +198,9 @@ public function getTestClasses($extension = NULL, array $types = []) {
        }
      }
-      $list[$info['group']][$classname] = $info;
+      foreach ($info['groups'] as $group) {
+        $list[$group][$classname] = $info;
+      }
    }
    // Sort the groups and tests within the groups by name.
@@ -325,6 +326,8 @@ public static function scanDirectory($namespace_prefix, $path) {
   *   - name: The test class name.
   *   - description: The test (PHPDoc) summary.
   *   - group: The test's first @group (parsed from PHPDoc annotations).
+   *   - groups: All of the test's @group annotations, as an array (parsed from
+   *     PHPDoc annotations).
   *   - requires: An associative array containing test requirements parsed from
   *     PHPDoc annotations:
   *     - module: List of Drupal module extension names the test depends on.
@@ -346,9 +349,14 @@ public static function getTestInfo($classname, $doc_comment = NULL) {
    preg_match_all('/^[ ]*\* \@([^\s]*) (.*$)/m', $doc_comment, $matches);
    if (isset($matches[1])) {
      foreach ($matches[1] as $key => $annotation) {
+        // For historical reasons, there is a single-value 'group' result key
+        // and a 'groups' key as an array.
+        if ($annotation === 'group') {
+          $annotations['groups'][] = $matches[2][$key];
+        }
        if (!empty($annotations[$annotation])) {
-          // Only have the first match per annotation. This deals with
+          // Only @group is allowed to have more than one annotation, in the
-          // multiple @group annotations.
+          // 'groups' key. Other annotations only have one value per key.
          continue;
        }
        $annotations[$annotation] = $matches[2][$key];
@@ -360,7 +368,9 @@ public static function getTestInfo($classname, $doc_comment = NULL) {
      throw new MissingGroupException(sprintf('Missing @group annotation in %s', $classname));
    }
    $info['group'] = $annotations['group'];
-    // Put PHPUnit test suites into their own custom groups.
+    $info['groups'] = $annotations['groups'];
+    // Sort out PHPUnit-runnable tests by type.
    if ($testsuite = static::getPhpunitTestSuite($classname)) {
      $info['type'] = 'PHPUnit-' . $testsuite;
    }

--- a/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php
+++ b/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php
@@ -15,10 +15,6 @@
 /**
 * @coversDefaultClass \Drupal\simpletest\TestDiscovery
 * @group simpletest
- *
- * Since TestDiscovery is expected to discover Simpletest-based tests, it will
- * likely trigger deprecation errors. Therefore, we add the legacy group.
- * @group legacy
 */
 class TestDiscoveryTest extends UnitTestCase {
@@ -36,13 +32,14 @@ public function infoParserProvider() {
    $tests[] = [
      // Expected result.
      [
-        'name' => 'Drupal\Tests\simpletest\Unit\TestDiscoveryTest',
+        'name' => static::class,
        'group' => 'simpletest',
+        'groups' => ['simpletest'],
        'description' => 'Tests \Drupal\simpletest\TestDiscovery.',
        'type' => 'PHPUnit-Unit',
      ],
      // Classname.
-      'Drupal\Tests\simpletest\Unit\TestDiscoveryTest',
+      static::class,
    ];
    // A core unit test.
@@ -51,6 +48,7 @@ public function infoParserProvider() {
      [
        'name' => 'Drupal\Tests\Core\DrupalTest',
        'group' => 'DrupalTest',
+        'groups' => ['DrupalTest'],
        'description' => 'Tests \Drupal.',
        'type' => 'PHPUnit-Unit',
      ],
@@ -64,6 +62,7 @@ public function infoParserProvider() {
      [
        'name' => 'Drupal\FunctionalTests\BrowserTestBaseTest',
        'group' => 'browsertestbase',
+        'groups' => ['browsertestbase'],
        'description' => 'Tests BrowserTestBase functionality.',
        'type' => 'PHPUnit-Functional',
      ],
@@ -77,6 +76,7 @@ public function infoParserProvider() {
      [
        'name' => '\Drupal\Tests\file\Kernel\FileItemValidationTest',
        'group' => 'file',
+        'groups' => ['file'],
        'description' => 'Tests that files referenced in file and image fields are always validated.',
        'type' => 'PHPUnit-Kernel',
      ],
@@ -91,6 +91,7 @@ public function infoParserProvider() {
      [
        'name' => 'Drupal\simpletest\Tests\ExampleSimpleTest',
        'group' => 'simpletest',
+        'groups' => ['simpletest'],
        'description' => 'Tests the Simpletest UI internal browser.',
        'type' => 'Simpletest',
      ],
@@ -111,6 +112,7 @@ public function infoParserProvider() {
      [
        'name' => 'Drupal\simpletest\Tests\ExampleSimpleTest',
        'group' => 'simpletest',
+        'groups' => ['simpletest'],
        'description' => 'Tests the Simpletest UI internal browser.',
        'type' => 'Simpletest',
      ],
@@ -133,6 +135,7 @@ public function infoParserProvider() {
      [
        'name' => 'Drupal\simpletest\Tests\ExampleSimpleTest',
        'group' => 'simpletest',
+        'groups' => ['simpletest'],
        'description' => 'Tests the Simpletest UI internal browser. * @',
        'type' => 'Simpletest',
      ],
@@ -153,6 +156,7 @@ public function infoParserProvider() {
      [
        'name' => 'Drupal\simpletest\Tests\ExampleSimpleTest',
        'group' => 'Test',
+        'groups' => ['Test', 'simpletest'],
        'description' => 'Tests the Simpletest UI internal browser.',
        'type' => 'Simpletest',
      ],
@@ -168,6 +172,33 @@ public function infoParserProvider() {
 ",
    ];
+    // A great number of @group annotations.
+    $tests['many-group-annotations'] = [
+      // Expected result.
+      [
+        'name' => 'Drupal\simpletest\Tests\ExampleSimpleTest',
+        'group' => 'Test',
+        'groups' => ['Test', 'simpletest', 'another', 'more', 'many', 'enough', 'whoa'],
+        'description' => 'Tests the Simpletest UI internal browser.',
+        'type' => 'Simpletest',
+      ],
+      // Classname.
+      'Drupal\simpletest\Tests\ExampleSimpleTest',
+      // Doc block.
+      "/**
+ * Tests the Simpletest UI internal browser.
+ *
+ * @group Test
+ * @group simpletest
+ * @group another
+ * @group more
+ * @group many
+ * @group enough
+ * @group whoa
+ */
+ ",
+    ];
    // @dependencies annotation.
    $tests[] = [
      // Expected result.
@@ -177,6 +208,7 @@ public function infoParserProvider() {
        'type' => 'Simpletest',
        'requires' => ['module' => ['test']],
        'group' => 'simpletest',
+        'groups' => ['simpletest'],
      ],
      // Classname.
      'Drupal\simpletest\Tests\ExampleSimpleTest',
@@ -199,6 +231,7 @@ public function infoParserProvider() {
        'type' => 'Simpletest',
        'requires' => ['module' => ['test', 'test1', 'test2']],
        'group' => 'simpletest',
+        'groups' => ['simpletest'],
      ],
      // Classname.
      'Drupal\simpletest\Tests\ExampleSimpleTest',
@@ -220,6 +253,7 @@ public function infoParserProvider() {
        'description' => 'Tests the Simpletest UI internal browser. And the summary line continues an there is no gap to the annotation.',
        'type' => 'Simpletest',
        'group' => 'simpletest',
+        'groups' => ['simpletest'],
      ],
      // Classname.
      'Drupal\simpletest\Tests\ExampleSimpleTest',
@@ -298,7 +332,7 @@ class FunctionalExampleTest {}
                'FunctionalExampleTest2.php' => str_replace(['FunctionalExampleTest', '@group example'], ['FunctionalExampleTest2', '@group example2'], $test_file),
              ],
              'Kernel' => [
-                'KernelExampleTest3.php' => str_replace(['FunctionalExampleTest', '@group example'], ['KernelExampleTest3', '@group example2'], $test_file),
+                'KernelExampleTest3.php' => str_replace(['FunctionalExampleTest', '@group example'], ['KernelExampleTest3', "@group example2\n * @group kernel\n"], $test_file),
                'KernelExampleTestBase.php' => str_replace(['FunctionalExampleTest', '@group example'], ['KernelExampleTestBase', '@group example2'], $test_file),
                'KernelExampleTrait.php' => str_replace(['FunctionalExampleTest', '@group example'], ['KernelExampleTrait', '@group example2'], $test_file),
                'KernelExampleInterface.php' => str_replace(['FunctionalExampleTest', '@group example'], ['KernelExampleInterface', '@group example2'], $test_file),
@@ -342,13 +376,14 @@ public function testGetTestClasses() {
    ];
    $test_discovery->setExtensions($extensions);
    $result = $test_discovery->getTestClasses();
-    $this->assertCount(2, $result);
+    $this->assertCount(3, $result);
    $this->assertEquals([
      'example' => [
        'Drupal\Tests\test_module\Functional\FunctionalExampleTest' => [
          'name' => 'Drupal\Tests\test_module\Functional\FunctionalExampleTest',
          'description' => 'Test description',
          'group' => 'example',
+          'groups' => ['example'],
          'type' => 'PHPUnit-Functional',
        ],
      ],
@@ -357,12 +392,23 @@ public function testGetTestClasses() {
          'name' => 'Drupal\Tests\test_module\Functional\FunctionalExampleTest2',
          'description' => 'Test description',
          'group' => 'example2',
+          'groups' => ['example2'],
          'type' => 'PHPUnit-Functional',
        ],
        'Drupal\Tests\test_module\Kernel\KernelExampleTest3' => [
          'name' => 'Drupal\Tests\test_module\Kernel\KernelExampleTest3',
          'description' => 'Test description',
          'group' => 'example2',
+          'groups' => ['example2', 'kernel'],
+          'type' => 'PHPUnit-Kernel',
+        ],
+      ],
+      'kernel' => [
+        'Drupal\Tests\test_module\Kernel\KernelExampleTest3' => [
+          'name' => 'Drupal\Tests\test_module\Kernel\KernelExampleTest3',
+          'description' => 'Test description',
+          'group' => 'example2',
+          'groups' => ['example2', 'kernel'],
          'type' => 'PHPUnit-Kernel',
        ],
      ],
@@ -385,7 +431,7 @@ public function testGetTestClassesWithSelectedTypes() {
    ];
    $test_discovery->setExtensions($extensions);
    $result = $test_discovery->getTestClasses(NULL, ['PHPUnit-Kernel']);
-    $this->assertCount(3, $result);
+    $this->assertCount(4, $result);
    $this->assertEquals([
      'example' => [],
      'example2' => [
@@ -393,6 +439,16 @@ public function testGetTestClassesWithSelectedTypes() {
          'name' => 'Drupal\Tests\test_module\Kernel\KernelExampleTest3',
          'description' => 'Test description',
          'group' => 'example2',
+          'groups' => ['example2', 'kernel'],
+          'type' => 'PHPUnit-Kernel',
+        ],
+      ],
+      'kernel' => [
+        'Drupal\Tests\test_module\Kernel\KernelExampleTest3' => [
+          'name' => 'Drupal\Tests\test_module\Kernel\KernelExampleTest3',
+          'description' => 'Test description',
+          'group' => 'example2',
+          'groups' => ['example2', 'kernel'],
          'type' => 'PHPUnit-Kernel',
        ],
      ],
@@ -401,6 +457,7 @@ public function testGetTestClassesWithSelectedTypes() {
          'name' => 'Drupal\Tests\test_profile_module\Kernel\KernelExampleTest4',
          'description' => 'Test description',
          'group' => 'example3',
+          'groups' => ['example3'],
          'type' => 'PHPUnit-Kernel',
        ],
      ],
@@ -429,6 +486,7 @@ public function testGetTestsInProfiles() {
          'name' => 'Drupal\Tests\test_profile_module\Kernel\KernelExampleTest4',
          'description' => 'Test description',
          'group' => 'example3',
+          'groups' => ['example3'],
          'type' => 'PHPUnit-Kernel',
        ],
      ],
@@ -512,15 +570,3 @@ protected function getExtensions() {
  }
 }
-namespace Drupal\simpletest\Tests;
-use Drupal\simpletest\WebTestBase;
-/**
- * Tests the Simpletest UI internal browser.
- *
- * @group simpletest
- */
-class ExampleSimpleTest extends WebTestBase {
-}
--- a/core/scripts/run-tests.sh
+++ b/core/scripts/run-tests.sh
@@ -62,7 +62,7 @@
 }
 if ($args['list']) {
-  // Display all available tests.
+  // Display all available tests organized by one @group annotation.
  echo "\nAvailable test groups & classes\n";
  echo "-------------------------------\n\n";
  try {
@@ -73,11 +73,18 @@
    echo (string) $e;
    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
  }
+  // A given class can appear in multiple groups. For historical reasons, we
+  // need to present each test only once. The test is shown in the group that is
+  // printed first.
+  $printed_tests = [];
  foreach ($groups as $group => $tests) {
    echo $group . "\n";
-    foreach ($tests as $class => $info) {
+    $tests = array_diff(array_keys($tests), $printed_tests);
-      echo " - $class\n";
+    foreach ($tests as $test) {
+      echo " - $test\n";
    }
+    $printed_tests = array_merge($printed_tests, $tests);
  }
  exit(SIMPLETEST_SCRIPT_EXIT_SUCCESS);
 }
@@ -1146,16 +1153,20 @@ function simpletest_script_get_test_list() {
        echo (string) $e;
        exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
      }
+      // Store all the groups so we can suggest alternatives if we need to.
+      $all_groups = array_keys($groups);
+      // Verify that the groups exist.
+      if (!empty($unknown_groups = array_diff($args['test_names'], $all_groups))) {
+        $first_group = reset($unknown_groups);
+        simpletest_script_print_error('Test group not found: ' . $first_group);
+        simpletest_script_print_alternatives($first_group, $all_groups);
+        exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
+      }
+      // Ensure our list of tests contains only one entry for each test.
      foreach ($args['test_names'] as $group_name) {
-        if (isset($groups[$group_name])) {
+        $test_list = array_merge($test_list, array_flip(array_keys($groups[$group_name])));
-          $test_list = array_merge($test_list, array_keys($groups[$group_name]));
-        }
-        else {
-          simpletest_script_print_error('Test group not found: ' . $group_name);
-          simpletest_script_print_alternatives($group_name, array_keys($groups));
-          exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
-        }
      }
+      $test_list = array_flip($test_list);
    }
  }