Skip to content
Permalink
Browse files
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.

Canonical link: https://commits.webkit.org/47475@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@56181 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
eseidel committed Mar 18, 2010
1 parent 021ced9 commit 8d29e48a7f6ab638210ca683c60aeacd69927d21
Showing 10 changed files with 233 additions and 33 deletions.
@@ -1,3 +1,41 @@
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>

Reviewed by Kenneth Rohde Christiansen.
@@ -69,24 +69,20 @@ def _cc_watchers(self, bug_id):
traceback.print_exc()
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 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 _did_error(self, patch, reason):
message = "%s: %s" % (self._error_status, reason)
self._update_status(message, patch)
# 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):
log("CAUTION: %s will discard all local changes in \"%s\"" % (self.name, self.tool.scm().checkout_root))
@@ -112,15 +108,7 @@ def process_work_item(self, work_item):
def handle_unexpected_error(self, work_item, message):
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):
self.options = options
@@ -136,15 +124,36 @@ def _update_status_for_script_error(cls, tool, state, script_error, is_error=Fal
return tool.status_server.update_status(cls.name, message, state["patch"], StringIO(output))


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)
AbstractPatchQueue.__init__(self)

# AbstractQueue methods
# AbstractPatchQueue methods

def begin_work_queue(self):
AbstractQueue.begin_work_queue(self)
AbstractPatchQueue.begin_work_queue(self)
self.committer_validator = CommitterValidator(self.tool.bugs)

def _validate_patches_in_commit_queue(self):
@@ -240,9 +249,9 @@ def handle_script_error(cls, tool, state, script_error):
validator.reject_patch_from_commit_queue(state["patch"].id(), cls._error_message_for_bug(tool, status_id, script_error))


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):
raise NotImplementedError, "subclasses must implement"
@@ -261,10 +270,10 @@ def status_server(self):
def is_terminal_status(self, status):
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)

def next_work_item(self):
@@ -37,7 +37,7 @@
from webkitpy.outputcapture import OutputCapture


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


@@ -0,0 +1,71 @@
# Copyright (c) 2009, Google Inc. All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
# met:
#
# * Redistributions of source code must retain the above copyright
# notice, this list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above
# copyright notice, this list of conditions and the following disclaimer
# in the documentation and/or other materials provided with the
# distribution.
# * Neither the name of Google Inc. nor the names of its
# contributors may be used to endorse or promote products derived from
# this software without specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

import os

from webkitpy.commands.queues import AbstractQueue
from webkitpy.irc.ircproxy import IRCProxy
from webkitpy.webkit_logging import log


class SheriffBot(AbstractQueue):
name = "sheriff-bot"

# AbstractQueue methods

def begin_work_queue(self):
AbstractQueue.begin_work_queue(self)
self.tool.ensure_irc_connected()

def work_item_log_path(self, svn_revision):
return os.path.join("%s-logs" % self.name, "%s.log" % svn_revision)

def next_work_item(self):
# FIXME: Call methods that analyze the build bots.
return None # FIXME: Should be an SVN revision number.

def should_proceed_with_work_item(self, svn_revision):
# Currently, we don't have any reasons not to proceed with work items.
return True

def process_work_item(self, svn_revision):
message = "r%s appears to have broken the build." % svn_revision
self.tool.irc().post(message)
# FIXME: What if run_webkit_patch throws an exception?
self.run_webkit_patch([
"create-rollout",
"--force-clean",
"--non-interactive",
"--parent-command=%s" % self.name,
# FIXME: We also need to CC the reviewer, committer, and contributor.
"--cc=%s" % ",".join(self.watchers),
svn_revision
])

def handle_unexpected_error(self, svn_revision, message):
log(message)
@@ -0,0 +1,43 @@
# Copyright (C) 2010 Google Inc. All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
# met:
#
# * Redistributions of source code must retain the above copyright
# notice, this list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above
# copyright notice, this list of conditions and the following disclaimer
# in the documentation and/or other materials provided with the
# distribution.
# * Neither the name of Google Inc. nor the names of its
# contributors may be used to endorse or promote products derived from
# this software without specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

import os

from webkitpy.commands.queuestest import QueuesTest
from webkitpy.commands.sheriffbot import SheriffBot
from webkitpy.mock_bugzillatool import MockBugzillaTool

class SheriffBotTest(QueuesTest):
def test_sheriff_bot(self):
expected_stderr = {
"begin_work_queue": "CAUTION: sheriff-bot will discard all local changes in \"%s\"\nRunning WebKit sheriff-bot.\n" % os.getcwd(),
"next_work_item": "",
"process_work_item": "MOCK: irc.post: r29837 appears to have broken the build.\n",
"handle_unexpected_error": "Mock error message\n"
}
self.assert_queue_outputs(SheriffBot(), work_item=29837, expected_stderr=expected_stderr)
@@ -373,6 +373,15 @@ def open_url(self, url):
pass


class MockIRC(object):

def post(self, message):
log("MOCK: irc.post: %s" % message)

def disconnect(self):
log("MOCK: irc.disconnect")


class MockStatusServer(object):

def __init__(self):
@@ -391,12 +400,20 @@ def __init__(self):
self.bugs = MockBugzilla()
self.buildbot = MockBuildBot()
self.executive = Mock()
self._irc = None
self.user = MockUser()
self._scm = MockSCM()
self.status_server = MockStatusServer()

def scm(self):
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"
@@ -263,6 +263,9 @@ def command_by_name(self, command_name):
def path(self):
raise NotImplementedError, "subclasses must implement"

def command_completed(self):
pass

def should_show_in_main_help(self, command):
return command.show_in_main_help

@@ -296,4 +299,6 @@ def main(self, argv=sys.argv):
log(failure_reason)
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
@@ -72,7 +72,7 @@ class TrivialTool(MultiCommandTool):
def __init__(self, commands=None):
MultiCommandTool.__init__(self, name="trivial-tool", commands=commands)

def path():
def path(self):
return __file__

def should_execute_command(self, command):
@@ -61,6 +61,7 @@ def __init__(self, path):
self.bugs = Bugzilla()
self.buildbot = BuildBot()
self.executive = Executive()
self._irc = None
self.user = User()
self._scm = None
self.status_server = StatusServer()
@@ -81,9 +82,24 @@ def scm(self):

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):
if not command.show_in_main_help:
return False
@@ -40,6 +40,7 @@
from webkitpy.commands.upload_unittest import *
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 *
from webkitpy.diff_parser_unittest import *

0 comments on commit 8d29e48

Please sign in to comment.