2009-11-28 Adam Barth <abarth@webkit.org>
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Nov 2009 01:50:48 +0000 (01:50 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Nov 2009 01:50:48 +0000 (01:50 +0000)
        Reviewed by Eric Seidel.

        StyleQueue scans ~100 bug pages every 5 minutes
        https://bugs.webkit.org/show_bug.cgi?id=31947

        Instead of getting the pending-review attachment ids by scanning each
        bug (which results in a network request), we should just get them all
        from webkit.org/pending-review in one shot.

        * Scripts/modules/bugzilla.py:
        * Scripts/modules/bugzilla_unittest.py:
        * Scripts/modules/commands/queries.py:
        * Scripts/modules/commands/queries_unittest.py:
        * Scripts/modules/commands/queues.py:
        * Scripts/modules/mock_bugzillatool.py:
        * Scripts/modules/patchcollection.py:

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

WebKitTools/ChangeLog
WebKitTools/Scripts/modules/bugzilla.py
WebKitTools/Scripts/modules/bugzilla_unittest.py
WebKitTools/Scripts/modules/commands/queries.py
WebKitTools/Scripts/modules/commands/queries_unittest.py
WebKitTools/Scripts/modules/commands/queues.py
WebKitTools/Scripts/modules/mock_bugzillatool.py
WebKitTools/Scripts/modules/patchcollection.py

index 9995641f0d0e545f08022407d84f75dbf82d1228..a88e17fb62faced915b0827527cdf76c33ddb04f 100644 (file)
@@ -1,3 +1,22 @@
+2009-11-28  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        StyleQueue scans ~100 bug pages every 5 minutes
+        https://bugs.webkit.org/show_bug.cgi?id=31947
+
+        Instead of getting the pending-review attachment ids by scanning each
+        bug (which results in a network request), we should just get them all
+        from webkit.org/pending-review in one shot.
+
+        * Scripts/modules/bugzilla.py:
+        * Scripts/modules/bugzilla_unittest.py:
+        * Scripts/modules/commands/queries.py:
+        * Scripts/modules/commands/queries_unittest.py:
+        * Scripts/modules/commands/queues.py:
+        * Scripts/modules/mock_bugzillatool.py:
+        * Scripts/modules/patchcollection.py:
+
 2009-11-28  Adam Barth  <abarth@webkit.org>
 
         Reviewed by Eric Seidel.
index 937e1ec1387ccf1d0e594ae27b86491b3f7f8cc2..2fbb4c26bb2c97d8666b1ade6f0b271496e48c3e 100644 (file)
@@ -43,7 +43,7 @@ from modules.committers import CommitterList
 
 # WebKit includes a built copy of BeautifulSoup in Scripts/modules
 # so this import should always succeed.
-from .BeautifulSoup import BeautifulSoup
+from .BeautifulSoup import BeautifulSoup, SoupStrainer
 
 try:
     from mechanize import Browser
@@ -309,6 +309,15 @@ class Bugzilla:
 
         return bug_ids
 
+    def _parse_attachment_ids_request_query(self, page):
+        digits = re.compile("\d+")
+        attachment_href = re.compile("attachment.cgi\?id=\d+&action=review")
+        attachment_links = SoupStrainer("a", href=attachment_href)
+        return [digits.search(tag["href"]).group(0) for tag in BeautifulSoup(page, parseOnlyThese=attachment_links)]
+
+    def _fetch_attachment_ids_request_query(self, query):
+        return self._parse_attachment_ids_request_query(urllib2.urlopen(query))
+
     def fetch_bug_ids_from_commit_queue(self):
         commit_queue_url = self.bug_server_url + "buglist.cgi?query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=commit-queue%2B"
         return self._fetch_bug_ids_advanced_query(commit_queue_url)
@@ -317,6 +326,10 @@ class Bugzilla:
         review_queue_url = self.bug_server_url + "buglist.cgi?query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review?"
         return self._fetch_bug_ids_advanced_query(review_queue_url)
 
+    def fetch_attachment_ids_from_review_queue(self):
+        review_queue_url = self.bug_server_url + "request.cgi?action=queue&type=review&group=type"
+        return self._fetch_attachment_ids_request_query(review_queue_url)
+
     def fetch_patches_from_commit_queue(self, reject_invalid_patches=False):
         patches_to_land = []
         for bug_id in self.fetch_bug_ids_from_commit_queue():
index 95964f3d6a12ce8b5767a2f80084427414788b29..50c20c56fdd5f6b0d0c2f6e40e673c441fdca411 100644 (file)
@@ -103,5 +103,56 @@ class BugzillaTest(unittest.TestCase):
         bugzilla = Bugzilla()
         self.assertEquals(27314, bugzilla._parse_bug_id_from_attachment_page(self._sample_attachment_detail_page))
 
+    _sample_request_page = """
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
+                      "http://www.w3.org/TR/html4/loose.dtd">
+<html>
+  <head>
+    <title>Request Queue</title>
+  </head>
+<body>
+
+<h3>Flag: review</h3>
+  <table class="requests" cellspacing="0" cellpadding="4" border="1">
+    <tr>
+        <th>Requester</th>
+        <th>Requestee</th>
+        <th>Bug</th>
+        <th>Attachment</th>
+        <th>Created</th>
+    </tr>
+    <tr>
+        <td>Shinichiro Hamaji &lt;hamaji&#64;chromium.org&gt;</td>
+        <td></td>
+        <td><a href="show_bug.cgi?id=30015">30015: text-transform:capitalize is failing in CSS2.1 test suite</a></td>
+        <td><a href="attachment.cgi?id=40511&amp;action=review">
+40511: Patch v0</a></td>
+        <td>2009-10-02 04:58 PST</td>
+    </tr>
+    <tr>
+        <td>Zan Dobersek &lt;zandobersek&#64;gmail.com&gt;</td>
+        <td></td>
+        <td><a href="show_bug.cgi?id=26304">26304: [GTK] Add controls for playing html5 video.</a></td>
+        <td><a href="attachment.cgi?id=40722&amp;action=review">
+40722: Media controls, the simple approach</a></td>
+        <td>2009-10-06 09:13 PST</td>
+    </tr>
+    <tr>
+        <td>Zan Dobersek &lt;zandobersek&#64;gmail.com&gt;</td>
+        <td></td>
+        <td><a href="show_bug.cgi?id=26304">26304: [GTK] Add controls for playing html5 video.</a></td>
+        <td><a href="attachment.cgi?id=40723&amp;action=review">
+40723: Adjust the media slider thumb size</a></td>
+        <td>2009-10-06 09:15 PST</td>
+    </tr>
+  </table>
+</body>
+</html>
+"""
+
+    def test_request_page_parsing(self):
+        bugzilla = Bugzilla()
+        self.assertEquals([40511, 40722, 40723], bugzilla._parse_attachment_ids_request_query(self._sample_request_page))
+
 if __name__ == '__main__':
     unittest.main()
index 2c817253b3ef667be4d5a3f4862a0329da871d9f..dbaa15916d98127ff8505e6ebc7b9219ef0c99b1 100644 (file)
@@ -78,6 +78,19 @@ class PatchesToCommit(Command):
             print "%s" % patch["url"]
 
 
+class PatchesToReview(Command):
+    name = "patches-to-review"
+    show_in_main_help = False
+    def __init__(self):
+        Command.__init__(self, "List patches that are pending review")
+
+    def execute(self, options, args, tool):
+        patch_ids = tool.bugs.fetch_attachment_ids_from_review_queue()
+        log("Patches pending review:")
+        for patch_id in patch_ids:
+            print patch_id
+
+
 class ReviewedPatches(Command):
     name = "reviewed-patches"
     show_in_main_help = False
index 90f565aaf1f0341dce2d5812639d42adc2dd896c..094c1283b32387bc47f2d728a63ae37c27197bfc 100644 (file)
@@ -40,6 +40,11 @@ class QueryCommandsTest(CommandsTest):
         expected_stderr = "Patches in commit queue:\n"
         self.assert_execute_outputs(PatchesToCommit(), None, expected_stdout, expected_stderr)
 
+    def test_patches_to_review(self):
+        expected_stdout = "197\n128\n"
+        expected_stderr = "Patches pending review:\n"
+        self.assert_execute_outputs(PatchesToReview(), None, expected_stdout, expected_stderr)
+
     def test_reviewed_patches(self):
         expected_stdout = "http://example.com/197\nhttp://example.com/128\n"
         self.assert_execute_outputs(ReviewedPatches(), [42], expected_stdout)
index f72dbac8ed415441079d6c1ad3e11677f818bd21..56077c398255b6e91508cc9c3cd2dfab4818088e 100644 (file)
@@ -164,8 +164,8 @@ class AbstractTryQueue(AbstractQueue, PersistentPatchCollectionDelegate, Landing
     def collection_name(self):
         return self.name
 
-    def fetch_potential_patches(self):
-        return self.tool.bugs.fetch_patches_from_review_queue(limit=3)
+    def fetch_potential_patch_ids(self):
+        return self.tool.bugs.fetch_attachment_ids_from_review_queue()
 
     def status_server(self):
         return self.tool.status()
@@ -178,7 +178,9 @@ class AbstractTryQueue(AbstractQueue, PersistentPatchCollectionDelegate, Landing
         self._patches = PersistentPatchCollection(self)
 
     def next_work_item(self):
-        return self._patches.next()
+        patch_id = self._patches.next()
+        if patch_id:
+            return self.tool.bugs.fetch_attachment(patch_id)
 
     def should_proceed_with_work_item(self, patch):
         raise NotImplementedError, "subclasses must implement"
index 9ebb2874d66bdfc2e1ea611685133de09bedae09..541abc8d87e10436f3ca1b483979832a33a7067d 100644 (file)
@@ -50,6 +50,9 @@ class MockBugzilla(Mock):
     def fetch_bug_ids_from_commit_queue(self):
         return [42, 75]
 
+    def fetch_attachment_ids_from_review_queue(self):
+        return [197, 128]
+
     def fetch_patches_from_commit_queue(self):
         return [self.patch1, self.patch2]
 
index 03fbea549ab99170c08cdaa0c812fc2be0718678..d8ca0db7d8e6c3c75d67fc9fa9ca7b9c8ecbd487 100644 (file)
@@ -63,7 +63,7 @@ class PersistentPatchCollectionDelegate:
     def collection_name(self):
         raise NotImplementedError, "subclasses must implement"
 
-    def fetch_potential_patches(self):
+    def fetch_potential_patch_ids(self):
         raise NotImplementedError, "subclasses must implement"
 
     def status_server(self):
@@ -79,12 +79,11 @@ class PersistentPatchCollection:
         self._status = self._delegate.status_server()
 
     def next(self):
-        patches = self._delegate.fetch_potential_patches()
-        for patch in patches:
-            last_status = self._status.patch_status(self._name, patch["id"])
+        patch_ids = self._delegate.fetch_potential_patch_ids()
+        for patch_id in patch_ids:
+            last_status = self._status.patch_status(self._name, patch_id)
             if not last_status: # FIXME: Add support for "Try again"
-                self._status.update_status(self._name, self._initial_status, patch)
-                return patch
+                return patch_id
 
     def done(self, patch):
         self._status.update_status(self._name, self._terminal_status, patch)