Improve Bugzilla status bubbles
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Sep 2014 21:55:04 +0000 (21:55 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Sep 2014 21:55:04 +0000 (21:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=137232

Reviewed by Ryosuke Niwa.

* QueueStatusServer/app.yaml: Will update again with an actual version when landing.

* QueueStatusServer/handlers/statusbubble.py: Eliminated yellow color, added
blue and orange. Significantly extended tooltips. Made bubbles show up even for queues
that are stuck, as it was only confusing that they disappeared after 99.

* QueueStatusServer/model/attachment.py: Removed functionality that was only used
by old bubbles. We need a lot more information to determine color, so the implementation
can not be here.

* QueueStatusServer/templates/statusbubble.html: Updated colors in CSS, made bubbles
always have a link for consistency. Added code to convert timestamps in tooltips
to local time zone.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@174087 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/QueueStatusServer/app.yaml
Tools/QueueStatusServer/handlers/statusbubble.py
Tools/QueueStatusServer/model/attachment.py
Tools/QueueStatusServer/templates/statusbubble.html

index 31d2b8d..1e7605b 100644 (file)
@@ -1,3 +1,24 @@
+2014-09-29  Alexey Proskuryakov  <ap@apple.com>
+
+        Improve Bugzilla status bubbles
+        https://bugs.webkit.org/show_bug.cgi?id=137232
+
+        Reviewed by Ryosuke Niwa.
+
+        * QueueStatusServer/app.yaml: Will update again with an actual version when landing.
+
+        * QueueStatusServer/handlers/statusbubble.py: Eliminated yellow color, added
+        blue and orange. Significantly extended tooltips. Made bubbles show up even for queues
+        that are stuck, as it was only confusing that they disappeared after 99.
+
+        * QueueStatusServer/model/attachment.py: Removed functionality that was only used
+        by old bubbles. We need a lot more information to determine color, so the implementation
+        can not be here.
+
+        * QueueStatusServer/templates/statusbubble.html: Updated colors in CSS, made bubbles
+        always have a link for consistency. Added code to convert timestamps in tooltips
+        to local time zone.
+
 2014-09-29  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r174045.
 2014-09-29  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r174045.
index 34c4cc8..5ce405e 100644 (file)
@@ -1,5 +1,5 @@
 application: webkit-queues
 application: webkit-queues
-version: 174015 # Bugzilla bug ID of last major change
+version: 174087 # Bugzilla bug ID of last major change
 runtime: python
 api_version: 1
 
 runtime: python
 api_version: 1
 
index c6ba916..fe468fa 100644 (file)
@@ -32,41 +32,151 @@ from google.appengine.ext import webapp
 from google.appengine.ext.webapp import template
 
 from model.attachment import Attachment
 from google.appengine.ext.webapp import template
 
 from model.attachment import Attachment
-from model.workitems import WorkItems
+from model.activeworkitems import ActiveWorkItems
+from model.patchlog import PatchLog
 from model.queues import Queue
 from model.queues import Queue
+from model.queuestatus import QueueStatus
+from model.workitems import WorkItems
+from sets import Set
 
 
+progress_statuses = Set([
+    "Cleaned working directory",
+    "Updated working directory",
+    "Applied patch",
+    "Built patch",
+    "Watchlist applied",
+    "Style checked",
+    "ChangeLog validated",
+    "Built patch",
+    "Able to build without patch",
+    "Passed tests",
+    "Able to pass tests without patch",
+    "Landed patch"
+])
 
 class StatusBubble(webapp.RequestHandler):
 
 class StatusBubble(webapp.RequestHandler):
+    def _iso_time(self, time):
+        return "[[" + time.isoformat() + "]]"
+
+    # queue_position includes items that are already active, so it's misleading.
+    # For a queue that has 8 bots, being #9 in the queue actually means being #1.
+    def _real_queue_position(self, queue, queue_position):
+        active_work_items = queue.active_work_items().item_ids
+        if active_work_items:
+            return queue_position - len(active_work_items)
+        else:
+            return queue_position
+
+    def _latest_resultative_status(self, statuses):
+        for status in statuses:
+            if not status.message in progress_statuses:
+                return status
+        return None
+
+    def _build_message_for_provisional_failure(self, queue, attachment, queue_position, statuses):
+        patch_log = PatchLog.lookup_if_exists(attachment.id, queue.name())
+        if not patch_log:
+            return "Internal error. No PatchLog entry in database."
+
+        is_active = attachment.id in queue.active_work_items().item_ids
+        try_count = patch_log.retry_count + (not is_active)  # retry_count is updated when a new attempt starts.
+        latest_resultative_status = self._latest_resultative_status(statuses)
+        tree_is_red = latest_resultative_status.message == "Unable to pass tests without patch (tree is red?)" or latest_resultative_status.message == "Unable to build without patch"
+
+        message = latest_resultative_status.message + "."
+        if is_active:
+            if tree_is_red:
+                message += "\n\nTrying again now."
+            else:
+                message += "\n\nThis result is not final, as the issue could be a pre-existing one. Trying to determine that now."
+                if try_count == 1:
+                    message += "\n\nPreviously completed a round of testing, but couldn't arrive to a definitive conclusion."
+                elif try_count > 1:
+                    message += "\n\nPreviously completed " + str(try_count) + " rounds of testing, but couldn't arrive to a definitive conclusion."
+        else:
+            real_queue_position = self._real_queue_position(queue, queue_position)
+            if tree_is_red:
+                message += "\n\nWill try again, currently #" + str(real_queue_position) + " in queue."
+            else:
+                message += "\n\nThis result is not final, as the issue can be a pre-existing one. "
+                if try_count == 1:
+                    message += "Completed one round "
+                else:
+                    message += "Completed " + str(try_count) + " rounds "
+                message += "of testing trying to determine that, but couldn't arrive to a definitive conclusion yet.\n\nWill try again, currently #" + str(real_queue_position) + " in queue."
+        message += "\n\nPlease click the bubble for detailed results.\n\n" + self._iso_time(statuses[0].date)
+        return message
+
     def _build_bubble(self, queue, attachment, queue_position):
     def _build_bubble(self, queue, attachment, queue_position):
-        queue_status = attachment.status_for_queue(queue)
         bubble = {
             "name": queue.short_name().lower(),
             "attachment_id": attachment.id,
         bubble = {
             "name": queue.short_name().lower(),
             "attachment_id": attachment.id,
-            "queue_position": queue_position,
-            "state": attachment.state_from_queue_status(queue_status) if queue_status else "none",
-            "status": queue_status,
         }
         }
+        # 10 recent statuses is enough to always include a resultative one, if there were any at all.
+        statuses = QueueStatus.all().filter('queue_name =', queue.name()).filter('active_patch_id =', attachment.id).order('-date').fetch(limit=10)
+        if not statuses:
+            if attachment.id in queue.active_work_items().item_ids:
+                bubble["state"] = "started"
+                bubble["details_message"] = "Started processing, no output yet.\n\n" + self._iso_time(queue.active_work_items().time_for_item(attachment.id))
+            else:
+                real_queue_position = self._real_queue_position(queue, queue_position)
+                bubble["state"] = "none"
+                bubble["details_message"] = "Waiting in queue, processing has not started yet.\n\nPosition in queue: " + str(real_queue_position)
+                bubble["queue_position"] = real_queue_position
+        else:
+            latest_resultative_status = self._latest_resultative_status(statuses)
+            if not latest_resultative_status:
+                bubble["state"] = "started"
+                bubble["details_message"] = ("Started processing.\n\nRecent messages:\n\n"
+                    + "\n".join([status.message for status in statuses]) + "\n\n" + self._iso_time(statuses[0].date))
+            elif statuses[0].message == "Pass":
+                bubble["state"] = "pass"
+                bubble["details_message"] = "Pass\n\n" + self._iso_time(statuses[0].date)
+            elif statuses[0].message == "Fail":
+                bubble["state"] = "fail"
+                bubble["details_message"] = statuses[1].message + "\n\n" + self._iso_time(statuses[0].date)
+            elif statuses[0].message == "Error: " + queue.name() + " did not process patch.":
+                bubble["state"] = "none"
+                bubble["details_message"] = "The patch is no longer eligible for processing."
+                if len(statuses) > 1:
+                    bubble["details_message"] += " Recent messages:\n\n" + "\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"
+                bubble["details_message"] = statuses[1].message + "\n\n" + self._iso_time(statuses[0].date)
+            elif statuses[0].message.startswith("Error: "):
+                bubble["state"] = "error"
+                bubble["details_message"] = "\n".join([status.message for status in statuses]) + "\n\n" + self._iso_time(statuses[0].date)
+            elif queue_position:
+                bubble["state"] = "provisional-fail"
+                bubble["details_message"] = self._build_message_for_provisional_failure(queue, attachment, queue_position, statuses)
+            else:
+                bubble["state"] = "error"
+                bubble["details_message"] = ("Internal error. Latest status implies that the patch should be in queue, but it is not. Recent messages:\n\n"
+                    + "\n".join([status.message for status in statuses]) + "\n\n" + self._iso_time(statuses[0].date))
+
+        if "details_message" in bubble:
+            bubble["details_message"] = queue.display_name() + "\n\n" + bubble["details_message"]
+
         return bubble
 
         return bubble
 
-    def _have_status_for(self, attachment, queue):
-        # Any pending queue is shown.
+    def _should_show_bubble_for(self, attachment, queue):
+         # Any pending queue is shown.
         if attachment.position_in_queue(queue):
             return True
         if attachment.position_in_queue(queue):
             return True
-        # Complete ewses are also shown.
+        # EWS queues are also shown when complete.
         return bool(queue.is_ews() and attachment.status_for_queue(queue))
 
     def _build_bubbles_for_attachment(self, attachment):
         show_submit_to_ews = True
         bubbles = []
         for queue in Queue.all():
         return bool(queue.is_ews() and attachment.status_for_queue(queue))
 
     def _build_bubbles_for_attachment(self, attachment):
         show_submit_to_ews = True
         bubbles = []
         for queue in Queue.all():
-            if not self._have_status_for(attachment, queue):
+            if not self._should_show_bubble_for(attachment, queue):
                 continue
             queue_position = attachment.position_in_queue(queue)
                 continue
             queue_position = attachment.position_in_queue(queue)
-            if queue_position and queue_position >= 100:
-                # This queue is so far behind it's not even worth showing.
-                continue
-            bubbles.append(self._build_bubble(queue, attachment, queue_position))
-            # If even one ews has status, we don't show the submit-to-ews button.
+            bubble = self._build_bubble(queue, attachment, queue_position)
+            if bubble:
+                bubbles.append(bubble)
+            # If at least one EWS queue has status, we don't show the submit-to-ews button.
             if queue.is_ews():
                 show_submit_to_ews = False
 
             if queue.is_ews():
                 show_submit_to_ews = False
 
index 4960709..2cd04e0 100644 (file)
@@ -34,24 +34,6 @@ from model.workitems import WorkItems
 
 
 class Attachment(object):
 
 
 class Attachment(object):
-    @classmethod
-    def recent(cls, limit=1):
-        statuses = QueueStatus.all().order("-date")
-        # Notice that we use both a set and a list here to keep the -date ordering.
-        ids = []
-        visited_ids = set()
-        for status in statuses:
-            attachment_id = status.active_patch_id
-            if not attachment_id:
-                continue
-            if attachment_id in visited_ids:
-                continue
-            visited_ids.add(attachment_id)
-            ids.append(attachment_id)
-            if len(visited_ids) >= limit:
-                break
-        return map(cls, ids)
-
     def __init__(self, attachment_id):
         self.id = attachment_id
         self._summary = None
     def __init__(self, attachment_id):
         self.id = attachment_id
         self._summary = None
@@ -63,20 +45,6 @@ class Attachment(object):
         self._summary = self._fetch_summary()
         return self._summary
 
         self._summary = self._fetch_summary()
         return self._summary
 
-    def state_from_queue_status(self, status):
-        table = {
-            "Pass" : "pass",
-            "Fail" : "fail",
-        }
-        state = table.get(status.message)
-        if state:
-            return state
-        if status.message.startswith("Error:"):
-            return "error"
-        if status:
-            return "pending"
-        return None
-
     def position_in_queue(self, queue):
         return self._queue_positions().get(queue.name())
 
     def position_in_queue(self, queue):
         return self._queue_positions().get(queue.name())
 
@@ -117,7 +85,6 @@ class Attachment(object):
             if status:
                 # summary() is a horrible API and should be killed.
                 summary[queue.name_with_underscores()] = {
             if status:
                 # summary() is a horrible API and should be killed.
                 summary[queue.name_with_underscores()] = {
-                    "state": self.state_from_queue_status(status),
                     "status": status,
                 }
         return summary
                     "status": status,
                 }
         return summary
index 51a3be8..bee5e83 100644 (file)
@@ -19,7 +19,7 @@ body {
     -moz-border-radius: 5px;
     -webkit-border-radius: 5px;
     border-radius: 5px;
     -moz-border-radius: 5px;
     -webkit-border-radius: 5px;
     border-radius: 5px;
-    border: 1px solid #AAA;
+    border: 1px solid rgba(1, 1, 1, 0.3);
     background-color: white;
     font-size: 11px;
     cursor: pointer;
     background-color: white;
     font-size: 11px;
     cursor: pointer;
@@ -27,29 +27,22 @@ body {
     color: black;
 }
 .status:hover {
     color: black;
 }
 .status:hover {
-    border-color: #666;
-}
-.none {
-    cursor: auto;
-}
-.none:hover {
-    border-color: #AAA;
+    border-color: rgba(1, 1, 1, 0.7);
 }
 .pass {
     background-color: #8FDF5F;
 }
 .pass {
     background-color: #8FDF5F;
-    border: 1px solid #4F8530;
 }
 .fail {
     background-color: #E98080;
 }
 .fail {
     background-color: #E98080;
-    border: 1px solid #A77272;
 }
 }
-.pending {
-    background-color: #FFFC6C;
-    border: 1px solid #C5C56D;
+.started {
+    background-color: #D8FFFA;
+}
+.provisional-fail {
+    background-color: #FFAF05;
 }
 .error {
   background-color: #E0B0FF;
 }
 .error {
   background-color: #E0B0FF;
-  border: 1px solid #ACA0B3;
 }
 .queue_position {
     font-size: 9px;
 }
 .queue_position {
     font-size: 9px;
@@ -72,9 +65,9 @@ window.addEventListener("message", function(e) {
 <div id="bubbleContainer">
   {% for bubble in bubbles %}
   <a class="status {{ bubble.state }}" target="_top"
 <div id="bubbleContainer">
   {% for bubble in bubbles %}
   <a class="status {{ bubble.state }}" target="_top"
-  {% if bubble.status %}
       href="/patch/{{ bubble.attachment_id }}"
       href="/patch/{{ bubble.attachment_id }}"
-      title="{{ bubble.status.date|timesince }} ago"
+  {% if bubble.details_message %}
+      title="{{ bubble.details_message }}"
   {% endif %}
   >
     {{ bubble.name }}
   {% endif %}
   >
     {{ bubble.name }}
@@ -91,6 +84,20 @@ window.addEventListener("message", function(e) {
     <input class="status" type="submit" value="Submit for EWS analysis">
   </form>
 {% endif %}
     <input class="status" type="submit" value="Submit for EWS analysis">
   </form>
 {% endif %}
+
+<script>
+// Convert from UTC dates to local.
+var bubbles = document.getElementsByClassName("status")
+for (var i = 0; i < bubbles.length; ++i) {
+    var bubble = bubbles[i];
+    if (bubble.hasAttribute("title")) {
+        var newTitle = bubble.getAttribute("title").replace(/\[\[(.+)\]\]/, function(match, isoDateString) {
+            return new Date(isoDateString).toString();
+        });
+        bubble.setAttribute("title", newTitle);
+    }
+}
+</script>
 </div>
 </body>
 </html>
 </div>
 </body>
 </html>