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 @@ -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>', - )); - } 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', - 'exclude' => 'optional regex | list', - )); - } 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 @@ +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 @@ -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 @@ 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 @@ +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 @@ +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', + )); + } 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', + 'exclude' => 'optional regex | list', + )); + } 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 @@ 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( - ' %s ', - $line); - break; - case 'U': - $out .= phutil_console_format( - ' %s ', - $line); - break; - case 'X': - $out .= phutil_console_format( - ' %s ', - $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; } }