Merge pull request #4 from finagin/master

Apply T12785 patch
sedmsk · May 14, 2019 · 0a22624 · 0a22624
2 parents f6c8bd9 + d61d6e1
commit 0a22624
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 74 deletions.
diff --git a/src/unit/engine/PhpunitTestEngine.php b/src/unit/engine/PhpunitTestEngine.php
@@ -52,7 +52,7 @@ public function run() {
       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 @@ public function run() {
 
       $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 @@ public function run() {
 
       $results[] = $this->parseTestResults(
         $test,
-        $tmpfiles[$test]['json'],
+        $tmpfiles[$test]['xml'],
         $tmpfiles[$test]['clover'],
         $stderr);
     }
@@ -90,17 +90,17 @@ public function run() {
   }
 
   /**
-   * Parse test results from phpunit json report.
+   * Parse test results from phpunit XML report.
    *
    * @param string $path Path to test
-   * @param string $json_tmp Path to phpunit json report
+   * @param string $xml_tmp Path to phpunit XML report
    * @param string $clover_tmp Path to phpunit clover report
    * @param string $stderr Data written to stderr
    *
    * @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)

diff --git a/src/unit/parser/ArcanistPhpunitTestResultParser.php b/src/unit/parser/ArcanistPhpunitTestResultParser.php
@@ -9,10 +9,10 @@
 final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
 
   /**
-   * Parse test results from phpunit json report
+   * Parse test results from phpunit XML report
    *
    * @param string $path Path to test
-   * @param string $test_results String containing phpunit json report
+   * @param string $test_results String containing phpunit XML report
    *
    * @return array
    */
@@ -25,7 +25,7 @@ public function parseTestResults($path, $test_results) {
       return array($result);
     }
 
-    $report = $this->getJsonReport($test_results);
+    $report = simplexml_load_string($test_results);
 
     // coverage is for all testcases in the executed $path
     $coverage = array();
@@ -36,56 +36,36 @@ public function parseTestResults($path, $test_results) {
     $last_test_finished = true;
 
     $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
-      }
-
+    foreach ($report->testsuite as $test_suite) {
       $status = ArcanistUnitTestResult::RESULT_PASS;
       $user_data = '';
 
-      if ('fail' == idx($event, 'status')) {
+      if ((int)$test_suite['failures'] > 0) {
         $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'));
+        foreach ($test_suite->testcase as $test_case) {
+          foreach ($test_case->failure as $failure) {
+              $user_data .= sprintf(
+                "\n%s",
+                (string)$failure);
+          }
         }
-      } 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'));
+      } else if ($test_suite['errors'] > 0) {
+        $status = ArcanistUnitTestResult::RESULT_BROKEN;
+        foreach ($test_suite->testcase as $test_case) {
+          foreach ($test_case->error as $error) {
+              $user_data .= sprintf(
+                "\n%s",
+                (string)$error);
           }
         }
       }
 
-      $name = preg_replace('/ \(.*\)/s', '', idx($event, 'test'));
+      $name = preg_replace('/ \(.*\)/s', '', $test_suite['name']);
 
       $result = new ArcanistUnitTestResult();
       $result->setName($name);
       $result->setResult($status);
-      $result->setDuration(idx($event, 'time'));
+      $result->setDuration((float)$test_suite['time']);
       $result->setCoverage($coverage);
       $result->setUserData($user_data);
 
@@ -95,7 +75,7 @@ public function parseTestResults($path, $test_results) {
 
     if (!$last_test_finished) {
       $results[] = id(new ArcanistUnitTestResult())
-        ->setName(idx($event, 'test')) // use last event
+        ->setName($test_suite['name']) // use last event
         ->setUserData($this->stderr)
         ->setResult(ArcanistUnitTestResult::RESULT_BROKEN);
     }
@@ -161,28 +141,4 @@ private function readCoverage() {
     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);
-  }
-
 }