diff --git a/core/modules/simpletest/src/TestDiscovery.php b/core/modules/simpletest/src/TestDiscovery.php index d5d7d8a576..f6ac67cd53 100644 --- a/core/modules/simpletest/src/TestDiscovery.php +++ b/core/modules/simpletest/src/TestDiscovery.php @@ -189,14 +189,17 @@ public function getTestClasses($extension = NULL, array $types = []) { // abstract class, trait or test fixture. continue; } - // 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 - // that approach. + // Skip this test class if it is a Simpletest-based test and requires + // unavailable modules. TestDiscovery should not filter out module + // requirements for PHPUnit-based test classes. + // @todo Move this behavior to \Drupal\simpletest\TestBase so tests can be + // marked as skipped, instead. // @see https://www.drupal.org/node/1273478 - if (!empty($info['requires']['module'])) { - if (array_diff($info['requires']['module'], $this->availableExtensions['module'])) { - continue; + if ($info['type'] == 'Simpletest') { + if (!empty($info['requires']['module'])) { + if (array_diff($info['requires']['module'], $this->availableExtensions['module'])) { + continue; + } } } diff --git a/core/modules/simpletest/src/Tests/SkipRequiredModulesTest.php b/core/modules/simpletest/src/Tests/SkipRequiredModulesTest.php new file mode 100644 index 0000000000..15469433df --- /dev/null +++ b/core/modules/simpletest/src/Tests/SkipRequiredModulesTest.php @@ -0,0 +1,28 @@ +fail('This test should have been skipped during discovery.'); + } + +} diff --git a/core/modules/simpletest/tests/src/Functional/MissingDependentModuleUnitTest.php b/core/modules/simpletest/tests/src/Functional/MissingDependentModuleUnitTest.php deleted file mode 100644 index acd55dd014..0000000000 --- a/core/modules/simpletest/tests/src/Functional/MissingDependentModuleUnitTest.php +++ /dev/null @@ -1,22 +0,0 @@ -fail('Running test with missing required module.'); - } - -} diff --git a/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php b/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php index df98377d15..abdeabe7ff 100644 --- a/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php +++ b/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php @@ -62,7 +62,7 @@ public function infoParserProvider() { [ 'name' => 'Drupal\FunctionalTests\BrowserTestBaseTest', 'group' => 'browsertestbase', - 'description' => 'Tests BrowserTestBase functionality.', + 'description' => 'Tests \Drupal\Tests\BrowserTestBase.', 'type' => 'PHPUnit-Functional', ], // Classname. diff --git a/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php index f73084f6f4..cd2b922e9e 100644 --- a/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php @@ -11,6 +11,7 @@ /** * Tests BrowserTestBase functionality. * + * @coversDefaultClass \Drupal\Tests\BrowserTestBase * @group browsertestbase */ class BrowserTestBaseTest extends BrowserTestBase { @@ -232,4 +233,48 @@ public function testInstall() { $this->assertTrue(file_exists($htaccess_filename), "$htaccess_filename exists"); } + /** + * Tests that a test method is skipped when it requires a module not present. + * + * In order to catch checkRequirements() regressions, we have to make a new + * test object and run checkRequirements() here. + * + * @covers ::checkRequirements + * @covers ::checkModuleRequirements + */ + public function testMethodRequiresModule() { + require __DIR__ . '/../../fixtures/BrowserMissingDependentModuleMethodTest.php'; + + $test = new BrowserMissingDependentModuleMethodTest(); + // We have to setName() to the method name we're concerned with. + $test->setName('testRequiresModule'); + $this->setExpectedException( + \PHPUnit_Framework_SkippedTestError::class, + 'Module drupal_dne_module is required.' + ); + $test->checkRequirements(); + } + + /** + * Tests that a test case is skipped when it requires a module not present. + * + * In order to catch checkRequirements() regressions, we have to make a new + * test object and run checkRequirements() here. + * + * @covers ::checkRequirements + * @covers ::checkModuleRequirements + */ + public function testRequiresModule() { + require __DIR__ . '/../../fixtures/BrowserMissingDependentModuleTest.php'; + + $test = new BrowserMissingDependentModuleTest(); + // We have to setName() to the method name we're concerned with. + $test->setName('testRequiresModule'); + $this->setExpectedException( + \PHPUnit_Framework_SkippedTestError::class, + 'Module drupal_dne_module is required.' + ); + $test->checkRequirements(); + } + } diff --git a/core/tests/Drupal/KernelTests/KernelTestBase.php b/core/tests/Drupal/KernelTests/KernelTestBase.php index 5621d4aa19..22e35575f0 100644 --- a/core/tests/Drupal/KernelTests/KernelTestBase.php +++ b/core/tests/Drupal/KernelTests/KernelTestBase.php @@ -21,6 +21,7 @@ use Drupal\simpletest\AssertHelperTrait; use Drupal\Tests\ConfigTestTrait; use Drupal\Tests\RandomGeneratorTrait; +use Drupal\Tests\TestRequirementsTrait; use Drupal\simpletest\TestServiceProvider; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\HttpFoundation\Request; @@ -45,9 +46,6 @@ * * @see \Drupal\Tests\KernelTestBase::$modules * @see \Drupal\Tests\KernelTestBase::enableModules() - * - * @todo Extend ::setRequirementsFromAnnotation() and ::checkRequirements() to - * account for '@requires module'. */ abstract class KernelTestBase extends \PHPUnit_Framework_TestCase implements ServiceProviderInterface { @@ -56,6 +54,7 @@ use AssertHelperTrait; use RandomGeneratorTrait; use ConfigTestTrait; + use TestRequirementsTrait; /** * {@inheritdoc} @@ -215,15 +214,6 @@ public static function setUpBeforeClass() { } /** - * Returns the drupal root directory. - * - * @return string - */ - protected static function getDrupalRoot() { - return dirname(dirname(substr(__DIR__, 0, -strlen(__NAMESPACE__)))); - } - - /** * {@inheritdoc} */ protected function setUp() { diff --git a/core/tests/Drupal/KernelTests/KernelTestBaseTest.php b/core/tests/Drupal/KernelTests/KernelTestBaseTest.php index 7d89fd7bd8..6c7fe8adab 100644 --- a/core/tests/Drupal/KernelTests/KernelTestBaseTest.php +++ b/core/tests/Drupal/KernelTests/KernelTestBaseTest.php @@ -206,6 +206,50 @@ public function testRenderWithTheme() { } /** + * Tests that a test method is skipped when it requires a module not present. + * + * In order to catch checkRequirements() regressions, we have to make a new + * test object and run checkRequirements() here. + * + * @covers ::checkRequirements + * @covers ::checkModuleRequirements + */ + public function testMethodRequiresModule() { + require __DIR__ . '/../../fixtures/KernelMissingDependentModuleMethodTest.php'; + + $test = new KernelMissingDependentModuleMethodTest(); + // We have to setName() to the method name we're concerned with. + $test->setName('testRequiresModule'); + $this->setExpectedException( + \PHPUnit_Framework_SkippedTestError::class, + 'Module drupal_dne_module is required.' + ); + $test->checkRequirements(); + } + + /** + * Tests that a test case is skipped when it requires a module not present. + * + * In order to catch checkRequirements() regressions, we have to make a new + * test object and run checkRequirements() here. + * + * @covers ::checkRequirements + * @covers ::checkModuleRequirements + */ + public function testRequiresModule() { + require __DIR__ . '/../../fixtures/KernelMissingDependentModuleTest.php'; + + $test = new KernelMissingDependentModuleTest(); + // We have to setName() to the method name we're concerned with. + $test->setName('testRequiresModule'); + $this->setExpectedException( + \PHPUnit_Framework_SkippedTestError::class, + 'Module drupal_dne_module is required.' + ); + $test->checkRequirements(); + } + + /** * {@inheritdoc} */ protected function tearDown() { diff --git a/core/tests/Drupal/Tests/BrowserTestBase.php b/core/tests/Drupal/Tests/BrowserTestBase.php index 754f9d9838..e6498d9d0e 100644 --- a/core/tests/Drupal/Tests/BrowserTestBase.php +++ b/core/tests/Drupal/Tests/BrowserTestBase.php @@ -60,6 +60,7 @@ createContentType as drupalCreateContentType; } use ConfigTestTrait; + use TestRequirementsTrait; use UserCreationTrait { createRole as drupalCreateRole; createUser as drupalCreateUser; diff --git a/core/tests/Drupal/Tests/TestRequirementsTrait.php b/core/tests/Drupal/Tests/TestRequirementsTrait.php new file mode 100644 index 0000000000..43a529e5c6 --- /dev/null +++ b/core/tests/Drupal/Tests/TestRequirementsTrait.php @@ -0,0 +1,85 @@ +getAnnotations(); + if (!empty($annotations['class']['requires'])) { + $this->checkModuleRequirements($root, $annotations['class']['requires']); + } + if (!empty($annotations['method']['requires'])) { + $this->checkModuleRequirements($root, $annotations['method']['requires']); + } + } + + /** + * Checks missing module requirements. + * + * Iterates through a list of requires annotations and looks for missing + * modules. The test will be skipped if any of the required modules is + * missing. + * + * @param string $root + * The path to the root of the Drupal installation to scan. + * @param string[] $annotations + * A list of requires annotations from either a method or class annotation. + * + * @throws \PHPUnit_Framework_SkippedTestError + * Thrown when the requirements are not met, and this test should be + * skipped. Callers should not catch this exception. + */ + private function checkModuleRequirements($root, array $annotations) { + // drupal_valid_ua() might not be loaded. + require_once $root . '/core/includes/bootstrap.inc'; + // Scan for modules. + $discovery = new ExtensionDiscovery($root, FALSE); + $discovery->setProfileDirectories([]); + $list = $discovery->scan('module'); + // Find requirements in the list of modules. + foreach ($annotations as $requirement) { + if (strpos($requirement, 'module ') === 0) { + $module = trim(str_replace('module ', '', $requirement)); + if (!isset($list[$module])) { + throw new \PHPUnit_Framework_SkippedTestError("Module $module is required."); + } + } + } + } + +} diff --git a/core/tests/fixtures/BrowserMissingDependentModuleMethodTest.php b/core/tests/fixtures/BrowserMissingDependentModuleMethodTest.php new file mode 100644 index 0000000000..5ce348baf3 --- /dev/null +++ b/core/tests/fixtures/BrowserMissingDependentModuleMethodTest.php @@ -0,0 +1,28 @@ +fail('Running test with missing required module.'); + } + +} diff --git a/core/tests/fixtures/BrowserMissingDependentModuleTest.php b/core/tests/fixtures/BrowserMissingDependentModuleTest.php new file mode 100644 index 0000000000..443a93deda --- /dev/null +++ b/core/tests/fixtures/BrowserMissingDependentModuleTest.php @@ -0,0 +1,30 @@ +fail('Running test with missing required module.'); + } + +} diff --git a/core/tests/fixtures/KernelMissingDependentModuleMethodTest.php b/core/tests/fixtures/KernelMissingDependentModuleMethodTest.php new file mode 100644 index 0000000000..ab2b4d8956 --- /dev/null +++ b/core/tests/fixtures/KernelMissingDependentModuleMethodTest.php @@ -0,0 +1,26 @@ +fail('Running test with missing required module.'); + } + +} diff --git a/core/tests/fixtures/KernelMissingDependentModuleTest.php b/core/tests/fixtures/KernelMissingDependentModuleTest.php new file mode 100644 index 0000000000..2ba5a5aad8 --- /dev/null +++ b/core/tests/fixtures/KernelMissingDependentModuleTest.php @@ -0,0 +1,28 @@ +fail('Running test with missing required module.'); + } + +}