EWS error message "Error: * did not process patch" should include exp…

…lanation

https://bugs.webkit.org/show_bug.cgi?id=159903
<rdar://problem/27410788>

Reviewed by Alexey Proskuryakov.

* QueueStatusServer/handlers/statusbubble.py:
(StatusBubble._build_bubble): Display more detailed error message on bubbles when patch
is not processed.
* QueueStatusServer/handlers/processingtimesjson.py:
(ProcessingTimesJSON._resultFromFinalStatus): Updated error message to match with rest
of the code.
* Scripts/webkitpy/tool/bot/commitqueuetask.py:
(CommitQueueTask.validate): Add more information about validation failure.
(CommitQueueTask.run): Pass the error details in the PatchIsNotValid exception.
* Scripts/webkitpy/tool/bot/earlywarningsystemtask.py:
(EarlyWarningSystemTask.validate): Add more information about validation failure.
(EarlyWarningSystemTask.run): Pass the error details in the PatchIsNotValid exception.
* Scripts/webkitpy/tool/bot/patchanalysistask.py:
(PatchIsNotValid.__init__): Add the failure_message argument.
* Scripts/webkitpy/tool/commands/earlywarningsystem.py:
(AbstractEarlyWarningSystem.review_patch): Re-word the error message and include
failure details.
* Scripts/webkitpy/tool/commands/queues.py:
(CommitQueue.process_work_item): Same.
(StyleQueue.review_patch): Same.
* Scripts/webkitpy/tool/commands/queues_unittest.py:
(test_non_valid_patch): Updated test-cases messages to match the above changes.


Canonical link: https://commits.webkit.org/178455@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@203830 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog

@@ -1,3 +1,34 @@
+2016-07-28  Aakash Jain  <aakash_jain@apple.com>
+
+        EWS error message "Error: * did not process patch" should include explanation
+        https://bugs.webkit.org/show_bug.cgi?id=159903
+        <rdar://problem/27410788>
+
+        Reviewed by Alexey Proskuryakov.
+
+        * QueueStatusServer/handlers/statusbubble.py:
+        (StatusBubble._build_bubble): Display more detailed error message on bubbles when patch
+        is not processed.
+        * QueueStatusServer/handlers/processingtimesjson.py:
+        (ProcessingTimesJSON._resultFromFinalStatus): Updated error message to match with rest
+        of the code.
+        * Scripts/webkitpy/tool/bot/commitqueuetask.py:
+        (CommitQueueTask.validate): Add more information about validation failure.
+        (CommitQueueTask.run): Pass the error details in the PatchIsNotValid exception.
+        * Scripts/webkitpy/tool/bot/earlywarningsystemtask.py:
+        (EarlyWarningSystemTask.validate): Add more information about validation failure.
+        (EarlyWarningSystemTask.run): Pass the error details in the PatchIsNotValid exception.
+        * Scripts/webkitpy/tool/bot/patchanalysistask.py:
+        (PatchIsNotValid.__init__): Add the failure_message argument.
+        * Scripts/webkitpy/tool/commands/earlywarningsystem.py:
+        (AbstractEarlyWarningSystem.review_patch): Re-word the error message and include
+        failure details.
+        * Scripts/webkitpy/tool/commands/queues.py:
+        (CommitQueue.process_work_item): Same.
+        (StyleQueue.review_patch): Same.
+        * Scripts/webkitpy/tool/commands/queues_unittest.py:
+        (test_non_valid_patch): Updated test-cases messages to match the above changes.
+
 2016-07-27  Alexey Proskuryakov  <ap@apple.com>
 
         LayoutTestRelay should wait for WebKitTestRunnerApp installation to complete

Tools/QueueStatusServer/handlers/processingtimesjson.py

@@ -40,7 +40,7 @@ def _resultFromFinalStatus(self, status_message, queue_name):
             return "pass"
         elif status_message == "Fail":
             return "fail"
-        elif status_message == "Error: " + queue_name + " did not process patch.":
+        elif "did not process patch" in status_message:
             return "not processed"
         elif status_message == "Error: " + queue_name + " unable to apply patch.":
             return "could not apply"

Tools/QueueStatusServer/handlers/statusbubble.py

@@ -140,14 +140,24 @@ def _build_bubble(self, queue, attachment, queue_position):
                 bubble["state"] = "fail"
                 message_to_display = statuses[1].message if len(statuses) > 1 else statuses[0].message
                 bubble["details_message"] = message_to_display + "\n\n" + self._iso_time(statuses[0].date)
-            elif statuses[0].message == "Error: " + queue.name() + " did not process patch.":
+            elif "did not process patch" in statuses[0].message:
                 bubble["state"] = "none"
                 bubble["details_message"] = "The patch is no longer eligible for processing."
+
+                if "Bug is already closed" in statuses[0].message:
+                    bubble["details_message"] += " Bug was already closed when EWS attempted to process it."
+                elif "Patch is marked r-" in statuses[0].message:
+                    bubble["details_message"] += " Patch was already marked r- when EWS attempted to process it."
+                elif "Patch is obsolete" in statuses[0].message:
+                    bubble["details_message"] += " Patch was obsolete when EWS attempted to process it."
+                elif "No patch committer found" in statuses[0].message:
+                    bubble["details_message"] += " Patch was not authorized by a commmitter."
+
                 if len(statuses) > 1:
                     if len(statuses) == 2:
-                        bubble["details_message"] += " One message was logged while the patch was still eligible:\n\n"
+                        bubble["details_message"] += "\nOne message was logged while the patch was still eligible:\n\n"
                     else:
-                        bubble["details_message"] += " Some messages were logged while the patch was still eligible:\n\n"
+                        bubble["details_message"] += "\nSome messages were logged while the patch was still eligible:\n\n"
                     bubble["details_message"] += "\n".join([status.message for status in statuses[1:]]) + "\n\n" + self._iso_time(statuses[0].date)
             elif statuses[0].message == "Error: " + queue.name() + " unable to apply patch.":
                 bubble["state"] = "fail"

Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py

@@ -43,12 +43,16 @@ def validate(self):
         # commit-queue is processing.
         self._patch = self._delegate.refetch_patch(self._patch)
         if self._patch.is_obsolete():
+            self.error = "Patch is obsolete."
             return False
         if self._patch.bug().is_closed():
+            self.error = "Bug is already closed."
             return False
         if not self._patch.committer():
+            self.error = "No patch committer found."
             return False
         if self._patch.review() == "-":
+            self.error = "Patch is marked r-."
             return False
         return True
 
@@ -69,7 +73,7 @@ def _did_pass_tests_recently(self):
 
     def run(self):
         if not self.validate():
-            raise PatchIsNotValid(self._patch)
+            raise PatchIsNotValid(self._patch, self.error)
         if not self._clean():
             return False
         if not self._update():
@@ -88,7 +92,7 @@ def run(self):
         # Make sure the patch is still valid before landing (e.g., make sure
         # no one has set commit-queue- since we started working on the patch.)
         if not self.validate():
-            raise PatchIsNotValid(self._patch)
+            raise PatchIsNotValid(self._patch, self.error)
         # FIXME: We should understand why the land failure occurred and retry if possible.
         if not self._land():
             return self.report_failure()

Tools/Scripts/webkitpy/tool/bot/earlywarningsystemtask.py

@@ -41,16 +41,19 @@ def __init__(self, delegate, patch, should_run_tests=True):
     def validate(self):
         self._patch = self._delegate.refetch_patch(self._patch)
         if self._patch.is_obsolete():
+            self.error = "Patch is obsolete."
             return False
         if self._patch.bug().is_closed():
+            self.error = "Bug is already closed."
             return False
         if self._patch.review() == "-":
+            self.error = "Patch is marked r-."
             return False
         return True
 
     def run(self):
         if not self.validate():
-            raise PatchIsNotValid(self._patch)
+            raise PatchIsNotValid(self._patch, self.error)
         if not self._clean():
             return False
         if not self._update():

Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py

@@ -37,9 +37,10 @@ def __init__(self, patch):
 
 
 class PatchIsNotValid(Exception):
-    def __init__(self, patch):
+    def __init__(self, patch, failure_message):
         Exception.__init__(self)
         self.patch = patch
+        self.failure_message = failure_message
 
 
 class PatchAnalysisTaskDelegate(object):

Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py

@@ -87,8 +87,8 @@ def review_patch(self, patch):
                 # Caller unlocks when review_patch returns True, so we only need to unlock on transient failure.
                 self._unlock_patch(patch)
             return succeeded
-        except PatchIsNotValid:
-            self._did_error(patch, "%s did not process patch." % self.name)
+        except PatchIsNotValid as error:
+            self._did_error(patch, "%s did not process patch. Reason: %s" % (self.name, error.failure_message))
             return False
         except UnableToApplyPatch, e:
             self._did_error(patch, "%s unable to apply patch." % self.name)

Tools/Scripts/webkitpy/tool/commands/queues.py

@@ -345,8 +345,8 @@ def process_work_item(self, patch):
                 return True
             self._unlock_patch(patch)
             return False
-        except PatchIsNotValid:
-            self._did_error(patch, "%s did not process patch." % self.name)
+        except PatchIsNotValid as error:
+            self._did_error(patch, "%s did not process patch. Reason: %s" % (self.name, error.failure_message))
             return False
         except ScriptError, e:
             validator = CommitterValidator(self._tool)
@@ -485,8 +485,8 @@ def review_patch(self, patch):
         except UnableToApplyPatch, e:
             self._did_error(patch, "%s unable to apply patch." % self.name)
             return False
-        except PatchIsNotValid:
-            self._did_error(patch, "%s did not process patch." % self.name)
+        except PatchIsNotValid as error:
+            self._did_error(patch, "%s did not process patch. Reason: %s" % (self.name, error.failure_message))
             return False
         except ScriptError, e:
             output = re.sub(r'Failed to run .+ exit_code: 1', '', e.output)

Tools/Scripts/webkitpy/tool/commands/queues_unittest.py

@@ -389,7 +389,7 @@ def test_non_valid_patch(self):
         patch = tool.bugs.fetch_attachment(10007)  # _patch8, resolved bug, without review flag, not marked obsolete (maybe already landed)
         expected_logs = {
             "begin_work_queue": self._default_begin_work_queue_logs("commit-queue"),
-            "process_work_item": """MOCK: update_status: commit-queue Error: commit-queue did not process patch.
+            "process_work_item": """MOCK: update_status: commit-queue Error: commit-queue did not process patch. Reason: Bug is already closed.
 MOCK: release_work_item: commit-queue 10007
 """,
         }
@@ -431,7 +431,7 @@ def test_manual_reject_during_processing(self):
 MOCK: update_status: commit-queue Built patch
 Running: webkit-patch --status-host=example.com build-and-test --no-clean --no-update --test --non-interactive --build-style=release --port=mac
 MOCK: update_status: commit-queue Passed tests
-MOCK: update_status: commit-queue Error: commit-queue did not process patch.
+MOCK: update_status: commit-queue Error: commit-queue did not process patch. Reason: Patch is obsolete.
 MOCK: release_work_item: commit-queue 10000
 """
         self.maxDiff = None