D19710.diff
No OneTemporary
Actions

Size

41 KB

Referenced Files

None

Subscribers

None

D19710.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
	@@ -142,7 +142,6 @@
	'ArcanistConduitException' => 'conduit/ArcanistConduitException.php',
	'ArcanistConfigOption' => 'config/option/ArcanistConfigOption.php',
	'ArcanistConfigurationDrivenLintEngine' => 'lint/engine/ArcanistConfigurationDrivenLintEngine.php',
	- 'ArcanistConfigurationDrivenUnitTestEngine' => 'unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php',
	'ArcanistConfigurationEngine' => 'config/ArcanistConfigurationEngine.php',
	'ArcanistConfigurationEngineExtension' => 'config/ArcanistConfigurationEngineExtension.php',
	'ArcanistConfigurationManager' => 'configuration/ArcanistConfigurationManager.php',
	@@ -166,6 +165,7 @@
	'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',
	'ArcanistDefaultsConfigurationSource' => 'config/source/ArcanistDefaultsConfigurationSource.php',
	'ArcanistDeprecationXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistDeprecationXHPASTLinterRule.php',
	'ArcanistDeprecationXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistDeprecationXHPASTLinterRuleTestCase.php',
	@@ -279,6 +279,7 @@
	'ArcanistJSONLintRenderer' => 'lint/renderer/ArcanistJSONLintRenderer.php',
	'ArcanistJSONLinter' => 'lint/linter/ArcanistJSONLinter.php',
	'ArcanistJSONLinterTestCase' => 'lint/linter/__tests__/ArcanistJSONLinterTestCase.php',
	+ 'ArcanistJSONUnitFormatter' => 'unit/formatter/ArcanistJSONUnitFormatter.php',
	'ArcanistJscsLinter' => 'lint/linter/ArcanistJscsLinter.php',
	'ArcanistJscsLinterTestCase' => 'lint/linter/__tests__/ArcanistJscsLinterTestCase.php',
	'ArcanistKeywordCasingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php',
	@@ -471,8 +472,10 @@
	'ArcanistUnexpectedReturnValueXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnexpectedReturnValueXHPASTLinterRule.php',
	'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',
	- 'ArcanistUnitTestEngine' => 'unit/engine/ArcanistUnitTestEngine.php',
	'ArcanistUnitTestResult' => 'unit/ArcanistUnitTestResult.php',
	'ArcanistUnitTestResultTestCase' => 'unit/__tests__/ArcanistUnitTestResultTestCase.php',
	'ArcanistUnitTestableLintEngine' => 'lint/engine/ArcanistUnitTestableLintEngine.php',
	@@ -947,7 +950,7 @@
	'PhutilUSEnglishLocale' => 'internationalization/locales/PhutilUSEnglishLocale.php',
	'PhutilUTF8StringTruncator' => 'utils/PhutilUTF8StringTruncator.php',
	'PhutilUTF8TestCase' => 'utils/__tests__/PhutilUTF8TestCase.php',
	- 'PhutilUnitTestEngine' => 'unit/engine/PhutilUnitTestEngine.php',
	+ 'PhutilUnitEngine' => 'unit/engine/PhutilUnitEngine.php',
	'PhutilUnitTestEngineTestCase' => 'unit/engine/__tests__/PhutilUnitTestEngineTestCase.php',
	'PhutilUnknownSymbolParserGeneratorException' => 'parser/generator/exception/PhutilUnknownSymbolParserGeneratorException.php',
	'PhutilUnreachableRuleParserGeneratorException' => 'parser/generator/exception/PhutilUnreachableRuleParserGeneratorException.php',
	@@ -1250,7 +1253,6 @@
	'ArcanistConduitException' => 'Exception',
	'ArcanistConfigOption' => 'Phobject',
	'ArcanistConfigurationDrivenLintEngine' => 'ArcanistLintEngine',
	- 'ArcanistConfigurationDrivenUnitTestEngine' => 'ArcanistUnitTestEngine',
	'ArcanistConfigurationEngine' => 'Phobject',
	'ArcanistConfigurationEngineExtension' => 'Phobject',
	'ArcanistConfigurationManager' => 'Phobject',
	@@ -1274,6 +1276,7 @@
	'ArcanistDeclarationParenthesesXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
	'ArcanistDefaultParametersXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
	'ArcanistDefaultParametersXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
	+ 'ArcanistDefaultUnitFormatter' => 'ArcanistUnitFormatter',
	'ArcanistDefaultsConfigurationSource' => 'ArcanistDictionaryConfigurationSource',
	'ArcanistDeprecationXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
	'ArcanistDeprecationXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
	@@ -1315,7 +1318,7 @@
	'ArcanistExtractUseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
	'ArcanistExtractUseXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
	'ArcanistFeatureWorkflow' => 'ArcanistWorkflow',
	- 'ArcanistFileConfigurationSource' => 'ArcanistConfigurationSource',
	+ 'ArcanistFileConfigurationSource' => 'ArcanistFilesystemConfigurationSource',
	'ArcanistFileDataRef' => 'Phobject',
	'ArcanistFileUploader' => 'Phobject',
	'ArcanistFilenameLinter' => 'ArcanistLinter',
	@@ -1387,6 +1390,7 @@
	'ArcanistJSONLintRenderer' => 'ArcanistLintRenderer',
	'ArcanistJSONLinter' => 'ArcanistLinter',
	'ArcanistJSONLinterTestCase' => 'ArcanistLinterTestCase',
	+ 'ArcanistJSONUnitFormatter' => 'ArcanistUnitFormatter',
	'ArcanistJscsLinter' => 'ArcanistExternalLinter',
	'ArcanistJscsLinterTestCase' => 'ArcanistExternalLinterTestCase',
	'ArcanistKeywordCasingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
	@@ -1579,8 +1583,10 @@
	'ArcanistUnexpectedReturnValueXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
	'ArcanistUnexpectedReturnValueXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
	'ArcanistUnitConsoleRenderer' => 'ArcanistUnitRenderer',
	+ 'ArcanistUnitEngine' => 'Phobject',
	+ 'ArcanistUnitFormatter' => 'Phobject',
	+ 'ArcanistUnitOverseer' => 'Phobject',
	'ArcanistUnitRenderer' => 'Phobject',
	- 'ArcanistUnitTestEngine' => 'Phobject',
	'ArcanistUnitTestResult' => 'Phobject',
	'ArcanistUnitTestResultTestCase' => 'PhutilTestCase',
	'ArcanistUnitTestableLintEngine' => 'ArcanistLintEngine',
	@@ -2079,7 +2085,7 @@
	'PhutilUSEnglishLocale' => 'PhutilLocale',
	'PhutilUTF8StringTruncator' => 'Phobject',
	'PhutilUTF8TestCase' => 'PhutilTestCase',
	- 'PhutilUnitTestEngine' => 'ArcanistUnitTestEngine',
	+ 'PhutilUnitEngine' => 'ArcanistUnitEngine',
	'PhutilUnitTestEngineTestCase' => 'PhutilTestCase',
	'PhutilUnknownSymbolParserGeneratorException' => 'PhutilParserGeneratorException',
	'PhutilUnreachableRuleParserGeneratorException' => 'PhutilParserGeneratorException',
	diff --git a/src/toolset/ArcanistWorkflow.php b/src/toolset/ArcanistWorkflow.php
	--- a/src/toolset/ArcanistWorkflow.php
	+++ b/src/toolset/ArcanistWorkflow.php
	@@ -263,4 +263,8 @@
	return clone $prompt;
	}

	+ protected function getWorkingCopy() {
	+ return $this->getConfigurationEngine()->getWorkingCopy();
	+ }
	+
	}
	diff --git a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
	deleted file mode 100644
	--- a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
	+++ /dev/null
	@@ -1,220 +0,0 @@
	-<?php
	-
	-final class ArcanistConfigurationDrivenUnitTestEngine
	- extends ArcanistUnitTestEngine {
	-
	- protected function supportsRunAllTests() {
	- $engines = $this->buildTestEngines();
	-
	- foreach ($engines as $engine) {
	- if ($engine->supportsRunAllTests()) {
	- return true;
	- }
	- }
	-
	- return false;
	- }
	-
	- public function buildTestEngines() {
	- $working_copy = $this->getWorkingCopy();
	- $config_path = $working_copy->getProjectPath('.arcunit');
	-
	- if (!Filesystem::pathExists($config_path)) {
	- throw new ArcanistUsageException(
	- pht(
	- "Unable to find '%s' file to configure test engines. Create an ".
	- "'%s' file in the root directory of the working copy.",
	- '.arcunit',
	- '.arcunit'));
	- }
	-
	- $data = Filesystem::readFile($config_path);
	- $config = null;
	- try {
	- $config = phutil_json_decode($data);
	- } catch (PhutilJSONParserException $ex) {
	- throw new PhutilProxyException(
	- pht(
	- "Expected '%s' file to be a valid JSON file, but ".
	- "failed to decode '%s'.",
	- '.arcunit',
	- $config_path),
	- $ex);
	- }
	-
	- $test_engines = $this->loadAvailableTestEngines();
	-
	- try {
	- PhutilTypeSpec::checkMap(
	- $config,
	- array(
	- 'engines' => 'map<string, map<string, wild>>',
	- ));
	- } catch (PhutilTypeCheckException $ex) {
	- throw new PhutilProxyException(
	- pht("Error in parsing '%s' file.", $config_path),
	- $ex);
	- }
	-
	- $built_test_engines = array();
	- $all_paths = $this->getPaths();
	-
	- foreach ($config['engines'] as $name => $spec) {
	- $type = idx($spec, 'type');
	-
	- if ($type !== null) {
	- if (empty($test_engines[$type])) {
	- throw new ArcanistUsageException(
	- pht(
	- "Test engine '%s' specifies invalid type '%s'. ".
	- "Available test engines are: %s.",
	- $name,
	- $type,
	- implode(', ', array_keys($test_engines))));
	- }
	-
	- $test_engine = clone $test_engines[$type];
	- } else {
	- // We'll raise an error below about the invalid "type" key.
	- // TODO: Can we just do the type check first, and simplify this a bit?
	- $test_engine = null;
	- }
	-
	- try {
	- PhutilTypeSpec::checkMap(
	- $spec,
	- array(
	- 'type' => 'string',
	- 'include' => 'optional regex \| list<regex>',
	- 'exclude' => 'optional regex \| list<regex>',
	- ));
	- } catch (PhutilTypeCheckException $ex) {
	- throw new PhutilProxyException(
	- pht(
	- "Error in parsing '%s' file, for test engine '%s'.",
	- '.arcunit',
	- $name),
	- $ex);
	- }
	-
	- if ($all_paths) {
	- $include = (array)idx($spec, 'include', array());
	- $exclude = (array)idx($spec, 'exclude', array());
	- $paths = $this->matchPaths(
	- $all_paths,
	- $include,
	- $exclude);
	-
	- $test_engine->setPaths($paths);
	- }
	-
	- $built_test_engines[] = $test_engine;
	- }
	-
	- return $built_test_engines;
	- }
	-
	- public function run() {
	- $renderer = $this->renderer;
	- $this->setRenderer(null);
	-
	- $paths = $this->getPaths();
	-
	- // If we are running with `--everything` then `$paths` will be `null`.
	- if (!$paths) {
	- $paths = array();
	- }
	-
	- $engines = $this->buildTestEngines();
	- $all_results = array();
	- $exceptions = array();
	-
	- foreach ($engines as $engine) {
	- $engine
	- ->setWorkingCopy($this->getWorkingCopy())
	- ->setEnableCoverage($this->getEnableCoverage())
	- ->setConfigurationManager($this->getConfigurationManager())
	- ->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`
	- // does not really mean `--everything`.
	- if ($engine->supportsRunAllTests()) {
	- $engine->setRunAllTests($this->getRunAllTests());
	- }
	-
	- try {
	- // TODO: Type check the results.
	- $results = $engine->run();
	- $all_results[] = $results;
	-
	- 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()) {
	- echo $renderer->renderUnitResult($result);
	- }
	- }
	- } catch (ArcanistNoEffectException $ex) {
	- $exceptions[] = $ex;
	- }
	- }
	-
	- 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($all_results);
	- }
	-
	- public function shouldEchoTestResults() {
	- return false;
	- }
	-
	- private function loadAvailableTestEngines() {
	- return id(new PhutilClassMapQuery())
	- ->setAncestorClass('ArcanistUnitTestEngine')
	- ->setUniqueMethod('getEngineConfigurationName', true)
	- ->execute();
	- }
	-
	- /**
	- * TODO: This is copied from @{class:ArcanistConfigurationDrivenLintEngine}.
	- */
	- private function matchPaths(array $paths, array $include, array $exclude) {
	- $match = array();
	-
	- foreach ($paths as $path) {
	- $keep = false;
	- if (!$include) {
	- $keep = true;
	- } else {
	- foreach ($include as $rule) {
	- if (preg_match($rule, $path)) {
	- $keep = true;
	- break;
	- }
	- }
	- }
	-
	- if (!$keep) {
	- continue;
	- }
	-
	- if ($exclude) {
	- foreach ($exclude as $rule) {
	- if (preg_match($rule, $path)) {
	- continue 2;
	- }
	- }
	- }
	-
	- $match[] = $path;
	- }
	-
	- return $match;
	- }
	-
	-}
	diff --git a/src/unit/engine/ArcanistUnitEngine.php b/src/unit/engine/ArcanistUnitEngine.php
	new file mode 100644
	--- /dev/null
	+++ b/src/unit/engine/ArcanistUnitEngine.php
	@@ -0,0 +1,69 @@
	+<?php
	+
	+abstract class ArcanistUnitEngine
	+ extends Phobject {
	+
	+ private $overseer;
	+ private $includePaths = array();
	+ private $excludePaths = array();
	+
	+ final public function setIncludePaths(array $include_paths) {
	+ $this->includePaths = $include_paths;
	+ return $this;
	+ }
	+
	+ final public function getIncludePaths() {
	+ return $this->includePaths;
	+ }
	+
	+ final public function setExcludePaths(array $exclude_paths) {
	+ $this->excludePaths = $exclude_paths;
	+ return $this;
	+ }
	+
	+ final public function getExcludePaths() {
	+ return $this->excludePaths;
	+ }
	+
	+ final public function getUnitEngineType() {
	+ return $this->getPhobjectClassConstant('ENGINETYPE');
	+ }
	+
	+ final public function getPath($to_file = null) {
	+ return Filesystem::concatenatePaths(
	+ array(
	+ $this->getOverseer()->getDirectory(),
	+ $to_file,
	+ ));
	+ }
	+
	+ final public function setOverseer(ArcanistUnitOverseer $overseer) {
	+ $this->overseer = $overseer;
	+ return $this;
	+ }
	+
	+ final public function getOverseer() {
	+ return $this->overseer;
	+ }
	+
	+ public static function getAllUnitEngines() {
	+ return id(new PhutilClassMapQuery())
	+ ->setAncestorClass(__CLASS__)
	+ ->setUniqueMethod('getUnitEngineType')
	+ ->execute();
	+ }
	+
	+ abstract public function runTests();
	+
	+ final protected function didRunTests(array $tests) {
	+ assert_instances_of($tests, 'ArcanistUnitTestResult');
	+
	+ // 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";
	+ }
	+ }
	+
	+}
	\ No newline at end of file
	diff --git a/src/unit/engine/ArcanistUnitTestEngine.php b/src/unit/engine/ArcanistUnitTestEngine.php
	deleted file mode 100644
	--- a/src/unit/engine/ArcanistUnitTestEngine.php
	+++ /dev/null
	@@ -1,98 +0,0 @@
	-<?php
	-
	-/**
	- * Manages unit test execution.
	- */
	-abstract class ArcanistUnitTestEngine extends Phobject {
	-
	- private $workingCopy;
	- private $paths = array();
	- private $enableCoverage;
	- private $runAllTests;
	- private $configurationManager;
	- protected $renderer;
	-
	- final public function __construct() {}
	-
	- public function getEngineConfigurationName() {
	- return null;
	- }
	-
	- final public function setRunAllTests($run_all_tests) {
	- if (!$this->supportsRunAllTests() && $run_all_tests) {
	- throw new Exception(
	- pht(
	- "Engine '%s' does not support %s.",
	- get_class($this),
	- '--everything'));
	- }
	-
	- $this->runAllTests = $run_all_tests;
	- return $this;
	- }
	-
	- final public function getRunAllTests() {
	- return $this->runAllTests;
	- }
	-
	- protected function supportsRunAllTests() {
	- return false;
	- }
	-
	- final public function setConfigurationManager(
	- ArcanistConfigurationManager $configuration_manager) {
	- $this->configurationManager = $configuration_manager;
	- return $this;
	- }
	-
	- final public function getConfigurationManager() {
	- return $this->configurationManager;
	- }
	-
	- final public function setWorkingCopy(
	- ArcanistWorkingCopyIdentity $working_copy) {
	-
	- $this->workingCopy = $working_copy;
	- return $this;
	- }
	-
	- final public function getWorkingCopy() {
	- return $this->workingCopy;
	- }
	-
	- final public function setPaths(array $paths) {
	- $this->paths = $paths;
	- return $this;
	- }
	-
	- final public function getPaths() {
	- return $this->paths;
	- }
	-
	- final public function setEnableCoverage($enable_coverage) {
	- $this->enableCoverage = $enable_coverage;
	- return $this;
	- }
	-
	- final public function getEnableCoverage() {
	- return $this->enableCoverage;
	- }
	-
	- final public function setRenderer(ArcanistUnitRenderer $renderer = null) {
	- $this->renderer = $renderer;
	- return $this;
	- }
	-
	- abstract public function run();
	-
	- /**
	- * Modify the return value of this function in the child class, if you do
	- * not need to echo the test results after all the tests have been run. This
	- * is the case for example when the child class prints the tests results
	- * while the tests are running.
	- */
	- public function shouldEchoTestResults() {
	- return true;
	- }
	-
	-}
	diff --git a/src/unit/engine/PhutilUnitTestEngine.php b/src/unit/engine/PhutilUnitEngine.php
	rename from src/unit/engine/PhutilUnitTestEngine.php
	rename to src/unit/engine/PhutilUnitEngine.php
	--- a/src/unit/engine/PhutilUnitTestEngine.php
	+++ b/src/unit/engine/PhutilUnitEngine.php
	@@ -1,62 +1,16 @@
	<?php

	-/**
	- * Very basic unit test engine which runs libphutil tests.
	- */
	-final class PhutilUnitTestEngine extends ArcanistUnitTestEngine {
	+final class PhutilUnitEngine
	+ extends ArcanistUnitEngine {

	- public function getEngineConfigurationName() {
	- return 'phutil';
	- }
	-
	- protected function supportsRunAllTests() {
	- return true;
	- }
	-
	- public function run() {
	- if ($this->getRunAllTests()) {
	- $run_tests = $this->getAllTests();
	- } else {
	- $run_tests = $this->getTestsForPaths();
	- }
	+ const ENGINETYPE = 'phutil';

	- if (!$run_tests) {
	- throw new ArcanistNoEffectException(pht('No tests to run.'));
	- }
	-
	- $enable_coverage = $this->getEnableCoverage();
	-
	- if ($enable_coverage !== false) {
	- if (!function_exists('xdebug_start_code_coverage')) {
	- if ($enable_coverage === true) {
	- throw new ArcanistUsageException(
	- pht(
	- 'You specified %s but %s is not available, so '.
	- 'coverage can not be enabled for %s.',
	- '--coverage',
	- 'XDebug',
	- __CLASS__));
	- }
	- } else {
	- $enable_coverage = true;
	- }
	- }
	+ public function runTests() {
	+ $run_tests = $this->getAllTests();

	$test_cases = array();
	-
	foreach ($run_tests as $test_class) {
	- $test_case = newv($test_class, array())
	- ->setEnableCoverage($enable_coverage)
	- ->setWorkingCopy($this->getWorkingCopy());
	-
	- if ($this->getPaths()) {
	- $test_case->setPaths($this->getPaths());
	- }
	-
	- if ($this->renderer) {
	- $test_case->setRenderer($this->renderer);
	- }
	-
	+ $test_case = newv($test_class, array());
	$test_cases[] = $test_case;
	}

	@@ -66,7 +20,11 @@

	$results = array();
	foreach ($test_cases as $test_case) {
	- $results[] = $test_case->run();
	+ $result_list = $test_case->run();
	+
	+ $this->didRunTests($result_list);
	+
	+ $results[] = $result_list;
	}
	$results = array_mergev($results);

	@@ -78,7 +36,7 @@
	}

	private function getAllTests() {
	- $project_root = $this->getWorkingCopy()->getProjectRoot();
	+ $project_root = $this->getPath();

	$symbols = id(new PhutilSymbolLoader())
	->setType('class')
	diff --git a/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php b/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php
	--- a/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php
	+++ b/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php
	@@ -76,8 +76,7 @@
	$failed = 0;
	$skipped = 0;

	- $test_case = id(new PhutilTestCaseTestCase())
	- ->setWorkingCopy($this->getWorkingCopy());
	+ $test_case = new PhutilTestCaseTestCase();

	foreach ($test_case->run() as $result) {
	if ($result->getResult() == ArcanistUnitTestResult::RESULT_FAIL) {
	@@ -161,8 +160,7 @@
	),
	);

	- $test_engine = id(new PhutilUnitTestEngine())
	- ->setWorkingCopy($this->getWorkingCopy());
	+ $test_engine = new PhutilUnitTestEngine();

	$library = phutil_get_current_library_name();
	$library_root = phutil_get_library_root($library);
	diff --git a/src/unit/engine/phutil/PhutilTestCase.php b/src/unit/engine/phutil/PhutilTestCase.php
	--- a/src/unit/engine/phutil/PhutilTestCase.php
	+++ b/src/unit/engine/phutil/PhutilTestCase.php
	@@ -651,6 +651,9 @@
	}

	final protected function getLink($method) {
	+ // TOOLSETS: Restore this.
	+ return null;
	+
	$base_uri = $this
	->getWorkingCopy()
	->getProjectConfig('phabricator.uri');
	diff --git a/src/unit/formatter/ArcanistDefaultUnitFormatter.php b/src/unit/formatter/ArcanistDefaultUnitFormatter.php
	new file mode 100644
	--- /dev/null
	+++ b/src/unit/formatter/ArcanistDefaultUnitFormatter.php
	@@ -0,0 +1,9 @@
	+<?php
	+
	+final class ArcanistDefaultUnitFormatter
	+ extends ArcanistUnitFormatter {
	+
	+ const FORMATTER_KEY = 'default';
	+
	+
	+}
	diff --git a/src/unit/formatter/ArcanistJSONUnitFormatter.php b/src/unit/formatter/ArcanistJSONUnitFormatter.php
	new file mode 100644
	--- /dev/null
	+++ b/src/unit/formatter/ArcanistJSONUnitFormatter.php
	@@ -0,0 +1,9 @@
	+<?php
	+
	+final class ArcanistJSONUnitFormatter
	+ extends ArcanistUnitFormatter {
	+
	+ const FORMATTER_KEY = 'json';
	+
	+
	+}
	diff --git a/src/unit/formatter/ArcanistUnitFormatter.php b/src/unit/formatter/ArcanistUnitFormatter.php
	new file mode 100644
	--- /dev/null
	+++ b/src/unit/formatter/ArcanistUnitFormatter.php
	@@ -0,0 +1,17 @@
	+<?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();
	+ }
	+
	+}
	diff --git a/src/unit/overseer/ArcanistUnitOverseer.php b/src/unit/overseer/ArcanistUnitOverseer.php
	new file mode 100644
	--- /dev/null
	+++ b/src/unit/overseer/ArcanistUnitOverseer.php
	@@ -0,0 +1,153 @@
	+<?php
	+
	+final class ArcanistUnitOverseer
	+ extends Phobject {
	+
	+ private $directory;
	+ private $paths = array();
	+ private $formatter;
	+
	+ public function setPaths($paths) {
	+ $this->paths = $paths;
	+ return $this;
	+ }
	+
	+ public function getPaths() {
	+ return $this->paths;
	+ }
	+
	+ public function setFormatter(ArcanistUnitFormatter $formatter) {
	+ $this->formatter = $formatter;
	+ return $this;
	+ }
	+
	+ public function getFormatter() {
	+ return $this->formatter;
	+ }
	+
	+ public function setDirectory($directory) {
	+ $this->directory = $directory;
	+ return $this;
	+ }
	+
	+ public function getDirectory() {
	+ return $this->directory;
	+ }
	+
	+ public function execute() {
	+ $engines = $this->loadEngines();
	+
	+ foreach ($engines as $engine) {
	+ $engine->setOverseer($this);
	+ }
	+
	+ $results = array();
	+
	+ foreach ($engines as $engine) {
	+ $tests = $engine->runTests();
	+ foreach ($tests as $test) {
	+ $results[] = $test;
	+ }
	+ }
	+
	+ return $results;
	+ }
	+
	+ private function loadEngines() {
	+ $root = $this->getDirectory();
	+
	+ $arcunit_path = Filesystem::concatenatePaths(array($root, '.arcunit'));
	+ $arcunit_display = Filesystem::readablePath($arcunit_path);
	+
	+ if (!Filesystem::pathExists($arcunit_path)) {
	+ throw new Exception(
	+ pht(
	+ 'No ".arcunit" file exists at path "%s". Create an ".arcunit" file '.
	+ 'to define how "arc unit" should run tests.',
	+ $arcunit_display));
	+ }
	+
	+ try {
	+ $data = Filesystem::readFile($arcunit_path);
	+ } catch (Exception $ex) {
	+ throw new PhutilProxyException(
	+ pht(
	+ 'Failed to read ".arcunit" file (at path "%s").',
	+ $arcunit_display),
	+ $ex);
	+ }
	+
	+ try {
	+ $spec = phutil_json_decode($data);
	+ } catch (PhutilJSONParserException $ex) {
	+ throw new PhutilProxyException(
	+ pht(
	+ 'Expected ".arcunit" file (at path "%s") to be a valid JSON file, '.
	+ 'but it could not be parsed.',
	+ $arcunit_display),
	+ $ex);
	+ }
	+
	+ try {
	+ PhutilTypeSpec::checkMap(
	+ $spec,
	+ array(
	+ 'engines' => 'map<string, wild>',
	+ ));
	+ } catch (PhutilTypeCheckException $ex) {
	+ throw new PhutilProxyException(
	+ pht(
	+ 'The ".arcunit" file (at path "%s") is not formatted correctly.',
	+ $arcunit_display),
	+ $ex);
	+ }
	+
	+ $all_engines = ArcanistUnitEngine::getAllUnitEngines();
	+
	+ $engines = array();
	+ foreach ($spec['engines'] as $key => $engine_spec) {
	+ try {
	+ PhutilTypeSpec::checkMap(
	+ $engine_spec,
	+ array(
	+ 'type' => 'string',
	+ 'include' => 'optional regex \| list<regex>',
	+ 'exclude' => 'optional regex \| list<regex>',
	+ ));
	+ } catch (PhutilTypeCheckException $ex) {
	+ throw new PhutilProxyException(
	+ pht(
	+ 'The ".arcunit" file (at path "%s") is not formatted correctly: '.
	+ 'the engine with key "%s" is specified improperly.',
	+ $arcunit_display,
	+ $key));
	+ }
	+
	+ $type = $engine_spec['type'];
	+ if (!isset($all_engines[$type])) {
	+ throw new Exception(
	+ pht(
	+ 'The ".arcunit" file (at path "%s") specifies an engine (with '.
	+ 'key "%s") of an unknown type ("%s").',
	+ $arcunit_display,
	+ $key,
	+ $type));
	+ }
	+
	+ $engine = clone $all_engines[$type];
	+
	+ if (isset($engine_spec['include'])) {
	+ $engine->setIncludePaths((array)$engine_spec['include']);
	+ }
	+
	+ if (isset($engine_spec['exclude'])) {
	+ $engine->setExcludePaths((array)$engine_spec['exclude']);
	+ }
	+
	+ $engines[] = $engine;
	+ }
	+
	+ return $engines;
	+ }
	+
	+}
	diff --git a/src/workflow/ArcanistUnitWorkflow.php b/src/workflow/ArcanistUnitWorkflow.php
	--- a/src/workflow/ArcanistUnitWorkflow.php
	+++ b/src/workflow/ArcanistUnitWorkflow.php
	@@ -1,430 +1,82 @@
	<?php

	-/**
	- * Runs unit tests which cover your changes.
	- */
	-final class ArcanistUnitWorkflow extends ArcanistWorkflow {
	-
	- const RESULT_OKAY = 0;
	- const RESULT_UNSOUND = 1;
	- const RESULT_FAIL = 2;
	- const RESULT_SKIP = 3;
	-
	- private $unresolvedTests;
	- private $testResults;
	- private $engine;
	+final class ArcanistUnitWorkflow
	+ extends ArcanistWorkflow {

	public function getWorkflowName() {
	return 'unit';
	}

	- public function getCommandSynopses() {
	- return phutil_console_format(<<<EOTEXT
	- unit [__options__] [__paths__]
	- unit [__options__] --rev [__rev__]
	+ public function getWorkflowInformation() {
	+ $help = pht(<<<EOTEXT
	+Run unit tests.
	EOTEXT
	- );
	- }
	+);

	- public function getCommandHelp() {
	- return phutil_console_format(<<<EOTEXT
	- Supports: git, svn, hg
	- Run unit tests that cover specified paths. If no paths are specified,
	- unit tests covering all modified files will be run.
	-EOTEXT
	- );
	+ return $this->newWorkflowInformation()
	+ ->addExample(pht('unit [__options__] __path__ __path__ ...'))
	+ ->addExample(pht('unit [__options__] --commit __commit__'))
	+ ->setHelp($help);
	}

	- public function getArguments() {
	+ public function getWorkflowArguments() {
	return array(
	- 'rev' => array(
	- 'param' => 'revision',
	- 'help' => pht(
	- 'Run unit tests covering changes since a specific revision.'),
	- 'supports' => array(
	- 'git',
	- 'hg',
	- ),
	- 'nosupport' => array(
	- 'svn' => pht(
	- 'Arc unit does not currently support %s in SVN.',
	- '--rev'),
	- ),
	- ),
	- 'engine' => array(
	- 'param' => 'classname',
	- 'help' => pht('Override configured unit engine for this project.'),
	- ),
	- 'coverage' => array(
	- 'help' => pht('Always enable coverage information.'),
	- 'conflicts' => array(
	- 'no-coverage' => null,
	- ),
	- ),
	- 'no-coverage' => array(
	- 'help' => pht('Always disable coverage information.'),
	- ),
	- 'detailed-coverage' => array(
	- 'help' => pht(
	- '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(
	- "With 'full', show full pretty report (Default). ".
	- "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',
	- 'help' => pht(
	- '(PROTOTYPE) Record a copy of the test results on the specified '.
	- 'Harbormaster build target.'),
	- ),
	- 'everything' => array(
	- 'help' => pht(
	- 'Run every test associated with a tracked file in the working '.
	- 'copy.'),
	- 'conflicts' => array(
	- 'rev' => pht('%s runs all tests.', '--everything'),
	- ),
	- ),
	- 'ugly' => array(
	- 'help' => pht(
	- 'With %s, use uglier (but more efficient) formatting.',
	- '--json'),
	- ),
	- '*' => 'paths',
	+ $this->newWorkflowArgument('commit')
	+ ->setParameter('commit'),
	+ $this->newWorkflowArgument('format')
	+ ->setParameter('format'),
	+ $this->newWorkflowArgument('everything'),
	+ $this->newWorkflowArgument('paths')
	+ ->setWildcard(true),
	+
	+ // TOOLSETS: Restore "--target".
	);
	}

	- public function requiresWorkingCopy() {
	- return true;
	- }
	-
	- public function requiresRepositoryAPI() {
	- return true;
	- }
	+ public function runWorkflow() {
	+ // If we're in a working copy, run tests from the working copy root.
	+ // Otherwise, run tests from the current working directory.

	- public function requiresConduit() {
	- return $this->shouldUploadResults();
	- }
	-
	- public function requiresAuthentication() {
	- return $this->shouldUploadResults();
	- }
	-
	- public function getEngine() {
	- return $this->engine;
	- }
	-
	- public function run() {
	$working_copy = $this->getWorkingCopy();
	-
	- $paths = $this->getArgument('paths');
	- $rev = $this->getArgument('rev');
	- $everything = $this->getArgument('everything');
	- if ($everything && $paths) {
	- throw new ArcanistUsageException(
	- pht(
	- 'You can not specify paths with %s. The %s flag runs every test '.
	- 'associated with a tracked file in the working copy.',
	- '--everything',
	- '--everything'));
	- }
	-
	- if ($everything) {
	- $paths = iterator_to_array($this->getRepositoryAPI()->getAllFiles());
	+ if ($working_copy) {
	+ $directory = $working_copy->getPath();
	} else {
	- $paths = $this->selectPathsForWorkflow($paths, $rev);
	+ $directory = getcwd();
	}

	- $this->engine = $this->newUnitTestEngine($this->getArgument('engine'));
	- if ($everything) {
	- $this->engine->setRunAllTests(true);
	- } else {
	- $this->engine->setPaths($paths);
	- }
	-
	- $renderer = new ArcanistUnitConsoleRenderer();
	- $this->engine->setRenderer($renderer);
	-
	- $enable_coverage = null; // Means "default".
	- if ($this->getArgument('coverage') \|\|
	- $this->getArgument('detailed-coverage')) {
	- $enable_coverage = true;
	- } else if ($this->getArgument('no-coverage')) {
	- $enable_coverage = false;
	- }
	- $this->engine->setEnableCoverage($enable_coverage);
	-
	- $results = $this->engine->run();
	-
	- $this->validateUnitEngineResults($this->engine, $results);
	-
	- $this->testResults = $results;
	-
	- $console = PhutilConsole::getConsole();
	-
	- $output_format = $this->getOutputFormat();
	+ $overseer = id(new ArcanistUnitOverseer())
	+ ->setDirectory($directory);

	- if ($output_format !== 'full') {
	- $console->disableOut();
	- }
	-
	- $unresolved = array();
	- $coverage = array();
	- 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;
	- }
	- if ($result->getCoverage()) {
	- foreach ($result->getCoverage() as $file => $report) {
	- $coverage[$file][] = $report;
	- }
	- }
	- }
	+ // TOOLSETS: For now, we're treating every invocation of "arc unit" as
	+ // though it is "arc unit --everything", and ignoring the "--commit" flag
	+ // and "paths" arguments.

	- if ($coverage) {
	- $file_coverage = array_fill_keys(
	- $paths,
	- 0);
	- $file_reports = array();
	- foreach ($coverage as $file => $reports) {
	- $report = ArcanistUnitTestResult::mergeCoverage($reports);
	- $cov = substr_count($report, 'C');
	- $uncov = substr_count($report, 'U');
	- if ($cov + $uncov) {
	- $coverage = $cov / ($cov + $uncov);
	- } else {
	- $coverage = 0;
	- }
	- $file_coverage[$file] = $coverage;
	- $file_reports[$file] = $report;
	- }
	- $console->writeOut("\n__%s__\n", pht('COVERAGE REPORT'));
	+ $formatter = $this->newUnitFormatter();
	+ $overseer->setFormatter($formatter);

	- asort($file_coverage);
	- foreach ($file_coverage as $file => $coverage) {
	- $console->writeOut(
	- " %s%% %s\n",
	- sprintf('% 3d', (int)(100 * $coverage)),
	- $file);
	-
	- $full_path = $working_copy->getProjectRoot().'/'.$file;
	- if ($this->getArgument('detailed-coverage') &&
	- Filesystem::pathExists($full_path) &&
	- is_file($full_path) &&
	- array_key_exists($file, $file_reports)) {
	- $console->writeOut(
	- '%s',
	- $this->renderDetailedCoverageReport(
	- Filesystem::readFile($full_path),
	- $file_reports[$file]));
	- }
	- }
	- }
	+ $overseer->execute();

	- $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);
	- }
	-
	- return $overall_result;
	- }
	-
	- public function getUnresolvedTests() {
	- return $this->unresolvedTests;
	- }
	-
	- public function getTestResults() {
	- return $this->testResults;
	+ return 0;
	}

	- private function renderDetailedCoverageReport($data, $report) {
	- $data = explode("\n", $data);
	-
	- $out = '';
	-
	- $n = 0;
	- foreach ($data as $line) {
	- $out .= sprintf('% 5d ', $n + 1);
	- $line = str_pad($line, 80, ' ');
	- if (empty($report[$n])) {
	- $c = 'N';
	- } else {
	- $c = $report[$n];
	- }
	- switch ($c) {
	- case 'C':
	- $out .= phutil_console_format(
	- '<bg:green> %s </bg>',
	- $line);
	- break;
	- case 'U':
	- $out .= phutil_console_format(
	- '<bg:red> %s </bg>',
	- $line);
	- break;
	- case 'X':
	- $out .= phutil_console_format(
	- '<bg:magenta> %s </bg>',
	- $line);
	- break;
	- default:
	- $out .= ' '.$line.' ';
	- break;
	- }
	- $out .= "\n";
	- $n++;
	+ private function newUnitFormatter() {
	+ $formatters = ArcanistUnitFormatter::getAllUnitFormatters();
	+ $format_key = $this->getArgument('format');
	+ if (!strlen($format_key)) {
	+ $format_key = ArcanistDefaultUnitFormatter::FORMATTER_KEY;
	}

	- return $out;
	- }
	-
	- private function getOutputFormat() {
	- if ($this->getArgument('ugly')) {
	- return 'ugly';
	- }
	- if ($this->getArgument('json')) {
	- return 'json';
	- }
	- $format = $this->getArgument('output');
	- $known_formats = array(
	- 'none' => 'none',
	- 'json' => 'json',
	- 'ugly' => 'ugly',
	- 'full' => 'full',
	- );
	- return idx($known_formats, $format, 'full');
	- }
	-
	-
	- /**
	- * Raise a tailored error when a unit test engine returns results in an
	- * invalid format.
	- *
	- * @param ArcanistUnitTestEngine The engine.
	- * @param wild Results from the engine.
	- */
	- private function validateUnitEngineResults(
	- ArcanistUnitTestEngine $engine,
	- $results) {
	-
	- if (!is_array($results)) {
	- throw new Exception(
	+ $formatter = idx($formatters, $format_key);
	+ if (!$formatter) {
	+ throw new ArcanistUsageException(
	pht(
	- 'Unit test engine (of class "%s") returned invalid results when '.
	- 'run (with method "%s"). Expected a list of "%s" objects as results.',
	- get_class($engine),
	- 'run()',
	- 'ArcanistUnitTestResult'));
	- }
	-
	- foreach ($results as $key => $result) {
	- if (!($result instanceof ArcanistUnitTestResult)) {
	- throw new Exception(
	- pht(
	- 'Unit test engine (of class "%s") returned invalid results when '.
	- 'run (with method "%s"). Expected a list of "%s" objects as '.
	- 'results, but value with key "%s" is not valid.',
	- get_class($engine),
	- 'run()',
	- 'ArcanistUnitTestResult',
	- $key));
	- }
	- }
	-
	- }
	-
	- public static function getHarbormasterTypeFromResult($unit_result) {
	- switch ($unit_result) {
	- case self::RESULT_OKAY:
	- case self::RESULT_SKIP:
	- $type = 'pass';
	- break;
	- default:
	- $type = 'fail';
	- break;
	- }
	-
	- return $type;
	- }
	-
	- private function shouldUploadResults() {
	- return ($this->getArgument('target') !== null);
	- }
	-
	- private function uploadTestResults(
	- $target_phid,
	- $unit_result,
	- array $unit) {
	-
	- // TODO: It would eventually be nice to stream test results up to the
	- // server as we go, but just get things working for now.
	-
	- $message_type = self::getHarbormasterTypeFromResult($unit_result);
	-
	- foreach ($unit as $key => $result) {
	- $dictionary = $result->toDictionary();
	- $unit[$key] = $this->getModernUnitDictionary($dictionary);
	+ 'Unit test output format ("%s") is unknown. Supported formats '.
	+ 'are: %s.',
	+ $format_key,
	+ implode(', ', array_keys($formatters))));
	}

	- $this->getConduit()->callMethodSynchronous(
	- 'harbormaster.sendmessage',
	- array(
	- 'buildTargetPHID' => $target_phid,
	- 'unit' => array_values($unit),
	- 'type' => $message_type,
	- ));
	+ return $formatter;
	}

	}

File Metadata

Mime Type: text/plain
Expires: Mon, Nov 25, 9:06 AM (19 h, 12 m)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 6785526
Default Alt Text: D19710.diff (41 KB)

D19710.diffNo OneTemporaryActions

D19710.diffView Options

File Metadata

Event Timeline

D19710.diff
No OneTemporary
Actions

D19710.diff
View Options