Issue #2810083 by Mile23, dawehner, Mixologic: Duplicate test results per fail/exception

core/lib/Drupal/Core/Test/TestStatus.php

@@ -0,0 +1,63 @@
+<?php
+
+namespace Drupal\Core\Test;
+
+/**
+ * Consolidates test result status information.
+ *
+ * For our test runners, a $status of 0 = passed test, 1 = failed test,
+ * 2 = exception, >2 indicates segfault timeout, or other type of system
+ * failure.
+ */
+class TestStatus {
+
+  /**
+   * Signify that the test result was a passed test.
+   */
+  const PASS = 0;
+
+  /**
+   * Signify that the test result was a failed test.
+   */
+  const FAIL = 1;
+
+  /**
+   * Signify that the test result was an exception or code error.
+   *
+   * This means that the test runner was able to exit and report an error.
+   */
+  const EXCEPTION = 2;
+
+  /**
+   * Signify a system error where the test runner was unable to complete.
+   *
+   * Note that SYSTEM actually represents the lowest value of system errors, and
+   * the returned value could be as high as 127. Since that's the case, this
+   * constant should be used for range comparisons, and not just for equality.
+   *
+   * @see http://php.net/manual/en/pcntl.constants.php
+   */
+  const SYSTEM = 3;
+
+  /**
+   * Turns a status code into a human-readable string.
+   *
+   * @param int $status
+   *   A test runner return code.
+   *
+   * @return string
+   *   The human-readable version of the status code.
+   */
+  public static function label($status) {
+    $statusMap = [
+      static::PASS => 'pass',
+      static::FAIL => 'fail',
+      static::EXCEPTION => 'exception',
+      static::SYSTEM => 'error',
+    ];
+    // For status 3 and higher, we want 'error.'
+    $label = $statusMap[$status > static::SYSTEM ? static::SYSTEM : $status];
+    return $label;
+  }
+
+}

core/modules/simpletest/simpletest.module

@@ -13,6 +13,7 @@ use Drupal\simpletest\TestBase;
 use Drupal\Core\Test\TestDatabase;
 use Drupal\simpletest\TestDiscovery;
 use Symfony\Component\Process\PhpExecutableFinder;
+use Drupal\Core\Test\TestStatus;
 
 /**
  * Implements hook_help().
@@ -183,16 +184,15 @@ function simpletest_run_phpunit_tests($test_id, array $unescaped_test_classnames
   $phpunit_file = simpletest_phpunit_xml_filepath($test_id);
   simpletest_phpunit_run_command($unescaped_test_classnames, $phpunit_file, $status, $output);
 
-  $rows = simpletest_phpunit_xml_to_rows($test_id, $phpunit_file);
-  // A $status of 0 = passed test, 1 = failed test, > 1 indicates segfault
-  // timeout, or other type of failure.
-  if ($status > 1) {
-    // Something broke during the execution of phpunit.
-    // Return an error record of all failed classes.
+  $rows = [];
+  if ($status == TestStatus::PASS) {
+    $rows = simpletest_phpunit_xml_to_rows($test_id, $phpunit_file);
+  }
+  else {
     $rows[] = [
       'test_id' => $test_id,
       'test_class' => implode(",", $unescaped_test_classnames),
-      'status' => 'fail',
+      'status' => TestStatus::label($status),
       'message' => 'PHPunit Test failed to complete; Error: ' . implode("\n", $output),
       'message_group' => 'Other',
       'function' => implode(",", $unescaped_test_classnames),
@@ -200,20 +200,6 @@ function simpletest_run_phpunit_tests($test_id, array $unescaped_test_classnames
       'file' => $phpunit_file,
     ];
   }
-
-  if ($status === 1) {
-    $rows[] = [
-      'test_id' => $test_id,
-      'test_class' => implode(",", $unescaped_test_classnames),
-      'status' => 'fail',
-      'message' => 'PHPunit Test failed to complete; Error: ' . implode("\n", $output),
-      'message_group' => 'Other',
-      'function' => implode(",", $unescaped_test_classnames),
-      'line' => '0',
-      'file' => $phpunit_file,
-    ];
-  }
-
   return $rows;
 }
 

core/modules/simpletest/tests/fixtures/simpletest_phpunit_run_command_test.php

@@ -2,22 +2,27 @@
 
 namespace Drupal\Tests\simpletest\Unit;
 
-use Drupal\Tests\UnitTestCase;
-
 /**
  * This test crashes PHP.
  *
  * To avoid accidentally running, it is not in a normal PSR-4 directory, the
  * file name does not adhere to PSR-4 and an environment variable also needs to
  * be set for the crash to happen.
+ *
+ * @see \Drupal\Tests\simpletest\Unit\SimpletestPhpunitRunCommandTest::testSimpletestPhpUnitRunCommand()
  */
-class SimpletestPhpunitRunCommandTestWillDie extends UnitTestCase {
+class SimpletestPhpunitRunCommandTestWillDie extends \PHPUnit_Framework_TestCase {
 
+  /**
+   * Performs the status specified by SimpletestPhpunitRunCommandTestWillDie.
+   */
   public function testWillDie() {
-    if (getenv('SimpletestPhpunitRunCommandTestWillDie') === 'fail') {
-      exit(2);
+    $status = (int) getenv('SimpletestPhpunitRunCommandTestWillDie');
+    if ($status == 0) {
+      $this->assertTrue(TRUE, 'Assertion to ensure test pass');
+      return;
     }
-    $this->assertTrue(TRUE, 'Assertion to ensure test pass');
+    exit($status);
   }
 
 }

core/modules/simpletest/tests/src/Unit/SimpletestPhpunitRunCommandTest.php

@@ -3,7 +3,7 @@
 namespace Drupal\Tests\simpletest\Unit;
 
 use Drupal\Core\DependencyInjection\ContainerBuilder;
-use Drupal\Tests\UnitTestCase;
+use Drupal\Core\File\FileSystemInterface;
 
 /**
  * Tests simpletest_run_phpunit_tests() handles PHPunit fatals correctly.
@@ -11,27 +11,92 @@ use Drupal\Tests\UnitTestCase;
  * @group simpletest
  *
  * @runTestsInSeparateProcesses
- * @preserveGlobalState disabled
  */
-class SimpletestPhpunitRunCommandTest extends UnitTestCase {
+class SimpletestPhpunitRunCommandTest extends \PHPUnit_Framework_TestCase {
 
-  function testSimpletestPhpUnitRunCommand() {
-    include_once __DIR__ . '/../../fixtures/simpletest_phpunit_run_command_test.php';
-    $app_root = __DIR__ . '/../../../../../..';
-    include_once "$app_root/core/modules/simpletest/simpletest.module";
+  /**
+   * Path to the app root.
+   *
+   * @var string
+   */
+  protected static $root;
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function setUpBeforeClass() {
+    parent::setUpBeforeClass();
+    // Figure out our app root.
+    self::$root = dirname(dirname(dirname(dirname(dirname(dirname(__DIR__))))));
+    // Include the files we need for tests. The stub test we will run is
+    // SimpletestPhpunitRunCommandTestWillDie which is located in
+    // simpletest_phpunit_run_command_test.php.
+    include_once self::$root . '/core/modules/simpletest/tests/fixtures/simpletest_phpunit_run_command_test.php';
+    // Since we're testing simpletest_run_phpunit_tests(), we need to include
+    // simpletest.module.
+    include_once self::$root . '/core/modules/simpletest/simpletest.module';
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+    // Organize our mock container.
     $container = new ContainerBuilder();
-    $container->set('app.root', $app_root);
-    $file_system = $this->prophesize('Drupal\Core\File\FileSystemInterface');
+    $container->set('app.root', self::$root);
+    $file_system = $this->prophesize(FileSystemInterface::class);
+    // The simpletest directory wrapper will always point to /tmp.
     $file_system->realpath('public://simpletest')->willReturn(sys_get_temp_dir());
     $container->set('file_system', $file_system->reveal());
     \Drupal::setContainer($container);
-    $test_id = basename(tempnam(sys_get_temp_dir(), 'xxx'));
-    foreach (['pass', 'fail'] as $status) {
-      putenv('SimpletestPhpunitRunCommandTestWillDie=' . $status);
-      $ret = simpletest_run_phpunit_tests($test_id, ['Drupal\Tests\simpletest\Unit\SimpletestPhpunitRunCommandTestWillDie']);
-      $this->assertSame($ret[0]['status'], $status);
+  }
+
+  /**
+   * Data provider for testSimpletestPhpUnitRunCommand().
+   *
+   * @return array
+   *   Arrays of status codes and the label they're expected to have.
+   */
+  public function provideStatusCodes() {
+    $data = [
+      [0, 'pass'],
+      [1, 'fail'],
+      [2, 'exception'],
+    ];
+    // All status codes 3 and above should be labeled 'error'.
+    // @todo: The valid values here would be 3 to 127. But since the test
+    // touches the file system a lot, we only have 3, 4, and 127 for speed.
+    foreach ([3, 4, 127] as $status) {
+      $data[] = [$status, 'error'];
     }
+    return $data;
+  }
+
+  /**
+   * Test the round trip for PHPUnit execution status codes.
+   *
+   * @covers ::simpletest_run_phpunit_tests
+   *
+   * @dataProvider provideStatusCodes
+   */
+  public function testSimpletestPhpUnitRunCommand($status, $label) {
+    $test_id = basename(tempnam(sys_get_temp_dir(), 'xxx'));
+    putenv('SimpletestPhpunitRunCommandTestWillDie=' . $status);
+    $ret = simpletest_run_phpunit_tests($test_id, [SimpletestPhpunitRunCommandTestWillDie::class]);
+    $this->assertSame($ret[0]['status'], $label);
+    putenv('SimpletestPhpunitRunCommandTestWillDie');
     unlink(simpletest_phpunit_xml_filepath($test_id));
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function tearDown() {
+    // We unset the $base_url global, since the test code sets it as a
+    // side-effect.
+    unset($GLOBALS['base_url']);
+    parent::tearDown();
+  }
+
 }