Changeset 56181 in webkit

Timestamp:

Mar 18, 2010 12:49:36 PM (14 years ago)

Author:

eric@webkit.org

Message:

2010-03-18 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

First cut at SheriffBot
​https://bugs.webkit.org/show_bug.cgi?id=36253

This patch contains a first attempt at writing a sheriff bot.
Currently, we're missing the logic that actually finds the SVN revision
numbers to complain about, but once we have that, we'll have the rest
of the infrustructure to ping IRC and to file bugs.

There's a lot to fill in for the SheriffBot, but this patch give us the
framework in which to do it.

This patch required a bit of refactoring of AbstractQueue because
SheriffBot is the first bot that doesn't process patches (it processes
SVN revisions). Accordingly, I've factored out AbstractPatchQueue to
hold the parts of AbstractQueue that are specific to dealing with
patches. Some of the choices here might not be obvious yet, but we can
tweak them as our needs become clearer.

• Scripts/webkitpy/commands/queues.py:
• Scripts/webkitpy/commands/queues_unittest.py:
• Scripts/webkitpy/commands/sheriffbot.py: Added.
• Scripts/webkitpy/commands/sheriffbot_unittest.py: Added.
• Scripts/webkitpy/mock_bugzillatool.py:

Added a MockIRC object to the mock tool.

• Scripts/webkitpy/multicommandtool.py:

Added a finalize method so the tool can disconnect from IRC
cleanly instead of just droping the socket.

• Scripts/webkitpy/multicommandtool_unittest.py:
• Scripts/webkitpy/patch/patcher.py:

Added support for talking to IRC.

• Scripts/webkitpy/unittests.py:

We should add a commands/unittests.py file at some point to make
the commands module more self-contained.

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/commands/queues.py (modified) (5 diffs)
• Scripts/webkitpy/commands/queues_unittest.py (modified) (1 diff)
• Scripts/webkitpy/commands/sheriffbot.py (added)
• Scripts/webkitpy/commands/sheriffbot_unittest.py (added)
• Scripts/webkitpy/mock_bugzillatool.py (modified) (3 diffs)
• Scripts/webkitpy/multicommandtool.py (modified) (2 diffs)
• Scripts/webkitpy/multicommandtool_unittest.py (modified) (1 diff)
• Scripts/webkitpy/patch/patcher.py (modified) (2 diffs)
• Scripts/webkitpy/unittests.py (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-03-18  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        First cut at SheriffBot
+        https://bugs.webkit.org/show_bug.cgi?id=36253
+
+        This patch contains a first attempt at writing a sheriff bot.
+        Currently, we're missing the logic that actually finds the SVN revision
+        numbers to complain about, but once we have that, we'll have the rest
+        of the infrustructure to ping IRC and to file bugs.
+
+        There's a lot to fill in for the SheriffBot, but this patch give us the
+        framework in which to do it.
+
+        This patch required a bit of refactoring of AbstractQueue because
+        SheriffBot is the first bot that doesn't process patches (it processes
+        SVN revisions).  Accordingly, I've factored out AbstractPatchQueue to
+        hold the parts of AbstractQueue that are specific to dealing with
+        patches.  Some of the choices here might not be obvious yet, but we can
+        tweak them as our needs become clearer.
+
+        * Scripts/webkitpy/commands/queues.py:
+        * Scripts/webkitpy/commands/queues_unittest.py:
+        * Scripts/webkitpy/commands/sheriffbot.py: Added.
+        * Scripts/webkitpy/commands/sheriffbot_unittest.py: Added.
+        * Scripts/webkitpy/mock_bugzillatool.py:
+            Added a MockIRC object to the mock tool.
+        * Scripts/webkitpy/multicommandtool.py:
+            Added a finalize method so the tool can disconnect from IRC
+            cleanly instead of just droping the socket.
+        * Scripts/webkitpy/multicommandtool_unittest.py:
+        * Scripts/webkitpy/patch/patcher.py:
+            Added support for talking to IRC.
+        * Scripts/webkitpy/unittests.py:
+            We should add a commands/unittests.py file at some point to make
+            the commands module more self-contained.
+
 2010-03-18  Antti Koivisto  <koivisto@iki.fi>
 

trunk/WebKitTools/Scripts/webkitpy/commands/queues.py

@@ -70 +70 @@
             log("Failed to CC watchers.")
 
-    def _update_status(self, message, patch=None, results_file=None):
-        self.tool.status_server.update_status(self.name, message, patch, results_file)
-
-    def _did_pass(self, patch):
-        self._update_status(self._pass_status, patch)
-
-    def _did_fail(self, patch):
-        self._update_status(self._fail_status, patch)
-
-    def _did_error(self, patch, reason):
-        message = "%s: %s" % (self._error_status, reason)
-        self._update_status(message, patch)
+    def run_webkit_patch(self, args):
+        webkit_patch_args = [self.tool.path()]
+        # FIXME: This is a hack, we should have a more general way to pass global options.
+        webkit_patch_args += ["--status-host=%s" % self.tool.status_server.host]
+        webkit_patch_args += map(str, args)
+        self.tool.executive.run_and_throw_if_fail(webkit_patch_args)
+
+    # QueueEngineDelegate methods
 
     def queue_log_path(self):
         return "%s.log" % self.name
 
-    def work_item_log_path(self, patch):
-        return os.path.join("%s-logs" % self.name, "%s.log" % patch.bug_id())
+    def work_item_log_path(self, work_item):
+        raise NotImplementedError, "subclasses must implement"
 
     def begin_work_queue(self):
@@ -113 +109 @@
         raise NotImplementedError, "subclasses must implement"
 
-    def run_webkit_patch(self, args):
-        webkit_patch_args = [self.tool.path()]
-        # FIXME: This is a hack, we should have a more general way to pass global options.
-        webkit_patch_args += ["--status-host=%s" % self.tool.status_server.host]
-        webkit_patch_args += map(str, args)
-        self.tool.executive.run_and_throw_if_fail(webkit_patch_args)
-
-    def log_progress(self, patch_ids):
-        log("%s in %s [%s]" % (pluralize("patch", len(patch_ids)), self.name, ", ".join(map(str, patch_ids))))
+    # Command methods
 
     def execute(self, options, args, tool, engine=QueueEngine):
@@ -137 +125 @@
 
 
-class CommitQueue(AbstractQueue, StepSequenceErrorHandler):
+class AbstractPatchQueue(AbstractQueue):
+    def _update_status(self, message, patch=None, results_file=None):
+        self.tool.status_server.update_status(self.name, message, patch, results_file)
+
+    def _did_pass(self, patch):
+        self._update_status(self._pass_status, patch)
+
+    def _did_fail(self, patch):
+        self._update_status(self._fail_status, patch)
+
+    def _did_error(self, patch, reason):
+        message = "%s: %s" % (self._error_status, reason)
+        self._update_status(message, patch)
+
+    def work_item_log_path(self, patch):
+        return os.path.join("%s-logs" % self.name, "%s.log" % patch.bug_id())
+
+    def log_progress(self, patch_ids):
+        log("%s in %s [%s]" % (pluralize("patch", len(patch_ids)), self.name, ", ".join(map(str, patch_ids))))
+
+
+class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
     name = "commit-queue"
     def __init__(self):
-        AbstractQueue.__init__(self)
-
-    # AbstractQueue methods
+        AbstractPatchQueue.__init__(self)
+
+    # AbstractPatchQueue methods
 
     def begin_work_queue(self):
-        AbstractQueue.begin_work_queue(self)
+        AbstractPatchQueue.begin_work_queue(self)
         self.committer_validator = CommitterValidator(self.tool.bugs)
 
@@ -241 +250 @@
 
 
-class AbstractReviewQueue(AbstractQueue, PersistentPatchCollectionDelegate, StepSequenceErrorHandler):
+class AbstractReviewQueue(AbstractPatchQueue, PersistentPatchCollectionDelegate, StepSequenceErrorHandler):
     def __init__(self, options=None):
-        AbstractQueue.__init__(self, options)
+        AbstractPatchQueue.__init__(self, options)
 
     def _review_patch(self, patch):
@@ -262 +271 @@
         return status == "Pass" or status == "Fail" or status.startswith("Error:")
 
-    # AbstractQueue methods
+    # AbstractPatchQueue methods
 
     def begin_work_queue(self):
-        AbstractQueue.begin_work_queue(self)
+        AbstractPatchQueue.begin_work_queue(self)
         self._patches = PersistentPatchCollection(self)
 

trunk/WebKitTools/Scripts/webkitpy/commands/queues_unittest.py

@@ -38 +38 @@
 
 
-class TestQueue(AbstractQueue):
+class TestQueue(AbstractPatchQueue):
     name = "test-queue"
 

trunk/WebKitTools/Scripts/webkitpy/mock_bugzillatool.py

@@ -374 +374 @@
 
 
+class MockIRC(object):
+
+    def post(self, message):
+        log("MOCK: irc.post: %s" % message)
+
+    def disconnect(self):
+        log("MOCK: irc.disconnect")
+
+
 class MockStatusServer(object):
 
@@ -392 +401 @@
         self.buildbot = MockBuildBot()
         self.executive = Mock()
+        self._irc = None
         self.user = MockUser()
         self._scm = MockSCM()
@@ -399 +409 @@
         return self._scm
 
+    def ensure_irc_connected(self):
+        if not self._irc:
+            self._irc = MockIRC()
+
+    def irc(self):
+        return self._irc
+
     def path(self):
         return "echo"

trunk/WebKitTools/Scripts/webkitpy/multicommandtool.py

@@ -264 +264 @@
         raise NotImplementedError, "subclasses must implement"
 
+    def command_completed(self):
+        pass
+
     def should_show_in_main_help(self, command):
         return command.show_in_main_help
@@ -297 +300 @@
             return 0 # FIXME: Should this really be 0?
 
-        return command.check_arguments_and_execute(options, args, self)
+        result = command.check_arguments_and_execute(options, args, self)
+        self.command_completed()
+        return result

trunk/WebKitTools/Scripts/webkitpy/multicommandtool_unittest.py

@@ -73 +73 @@
         MultiCommandTool.__init__(self, name="trivial-tool", commands=commands)
 
-    def path():
+    def path(self):
         return __file__
 

trunk/WebKitTools/Scripts/webkitpy/patch/patcher.py

@@ -62 +62 @@
         self.buildbot = BuildBot()
         self.executive = Executive()
+        self._irc = None
         self.user = User()
         self._scm = None
@@ -82 +83 @@
         return self._scm
 
+    # FIXME: Add a parameter for nickname?
+    def ensure_irc_connected(self):
+        if not self._irc:
+            self._irc = IRCProxy()
+
+    def irc(self):
+        # We don't automatically construct IRCProxy here because constructing
+        # IRCProxy actually connects to IRC.  We want clients to explicitly
+        # connect to IRC.
+        return self._irc
+
     def path(self):
         return self._path
+
+    def command_completed(self):
+        if self._irc:
+            self._irc.disconnect()
 
     def should_show_in_main_help(self, command):

trunk/WebKitTools/Scripts/webkitpy/unittests.py

@@ -41 +41 @@
 from webkitpy.commands.queries_unittest import *
 from webkitpy.commands.queues_unittest import *
+from webkitpy.commands.sheriffbot_unittest import *
 from webkitpy.committers_unittest import *
 from webkitpy.credentials_unittest import *