From 777c119a0eb13b6d7941af95c90b558c28542e41 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 29 Sep 2012 18:03:43 -0700 Subject: [PATCH] Search for more test locations in PHPUnitTestEngine Summary: See https://github.com/facebook/arcanist/pull/53 - Work correctly for directories with `%` in their name; this breaks under sprintf(). - Search for `src/` -> `tests/` style directories. - Add coverage for search paths. Test Plan: Ran unit tests. I don't have a working test case for PHPUnit tests, can one of you guys apply this and verify I didn't break your setups? Reviewers: quard, aurelijus Reviewed By: aurelijus CC: aran Differential Revision: https://secure.phabricator.com/D3558 --- src/__phutil_library_map__.php | 2 + src/unit/engine/PhpunitTestEngine.php | 161 +++++++++++++----- .../__tests__/PHPUnitTestEngineTestCase.php | 61 +++++++ 3 files changed, 186 insertions(+), 38 deletions(-) create mode 100644 src/unit/engine/__tests__/PHPUnitTestEngineTestCase.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index af7e0195..acbcf472 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -135,6 +135,7 @@ phutil_register_library_map(array( 'ComprehensiveLintEngine' => 'lint/engine/ComprehensiveLintEngine.php', 'ExampleLintEngine' => 'lint/engine/ExampleLintEngine.php', 'NoseTestEngine' => 'unit/engine/NoseTestEngine.php', + 'PHPUnitTestEngineTestCase' => 'unit/engine/__tests__/PHPUnitTestEngineTestCase.php', 'PhpunitTestEngine' => 'unit/engine/PhpunitTestEngine.php', 'PhutilLintEngine' => 'lint/engine/PhutilLintEngine.php', 'PhutilUnitTestEngine' => 'unit/engine/PhutilUnitTestEngine.php', @@ -239,6 +240,7 @@ phutil_register_library_map(array( 'ComprehensiveLintEngine' => 'ArcanistLintEngine', 'ExampleLintEngine' => 'ArcanistLintEngine', 'NoseTestEngine' => 'ArcanistBaseUnitTestEngine', + 'PHPUnitTestEngineTestCase' => 'ArcanistTestCase', 'PhpunitTestEngine' => 'ArcanistBaseUnitTestEngine', 'PhutilLintEngine' => 'ArcanistLintEngine', 'PhutilUnitTestEngine' => 'ArcanistBaseUnitTestEngine', diff --git a/src/unit/engine/PhpunitTestEngine.php b/src/unit/engine/PhpunitTestEngine.php index baa7a201..d76423f5 100644 --- a/src/unit/engine/PhpunitTestEngine.php +++ b/src/unit/engine/PhpunitTestEngine.php @@ -272,54 +272,139 @@ final class PhpunitTestEngine extends ArcanistBaseUnitTestEngine { /** - * Some nasty guessing here. - * - * Walk up to the project root trying to find - * [Tt]ests directory and replicate the structure there. - * - * Assume that the class path is - * /www/project/module/package/subpackage/FooBar.php - * and a project root is /www/project it will look for it by these paths: - * /www/project/module/package/subpackage/[Tt]ests/FooBarTest.php - * /www/project/module/package/[Tt]ests/subpackage/FooBarTest.php - * /www/project/module/[Tt]ests/package/subpackage/FooBarTest.php - * /www/project/Tt]ests/module/package/subpackage/FooBarTest.php - * - * TODO: Add support for finding tests based on PSR-1 naming conventions: - * /www/project/src/Something/Foo/Bar.php tests should be detected in - * /www/project/tests/Something/Foo/BarTest.php + * Search for test cases for a given file in a large number of "reasonable" + * locations. See @{method:getSearchLocationsForTests} for specifics. * * TODO: Add support for finding tests in testsuite folders from * phpunit.xml configuration. * - * @param string $path - * - * @return string|boolean + * @param string PHP file to locate test cases for. + * @return string|null Path to test cases, or null. */ private function findTestFile($path) { - $expected_file = substr(basename($path), 0, -4) . 'Test.php'; - $expected_dir = null; - $dirname = dirname($path); - foreach (Filesystem::walkToRoot($dirname) as $dir) { - $expected_dir = DIRECTORY_SEPARATOR - . substr($dirname, strlen($dir) + 1) - . $expected_dir; - $look_for = $dir . DIRECTORY_SEPARATOR - . '%s' . $expected_dir . $expected_file; + $root = $this->projectRoot; + $path = Filesystem::resolvePath($path, $root); - if (Filesystem::pathExists(sprintf($look_for, 'Tests'))) { - return sprintf($look_for, 'Tests'); - } else if (Filesystem::pathExists(sprintf($look_for, 'Tests'))) { - return sprintf($look_for, 'Tests'); + $file = basename($path); + $possible_files = array( + $file, + substr($file, 0, -4).'Test.php', + ); + + $search = self::getSearchLocationsForTests($path); + + foreach ($search as $search_path) { + foreach ($possible_files as $possible_file) { + $full_path = $search_path.$possible_file; + if (!Filesystem::pathExists($full_path)) { + // If the file doesn't exist, it's clearly a miss. + continue; + } + if (!Filesystem::isDescendant($full_path, $root)) { + // Don't look above the project root. + continue; + } + if (Filesystem::resolvePath($full_path) == $path) { + // Don't return the original file. + continue; + } + return $full_path; } - - if ($dir == $this->projectRoot) { - break; - } - } - return false; + return null; + } + + + /** + * Get places to look for PHP Unit tests that cover a given file. For some + * file "/a/b/c/X.php", we look in the same directory: + * + * /a/b/c/ + * + * We then look in all parent directories for a directory named "tests/" + * (or "Tests/"): + * + * /a/b/c/tests/ + * /a/b/tests/ + * /a/tests/ + * /tests/ + * + * We also try to replace each directory component with "tests/": + * + * /a/b/tests/ + * /a/tests/c/ + * /tests/b/c/ + * + * We also try to add "tests/" at each directory level: + * + * /a/b/c/tests/ + * /a/b/tests/c/ + * /a/tests/b/c/ + * /tests/a/b/c/ + * + * This finds tests with a layout like: + * + * docs/ + * src/ + * tests/ + * + * ...or similar. This list will be further pruned by the caller; it is + * intentionally filesystem-agnostic to be unit testable. + * + * @param string PHP file to locate test cases for. + * @return list List of directories to search for tests in. + */ + public static function getSearchLocationsForTests($path) { + $file = basename($path); + $dir = dirname($path); + + $test_dir_names = array('tests', 'Tests'); + + $try_directories = array(); + + // Try in the current directory. + $try_directories[] = array($dir); + + // Try in a tests/ directory anywhere in the ancestry. + foreach (Filesystem::walkToRoot($dir) as $parent_dir) { + if ($parent_dir == '/') { + // We'll restore this later. + $parent_dir = ''; + } + foreach ($test_dir_names as $test_dir_name) { + $try_directories[] = array($parent_dir, $test_dir_name); + } + } + + // Try replacing each directory component with 'tests/'. + $parts = trim($dir, DIRECTORY_SEPARATOR); + $parts = explode(DIRECTORY_SEPARATOR, $parts); + foreach (array_reverse(array_keys($parts)) as $key) { + foreach ($test_dir_names as $test_dir_name) { + $try = $parts; + $try[$key] = $test_dir_name; + array_unshift($try, ''); + $try_directories[] = $try; + } + } + + // Try adding 'tests/' at each level. + foreach (array_reverse(array_keys($parts)) as $key) { + foreach ($test_dir_names as $test_dir_name) { + $try = $parts; + $try[$key] = $test_dir_name.DIRECTORY_SEPARATOR.$try[$key]; + array_unshift($try, ''); + $try_directories[] = $try; + } + } + + $results = array(); + foreach ($try_directories as $parts) { + $results[implode(DIRECTORY_SEPARATOR, $parts).DIRECTORY_SEPARATOR] = true; + } + + return array_keys($results); } /** diff --git a/src/unit/engine/__tests__/PHPUnitTestEngineTestCase.php b/src/unit/engine/__tests__/PHPUnitTestEngineTestCase.php new file mode 100644 index 00000000..882122a8 --- /dev/null +++ b/src/unit/engine/__tests__/PHPUnitTestEngineTestCase.php @@ -0,0 +1,61 @@ +assertEqual( + array( + '/path/to/some/file/', + '/path/to/some/file/tests/', + '/path/to/some/file/Tests/', + '/path/to/some/tests/', + '/path/to/some/Tests/', + '/path/to/tests/', + '/path/to/Tests/', + '/path/tests/', + '/path/Tests/', + '/tests/', + '/Tests/', + '/path/to/tests/file/', + '/path/to/Tests/file/', + '/path/tests/some/file/', + '/path/Tests/some/file/', + '/tests/to/some/file/', + '/Tests/to/some/file/', + '/path/to/some/tests/file/', + '/path/to/some/Tests/file/', + '/path/to/tests/some/file/', + '/path/to/Tests/some/file/', + '/path/tests/to/some/file/', + '/path/Tests/to/some/file/', + '/tests/path/to/some/file/', + '/Tests/path/to/some/file/', + ), + PhpunitTestEngine::getSearchLocationsForTests($path)); + } + +}