[Wilds] Rename "formatters" to "sinks" and restore the console output sufficiently to see which tests are failing

Summary:
Ref T13098. Since I plan to implement "send the results to Harbormaster" as another type of formatter/output/sink, just rename the objects which receive unit test results and print/write/transmit them into "Sinks" (in the sense of  Source/Sink).

Get the default console sink working well enough to see what's failing. As with all other changes in this series this is very rough, but the general idea is that I want to:

  - Let sinks stream both ongoing status information and final results.
  - For the console output, try to increase the signal-to-noise ratio of the output stream. Today, it's too easy to lose a failed test in the results. I want to improve this by outputting less frequently and summarizing passes ("93 tests passed.") so that the streaming output mostly shows failures and it's easier to make a decision to `^C` and revise if you see something you don't like.
  - Also, add a summary mode at the end which makes sure failures show up on the console and aren't scrolled up 30 pages. For now, this is quite rough.

Test Plan:
```
373 PASSED * 16 SKIPPED * 162 FAILED/BROKEN/UNSTABLE
```

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19711

src/__phutil_library_map__.php

@@ -165,7 +165,7 @@ phutil_register_library_map(array(
     'ArcanistDeclarationParenthesesXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistDeclarationParenthesesXHPASTLinterRuleTestCase.php',
     'ArcanistDefaultParametersXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistDefaultParametersXHPASTLinterRule.php',
     'ArcanistDefaultParametersXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistDefaultParametersXHPASTLinterRuleTestCase.php',
-    'ArcanistDefaultUnitFormatter' => 'unit/formatter/ArcanistDefaultUnitFormatter.php',
+    'ArcanistDefaultUnitSink' => 'unit/sink/ArcanistDefaultUnitSink.php',
     'ArcanistDefaultsConfigurationSource' => 'config/source/ArcanistDefaultsConfigurationSource.php',
     'ArcanistDeprecationXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistDeprecationXHPASTLinterRule.php',
     'ArcanistDeprecationXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistDeprecationXHPASTLinterRuleTestCase.php',
@@ -279,7 +279,7 @@ phutil_register_library_map(array(
     'ArcanistJSONLintRenderer' => 'lint/renderer/ArcanistJSONLintRenderer.php',
     'ArcanistJSONLinter' => 'lint/linter/ArcanistJSONLinter.php',
     'ArcanistJSONLinterTestCase' => 'lint/linter/__tests__/ArcanistJSONLinterTestCase.php',
-    'ArcanistJSONUnitFormatter' => 'unit/formatter/ArcanistJSONUnitFormatter.php',
+    'ArcanistJSONUnitSink' => 'unit/sink/ArcanistJSONUnitSink.php',
     'ArcanistJscsLinter' => 'lint/linter/ArcanistJscsLinter.php',
     'ArcanistJscsLinterTestCase' => 'lint/linter/__tests__/ArcanistJscsLinterTestCase.php',
     'ArcanistKeywordCasingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php',
@@ -473,9 +473,9 @@ phutil_register_library_map(array(
     'ArcanistUnexpectedReturnValueXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUnexpectedReturnValueXHPASTLinterRuleTestCase.php',
     'ArcanistUnitConsoleRenderer' => 'unit/renderer/ArcanistUnitConsoleRenderer.php',
     'ArcanistUnitEngine' => 'unit/engine/ArcanistUnitEngine.php',
-    'ArcanistUnitFormatter' => 'unit/formatter/ArcanistUnitFormatter.php',
     'ArcanistUnitOverseer' => 'unit/overseer/ArcanistUnitOverseer.php',
     'ArcanistUnitRenderer' => 'unit/renderer/ArcanistUnitRenderer.php',
+    'ArcanistUnitSink' => 'unit/sink/ArcanistUnitSink.php',
     'ArcanistUnitTestResult' => 'unit/ArcanistUnitTestResult.php',
     'ArcanistUnitTestResultTestCase' => 'unit/__tests__/ArcanistUnitTestResultTestCase.php',
     'ArcanistUnitTestableLintEngine' => 'lint/engine/ArcanistUnitTestableLintEngine.php',
@@ -1276,7 +1276,7 @@ phutil_register_library_map(array(
     'ArcanistDeclarationParenthesesXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistDefaultParametersXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistDefaultParametersXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
-    'ArcanistDefaultUnitFormatter' => 'ArcanistUnitFormatter',
+    'ArcanistDefaultUnitSink' => 'ArcanistUnitSink',
     'ArcanistDefaultsConfigurationSource' => 'ArcanistDictionaryConfigurationSource',
     'ArcanistDeprecationXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistDeprecationXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
@@ -1390,7 +1390,7 @@ phutil_register_library_map(array(
     'ArcanistJSONLintRenderer' => 'ArcanistLintRenderer',
     'ArcanistJSONLinter' => 'ArcanistLinter',
     'ArcanistJSONLinterTestCase' => 'ArcanistLinterTestCase',
-    'ArcanistJSONUnitFormatter' => 'ArcanistUnitFormatter',
+    'ArcanistJSONUnitSink' => 'ArcanistUnitSink',
     'ArcanistJscsLinter' => 'ArcanistExternalLinter',
     'ArcanistJscsLinterTestCase' => 'ArcanistExternalLinterTestCase',
     'ArcanistKeywordCasingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
@@ -1584,9 +1584,9 @@ phutil_register_library_map(array(
     'ArcanistUnexpectedReturnValueXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistUnitConsoleRenderer' => 'ArcanistUnitRenderer',
     'ArcanistUnitEngine' => 'Phobject',
-    'ArcanistUnitFormatter' => 'Phobject',
     'ArcanistUnitOverseer' => 'Phobject',
     'ArcanistUnitRenderer' => 'Phobject',
+    'ArcanistUnitSink' => 'Phobject',
     'ArcanistUnitTestResult' => 'Phobject',
     'ArcanistUnitTestResultTestCase' => 'PhutilTestCase',
     'ArcanistUnitTestableLintEngine' => 'ArcanistLintEngine',

src/unit/ArcanistUnitTestResult.php

@@ -175,13 +175,8 @@ final class ArcanistUnitTestResult extends Phobject {
   }
 
   public static function getAllResultCodes() {
-    return array(
+    $map = self::getResultCodeSpecs();
-      self::RESULT_PASS,
+    return array_keys($map);
-      self::RESULT_FAIL,
-      self::RESULT_SKIP,
-      self::RESULT_BROKEN,
-      self::RESULT_UNSOUND,
-    );
   }
 
   public static function getResultCodeName($result_code) {
@@ -205,31 +200,56 @@ final class ArcanistUnitTestResult extends Phobject {
     return idx($specs, $result_code);
   }
 
+  public function getANSIColor() {
+    $spec = $this->getResultMap();
+    return idx($spec, 'color.ansi', 'red');
+  }
+
+  public function getResultLabel() {
+    $spec = $this->getResultMap();
+    return idx($spec, 'label', $this->getResult());
+  }
+
+  private function getResultMap() {
+    $map = self::getResultCodeSpecs();
+    return idx($map, $this->getResult(), array());
+  }
+
   private static function getResultCodeSpecs() {
     return array(
       self::RESULT_PASS => array(
         'name' => pht('Pass'),
+        'label' => pht('PASS'),
+        'color.ansi' => 'green',
         'description' => pht(
           'The test passed.'),
       ),
       self::RESULT_FAIL => array(
         'name' => pht('Fail'),
+        'label' => pht('FAIL'),
+        'color.ansi' => 'red',
         'description' => pht(
           'The test failed.'),
       ),
       self::RESULT_SKIP => array(
         'name' => pht('Skip'),
+        'label' => pht('SKIP'),
+        'color.ansi' => 'cyan',
         'description' => pht(
           'The test was not executed.'),
       ),
       self::RESULT_BROKEN => array(
         'name' => pht('Broken'),
+        'label' => pht('BROKEN'),
+        'color.ansi' => 'red',
         'description' => pht(
           'The test failed in an abnormal or severe way. For example, the '.
           'harness crashed instead of reporting a failure.'),
       ),
       self::RESULT_UNSOUND => array(
         'name' => pht('Unsound'),
+        'label' => pht('UNSOUND'),
+        'color.ansi' => 'yellow',
         'description' => pht(
           'The test failed, but this change is probably not what broke it. '.
           'For example, it might have already been failing.'),
@@ -237,5 +257,15 @@ final class ArcanistUnitTestResult extends Phobject {
     );
   }
 
+  public function getDisplayName() {
+    $name = $this->getName();
+
+    $namespace = $this->getNamespace();
+    if (strlen($namespace)) {
+      $name = $namespace.'::'.$name;
+    }
+
+    return $name;
+  }
 
 }

src/unit/engine/ArcanistUnitEngine.php

@@ -56,14 +56,7 @@ abstract class ArcanistUnitEngine
   abstract public function runTests();
 
   final protected function didRunTests(array $tests) {
-    assert_instances_of($tests, 'ArcanistUnitTestResult');
+    return $this->getOverseer()->didRunTests($tests);
-
-    // TOOLSETS: Pass this stuff to result output so it can print progress or
-    // stream results.
-
-    foreach ($tests as $test) {
-      echo "Ran Test: ".$test->getNamespace().'::'.$test->getName()."\n";
-    }
   }
 
 }

src/unit/formatter/ArcanistDefaultUnitFormatter.php

@@ -1,9 +0,0 @@
-<?php
-
-final class ArcanistDefaultUnitFormatter
-  extends ArcanistUnitFormatter {
-
-  const FORMATTER_KEY = 'default';
-
-
-}

src/unit/formatter/ArcanistJSONUnitFormatter.php

@@ -1,9 +0,0 @@
-<?php
-
-final class ArcanistJSONUnitFormatter
-  extends ArcanistUnitFormatter {
-
-  const FORMATTER_KEY = 'json';
-
-
-}

src/unit/formatter/ArcanistUnitFormatter.php

@@ -1,17 +0,0 @@
-<?php
-
-abstract class ArcanistUnitFormatter
-  extends Phobject {
-
-  final public function getUnitFormatterKey() {
-    return $this->getPhobjectClassConstant('FORMATTER_KEY');
-  }
-
-  public static function getAllUnitFormatters() {
-    return id(new PhutilClassMapQuery())
-      ->setAncestorClass(__CLASS__)
-      ->setUniqueMethod('getUnitFormatterKey')
-      ->execute();
-  }
-
-}

src/unit/overseer/ArcanistUnitOverseer.php

@@ -5,9 +5,9 @@ final class ArcanistUnitOverseer
 
   private $directory;
   private $paths = array();
-  private $formatter;
+  private $sinks = array();
 
-  public function setPaths($paths) {
+  public function setPaths(array $paths) {
     $this->paths = $paths;
     return $this;
   }
@@ -16,13 +16,14 @@ final class ArcanistUnitOverseer
     return $this->paths;
   }
 
-  public function setFormatter(ArcanistUnitFormatter $formatter) {
+  public function setSinks(array $sinks) {
-    $this->formatter = $formatter;
+    assert_instances_of($sinks, 'ArcanistUnitSink');
+    $this->sinks = $sinks;
     return $this;
   }
 
-  public function getFormatter() {
+  public function getSinks() {
-    return $this->formatter;
+    return $this->sinks;
   }
 
   public function setDirectory($directory) {
@@ -50,9 +51,27 @@ final class ArcanistUnitOverseer
       }
     }
 
+    $this->didCompleteTests($results);
+
     return $results;
   }
 
+  public function didRunTests(array $tests) {
+    assert_instances_of($tests, 'ArcanistUnitTestResult');
+
+    foreach ($this->getSinks() as $sink) {
+      $sink->sinkPartialResults($tests);
+    }
+  }
+
+  private function didCompleteTests(array $tests) {
+    assert_instances_of($tests, 'ArcanistUnitTestResult');
+
+    foreach ($this->getSinks() as $sink) {
+      $sink->sinkFinalResults($tests);
+    }
+  }
+
   private function loadEngines() {
     $root = $this->getDirectory();
 

src/unit/sink/ArcanistDefaultUnitSink.php

@@ -0,0 +1,169 @@
+<?php
+
+final class ArcanistDefaultUnitSink
+  extends ArcanistUnitSink {
+
+  const SINKKEY = 'default';
+
+  private $lastUpdateTime;
+  private $buffer = array();
+
+  public function sinkPartialResults(array $results) {
+
+    // We want to show the user both regular progress reports and make sure
+    // that important results aren't scrolled off screen. We'll print a summary
+    // at the end so it's not critical that users can never miss important
+    // results, but they may (for example) want to ^C early if tests fail and
+    // their fate is sealed.
+
+
+    $failed = array();
+    foreach ($results as $result) {
+      $result_code = $result->getResult();
+      switch ($result_code) {
+        case ArcanistUnitTestResult::RESULT_PASS:
+        case ArcanistUnitTestResult::RESULT_SKIP:
+          break;
+        default:
+          $failed[] = $result;
+          break;
+      }
+    }
+
+    $now = microtime(true);
+
+    if (!$failed) {
+      if ($this->lastUpdateTime) {
+        $delay = 1;
+        if (($now - $this->lastUpdateTime) < $delay) {
+          $this->buffer[] = $results;
+          return;
+        }
+      }
+    }
+
+    $this->buffer[] = $results;
+    $results = array_mergev($this->buffer);
+    $this->buffer = array();
+
+    $pass_count = 0;
+    $skip_count = 0;
+    $failed = array();
+    foreach ($results as $result) {
+      $result_code = $result->getResult();
+      switch ($result_code) {
+        case ArcanistUnitTestResult::RESULT_PASS:
+          $pass_count++;
+          break;
+        case ArcanistUnitTestResult::RESULT_SKIP:
+          $skip_count++;
+          break;
+        default:
+          $failed[] = $result;
+          break;
+      }
+    }
+
+    if ($pass_count) {
+      echo tsprintf(
+        "%s\n",
+        pht('%s tests passed.', $pass_count));
+    }
+
+    if ($skip_count) {
+      echo tsprintf(
+        "%s\n",
+        pht('%s tests skipped.', $skip_count));
+    }
+
+    foreach ($failed as $result) {
+      echo $this->getDisplayForTest($result, false);
+    }
+
+    $this->lastUpdateTime = $now;
+
+    return $this;
+  }
+
+  public function sinkFinalResults(array $results) {
+
+    $passed = array();
+    $skipped = array();
+    $failed = array();
+    foreach ($results as $result) {
+      $result_code = $result->getResult();
+      switch ($result_code) {
+        case ArcanistUnitTestResult::RESULT_PASS:
+          $passed[] = $result;
+          break;
+        case ArcanistUnitTestResult::RESULT_SKIP:
+          $skipped[] = $result;
+          break;
+        default:
+          $failed[] = $result;
+          break;
+      }
+    }
+
+    echo tsprintf(
+      "%s\n",
+      pht('RESULT SUMMARY'));
+
+
+    if ($skipped) {
+      echo tsprintf(
+        "%s\n",
+        pht('SKIPPED TESTS'));
+
+      foreach ($skipped as $result) {
+        echo $this->getDisplayForTest($result);
+      }
+    }
+
+    if ($failed) {
+      echo tsprintf(
+        "%s\n",
+        pht('FAILED TESTS'));
+
+      foreach ($failed as $result) {
+        echo $this->getDisplayForTest($result);
+      }
+    }
+
+
+    echo tsprintf(
+      "**<bg:red> ~~~ %s </bg>**\n",
+      pht(
+        "%s PASSED * %s SKIPPED * %s FAILED/BROKEN/UNSTABLE",
+        phutil_count($passed),
+        phutil_count($skipped),
+        phutil_count($failed)));
+  }
+
+  private function getDisplayForTest(ArcanistUnitTestResult $result) {
+
+    $color = $result->getANSIColor();
+    $status = $result->getResultLabel();
+    $name = $result->getDisplayName();
+
+    // TOOLSETS: Restore timing information.
+    $timing = '    ';
+
+    $output = tsprintf(
+      "**<bg:".$color."> %s </bg>** %s %s\n",
+      $status,
+      $timing,
+      $name);
+
+    $user_data = $result->getUserData();
+    if (strlen($user_data)) {
+      $output = tsprintf(
+        "%s%B\n",
+        $output,
+        $user_data);
+    }
+
+    return $output;
+  }
+
+}

src/unit/sink/ArcanistJSONUnitSink.php

@@ -0,0 +1,9 @@
+<?php
+
+final class ArcanistJSONUnitSink
+  extends ArcanistUnitSink {
+
+  const SINKKEY = 'json';
+
+
+}

src/unit/sink/ArcanistUnitSink.php

@@ -0,0 +1,31 @@
+<?php
+
+abstract class ArcanistUnitSink
+  extends Phobject {
+
+  private $results;
+
+  final public function getUnitSinkKey() {
+    return $this->getPhobjectClassConstant('SINKKEY');
+  }
+
+  public static function getAllUnitSinks() {
+    return id(new PhutilClassMapQuery())
+      ->setAncestorClass(__CLASS__)
+      ->setUniqueMethod('getUnitSinkKey')
+      ->execute();
+  }
+
+  public function sinkPartialResults(array $results) {
+    return $this;
+  }
+
+  public function sinkFinalResults(array $results) {
+    return $this;
+  }
+
+  public function getOutput() {
+    return null;
+  }
+
+}

src/workflow/ArcanistUnitWorkflow.php

@@ -23,7 +23,7 @@ EOTEXT
     return array(
       $this->newWorkflowArgument('commit')
         ->setParameter('commit'),
-      $this->newWorkflowArgument('format')
+      $this->newWorkflowArgument('sink')
         ->setParameter('format'),
       $this->newWorkflowArgument('everything'),
       $this->newWorkflowArgument('paths')
@@ -51,32 +51,40 @@ EOTEXT
     // though it is "arc unit --everything", and ignoring the "--commit" flag
     // and "paths" arguments.
 
-    $formatter = $this->newUnitFormatter();
+    $sinks = array();
-    $overseer->setFormatter($formatter);
+    $sinks[] = $this->newUnitSink();
+    $overseer->setSinks($sinks);
 
     $overseer->execute();
 
+    foreach ($sinks as $sink) {
+      $result = $sink->getOutput();
+      if ($result !== null) {
+        echo $result;
+      }
+    }
+
     return 0;
   }
 
-  private function newUnitFormatter() {
+  private function newUnitSink() {
-    $formatters = ArcanistUnitFormatter::getAllUnitFormatters();
+    $sinks = ArcanistUnitSink::getAllUnitSinks();
-    $format_key = $this->getArgument('format');
+    $sink_key = $this->getArgument('sink');
-    if (!strlen($format_key)) {
+    if (!strlen($sink_key)) {
-      $format_key = ArcanistDefaultUnitFormatter::FORMATTER_KEY;
+      $sink_key = ArcanistDefaultUnitSink::SINKKEY;
     }
 
-    $formatter = idx($formatters, $format_key);
+    $sink = idx($sinks, $sink_key);
-    if (!$formatter) {
+    if (!$sink) {
       throw new ArcanistUsageException(
         pht(
-          'Unit test output format ("%s") is unknown. Supported formats '.
+          'Unit test output sink ("%s") is unknown. Supported sinks '.
           'are: %s.',
-          $format_key,
+          $sink_key,
-          implode(', ', array_keys($formatters))));
+          implode(', ', array_keys($sinks))));
     }
 
-    return $formatter;
+    return $sink;
   }
 
 }