[Arcanist Configuration Driven Unit Test Engine] Test Result is echod in the wrong case

I think I found a bug in ArcanistConfigurationDrivenUnitTestEngine.

Reproduction Instructions
Variant 1:

  • create a subclass of ArcanistUnitTestEngine that returns true for shouldEchoTestResults (which is the default)
  • add it it .arcunit (and use ArcanistConfigurationDrivenUnitTestEngine as the engine, also default)
  • run unit tests => no test result output

Variant 2 (inside e.g. arc worktree):

  • run arc unit --engine PhutilUnitTestEngine => Test results are output twice.

The problem seems to be that, PhutilUnitTestEngine doesn’t specify that it does print test results by itself, and ArcanistConfigurationDrivenUnitTestEngine checks for the inverse condition.

Phabricator/Arcanist Version
arcanist 6937d389472d63fe3fb251cbf733f8d21ba33950 (4 May 2020)

Proposed Solution

diff --git a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
index 5576d357..63ba7ce8 100644
--- a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
+++ b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
@@ -151,7 +151,7 @@ final class ArcanistConfigurationDrivenUnitTestEngine
         foreach ($results as $result) {
           // If the proxied engine renders its own test results then there
           // is no need to render them again here.
-          if (!$engine->shouldEchoTestResults()) {
+          if ($engine->shouldEchoTestResults()) {
             echo $renderer->renderUnitResult($result);
           }
         }
diff --git a/src/unit/engine/PhutilUnitTestEngine.php b/src/unit/engine/PhutilUnitTestEngine.php
index 42481434..e856fce9 100644
--- a/src/unit/engine/PhutilUnitTestEngine.php
+++ b/src/unit/engine/PhutilUnitTestEngine.php
@@ -77,6 +77,10 @@ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine {
     return $results;
   }
 
+  public function shouldEchoTestResults() {
+    return false;
+  }
+
   private function getAllTests() {
     $project_root = $this->getWorkingCopy()->getProjectRoot();