Changeset 174087 in webkit

Timestamp:

Sep 29, 2014 2:55:04 PM (10 years ago)

Author:

ap@apple.com

Message:

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.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• QueueStatusServer/app.yaml (modified) (1 diff)
• QueueStatusServer/handlers/statusbubble.py (modified) (2 diffs)
• QueueStatusServer/model/attachment.py (modified) (3 diffs)
• QueueStatusServer/templates/statusbubble.html (modified) (4 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

trunk/Tools/QueueStatusServer/app.yaml

@@ -1 +1 @@
 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

trunk/Tools/QueueStatusServer/handlers/statusbubble.py

@@ -33 +33 @@
 
 from model.attachment import Attachment
+from model.activeworkitems import ActiveWorkItems
+from model.patchlog import PatchLog
+from model.queues import Queue
+from model.queuestatus import QueueStatus
 from model.workitems import WorkItems
-from model.queues import Queue
+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):
+    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):
-        queue_status = attachment.status_for_queue(queue)
         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
 
-    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
-        # Complete ewses are also shown.
+        # EWS queues are also shown when complete.
         return bool(queue.is_ews() and attachment.status_for_queue(queue))
 
@@ -60 +171 @@
         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)
-            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

trunk/Tools/QueueStatusServer/model/attachment.py

@@ -35 +35 @@
 
 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
@@ -63 +45 @@
         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):
@@ -118 +86 @@
                 # summary() is a horrible API and should be killed.
                 summary[queue.name_with_underscores()] = {
-                    "state": self.state_from_queue_status(status),
                     "status": status,
                 }

trunk/Tools/QueueStatusServer/templates/statusbubble.html

@@ -20 +20 @@
     -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;
@@ -28 +28 @@
 }
 .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;
-    border: 1px solid #4F8530;
 }
 .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;
-  border: 1px solid #ACA0B3;
 }
 .queue_position {
@@ -73 +66 @@
   {% for bubble in bubbles %}
   <a class="status {{ bubble.state }}" target="_top"
-  {% if bubble.status %}
       href="/patch/{{ bubble.attachment_id }}"
-      title="{{ bubble.status.date|timesince }} ago"
+  {% if bubble.details_message %}
+      title="{{ bubble.details_message }}"
   {% endif %}
   >
@@ -92 +85 @@
   </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>