Duplicate test results per fail/exception

Nice!

+++ b/core/lib/Drupal/Core/Test/TestStatus.php
@@ -0,0 +1,36 @@
+  const SYSTEM = 3;

+++ b/core/modules/simpletest/simpletest.module
@@ -183,29 +184,15 @@ function simpletest_run_phpunit_tests($test_id, array $unescaped_test_classnames
+      'status' => TestStatus::labelForCode($status),

SYSTEM can be any number between 3 and 127, so Im not sure this would work. You may have to fill the status map in labelForCode with all the possible system signal errors. Everywhere we see SYSTEM we'll have to make sure we never say === we'll always have to test with >= SYSTEM as it will almost never be 3. Segfaults end up being signal 11, and there is a slew of other signals that are different from OS to OS. Suffice to say if its > 2, we consider all of those signals to be abnormal aborts as if it were a segfault.

More info here: http://php.net/manual/en/pcntl.constants.php

Otherwise, the rest of this is pretty sweet polish. Me gusta. ++

+++ b/core/lib/Drupal/Core/Test/TestStatus.php
@@ -0,0 +1,64 @@
+    if ($status >= static::SYSTEM) {
+      $label = $label . ': ' . $status;
+    }

I would leave the status code off of the label. Signal numbers vary from OS to OS, so its doubtful that we'd ever want logic branches based on the signal number - easier to say if ('error') than to substr for 'error' etc. It might be interesting to have that data, but something tells me that at most we'll only ever see a few signals there.

Otherwise, still LGTM.

This looks quite great and almost ready.

+++ b/core/modules/simpletest/tests/fixtures/simpletest_phpunit_run_command_test.php
@@ -10,12 +11,20 @@
-      exit(2);
+      exit(TestStatus::FAIL);
+    }
+    if (getenv('SimpletestPhpunitRunCommandTestWillDie') === 'exception') {
+      exit(TestStatus::EXCEPTION);
+    }
+    if (getenv('SimpletestPhpunitRunCommandTestWillDie') === 'error') {
+      exit(TestStatus::SYSTEM);
     }

Should we also have a test for the case of exit code > 2?

1. +++ b/core/modules/simpletest/tests/fixtures/simpletest_phpunit_run_command_test.php
   @@ -12,21 +9,19 @@
   +    $status = (int)getenv('SimpletestPhpunitRunCommandTestWillDie');
   

   afaik we use a space after the casting

2. +++ b/core/modules/simpletest/tests/fixtures/simpletest_phpunit_run_command_test.php
   @@ -12,21 +9,19 @@
   +    else exit($status);
   

   Note: this is not our CS

3. +++ b/core/modules/simpletest/tests/src/Unit/SimpletestPhpunitRunCommandTest.php
   @@ -3,35 +3,99 @@
   +    include_once self::$root . '/core/modules/simpletest/tests/fixtures/simpletest_phpunit_run_command_test.php';
   ...
   +    include_once self::$root . '/core/modules/simpletest/simpletest.module';
   

   Includes change global state and given that we should run in a separate process

4. +++ b/core/modules/simpletest/tests/src/Unit/SimpletestPhpunitRunCommandTest.php
   @@ -3,35 +3,99 @@
   +    // 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'];
        }
   

   Nice additional test coverage

Thank you!

This would be great to get in sooner than later. Would help with fixing a lot of strange wtf reporting issues with errors.

It all passes and is RTBC, can we include it?

Committed and pushed 30f78fc to 8.3.x and 81ef407 to 8.2.x. Thanks!

I've committed this to 8.2.x as well because this part of the test-runner which should be viewed as @internal and correct reporting of results on DrupalCI outweighs any risks to alternate test runner implementations.

diff --git a/core/lib/Drupal/Core/Test/TestStatus.php b/core/lib/Drupal/Core/Test/TestStatus.php
index 47216f7..b70dcdd 100644
--- a/core/lib/Drupal/Core/Test/TestStatus.php
+++ b/core/lib/Drupal/Core/Test/TestStatus.php
@@ -59,4 +59,5 @@ public static function label($status) {
     $label = $statusMap[$status > static::SYSTEM ? static::SYSTEM : $status];
     return $label;
   }
+
 }
diff --git a/core/modules/simpletest/tests/src/Unit/SimpletestPhpunitRunCommandTest.php b/core/modules/simpletest/tests/src/Unit/SimpletestPhpunitRunCommandTest.php
index 7cd3fc7..a96db96 100644
--- a/core/modules/simpletest/tests/src/Unit/SimpletestPhpunitRunCommandTest.php
+++ b/core/modules/simpletest/tests/src/Unit/SimpletestPhpunitRunCommandTest.php
@@ -4,7 +4,6 @@
 
 use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Core\File\FileSystemInterface;
-use Drupal\Tests\simpletest\Unit\SimpletestPhpunitRunCommandTestWillDie;
 
 /**
  * Tests simpletest_run_phpunit_tests() handles PHPunit fatals correctly.

Minor code style stuff fixed on commit.

Yay!