From 83a0083b3b47aabaf76fcaf4604a19cd30ac3119 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 1 Feb 2011 20:32:10 -0800 Subject: [PATCH] Improve argument passthru behavior to make added args available to test engines. Summary: When you add new arguments to the unit command, they need to be threaded through to the unit engine. This enables you to specify passthru command which will be shipped through the callstack. Test Plan: Ran 'arc diff --maxtests 8 --apply-patches --trace' and verified resulting behavior was correct. Differential Revision: 207877 Reviewed By: dschleimer Reviewers: dschleimer CC: dschleimer Revert Plan: OK --- .../base/ArcanistBaseUnitTestEngine.php | 10 +++++++ src/unit/engine/base/__init__.php | 2 ++ src/workflow/base/ArcanistBaseWorkflow.php | 25 +++++++++++++++++ src/workflow/diff/ArcanistDiffWorkflow.php | 28 +++++++++---------- src/workflow/unit/ArcanistUnitWorkflow.php | 1 + 5 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/unit/engine/base/ArcanistBaseUnitTestEngine.php b/src/unit/engine/base/ArcanistBaseUnitTestEngine.php index 7ec25c5e..cfc1b6f7 100644 --- a/src/unit/engine/base/ArcanistBaseUnitTestEngine.php +++ b/src/unit/engine/base/ArcanistBaseUnitTestEngine.php @@ -20,6 +20,7 @@ abstract class ArcanistBaseUnitTestEngine { private $workingCopy; private $paths; + private $arguments = array(); final public function __construct() { @@ -45,6 +46,15 @@ abstract class ArcanistBaseUnitTestEngine { return $this->paths; } + final public function setArguments(array $arguments) { + $this->arguments = $arguments; + return $this; + } + + final public function getArgument($key, $default = null) { + return idx($this->arguments, $key, $default); + } + abstract public function run(); } diff --git a/src/unit/engine/base/__init__.php b/src/unit/engine/base/__init__.php index 69d1e77a..3f0e488f 100644 --- a/src/unit/engine/base/__init__.php +++ b/src/unit/engine/base/__init__.php @@ -6,5 +6,7 @@ +phutil_require_module('phutil', 'utils'); + phutil_require_source('ArcanistBaseUnitTestEngine.php'); diff --git a/src/workflow/base/ArcanistBaseWorkflow.php b/src/workflow/base/ArcanistBaseWorkflow.php index a9544766..410040ff 100644 --- a/src/workflow/base/ArcanistBaseWorkflow.php +++ b/src/workflow/base/ArcanistBaseWorkflow.php @@ -555,4 +555,29 @@ class ArcanistBaseWorkflow { return array('any'); } + protected function getPassthruArgumentsAsMap($command) { + $map = array(); + foreach ($this->getCompleteArgumentSpecification() as $key => $spec) { + if (!empty($spec['passthru'][$command])) { + if (isset($this->arguments[$key])) { + $map[$key] = $this->arguments[$key]; + } + } + } + return $map; + } + + protected function getPassthruArgumentsAsArgv($command) { + $spec = $this->getCompleteArgumentSpecification(); + $map = $this->getPassthruArgumentsAsMap($command); + $argv = array(); + foreach ($map as $key => $value) { + $argv[] = '--'.$key; + if (!empty($spec[$key]['param'])) { + $argv[] = $value; + } + } + return $argv; + } + } diff --git a/src/workflow/diff/ArcanistDiffWorkflow.php b/src/workflow/diff/ArcanistDiffWorkflow.php index fa0fe94e..974253a7 100644 --- a/src/workflow/diff/ArcanistDiffWorkflow.php +++ b/src/workflow/diff/ArcanistDiffWorkflow.php @@ -149,10 +149,16 @@ EOTEXT 'lintall' => array( 'help' => "Raise all lint warnings, not just those on lines you changed.", + 'passthru' => array( + 'lint' => true, + ), ), 'advice' => array( 'help' => "Raise lint advice in addition to lint warnings and errors.", + 'passthru' => array( + 'lint' => true, + ), ), 'apply-patches' => array( 'help' => @@ -161,12 +167,18 @@ EOTEXT 'conflicts' => array( 'never-apply-patches' => true, ), + 'passthru' => array( + 'lint' => true, + ), ), 'never-apply-patches' => array( 'help' => 'Never apply patches suggested by lint.', 'conflicts' => array( 'apply-patches' => true, ), + 'passthru' => array( + 'lint' => true, + ), ), '*' => 'paths', ); @@ -836,19 +848,7 @@ EOTEXT echo "Linting...\n"; try { - $argv = array(); - if ($this->getArgument('lintall')) { - $argv[] = '--lintall'; - } - if ($this->getArgument('advice')) { - $argv[] = '--advice'; - } - if ($this->getArgument('apply-patches')) { - $argv[] = '--apply-patches'; - } - if ($this->getArgument('never-apply-patches')) { - $argv[] = '--never-apply-patches'; - } + $argv = $this->getPassthruArgumentsAsArgv('lint'); if ($repository_api instanceof ArcanistSubversionAPI) { $argv = array_merge($argv, array_keys($paths)); } else { @@ -905,7 +905,7 @@ EOTEXT echo "Running unit tests...\n"; try { - $argv = array(); + $argv = $this->getPassthruArgumentsAsArgv('unit'); if ($repository_api instanceof ArcanistSubversionAPI) { $argv = array_merge($argv, array_keys($paths)); } diff --git a/src/workflow/unit/ArcanistUnitWorkflow.php b/src/workflow/unit/ArcanistUnitWorkflow.php index 800d4d0b..e4d517e8 100644 --- a/src/workflow/unit/ArcanistUnitWorkflow.php +++ b/src/workflow/unit/ArcanistUnitWorkflow.php @@ -77,6 +77,7 @@ EOTEXT $engine = newv($engine_class, array()); $engine->setWorkingCopy($working_copy); $engine->setPaths($paths); + $engine->setArguments($this->getPassthruArgumentsAsMap('unit')); $results = $engine->run();