From 2af907ca36fe25ea19fbf9c5f74a9726aef009d6 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Mon, 11 Mar 2019 17:02:19 +0000 Subject: [PATCH] Issue #2296615 by Mile23, chr.fritsch, jibran, borisson_, dawehner, alexpott, andypost: Accurately support multiple @groups per test class --- core/modules/simpletest/src/TestDiscovery.php | 28 ++++-- .../tests/src/Unit/TestDiscoveryTest.php | 88 ++++++++++++++----- core/scripts/run-tests.sh | 35 +++++--- 3 files changed, 109 insertions(+), 42 deletions(-) mode change 100644 => 100755 core/scripts/run-tests.sh diff --git a/core/modules/simpletest/src/TestDiscovery.php b/core/modules/simpletest/src/TestDiscovery.php index 92c12fe6096..671048d702f 100644 --- 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 @@ class TestDiscovery { * 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 @@ class TestDiscovery { * @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 @@ class TestDiscovery { } } - $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 @@ class TestDiscovery { * - 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 @@ class TestDiscovery { 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 - // multiple @group annotations. + // Only @group is allowed to have more than one annotation, in the + // 'groups' key. Other annotations only have one value per key. continue; } $annotations[$annotation] = $matches[2][$key]; @@ -360,7 +368,9 @@ class TestDiscovery { 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; } diff --git a/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php b/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php index 0d2843e97aa..fa3a93bf633 100644 --- a/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php +++ b/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php @@ -15,10 +15,6 @@ use org\bovigo\vfs\vfsStream; /** * @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 @@ class TestDiscoveryTest extends UnitTestCase { $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 @@ class TestDiscoveryTest extends UnitTestCase { [ 'name' => 'Drupal\Tests\Core\DrupalTest', 'group' => 'DrupalTest', + 'groups' => ['DrupalTest'], 'description' => 'Tests \Drupal.', 'type' => 'PHPUnit-Unit', ], @@ -64,6 +62,7 @@ class TestDiscoveryTest extends UnitTestCase { [ 'name' => 'Drupal\FunctionalTests\BrowserTestBaseTest', 'group' => 'browsertestbase', + 'groups' => ['browsertestbase'], 'description' => 'Tests BrowserTestBase functionality.', 'type' => 'PHPUnit-Functional', ], @@ -77,6 +76,7 @@ class TestDiscoveryTest extends UnitTestCase { [ '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 @@ class TestDiscoveryTest extends UnitTestCase { [ 'name' => 'Drupal\simpletest\Tests\ExampleSimpleTest', 'group' => 'simpletest', + 'groups' => ['simpletest'], 'description' => 'Tests the Simpletest UI internal browser.', 'type' => 'Simpletest', ], @@ -111,6 +112,7 @@ class TestDiscoveryTest extends UnitTestCase { [ 'name' => 'Drupal\simpletest\Tests\ExampleSimpleTest', 'group' => 'simpletest', + 'groups' => ['simpletest'], 'description' => 'Tests the Simpletest UI internal browser.', 'type' => 'Simpletest', ], @@ -133,6 +135,7 @@ class TestDiscoveryTest extends UnitTestCase { [ 'name' => 'Drupal\simpletest\Tests\ExampleSimpleTest', 'group' => 'simpletest', + 'groups' => ['simpletest'], 'description' => 'Tests the Simpletest UI internal browser. * @', 'type' => 'Simpletest', ], @@ -153,6 +156,7 @@ class TestDiscoveryTest extends UnitTestCase { [ 'name' => 'Drupal\simpletest\Tests\ExampleSimpleTest', 'group' => 'Test', + 'groups' => ['Test', 'simpletest'], 'description' => 'Tests the Simpletest UI internal browser.', 'type' => 'Simpletest', ], @@ -168,6 +172,33 @@ class TestDiscoveryTest extends UnitTestCase { ", ]; + // 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 @@ class TestDiscoveryTest extends UnitTestCase { 'type' => 'Simpletest', 'requires' => ['module' => ['test']], 'group' => 'simpletest', + 'groups' => ['simpletest'], ], // Classname. 'Drupal\simpletest\Tests\ExampleSimpleTest', @@ -199,6 +231,7 @@ class TestDiscoveryTest extends UnitTestCase { 'type' => 'Simpletest', 'requires' => ['module' => ['test', 'test1', 'test2']], 'group' => 'simpletest', + 'groups' => ['simpletest'], ], // Classname. 'Drupal\simpletest\Tests\ExampleSimpleTest', @@ -220,6 +253,7 @@ class TestDiscoveryTest extends UnitTestCase { '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 @@ EOF; '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 @@ EOF; ]; $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 @@ EOF; '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 @@ EOF; ]; $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 @@ EOF; '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 @@ EOF; 'name' => 'Drupal\Tests\test_profile_module\Kernel\KernelExampleTest4', 'description' => 'Test description', 'group' => 'example3', + 'groups' => ['example3'], 'type' => 'PHPUnit-Kernel', ], ], @@ -429,6 +486,7 @@ EOF; 'name' => 'Drupal\Tests\test_profile_module\Kernel\KernelExampleTest4', 'description' => 'Test description', 'group' => 'example3', + 'groups' => ['example3'], 'type' => 'PHPUnit-Kernel', ], ], @@ -512,15 +570,3 @@ class TestTestDiscovery extends TestDiscovery { } } - -namespace Drupal\simpletest\Tests; - -use Drupal\simpletest\WebTestBase; - -/** - * Tests the Simpletest UI internal browser. - * - * @group simpletest - */ -class ExampleSimpleTest extends WebTestBase { -} diff --git a/core/scripts/run-tests.sh b/core/scripts/run-tests.sh old mode 100644 new mode 100755 index 008ec309938..36f6c471709 --- a/core/scripts/run-tests.sh +++ b/core/scripts/run-tests.sh @@ -62,7 +62,7 @@ if ($args['execute-test']) { } 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 @@ if ($args['list']) { 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) { - echo " - $class\n"; + $tests = array_diff(array_keys($tests), $printed_tests); + 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); } - foreach ($args['test_names'] as $group_name) { - if (isset($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); - } + // 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) { + $test_list = array_merge($test_list, array_flip(array_keys($groups[$group_name]))); + } + $test_list = array_flip($test_list); } }