Changeset 56233 in webkit

Timestamp:

Mar 19, 2010 2:44:25 AM (14 years ago)

Author:

abarth@webkit.org

Message:

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

Reviewed by Eric Seidel.

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

This patch should contain a complete SheriffBot that's capable of
saying reasonable things on IRC. I had to refactor the use of
CommitInfo to make the SheriffBot testable, but I did the minimum
necessary. We should grow webkitcheckout over time to contain the
knowledge of ChangeLogs from scm.

• Scripts/webkitpy/commands/sheriffbot.py:
• Scripts/webkitpy/commands/sheriffbot_unittest.py:
• Scripts/webkitpy/mock_bugzillatool.py:
• Scripts/webkitpy/patch/patcher.py:
• Scripts/webkitpy/webkitcheckout.py: Added.

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/commands/sheriffbot.py (modified) (2 diffs)
• Scripts/webkitpy/commands/sheriffbot_unittest.py (modified) (1 diff)
• Scripts/webkitpy/mock_bugzillatool.py (modified) (7 diffs)
• Scripts/webkitpy/patch/patcher.py (modified) (3 diffs)
• Scripts/webkitpy/webkitcheckout.py (copied) (copied from trunk/WebKitTools/Scripts/webkitpy/commands/sheriffbot_unittest.py) (2 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-03-19  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Second cut at SheriffBot
+        https://bugs.webkit.org/show_bug.cgi?id=36353
+
+        This patch should contain a complete SheriffBot that's capable of
+        saying reasonable things on IRC.  I had to refactor the use of
+        CommitInfo to make the SheriffBot testable, but I did the minimum
+        necessary.  We should grow webkitcheckout over time to contain the
+        knowledge of ChangeLogs from scm.
+
+        * Scripts/webkitpy/commands/sheriffbot.py:
+        * Scripts/webkitpy/commands/sheriffbot_unittest.py:
+        * Scripts/webkitpy/mock_bugzillatool.py:
+        * Scripts/webkitpy/patch/patcher.py:
+        * Scripts/webkitpy/webkitcheckout.py: Added.
+
 2010-03-19  Adam Barth  <abarth@webkit.org>
 

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

@@ -30 +30 @@
 
 from webkitpy.commands.queues import AbstractQueue
-from webkitpy.irc.ircproxy import IRCProxy
 from webkitpy.webkit_logging import log
 
@@ -43 +42 @@
         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 work_item_log_path(self, failure_info):
+        return os.path.join("%s-logs" % self.name, "%s.log" % failure_info["svn_revision"])
 
     def next_work_item(self):
-        # FIXME: Call methods that analyze the build bots.
-        return None # FIXME: Should be an SVN revision number.
+        for svn_revision, builders in self.tool.buildbot.revisions_causing_failures().items():
+            if self.tool.status_server.svn_revision(svn_revision):
+                continue
+            return {
+                "svn_revision": svn_revision,
+                "builders": builders
+            }
+        return None
 
-    def should_proceed_with_work_item(self, svn_revision):
+    def should_proceed_with_work_item(self, failure_info):
         # 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 process_work_item(self, failure_info):
+        svn_revision = failure_info["svn_revision"]
+        builders = failure_info["builders"]
 
-    def handle_unexpected_error(self, svn_revision, message):
+        commit_info = self.tool.checkout().commit_info_for_revision(svn_revision)
+        responsible_parties = [
+            commit_info.committer(),
+            commit_info.author(),
+            commit_info.reviewer()
+        ]
+        irc_nicknames = sorted(set([party.irc_nickname for party in responsible_parties if party and party.irc_nickname]))
+        irc_prefix = ": " if irc_nicknames else ""
+        irc_message = "%s%sr%s appears to have broken %s" % (
+            ", ".join(irc_nicknames),
+            irc_prefix,
+            svn_revision,
+            ", ".join([builder.name() for builder in builders]))
+
+        self.tool.irc().post(irc_message)
+
+        for builder in builders:
+            self.tool.status_server.update_svn_revision(svn_revision, builder.name())
+
+    def handle_unexpected_error(self, failure_info, message):
         log(message)

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

@@ -31 +31 @@
 from webkitpy.commands.queuestest import QueuesTest
 from webkitpy.commands.sheriffbot import SheriffBot
-from webkitpy.mock_bugzillatool import MockBugzillaTool
+from webkitpy.mock_bugzillatool import MockBugzillaTool, mock_builder
 
 class SheriffBotTest(QueuesTest):
     def test_sheriff_bot(self):
+        mock_work_item = {
+            "svn_revision": 29837,
+            "builders": [mock_builder]
+        }
         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",
+            "process_work_item": "MOCK: irc.post: abarth, darin, eseidel: r29837 appears to have broken Mock builder name (Tests)\n",
             "handle_unexpected_error": "Mock error message\n"
         }
-        self.assert_queue_outputs(SheriffBot(), work_item=29837, expected_stderr=expected_stderr)
+        self.assert_queue_outputs(SheriffBot(), work_item=mock_work_item, expected_stderr=expected_stderr)

trunk/WebKitTools/Scripts/webkitpy/mock_bugzillatool.py

@@ -30 +30 @@
 
 from webkitpy.bugzilla import Bug, Attachment
+from webkitpy.commitinfo import CommitInfo
 from webkitpy.committers import CommitterList, Reviewer
 from webkitpy.scm import CommitMessage
@@ -180 +181 @@
     "attachments": [_patch4, _patch5, _patch6],
 }
+
+
+class MockBuilder(object):
+
+    def name(self):
+        return "Mock builder name (Tests)"
+
+
+mock_builder = MockBuilder()
 
 
@@ -285 +295 @@
 
 
-class MockBuildBot(Mock):
+class MockBuildBot(object):
 
     def __init__(self):
@@ -307 +317 @@
         self._tree_is_on_fire = True
 
+    def revisions_causing_failures(self):
+        return {
+            "29837": [mock_builder]
+        }
 
 class MockSCM(Mock):
@@ -354 +368 @@
 
 
+class MockWebKitCheckout(object):
+
+    def commit_info_for_revision(self, svn_revision):
+        return CommitInfo(svn_revision, "eric@webkit.org", {
+            "bug_id": 42,
+            "author_name": "Adam Barth",
+            "author_email": "abarth@webkit.org",
+            "reviewer_text": "Darin Adler"
+        })
+
+
 class MockUser(object):
 
@@ -391 +416 @@
         return None
 
+    def svn_revision(self, svn_revision):
+        return None
+
     def update_status(self, queue_name, status, patch=None, results_file=None):
         return 187
+
+    def update_svn_revision(self, svn_revision, broken_bot):
+        return 191
 
 
@@ -404 +435 @@
         self.user = MockUser()
         self._scm = MockSCM()
+        self._checkout = MockWebKitCheckout()
         self.status_server = MockStatusServer()
 
     def scm(self):
         return self._scm
+
+    def checkout(self):
+        return self._checkout
 
     def ensure_irc_connected(self):

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

@@ -42 +42 @@
 from webkitpy.executive import Executive
 from webkitpy.webkit_logging import log
+from webkitpy.webkitcheckout import WebKitCheckout
 from webkitpy.multicommandtool import MultiCommandTool
 from webkitpy.scm import detect_scm_system
@@ -64 +65 @@
         self.user = User()
         self._scm = None
+        self._checkout = None
         self.status_server = StatusServer()
 
@@ -81 +83 @@
 
         return self._scm
+
+    def checkout():
+        if not self._checkout:
+            self._checkout = WebKitCheckout(self.scm())
+        return self._checkout
 
     # FIXME: Add a parameter for nickname?

trunk/WebKitTools/Scripts/webkitpy/webkitcheckout.py

@@ -1 @@
-# Copyright (C) 2010 Google Inc. All rights reserved.
-#
+# 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
+# 
+#     * 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
+#     * 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
+#     * 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
@@ -27 +27 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-import os
+from webkitpy.commitinfo import CommitInfo
 
-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)
+# This class represents the WebKit-specific parts of the checkout (like
+# ChangeLogs).
+# FIXME: Move a bunch of ChangeLog-specific processing from SCM to this object.
+class WebKitCheckout(object):
+    def __init__(self, scm):
+        self._scm = scm
+
+    def commit_info_for_revision(self, svn_revision):
+        return CommitInfo.commit_info_for_revision(self._scm, svn_revision)