Page MenuHomePhabricator

D15895.id.diff
No OneTemporary

D15895.id.diff

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<regex>',
- 'exclude' => 'optional regex | list<regex>',
- ));
+ $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<regex>',
+ 'exclude' => 'optional regex | list<regex>',
+ );
+ }
+
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<string, string>',
+ '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;
+ }
}

File Metadata

Mime Type
text/plain
Expires
Sun, May 19, 5:09 AM (2 w, 10 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6287239
Default Alt Text
D15895.id.diff (7 KB)

Event Timeline