From 66ab1c955d27ff7875f62eb51a0bd6c4770a7ae3 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@freelancer.com>
Date: Sun, 15 Nov 2015 20:04:59 +0000
Subject: [PATCH] Remove arguments from unit test engines

Summary: Ref T9131. This doesn't seem to be used... it seems like it is a relic of postponed test results.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T9131

Differential Revision: https://secure.phabricator.com/D14487
---
 ...anistConfigurationDrivenUnitTestEngine.php | 29 ++++++++++++++-----
 src/unit/engine/ArcanistUnitTestEngine.php    | 22 +-------------
 src/unit/engine/PhutilUnitTestEngine.php      |  4 ---
 src/workflow/ArcanistUnitWorkflow.php         |  8 -----
 4 files changed, 23 insertions(+), 40 deletions(-)

diff --git a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
index e24b4d5f..652a46d2 100644
--- a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
+++ b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
@@ -115,6 +115,9 @@ final class ArcanistConfigurationDrivenUnitTestEngine
   }
 
   public function run() {
+    $renderer = $this->renderer;
+    $this->setRenderer(null);
+
     $paths = $this->getPaths();
 
     // If we are running with `--everything` then `$paths` will be `null`.
@@ -122,14 +125,15 @@ final class ArcanistConfigurationDrivenUnitTestEngine
       $paths = array();
     }
 
-    $engines    = $this->buildTestEngines();
-    $results    = array();
-    $exceptions = array();
+    $engines     = $this->buildTestEngines();
+    $all_results = array();
+    $exceptions  = array();
 
     foreach ($engines as $engine) {
       $engine
         ->setWorkingCopy($this->getWorkingCopy())
-        ->setEnableCoverage($this->getEnableCoverage());
+        ->setEnableCoverage($this->getEnableCoverage())
+        ->setRenderer($renderer);
 
       // TODO: At some point, maybe we should emit a warning here if an engine
       // doesn't support `--everything`, to reduce surprise when `--everything`
@@ -140,19 +144,30 @@ final class ArcanistConfigurationDrivenUnitTestEngine
 
       try {
         // TODO: Type check the results.
-        $results[] = $engine->run();
+        $results = $engine->run();
+        $all_results[] = $results;
+
+        foreach ($results as $result) {
+          if ($engine->shouldEchoTestResults()) {
+            echo $renderer->renderUnitResult($result);
+          }
+        }
       } catch (ArcanistNoEffectException $ex) {
         $exceptions[] = $ex;
       }
     }
 
-    if (!$results) {
+    if (!$all_results) {
       // If all engines throw an `ArcanistNoEffectException`, then we should
       // preserve this behavior.
       throw new ArcanistNoEffectException(pht('No tests to run.'));
     }
 
-    return array_mergev($results);
+    return array_mergev($all_results);
+  }
+
+  public function shouldEchoTestResults() {
+    return false;
   }
 
   private function loadAvailableTestEngines() {
diff --git a/src/unit/engine/ArcanistUnitTestEngine.php b/src/unit/engine/ArcanistUnitTestEngine.php
index e105f216..b54bafea 100644
--- a/src/unit/engine/ArcanistUnitTestEngine.php
+++ b/src/unit/engine/ArcanistUnitTestEngine.php
@@ -7,8 +7,6 @@ abstract class ArcanistUnitTestEngine extends Phobject {
 
   private $workingCopy;
   private $paths = array();
-  private $arguments = array();
-  private $enableAsyncTests;
   private $enableCoverage;
   private $runAllTests;
   private $configurationManager;
@@ -71,24 +69,6 @@ abstract class ArcanistUnitTestEngine extends Phobject {
     return $this->paths;
   }
 
-  final public function setArguments(array $arguments) {
-    $this->arguments = $arguments;
-    return $this;
-  }
-
-  final public function getArgument($key, $default = null) {
-    return idx($this->arguments, $key, $default);
-  }
-
-  final public function setEnableAsyncTests($enable_async_tests) {
-    $this->enableAsyncTests = $enable_async_tests;
-    return $this;
-  }
-
-  final public function getEnableAsyncTests() {
-    return $this->enableAsyncTests;
-  }
-
   final public function setEnableCoverage($enable_coverage) {
     $this->enableCoverage = $enable_coverage;
     return $this;
@@ -98,7 +78,7 @@ abstract class ArcanistUnitTestEngine extends Phobject {
     return $this->enableCoverage;
   }
 
-  final public function setRenderer(ArcanistUnitRenderer $renderer) {
+  final public function setRenderer(ArcanistUnitRenderer $renderer = null) {
     $this->renderer = $renderer;
     return $this;
   }
diff --git a/src/unit/engine/PhutilUnitTestEngine.php b/src/unit/engine/PhutilUnitTestEngine.php
index 110f8e35..42481434 100644
--- a/src/unit/engine/PhutilUnitTestEngine.php
+++ b/src/unit/engine/PhutilUnitTestEngine.php
@@ -221,8 +221,4 @@ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine {
     return $paths;
   }
 
-  public function shouldEchoTestResults() {
-    return !$this->renderer;
-  }
-
 }
diff --git a/src/workflow/ArcanistUnitWorkflow.php b/src/workflow/ArcanistUnitWorkflow.php
index 04d8c8e0..a1aadd3b 100644
--- a/src/workflow/ArcanistUnitWorkflow.php
+++ b/src/workflow/ArcanistUnitWorkflow.php
@@ -151,7 +151,6 @@ EOTEXT
     } else {
       $this->engine->setPaths($paths);
     }
-    $this->engine->setArguments($this->getPassthruArgumentsAsMap('unit'));
 
     $renderer = new ArcanistUnitConsoleRenderer();
     $this->engine->setRenderer($renderer);
@@ -165,13 +164,6 @@ EOTEXT
     }
     $this->engine->setEnableCoverage($enable_coverage);
 
-    // Enable possible async tests only for 'arc diff' not 'arc unit'
-    if ($this->getParentWorkflow()) {
-      $this->engine->setEnableAsyncTests(true);
-    } else {
-      $this->engine->setEnableAsyncTests(false);
-    }
-
     $results = $this->engine->run();
 
     $this->validateUnitEngineResults($this->engine, $results);