Issue #2937469 by alexpott, Mixologic: Automatic upgrade to PHPUnit 6 is dangerous

composer.json

@@ -52,7 +52,7 @@
         "post-autoload-dump": "Drupal\\Core\\Composer\\Composer::ensureHtaccess",
         "post-package-install": "Drupal\\Core\\Composer\\Composer::vendorTestCodeCleanup",
         "post-package-update": "Drupal\\Core\\Composer\\Composer::vendorTestCodeCleanup",
-        "post-install-cmd": "Drupal\\Core\\Composer\\Composer::upgradePHPUnit",
+        "drupal-phpunit-upgrade-check": "Drupal\\Core\\Composer\\Composer::upgradePHPUnit",
         "drupal-phpunit-upgrade": "@composer update phpunit/phpunit --with-dependencies --no-progress",
         "phpcs": "phpcs --standard=core/phpcs.xml.dist --runtime-set installed_paths $($COMPOSER_BINARY config vendor-dir)/drupal/coder/coder_sniffer --",
         "phpcbf": "phpcbf --standard=core/phpcs.xml.dist --runtime-set installed_paths $($COMPOSER_BINARY config vendor-dir)/drupal/coder/coder_sniffer --"

core/lib/Drupal/Core/Composer/Composer.php

@@ -162,13 +162,29 @@ EOT;
 
     // If the PHP version is 7.2 or above and PHPUnit is less than version 6
     // call the drupal-phpunit-upgrade script to upgrade PHPUnit.
-    if (version_compare(PHP_MAJOR_VERSION . '.' . PHP_MINOR_VERSION, '7.2') >= 0 && version_compare($phpunit_package->getVersion(), '6.1') < 0) {
+    if (!static::upgradePHPUnitCheck($phpunit_package->getVersion())) {
       $event->getComposer()
         ->getEventDispatcher()
         ->dispatchScript('drupal-phpunit-upgrade');
     }
   }
 
+  /**
+   * Determines if PHPUnit needs to be upgraded.
+   *
+   * This method is located in this file because it is possible that it is
+   * called before the autoloader is available.
+   *
+   * @param string $phpunit_version
+   *   The PHPUnit version string.
+   *
+   * @return bool
+   *   TRUE if the PHPUnit needs to be upgraded, FALSE if not.
+   */
+  public static function upgradePHPUnitCheck($phpunit_version) {
+    return !(version_compare(PHP_MAJOR_VERSION . '.' . PHP_MINOR_VERSION, '7.2') >= 0 && version_compare($phpunit_version, '6.1') < 0);
+  }
+
   /**
    * Remove possibly problematic test files from vendored projects.
    *

core/scripts/run-tests.sh

@@ -9,6 +9,7 @@ use Drupal\Component\FileSystem\FileSystem;
 use Drupal\Component\Utility\Html;
 use Drupal\Component\Utility\Timer;
 use Drupal\Component\Uuid\Php;
+use Drupal\Core\Composer\Composer;
 use Drupal\Core\Asset\AttachedAssets;
 use Drupal\Core\Database\Database;
 use Drupal\Core\StreamWrapper\PublicStream;
@@ -18,10 +19,9 @@ use Drupal\simpletest\Form\SimpletestResultsForm;
 use Drupal\simpletest\TestBase;
 use Drupal\simpletest\TestDiscovery;
 use PHPUnit\Framework\TestCase;
+use PHPUnit\Runner\Version;
 use Symfony\Component\HttpFoundation\Request;
 
-$autoloader = require_once __DIR__ . '/../../autoload.php';
-
 // Define some colors for display.
 // A nice calming green.
 const SIMPLETEST_SCRIPT_COLOR_PASS = 32;
@@ -37,11 +37,6 @@ const SIMPLETEST_SCRIPT_EXIT_SUCCESS = 0;
 const SIMPLETEST_SCRIPT_EXIT_FAILURE = 1;
 const SIMPLETEST_SCRIPT_EXIT_EXCEPTION = 2;
 
-if (!class_exists(TestCase::class)) {
-  echo "\nrun-tests.sh requires the PHPUnit testing framework. Please use 'composer install --dev' to ensure that it is present.\n\n";
-  exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
-}
-
 // Set defaults and get overrides.
 list($args, $count) = simpletest_script_parse_args();
 
@@ -52,14 +47,9 @@ if ($args['help'] || $count == 0) {
 
 simpletest_script_init();
 
-try {
+if (!class_exists(TestCase::class)) {
-  $request = Request::createFromGlobals();
+  echo "\nrun-tests.sh requires the PHPUnit testing framework. Please use 'composer install --dev' to ensure that it is present.\n\n";
-  $kernel = TestRunnerKernel::createFromRequest($request, $autoloader);
+  exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
-  $kernel->prepareLegacyRequest($request);
-}
-catch (Exception $e) {
-  echo (string) $e;
-  exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
 }
 
 if ($args['execute-test']) {
@@ -142,6 +132,18 @@ if ($args['clean']) {
   exit(SIMPLETEST_SCRIPT_EXIT_SUCCESS);
 }
 
+// Ensure we have the correct PHPUnit version for the version of PHP.
+if (class_exists('\PHPUnit_Runner_Version')) {
+  $phpunit_version = \PHPUnit_Runner_Version::id();
+}
+else {
+  $phpunit_version = Version::id();
+}
+if (!Composer::upgradePHPUnitCheck($phpunit_version)) {
+  simpletest_script_print_error("PHPUnit testing framework version 6 or greater is required when running on PHP 7.2 or greater. Run the command 'composer run-script drupal-phpunit-upgrade' in order to fix this.");
+  exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
+}
+
 $test_list = simpletest_script_get_test_list();
 
 // Try to allocate unlimited time to run the tests.
@@ -463,6 +465,25 @@ function simpletest_script_init() {
     exit(SIMPLETEST_SCRIPT_EXIT_FAILURE);
   }
 
+  // Detect if we're in the top-level process using the private 'execute-test'
+  // argument. Determine if being run on drupal.org's testing infrastructure
+  // using the presence of 'drupaltestbot' in the database url.
+  // @todo https://www.drupal.org/project/drupalci_testbot/issues/2860941 Use
+  //   better environment variable to detect DrupalCI.
+  // @todo https://www.drupal.org/project/drupal/issues/2942473 Remove when
+  //   dropping PHPUnit 4 and PHP 5 support.
+  if (!$args['execute-test'] && preg_match('/drupalci/', $args['sqlite'])) {
+    // Update PHPUnit if needed and possible. There is a later check once the
+    // autoloader is in place to ensure we're on the correct version. We need to
+    // do this before the autoloader is in place to ensure that it is correct.
+    $composer = ($composer = rtrim('\\' === DIRECTORY_SEPARATOR ? preg_replace('/[\r\n].*/', '', `where.exe composer.phar`) : `which composer.phar`))
+      ? $php . ' ' . escapeshellarg($composer)
+      : 'composer';
+    passthru("$composer run-script drupal-phpunit-upgrade-check");
+  }
+
+  $autoloader = require_once __DIR__ . '/../../autoload.php';
+
   // Get URL from arguments.
   if (!empty($args['url'])) {
     $parsed_url = parse_url($args['url']);
@@ -521,6 +542,17 @@ function simpletest_script_init() {
   }
 
   chdir(realpath(__DIR__ . '/../..'));
+
+  // Prepare the kernel.
+  try {
+    $request = Request::createFromGlobals();
+    $kernel = TestRunnerKernel::createFromRequest($request, $autoloader);
+    $kernel->prepareLegacyRequest($request);
+  }
+  catch (Exception $e) {
+    echo (string) $e;
+    exit(SIMPLETEST_SCRIPT_EXIT_EXCEPTION);
+  }
 }
 
 /**

core/tests/bootstrap.php

@@ -8,6 +8,7 @@
  */
 
 use Drupal\Component\Assertion\Handle;
+use Drupal\Core\Composer\Composer;
 use PHPUnit\Runner\Version;
 
 /**
@@ -150,6 +151,19 @@ function drupal_phpunit_populate_class_loader() {
 // Do class loader population.
 drupal_phpunit_populate_class_loader();
 
+// Ensure we have the correct PHPUnit version for the version of PHP.
+if (class_exists('\PHPUnit_Runner_Version')) {
+  $phpunit_version = \PHPUnit_Runner_Version::id();
+}
+else {
+  $phpunit_version = Version::id();
+}
+if (!Composer::upgradePHPUnitCheck($phpunit_version)) {
+  $message = "PHPUnit testing framework version 6 or greater is required when running on PHP 7.2 or greater. Run the command 'composer run-script drupal-phpunit-upgrade' in order to fix this.";
+  echo "\033[31m" . $message . "\n\033[0m";
+  exit(1);
+}
+
 // Set sane locale settings, to ensure consistent string, dates, times and
 // numbers handling.
 // @see \Drupal\Core\DrupalKernel::bootEnvironment()
@@ -170,7 +184,7 @@ Handle::register();
 
 // PHPUnit 4 to PHPUnit 6 bridge. Tests written for PHPUnit 4 need to work on
 // PHPUnit 6 with a minimum of fuss.
-if (class_exists('PHPUnit\Runner\Version') && version_compare(Version::id(), '6.1', '>=')) {
+if (version_compare($phpunit_version, '6.1', '>=')) {
   class_alias('\PHPUnit\Framework\AssertionFailedError', '\PHPUnit_Framework_AssertionFailedError');
   class_alias('\PHPUnit\Framework\Constraint\Count', '\PHPUnit_Framework_Constraint_Count');
   class_alias('\PHPUnit\Framework\Error\Error', '\PHPUnit_Framework_Error');