Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json.

Summary:
This change updates PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use phpunit's junit output rather than json.

--log-json was deprecated in PHPUnit 5.7, and removed in PHPUnit 6.0.0 (2017-02-03).

Fixes T15667

Test Plan:
Download an example PHP repo, with arcconfig set to 'PhpunitTestEngine'. Example:
```
git clone https://github.com/campbsb/example-phorge-php-project.git
```

Install phpunit using composer, and test it
```
cd example-phorge-php-project
php ~/composer.phar update
phpunit
```
In a PHP phorge repo, set the following in .arcconfig:
```
    "unit.engine": "PhpunitTestEngine",
    "phpunit_config": "phpunit.xml",
```
Make a non-breaking change to src/Service.php (eg add a space), and run 'arc unit'

Reviewers: O1 Blessed Committers, avivey, speck

Reviewed By: O1 Blessed Committers, avivey, speck

Subscribers: avivey, Ekubischta, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15667

Differential Revision: https://we.phorge.it/D25472

src/unit/engine/PhpunitTestEngine.php

@@ -52,7 +52,7 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
       if (!Filesystem::pathExists($test_path)) {
         continue;
       }
-      $json_tmp = new TempFile();
+      $xml_tmp = new TempFile();
       $clover_tmp = null;
       $clover = null;
       if ($this->getEnableCoverage() !== false) {
@@ -64,10 +64,10 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
 
       $stderr = '-d display_errors=stderr';
 
-      $futures[$test_path] = new ExecFuture('%C %C %C --log-json %s %C %s',
-        $this->phpunitBinary, $config, $stderr, $json_tmp, $clover, $test_path);
+      $futures[$test_path] = new ExecFuture('%C %C %C --log-junit %s %C %s',
+        $this->phpunitBinary, $config, $stderr, $xml_tmp, $clover, $test_path);
       $tmpfiles[$test_path] = array(
-        'json' => $json_tmp,
+        'xml' => $xml_tmp,
         'clover' => $clover_tmp,
       );
     }
@@ -81,7 +81,7 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
 
       $results[] = $this->parseTestResults(
         $test,
-        $tmpfiles[$test]['json'],
+        $tmpfiles[$test]['xml'],
         $tmpfiles[$test]['clover'],
         $stderr);
     }
@@ -99,8 +99,8 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
    *
    * @return array
    */
-  private function parseTestResults($path, $json_tmp, $clover_tmp, $stderr) {
-    $test_results = Filesystem::readFile($json_tmp);
+  private function parseTestResults($path, $xml_tmp, $clover_tmp, $stderr) {
+    $test_results = Filesystem::readFile($xml_tmp);
     return id(new ArcanistPhpunitTestResultParser())
       ->setEnableCoverage($this->getEnableCoverage())
       ->setProjectRoot($this->projectRoot)

src/unit/parser/ArcanistPhpunitTestResultParser.php

@@ -25,80 +25,19 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
       return array($result);
     }
 
-    $report = $this->getJsonReport($test_results);
-
     // coverage is for all testcases in the executed $path
     $coverage = array();
     if ($this->enableCoverage !== false) {
       $coverage = $this->readCoverage();
     }
 
-    $last_test_finished = true;
+    $xunit_result_parser = new ArcanistXUnitTestResultParser();
+    $results = $xunit_result_parser->parseTestResults($test_results);
 
-    $results = array();
-    foreach ($report as $event) {
-      switch (idx($event, 'event')) {
-      case 'test':
-        break;
-      case 'testStart':
-        $last_test_finished = false;
-        // fall through
-        default:
-          continue 2; // switch + loop
-      }
-
-      $status = ArcanistUnitTestResult::RESULT_PASS;
-      $user_data = '';
-
-      if ('fail' == idx($event, 'status')) {
-        $status = ArcanistUnitTestResult::RESULT_FAIL;
-        $user_data  .= idx($event, 'message')."\n";
-        foreach (idx($event, 'trace') as $trace) {
-          $user_data .= sprintf(
-            "\n%s:%s",
-            idx($trace, 'file'),
-            idx($trace, 'line'));
-        }
-      } else if ('error' == idx($event, 'status')) {
-        if (strpos(idx($event, 'message'), 'Skipped Test') !== false) {
-          $status = ArcanistUnitTestResult::RESULT_SKIP;
-          $user_data .= idx($event, 'message');
-        } else if (strpos(
-            idx($event, 'message'),
-            'Incomplete Test') !== false) {
-          $status = ArcanistUnitTestResult::RESULT_SKIP;
-          $user_data .= idx($event, 'message');
-        } else {
-          $status = ArcanistUnitTestResult::RESULT_BROKEN;
-          $user_data  .= idx($event, 'message');
-          foreach (idx($event, 'trace') as $trace) {
-            $user_data .= sprintf(
-              "\n%s:%s",
-              idx($trace, 'file'),
-              idx($trace, 'line'));
-          }
-        }
-      }
-
-      $name = preg_replace('/ \(.*\)/s', '', idx($event, 'test'));
-
-      $result = new ArcanistUnitTestResult();
-      $result->setName($name);
-      $result->setResult($status);
-      $result->setDuration(idx($event, 'time'));
+    foreach ($results as $result) {
       $result->setCoverage($coverage);
-      $result->setUserData($user_data);
-
-      $results[] = $result;
-      $last_test_finished = true;
     }
 
-    if (!$last_test_finished) {
-      $results[] = id(new ArcanistUnitTestResult())
-        ->setName(idx($event, 'test')) // use last event
-        ->setUserData($this->stderr)
-        ->setResult(ArcanistUnitTestResult::RESULT_BROKEN);
-    }
     return $results;
   }
 
@@ -161,28 +100,4 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
     return $reports;
   }
 
-  /**
-   * We need this non-sense to make json generated by phpunit
-   * valid.
-   *
-   * @param string $json String containing JSON report
-   * @return array JSON decoded array
-   */
-  private function getJsonReport($json) {
-
-    if (empty($json)) {
-      throw new Exception(
-        pht(
-          'JSON report file is empty, it probably means that phpunit '.
-          'failed to run tests. Try running %s with %s option and then run '.
-          'generated phpunit command yourself, you might get the answer.',
-          'arc unit',
-          '--trace'));
-    }
-
-    $json = preg_replace('/}{\s*"/', '},{"', $json);
-    $json = '['.$json.']';
-    return phutil_json_decode($json);
-  }
-
 }