[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

abarth at webkit.org abarth at webkit.org
Wed Dec 22 13:56:27 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit d1d17d35e3c02815069a198309fc2d58b519782e
Author: abarth at webkit.org <abarth at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Thu Sep 30 02:53:43 2010 +0000

    2010-09-29  Adam Barth  <abarth at webkit.org>
    
            Reviewed by Eric Seidel.
    
            Move more SheriffBot smarts into FailureMap
            https://bugs.webkit.org/show_bug.cgi?id=46703
    
            This patch pushes the FailureMap model object further into the
            SheriffBot machine.  In addition, it moves a couple operations on this
            object from SheriffBot itself to the model.
    
            Eventually, FailureMap will be the canonical context object for
            SheriffBot operations.  FailureMap represents a map of the current
            failures on the bots that might require remediation.
    
            * Scripts/webkitpy/common/net/failuremap.py:
            * Scripts/webkitpy/common/net/regressionwindow.py:
            * Scripts/webkitpy/tool/commands/queries.py:
            * Scripts/webkitpy/tool/commands/sheriffbot.py:
            * Scripts/webkitpy/tool/mocktool.py:
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@68740 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 533ca3c..83a26aa 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,24 @@
+2010-09-29  Adam Barth  <abarth at webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Move more SheriffBot smarts into FailureMap
+        https://bugs.webkit.org/show_bug.cgi?id=46703
+
+        This patch pushes the FailureMap model object further into the
+        SheriffBot machine.  In addition, it moves a couple operations on this
+        object from SheriffBot itself to the model.
+
+        Eventually, FailureMap will be the canonical context object for
+        SheriffBot operations.  FailureMap represents a map of the current
+        failures on the bots that might require remediation.
+
+        * Scripts/webkitpy/common/net/failuremap.py:
+        * Scripts/webkitpy/common/net/regressionwindow.py:
+        * Scripts/webkitpy/tool/commands/queries.py:
+        * Scripts/webkitpy/tool/commands/sheriffbot.py:
+        * Scripts/webkitpy/tool/mocktool.py:
+
 2010-09-29  Tony Chang  <tony at chromium.org>
 
         Reviewed by James Robinson.
diff --git a/WebKitTools/Scripts/webkitpy/common/net/failuremap.py b/WebKitTools/Scripts/webkitpy/common/net/failuremap.py
index 98e4b8f..b2efe08 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/failuremap.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/failuremap.py
@@ -37,6 +37,14 @@ class FailureMap(object):
             'regression_window': regression_window,
         })
 
+    def is_empty(self):
+        return not self._failures
+
+    def failing_revisions(self):
+        failing_revisions = [failure_info['regression_window'].revisions()
+                             for failure_info in self._failures]
+        return sorted(set(sum(failing_revisions, [])))
+
     def revisions_causing_failures(self):
         revision_to_failing_bots = {}
         for failure_info in self._failures:
@@ -46,3 +54,28 @@ class FailureMap(object):
                 failing_bots.append(failure_info['builder'])
                 revision_to_failing_bots[revision] = failing_bots
         return revision_to_failing_bots
+
+    def _old_failures(self, is_old_failure):
+        return filter(lambda revision: is_old_failure(revision),
+                      self.failing_revisions())
+
+    def _builders_failing_because_of(self, revisions):
+        revision_set = set(revisions)
+        return [failure_info['builder'] for failure_info in self._failures
+                if revision_set.intersection(
+                    failure_info['regression_window'].revisions())]
+
+    # FIXME: We should re-process old failures after some time delay.
+    # https://bugs.webkit.org/show_bug.cgi?id=36581
+    def filter_out_old_failures(self, is_old_failure):
+        old_failures = self._old_failures(is_old_failure)
+        old_failing_builder_names = set([builder.name()
+            for builder in self._builders_failing_because_of(old_failures)])
+
+        # We filter out all the failing builders that could have been caused
+        # by old_failures.  We could miss some new failures this way, but
+        # emperically, this reduces the amount of spam we generate.
+        failures = self._failures
+        self._failures = [failure_info for failure_info in failures
+            if failure_info['builder'].name() not in old_failing_builder_names]
+        self._cache = {}
diff --git a/WebKitTools/Scripts/webkitpy/common/net/failuremap_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/failuremap_unittest.py
new file mode 100644
index 0000000..b79c687
--- /dev/null
+++ b/WebKitTools/Scripts/webkitpy/common/net/failuremap_unittest.py
@@ -0,0 +1,77 @@
+# 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 unittest
+
+from webkitpy.common.net.buildbot import Build
+from webkitpy.common.net.failuremap import *
+from webkitpy.common.net.regressionwindow import RegressionWindow
+from webkitpy.tool.mocktool import MockBuilder
+
+
+class FailureMapTest(unittest.TestCase):
+    builder1 = MockBuilder("Builder1")
+    builder2 = MockBuilder("Builder2")
+
+    build1a = Build(builder1, build_number=22, revision=1233, is_green=True)
+    build1b = Build(builder1, build_number=23, revision=1234, is_green=False)
+    build2a = Build(builder2, build_number=89, revision=1233, is_green=True)
+    build2b = Build(builder2, build_number=90, revision=1235, is_green=False)
+
+    regression_window1 = RegressionWindow(build1a, build1b)
+    regression_window2 = RegressionWindow(build2a, build2b)
+
+    def _make_failure_map(self):
+        failure_map = FailureMap()
+        failure_map.add_regression_window(self.builder1, self.regression_window1)
+        failure_map.add_regression_window(self.builder2, self.regression_window2)
+        return failure_map
+
+    def test_failing_revisions(self):
+        failure_map = self._make_failure_map()
+        self.assertEquals(failure_map.failing_revisions(), [1234, 1235])
+
+    def test_new_failures(self):
+        failure_map = self._make_failure_map()
+        failure_map.filter_out_old_failures(lambda revision: False)
+        self.assertEquals(failure_map.revisions_causing_failures(), {
+            1234: [self.builder1, self.builder2],
+            1235: [self.builder2],
+        })
+
+    def test_new_failures_with_old_revisions(self):
+        failure_map = self._make_failure_map()
+        failure_map.filter_out_old_failures(lambda revision: revision == 1234)
+        self.assertEquals(failure_map.revisions_causing_failures(), {})
+
+    def test_new_failures_with_more_old_revisions(self):
+        failure_map = self._make_failure_map()
+        failure_map.filter_out_old_failures(lambda revision: revision == 1235)
+        self.assertEquals(failure_map.revisions_causing_failures(), {
+            1234: [self.builder1],
+        })
diff --git a/WebKitTools/Scripts/webkitpy/common/net/regressionwindow.py b/WebKitTools/Scripts/webkitpy/common/net/regressionwindow.py
index 231459f..fcceb35 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/regressionwindow.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/regressionwindow.py
@@ -32,6 +32,7 @@ class RegressionWindow(object):
         self._build_before_failure = build_before_failure
         self._failing_build = failing_build
         self._common_failures = common_failures
+        self._revisions = None
 
     def build_before_failure(self):
         return self._build_before_failure
@@ -43,6 +44,8 @@ class RegressionWindow(object):
         return self._common_failures
 
     def revisions(self):
-        revisions = range(self._failing_build.revision(), self._build_before_failure.revision(), -1)
-        revisions.reverse()
-        return revisions
+        # Cache revisions to avoid excessive allocations.
+        if not self._revisions:
+            self._revisions = range(self._failing_build.revision(), self._build_before_failure.revision(), -1)
+            self._revisions.reverse()
+        return self._revisions
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queries.py b/WebKitTools/Scripts/webkitpy/tool/commands/queries.py
index c6e45aa..f7f8b19 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queries.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queries.py
@@ -162,15 +162,6 @@ class WhatBroke(AbstractDeclarativeCommand):
             print "All builders are passing!"
 
 
-class WhoBrokeIt(AbstractDeclarativeCommand):
-    name = "who-broke-it"
-    help_text = "Print a list of revisions causing failures on %s" % BuildBot.default_host
-
-    def execute(self, options, args, tool):
-        for revision, builders in self._tool.buildbot.failure_map(False).revisions_causing_failures().items():
-            print "r%s appears to have broken %s" % (revision, [builder.name() for builder in builders])
-
-
 class ResultsFor(AbstractDeclarativeCommand):
     name = "results-for"
     help_text = "Print a list of failures for the passed revision from bots on %s" % BuildBot.default_host
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py
index 23d013d..cd83c0d 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py
@@ -54,57 +54,30 @@ class SheriffBot(AbstractQueue, StepSequenceErrorHandler):
         self._irc_bot = SheriffIRCBot(self._tool, self._sheriff)
         self._tool.ensure_irc_connected(self._irc_bot.irc_delegate())
 
-    def work_item_log_path(self, new_failures):
-        return os.path.join("%s-logs" % self.name, "%s.log" % new_failures.keys()[0])
-
-    def _new_failures(self, revisions_causing_failures, old_failing_svn_revisions):
-        # We ignore failures that might have been caused by svn_revisions that
-        # we've already complained about.  This is conservative in the sense
-        # that we might be ignoring some new failures, but our experience has
-        # been that skipping this check causes a lot of spam for builders that
-        # take a long time to cycle.
-        old_failing_builder_names = []
-        for svn_revision in old_failing_svn_revisions:
-            old_failing_builder_names.extend(
-                [builder.name() for builder in revisions_causing_failures[svn_revision]])
-
-        new_failures = {}
-        for svn_revision, builders in revisions_causing_failures.items():
-            if svn_revision in old_failing_svn_revisions:
-                # FIXME: We should re-process the work item after some time delay.
-                # https://bugs.webkit.org/show_bug.cgi?id=36581
-                continue
-            new_builders = [builder for builder in builders
-                            if builder.name() not in old_failing_builder_names]
-            if new_builders:
-                new_failures[svn_revision] = new_builders
-
-        return new_failures
+    def work_item_log_path(self, failure_map):
+        return None
+
+    def _is_old_failure(self, svn_revision):
+        return self._tool.status_server.svn_revision(svn_revision)
 
     def next_work_item(self):
         self._irc_bot.process_pending_messages()
         self._update()
 
-        # We do one read from buildbot to ensure a consistent view.
-        revisions_causing_failures = self._tool.buildbot.failure_map().revisions_causing_failures()
-
-        # Similarly, we read once from our the status_server.
-        old_failing_svn_revisions = []
-        for svn_revision in revisions_causing_failures.keys():
-            if self._tool.status_server.svn_revision(svn_revision):
-                old_failing_svn_revisions.append(svn_revision)
-
-        new_failures = self._new_failures(revisions_causing_failures,
-                                          old_failing_svn_revisions)
+        # FIXME: We need to figure out how to provoke_flaky_builders.
 
-        self._sheriff.provoke_flaky_builders(revisions_causing_failures)
-        return new_failures
+        failure_map = self._tool.buildbot.failure_map()
+        failure_map.filter_out_old_failures(self._is_old_failure)
+        if failure_map.is_empty():
+            return None
+        return failure_map
 
-    def should_proceed_with_work_item(self, new_failures):
+    def should_proceed_with_work_item(self, failure_map):
         # Currently, we don't have any reasons not to proceed with work items.
         return True
 
-    def process_work_item(self, new_failures):
+    def process_work_item(self, failure_map):
+        new_failures = failure_map.revisions_causing_failures()
         blame_list = new_failures.keys()
         for svn_revision, builders in new_failures.items():
             try:
@@ -124,7 +97,7 @@ class SheriffBot(AbstractQueue, StepSequenceErrorHandler):
                                                                 builder.name())
         return True
 
-    def handle_unexpected_error(self, new_failures, message):
+    def handle_unexpected_error(self, failure_map, message):
         log(message)
 
     # StepSequenceErrorHandler methods
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
index a63ec24..5343ca7 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
@@ -31,6 +31,7 @@ import os
 from webkitpy.tool.commands.queuestest import QueuesTest
 from webkitpy.tool.commands.sheriffbot import SheriffBot
 from webkitpy.tool.mocktool import MockBuilder
+from webkitpy.thirdparty.mock import Mock
 
 
 class SheriffBotTest(QueuesTest):
@@ -38,7 +39,8 @@ class SheriffBotTest(QueuesTest):
     builder2 = MockBuilder("Builder2")
 
     def test_sheriff_bot(self):
-        mock_work_item = {
+        mock_work_item = Mock()
+        mock_work_item.revisions_causing_failures = lambda: {
             29837: [self.builder1],
         }
         expected_stderr = {
@@ -48,26 +50,3 @@ class SheriffBotTest(QueuesTest):
             "handle_unexpected_error": "Mock error message\n"
         }
         self.assert_queue_outputs(SheriffBot(), work_item=mock_work_item, expected_stderr=expected_stderr)
-
-    revisions_causing_failures = {
-        1234: [builder1],
-        1235: [builder1, builder2],
-    }
-
-    def test_new_failures(self):
-        old_failing_svn_revisions = []
-        self.assertEquals(SheriffBot()._new_failures(self.revisions_causing_failures,
-                                                     old_failing_svn_revisions),
-                          self.revisions_causing_failures)
-
-    def test_new_failures_with_old_revisions(self):
-        old_failing_svn_revisions = [1234]
-        self.assertEquals(SheriffBot()._new_failures(self.revisions_causing_failures,
-                                                     old_failing_svn_revisions),
-                          {1235: [builder2]})
-
-    def test_new_failures_with_old_revisions(self):
-        old_failing_svn_revisions = [1235]
-        self.assertEquals(SheriffBot()._new_failures(self.revisions_causing_failures,
-                                                     old_failing_svn_revisions),
-                          {})
diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool.py b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
index fd25579..ba52883 100644
--- a/WebKitTools/Scripts/webkitpy/tool/mocktool.py
+++ b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
@@ -354,11 +354,17 @@ class MockFailureMap():
     def __init__(self, buildbot):
         self._buildbot = buildbot
 
+    def is_empty(self):
+        return False
+
     def revisions_causing_failures(self):
         return {
             "29837": [self._buildbot.builder_with_name("Builder1")],
         }
 
+    def filter_out_old_failures(self, is_old_revision):
+        pass
+
 
 class MockBuildBot(object):
     buildbot_host = "dummy_buildbot_host"

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list