Use ExtensionDiscovery to register test namespaces for running PHPUnit

1. +++ b/core/lib/Drupal/Core/EventSubscriber/SpecialAttributesRouteSubscriber.php
   @@ -34,12 +34,10 @@ protected function alterRoutes(RouteCollection $collection) {
   -        $placeholders = array('@variables' => implode(', ', $not_allowed_variables));
   -        drupal_set_message(String::format('The following variables are reserved names by drupal: @variables', $placeholders));
   -        watchdog('error', 'The following variables are reserved names by drupal: @variables', $placeholders);
   -        return FALSE;
   +        $reserved = implode(', ', $not_allowed_variables);
   +        throw new \LogicException(sprintf('Route %s uses reserved variable names: %s', $name, $reserved));
   

   not sure route name is passed...

   also '@' check_plain() is removed, are you sure that all data secure?

2. +++ b/core/lib/Drupal/Core/Extension/ExtensionDiscovery.php
   @@ -136,7 +136,14 @@ public function scan($type, $include_tests = NULL) {
   +    if (function_exists('conf_path')) {
   ...
   +    else {
   +      $searchdirs[static::ORIGIN_SITE] = 'sites/default';
   

   why this is used?

+++ b/core/tests/bootstrap.php
@@ -2,88 +2,24 @@
+// As long as no further classes are needed, a single include is sufficient.
+require_once __DIR__ . '/../modules/simpletest/src/TestDiscovery.php';

Maybe not as part of this issue, but this hints to me that that class should be in \Drupal\Core.

Patch itself looks good.

An OOP solution would be better of course. A solution that all systems could leverage would be ideal.

+++ b/core/lib/Drupal/Core/Extension/ExtensionDiscovery.php
@@ -136,7 +136,14 @@ public function scan($type, $include_tests = NULL) {
+      $searchdirs[static::ORIGIN_SITE] = conf_path();

You mean DrupalKernel::getSitePath() of course. :-)

That is, soon this bootstrap script will require a kernel object with a container.

Which is bad.

+require_once __DIR__ . '/../modules/simpletest/src/TestDiscovery.php';
Maybe not as part of this issue, but this hints to me that that class should be in \Drupal\Core.

Really \Drupal\Tests, which means SimpleTest can't get it. :-) That's one reason this script is lean and mean and functional.

So Drupal\Core\Extension\ExtensionDiscovery has reduced its dependencies, and you can (almost) inject the site path (see: #2605654: Modify Drupal\Core\Extension\ExtensionDiscovery to allow for scanning multiple file systems, enabling vfsStream testing).

Here are two patches:

The small one is a POC and changes core/tests/bootstrap.php to use ExtensionDiscovery. It will fail, because core/includes/bootstrap.inc is a dependency of ExtensionDiscovery, and many tests make stub definitions of functions declared in bootstrap.inc, so there will be errors. Also there are failures related to the issue linked above.

The large one changes many of these tests and their code in order to make some of them pass. This is a WIP. It touches a lot of code, so it might not be RC-able, but it should be. It includes the changes in #2605654: Modify Drupal\Core\Extension\ExtensionDiscovery to allow for scanning multiple file systems, enabling vfsStream testing patch #2, so a re-roll might be necessary at some point.

In my opinion this issue is a bug report, because we had false passes on a number of tests before making this change in dependencies. The best example of this is UpdateRegistryTest which uses vfsStream to mock a whole Drupal filesystem. If you run the test in isolation, ExtensionDiscovery only runs once, which hides its static problems.

After #2499239: Use test suite classes to discover different test types under phpunit, allow contrib harmony with run-tests I'm calling this CWF, since the new behavior for the phpunit tool is to use TestDiscovery::scanDirectory() to find tests per testsuite. (This issue was originally about using TestDiscovery in bootstrap.php.)

bootstrap.php still uses its own functions to discover extensions, which is fine because we want to be able to test ExtensionDiscovery in isolation.

As always: Re-open if you disagree.