Changeset 52641 in webkit

Timestamp:

Dec 29, 2009 8:47:56 PM (14 years ago)

Author:

eric@webkit.org

Message:

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

Reviewed by Adam Barth.

Need a script to assign bugs with r+ patches to committers for landing
​https://bugs.webkit.org/show_bug.cgi?id=33009

This is just one more small tool to help in the fight against our
ever-growing list of to-be-committed patches.

• Scripts/modules/bugzilla.py:
  • Rename assign_to_email to assigned_to_email (typo).
  • Add assigned_to_email() method on Bug.
  • Add reassign_bug method.
  • Add Bugzilla.unassigned_email, eventually should move to some webkit_config.py module.
• Scripts/modules/bugzilla_unittest.py:
  • Update test after assigned_to_email rename.
• Scripts/modules/commands/commandtest.py:
  • Call bind_to_tool to that self.tool works in Command testing.
• Scripts/modules/commands/download.py:
  • Move AbstractDeclarativeCommmand multicommandtool.py, it should be part of Command.
• Scripts/modules/commands/queries_unittest.py:
  • One of the test patches is now posted by "​eric@webkit.org" which is a committer.
  • Eventually we'll mock out CommitterList and be able to better control what's a committer and what's not.
• Scripts/modules/commands/upload.py:
  • Add new assign-to-committer command.
• Scripts/modules/commands/upload_unittest.py:
  • Add basic assign-to-committer test.
• Scripts/modules/committers.py:
  • Add bugzilla_email() accessor.
• Scripts/modules/committers_unittest.py:
  • Test our assumption that bugzilla_email is the first email.
• Scripts/modules/mock_bugzillatool.py:
  • Add _id_to_object_dictionary for generating bug_cache from list of bugs.
  • Remove unused fetch_attachments_from_bug.
  • Add fetch_bug support and a bug_cache.
• Scripts/modules/multicommandtool.py:
  • Move AbstractDeclarativeCommmand here from download.py

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/modules/bugzilla.py (modified) (4 diffs)
• Scripts/modules/bugzilla_unittest.py (modified) (1 diff)
• Scripts/modules/commands/commandtest.py (modified) (1 diff)
• Scripts/modules/commands/download.py (modified) (1 diff)
• Scripts/modules/commands/queries_unittest.py (modified) (1 diff)
• Scripts/modules/commands/upload.py (modified) (2 diffs)
• Scripts/modules/commands/upload_unittest.py (modified) (1 diff)
• Scripts/modules/committers.py (modified) (1 diff)
• Scripts/modules/committers_unittest.py (modified) (1 diff)
• Scripts/modules/mock_bugzillatool.py (modified) (4 diffs)
• Scripts/modules/multicommandtool.py (modified) (2 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2009-12-29  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        Need a script to assign bugs with r+ patches to committers for landing
+        https://bugs.webkit.org/show_bug.cgi?id=33009
+
+        This is just one more small tool to help in the fight against our
+        ever-growing list of to-be-committed patches.
+
+        * Scripts/modules/bugzilla.py:
+         - Rename assign_to_email to assigned_to_email (typo).
+         - Add assigned_to_email() method on Bug.
+         - Add reassign_bug method.
+         - Add Bugzilla.unassigned_email, eventually should move to some webkit_config.py module.
+        * Scripts/modules/bugzilla_unittest.py:
+         - Update test after assigned_to_email rename.
+        * Scripts/modules/commands/commandtest.py:
+         - Call bind_to_tool to that self.tool works in Command testing.
+        * Scripts/modules/commands/download.py:
+         - Move AbstractDeclarativeCommmand multicommandtool.py, it should be part of Command.
+        * Scripts/modules/commands/queries_unittest.py:
+         - One of the test patches is now posted by "eric@webkit.org" which is a committer.
+         - Eventually we'll mock out CommitterList and be able to better control what's a committer and what's not.
+        * Scripts/modules/commands/upload.py:
+         - Add new assign-to-committer command.
+        * Scripts/modules/commands/upload_unittest.py:
+         - Add basic assign-to-committer test.
+        * Scripts/modules/committers.py:
+         - Add bugzilla_email() accessor.
+        * Scripts/modules/committers_unittest.py:
+         - Test our assumption that bugzilla_email is the first email.
+        * Scripts/modules/mock_bugzillatool.py:
+         - Add _id_to_object_dictionary for generating bug_cache from list of bugs.
+         - Remove unused fetch_attachments_from_bug.
+         - Add fetch_bug support and a bug_cache.
+        * Scripts/modules/multicommandtool.py:
+         - Move AbstractDeclarativeCommmand here from download.py
+
 2009-12-29  Adam Barth  <abarth@webkit.org>
 

trunk/WebKitTools/Scripts/modules/bugzilla.py

@@ -70 +70 @@
         self.bug_dictionary = bug_dictionary
 
+    def assigned_to_email(self):
+        return self.bug_dictionary["assigned_to_email"]
+
     # Rarely do we actually want obsolete attachments
     def attachments(self, include_obsolete=False):
@@ -94 +97 @@
         self.committers = committers
 
-    # Defaults (until we support better option parsing):
+    # FIXME: Much of this should go into some sort of config module:
     bug_server_host = "bugs.webkit.org"
     bug_server_regex = "https?://%s/" % re.sub('\.', '\\.', bug_server_host)
     bug_server_url = "https://%s/" % bug_server_host
+    unassigned_email = "webkit-unassigned@lists.webkit.org"
 
     def bug_url_for_bug_id(self, bug_id, xml=False):
@@ -139 +143 @@
         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["assigned_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')]
@@ -517 +521 @@
         self.browser.submit()
 
+    def reassign_bug(self, bug_id, assignee, comment_text=None):
+        self.authenticate()
+
+        log("Assigning bug %s to %s" % (bug_id, assignee))
+        if self.dryrun:
+            log(comment_text)
+            return
+
+        self.browser.open(self.bug_url_for_bug_id(bug_id))
+        self.browser.select_form(name="changeform")
+        if comment_text:
+            log(comment_text)
+            self.browser["comment"] = comment_text
+        self.browser["assigned_to"] = assignee
+        self.browser.submit()
+
     def reopen_bug(self, bug_id, comment_text):
         self.authenticate()

trunk/WebKitTools/Scripts/modules/bugzilla_unittest.py

@@ -175 +175 @@
         "cc_emails" : ["foo@bar.com", "example@example.com"],
         "reporter_email" : "eric@webkit.org",
-        "assign_to_email" : "webkit-unassigned@lists.webkit.org",
+        "assigned_to_email" : "webkit-unassigned@lists.webkit.org",
         "attachments" : [{
             'name': u'Patch',

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

@@ -35 +35 @@
 class CommandsTest(unittest.TestCase):
     def assert_execute_outputs(self, command, args, expected_stdout="", expected_stderr="", options=Mock(), tool=MockBugzillaTool()):
+        command.bind_to_tool(tool)
         OutputCapture().assert_outputs(self, command.execute, [options, args, tool], expected_stdout=expected_stdout, expected_stderr=expected_stderr)

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

@@ -41 +41 @@
 from modules.grammar import pluralize
 from modules.logging import error, log
-from modules.multicommandtool import Command
+from modules.multicommandtool import AbstractDeclarativeCommmand, Command
 from modules.stepsequence import StepSequence
-
-
-# FIXME: Move this to a more general location.
-class AbstractDeclarativeCommmand(Command):
-    help_text = None
-    argument_names = None
-    def __init__(self, options):
-        Command.__init__(self, self.help_text, self.argument_names, options)
 
 

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

@@ -45 +45 @@
 
     def test_patches_to_commit_queue(self):
-        expected_stdout = "http://example.com/197&action=edit\nhttp://example.com/128&action=edit\n"
-        expected_stderr = ""
+        expected_stdout = "http://example.com/197&action=edit\n"
+        expected_stderr = "128 committer = \"Eric Seidel\" <eric@webkit.org>\n"
         options = Mock()
         options.bugs = False

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

@@ -40 +40 @@
 from modules.commands.download import AbstractSequencedCommmand
 from modules.comments import bug_comment_from_svn_revision
+from modules.committers import CommitterList
 from modules.grammar import pluralize
 from modules.logging import error, log
-from modules.multicommandtool import Command
+from modules.multicommandtool import Command, AbstractDeclarativeCommmand
 
 # FIXME: Requires unit test.
@@ -53 +54 @@
         os.chdir(tool.scm().checkout_root)
         print "%s" % tool.scm().commit_message_for_this_commit().message()
+
+
+class AssignToCommitter(AbstractDeclarativeCommmand):
+    name = "assign-to-committer"
+    help_text = "Assign bug to whoever attached the most recent r+'d patch"
+
+    def _assign_bug_to_last_patch_attacher(self, bug_id):
+        committers = CommitterList()
+        bug = self.tool.bugs.fetch_bug(bug_id)
+        assigned_to_email = bug.assigned_to_email()
+        if assigned_to_email != self.tool.bugs.unassigned_email:
+            log("Bug %s is already assigned to %s (%s)." % (bug_id, assigned_to_email, committers.committer_by_email(assigned_to_email)))
+            return
+
+        # FIXME: This should call a reviewed_patches() method on bug instead of re-fetching.
+        reviewed_patches = self.tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
+        if not reviewed_patches:
+            log("Bug %s has no non-obsolete patches, ignoring." % bug_id)
+            return
+        latest_patch = reviewed_patches[-1]
+        attacher_email = latest_patch["attacher_email"]
+        committer = committers.committer_by_email(attacher_email)
+        if not committer:
+            log("Attacher %s is not a committer.  Bug %s likely needs commit-queue+." % (attacher_email, bug_id))
+            return
+
+        reassign_message = "Attachment %s was posted by a committer and has review+, assigning to %s for commit." % (latest_patch["id"], committer.full_name)
+        self.tool.bugs.reassign_bug(bug_id, committer.bugzilla_email(), reassign_message)
+
+    def execute(self, options, args, tool):
+        for bug_id in tool.bugs.fetch_bug_ids_from_needs_commit_list():
+            self._assign_bug_to_last_patch_attacher(bug_id)
 
 

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

@@ -31 +31 @@
 from modules.commands.commandtest import CommandsTest
 from modules.commands.upload import *
+from modules.mock_bugzillatool import MockBugzillaTool
 
 class UploadCommandsTest(CommandsTest):
+    def test_assign_to_committer(self):
+        tool = MockBugzillaTool()
+        expected_stderr = "Bug 75 is already assigned to foo@foo.com (None).\nBug 76 has no non-obsolete patches, ignoring.\n"
+        self.assert_execute_outputs(AssignToCommitter(), [], expected_stderr=expected_stderr, tool=tool)
+        tool.bugs.reassign_bug.assert_called_with(42, "eric@webkit.org", "Attachment 128 was posted by a committer and has review+, assigning to Eric Seidel for commit.")
+
     def test_obsolete_attachments(self):
         expected_stderr = "Obsoleting 2 old patches on bug 42\n"

trunk/WebKitTools/Scripts/modules/committers.py

@@ -37 +37 @@
             self.emails = email_or_emails
         self.can_review = False
+
+    # FIXME: We're assuming the first email is a valid bugzilla email, which might not be right.
+    def bugzilla_email(self):
+        return self.emails[0]
 
     def __str__(self):

trunk/WebKitTools/Scripts/modules/committers_unittest.py

@@ -44 +44 @@
         self.assertEqual(committer_list.reviewer_by_email('so_two@gmail.com'), reviewer)
 
+        # Test that the first email is assumed to be the Bugzilla email address (for now)
+        self.assertEqual(committer_list.committer_by_email('two@rad.com').bugzilla_email(), 'two@test.com')
+
         # Test that a known committer is not returned during reviewer lookup
         self.assertEqual(committer_list.reviewer_by_email('one@test.com'), None)

trunk/WebKitTools/Scripts/modules/mock_bugzillatool.py

@@ -31 +31 @@
 from modules.mock import Mock
 from modules.scm import CommitMessage
-
+from modules.bugzilla import Bug
+
+def _id_to_object_dictionary(objects):
+    dictionary = {}
+    for thing in objects:
+        dictionary[thing["id"]] = thing
+    return dictionary
 
 class MockBugzilla(Mock):
@@ -48 +54 @@
         "is_obsolete" : False,
         "reviewer" : "Reviewer2",
-        "attacher_email" : "Contributer2",
+        "attacher_email" : "eric@webkit.org",
     }
     bug_server_url = "http://example.com"
+    unassigned_email = "unassigned@example.com"
 
     def fetch_bug_ids_from_commit_queue(self):
@@ -60 +67 @@
     def fetch_patches_from_commit_queue(self, reject_invalid_patches=False):
         return [self.patch1, self.patch2]
+
+    def fetch_bug_ids_from_needs_commit_list(self):
+        return [42, 75, 76]
+
+    bug1 = {
+        "id" : 42,
+        "assigned_to_email" : unassigned_email,
+        "attachments" : [patch1, patch2],
+    }
+    bug2 = {
+        "id" : 75,
+        "assigned_to_email" : "foo@foo.com",
+        "attachments" : [],
+    }
+    bug3 = {
+        "id" : 76,
+        "assigned_to_email" : unassigned_email,
+        "attachments" : [],
+    }
+
+    bug_cache = _id_to_object_dictionary([bug1, bug2, bug3])
+
+    def fetch_bug(self, bug_id):
+        return Bug(self.bug_cache.get(bug_id))
 
     def fetch_patches_from_pending_commit_list(self):
@@ -68 +99 @@
             return [self.patch1, self.patch2]
         return []
-
-    def fetch_attachments_from_bug(self, bug_id):
-        if bug_id == 42:
-            return [self.patch1, self.patch2]
-        return None
 
     def fetch_patches_from_bug(self, bug_id):

trunk/WebKitTools/Scripts/modules/multicommandtool.py

@@ -39 +39 @@
 from modules.logging import log
 
+
 class Command(object):
     name = None
@@ -126 +127 @@
         return self.check_arguments_and_execute(options, args)
 
+
+# FIXME: This should just be rolled into Command.  help_text and argument_names do not need to be instance variables.
+class AbstractDeclarativeCommmand(Command):
+    help_text = None
+    argument_names = None
+    def __init__(self, options=None):
+        Command.__init__(self, self.help_text, self.argument_names, options)
+
+
 class HelpPrintingOptionParser(OptionParser):
     def __init__(self, epilog_method=None, *args, **kwargs):