diff --git a/src/lint/linter/ArcanistFlake8Linter.php b/src/lint/linter/ArcanistFlake8Linter.php --- a/src/lint/linter/ArcanistFlake8Linter.php +++ b/src/lint/linter/ArcanistFlake8Linter.php @@ -47,7 +47,7 @@ list($stdout) = execx('%C --version', $this->getExecutableCommand()); $matches = array(); - if (preg_match('/^(?P\d+\.\d+\.\d+)\b/', $stdout, $matches)) { + if (preg_match('/^(?P\d+\.\d+(?:\.\d+)?)\b/', $stdout, $matches)) { return $matches['version']; } else { return false; diff --git a/src/lint/linter/ArcanistJSHintLinter.php b/src/lint/linter/ArcanistJSHintLinter.php --- a/src/lint/linter/ArcanistJSHintLinter.php +++ b/src/lint/linter/ArcanistJSHintLinter.php @@ -46,11 +46,15 @@ } public function getVersion() { - list($stdout) = execx('%C --version', $this->getExecutableCommand()); + // NOTE: JSHint emits version information on stderr, not stdout. + + list($stdout, $stderr) = execx( + '%C --version', + $this->getExecutableCommand()); $matches = array(); $regex = '/^jshint v(?P\d+\.\d+\.\d+)$/'; - if (preg_match($regex, $stdout, $matches)) { + if (preg_match($regex, $stderr, $matches)) { return $matches['version']; } else { return false; diff --git a/src/lint/linter/__tests__/ArcanistArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistArcanistLinterTestCase.php --- a/src/lint/linter/__tests__/ArcanistArcanistLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistArcanistLinterTestCase.php @@ -1,8 +1,5 @@ executeTestsInDirectory( dirname(__FILE__).'/flake8/', - new ArcanistFlake8Linter()); + new ArcanistFlake8Linter(), + '2.1.0'); } } diff --git a/src/lint/linter/__tests__/ArcanistJSHintLinterTestCase.php b/src/lint/linter/__tests__/ArcanistJSHintLinterTestCase.php --- a/src/lint/linter/__tests__/ArcanistJSHintLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistJSHintLinterTestCase.php @@ -6,7 +6,8 @@ public function testJSHintLinter() { $this->executeTestsInDirectory( dirname(__FILE__).'/jshint/', - new ArcanistJSHintLinter()); + new ArcanistJSHintLinter(), + '2.5.0'); } } diff --git a/src/lint/linter/__tests__/ArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistLinterTestCase.php --- a/src/lint/linter/__tests__/ArcanistLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistLinterTestCase.php @@ -2,12 +2,14 @@ /** * Facilitates implementation of test cases for @{class:ArcanistLinter}s. - * - * @group testcase */ abstract class ArcanistLinterTestCase extends ArcanistPhutilTestCase { - public function executeTestsInDirectory($root, ArcanistLinter $linter) { + public function executeTestsInDirectory( + $root, + ArcanistLinter $linter, + $minimum_version = null) { + $files = id(new FileFinder($root)) ->withType('f') ->withSuffix('lint-test') @@ -15,8 +17,13 @@ $test_count = 0; foreach ($files as $file) { - $this->lintFile($root.$file, $linter); + $this->lintFile($root.$file, $linter, $minimum_version); $test_count++; + + // If we've checked the version once, we don't need to keep checking. + // It's often moderately expensive to check versions for external linters, + // as we have to invoke them. + $minimum_version = null; } $this->assertTrue( @@ -24,7 +31,7 @@ pht('Expected to find some .lint-test tests in directory %s!', $root)); } - private function lintFile($file, $linter) { + private function lintFile($file, $linter, $minimum_version) { $linter = clone $linter; $contents = Filesystem::readFile($file); @@ -101,6 +108,23 @@ $engine->addLinter($linter); $engine->addFileData($path_name, $data); + // Check if we have the required minimum version. If we can't figure out + // the version, just continue: we're likely missing the linter, and will + // skip the test automatically later on. + if ($minimum_version !== null) { + $version = $linter->getVersion(); + if ($version) { + if (version_compare($version, $minimum_version, '<')) { + $this->assertSkipped( + pht( + 'The installed version of this linter is too old to run '. + 'this test (version "%s" is required, "%s" is installed).', + $minimum_version, + $version)); + } + } + } + $results = $engine->run(); $this->assertEqual( @@ -114,6 +138,8 @@ } catch (ArcanistPhutilTestTerminatedException $ex) { throw $ex; + } catch (ArcanistPhutilTestSkippedException $ex) { + throw $ex; } catch (Exception $exception) { $caught_exception = true; if ($exception instanceof PhutilAggregateException) {