Changeset 52594 in webkit

Timestamp:

Dec 27, 2009 11:59:00 PM (14 years ago)

Author:

eric@webkit.org

Message:

2009-12-27 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

Add more awesome bug-parsing logic to bugzilla.py in preparation for assign-to-committer command
​https://bugs.webkit.org/show_bug.cgi?id=32980

• Scripts/modules/bugzilla.py:
  • Add a new _parse_bug_page function and use it in fetch_attachments_from_bug
  • Replace fetch_title_from_bug with a new fetch_bug call instead.
  • Use list comprehensions where possible to reduce code duplication.
• Scripts/modules/bugzilla_unittest.py:
  • Add a minimal bug parsing test.
  • Share code between bug parsing and attachment parsing tests with _assert_dictionaries_equal
• Scripts/modules/commands/upload.py:
  • Use fetch_bug(bug_id)title? instead of fetch_title_from_bug

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/modules/bugzilla.py (modified) (4 diffs)
• Scripts/modules/bugzilla_unittest.py (modified) (2 diffs)
• Scripts/modules/commands/upload.py (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2009-12-27  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        Add more awesome bug-parsing logic to bugzilla.py in preparation for assign-to-committer command
+        https://bugs.webkit.org/show_bug.cgi?id=32980
+
+        * Scripts/modules/bugzilla.py:
+         - Add a new _parse_bug_page function and use it in fetch_attachments_from_bug
+         - Replace fetch_title_from_bug with a new fetch_bug call instead.
+         - Use list comprehensions where possible to reduce code duplication.
+        * Scripts/modules/bugzilla_unittest.py:
+         - Add a minimal bug parsing test.
+         - Share code between bug parsing and attachment parsing tests with _assert_dictionaries_equal
+        * Scripts/modules/commands/upload.py:
+         - Use fetch_bug(bug_id)["title"] instead of fetch_title_from_bug
+
 2009-12-27  Adam Barth  <abarth@webkit.org>
 

trunk/WebKitTools/Scripts/modules/bugzilla.py

@@ -113 +113 @@
         return attachment
 
-    def fetch_attachments_from_bug(self, bug_id):
+    def _parse_bug_page(self, page):
+        soup = BeautifulSoup(page)
+        bug = {}
+        bug["id"] = int(soup.find("bug_id").string)
+        bug["title"] = unicode(soup.find("short_desc").string)
+        bug["reporter_email"] = str(soup.find("reporter").string)
+        bug["assign_to_email"] = str(soup.find("assigned_to").string)
+        bug["cc_emails"] = [str(element.string) for element in soup.findAll('cc')]
+        bug["attachments"] = [self._parse_attachment_element(element, bug_id) for element in soup.findAll('attachment')]
+        return bug
+
+    def fetch_bug(self, bug_id):
         bug_url = self.bug_url_for_bug_id(bug_id, xml=True)
         log("Fetching: %s" % bug_url)
-
         page = self.browser.open(bug_url)
-        soup = BeautifulSoup(page)
-
-        attachments = []
-        for element in soup.findAll('attachment'):
-            attachment = self._parse_attachment_element(element, bug_id)
-            attachments.append(attachment)
-        return attachments
+        return self._parse_bug_page(page)
+
+    # This should be an attachments() method on a Bug object.
+    def fetch_attachments_from_bug(self, bug_id):
+        bug = self.fetch_bug(bug_id)
+        if not bug:
+            return None
+        return bug["attachments"]
 
     def _parse_bug_id_from_attachment_page(self, page):
@@ -156 +167 @@
         return None # This should never be hit.
 
-    def fetch_title_from_bug(self, bug_id):
-        bug_url = self.bug_url_for_bug_id(bug_id, xml=True)
-        page = self.browser.open(bug_url)
-        soup = BeautifulSoup(page)
-        return soup.find('short_desc').string
-
     def fetch_patches_from_bug(self, bug_id):
-        patches = []
-        for attachment in self.fetch_attachments_from_bug(bug_id):
-            if attachment['is_patch'] and not attachment['is_obsolete']:
-                patches.append(attachment)
-        return patches
+        return [patch for patch in self.fetch_attachments_from_bug(bug_id) if patch['is_patch'] and not patch['is_obsolete']]
 
     # _view_source_link belongs in some sort of webkit_config.py module.
@@ -216 +217 @@
 
     def fetch_unreviewed_patches_from_bug(self, bug_id):
-        unreviewed_patches = []
-        for attachment in self.fetch_attachments_from_bug(bug_id):
-            if attachment.get('review') == '?' and not attachment['is_obsolete']:
-                unreviewed_patches.append(attachment)
-        return unreviewed_patches
-
+        return [patch for patch in self.fetch_attachments_from_bug(bug_id) if patch.get('review') == '?' and not patch['is_obsolete']]
+
+    # FIXME: fetch_reviewed_patches_from_bug and fetch_commit_queue_patches_from_bug
+    # should share more code and use list comprehensions.
     def fetch_reviewed_patches_from_bug(self, bug_id, reject_invalid_patches=False):
         reviewed_patches = []
@@ -239 +238 @@
         page = self.browser.open(query)
         soup = BeautifulSoup(page)
-
-        bug_ids = []
-        # Grab the cells in the first column (which happens to be the bug ids)
-        for bug_link_cell in soup('td', "first-child"): # tds with the class "first-child"
-            bug_link = bug_link_cell.find("a")
-            bug_ids.append(int(bug_link.string)) # the contents happen to be the bug id
-
-        return bug_ids
+        # The contents of the <a> inside the cells in the first column happen to be the bug id.
+        return [int(bug_link_cell.find("a").string) for bug_link_cell in soup('td', "first-child")]
 
     def _parse_attachment_ids_request_query(self, page):

trunk/WebKitTools/Scripts/modules/bugzilla_unittest.py

@@ -51 +51 @@
 
 class BugzillaTest(unittest.TestCase):
-
     _example_attachment = '''
         <attachment
@@ -105 +104 @@
         self.assertEquals(None, parse_bug_id("http://bugs.webkit.org/show_bug.cgi?ctype=xml&id=12345"))
 
-
+    _example_bug = """
+<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
+<!DOCTYPE bugzilla SYSTEM "https://bugs.webkit.org/bugzilla.dtd">
+<bugzilla version="3.2.3"
+          urlbase="https://bugs.webkit.org/"
+          maintainer="admin@webkit.org"
+          exporter="eric@webkit.org"
+>
+    <bug>
+          <bug_id>32585</bug_id>
+          <creation_ts>2009-12-15 15:17 PST</creation_ts>
+          <short_desc>bug to test bugzilla-tool and commit-queue failures</short_desc>
+          <delta_ts>2009-12-27 21:04:50 PST</delta_ts>
+          <reporter_accessible>1</reporter_accessible>
+          <cclist_accessible>1</cclist_accessible>
+          <classification_id>1</classification_id>
+          <classification>Unclassified</classification>
+          <product>WebKit</product>
+          <component>Tools / Tests</component>
+          <version>528+ (Nightly build)</version>
+          <rep_platform>PC</rep_platform>
+          <op_sys>Mac OS X 10.5</op_sys>
+          <bug_status>NEW</bug_status>
+          <priority>P2</priority>
+          <bug_severity>Normal</bug_severity>
+          <target_milestone>---</target_milestone>
+          <everconfirmed>1</everconfirmed>
+          <reporter name="Eric Seidel">eric@webkit.org</reporter>
+          <assigned_to name="Nobody">webkit-unassigned@lists.webkit.org</assigned_to>
+          <cc>foo@bar.com</cc>
+    <cc>example@example.com</cc>
+          <long_desc isprivate="0">
+            <who name="Eric Seidel">eric@webkit.org</who>
+            <bug_when>2009-12-15 15:17:28 PST</bug_when>
+            <thetext>bug to test bugzilla-tool and commit-queue failures
+
+Ignore this bug.  Just for testing failure modes of bugzilla-tool and the commit-queue.</thetext>
+          </long_desc>
+    </bug>
+</bugzilla>
+"""
+    _expected_example_bug_parsing = {
+        "id" : 32585,
+        "title" : u"bug to test bugzilla-tool and commit-queue failures",
+        "cc_emails" : ["foo@bar.com", "example@example.com"],
+        "reporter_email" : "eric@webkit.org",
+        "assign_to_email" : "webkit-unassigned@lists.webkit.org",
+        "attachments" : [],
+    }
+
+    def _assert_dictionaries_equal(self, actual, expected):
+        # Make sure we aren't parsing more or less than we expect
+        self.assertEquals(sorted(actual.keys()), sorted(expected.keys()))
+
+        for key, expected_value in expected.items():
+            self.assertEquals(actual[key], expected_value, ("Failure for key: %s: Actual='%s' Expected='%s'" % (key, actual[key], expected_value)))
+
+    def test_bug_parsing(self):
+        bug = Bugzilla()._parse_bug_page(self._example_bug)
+        self._assert_dictionaries_equal(bug, self._expected_example_bug_parsing)
+
+    # This could be combined into test_bug_parsing later if desired.
     def test_attachment_parsing(self):
         bugzilla = Bugzilla()
-
         soup = BeautifulSoup(self._example_attachment)
         attachment_element = soup.find("attachment")
         attachment = bugzilla._parse_attachment_element(attachment_element, self._expected_example_attachment_parsing['bug_id'])
         self.assertTrue(attachment)
-
-        # Make sure we aren't parsing more or less than we expect
-        self.assertEquals(sorted(attachment.keys()), sorted(self._expected_example_attachment_parsing.keys()))
-
-        for key, expected_value in self._expected_example_attachment_parsing.items():
-            self.assertEquals(attachment[key], expected_value, ("Failure for key: %s: Actual='%s' Expected='%s'" % (key, attachment[key], expected_value)))
+        self._assert_dictionaries_equal(attachment, self._expected_example_attachment_parsing)
 
     _sample_attachment_detail_page = """

trunk/WebKitTools/Scripts/modules/commands/upload.py

@@ -223 +223 @@
             (bug_id, svn_revision) = self._determine_bug_id_and_svn_revision(tool, bug_id, svn_revision)
 
-        log("Bug: <%s> %s" % (tool.bugs.short_bug_url_for_bug_id(bug_id), tool.bugs.fetch_title_from_bug(bug_id)))
+        log("Bug: <%s> %s" % (tool.bugs.short_bug_url_for_bug_id(bug_id), tool.bugs.fetch_bug(bug_id)["title"]))
         log("Revision: %s" % svn_revision)