diff --git a/src/unit/engine/ArcanistUnitTestEngine.php b/src/unit/engine/ArcanistUnitTestEngine.php index 5116b964..95d67313 100644 --- a/src/unit/engine/ArcanistUnitTestEngine.php +++ b/src/unit/engine/ArcanistUnitTestEngine.php @@ -8,14 +8,14 @@ abstract class ArcanistUnitTestEngine { 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 @@ abstract class ArcanistUnitTestEngine { return $this; } - public function getRunAllTests() { + final public function getRunAllTests() { return $this->runAllTests; } @@ -36,15 +36,13 @@ abstract class ArcanistUnitTestEngine { 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 @@ abstract class ArcanistUnitTestEngine { 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 index 1abf1e74..58cf52df 100644 --- a/src/unit/engine/PhutilUnitTestEngine.php +++ b/src/unit/engine/PhutilUnitTestEngine.php @@ -21,14 +21,16 @@ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine { } $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 @@ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine { } } - $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,75 +107,24 @@ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine { 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 The names of the test case classes to be executed. + */ private function getTestsForPaths() { - $project_root = $this->getWorkingCopy()->getProjectRoot(); - - $look_here = 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:'), - $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.'))); - } - - $path = Filesystem::resolvePath($path, $project_root); - - if (!is_dir($path)) { - $path = dirname($path); - } - - if ($path == $library_root) { - $look_here[$library_name.':.'] = array( - 'library' => $library_name, - 'path' => '', - ); - } else 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. - + $look_here = $this->getTestPaths(); $run_tests = array(); + foreach ($look_here as $path_info) { $library = $path_info['library']; $path = $path_info['path']; @@ -180,7 +132,7 @@ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine { $symbols = id(new PhutilSymbolLoader()) ->setType('class') ->setLibrary($library) - ->setPathPrefix(($path ? $path.'/' : '').'__tests__/') + ->setPathPrefix($path) ->setAncestorClass('PhutilTestCase') ->setConcreteOnly(true) ->selectAndLoadSymbols(); @@ -189,9 +141,71 @@ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine { $run_tests[$symbol['name']] = true; } } - $run_tests = array_keys($run_tests); - return $run_tests; + return array_keys($run_tests); + } + + /** + * Returns the paths in which we should look for tests to execute. + * + * @return list A list of paths in which to search for test cases. + */ + 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( + 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, + '.arcconfig.')); + } + + $path = Filesystem::resolvePath($path, $root); + $library_path = Filesystem::readablePath($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; + } + + if (is_file($path) && preg_match('@(?:^|/)__tests__/@', $path)) { + $paths[$library_name.':'.$library_path] = array( + 'library' => $library_name, + 'path' => $library_path, + ); + continue; + } + + while (($library_path = dirname($library_path)) != '.') { + $paths[$library_name.':'.$library_path] = array( + 'library' => $library_name, + 'path' => $library_path.'/__tests__/', + ); + } + } + + return $paths; } public function shouldEchoTestResults() { diff --git a/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php b/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php index 0df0512d..11c132d9 100644 --- a/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php +++ b/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php @@ -24,7 +24,7 @@ final class PhutilUnitTestEngineTestCase extends PhutilTestCase { self::$allTestsCounter--; - $actual_test_count = 4; + $actual_test_count = 5; $this->assertEqual( $actual_test_count, @@ -120,4 +120,47 @@ final class PhutilUnitTestEngineTestCase extends PhutilTestCase { } } + public function testGetTestPaths() { + $tests = array( + array( + array(), + array(), + ), + + array( + array(__FILE__), + array(__FILE__), + ), + + array( + array(dirname(__FILE__)), + array( + 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 index a3f98510..68f0cdb3 100644 --- a/src/unit/engine/phutil/PhutilTestCase.php +++ b/src/unit/engine/phutil/PhutilTestCase.php @@ -166,8 +166,10 @@ abstract class PhutilTestCase { * @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 @@ abstract class PhutilTestCase { array $map, $callable, $exception_class = 'Exception') { + return $this->tryTestCases( array_fuse(array_keys($map)), array_values($map), @@ -655,7 +658,7 @@ abstract class PhutilTestCase { return (string)$uri; } - public function setRenderer(ArcanistUnitRenderer $renderer) { + final public function setRenderer(ArcanistUnitRenderer $renderer) { $this->renderer = $renderer; return $this; }