Page MenuHomePhabricator
Files F14447030

D14466.id34986.diff
No OneTemporary
Actions

D14466.id34986.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -62,6 +62,7 @@
'ArcanistConfigurationDrivenUnitTestEngine' => 'unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php',
'ArcanistConfigurationManager' => 'configuration/ArcanistConfigurationManager.php',
'ArcanistConsoleLintRenderer' => 'lint/renderer/ArcanistConsoleLintRenderer.php',
+ 'ArcanistConsoleUnitRenderer' => 'unit/renderer/ArcanistConsoleUnitRenderer.php',
'ArcanistConstructorParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistConstructorParenthesesXHPASTLinterRule.php',
'ArcanistControlStatementSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistControlStatementSpacingXHPASTLinterRule.php',
'ArcanistCoverWorkflow' => 'workflow/ArcanistCoverWorkflow.php',
@@ -142,6 +143,7 @@
'ArcanistJSONLintRenderer' => 'lint/renderer/ArcanistJSONLintRenderer.php',
'ArcanistJSONLinter' => 'lint/linter/ArcanistJSONLinter.php',
'ArcanistJSONLinterTestCase' => 'lint/linter/__tests__/ArcanistJSONLinterTestCase.php',
+ 'ArcanistJSONUnitRenderer' => 'unit/renderer/ArcanistJSONUnitRenderer.php',
'ArcanistJscsLinter' => 'lint/linter/ArcanistJscsLinter.php',
'ArcanistJscsLinterTestCase' => 'lint/linter/__tests__/ArcanistJscsLinterTestCase.php',
'ArcanistKeywordCasingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php',
@@ -182,6 +184,7 @@
'ArcanistNoLintLinterTestCase' => 'lint/linter/__tests__/ArcanistNoLintLinterTestCase.php',
'ArcanistNoParentScopeXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistNoParentScopeXHPASTLinterRule.php',
'ArcanistNoneLintRenderer' => 'lint/renderer/ArcanistNoneLintRenderer.php',
+ 'ArcanistNoneUnitRenderer' => 'unit/renderer/ArcanistNoneUnitRenderer.php',
'ArcanistObjectOperatorSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistObjectOperatorSpacingXHPASTLinterRule.php',
'ArcanistPEP8Linter' => 'lint/linter/ArcanistPEP8Linter.php',
'ArcanistPEP8LinterTestCase' => 'lint/linter/__tests__/ArcanistPEP8LinterTestCase.php',
@@ -249,11 +252,11 @@
'ArcanistTodoCommentXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistTodoCommentXHPASTLinterRule.php',
'ArcanistTodoWorkflow' => 'workflow/ArcanistTodoWorkflow.php',
'ArcanistUSEnglishTranslation' => 'internationalization/ArcanistUSEnglishTranslation.php',
+ 'ArcanistUglyJSONUnitRenderer' => 'unit/renderer/ArcanistUglyJSONUnitRenderer.php',
'ArcanistUnableToParseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnableToParseXHPASTLinterRule.php',
'ArcanistUnaryPostfixExpressionSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnaryPostfixExpressionSpacingXHPASTLinterRule.php',
'ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRule.php',
'ArcanistUndeclaredVariableXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php',
- 'ArcanistUnitConsoleRenderer' => 'unit/renderer/ArcanistUnitConsoleRenderer.php',
'ArcanistUnitRenderer' => 'unit/renderer/ArcanistUnitRenderer.php',
'ArcanistUnitTestEngine' => 'unit/engine/ArcanistUnitTestEngine.php',
'ArcanistUnitTestResult' => 'unit/ArcanistUnitTestResult.php',
@@ -351,6 +354,7 @@
'ArcanistConfigurationDrivenUnitTestEngine' => 'ArcanistUnitTestEngine',
'ArcanistConfigurationManager' => 'Phobject',
'ArcanistConsoleLintRenderer' => 'ArcanistLintRenderer',
+ 'ArcanistConsoleUnitRenderer' => 'ArcanistUnitRenderer',
'ArcanistConstructorParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistControlStatementSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistCoverWorkflow' => 'ArcanistWorkflow',
@@ -431,6 +435,7 @@
'ArcanistJSONLintRenderer' => 'ArcanistLintRenderer',
'ArcanistJSONLinter' => 'ArcanistLinter',
'ArcanistJSONLinterTestCase' => 'ArcanistLinterTestCase',
+ 'ArcanistJSONUnitRenderer' => 'ArcanistUnitRenderer',
'ArcanistJscsLinter' => 'ArcanistExternalLinter',
'ArcanistJscsLinterTestCase' => 'ArcanistExternalLinterTestCase',
'ArcanistKeywordCasingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
@@ -471,6 +476,7 @@
'ArcanistNoLintLinterTestCase' => 'ArcanistLinterTestCase',
'ArcanistNoParentScopeXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistNoneLintRenderer' => 'ArcanistLintRenderer',
+ 'ArcanistNoneUnitRenderer' => 'ArcanistUnitRenderer',
'ArcanistObjectOperatorSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistPEP8Linter' => 'ArcanistExternalLinter',
'ArcanistPEP8LinterTestCase' => 'ArcanistExternalLinterTestCase',
@@ -538,11 +544,11 @@
'ArcanistTodoCommentXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistTodoWorkflow' => 'ArcanistWorkflow',
'ArcanistUSEnglishTranslation' => 'PhutilTranslation',
+ 'ArcanistUglyJSONUnitRenderer' => 'ArcanistUnitRenderer',
'ArcanistUnableToParseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistUnaryPostfixExpressionSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistUndeclaredVariableXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
- 'ArcanistUnitConsoleRenderer' => 'ArcanistUnitRenderer',
'ArcanistUnitRenderer' => 'Phobject',
'ArcanistUnitTestEngine' => 'Phobject',
'ArcanistUnitTestResult' => 'Phobject',
diff --git a/src/unit/renderer/ArcanistUnitConsoleRenderer.php b/src/unit/renderer/ArcanistConsoleUnitRenderer.php
rename from src/unit/renderer/ArcanistUnitConsoleRenderer.php
rename to src/unit/renderer/ArcanistConsoleUnitRenderer.php
--- a/src/unit/renderer/ArcanistUnitConsoleRenderer.php
+++ b/src/unit/renderer/ArcanistConsoleUnitRenderer.php
@@ -1,6 +1,10 @@
<?php
-final class ArcanistUnitConsoleRenderer extends ArcanistUnitRenderer {
+final class ArcanistConsoleUnitRenderer extends ArcanistUnitRenderer {
+
+ public function getId() {
+ return 'full';
+ }
public function renderUnitResult(ArcanistUnitTestResult $result) {
$result_code = $result->getResult();
diff --git a/src/unit/renderer/ArcanistJSONUnitRenderer.php b/src/unit/renderer/ArcanistJSONUnitRenderer.php
new file mode 100644
--- /dev/null
+++ b/src/unit/renderer/ArcanistJSONUnitRenderer.php
@@ -0,0 +1,27 @@
+<?php
+
+/**
+ * This Renderer uses a bit of hack since it doesn't render the JSON
+ * progessively. It uses the postamble to conver the results.
+ */
+final class ArcanistJSONUnitRenderer extends ArcanistUnitRenderer {
+
+ private $results = array();
+
+ public function getId() {
+ return 'json';
+ }
+
+ public function renderUnitResult(ArcanistUnitTestResult $result) {
+ array_push($this->results, $result);
+
+ return '';
+ }
+
+ public function renderPostamble() {
+ assert_instances_of($this->results, 'ArcanistUnitTestResult');
+ $data = array_values(mpull($this->results, 'toDictionary'));
+ return id(new PhutilJSON())->encodeFormatted($data);
+ }
+
+}
diff --git a/src/unit/renderer/ArcanistNoneUnitRenderer.php b/src/unit/renderer/ArcanistNoneUnitRenderer.php
new file mode 100644
--- /dev/null
+++ b/src/unit/renderer/ArcanistNoneUnitRenderer.php
@@ -0,0 +1,13 @@
+<?php
+
+final class ArcanistNoneUnitRenderer extends ArcanistUnitRenderer {
+
+ public function getId() {
+ return 'none';
+ }
+
+ public function renderUnitResult(ArcanistUnitTestResult $result) {
+ return '';
+ }
+
+}
diff --git a/src/unit/renderer/ArcanistUglyJSONUnitRenderer.php b/src/unit/renderer/ArcanistUglyJSONUnitRenderer.php
new file mode 100644
--- /dev/null
+++ b/src/unit/renderer/ArcanistUglyJSONUnitRenderer.php
@@ -0,0 +1,27 @@
+<?php
+
+/**
+ * This Renderer uses a bit of hack since it doesn't render the JSON
+ * progessively. It uses the postamble to conver the results.
+ */
+final class ArcanistUglyJSONUnitRenderer extends ArcanistUnitRenderer {
+
+ private $results = array();
+
+ public function getId() {
+ return 'ugly';
+ }
+
+ public function renderUnitResult(ArcanistUnitTestResult $result) {
+ array_push($this->results, $result);
+
+ return '';
+ }
+
+ public function renderPostamble() {
+ assert_instances_of($this->results, 'ArcanistUnitTestResult');
+ $data = array_values(mpull($this->results, 'toDictionary'));
+ return phutil_json_encode($data);
+ }
+
+}
diff --git a/src/unit/renderer/ArcanistUnitRenderer.php b/src/unit/renderer/ArcanistUnitRenderer.php
--- a/src/unit/renderer/ArcanistUnitRenderer.php
+++ b/src/unit/renderer/ArcanistUnitRenderer.php
@@ -1,5 +1,33 @@
<?php
abstract class ArcanistUnitRenderer extends Phobject {
+
+ final public static function loadAllRenderers() {
+ return id(new PhutilClassMapQuery())
+ ->setAncestorClass(__CLASS__)
+ ->setUniqueMethod('getId')
+ ->execute();
+ }
+
+ final public static function getRendererById($id) {
+ $renderers = self::loadAllRenderers();
+ if ($renderers[$id]) {
+ return $renderers[$id];
+ } else {
+ throw new Exception('Renderer, '.$id.', does not exist.');
+ }
+ }
+
+ abstract public function getId();
+
+ public function renderPreamble() {
+ return '';
+ }
+
abstract public function renderUnitResult(ArcanistUnitTestResult $result);
+
+ public function renderPostamble() {
+ return '';
+ }
+
}
diff --git a/src/workflow/ArcanistUnitWorkflow.php b/src/workflow/ArcanistUnitWorkflow.php
--- a/src/workflow/ArcanistUnitWorkflow.php
+++ b/src/workflow/ArcanistUnitWorkflow.php
@@ -69,9 +69,6 @@
'Show a detailed coverage report on the CLI. Implies %s.',
'--coverage'),
),
- 'json' => array(
- 'help' => pht('Report results in JSON format.'),
- ),
'output' => array(
'param' => 'format',
'help' => pht(
@@ -79,10 +76,6 @@
"With 'json', report results in JSON format. ".
"With 'ugly', use uglier (but more efficient) JSON formatting. ".
"With 'none', don't print results."),
- 'conflicts' => array(
- 'json' => pht('Only one output format allowed'),
- 'ugly' => pht('Only one output format allowed'),
- ),
),
'target' => array(
'param' => 'phid',
@@ -96,11 +89,6 @@
'rev' => pht('%s runs all tests.', '--everything'),
),
),
- 'ugly' => array(
- 'help' => pht(
- 'With %s, use uglier (but more efficient) formatting.',
- '--json'),
- ),
'*' => 'paths',
);
}
@@ -153,7 +141,9 @@
}
$this->engine->setArguments($this->getPassthruArgumentsAsMap('unit'));
- $renderer = new ArcanistUnitConsoleRenderer();
+ // Get renderer based on '--output' argument
+ $renderer = ArcanistUnitRenderer::getRendererById($this->getOutputFormat());
+
$this->engine->setRenderer($renderer);
$enable_coverage = null; // Means "default".
@@ -165,7 +155,7 @@
}
$this->engine->setEnableCoverage($enable_coverage);
- // Enable possible async tests only for 'arc diff' not 'arc unit'
+ // Enable possible async tests only for 'arc diff' not 'arc unit'.
if ($this->getParentWorkflow()) {
$this->engine->setEnableAsyncTests(true);
} else {
@@ -178,24 +168,30 @@
$this->testResults = $results;
- $console = PhutilConsole::getConsole();
-
- $output_format = $this->getOutputFormat();
-
- if ($output_format !== 'full') {
- $console->disableOut();
- }
-
$unresolved = array();
$coverage = array();
+ $overall_result = self::RESULT_OKAY;
+ // Gather unresolved tests and coverage while determining the
+ // 'overall_result'.
+
foreach ($results as $result) {
$result_code = $result->getResult();
- if ($this->engine->shouldEchoTestResults()) {
- $console->writeOut('%s', $renderer->renderUnitResult($result));
- }
if ($result_code != ArcanistUnitTestResult::RESULT_PASS) {
$unresolved[] = $result;
}
+
+ // Note: This means as soon as a RESULT_FAIL/BROKEN/UNSOUND are found,
+ // they persist as the '$overall_result'.
+ if ($overall_result != self::RESULT_FAIL) {
+ $result_code = $result->getResult();
+ if ($result_code == ArcanistUnitTestResult::RESULT_FAIL ||
+ $result_code == ArcanistUnitTestResult::RESULT_BROKEN) {
+ $overall_result = self::RESULT_FAIL;
+ } else if ($result_code == ArcanistUnitTestResult::RESULT_UNSOUND) {
+ $overall_result = self::RESULT_UNSOUND;
+ }
+ }
+
if ($result->getCoverage()) {
foreach ($result->getCoverage() as $file => $report) {
$coverage[$file][] = $report;
@@ -203,6 +199,25 @@
}
}
+ $this->unresolvedTests = $unresolved;
+
+ // Set up output methods.
+ $console = PhutilConsole::getConsole();
+
+ // Write out Unit Test results to console or file.
+ if ($this->engine->shouldEchoTestResults()) {
+ $preamble = $renderer->renderPreamble();
+ $console->writeOut('%s', $preamble);
+
+ foreach ($results as $result) {
+ $console->writeOut('%s', $renderer->renderUnitResult($result));
+ }
+
+ $postamble = $renderer->renderPostamble();
+ $console->writeOut('%s', $postamble);
+ }
+
+ // Process and render coverage results.
if ($coverage) {
$file_coverage = array_fill_keys(
$paths,
@@ -243,41 +258,6 @@
}
}
- $this->unresolvedTests = $unresolved;
-
- $overall_result = self::RESULT_OKAY;
- foreach ($results as $result) {
- $result_code = $result->getResult();
- if ($result_code == ArcanistUnitTestResult::RESULT_FAIL ||
- $result_code == ArcanistUnitTestResult::RESULT_BROKEN) {
- $overall_result = self::RESULT_FAIL;
- break;
- } else if ($result_code == ArcanistUnitTestResult::RESULT_UNSOUND) {
- $overall_result = self::RESULT_UNSOUND;
- }
- }
-
- if ($output_format !== 'full') {
- $console->enableOut();
- }
- $data = array_values(mpull($results, 'toDictionary'));
- switch ($output_format) {
- case 'ugly':
- $console->writeOut('%s', json_encode($data));
- break;
- case 'json':
- $json = new PhutilJSON();
- $console->writeOut('%s', $json->encodeFormatted($data));
- break;
- case 'full':
- // already printed
- break;
- case 'none':
- // do nothing
- break;
- }
-
-
$target_phid = $this->getArgument('target');
if ($target_phid) {
$this->uploadTestResults($target_phid, $overall_result, $results);
@@ -336,12 +316,6 @@
}
private function getOutputFormat() {
- if ($this->getArgument('ugly')) {
- return 'ugly';
- }
- if ($this->getArgument('json')) {
- return 'json';
- }
$format = $this->getArgument('output');
$known_formats = array(
'none' => 'none',

File Metadata

Mime Type
text/plain
Expires
Fri, Dec 27, 1:59 PM (9 h, 8 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6932019
Default Alt Text
D14466.id34986.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility