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 @@ -8,14 +8,14 @@ private $workingCopy; private $paths; private $arguments = array(); - protected $diffID; private $enableAsyncTests; private $enableCoverage; private $runAllTests; protected $renderer; + final public function __construct() {} - public function setRunAllTests($run_all_tests) { + final public function setRunAllTests($run_all_tests) { if (!$this->supportsRunAllTests() && $run_all_tests) { throw new Exception( pht( @@ -28,7 +28,7 @@ return $this; } - public function getRunAllTests() { + final public function getRunAllTests() { return $this->runAllTests; } @@ -36,15 +36,13 @@ return false; } - final public function __construct() {} - - public function setConfigurationManager( + final public function setConfigurationManager( ArcanistConfigurationManager $configuration_manager) { $this->configurationManager = $configuration_manager; return $this; } - public function getConfigurationManager() { + final public function getConfigurationManager() { return $this->configurationManager; } @@ -95,7 +93,7 @@ return $this->enableCoverage; } - public function setRenderer(ArcanistUnitRenderer $renderer) { + final public function setRenderer(ArcanistUnitRenderer $renderer) { $this->renderer = $renderer; 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 @@ -21,14 +21,16 @@ } $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 xdebug is not available, so '. + 'You specified %s but %s is not available, so '. 'coverage can not be enabled for %s.', '--coverage', + 'XDebug', __CLASS__)); } } else { @@ -36,20 +38,21 @@ } } - $project_root = $this->getWorkingCopy()->getProjectRoot(); - $test_cases = array(); + foreach ($run_tests as $test_class) { - $test_case = newv($test_class, array()); - $test_case->setEnableCoverage($enable_coverage); - $test_case->setWorkingCopy($this->getWorkingCopy()); + $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_cases[] = $test_case; } @@ -104,94 +107,109 @@ return $run_tests; } + /** + * Retrieve all relevant test cases. + * + * Looks for any class that extends @{class:PhutilTestCase} inside a + * `__tests__` directory in any parent directory of every affected file. + * + * The idea is that "infrastructure/__tests__/" tests defines general tests + * for all of "infrastructure/", and those tests run for any change in + * "infrastructure/". However, "infrastructure/concrete/rebar/__tests__/" + * defines more specific tests that run only when "rebar/" (or some + * subdirectory) changes. + * + * @return list + */ private function getTestsForPaths() { - $project_root = $this->getWorkingCopy()->getProjectRoot(); + $look_here = $this->getTestPaths(); - $look_here = array(); + $run_tests = array(); + foreach ($look_here as $path_info) { + $library = $path_info['library']; + $path = $path_info['path']; + + $symbols = id(new PhutilSymbolLoader()) + ->setType('class') + ->setLibrary($library) + ->setPathPrefix($path) + ->setAncestorClass('PhutilTestCase') + ->setConcreteOnly(true) + ->selectAndLoadSymbols(); + + foreach ($symbols as $symbol) { + $run_tests[$symbol['name']] = true; + } + } + $run_tests = array_keys($run_tests); + + return $run_tests; + } + + /** + * Returns the paths in which we should look for tests to execute. + * + * @return list + */ + public function getTestPaths() { + $root = $this->getWorkingCopy()->getProjectRoot(); + $paths = array(); foreach ($this->getPaths() as $path) { $library_root = phutil_get_library_root_for_path($path); + if (!$library_root) { continue; } + $library_name = phutil_get_library_name_for_root($library_root); if (!$library_name) { throw new Exception( - sprintf( - "%s\n\n %s\n\n%s\n\n - %s\n - %s\n", - pht( - 'Attempting to run unit tests on a libphutil library '. - 'which has not been loaded, at:'), + pht( + "Attempting to run unit tests on a libphutil library which has ". + "not been loaded, at:\n\n". + " %s\n\n". + "This probably means one of two things:\n\n". + " - You may need to add this library to %s.\n". + " - You may be running tests on a copy of libphutil or ". + "arcanist using a different copy of libphutil or arcanist. ". + "This operation is not supported.\n", $library_root, - pht('This probably means one of two things:'), - pht( - 'You may need to add this library to %s.', - '.arcconfig.'), - pht( - 'You may be running tests on a copy of libphutil or '. - 'arcanist using a different copy of libphutil or arcanist. '. - 'This operation is not supported.'))); + '.arcconfig.')); } - $path = Filesystem::resolvePath($path, $project_root); - - if (!is_dir($path)) { - $path = dirname($path); - } + $path = Filesystem::resolvePath($path, $root); + $library_path = Filesystem::readablePath($path, $library_root); - if ($path == $library_root) { - $look_here[$library_name.':.'] = array( - 'library' => $library_name, - 'path' => '', - ); - } else if (!Filesystem::isDescendant($path, $library_root)) { + if (!Filesystem::isDescendant($path, $library_root)) { // We have encountered some kind of symlink maze -- for instance, $path // is some symlink living outside the library that links into some file // inside the library. Just ignore these cases, since the affected file // does not actually lie within the library. continue; - } else { - $library_path = Filesystem::readablePath($path, $library_root); - do { - $look_here[$library_name.':'.$library_path] = array( - 'library' => $library_name, - 'path' => $library_path, - ); - $library_path = dirname($library_path); - } while ($library_path != '.'); } - } - // Look for any class that extends PhutilTestCase inside a `__tests__` - // directory in any parent directory of every affected file. - // - // The idea is that "infrastructure/__tests__/" tests defines general tests - // for all of "infrastructure/", and those tests run for any change in - // "infrastructure/". However, "infrastructure/concrete/rebar/__tests__/" - // defines more specific tests that run only when rebar/ (or some - // subdirectory) changes. - - $run_tests = array(); - foreach ($look_here as $path_info) { - $library = $path_info['library']; - $path = $path_info['path']; + if (is_file($path) && preg_match('@(?:^|/)__tests__/@', $path)) { + $paths[$library_name.':'.$library_path] = array( + 'library' => $library_name, + 'path' => $library_path, + ); + continue; + } - $symbols = id(new PhutilSymbolLoader()) - ->setType('class') - ->setLibrary($library) - ->setPathPrefix(($path ? $path.'/' : '').'__tests__/') - ->setAncestorClass('PhutilTestCase') - ->setConcreteOnly(true) - ->selectAndLoadSymbols(); + do { + $path = $library_path != $library_root ? $library_path : ''; - foreach ($symbols as $symbol) { - $run_tests[$symbol['name']] = true; - } + $paths[$library_name.':'.$library_path] = array( + 'library' => $library_name, + 'path' => $path.'/__tests__/', + ); + $library_path = dirname($library_path); + } while ($library_path != '.' && $library_path != $library_root); } - $run_tests = array_keys($run_tests); - return $run_tests; + return $paths; } public function shouldEchoTestResults() { 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 @@ -24,7 +24,7 @@ self::$allTestsCounter--; - $actual_test_count = 4; + $actual_test_count = 5; $this->assertEqual( $actual_test_count, @@ -120,4 +120,48 @@ } } + public function testGetTestPaths() { + $tests = array( + array( + array(), + array(), + ), + + array( + array(__FILE__), + array(__FILE__), + ), + + array( + array(dirname(__FILE__)), + array( + dirname(__FILE__).'/__tests__/', + dirname(dirname(__FILE__)).'/__tests__/', + dirname(dirname(dirname(__FILE__))).'/__tests__/', + ), + ), + ); + + $test_engine = id(new PhutilUnitTestEngine()) + ->setWorkingCopy($this->getWorkingCopy()); + + foreach ($tests as $test) { + list($paths, $tests) = $test; + $expected = array(); + + foreach ($tests as $path) { + $library_root = phutil_get_library_root_for_path($path); + $library = phutil_get_library_name_for_root($library_root); + + $expected[] = array( + 'library' => $library, + 'path' => Filesystem::readablePath($path, $library_root), + ); + } + + $test_engine->setPaths($paths); + $this->assertEqual($expected, array_values($test_engine->getTestPaths())); + } + } + } 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 @@ -166,8 +166,10 @@ * @return void * @task exceptions */ - final protected function assertException($expected_exception_class, - $callable) { + final protected function assertException( + $expected_exception_class, + $callable) { + $this->tryTestCases( array('assertException' => array()), array(false), @@ -300,6 +302,7 @@ array $map, $callable, $exception_class = 'Exception') { + return $this->tryTestCases( array_fuse(array_keys($map)), array_values($map), @@ -655,7 +658,7 @@ return (string)$uri; } - public function setRenderer(ArcanistUnitRenderer $renderer) { + final public function setRenderer(ArcanistUnitRenderer $renderer) { $this->renderer = $renderer; return $this; }