Issue #2928249 by alexpott, mondrake: Introduce a PHPUnit 6+ compatibility...

Issue #2928249 by alexpott, mondrake: Introduce a PHPUnit 6+ compatibility layer for Drupal\Tests\Listeners classes

core/modules/simpletest/src/WebTestBase.php

@@ -18,7 +18,7 @@
 use Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait;
 use Drupal\Tests\EntityViewTrait;
 use Drupal\Tests\block\Traits\BlockCreationTrait as BaseBlockCreationTrait;
-use Drupal\Tests\Listeners\DeprecationListener;
+use Drupal\Tests\Listeners\DeprecationListenerTrait;
 use Drupal\Tests\node\Traits\ContentTypeCreationTrait;
 use Drupal\Tests\node\Traits\NodeCreationTrait;
 use Drupal\Tests\Traits\Core\CronRunTrait;
@@ -698,7 +698,7 @@ protected function curlHeaderCallback($curlHandler, $header) {
       if ($parameters[1] === 'User deprecated function') {
         if (getenv('SYMFONY_DEPRECATIONS_HELPER') !== 'disabled') {
           $message = (string) $parameters[0];
-          if (!in_array($message, DeprecationListener::getSkippedDeprecations())) {
+          if (!in_array($message, DeprecationListenerTrait::getSkippedDeprecations())) {
             call_user_func_array([&$this, 'error'], $parameters);
           }
         }

core/phpunit.xml.dist

@@ -49,16 +49,11 @@
     </testsuite>
   </testsuites>
   <listeners>
-    <listener class="\Drupal\Tests\Listeners\DeprecationListener">
+    <listener class="\Drupal\Tests\Listeners\DrupalListener">
     </listener>
-    <!-- The Symfony deprecation listener has to come after the Drupal
-    deprecation listener -->
+    <!-- The Symfony deprecation listener has to come after the Drupal listener -->
     <listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener">
     </listener>
-    <listener class="\Drupal\Tests\Listeners\DrupalStandardsListener">
-    </listener>
-    <listener class="\Drupal\Tests\Listeners\DrupalComponentTestListener">
-    </listener>
   </listeners>
   <!-- Filter for coverage reports. -->
   <filter>

core/tests/Drupal/Tests/Listeners/DeprecationListener.php → core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php

@@ -9,12 +9,18 @@
  *   This class will be removed once all the deprecation notices have been
  *   fixed.
  */
-class DeprecationListener extends \PHPUnit_Framework_BaseTestListener {
+trait DeprecationListenerTrait {
 
   /**
-   * {@inheritdoc}
+   * Reacts to the end of a test.
+   *
+   * @param \PHPUnit\Framework\Test|\PHPUnit_Framework_Test $test
+   *   The test object that has ended its test run.
+   * @param float $time
+   *   The time the test took.
    */
-  public function endTest(\PHPUnit_Framework_Test $test, $time) {
+  protected function deprecationEndTest($test, $time) {
+    /** @var \PHPUnit\Framework\Test $test */
     // Need to edit the file of deprecations.
     if ($file = getenv('SYMFONY_DEPRECATIONS_SERIALIZE')) {
       $deprecations = file_get_contents($file);

core/tests/Drupal/Tests/Listeners/DrupalComponentTestListener.php → core/tests/Drupal/Tests/Listeners/DrupalComponentTestListenerTrait.php

@@ -5,20 +5,28 @@
 use Drupal\KernelTests\KernelTestBase;;
 use Drupal\Tests\BrowserTestBase;;
 use Drupal\Tests\UnitTestCase;
-use PHPUnit\Framework\BaseTestListener;
+use PHPUnit\Framework\AssertionFailedError;
 
 /**
  * Ensures that no component tests are extending a core test base class.
+ *
+ * @internal
  */
-class DrupalComponentTestListener extends BaseTestListener {
+trait DrupalComponentTestListenerTrait {
 
   /**
-   * {@inheritdoc}
+   * Reacts to the end of a test.
+   *
+   * @param \PHPUnit\Framework\Test|\PHPUnit_Framework_Test $test
+   *   The test object that has ended its test run.
+   * @param float $time
+   *   The time the test took.
    */
-  public function endTest(\PHPUnit_Framework_Test $test, $time) {
+  protected function componentEndTest($test, $time) {
+    /** @var \PHPUnit\Framework\Test $test */
     if (substr($test->toString(), 0, 22) == 'Drupal\Tests\Component') {
       if ($test instanceof BrowserTestBase || $test instanceof KernelTestBase || $test instanceof UnitTestCase) {
-        $error = new \PHPUnit_Framework_AssertionFailedError('Component tests should not extend a core test base class.');
+        $error = new AssertionFailedError('Component tests should not extend a core test base class.');
         $test->getTestResultObject()->addFailure($test, $error, $time);
       }
     }

core/tests/Drupal/Tests/Listeners/DrupalListener.php

+<?php
+
+namespace Drupal\Tests\Listeners;
+
+use PHPUnit\Framework\BaseTestListener;
+use PHPUnit\Framework\Test;
+
+if (class_exists('PHPUnit_Runner_Version') && version_compare(\PHPUnit_Runner_Version::id(), '6.0.0', '<')) {
+  class_alias('Drupal\Tests\Listeners\Legacy\DrupalListener', 'Drupal\Tests\Listeners\DrupalListener');
+  // Using an early return instead of a else does not work when using the
+  // PHPUnit phar due to some weird PHP behavior (the class gets defined without
+  // executing the code before it and so the definition is not properly
+  // conditional).
+}
+else {
+  /**
+   * Listens to PHPUnit test runs.
+   *
+   * @internal
+   */
+  class DrupalListener extends BaseTestListener {
+    use DeprecationListenerTrait;
+    use DrupalComponentTestListenerTrait;
+    use DrupalStandardsListenerTrait;
+
+    /**
+     * {@inheritdoc}
+     */
+    public function endTest(Test $test, $time) {
+      $this->deprecationEndTest($test, $time);
+      $this->componentEndTest($test, $time);
+      $this->standardsEndTest($test, $time);
+    }
+
+  }
+}

core/tests/Drupal/Tests/Listeners/DrupalStandardsListener.php → core/tests/Drupal/Tests/Listeners/DrupalStandardsListenerTrait.php

@@ -2,15 +2,18 @@
 
 namespace Drupal\Tests\Listeners;
 
-use PHPUnit\Framework\BaseTestListener;
+use PHPUnit\Framework\AssertionFailedError;
 use PHPUnit\Framework\TestCase;
+use PHPUnit\Framework\TestSuite;
 
 /**
  * Listens for PHPUnit tests and fails those with invalid coverage annotations.
  *
  * Enforces various coding standards within test runs.
+ *
+ * @internal
  */
-class DrupalStandardsListener extends BaseTestListener {
+trait DrupalStandardsListenerTrait {
 
   /**
    * Signals a coding standards failure to the user.
@@ -21,10 +24,10 @@ class DrupalStandardsListener extends BaseTestListener {
    *   The message to add to the failure notice. The test class name and test
    *   name will be appended to this message automatically.
    */
-  protected function fail(TestCase $test, $message) {
+  private function fail(TestCase $test, $message) {
     // Add the report to the test's results.
     $message .= ': ' . get_class($test) . '::' . $test->getName();
-    $fail = new \PHPUnit_Framework_AssertionFailedError($message);
+    $fail = new AssertionFailedError($message);
     $result = $test->getTestResultObject();
     $result->addFailure($test, $fail, 0);
   }
@@ -38,7 +41,7 @@ protected function fail(TestCase $test, $message) {
    * @return bool
    *   TRUE if the class exists, FALSE otherwise.
    */
-  protected function classExists($class) {
+  private function classExists($class) {
     return class_exists($class, TRUE) || trait_exists($class, TRUE) || interface_exists($class, TRUE);
   }
 
@@ -50,7 +53,7 @@ protected function classExists($class) {
    * @param \PHPUnit\Framework\TestCase $test
    *   The test to examine.
    */
-  public function checkValidCoversForTest(TestCase $test) {
+  private function checkValidCoversForTest(TestCase $test) {
     // If we're generating a coverage report already, don't do anything here.
     if ($test->getTestResultObject() && $test->getTestResultObject()->getCollectCodeCoverageInformation()) {
       return;
@@ -141,7 +144,7 @@ public function checkValidCoversForTest(TestCase $test) {
   }
 
   /**
-   * {@inheritdoc}
+   * Reacts to the end of a test.
    *
    * We must mark this method as belonging to the special legacy group because
    * it might trigger an E_USER_DEPRECATED error during coverage annotation
@@ -151,22 +154,58 @@ public function checkValidCoversForTest(TestCase $test) {
    *
    * @group legacy
    *
+   * @param \PHPUnit\Framework\Test|\PHPUnit_Framework_Test $test
+   *   The test object that has ended its test run.
+   * @param float $time
+   *   The time the test took.
+   *
    * @see http://symfony.com/doc/current/components/phpunit_bridge.html#mark-tests-as-legacy
    */
-  public function endTest(\PHPUnit_Framework_Test $test, $time) {
+  private function doEndTest($test, $time) {
     // \PHPUnit_Framework_Test does not have any useful methods of its own for
     // our purpose, so we have to distinguish between the different known
     // subclasses.
     if ($test instanceof TestCase) {
       $this->checkValidCoversForTest($test);
     }
-    elseif ($test instanceof \PHPUnit_Framework_TestSuite) {
+    elseif ($this->isTestSuite($test)) {
       foreach ($test->getGroupDetails() as $tests) {
         foreach ($tests as $test) {
-          $this->endTest($test, $time);
+          $this->doEndTest($test, $time);
         }
       }
     }
   }
 
+  /**
+   * Determine if a test object is a test suite regardless of PHPUnit version.
+   *
+   * @param \PHPUnit\Framework\Test|\PHPUnit_Framework_Test $test
+   *   The test object to test if it is a test suite.
+   *
+   * @return bool
+   *   TRUE if it is a test suite, FALSE if not.
+   */
+  private function isTestSuite($test) {
+    if (class_exists('\PHPUnit_Framework_TestSuite') && $test instanceof \PHPUnit_Framework_TestSuite) {
+      return TRUE;
+    }
+    if (class_exists('PHPUnit\Framework\TestSuite') && $test instanceof TestSuite) {
+      return TRUE;
+    }
+    return FALSE;
+  }
+
+  /**
+   * Reacts to the end of a test.
+   *
+   * @param \PHPUnit\Framework\Test|\PHPUnit_Framework_Test $test
+   *   The test object that has ended its test run.
+   * @param float $time
+   *   The time the test took.
+   */
+  protected function standardsEndTest($test, $time) {
+    $this->doEndTest($test, $time);
+  }
+
 }

core/tests/Drupal/Tests/Listeners/HtmlOutputPrinter.php

@@ -2,70 +2,41 @@
 
 namespace Drupal\Tests\Listeners;
 
-/**
- * Defines a class for providing html output results for functional tests.
- */
-class HtmlOutputPrinter extends \PHPUnit_TextUI_ResultPrinter {
-
+use PHPUnit\Framework\TestResult;
+use PHPUnit\TextUI\ResultPrinter;
+
+if (class_exists('PHPUnit_Runner_Version') && version_compare(\PHPUnit_Runner_Version::id(), '6.0.0', '<')) {
+  class_alias('Drupal\Tests\Listeners\Legacy\HtmlOutputPrinter', 'Drupal\Tests\Listeners\HtmlOutputPrinter');
+  // Using an early return instead of a else does not work when using the
+  // PHPUnit phar due to some weird PHP behavior (the class gets defined without
+  // executing the code before it and so the definition is not properly
+  // conditional).
+}
+else {
   /**
-   * File to write html links to.
+   * Defines a class for providing html output results for functional tests.
    *
-   * @var string
+   * @internal
    */
-  protected $browserOutputFile;
-
-  /**
-   * {@inheritdoc}
-   */
-  public function __construct($out, $verbose, $colors, $debug, $numberOfColumns) {
-    parent::__construct($out, $verbose, $colors, $debug, $numberOfColumns);
-    if ($html_output_directory = getenv('BROWSERTEST_OUTPUT_DIRECTORY')) {
-      // Initialize html output debugging.
-      $html_output_directory = rtrim($html_output_directory, '/');
-
-      // Check if directory exists.
-      if (!is_dir($html_output_directory) || !is_writable($html_output_directory)) {
-        $this->writeWithColor('bg-red, fg-black', "HTML output directory $html_output_directory is not a writable directory.");
-      }
-      else {
-        // Convert to a canonicalized absolute pathname just in case the current
-        // working directory is changed.
-        $html_output_directory = realpath($html_output_directory);
-        $this->browserOutputFile = tempnam($html_output_directory, 'browser_output_');
-        if ($this->browserOutputFile) {
-          touch($this->browserOutputFile);
-        }
-        else {
-          $this->writeWithColor('bg-red, fg-black', "Unable to create a temporary file in $html_output_directory.");
-        }
-      }
-    }
-
-    if ($this->browserOutputFile) {
-      putenv('BROWSERTEST_OUTPUT_FILE=' . $this->browserOutputFile);
-    }
-    else {
-      // Remove any environment variable.
-      putenv('BROWSERTEST_OUTPUT_FILE');
+  class HtmlOutputPrinter extends ResultPrinter {
+    use HtmlOutputPrinterTrait;
+    /**
+     * {@inheritdoc}
+     */
+    public function __construct($out = NULL, $verbose = FALSE, $colors = self::COLOR_DEFAULT, $debug = FALSE, $numberOfColumns = 80, $reverse = FALSE) {
+      parent::__construct($out, $verbose, $colors, $debug, $numberOfColumns, $reverse);
+
+      $this->setUpHtmlOutput();
     }
-  }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function printResult(\PHPUnit_Framework_TestResult $result) {
-    parent::printResult($result);
+    /**
+     * {@inheritdoc}
+     */
+    public function printResult(TestResult $result) {
+      parent::printResult($result);
 
-    if ($this->browserOutputFile) {
-      $contents = file_get_contents($this->browserOutputFile);
-      if ($contents) {
-        $this->writeNewLine();
-        $this->writeWithColor('bg-yellow, fg-black', 'HTML output was generated');
-        $this->write($contents);
-      }
-      // No need to keep the file around any more.
-      unlink($this->browserOutputFile);
+      $this->printHtmlOutput();
     }
-  }
 
+  }
 }

core/tests/Drupal/Tests/Listeners/HtmlOutputPrinterTrait.php

+<?php
+
+namespace Drupal\Tests\Listeners;
+
+/**
+ * Defines a class for providing html output results for functional tests.
+ *
+ * @internal
+ */
+trait HtmlOutputPrinterTrait {
+
+  /**
+   * File to write html links to.
+   *
+   * @var string
+   */
+  protected $browserOutputFile;
+
+  /**
+   * Creates the file to list the HTML output created during the test.
+   *
+   * @see \Drupal\Tests\BrowserTestBase::initBrowserOutputFile()
+   */
+  protected function setUpHtmlOutput() {
+    if ($html_output_directory = getenv('BROWSERTEST_OUTPUT_DIRECTORY')) {
+      // Initialize html output debugging.
+      $html_output_directory = rtrim($html_output_directory, '/');
+
+      // Check if directory exists.
+      if (!is_dir($html_output_directory) || !is_writable($html_output_directory)) {
+        $this->writeWithColor('bg-red, fg-black', "HTML output directory $html_output_directory is not a writable directory.");
+      }
+      else {
+        // Convert to a canonicalized absolute pathname just in case the current
+        // working directory is changed.
+        $html_output_directory = realpath($html_output_directory);
+        $this->browserOutputFile = tempnam($html_output_directory, 'browser_output_');
+        if ($this->browserOutputFile) {
+          touch($this->browserOutputFile);
+        }
+        else {
+          $this->writeWithColor('bg-red, fg-black', "Unable to create a temporary file in $html_output_directory.");
+        }
+      }
+    }
+
+    if ($this->browserOutputFile) {
+      putenv('BROWSERTEST_OUTPUT_FILE=' . $this->browserOutputFile);
+    }
+    else {
+      // Remove any environment variable.
+      putenv('BROWSERTEST_OUTPUT_FILE');
+    }
+  }
+
+  /**
+   * Prints the list of HTML output generated during the test.
+   */
+  protected function printHtmlOutput() {
+    if ($this->browserOutputFile) {
+      $contents = file_get_contents($this->browserOutputFile);
+      if ($contents) {
+        $this->writeNewLine();
+        $this->writeWithColor('bg-yellow, fg-black', 'HTML output was generated');
+        $this->write($contents);
+      }
+      // No need to keep the file around any more.
+      unlink($this->browserOutputFile);
+    }
+  }
+
+}

core/tests/Drupal/Tests/Listeners/Legacy/DrupalListener.php

+<?php
+
+namespace Drupal\Tests\Listeners\Legacy;
+
+use Drupal\Tests\Listeners\DeprecationListenerTrait;
+use Drupal\Tests\Listeners\DrupalComponentTestListenerTrait;
+use Drupal\Tests\Listeners\DrupalStandardsListenerTrait;
+
+/**
+ * Listens to PHPUnit test runs.
+ *
+ * @internal
+ *   This class is not public Drupal API.
+ */
+class DrupalListener extends \PHPUnit_Framework_BaseTestListener {
+  use DeprecationListenerTrait;
+  use DrupalComponentTestListenerTrait;
+  use DrupalStandardsListenerTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  public function endTest(\PHPUnit_Framework_Test $test, $time) {
+    $this->deprecationEndTest($test, $time);
+    $this->componentEndTest($test, $time);
+    $this->standardsEndTest($test, $time);
+  }
+
+}

core/tests/Drupal/Tests/Listeners/Legacy/HtmlOutputPrinter.php

+<?php
+
+namespace Drupal\Tests\Listeners\Legacy;
+
+use Drupal\Tests\Listeners\HtmlOutputPrinterTrait;
+
+/**
+ * Defines a class for providing html output results for functional tests.
+ *
+ * @internal
+ */
+class HtmlOutputPrinter extends \PHPUnit_TextUI_ResultPrinter {
+  use HtmlOutputPrinterTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  public function __construct($out, $verbose, $colors, $debug, $numberOfColumns) {
+    parent::__construct($out, $verbose, $colors, $debug, $numberOfColumns);
+
+    $this->setUpHtmlOutput();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function printResult(\PHPUnit_Framework_TestResult $result) {
+    parent::printResult($result);
+
+    $this->printHtmlOutput();
+  }
+
+}

core/tests/Drupal/Tests/TestSuites/TestSuiteBaseTest.php

@@ -34,6 +34,13 @@ protected function getFilesystem() {
             ],
             'Tests' => [
               'CoreUnitTest.php' => '<?php',
+              // Ensure that the following files are not found as tests.
+              'Listeners' => [
+                'Listener.php' => '<?php',
+                'Legacy' => [
+                  'Listener.php' => '<?php',
+                ]
+              ]
             ],
           ],
         ],
@@ -91,8 +98,7 @@ public function testAddTestsBySuiteNamespaceCore($filesystem, $suite_namespace,
     $ref_add_tests->invokeArgs($stub, [vfsStream::url('root'), $suite_namespace]);
 
     // Determine if we loaded the expected test files.
-    $this->assertNotEmpty($stub->testFiles);
-    $this->assertEmpty(array_diff_assoc($expected_tests, $stub->testFiles));
+    $this->assertEquals($expected_tests, $stub->testFiles);
   }
 
   /**

core/tests/TestSuites/TestSuiteBase.php

@@ -3,11 +3,12 @@
 namespace Drupal\Tests\TestSuites;
 
 use Drupal\simpletest\TestDiscovery;
+use PHPUnit\Framework\TestSuite;
 
 /**
  * Base class for Drupal test suites.
  */
-abstract class TestSuiteBase extends \PHPUnit_Framework_TestSuite {
+abstract class TestSuiteBase extends TestSuite {
 
   /**
    * Finds extensions in a Drupal installation.
@@ -40,7 +41,12 @@ protected function addTestsBySuiteNamespace($root, $suite_namespace) {
     // always inside of core/tests/Drupal/${suite_namespace}Tests. The exception
     // to this is Unit tests for historical reasons.
     if ($suite_namespace == 'Unit') {
-      $this->addTestFiles(TestDiscovery::scanDirectory("Drupal\\Tests\\", "$root/core/tests/Drupal/Tests"));
+      $tests = TestDiscovery::scanDirectory("Drupal\\Tests\\", "$root/core/tests/Drupal/Tests");
+      $tests = array_filter($tests, function ($test) use ($root) {
+        // The Listeners directory does not contain tests.
+        return !preg_match("@^$root/core/tests/Drupal/Tests/Listeners(/|$)@", dirname($test));
+      });
+      $this->addTestFiles($tests);
     }
     else {
       $this->addTestFiles(TestDiscovery::scanDirectory("Drupal\\${suite_namespace}Tests\\", "$root/core/tests/Drupal/${suite_namespace}Tests"));