diff --git a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php --- a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php +++ b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php @@ -81,13 +81,11 @@ } try { + // TODO: Perhaps we should emit a warning if child engines re-use keys + // from the defaults, since their specs will not be respected. PhutilTypeSpec::checkMap( $spec, - array( - 'type' => 'string', - 'include' => 'optional regex | list', - 'exclude' => 'optional regex | list', - )); + $this->getDefaultEngineOptions() + $test_engine->getEngineOptions()); } catch (PhutilTypeCheckException $ex) { throw new PhutilProxyException( pht( @@ -108,12 +106,26 @@ $test_engine->setPaths($paths); } + // Apply any values found from `getEngineOptions` to the engine + foreach ($test_engine->getEngineOptions() as $key => $type) { + $value = idx($spec, $key, ''); + $test_engine->setOption($key, $value); + } + $built_test_engines[] = $test_engine; } return $built_test_engines; } + private function getDefaultEngineOptions() { + return array( + 'type' => 'string', + 'include' => 'optional regex | list', + 'exclude' => 'optional regex | list', + ); + } + public function run() { $renderer = $this->renderer; $this->setRenderer(null); @@ -150,7 +162,7 @@ foreach ($results as $result) { // If the proxied engine renders its own test results then there // is no need to render them again here. - if (!$engine->shouldEchoTestResults()) { + if ($engine->shouldEchoTestResults()) { echo $renderer->renderUnitResult($result); } } diff --git a/src/unit/engine/ArcanistUnitTestEngine.php b/src/unit/engine/ArcanistUnitTestEngine.php --- a/src/unit/engine/ArcanistUnitTestEngine.php +++ b/src/unit/engine/ArcanistUnitTestEngine.php @@ -10,6 +10,7 @@ private $enableCoverage; private $runAllTests; private $configurationManager; + private $options = array(); protected $renderer; final public function __construct() {} @@ -18,6 +19,23 @@ return null; } + /** + * Modify the return value of this function to accept configuration values + * from `.arcunit`. Must be in the form used by `PhutilTypeSpec`. + */ + public function getEngineOptions() { + return array(); + } + + final protected function getOption($key, $default = null) { + return idx($this->options, $key, $default); + } + + final protected function setOption($key, $value) { + $this->options[$key] = $value; + return $this; + } + final public function setRunAllTests($run_all_tests) { if (!$this->supportsRunAllTests() && $run_all_tests) { throw new Exception( diff --git a/src/unit/engine/PhpunitTestEngine.php b/src/unit/engine/PhpunitTestEngine.php --- a/src/unit/engine/PhpunitTestEngine.php +++ b/src/unit/engine/PhpunitTestEngine.php @@ -9,6 +9,11 @@ private $phpunitBinary = 'phpunit'; private $affectedTests; private $projectRoot; + private $flags = []; + + public function getEngineConfigurationName() { + return 'phpunit'; + } public function run() { $this->projectRoot = $this->getWorkingCopy()->getProjectRoot(); @@ -45,14 +50,23 @@ throw new ArcanistNoEffectException(pht('No tests to run.')); } - $this->prepareConfigFile(); + $this->prepareConfig(); + if (!Filesystem::binaryExists($this->phpunitBinary)) { + throw new Exception(pht( + "PHPUnit binary not found at '%s'", + $this->phpunitBinary)); + } + $futures = array(); $tmpfiles = array(); + + $all_flags = implode(' ', $this->flags); foreach ($this->affectedTests as $class_path => $test_path) { if (!Filesystem::pathExists($test_path)) { continue; } $json_tmp = new TempFile(); + $json = csprintf('--log-json %s', $json_tmp); $clover_tmp = null; $clover = null; if ($this->getEnableCoverage() !== false) { @@ -60,12 +74,8 @@ $clover = csprintf('--coverage-clover %s', $clover_tmp); } - $config = $this->configFile ? csprintf('-c %s', $this->configFile) : null; - - $stderr = '-d display_errors=stderr'; - - $futures[$test_path] = new ExecFuture('%C %C %C --log-json %s %C %s', - $this->phpunitBinary, $config, $stderr, $json_tmp, $clover, $test_path); + $futures[$test_path] = new ExecFuture('%C %C %C %C %s', + $this->phpunitBinary, $all_flags, $json, $clover, $test_path); $tmpfiles[$test_path] = array( 'json' => $json_tmp, 'clover' => $clover_tmp, @@ -248,17 +258,42 @@ } /** - * Tries to find and update phpunit configuration file based on - * `phpunit_config` option in `.arcconfig`. + * Apply configuration values from all possible sources */ - private function prepareConfigFile() { + private function prepareConfig() { + // .arcconfig, .arcrc, etc. + $manager = $this->getConfigurationManager(); + if ($manager) { + $this->prepareConfigFromManager($manager); + } + // .arcunit + foreach ($this->getOption('flags', array()) as $k => $v) { + $v = str_replace('$ROOT', $this->projectRoot, $v); + $this->flags[] = (string)csprintf('%C %s', $k, $v); + } + if ($bin = $this->getOption('binary')) { + $this->setBinaryPath($bin); + } + + // Try to avoid errors in tests from being suppressed (TODO: leave this as + // an exercise to the person configuring the engine?) + $this->flags[] = '-d display_errors=stderr'; + } + + /** + * Apply configuration from working copy configs. Eventually this may be + * deprecated in favor of `.arcunit`. + */ + private function prepareConfigFromManager( + ArcanistConfigurationManager $manager) { $project_root = $this->projectRoot.DIRECTORY_SEPARATOR; - $config = $this->getConfigurationManager()->getConfigFromAnySource( + $config = $manager->getConfigFromAnySource( 'phpunit_config'); if ($config) { if (Filesystem::pathExists($project_root.$config)) { - $this->configFile = $project_root.$config; + $this->flags[] = (string)csprintf('%C %s', + '--configuration', $project_root.$config); } else { throw new Exception( pht( @@ -266,15 +301,30 @@ $project_root.$config)); } } - $bin = $this->getConfigurationManager()->getConfigFromAnySource( + $bin = $manager->getConfigFromAnySource( 'unit.phpunit.binary'); if ($bin) { - if (Filesystem::binaryExists($bin)) { - $this->phpunitBinary = $bin; - } else { - $this->phpunitBinary = Filesystem::resolvePath($bin, $project_root); - } + $this->setBinaryPath($bin); + } + } + + public function getEngineOptions() { + return array( + 'flags' => 'optional map', + 'binary' => 'optional string', + ); + } + + private function setBinaryPath($bin) { + // Try exactly as-is + if (Filesystem::binaryExists($bin)) { + $this->phpunitBinary = $bin; + } else { + // This may resolve wrong, but we check before run that the binary exists. + // It's done there instead of here so the default value is also checked. + $this->phpunitBinary = Filesystem::resolvePath($bin, $this->projectRoot); } + return $this; } } diff --git a/src/unit/engine/PhutilUnitTestEngine.php b/src/unit/engine/PhutilUnitTestEngine.php --- a/src/unit/engine/PhutilUnitTestEngine.php +++ b/src/unit/engine/PhutilUnitTestEngine.php @@ -221,4 +221,9 @@ return $paths; } + public function shouldEchoTestResults() { + // If a renderer exists, PhutilTestCase will automatically render itself; + // otherwise, expect an upstream renderer to handle this. + return !$this->renderer; + } }