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

@@ -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',

src/unit/engine/PhpunitTestEngine.php

@@ -272,54 +272,139 @@ final class PhpunitTestEngine extends ArcanistBaseUnitTestEngine {
 
 
   /**
-   * Some nasty guessing here.
+   * Search for test cases for a given file in a large number of "reasonable"
-   *
+   * locations. See @{method:getSearchLocationsForTests} for specifics.
-   * 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
    *
    * TODO: Add support for finding tests in testsuite folders from
    * phpunit.xml configuration.
    *
-   * @param string $path
+   * @param   string      PHP file to locate test cases for.
-   *
+   * @return  string|null Path to test cases, or null.
-   * @return string|boolean
    */
   private function findTestFile($path) {
-    $expected_file = substr(basename($path), 0, -4) . 'Test.php';
+    $root = $this->projectRoot;
-    $expected_dir = null;
+    $path = Filesystem::resolvePath($path, $root);
-    $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;
 
-      if (Filesystem::pathExists(sprintf($look_for, 'Tests'))) {
+    $file = basename($path);
-        return sprintf($look_for, 'Tests');
+    $possible_files = array(
-      } else if (Filesystem::pathExists(sprintf($look_for, 'Tests'))) {
+      $file,
-        return sprintf($look_for, 'Tests');
+      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) {
+    return null;
-        break;
   }
 
+
+  /**
+   * 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<string>  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);
+      }
     }
 
-    return false;
+    // 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);
   }
 
   /**

src/unit/engine/__tests__/PHPUnitTestEngineTestCase.php

@@ -0,0 +1,61 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Tests for @{class:PHPUnitTestEngine}.
+ *
+ * @group testcase
+ */
+final class PHPUnitTestEngineTestCase extends ArcanistTestCase {
+
+  public function testSearchLocations() {
+
+    $path = '/path/to/some/file/X.php';
+
+    $this->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));
+  }
+
+}