Commit 6387ef6c authored by eric@webkit.org's avatar eric@webkit.org

2010-12-09 Eric Seidel <eric@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
parent b69181d8
2010-12-09 Eric Seidel <eric@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:
2010-12-09 Eric Seidel <eric@webkit.org>
Reviewed by Adam Barth.
......
......@@ -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:
......
......@@ -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@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.
......
......@@ -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)
......
......@@ -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@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',
'-',
......
# 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"
......@@ -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()], [])
......
......@@ -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@example.com)
Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com)
MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Rejecting patch 128 from commit-queue.' and additional comment 'non-committer@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@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.
......
# 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)
# 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@test.com", "test1@test.com"])
self._assert_emails_for_test(["test1@test.com", "test2@test.com"])
# report_flaky_tests is tested by queues_unittest
......@@ -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):
......
......@@ -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"
......
......@@ -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":
......
......@@ -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
......
......@@ -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@example.com)
Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com)
MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Rejecting patch 128 from commit-queue.' and additional comment 'non-committer@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@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@test.com", "test1@test.com"])
self._assert_emails_for_tests(["test1@test.com", "test2@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