Merge WebKit at r70209: Initial merge by Git

Change-Id: Id23a68efa36e9d1126bcce0b137872db00892c8e

13 files changed, 356 insertions, 183 deletions

diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py b/WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py
index de92cd3..adc6d81 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py
@@ -33,5 +33,16 @@ from webkitpy.tool.mocktool import MockOptions, MockTool
 
 class CommandsTest(unittest.TestCase):
     def assert_execute_outputs(self, command, args, expected_stdout="", expected_stderr="", options=MockOptions(), tool=MockTool()):
+        options.blocks = True
+        options.cc = 'MOCK cc'
+        options.component = 'MOCK component'
+        options.confirm = True
+        options.email = 'MOCK email'
+        options.git_commit = 'MOCK git commit'
+        options.obsolete_patches = True
+        options.open_bug = True
+        options.port = 'MOCK port'
+        options.quiet = True
+        options.reviewer = 'MOCK reviewer'
         command.bind_to_tool(tool)
         OutputCapture().assert_outputs(self, command.execute, [options, args, tool], expected_stdout=expected_stdout, expected_stderr=expected_stderr)
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download.py b/WebKitTools/Scripts/webkitpy/tool/commands/download.py
index 9916523..ed5c604 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/download.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/download.py
@@ -43,6 +43,17 @@ from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand
 from webkitpy.common.system.deprecated_logging import error, log
 
 
+class Clean(AbstractSequencedCommand):
+    name = "clean"
+    help_text = "Clean the working copy"
+    steps = [
+        steps.CleanWorkingDirectory,
+    ]
+
+    def _prepare_state(self, options, args, tool):
+        options.force_clean = True
+
+
 class Update(AbstractSequencedCommand):
     name = "update"
     help_text = "Update working copy (used internally)"
@@ -61,6 +72,9 @@ class Build(AbstractSequencedCommand):
         steps.Build,
     ]
 
+    def _prepare_state(self, options, args, tool):
+        options.build = True
+
 
 class BuildAndTest(AbstractSequencedCommand):
     name = "build-and-test"
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py
index faddd50..6af1f64 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py
@@ -57,15 +57,19 @@ class AbstractRolloutPrepCommandTest(unittest.TestCase):
 class DownloadCommandsTest(CommandsTest):
     def _default_options(self):
         options = MockOptions()
-        options.force_clean = False
-        options.clean = True
+        options.build = True
+        options.build_style = True
         options.check_builders = True
-        options.quiet = False
+        options.check_style = True
+        options.clean = True
+        options.close_bug = True
+        options.force_clean = False
+        options.force_patch = True
         options.non_interactive = False
-        options.update = True
-        options.build = True
+        options.parent_command = 'MOCK parent command'
+        options.quiet = False
         options.test = True
-        options.close_bug = True
+        options.update = True
         return options
 
     def test_build(self):
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py
index 86e2e15..5ec468e 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py
@@ -48,7 +48,6 @@ class AbstractEarlyWarningSystem(AbstractReviewQueue):
             self.run_webkit_patch([
                 "build",
                 self.port.flag(),
-                "--build",
                 "--build-style=%s" % self._build_style,
                 "--force-clean",
                 "--no-update",
@@ -149,10 +148,6 @@ class ChromiumWindowsEWS(AbstractChromiumEWS):
     name = "cr-win-ews"
 
 
-class ChromiumMacEWS(AbstractChromiumEWS):
-    name = "cr-mac-ews"
-
-
 # For platforms that we can't run inside a VM (like Mac OS X), we require
 # patches to be uploaded by committers, who are generally trustworthy folk. :)
 class AbstractCommitterOnlyEWS(AbstractEarlyWarningSystem):
@@ -167,6 +162,14 @@ class AbstractCommitterOnlyEWS(AbstractEarlyWarningSystem):
         return AbstractEarlyWarningSystem.process_work_item(self, patch)
 
 
+# FIXME: Inheriting from AbstractCommitterOnlyEWS is kinda a hack, but it
+# happens to work because AbstractChromiumEWS and AbstractCommitterOnlyEWS
+# provide disjoint sets of functionality, and Python is otherwise smart
+# enough to handle the diamond inheritance.
+class ChromiumMacEWS(AbstractChromiumEWS, AbstractCommitterOnlyEWS):
+    name = "cr-mac-ews"
+
+
 class MacEWS(AbstractCommitterOnlyEWS):
     name = "mac-ews"
     port_name = "mac"
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
index 3b0ea47..c400f81 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
@@ -48,9 +48,9 @@ class EarlyWarningSytemTest(QueuesTest):
         expected_stderr = {
             "begin_work_queue": self._default_begin_work_queue_stderr(ews.name, os.getcwd()),  # FIXME: Use of os.getcwd() is wrong, should be scm.checkout_root
             "handle_unexpected_error": "Mock error message\n",
-            "next_work_item": "MOCK: update_work_items: %(name)s [103]\n" % string_replacemnts,
-            "process_work_item": "MOCK: update_status: %(name)s Pass\n" % string_replacemnts,
-            "handle_script_error": "MOCK: update_status: %(name)s ScriptError error message\nMOCK bug comment: bug_id=142, cc=%(watchers)s\n--- Begin comment ---\\Attachment 197 did not build on %(port)s:\nBuild output: http://dummy_url\n--- End comment ---\n\n" % string_replacemnts,
+            "next_work_item": "",
+            "process_work_item": "MOCK: update_status: %(name)s Pass\nMOCK: release_work_item: %(name)s 197\n" % string_replacemnts,
+            "handle_script_error": "MOCK: update_status: %(name)s ScriptError error message\nMOCK bug comment: bug_id=142, cc=%(watchers)s\n--- Begin comment ---\nAttachment 197 did not build on %(port)s:\nBuild output: http://dummy_url\n--- End comment ---\n\n" % string_replacemnts,
         }
         return expected_stderr
 
@@ -85,3 +85,12 @@ class EarlyWarningSytemTest(QueuesTest):
             "handle_script_error": SystemExit,
         }
         self.assert_queue_outputs(ews, expected_stderr=expected_stderr, expected_exceptions=expected_exceptions)
+
+    def test_chromium_mac_ews(self):
+        ews = ChromiumMacEWS()
+        expected_stderr = self._default_expected_stderr(ews)
+        expected_stderr["process_work_item"] = "MOCK: update_status: cr-mac-ews Error: cr-mac-ews cannot process patches from non-committers :(\n"
+        expected_exceptions = {
+            "handle_script_error": SystemExit,
+        }
+        self.assert_queue_outputs(ews, expected_stderr=expected_stderr, expected_exceptions=expected_exceptions)
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queries.py b/WebKitTools/Scripts/webkitpy/tool/commands/queries.py
index c6e45aa..16ddc2c 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queries.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queries.py
@@ -30,6 +30,8 @@
 
 from optparse import make_option
 
+import webkitpy.tool.steps as steps
+
 from webkitpy.common.checkout.commitinfo import CommitInfo
 from webkitpy.common.config.committers import CommitterList
 from webkitpy.common.net.buildbot import BuildBot
@@ -41,6 +43,21 @@ from webkitpy.common.system.deprecated_logging import log
 from webkitpy.layout_tests import port
 
 
+class SuggestReviewers(AbstractDeclarativeCommand):
+    name = "suggest-reviewers"
+    help_text = "Suggest reviewers for a patch based on recent changes to the modified files."
+
+    def __init__(self):
+        options = [
+            steps.Options.git_commit,
+        ]
+        AbstractDeclarativeCommand.__init__(self, options=options)
+
+    def execute(self, options, args, tool):
+        reviewers = tool.checkout().suggested_reviewers(options.git_commit)
+        print "\n".join([reviewer.full_name for reviewer in reviewers])
+
+
 class BugsToCommit(AbstractDeclarativeCommand):
     name = "bugs-to-commit"
     help_text = "List bugs in the commit-queue"
@@ -162,15 +179,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
@@ -197,17 +205,21 @@ class FailureReason(AbstractDeclarativeCommand):
     name = "failure-reason"
     help_text = "Lists revisions where individual test failures started at %s" % BuildBot.default_host
 
-    def _print_blame_information_for_transition(self, green_build, red_build, failing_tests):
-        regression_window = RegressionWindow(green_build, red_build)
-        revisions = regression_window.revisions()
+    def _blame_line_for_revision(self, revision):
+        try:
+            commit_info = self._tool.checkout().commit_info_for_revision(revision)
+        except Exception, e:
+            return "FAILED to fetch CommitInfo for r%s, exception: %s" % (revision, e)
+        if not commit_info:
+            return "FAILED to fetch CommitInfo for r%s, likely missing ChangeLog" % revision
+        return commit_info.blame_string(self._tool.bugs)
+
+    def _print_blame_information_for_transition(self, regression_window, failing_tests):
+        red_build = regression_window.failing_build()
         print "SUCCESS: Build %s (r%s) was the first to show failures: %s" % (red_build._number, red_build.revision(), failing_tests)
         print "Suspect revisions:"
-        for revision in revisions:
-            commit_info = self._tool.checkout().commit_info_for_revision(revision)
-            if commit_info:
-                print commit_info.blame_string(self._tool.bugs)
-            else:
-                print "FAILED to fetch CommitInfo for r%s, likely missing ChangeLog" % revision
+        for revision in regression_window.revisions():
+            print self._blame_line_for_revision(revision)
 
     def _explain_failures_for_builder(self, builder, start_revision):
         print "Examining failures for \"%s\", starting at r%s" % (builder.name(), start_revision)
@@ -244,7 +256,8 @@ class FailureReason(AbstractDeclarativeCommand):
                 print "No change in build %s (r%s), %s unexplained failures (%s in this build)" % (build._number, build.revision(), len(results_to_explain), len(failures))
                 last_build_with_results = build
                 continue
-            self._print_blame_information_for_transition(build, last_build_with_results, fixed_results)
+            regression_window = RegressionWindow(build, last_build_with_results)
+            self._print_blame_information_for_transition(regression_window, fixed_results)
             last_build_with_results = build
             results_to_explain -= fixed_results
         if results_to_explain:
@@ -274,6 +287,75 @@ class FailureReason(AbstractDeclarativeCommand):
         return self._explain_failures_for_builder(builder, start_revision=int(start_revision))
 
 
+class FindFlakyTests(AbstractDeclarativeCommand):
+    name = "find-flaky-tests"
+    help_text = "Lists tests that often fail for a single build at %s" % BuildBot.default_host
+
+    def _find_failures(self, builder, revision):
+        build = builder.build_for_revision(revision, allow_failed_lookups=True)
+        if not build:
+            print "No build for %s" % revision
+            return (None, None)
+        results = build.layout_test_results()
+        if not results:
+            print "No results build %s (r%s)" % (build._number, build.revision())
+            return (None, None)
+        failures = set(results.failing_tests())
+        if len(failures) >= 20:
+            # FIXME: We may need to move this logic into the LayoutTestResults class.
+            # The buildbot stops runs after 20 failures so we don't have full results to work with here.
+            print "Too many failures in build %s (r%s), ignoring." % (build._number, build.revision())
+            return (None, None)
+        return (build, failures)
+
+    def _increment_statistics(self, flaky_tests, flaky_test_statistics):
+        for test in flaky_tests:
+            count = flaky_test_statistics.get(test, 0)
+            flaky_test_statistics[test] = count + 1
+
+    def _print_statistics(self, statistics):
+        print "=== Results ==="
+        print "Occurances Test name"
+        for value, key in sorted([(value, key) for key, value in statistics.items()]):
+            print "%10d %s" % (value, key)
+
+    def _walk_backwards_from(self, builder, start_revision, limit):
+        flaky_test_statistics = {}
+        all_previous_failures = set([])
+        one_time_previous_failures = set([])
+        previous_build = None
+        for i in range(limit):
+            revision = start_revision - i
+            print "Analyzing %s ... " % revision,
+            (build, failures) = self._find_failures(builder, revision)
+            if failures == None:
+                # Notice that we don't loop on the empty set!
+                continue
+            print "has %s failures" % len(failures)
+            flaky_tests = one_time_previous_failures - failures
+            if flaky_tests:
+                print "Flaky tests: %s %s" % (sorted(flaky_tests),
+                                              previous_build.results_url())
+            self._increment_statistics(flaky_tests, flaky_test_statistics)
+            one_time_previous_failures = failures - all_previous_failures
+            all_previous_failures = failures
+            previous_build = build
+        self._print_statistics(flaky_test_statistics)
+
+    def _builder_to_analyze(self):
+        statuses = self._tool.buildbot.builder_statuses()
+        choices = [status["name"] for status in statuses]
+        chosen_name = User.prompt_with_list("Which builder to analyze:", choices)
+        for status in statuses:
+            if status["name"] == chosen_name:
+                return (self._tool.buildbot.builder_with_name(chosen_name), status["built_revision"])
+
+    def execute(self, options, args, tool):
+        (builder, latest_revision) = self._builder_to_analyze()
+        limit = self._tool.user.prompt("How many revisions to look through? [10000] ") or 10000
+        return self._walk_backwards_from(builder, latest_revision, limit=int(limit))
+
+
 class TreeStatus(AbstractDeclarativeCommand):
     name = "tree-status"
     help_text = "Print the status of the %s buildbots" % BuildBot.default_host
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queries_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queries_unittest.py
index 7dddfe7..05a4a5c 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queries_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queries_unittest.py
@@ -26,12 +26,15 @@
 # (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.bugzilla import Bugzilla
 from webkitpy.thirdparty.mock import Mock
 from webkitpy.tool.commands.commandtest import CommandsTest
 from webkitpy.tool.commands.queries import *
 from webkitpy.tool.mocktool import MockTool
 
+
 class QueryCommandsTest(CommandsTest):
     def test_bugs_to_commit(self):
         expected_stderr = "Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com)\n"
@@ -71,3 +74,17 @@ class QueryCommandsTest(CommandsTest):
 
         expected_stdout = "Test 'media' is not skipped by any port.\n"
         self.assert_execute_outputs(SkippedPorts(), ("media",), expected_stdout)
+
+
+class FailureReasonTest(unittest.TestCase):
+    def test_blame_line_for_revision(self):
+        tool = MockTool()
+        command = FailureReason()
+        command.bind_to_tool(tool)
+        # This is an artificial example, mostly to test the CommitInfo lookup failure case.
+        self.assertEquals(command._blame_line_for_revision(None), "FAILED to fetch CommitInfo for rNone, likely missing ChangeLog")
+
+        def raising_mock(self):
+            raise Exception("MESSAGE")
+        tool.checkout().commit_info_for_revision = raising_mock
+        self.assertEquals(command._blame_line_for_revision(None), "FAILED to fetch CommitInfo for rNone, exception: MESSAGE")
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
index 80fd2ea..7b3002a 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
@@ -27,6 +27,9 @@
 # (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 __future__ import with_statement
+
+import codecs
 import time
 import traceback
 import os
@@ -36,17 +39,18 @@ from optparse import make_option
 from StringIO import StringIO
 
 from webkitpy.common.net.bugzilla import CommitterValidator
+from webkitpy.common.net.layouttestresults import path_for_layout_test, LayoutTestResults
 from webkitpy.common.net.statusserver import StatusServer
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.common.system.deprecated_logging import error, log
 from webkitpy.tool.commands.stepsequence import StepSequenceErrorHandler
-from webkitpy.tool.bot.commitqueuetask import CommitQueueTask
-from webkitpy.tool.bot.feeders import CommitQueueFeeder
-from webkitpy.tool.bot.patchcollection import PersistentPatchCollection, PersistentPatchCollectionDelegate
+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.grammar import pluralize
 from webkitpy.tool.multicommandtool import Command, TryAgain
 
+
 class AbstractQueue(Command, QueueEngineDelegate):
     watchers = [
     ]
@@ -78,6 +82,10 @@ class AbstractQueue(Command, QueueEngineDelegate):
         # because our global option code looks for the first argument which does
         # not begin with "-" and assumes that is the command name.
         webkit_patch_args += ["--status-host=%s" % self._tool.status_server.host]
+        if self._tool.status_server.bot_id:
+            webkit_patch_args += ["--bot-id=%s" % self._tool.status_server.bot_id]
+        if self._options.port:
+            webkit_patch_args += ["--port=%s" % self._options.port]
         webkit_patch_args.extend(args)
         return self._tool.executive.run_and_throw_if_fail(webkit_patch_args)
 
@@ -94,7 +102,7 @@ class AbstractQueue(Command, QueueEngineDelegate):
 
     def begin_work_queue(self):
         log("CAUTION: %s will discard all local changes in \"%s\"" % (self.name, self._tool.scm().checkout_root))
-        if self.options.confirm:
+        if self._options.confirm:
             response = self._tool.user.prompt("Are you sure?  Type \"yes\" to continue: ")
             if (response != "yes"):
                 error("User declined.")
@@ -106,7 +114,7 @@ class AbstractQueue(Command, QueueEngineDelegate):
 
     def should_continue_work_queue(self):
         self._iteration_count += 1
-        return not self.options.iterations or self._iteration_count <= self.options.iterations
+        return not self._options.iterations or self._iteration_count <= self._options.iterations
 
     def next_work_item(self):
         raise NotImplementedError, "subclasses must implement"
@@ -123,7 +131,7 @@ class AbstractQueue(Command, QueueEngineDelegate):
     # Command methods
 
     def execute(self, options, args, tool, engine=QueueEngine):
-        self.options = options  # FIXME: This code is wrong.  Command.options is a list, this assumes an Options element!
+        self._options = options # FIXME: This code is wrong.  Command.options is a list, this assumes an Options element!
         self._tool = tool  # FIXME: This code is wrong too!  Command.bind_to_tool handles this!
         return engine(self.name, self, self._tool.wakeup_event).run()
 
@@ -159,6 +167,7 @@ class FeederQueue(AbstractQueue):
         AbstractQueue.begin_work_queue(self)
         self.feeders = [
             CommitQueueFeeder(self._tool),
+            EWSFeeder(self._tool),
         ]
 
     def next_work_item(self):
@@ -190,6 +199,9 @@ class AbstractPatchQueue(AbstractQueue):
     def _fetch_next_work_item(self):
         return self._tool.status_server.next_work_item(self.name)
 
+    def _release_work_item(self, patch):
+        self._tool.status_server.release_work_item(self.name, patch)
+
     def _did_pass(self, patch):
         self._update_status(self._pass_status, patch)
 
@@ -207,7 +219,7 @@ class AbstractPatchQueue(AbstractQueue):
         return os.path.join(self._log_directory(), "%s.log" % patch.bug_id())
 
 
-class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
+class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskDelegate):
     name = "commit-queue"
 
     # AbstractPatchQueue methods
@@ -229,7 +241,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
 
     def process_work_item(self, patch):
         self._cc_watchers(patch.bug_id())
-        task = CommitQueueTask(self._tool, self, patch)
+        task = CommitQueueTask(self, patch)
         try:
             if task.run():
                 self._did_pass(patch)
@@ -239,10 +251,22 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
             validator = CommitterValidator(self._tool.bugs)
             validator.reject_patch_from_commit_queue(patch.id(), self._error_message_for_bug(task.failure_status_id, e))
             self._did_fail(patch)
+        self._release_work_item(patch)
+
+    def _error_message_for_bug(self, status_id, script_error):
+        if not script_error.output:
+            return script_error.message_with_output()
+        results_link = self._tool.status_server.results_url_for_status(status_id)
+        return "%s\nFull output: %s" % (script_error.message_with_output(), results_link)
 
     def handle_unexpected_error(self, patch, message):
         self.committer_validator.reject_patch_from_commit_queue(patch.id(), message)
 
+    # CommitQueueTaskDelegate methods
+
+    def run_command(self, command):
+        self.run_webkit_patch(command)
+
     def command_passed(self, message, patch):
         self._update_status(message, patch=patch)
 
@@ -250,11 +274,36 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
         failure_log = self._log_from_script_error_for_upload(script_error)
         return self._update_status(message, patch=patch, results_file=failure_log)
 
-    def _error_message_for_bug(self, status_id, script_error):
-        if not script_error.output:
-            return script_error.message_with_output()
-        results_link = self._tool.status_server.results_url_for_status(status_id)
-        return "%s\nFull output: %s" % (script_error.message_with_output(), results_link)
+    # FIXME: This exists for mocking, but should instead be mocked via
+    # some sort of tool.filesystem() object.
+    def _read_file_contents(self, path):
+        try:
+            with codecs.open(path, "r", "utf-8") as open_file:
+                return open_file.read()
+        except OSError, e:  # File does not exist or can't be read.
+            return None
+
+    # FIXME: This may belong on the Port object.
+    def layout_test_results(self):
+        results_path = self._tool.port().layout_tests_results_path()
+        results_html = self._read_file_contents(results_path)
+        if not results_html:
+            return None
+        return LayoutTestResults.results_from_string(results_html)
+
+    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 [commit_info.author().bugzilla_email() for commit_info in commit_infos if commit_info.author()]
+
+    def report_flaky_tests(self, patch, flaky_tests):
+        authors = self._author_emails_for_tests(flaky_tests)
+        cc_explaination = "  The author(s) of the test(s) have been CCed on this bug." if authors else ""
+        message = "The %s encountered the following flaky tests while processing attachment %s:\n\n%s\n\nPlease file bugs against the tests.%s  The commit-queue is continuing to process your patch." % (self.name, patch.id(), "\n".join(flaky_tests), cc_explaination)
+        self._tool.bugs.post_comment_to_bug(patch.bug_id(), message, cc=authors)
 
     # StepSequenceErrorHandler methods
 
@@ -278,6 +327,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
         raise TryAgain()
 
 
+# FIXME: All the Rietveld code is no longer used and should be deleted.
 class RietveldUploadQueue(AbstractPatchQueue, StepSequenceErrorHandler):
     name = "rietveld-upload-queue"
 
@@ -323,40 +373,27 @@ class RietveldUploadQueue(AbstractPatchQueue, StepSequenceErrorHandler):
         cls._reject_patch(tool, state["patch"].id())
 
 
-class AbstractReviewQueue(AbstractPatchQueue, PersistentPatchCollectionDelegate, StepSequenceErrorHandler):
+class AbstractReviewQueue(AbstractPatchQueue, StepSequenceErrorHandler):
+    """This is the base-class for the EWS queues and the style-queue."""
     def __init__(self, options=None):
         AbstractPatchQueue.__init__(self, options)
 
     def review_patch(self, patch):
-        raise NotImplementedError, "subclasses must implement"
-
-    # PersistentPatchCollectionDelegate methods
-
-    def collection_name(self):
-        return self.name
-
-    def fetch_potential_patch_ids(self):
-        return self._tool.bugs.queries.fetch_attachment_ids_from_review_queue()
-
-    def status_server(self):
-        return self._tool.status_server
-
-    def is_terminal_status(self, status):
-        return status == "Pass" or status == "Fail" or status.startswith("Error:")
+        raise NotImplementedError("subclasses must implement")
 
     # AbstractPatchQueue methods
 
     def begin_work_queue(self):
         AbstractPatchQueue.begin_work_queue(self)
-        self._patches = PersistentPatchCollection(self)
 
     def next_work_item(self):
-        patch_id = self._patches.next()
-        if patch_id:
-            return self._tool.bugs.fetch_attachment(patch_id)
+        patch_id = self._fetch_next_work_item()
+        if not patch_id:
+            return None
+        return self._tool.bugs.fetch_attachment(patch_id)
 
     def should_proceed_with_work_item(self, patch):
-        raise NotImplementedError, "subclasses must implement"
+        raise NotImplementedError("subclasses must implement")
 
     def process_work_item(self, patch):
         try:
@@ -368,6 +405,8 @@ class AbstractReviewQueue(AbstractPatchQueue, PersistentPatchCollectionDelegate,
             if e.exit_code != QueueEngine.handled_error_code:
                 self._did_fail(patch)
             raise e
+        finally:
+            self._release_work_item(patch)
 
     def handle_unexpected_error(self, patch, message):
         log(message)
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
index 029814e..b37b5dc 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
@@ -60,24 +60,31 @@ class AbstractQueueTest(CommandsTest):
     def test_log_directory(self):
         self.assertEquals(TestQueue()._log_directory(), "test-queue-logs")
 
-    def _assert_run_webkit_patch(self, run_args):
+    def _assert_run_webkit_patch(self, run_args, port=None):
         queue = TestQueue()
         tool = MockTool()
+        tool.status_server.bot_id = "gort"
         tool.executive = Mock()
         queue.bind_to_tool(tool)
+        queue._options = Mock()
+        queue._options.port = port
 
         queue.run_webkit_patch(run_args)
-        expected_run_args = ["echo", "--status-host=example.com"] + run_args
+        expected_run_args = ["echo", "--status-host=example.com", "--bot-id=gort"]
+        if port:
+            expected_run_args.append("--port=%s" % port)
+        expected_run_args.extend(run_args)
         tool.executive.run_and_throw_if_fail.assert_called_with(expected_run_args)
 
     def test_run_webkit_patch(self):
         self._assert_run_webkit_patch([1])
         self._assert_run_webkit_patch(["one", 2])
+        self._assert_run_webkit_patch([1], port="mockport")
 
     def test_iteration_count(self):
         queue = TestQueue()
-        queue.options = Mock()
-        queue.options.iterations = 3
+        queue._options = Mock()
+        queue._options.iterations = 3
         self.assertTrue(queue.should_continue_work_queue())
         self.assertTrue(queue.should_continue_work_queue())
         self.assertTrue(queue.should_continue_work_queue())
@@ -85,7 +92,7 @@ class AbstractQueueTest(CommandsTest):
 
     def test_no_iteration_count(self):
         queue = TestQueue()
-        queue.options = Mock()
+        queue._options = Mock()
         self.assertTrue(queue.should_continue_work_queue())
         self.assertTrue(queue.should_continue_work_queue())
         self.assertTrue(queue.should_continue_work_queue())
@@ -128,6 +135,8 @@ MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Reject
 - If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.'
 MOCK: update_work_items: commit-queue [106, 197]
 Feeding commit-queue items [106, 197]
+Feeding EWS (1 r? patch, 1 new)
+MOCK: submit_to_ews: 103
 """,
             "handle_unexpected_error": "Mock error message\n",
         }
@@ -139,25 +148,13 @@ class AbstractPatchQueueTest(CommandsTest):
         queue = AbstractPatchQueue()
         tool = MockTool()
         queue.bind_to_tool(tool)
+        queue._options = Mock()
+        queue._options.port = None
         self.assertEquals(queue._fetch_next_work_item(), None)
         tool.status_server = MockStatusServer(work_items=[2, 1, 3])
         self.assertEquals(queue._fetch_next_work_item(), 2)
 
 
-class AbstractReviewQueueTest(CommandsTest):
-    def test_patch_collection_delegate_methods(self):
-        queue = TestReviewQueue()
-        tool = MockTool()
-        queue.bind_to_tool(tool)
-        self.assertEquals(queue.collection_name(), "test-review-queue")
-        self.assertEquals(queue.fetch_potential_patch_ids(), [103])
-        queue.status_server()
-        self.assertTrue(queue.is_terminal_status("Pass"))
-        self.assertTrue(queue.is_terminal_status("Fail"))
-        self.assertTrue(queue.is_terminal_status("Error: Your patch exploded"))
-        self.assertFalse(queue.is_terminal_status("Foo"))
-
-
 class NeedsUpdateSequence(StepSequence):
     def _run(self, tool, options, state):
         raise CheckoutNeedsUpdate([], 1, "", None)
@@ -172,7 +169,20 @@ class AlwaysCommitQueueTool(object):
 
 
 class SecondThoughtsCommitQueue(CommitQueue):
-    def _build_and_test_patch(self, patch, first_run=True):
+    def __init__(self):
+        self._reject_patch = False
+        CommitQueue.__init__(self)
+
+    def run_command(self, command):
+        # We want to reject the patch after the first validation,
+        # so wait to reject it until after some other command has run.
+        self._reject_patch = True
+        return CommitQueue.run_command(self, command)
+
+    def refetch_patch(self, patch):
+        if not self._reject_patch:
+            return self._tool.bugs.fetch_attachment(patch.id())
+
         attachment_dictionary = {
             "id": patch.id(),
             "bug_id": patch.bug_id(),
@@ -185,9 +195,7 @@ class SecondThoughtsCommitQueue(CommitQueue):
             "committer_email": "foo@bar.com",
             "attacher_email": "Contributer1",
         }
-        patch = Attachment(attachment_dictionary, None)
-        self._tool.bugs.set_override_patch(patch)
-        return True
+        return Attachment(attachment_dictionary, None)
 
 
 class CommitQueueTest(QueuesTest):
@@ -215,6 +223,7 @@ MOCK: update_status: commit-queue Pass
             "process_work_item": """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: 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_script_error": "ScriptError error message\n",
@@ -236,7 +245,7 @@ MOCK: update_status: commit-queue Fail
             "next_work_item": "",
             "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
 MOCK: update_status: commit-queue Applied patch
-MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet']
+MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build', '--no-clean', '--no-update', '--build-style=both', '--quiet']
 MOCK: update_status: commit-queue Built patch
 MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
 MOCK: update_status: commit-queue Passed tests
@@ -259,7 +268,7 @@ MOCK: update_status: commit-queue Pass
             "next_work_item": "",
             "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
 MOCK: update_status: commit-queue Applied patch
-MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet']
+MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build', '--no-clean', '--no-update', '--build-style=both', '--quiet']
 MOCK: update_status: commit-queue Built patch
 MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
 MOCK: update_status: commit-queue Passed tests
@@ -290,14 +299,39 @@ MOCK: update_status: commit-queue Pass
     def test_manual_reject_during_processing(self):
         queue = SecondThoughtsCommitQueue()
         queue.bind_to_tool(MockTool())
+        queue._options = Mock()
+        queue._options.port = None
         expected_stderr = """MOCK: update_status: commit-queue Applied patch
 MOCK: update_status: commit-queue Built patch
 MOCK: update_status: commit-queue Passed tests
-MOCK: update_status: commit-queue Landed patch
-MOCK: update_status: commit-queue Pass
+MOCK: update_status: commit-queue Retry
+MOCK: release_work_item: commit-queue 197
 """
         OutputCapture().assert_outputs(self, queue.process_work_item, [MockPatch()], expected_stderr=expected_stderr)
 
+    def test_report_flaky_tests(self):
+        queue = CommitQueue()
+        queue.bind_to_tool(MockTool())
+        expected_stderr = """MOCK bug comment: bug_id=142, cc=['abarth@webkit.org']
+--- Begin comment ---
+The commit-queue encountered the following flaky tests while processing attachment 197:
+
+foo/bar.html
+bar/baz.html
+
+Please file bugs against the tests.  The author(s) of the test(s) have been CCed on this bug.  The commit-queue is continuing to process your patch.
+--- End comment ---
+
+"""
+        OutputCapture().assert_outputs(self, queue.report_flaky_tests, [MockPatch(), ["foo/bar.html", "bar/baz.html"]], expected_stderr=expected_stderr)
+
+    def test_layout_test_results(self):
+        queue = CommitQueue()
+        queue.bind_to_tool(MockTool())
+        queue._read_file_contents = lambda path: None
+        self.assertEquals(queue.layout_test_results(), None)
+        queue._read_file_contents = lambda path: ""
+        self.assertEquals(queue.layout_test_results(), None)
 
 class RietveldUploadQueueTest(QueuesTest):
     def test_rietveld_upload_queue(self):
@@ -315,11 +349,11 @@ class StyleQueueTest(QueuesTest):
     def test_style_queue(self):
         expected_stderr = {
             "begin_work_queue": self._default_begin_work_queue_stderr("style-queue", MockSCM.fake_checkout_root),
-            "next_work_item": "MOCK: update_work_items: style-queue [103]\n",
+            "next_work_item": "",
             "should_proceed_with_work_item": "MOCK: update_status: style-queue Checking style\n",
-            "process_work_item": "MOCK: update_status: style-queue Pass\n",
+            "process_work_item": "MOCK: update_status: style-queue Pass\nMOCK: release_work_item: style-queue 197\n",
             "handle_unexpected_error": "Mock error message\n",
-            "handle_script_error": "MOCK: update_status: style-queue ScriptError error message\nMOCK bug comment: bug_id=142, cc=[]\n--- Begin comment ---\\Attachment 197 did not pass style-queue:\n\nScriptError error message\n\nIf any of these errors are false positives, please file a bug against check-webkit-style.\n--- End comment ---\n\n",
+            "handle_script_error": "MOCK: update_status: style-queue ScriptError error message\nMOCK bug comment: bug_id=142, cc=[]\n--- Begin comment ---\nAttachment 197 did not pass style-queue:\n\nScriptError error message\n\nIf any of these errors are false positives, please file a bug against check-webkit-style.\n--- End comment ---\n\n",
         }
         expected_exceptions = {
             "handle_script_error": SystemExit,
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py
index 9f3583d..379d380 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py
@@ -80,13 +80,18 @@ class QueuesTest(unittest.TestCase):
         string_replacements = {"name": name, 'checkout_dir': checkout_dir}
         return "CAUTION: %(name)s will discard all local changes in \"%(checkout_dir)s\"\nRunning WebKit %(name)s.\nMOCK: update_status: %(name)s Starting Queue\n" % string_replacements
 
-    def assert_queue_outputs(self, queue, args=None, work_item=None, expected_stdout=None, expected_stderr=None, expected_exceptions=None, options=Mock(), tool=MockTool()):
+    def assert_queue_outputs(self, queue, args=None, work_item=None, expected_stdout=None, expected_stderr=None, expected_exceptions=None, options=None, tool=None):
+        if not tool:
+            tool = MockTool()
         if not expected_stdout:
             expected_stdout = {}
         if not expected_stderr:
             expected_stderr = {}
         if not args:
             args = []
+        if not options:
+            options = Mock()
+            options.port = None
         if not work_item:
             work_item = self.mock_work_item
         tool.user.prompt = lambda message: "yes"
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py
index 23d013d..145f485 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py
@@ -54,77 +54,47 @@ 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, revision):
+        return self._tool.status_server.svn_revision(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)
+        # FIXME: We need to figure out how to provoke_flaky_builders.
 
-        new_failures = self._new_failures(revisions_causing_failures,
-                                          old_failing_svn_revisions)
+        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
 
-        self._sheriff.provoke_flaky_builders(revisions_causing_failures)
-        return new_failures
-
-    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):
-        blame_list = new_failures.keys()
-        for svn_revision, builders in new_failures.items():
+    def process_work_item(self, failure_map):
+        failing_revisions = failure_map.failing_revisions()
+        for revision in failing_revisions:
+            builders = failure_map.builders_failing_for(revision)
+            tests = failure_map.tests_failing_for(revision)
             try:
-                commit_info = self._tool.checkout().commit_info_for_revision(svn_revision)
+                commit_info = self._tool.checkout().commit_info_for_revision(revision)
                 if not commit_info:
                     print "FAILED to fetch CommitInfo for r%s, likely missing ChangeLog" % revision
                     continue
                 self._sheriff.post_irc_warning(commit_info, builders)
-                self._sheriff.post_blame_comment_on_bug(commit_info,
-                                                        builders,
-                                                        blame_list)
-                self._sheriff.post_automatic_rollout_patch(commit_info,
-                                                           builders)
+                self._sheriff.post_blame_comment_on_bug(commit_info, builders, tests)
+
             finally:
                 for builder in builders:
-                    self._tool.status_server.update_svn_revision(svn_revision,
-                                                                builder.name())
+                    self._tool.status_server.update_svn_revision(revision, 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..32eb016 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
@@ -30,7 +30,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.tool.mocktool import *
 
 
 class SheriffBotTest(QueuesTest):
@@ -38,36 +38,19 @@ class SheriffBotTest(QueuesTest):
     builder2 = MockBuilder("Builder2")
 
     def test_sheriff_bot(self):
-        mock_work_item = {
-            29837: [self.builder1],
-        }
+        mock_work_item = MockFailureMap(MockTool().buildbot)
         expected_stderr = {
             "begin_work_queue": self._default_begin_work_queue_stderr("sheriff-bot", os.getcwd()),
             "next_work_item": "",
-            "process_work_item": "MOCK: irc.post: abarth, darin, eseidel: http://trac.webkit.org/changeset/29837 might have broken Builder1\nMOCK bug comment: bug_id=42, cc=['abarth@webkit.org', 'eric@webkit.org']\n--- Begin comment ---\\http://trac.webkit.org/changeset/29837 might have broken Builder1\n--- End comment ---\n\n",
+            "process_work_item": """MOCK: irc.post: abarth, darin, eseidel: http://trac.webkit.org/changeset/29837 might have broken Builder1
+MOCK bug comment: bug_id=42, cc=['abarth@webkit.org', 'eric@webkit.org']
+--- Begin comment ---
+http://trac.webkit.org/changeset/29837 might have broken Builder1
+The following tests are not passing:
+mock-test-1
+--- End comment ---
+
+""",
             "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/commands/upload_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py
index 5f3f400..0d096b6 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py
@@ -52,11 +52,12 @@ class UploadCommandsTest(CommandsTest):
 
     def test_post(self):
         options = MockOptions()
+        options.cc = None
+        options.check_style = True
+        options.comment = None
         options.description = "MOCK description"
         options.request_commit = False
         options.review = True
-        options.comment = None
-        options.cc = None
         expected_stderr = """Running check-webkit-style
 MOCK: user.open_url: file://...
 Obsoleting 2 old patches on bug 42
@@ -81,11 +82,12 @@ MOCK: user.open_url: http://example.com/42
 
     def test_upload(self):
         options = MockOptions()
+        options.cc = None
+        options.check_style = True
+        options.comment = None
         options.description = "MOCK description"
         options.request_commit = False
         options.review = True
-        options.comment = None
-        options.cc = None
         expected_stderr = """Running check-webkit-style
 MOCK: user.open_url: file://...
 Obsoleting 2 old patches on bug 42
@@ -103,7 +105,7 @@ MOCK: user.open_url: http://example.com/42
         options = Mock()
         options.bug_id = 42
         options.comment = "MOCK comment"
-        expected_stderr = "Bug: <http://example.com/42> Bug with two r+'d and cq+'d patches, one of which has an invalid commit-queue setter.\nRevision: 9876\nMOCK: user.open_url: http://example.com/42\nAdding comment to Bug 42.\nMOCK bug comment: bug_id=42, cc=None\n--- Begin comment ---\\MOCK comment\n\nCommitted r9876: <http://trac.webkit.org/changeset/9876>\n--- End comment ---\n\n"
+        expected_stderr = "Bug: <http://example.com/42> Bug with two r+'d and cq+'d patches, one of which has an invalid commit-queue setter.\nRevision: 9876\nMOCK: user.open_url: http://example.com/42\nAdding comment to Bug 42.\nMOCK bug comment: bug_id=42, cc=None\n--- Begin comment ---\nMOCK comment\n\nCommitted r9876: <http://trac.webkit.org/changeset/9876>\n--- End comment ---\n\n"
         self.assert_execute_outputs(MarkBugFixed(), [], expected_stderr=expected_stderr, tool=tool, options=options)
 
     def test_edit_changelog(self):