Page MenuHomePhabricator
Files F15335982

D9060.diff
No OneTemporary
Actions

D9060.diff
View Options

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<version>\d+\.\d+\.\d+)\b/', $stdout, $matches)) {
+ if (preg_match('/^(?P<version>\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<version>\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 @@
<?php
-/**
- * @group testcase
- */
abstract class ArcanistArcanistLinterTestCase extends ArcanistLinterTestCase {
protected function getLink($method) {
diff --git a/src/lint/linter/__tests__/ArcanistFlake8LinterTestCase.php b/src/lint/linter/__tests__/ArcanistFlake8LinterTestCase.php
--- a/src/lint/linter/__tests__/ArcanistFlake8LinterTestCase.php
+++ b/src/lint/linter/__tests__/ArcanistFlake8LinterTestCase.php
@@ -1,8 +1,6 @@
<?php
/**
* Test cases for @{class:ArcanistFlake8Linter}.
- *
- * @group testcase
*/
final class ArcanistFlake8LinterTestCase
extends ArcanistArcanistLinterTestCase {
@@ -10,7 +8,8 @@
public function testFlake8Lint() {
return $this->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) {

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 9, 5:56 PM (5 h, 19 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7390321
Default Alt Text
D9060.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility