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

eric at webkit.org eric at webkit.org
Wed Dec 22 18:21:53 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit 6387ef6c87bd061a1541028a55dbcae28e4ee425
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Dec 10 08:26:30 2010 +0000

    2010-12-09  Eric Seidel  <eric at webkit.org>
    
            Reviewed by Adam Barth.
    
            The commit-queue should file bugs about flaky tests it encounters
            https://bugs.webkit.org/show_bug.cgi?id=50803
    
            This change got a bit big.  I also added a new config.urls file
            because I needed to share the view_source_url code with committervalidator.py.
    
            This adds a new class FlakyTestReporter which holds all the logic about
            reporting flaky tests to bugzilla.
    
            Right now this code knows how to look up bugs for flaky tests.
            If it can't find a bug filed from the commit-queue, it will open a new
            one, ccing the relevant people and adding information about the failure.
    
            It is not yet smart enough to chase down duplicate chains, or to include
            the actual failure diff.  But those can be added in later iterations.
    
            * Scripts/webkitpy/common/checkout/api.py:
            * Scripts/webkitpy/common/checkout/changelog.py:
            * Scripts/webkitpy/common/checkout/commitinfo.py:
            * Scripts/webkitpy/common/config/committervalidator.py:
            * Scripts/webkitpy/common/config/urls.py: Copied from WebKitTools/Scripts/webkitpy/tool/comments.py.
            * Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
            * Scripts/webkitpy/tool/bot/flakytestreporter.py: Added.
            * Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py: Copied from WebKitTools/Scripts/webkitpy/tool/comments.py.
            * Scripts/webkitpy/tool/bot/irc_command.py:
            * Scripts/webkitpy/tool/bot/sheriff.py:
            * Scripts/webkitpy/tool/commands/download.py:
            * Scripts/webkitpy/tool/commands/queues.py:
            * Scripts/webkitpy/tool/commands/queues_unittest.py:
            * Scripts/webkitpy/tool/comments.py:
            * Scripts/webkitpy/tool/mocktool.py:
            * Scripts/webkitpy/tool/steps/commit.py:
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@73691 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index ba6df0f..63c2b32 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -2,6 +2,43 @@
 
         Reviewed by Adam Barth.
 
+        The commit-queue should file bugs about flaky tests it encounters
+        https://bugs.webkit.org/show_bug.cgi?id=50803
+
+        This change got a bit big.  I also added a new config.urls file
+        because I needed to share the view_source_url code with committervalidator.py.
+
+        This adds a new class FlakyTestReporter which holds all the logic about
+        reporting flaky tests to bugzilla.
+
+        Right now this code knows how to look up bugs for flaky tests.
+        If it can't find a bug filed from the commit-queue, it will open a new
+        one, ccing the relevant people and adding information about the failure.
+
+        It is not yet smart enough to chase down duplicate chains, or to include
+        the actual failure diff.  But those can be added in later iterations.
+
+        * Scripts/webkitpy/common/checkout/api.py:
+        * Scripts/webkitpy/common/checkout/changelog.py:
+        * Scripts/webkitpy/common/checkout/commitinfo.py:
+        * Scripts/webkitpy/common/config/committervalidator.py:
+        * Scripts/webkitpy/common/config/urls.py: Copied from WebKitTools/Scripts/webkitpy/tool/comments.py.
+        * Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
+        * Scripts/webkitpy/tool/bot/flakytestreporter.py: Added.
+        * Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py: Copied from WebKitTools/Scripts/webkitpy/tool/comments.py.
+        * Scripts/webkitpy/tool/bot/irc_command.py:
+        * Scripts/webkitpy/tool/bot/sheriff.py:
+        * Scripts/webkitpy/tool/commands/download.py:
+        * Scripts/webkitpy/tool/commands/queues.py:
+        * Scripts/webkitpy/tool/commands/queues_unittest.py:
+        * Scripts/webkitpy/tool/comments.py:
+        * Scripts/webkitpy/tool/mocktool.py:
+        * Scripts/webkitpy/tool/steps/commit.py:
+
+2010-12-09  Eric Seidel  <eric at webkit.org>
+
+        Reviewed by Adam Barth.
+
         Teach webkit-patch how to search bugzilla
         https://bugs.webkit.org/show_bug.cgi?id=50500
 
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/api.py b/WebKitTools/Scripts/webkitpy/common/checkout/api.py
index dbe1e84..b84cf9b 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/api.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/api.py
@@ -29,6 +29,7 @@
 import os
 import StringIO
 
+from webkitpy.common.config import urls
 from webkitpy.common.checkout.changelog import ChangeLog
 from webkitpy.common.checkout.commitinfo import CommitInfo
 from webkitpy.common.checkout.scm import CommitMessage
@@ -104,8 +105,7 @@ class Checkout(object):
         changelog_paths = self.modified_changelogs(git_commit, changed_files)
         if not len(changelog_paths):
             raise ScriptError(message="Found no modified ChangeLogs, cannot create a commit message.\n"
-                              "All changes require a ChangeLog.  See:\n"
-                              "http://webkit.org/coding/contributing.html")
+                              "All changes require a ChangeLog.  See:\n %s" % urls.contribution_guidelines)
 
         changelog_messages = []
         for changelog_path in changelog_paths:
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/changelog.py b/WebKitTools/Scripts/webkitpy/common/checkout/changelog.py
index 40657eb..d3f990c 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/changelog.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/changelog.py
@@ -36,16 +36,10 @@ import textwrap
 
 from webkitpy.common.system.deprecated_logging import log
 from webkitpy.common.config.committers import CommitterList
+from webkitpy.common.config import urls
 from webkitpy.common.net.bugzilla import parse_bug_id
 
 
-def view_source_url(revision_number):
-    # FIMXE: This doesn't really belong in this file, but we don't have a
-    # better home for it yet.
-    # Maybe eventually a webkit_config.py?
-    return "http://trac.webkit.org/changeset/%s" % revision_number
-
-
 class ChangeLogEntry(object):
     # e.g. 2009-06-03  Eric Seidel  <eric at webkit.org>
     date_line_regexp = r'^(?P<date>\d{4}-\d{2}-\d{2})\s+(?P<name>.+?)\s+<(?P<email>[^<>]+)>$'
@@ -153,7 +147,7 @@ class ChangeLog(object):
     # This probably does not belong in changelogs.py
     def _message_for_revert(self, revision, reason, bug_url):
         message = "Unreviewed, rolling out r%s.\n" % revision
-        message += "%s\n" % view_source_url(revision)
+        message += "%s\n" % urls.view_revision_url(revision)
         if bug_url:
             message += "%s\n" % bug_url
         # Add an extra new line after the rollout links, before any reason.
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/commitinfo.py b/WebKitTools/Scripts/webkitpy/common/checkout/commitinfo.py
index 448d530..f121f36 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/commitinfo.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/commitinfo.py
@@ -28,7 +28,7 @@
 #
 # WebKit's python module for holding information on a commit
 
-from webkitpy.common.checkout.changelog import view_source_url
+from webkitpy.common.config import urls
 from webkitpy.common.config.committers import CommitterList
 
 
@@ -84,7 +84,7 @@ class CommitInfo(object):
     # FIXME: It is slightly lame that this "view" method is on this "model" class (in MVC terms)
     def blame_string(self, bugs):
         string = "r%s:\n" % self.revision()
-        string += "  %s\n" % view_source_url(self.revision())
+        string += "  %s\n" % urls.view_revision_url(self.revision())
         string += "  Bug: %s (%s)\n" % (self.bug_id(), bugs.bug_url_for_bug_id(self.bug_id()))
         author_line = "\"%s\" <%s>" % (self.author_name(), self.author_email())
         string += "  Author: %s\n" % (self.author() or author_line)
diff --git a/WebKitTools/Scripts/webkitpy/common/config/committervalidator.py b/WebKitTools/Scripts/webkitpy/common/config/committervalidator.py
index b7b2990..a26303b 100644
--- a/WebKitTools/Scripts/webkitpy/common/config/committervalidator.py
+++ b/WebKitTools/Scripts/webkitpy/common/config/committervalidator.py
@@ -31,7 +31,7 @@
 import os
 
 from webkitpy.common.system.ospath import relpath
-from webkitpy.common.config import committers
+from webkitpy.common.config import committers, urls
 
 
 class CommitterValidator(object):
@@ -39,10 +39,6 @@ class CommitterValidator(object):
     def __init__(self, bugzilla):
         self._bugzilla = bugzilla
 
-    # _view_source_url belongs in some sort of webkit_config.py module.
-    def _view_source_url(self, local_path):
-        return "http://trac.webkit.org/browser/trunk/%s" % local_path
-
     def _checkout_root(self):
         # FIXME: This is a hack, we would have this from scm.checkout_root
         # if we had any way to get to an scm object here.
@@ -59,8 +55,6 @@ class CommitterValidator(object):
         return ".".join([path, "py"])
 
     def _flag_permission_rejection_message(self, setter_email, flag_name):
-        # Should come from some webkit_config.py
-        contribution_guidlines = "http://webkit.org/coding/contributing.html"
         # This could be queried from the status_server.
         queue_administrator = "eseidel at chromium.org"
         # This could be queried from the tool.
@@ -69,9 +63,9 @@ class CommitterValidator(object):
         message = "%s does not have %s permissions according to %s." % (
                         setter_email,
                         flag_name,
-                        self._view_source_url(committers_list))
+                        urls.view_source_url(committers_list))
         message += "\n\n- If you do not have %s rights please read %s for instructions on how to use bugzilla flags." % (
-                        flag_name, contribution_guidlines)
+                        flag_name, urls.contribution_guidelines)
         message += "\n\n- If you have %s rights please correct the error in %s by adding yourself to the file (no review needed).  " % (
                         flag_name, committers_list)
         message += "The %s restarts itself every 2 hours.  After restart the %s will correctly respect your %s rights." % (
@@ -102,7 +96,7 @@ class CommitterValidator(object):
     def reject_patch_from_commit_queue(self,
                                        attachment_id,
                                        additional_comment_text=None):
-        comment_text = "Rejecting patch %s from commit-queue." % attachment_id
+        comment_text = "Rejecting attachment %s from commit-queue." % attachment_id
         self._bugzilla.set_flag_on_attachment(attachment_id,
                                               "commit-queue",
                                               "-",
@@ -112,7 +106,7 @@ class CommitterValidator(object):
     def reject_patch_from_review_queue(self,
                                        attachment_id,
                                        additional_comment_text=None):
-        comment_text = "Rejecting patch %s from review queue." % attachment_id
+        comment_text = "Rejecting attachment %s from review queue." % attachment_id
         self._bugzilla.set_flag_on_attachment(attachment_id,
                                               'review',
                                               '-',
diff --git a/WebKitTools/Scripts/webkitpy/common/config/urls.py b/WebKitTools/Scripts/webkitpy/common/config/urls.py
new file mode 100644
index 0000000..dfa6d69
--- /dev/null
+++ b/WebKitTools/Scripts/webkitpy/common/config/urls.py
@@ -0,0 +1,38 @@
+# 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.
+
+
+def view_source_url(local_path):
+    return "http://trac.webkit.org/browser/trunk/%s" % local_path
+
+
+def view_revision_url(revision_number):
+    return "http://trac.webkit.org/changeset/%s" % revision_number
+
+
+contribution_guidelines = "http://webkit.org/coding/contributing.html"
diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py
index a0609c5..2e6b7e8 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py
@@ -146,6 +146,14 @@ class BugzillaQueries(object):
         quicksearch_url = "buglist.cgi?quicksearch=%s" % urllib.quote(search_string)
         return self._fetch_bugs_from_advanced_query(quicksearch_url)
 
+    def fetch_bugs_matching_search(self, search_string, author_email=None):
+        query = "buglist.cgi?query_format=advanced"
+        if search_string:
+            query += "&short_desc_type=allwordssubstr&short_desc=%s" % urllib.quote(search_string)
+        if author_email:
+            query += "&emailreporter1=1&emailtype1=substring&email1=%s" % urllib.quote(search_string)
+        return self._fetch_bugs_from_advanced_query(query)
+
     def fetch_patches_from_pending_commit_list(self):
         return sum([self._fetch_bug(bug_id).reviewed_patches()
             for bug_id in self.fetch_bug_ids_from_pending_commit_list()], [])
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/feeders_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/feeders_unittest.py
index 5ce00b4..ec555cb 100644
--- a/WebKitTools/Scripts/webkitpy/tool/bot/feeders_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/feeders_unittest.py
@@ -40,7 +40,7 @@ class FeedersTest(unittest.TestCase):
         feeder = CommitQueueFeeder(MockTool())
         expected_stderr = u"""Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
 Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
-MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Rejecting patch 128 from commit-queue.' and additional comment 'non-committer at example.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.
+MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Rejecting attachment 128 from commit-queue.' and additional comment 'non-committer at example.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.
 
 - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.
 
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/flakytestreporter.py b/WebKitTools/Scripts/webkitpy/tool/bot/flakytestreporter.py
new file mode 100644
index 0000000..01874e8
--- /dev/null
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/flakytestreporter.py
@@ -0,0 +1,114 @@
+# 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 logging
+
+from webkitpy.common.net.layouttestresults import path_for_layout_test, LayoutTestResults
+from webkitpy.common.config import urls
+from webkitpy.tool.grammar import pluralize, join_with_separators
+
+_log = logging.getLogger(__name__)
+
+
+class FlakyTestReporter(object):
+    def __init__(self, tool, bot_name):
+        self._tool = tool
+        self._bot_name = bot_name
+
+    def _author_emails_for_test(self, flaky_test):
+        test_path = path_for_layout_test(flaky_test)
+        commit_infos = self._tool.checkout().recent_commit_infos_for_files([test_path])
+        # This ignores authors which are not committers because we don't have their bugzilla_email.
+        return set([commit_info.author().bugzilla_email() for commit_info in commit_infos if commit_info.author()])
+
+    def _bugzilla_email(self):
+        # This is kinda a funny way to get the bugzilla email,
+        # we could also just create a Credentials object directly
+        # but some of the Credentials logic is in bugzilla.py too...
+        self._tool.bugs.authenticate()
+        return self._tool.bugs.username
+
+    def _lookup_bug_for_flaky_test(self, flaky_test):
+        bot_email = self._bugzilla_email()
+        bugs = self._tool.bugs.queries.fetch_bugs_matching_search(search_string=flaky_test, author_email=bot_email)
+        if not bugs:
+            return None
+        if len(bugs) > 1:
+            _log.warn("Found %s %s matching '%s' from the %s (%s), using the first." % (pluralize('bug', len(bugs)), bugs, flaky_test, self._bot_name, bot_email))
+        return bugs[0]
+
+    def _create_bug_for_flaky_test(self, flaky_test, author_emails, latest_flake_message):
+        format_values = {
+            'test': flaky_test,
+            'authors': join_with_separators(author_emails),
+            'flake_message': latest_flake_message,
+            'test_url': urls.view_source_url(flaky_test),
+            'bot_name': self._bot_name,
+        }
+        title = "Flaky Test: %(test)s" % format_values
+        description = """This is an automatically generated bug from the %(bot_name)s.
+%(test)s has been flaky on the %(bot_name)s.
+%(test)s was authored by %(authors)s.
+%(test_url)s
+
+%(flake_message)s
+
+The bots will update this with information from each new failure.
+
+If you would like to track this test fix with another bug, please close this bug as a duplicate.
+""" % format_values
+
+        self._tool_bugs.create_bug(title, description,
+            component="Tools / Tests",
+            cc=",".join(author_bugzilla_emails))
+
+    # This is over-engineered, but it makes for pretty bug messages.
+    def _optional_author_string(self, author_emails):
+        if not author_emails:
+            return ""
+        heading_string = plural('author') if len(author_emails) > 1 else 'author'
+        authors_string = join_with_separators(sorted(author_emails))
+        return " (%s: %s)" % (heading_string, authors_string)
+
+    def report_flaky_tests(self, flaky_tests, patch):
+        message = "The %s encountered the following flaky tests while processing attachment %s:\n\n" % (self._bot_name, patch.id())
+        for flaky_test in flaky_tests:
+            bug = self._lookup_bug_for_flaky_test(flaky_test)
+            latest_flake_message = "The %s just saw %s flake while processing attachment %s on bug %s." % (self._bot_name, flaky_test, patch.id(), patch.bug_id())
+            author_emails = self._author_emails_for_test(flaky_test)
+            if not bug:
+                self._create_bug_for_flaky_test(flaky_test, latest_flake_message)
+            else:
+                # FIXME: If the bug is closed we should follow the duplicate chain to the last bug,
+                # and then re-open the last bug if that too is closed.
+                self._tool.bugs.post_comment_to_bug(patch.bug_id(), latest_flake_message)
+
+            message += "%s bug %s%s\n" % (flaky_test, patch.bug_id(), self._optional_author_string(author_emails))
+
+        message += "The %s is continuing to process your patch." % self._bot_name
+        self._tool.bugs.post_comment_to_bug(patch.bug_id(), message)
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py
new file mode 100644
index 0000000..1643f1e
--- /dev/null
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py
@@ -0,0 +1,62 @@
+# 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.config.committers import Committer
+from webkitpy.tool.bot.flakytestreporter import FlakyTestReporter
+from webkitpy.tool.mocktool import MockTool
+
+
+# Creating fake CommitInfos is a pain, so we use a mock one here.
+class MockCommitInfo(object):
+    def __init__(self, author_email):
+        self._author_email = author_email
+
+    def author(self):
+        # It's definitely possible to have commits with authors who
+        # are not in our committers.py list.
+        if not self._author_email:
+            return None
+        return Committer("Mock Committer", self._author_email)
+
+
+class FlakyTestReporterTest(unittest.TestCase):
+    def _assert_emails_for_test(self, emails):
+        tool = MockTool()
+        reporter = FlakyTestReporter(tool, 'dummy-queue')
+        commit_infos = [MockCommitInfo(email) for email in emails]
+        tool.checkout().recent_commit_infos_for_files = lambda paths: set(commit_infos)
+        self.assertEqual(reporter._author_emails_for_test([]), set(emails))
+
+    def test_author_emails_for_test(self):
+        self._assert_emails_for_test([])
+        self._assert_emails_for_test(["test1 at test.com", "test1 at test.com"])
+        self._assert_emails_for_test(["test1 at test.com", "test2 at test.com"])
+
+    # report_flaky_tests is tested by queues_unittest
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/irc_command.py b/WebKitTools/Scripts/webkitpy/tool/bot/irc_command.py
index a848472..0c17c9f 100644
--- a/WebKitTools/Scripts/webkitpy/tool/bot/irc_command.py
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/irc_command.py
@@ -29,7 +29,7 @@
 import random
 import webkitpy.common.config.irc as config_irc
 
-from webkitpy.common.checkout.changelog import view_source_url
+from webkitpy.common.config import urls
 from webkitpy.tool.bot.queueengine import TerminateQueue
 from webkitpy.common.net.bugzilla import parse_bug_id
 from webkitpy.common.system.executive import ScriptError
@@ -43,7 +43,7 @@ class IRCCommand(object):
 class LastGreenRevision(IRCCommand):
     def execute(self, nick, args, tool, sheriff):
         return "%s: %s" % (nick,
-            view_source_url(tool.buildbot.last_green_revision()))
+            urls.view_revision_url(tool.buildbot.last_green_revision()))
 
 
 class Restart(IRCCommand):
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py b/WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py
index da506bc..43f3221 100644
--- a/WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py
@@ -26,7 +26,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-from webkitpy.common.checkout.changelog import view_source_url
+from webkitpy.common.config import urls
 from webkitpy.common.net.bugzilla import parse_bug_id
 from webkitpy.common.system.deprecated_logging import log
 from webkitpy.common.system.executive import ScriptError
@@ -46,7 +46,7 @@ class Sheriff(object):
         irc_message = "%s%s%s might have broken %s" % (
             ", ".join(irc_nicknames),
             irc_prefix,
-            view_source_url(commit_info.revision()),
+            urls.view_revision_url(commit_info.revision()),
             join_with_separators([builder.name() for builder in builders]))
 
         self._tool.irc().post(irc_message)
@@ -81,7 +81,7 @@ class Sheriff(object):
         if not commit_info.bug_id():
             return
         comment = "%s might have broken %s" % (
-            view_source_url(commit_info.revision()),
+            urls.view_revision_url(commit_info.revision()),
             join_with_separators([builder.name() for builder in builders]))
         if tests:
             comment += "\nThe following tests are not passing:\n"
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download.py b/WebKitTools/Scripts/webkitpy/tool/commands/download.py
index 457c050..8abd28f 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/download.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/download.py
@@ -31,7 +31,8 @@ import os
 
 import webkitpy.tool.steps as steps
 
-from webkitpy.common.checkout.changelog import ChangeLog, view_source_url
+from webkitpy.common.checkout.changelog import ChangeLog
+from webkitpy.common.config import urls
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.tool.commands.abstractsequencedcommand import AbstractSequencedCommand
 from webkitpy.tool.commands.stepsequence import StepSequence
@@ -355,7 +356,7 @@ class CreateRollout(AbstractRolloutPrepCommand):
         state["bug_blocked"] = state["bug_id"]
         del state["bug_id"]
         state["bug_title"] = "REGRESSION(r%s): %s" % (state["revision"], state["reason"])
-        state["bug_description"] = "%s broke the build:\n%s" % (view_source_url(state["revision"]), state["reason"])
+        state["bug_description"] = "%s broke the build:\n%s" % (urls.view_revision_url(state["revision"]), state["reason"])
         # FIXME: If we had more context here, we could link to other open bugs
         #        that mention the test that regressed.
         if options.parent_command == "sheriff-bot":
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
index bfaeb08..858713e 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
@@ -40,15 +40,15 @@ from StringIO import StringIO
 
 from webkitpy.common.config.committervalidator import CommitterValidator
 from webkitpy.common.net.bugzilla import Attachment
-from webkitpy.common.net.layouttestresults import path_for_layout_test, LayoutTestResults
+from webkitpy.common.net.layouttestresults import LayoutTestResults
 from webkitpy.common.net.statusserver import StatusServer
 from webkitpy.common.system.deprecated_logging import error, log
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.tool.bot.commitqueuetask import CommitQueueTask, CommitQueueTaskDelegate
 from webkitpy.tool.bot.feeders import CommitQueueFeeder, EWSFeeder
 from webkitpy.tool.bot.queueengine import QueueEngine, QueueEngineDelegate
+from webkitpy.tool.bot.flakytestreporter import FlakyTestReporter
 from webkitpy.tool.commands.stepsequence import StepSequenceErrorHandler
-from webkitpy.tool.grammar import pluralize, join_with_separators
 from webkitpy.tool.multicommandtool import Command, TryAgain
 
 
@@ -312,20 +312,9 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskD
     def refetch_patch(self, patch):
         return self._tool.bugs.fetch_attachment(patch.id())
 
-    def _author_emails_for_tests(self, flaky_tests):
-        test_paths = map(path_for_layout_test, flaky_tests)
-        commit_infos = self._tool.checkout().recent_commit_infos_for_files(test_paths)
-        return set([commit_info.author().bugzilla_email() for commit_info in commit_infos if commit_info.author()])
-
     def report_flaky_tests(self, patch, flaky_tests):
-        message = "The %s encountered the following flaky tests while processing attachment %s:" % (self.name, patch.id())
-        message += "\n\n%s\n\n" % ("\n".join(flaky_tests))
-        message += "Please file bugs against the tests.  "
-        author_emails = self._author_emails_for_tests(flaky_tests)
-        if author_emails:
-            message += "These tests were authored by %s.  " % (join_with_separators(sorted(author_emails)))
-        message += "The commit-queue is continuing to process your patch."
-        self._tool.bugs.post_comment_to_bug(patch.bug_id(), message)
+        reporter = FlakyTestReporter(self._tool, self.name)
+        reporter.report_flaky_tests(flaky_tests, patch)
 
     # StepSequenceErrorHandler methods
 
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
index 087297a..bc9f30c 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
@@ -29,7 +29,6 @@
 import os
 
 from webkitpy.common.checkout.scm import CheckoutNeedsUpdate
-from webkitpy.common.config.committers import Committer
 from webkitpy.common.net.bugzilla import Attachment
 from webkitpy.common.system.outputcapture import OutputCapture
 from webkitpy.thirdparty.mock import Mock
@@ -124,7 +123,7 @@ class FeederQueueTest(QueuesTest):
             "next_work_item": "",
             "process_work_item": """Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
 Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
-MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Rejecting patch 128 from commit-queue.' and additional comment 'non-committer at example.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.
+MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Rejecting attachment 128 from commit-queue.' and additional comment 'non-committer at example.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.
 
 - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.
 
@@ -198,19 +197,6 @@ class SecondThoughtsCommitQueue(CommitQueue):
         return Attachment(attachment_dictionary, None)
 
 
-# Creating fake CommitInfos is a pain, so we use a mock one here.
-class MockCommitInfo(object):
-    def __init__(self, author_email):
-        self._author_email = author_email
-
-    def author(self):
-        # It's definitely possible to have commits with authors who
-        # are not in our committers.py list.
-        if not self._author_email:
-            return None
-        return Committer("Mock Committer", self._author_email)
-
-
 class CommitQueueTest(QueuesTest):
     def test_commit_queue(self):
         expected_stderr = {
@@ -225,7 +211,7 @@ MOCK: update_status: commit-queue Landed patch
 MOCK: update_status: commit-queue Pass
 MOCK: release_work_item: commit-queue 197
 """,
-            "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting patch 197 from commit-queue.' and additional comment 'Mock error message'\n",
+            "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting attachment 197 from commit-queue.' and additional comment 'Mock error message'\n",
             "handle_script_error": "ScriptError error message\n",
         }
         self.assert_queue_outputs(CommitQueue(), expected_stderr=expected_stderr)
@@ -237,11 +223,11 @@ MOCK: release_work_item: commit-queue 197
             "next_work_item": "",
             "process_work_item": """MOCK: update_status: commit-queue Cleaned working directory
 MOCK: update_status: commit-queue Patch does not apply
-MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting patch 197 from commit-queue.' and additional comment 'MOCK script error'
+MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting attachment 197 from commit-queue.' and additional comment 'MOCK script error'
 MOCK: update_status: commit-queue Fail
 MOCK: release_work_item: commit-queue 197
 """,
-            "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting patch 197 from commit-queue.' and additional comment 'Mock error message'\n",
+            "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting attachment 197 from commit-queue.' and additional comment 'Mock error message'\n",
             "handle_script_error": "ScriptError error message\n",
         }
         queue = CommitQueue()
@@ -276,7 +262,7 @@ MOCK: update_status: commit-queue Landed patch
 MOCK: update_status: commit-queue Pass
 MOCK: release_work_item: commit-queue 197
 """,
-            "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting patch 197 from commit-queue.' and additional comment 'Mock error message'\n",
+            "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting attachment 197 from commit-queue.' and additional comment 'Mock error message'\n",
             "handle_script_error": "ScriptError error message\n",
         }
         self.assert_queue_outputs(CommitQueue(), tool=tool, expected_stderr=expected_stderr)
@@ -301,7 +287,7 @@ MOCK: update_status: commit-queue Landed patch
 MOCK: update_status: commit-queue Pass
 MOCK: release_work_item: commit-queue 106
 """,
-            "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '106' with comment 'Rejecting patch 106 from commit-queue.' and additional comment 'Mock error message'\n",
+            "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '106' with comment 'Rejecting attachment 106 from commit-queue.' and additional comment 'Mock error message'\n",
             "handle_script_error": "ScriptError error message\n",
         }
         self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_stderr=expected_stderr)
@@ -335,30 +321,26 @@ MOCK: release_work_item: commit-queue 197
 """
         OutputCapture().assert_outputs(self, queue.process_work_item, [QueuesTest.mock_work_item], expected_stderr=expected_stderr)
 
-    def _assert_emails_for_tests(self, emails):
-        queue = CommitQueue()
-        tool = MockTool()
-        queue.bind_to_tool(tool)
-        commit_infos = [MockCommitInfo(email) for email in emails]
-        tool.checkout().recent_commit_infos_for_files = lambda paths: set(commit_infos)
-        self.assertEqual(queue._author_emails_for_tests([]), set(emails))
-
-    def test_author_emails_for_tests(self):
-        self._assert_emails_for_tests([])
-        self._assert_emails_for_tests(["test1 at test.com", "test1 at test.com"])
-        self._assert_emails_for_tests(["test1 at test.com", "test2 at test.com"])
-
     def test_report_flaky_tests(self):
         queue = CommitQueue()
         queue.bind_to_tool(MockTool())
         expected_stderr = """MOCK bug comment: bug_id=42, cc=None
 --- Begin comment ---
-The commit-queue encountered the following flaky tests while processing attachment 197:
+The commit-queue just saw foo/bar.html flake while processing attachment 197 on bug 42.
+--- End comment ---
 
-foo/bar.html
-bar/baz.html
+MOCK bug comment: bug_id=42, cc=None
+--- Begin comment ---
+The commit-queue just saw bar/baz.html flake while processing attachment 197 on bug 42.
+--- End comment ---
+
+MOCK bug comment: bug_id=42, cc=None
+--- Begin comment ---
+The commit-queue encountered the following flaky tests while processing attachment 197:
 
-Please file bugs against the tests.  These tests were authored by abarth at webkit.org.  The commit-queue is continuing to process your patch.
+foo/bar.html bug 42 (author: abarth at webkit.org)
+bar/baz.html bug 42 (author: abarth at webkit.org)
+The commit-queue is continuing to process your patch.
 --- End comment ---
 
 """
diff --git a/WebKitTools/Scripts/webkitpy/tool/comments.py b/WebKitTools/Scripts/webkitpy/tool/comments.py
index 83f2be8..771953e 100755
--- a/WebKitTools/Scripts/webkitpy/tool/comments.py
+++ b/WebKitTools/Scripts/webkitpy/tool/comments.py
@@ -30,12 +30,11 @@
 # A tool for automating dealing with bugzilla, posting patches, committing
 # patches, etc.
 
-from webkitpy.common.checkout.changelog import view_source_url
+from webkitpy.common.config import urls
 
 
 def bug_comment_from_svn_revision(svn_revision):
-    return "Committed r%s: <%s>" % (svn_revision,
-                                    view_source_url(svn_revision))
+    return "Committed r%s: <%s>" % (svn_revision, urls.view_revision_url(svn_revision))
 
 
 def bug_comment_from_commit_text(scm, commit_text):
diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool.py b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
index 0040b1f..e2c816d 100644
--- a/WebKitTools/Scripts/webkitpy/tool/mocktool.py
+++ b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
@@ -225,6 +225,8 @@ class MockBugzillaQueries(Mock):
     def fetch_patches_from_pending_commit_list(self):
         return sum([bug.reviewed_patches() for bug in self._all_bugs()], [])
 
+    def fetch_bugs_matching_search(self, search_string, author_email=None):
+        return [self.bugzilla.fetch_bug(76), self.bugzilla.fetch_bug(77)]
 
 _mock_reviewer = Reviewer("Foo Bar", "foo at bar.com")
 
diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/commit.py b/WebKitTools/Scripts/webkitpy/tool/steps/commit.py
index 5c6bdb7..5aa6b51 100644
--- a/WebKitTools/Scripts/webkitpy/tool/steps/commit.py
+++ b/WebKitTools/Scripts/webkitpy/tool/steps/commit.py
@@ -26,8 +26,8 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-from webkitpy.common.checkout.changelog import view_source_url
 from webkitpy.common.checkout.scm import AuthenticationError, AmbiguousCommitError
+from webkitpy.common.config import urls
 from webkitpy.common.system.deprecated_logging import log
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.common.system.user import User
@@ -66,7 +66,7 @@ class Commit(AbstractStep):
                 scm = self._tool.scm()
                 commit_text = scm.commit_with_message(self._commit_message, git_commit=self._options.git_commit, username=username, force_squash=force_squash)
                 svn_revision = scm.svn_revision_from_commit_text(commit_text)
-                log("Committed r%s: <%s>" % (svn_revision, view_source_url(svn_revision)))
+                log("Committed r%s: <%s>" % (svn_revision, urls.view_revision_url(svn_revision)))
                 self._state["commit_text"] = commit_text
                 break;
             except AmbiguousCommitError, e:

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list