From 2df96ed405e218e1e0bb4df1835a67749ae25e9e Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 29 Sep 2015 07:09:19 -0700 Subject: [PATCH 1/4] Allow users to skip "arc:prompt" by entering no text Summary: Fixes T9476. Currently, if you enter no text in "arc:prompt", we abort resolution immediately. However, this is a bit inconsistent (other rules that fail to resolve are skipped) it is reasonable to put some default rule behind "arc:prompt" so that you can just mash enter to select it. This construction is unusual, but seems fine and sensible to me. If you're using "arc:prompt" as the last rule, the behavior is the same as before, so this should only make the rule more useful. Test Plan: - Ran `arc which --base 'arc:prompt, git:HEAD^'` with and without the patch. - Without the patch, entering no text at "arc:prompt" failed immediately. - With the patch, entering no text caused fallback to the "git:HEAD^" rule. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9476 Differential Revision: https://secure.phabricator.com/D14187 --- src/parser/ArcanistBaseCommitParser.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/parser/ArcanistBaseCommitParser.php b/src/parser/ArcanistBaseCommitParser.php index 9650ae1b..af2a71ff 100644 --- a/src/parser/ArcanistBaseCommitParser.php +++ b/src/parser/ArcanistBaseCommitParser.php @@ -133,7 +133,13 @@ final class ArcanistBaseCommitParser extends Phobject { case 'prompt': $reason = pht('it is what you typed when prompted.'); $this->api->setBaseCommitExplanation($reason); - return phutil_console_prompt(pht('Against which commit?')); + $result = phutil_console_prompt(pht('Against which commit?')); + if (!strlen($result)) { + // Allow the user to continue to the next rule by entering no + // text. + return null; + } + return $result; case 'local': case 'user': case 'project': From d3abb6557252a5bfb820f8a9465c26c5fb472e95 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 29 Sep 2015 09:51:51 -0700 Subject: [PATCH 2/4] Add a --target flag to `arc unit` for uploading results Summary: Ref T5821. Basic idea here is that Harbormaster can run `arc unit --everything --target ...` to get a build target updated. Test Plan: Ran `arc unit --target ...`, saw web UI update with test results. Reviewers: chad Reviewed By: chad Maniphest Tasks: T5821 Differential Revision: https://secure.phabricator.com/D14190 --- src/workflow/ArcanistDiffWorkflow.php | 29 +------------ src/workflow/ArcanistUnitWorkflow.php | 62 +++++++++++++++++++++++++++ src/workflow/ArcanistWorkflow.php | 19 ++++++++ 3 files changed, 82 insertions(+), 28 deletions(-) diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php index 3074c166..eeabfdfb 100644 --- a/src/workflow/ArcanistDiffWorkflow.php +++ b/src/workflow/ArcanistDiffWorkflow.php @@ -2753,15 +2753,7 @@ EOTEXT $unit[$key] = $this->getModernUnitDictionary($message); } - switch ($unit_result) { - case ArcanistUnitWorkflow::RESULT_OKAY: - case ArcanistUnitWorkflow::RESULT_SKIP: - $type = 'pass'; - break; - default: - $type = 'fail'; - break; - } + $type = ArcanistUnitWorkflow::getHarbormasterTypeFromResult($unit_result); $futures[] = $this->getConduit()->callMethod( 'harbormaster.sendmessage', @@ -2785,23 +2777,4 @@ EOTEXT } } - private function getModernLintDictionary(array $map) { - $map = $this->getModernCommonDictionary($map); - return $map; - } - - private function getModernUnitDictionary(array $map) { - $map = $this->getModernCommonDictionary($map); - return $map; - } - - private function getModernCommonDictionary(array $map) { - foreach ($map as $key => $value) { - if ($value === null) { - unset($map[$key]); - } - } - return $map; - } - } diff --git a/src/workflow/ArcanistUnitWorkflow.php b/src/workflow/ArcanistUnitWorkflow.php index 1fcee32a..04d8c8e0 100644 --- a/src/workflow/ArcanistUnitWorkflow.php +++ b/src/workflow/ArcanistUnitWorkflow.php @@ -84,6 +84,12 @@ EOTEXT 'ugly' => pht('Only one output format allowed'), ), ), + 'target' => array( + 'param' => 'phid', + 'help' => pht( + '(PROTOTYPE) Record a copy of the test results on the specified '. + 'Harbormaster build target.'), + ), 'everything' => array( 'help' => pht('Run every test.'), 'conflicts' => array( @@ -107,6 +113,14 @@ EOTEXT return true; } + public function requiresConduit() { + return $this->shouldUploadResults(); + } + + public function requiresAuthentication() { + return $this->shouldUploadResults(); + } + public function getEngine() { return $this->engine; } @@ -263,6 +277,12 @@ EOTEXT break; } + + $target_phid = $this->getArgument('target'); + if ($target_phid) { + $this->uploadTestResults($target_phid, $overall_result, $results); + } + return $overall_result; } @@ -370,4 +390,46 @@ EOTEXT } + public static function getHarbormasterTypeFromResult($unit_result) { + switch ($unit_result) { + case self::RESULT_OKAY: + case self::RESULT_SKIP: + $type = 'pass'; + break; + default: + $type = 'fail'; + break; + } + + return $type; + } + + private function shouldUploadResults() { + return ($this->getArgument('target') !== null); + } + + private function uploadTestResults( + $target_phid, + $unit_result, + array $unit) { + + // TODO: It would eventually be nice to stream test results up to the + // server as we go, but just get things working for now. + + $message_type = self::getHarbormasterTypeFromResult($unit_result); + + foreach ($unit as $key => $result) { + $dictionary = $result->toDictionary(); + $unit[$key] = $this->getModernUnitDictionary($dictionary); + } + + $this->getConduit()->callMethodSynchronous( + 'harbormaster.sendmessage', + array( + 'buildTargetPHID' => $target_phid, + 'unit' => array_values($unit), + 'type' => $message_type, + )); + } + } diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php index 54649103..c07279cc 100644 --- a/src/workflow/ArcanistWorkflow.php +++ b/src/workflow/ArcanistWorkflow.php @@ -2040,5 +2040,24 @@ abstract class ArcanistWorkflow extends Phobject { } } + protected function getModernLintDictionary(array $map) { + $map = $this->getModernCommonDictionary($map); + return $map; + } + + protected function getModernUnitDictionary(array $map) { + $map = $this->getModernCommonDictionary($map); + return $map; + } + + private function getModernCommonDictionary(array $map) { + foreach ($map as $key => $value) { + if ($value === null) { + unset($map[$key]); + } + } + return $map; + } + } From f0c57986bc3256b8a4f2e67d0cc62e97c1a43b12 Mon Sep 17 00:00:00 2001 From: Vihang Mehta Date: Fri, 2 Oct 2015 05:45:48 -0700 Subject: [PATCH 3/4] Fix ArcanistClosureLinter Summary: Fixes T9498 Test Plan: Run `arc lint` with errors that get caught and reported by `ArcanistClosureLinter` Reviewers: joshuaspence, epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Maniphest Tasks: T9498 Differential Revision: https://secure.phabricator.com/D14220 --- src/lint/linter/ArcanistClosureLinter.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lint/linter/ArcanistClosureLinter.php b/src/lint/linter/ArcanistClosureLinter.php index 5cdf6259..75eb1511 100644 --- a/src/lint/linter/ArcanistClosureLinter.php +++ b/src/lint/linter/ArcanistClosureLinter.php @@ -50,6 +50,7 @@ final class ArcanistClosureLinter extends ArcanistExternalLinter { $message = id(new ArcanistLintMessage()) ->setPath($path) ->setLine($matches[1]) + ->setName('GJSLINT'.$matches[2]) ->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR) ->setCode($this->getLinterName().$matches[2]) ->setDescription($matches[3]); From 1b4a3e0c5eb94dafaee3dad2752bbb30db8b8350 Mon Sep 17 00:00:00 2001 From: Vihang Mehta Date: Fri, 2 Oct 2015 08:58:15 -0700 Subject: [PATCH 4/4] Set path on more linters Summary: This is in a similar vein as D14220 and sets a name on linter messages. This should handle issues from D14165. Test Plan: Run as many of the changed linters as possible + existing linter tests. Reviewers: chad, #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D14225 --- src/lint/linter/ArcanistCSSLintLinter.php | 1 + src/lint/linter/ArcanistGoLintLinter.php | 1 + src/lint/linter/ArcanistJSONLintLinter.php | 1 + src/lint/linter/ArcanistJscsLinter.php | 1 + src/lint/linter/ArcanistPyFlakesLinter.php | 1 + 5 files changed, 5 insertions(+) diff --git a/src/lint/linter/ArcanistCSSLintLinter.php b/src/lint/linter/ArcanistCSSLintLinter.php index 7436b194..a7583959 100644 --- a/src/lint/linter/ArcanistCSSLintLinter.php +++ b/src/lint/linter/ArcanistCSSLintLinter.php @@ -72,6 +72,7 @@ final class ArcanistCSSLintLinter extends ArcanistExternalLinter { ->setLine($child->getAttribute('line')) ->setChar($child->getAttribute('char')) ->setCode($this->getLinterName()) + ->setName($this->getLinterName()) ->setDescription($child->getAttribute('reason')) ->setOriginalText( substr( diff --git a/src/lint/linter/ArcanistGoLintLinter.php b/src/lint/linter/ArcanistGoLintLinter.php index 0252d4c9..419acf41 100644 --- a/src/lint/linter/ArcanistGoLintLinter.php +++ b/src/lint/linter/ArcanistGoLintLinter.php @@ -53,6 +53,7 @@ final class ArcanistGoLintLinter extends ArcanistExternalLinter { $message->setLine($matches[1]); $message->setChar($matches[2]); $message->setCode($this->getLinterName()); + $message->setName($this->getLinterName()); $message->setDescription(ucfirst(trim($matches[3]))); $message->setSeverity(ArcanistLintSeverity::SEVERITY_ADVICE); diff --git a/src/lint/linter/ArcanistJSONLintLinter.php b/src/lint/linter/ArcanistJSONLintLinter.php index 7fd00254..7a157d61 100644 --- a/src/lint/linter/ArcanistJSONLintLinter.php +++ b/src/lint/linter/ArcanistJSONLintLinter.php @@ -72,6 +72,7 @@ final class ArcanistJSONLintLinter extends ArcanistExternalLinter { $message->setLine($matches['line']); $message->setChar($matches['column']); $message->setCode($this->getLinterName()); + $message->setName($this->getLinterName()); $message->setDescription(ucfirst($matches['description'])); $message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR); diff --git a/src/lint/linter/ArcanistJscsLinter.php b/src/lint/linter/ArcanistJscsLinter.php index 338e08d3..88f046b7 100644 --- a/src/lint/linter/ArcanistJscsLinter.php +++ b/src/lint/linter/ArcanistJscsLinter.php @@ -109,6 +109,7 @@ final class ArcanistJscsLinter extends ArcanistExternalLinter { $message->setLine($error->getAttribute('line')); $message->setChar($error->getAttribute('column')); $message->setCode('JSCS'); + $message->setName('JSCS'); $message->setDescription($error->getAttribute('message')); switch ($error->getAttribute('severity')) { diff --git a/src/lint/linter/ArcanistPyFlakesLinter.php b/src/lint/linter/ArcanistPyFlakesLinter.php index c312487c..e8a37a79 100644 --- a/src/lint/linter/ArcanistPyFlakesLinter.php +++ b/src/lint/linter/ArcanistPyFlakesLinter.php @@ -71,6 +71,7 @@ final class ArcanistPyFlakesLinter extends ArcanistExternalLinter { $message->setPath($path); $message->setLine($matches[2]); $message->setCode($this->getLinterName()); + $message->setName($this->getLinterName()); $message->setDescription($description); $message->setSeverity($severity);