Changeset 57125 in webkit

Timestamp:

Apr 5, 2010 10:52:51 PM (14 years ago)

Author:

abarth@webkit.org

Message:

2010-04-05 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

SheriffBot should force_build builders that are idle and have failed exactly once
​https://bugs.webkit.org/show_bug.cgi?id=37059

We can get into a deadlocked state where the commit-queue is stopped
because the builders are red but the SheriffBot hasn't taken action
because the builder has failed only once. The SheriffBot should force
build idle builders that have failed exactly once to either turn the
tree green again (if the test was flaky) or trigger the "failed twice"
remedies (IRC and bug posts).

• Scripts/webkitpy/common/net/buildbot.py:
• Scripts/webkitpy/tool/bot/sheriff.py:
• Scripts/webkitpy/tool/bot/sheriff_unittest.py:
• Scripts/webkitpy/tool/commands/sheriffbot.py:
• Scripts/webkitpy/tool/mocktool.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/common/net/buildbot.py (modified) (1 diff)
• Scripts/webkitpy/tool/bot/sheriff.py (modified) (1 diff)
• Scripts/webkitpy/tool/bot/sheriff_unittest.py (modified) (2 diffs)
• Scripts/webkitpy/tool/commands/sheriffbot.py (modified) (2 diffs)
• Scripts/webkitpy/tool/mocktool.py (modified) (2 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-04-05  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        SheriffBot should force_build builders that are idle and have failed exactly once
+        https://bugs.webkit.org/show_bug.cgi?id=37059
+
+        We can get into a deadlocked state where the commit-queue is stopped
+        because the builders are red but the SheriffBot hasn't taken action
+        because the builder has failed only once.  The SheriffBot should force
+        build idle builders that have failed exactly once to either turn the
+        tree green again (if the test was flaky) or trigger the "failed twice"
+        remedies (IRC and bug posts).
+
+        * Scripts/webkitpy/common/net/buildbot.py:
+        * Scripts/webkitpy/tool/bot/sheriff.py:
+        * Scripts/webkitpy/tool/bot/sheriff_unittest.py:
+        * Scripts/webkitpy/tool/commands/sheriffbot.py:
+        * Scripts/webkitpy/tool/mocktool.py:
+
 2010-04-05  Chris Jerdonek  <cjerdonek@webkit.org>
 

trunk/WebKitTools/Scripts/webkitpy/common/net/buildbot.py

@@ -370 +370 @@
         return [builder["name"] for builder in self.red_core_builders()]
 
+    def idle_red_core_builders(self):
+        return [builder for builder in self.red_core_builders() if builder["activity"] == "idle"]
+
     def core_builders_are_green(self):
         return not self.red_core_builders()

trunk/WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py

@@ -108 +108 @@
                                             comment,
                                             cc=self._sheriffbot.watchers)
+
+    # FIXME: Should some of this logic be on BuildBot?
+    def provoke_flaky_builders(self, revisions_causing_failures):
+        # We force_build builders that are red but have not "failed" (i.e.,
+        # been red twice). We do this to avoid a deadlock situation where a
+        # flaky test blocks the commit-queue and there aren't any other
+        # patches being landed to re-spin the builder.
+        failed_builders = sum([revisions_causing_failures[key] for
+                               key in revisions_causing_failures.keys()], [])
+        failed_builder_names = \
+            set([builder.name() for builder in failed_builders])
+        idle_red_builder_names = \
+            set([builder["name"]
+                 for builder in self._tool.buildbot.idle_red_core_builders()])
+
+        # We only want to provoke these builders if they are idle and have not
+        # yet "failed" (i.e., been red twice) to avoid overloading the bots.
+        flaky_builder_names = idle_red_builder_names - failed_builder_names
+
+        for name in flaky_builder_names:
+            flaky_builder = self._tool.buildbot.builder_with_name(name)
+            flaky_builder.force_build(username=self._sheriffbot.name,
+                                      comments="Probe for flakiness.")

trunk/WebKitTools/Scripts/webkitpy/tool/bot/sheriff_unittest.py

@@ -38 +38 @@
 
 class MockSheriffBot(object):
+    name = "mock-sheriff-bot"
     watchers = [
         "watcher@example.com",
@@ -77 +78 @@
         expected_stderr = u"MOCK bug comment: bug_id=1234, cc=['watcher@example.com']\n--- Begin comment ---\\http://trac.webkit.org/changeset/4321 might have broken Foo and Bar\n--- End comment ---\n\nMOCK bug comment: bug_id=1234, cc=['watcher@example.com']\n--- Begin comment ---\\http://trac.webkit.org/changeset/4321 might have broken Foo and Bar\n--- End comment ---\n\nMOCK bug comment: bug_id=1234, cc=['watcher@example.com']\n--- Begin comment ---\\http://trac.webkit.org/changeset/4321 might have broken Foo and Bar\nThe following changes are on the blame list:\nhttp://trac.webkit.org/changeset/841\nhttp://trac.webkit.org/changeset/5646\n--- End comment ---\n\n"
         OutputCapture().assert_outputs(self, run, expected_stderr=expected_stderr)
+
+    def test_provoke_flaky_builders(self):
+        def run():
+            tool = MockTool()
+            tool.buildbot.light_tree_on_fire()
+            sheriff = Sheriff(tool, MockSheriffBot())
+            revisions_causing_failures = {}
+            sheriff.provoke_flaky_builders(revisions_causing_failures)
+        expected_stderr = "MOCK: force_build: name=Builder2, username=mock-sheriff-bot, comments=Probe for flakiness.\n"
+        OutputCapture().assert_outputs(self, run, expected_stderr=expected_stderr)

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

@@ -62 +62 @@
         self._update()
         new_failures = {}
-        for svn_revision, builders in self.tool.buildbot.revisions_causing_failures().items():
+        revisions_causing_failures = self.tool.buildbot.revisions_causing_failures()
+        for svn_revision, builders in revisions_causing_failures.items():
             if self.tool.status_server.svn_revision(svn_revision):
                 # FIXME: We should re-process the work item after some time delay.
@@ -68 +69 @@
                 continue
             new_failures[svn_revision] = builders
+        self._sheriff.provoke_flaky_builders(revisions_causing_failures)
         return new_failures
 

trunk/WebKitTools/Scripts/webkitpy/tool/mocktool.py

@@ -297 +297 @@
 
 
+class MockBuilder(object):
+    def __init__(self, name):
+        self._name = name
+
+    def force_build(self, username, comments):
+        log("MOCK: force_build: name=%s, username=%s, comments=%s" % (
+            self._name, username, comments))
+
+
 class MockBuildBot(object):
-
     def __init__(self):
-        self._tree_is_on_fire = False
-
-    def builder_statuses(self):
-        return [{
+        self._mock_builder1_status = {
             "name": "Builder1",
             "is_green": True,
-        }, {
+            "activity": "building",
+        }
+        self._mock_builder2_status = {
             "name": "Builder2",
-            "is_green": not self._tree_is_on_fire,
-        }]
+            "is_green": True,
+            "activity": "idle",
+        }
+
+    def builder_with_name(self, name):
+        return MockBuilder(name)
+
+    def builder_statuses(self):
+        return [
+            self._mock_builder1_status,
+            self._mock_builder2_status,
+        ]
 
     def red_core_builders_names(self):
-        if self._tree_is_on_fire:
-            return "Builder2"
+        if not self._mock_builder2_status["is_green"]:
+            return [self._mock_builder2_status["name"]]
+        return []
+
+    def red_core_builders(self):
+        if not self._mock_builder2_status["is_green"]:
+            return [self._mock_builder2_status]
+        return []
+
+    def idle_red_core_builders(self):
+        if not self._mock_builder2_status["is_green"]:
+            return [self._mock_builder2_status]
         return []
 
@@ -320 +347 @@
 
     def light_tree_on_fire(self):
-        self._tree_is_on_fire = True
+        self._mock_builder2_status["is_green"] = False
 
     def revisions_causing_failures(self):