From 8c589f1f759f0913135b8cc6959a6c1589e14ae4 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Wed, 3 Jun 2015 21:07:17 +1000 Subject: [PATCH] Improve test discovery with PhutilUnitTestEngine Summary: Ref T8042. Tests were not being discovered in two different scenarios: # Files modified at the same level as the library root directory. # "Normal" directories like `src/unit/engine`. This regression was caused by D12689. Test Plan: Added unit tests. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T8042 Differential Revision: https://secure.phabricator.com/D13126 --- src/unit/engine/PhutilUnitTestEngine.php | 12 +++++-- .../PhutilUnitTestEngineTestCase.php | 33 +++++++++++++++---- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/unit/engine/PhutilUnitTestEngine.php b/src/unit/engine/PhutilUnitTestEngine.php index 58cf52df..4f4e1e9a 100644 --- a/src/unit/engine/PhutilUnitTestEngine.php +++ b/src/unit/engine/PhutilUnitTestEngine.php @@ -197,12 +197,20 @@ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine { continue; } - while (($library_path = dirname($library_path)) != '.') { + if ($path == $library_root) { + $paths[$library_name.':.'] = array( + 'library' => $library_name, + 'path' => '__tests__/', + ); + continue; + } + + do { $paths[$library_name.':'.$library_path] = array( 'library' => $library_name, 'path' => $library_path.'/__tests__/', ); - } + } while (($library_path = dirname($library_path)) != '.'); } return $paths; diff --git a/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php b/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php index 11c132d9..2576bdd2 100644 --- a/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php +++ b/src/unit/engine/__tests__/PhutilUnitTestEngineTestCase.php @@ -122,29 +122,47 @@ final class PhutilUnitTestEngineTestCase extends PhutilTestCase { public function testGetTestPaths() { $tests = array( - array( + 'empty' => array( array(), array(), ), - array( + 'test file' => array( array(__FILE__), array(__FILE__), ), - array( - array(dirname(__FILE__)), + 'test directory' => array( + array( + dirname(__FILE__), + ), + array( + // This is odd, but harmless. + dirname(dirname(__FILE__)).'/__tests__/__tests__/', + + dirname(dirname(__FILE__)).'/__tests__/', + dirname(dirname(dirname(__FILE__))).'/__tests__/', + ), + ), + 'normal directory' => array( + array( + dirname(dirname(__FILE__)), + ), array( dirname(dirname(__FILE__)).'/__tests__/', dirname(dirname(dirname(__FILE__))).'/__tests__/', ), ), + 'library root' => array( + array(phutil_get_library_root()), + array(phutil_get_library_root().'/__tests__/'), + ), ); $test_engine = id(new PhutilUnitTestEngine()) ->setWorkingCopy($this->getWorkingCopy()); - foreach ($tests as $test) { + foreach ($tests as $name => $test) { list($paths, $tests) = $test; $expected = array(); @@ -159,7 +177,10 @@ final class PhutilUnitTestEngineTestCase extends PhutilTestCase { } $test_engine->setPaths($paths); - $this->assertEqual($expected, array_values($test_engine->getTestPaths())); + $this->assertEqual( + $expected, + array_values($test_engine->getTestPaths()), + pht('Test paths for: "%s"', $name)); } }