Merge WebKit at r59636: Initial merge by git

Change-Id: I59b289c4e6b18425f06ce41cc9d34c522515de91

9 files changed, 154 insertions, 48 deletions

diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download.py b/WebKitTools/Scripts/webkitpy/tool/commands/download.py
index c66b95c..a283da9 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/download.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/download.py
@@ -260,8 +260,9 @@ class AbstractRolloutPrepCommand(AbstractSequencedCommand):
             #       of create-rollout for bug URLs.  It should do better
             #       parsing instead.
             log("Preparing rollout for bug %s." % commit_info.bug_id())
-            return commit_info
-        log("Unable to parse bug number from diff.")
+        else:
+            log("Unable to parse bug number from diff.")
+        return commit_info
 
     def _prepare_state(self, options, args, tool):
         revision = args[0]
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py
index 926037c..4dd9d7f 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py
@@ -26,9 +26,32 @@
 # (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.system.outputcapture import OutputCapture
 from webkitpy.thirdparty.mock import Mock
 from webkitpy.tool.commands.commandtest import CommandsTest
 from webkitpy.tool.commands.download import *
+from webkitpy.tool.mocktool import MockTool
+
+
+class AbstractRolloutPrepCommandTest(unittest.TestCase):
+    def test_commit_info(self):
+        command = AbstractRolloutPrepCommand()
+        tool = MockTool()
+        command.bind_to_tool(tool)
+        output = OutputCapture()
+
+        expected_stderr = "Preparing rollout for bug 42.\n"
+        commit_info = output.assert_outputs(self, command._commit_info, [1234], expected_stderr=expected_stderr)
+        self.assertTrue(commit_info)
+
+        mock_commit_info = Mock()
+        mock_commit_info.bug_id = lambda: None
+        tool._checkout.commit_info_for_revision = lambda revision: mock_commit_info
+        expected_stderr = "Unable to parse bug number from diff.\n"
+        commit_info = output.assert_outputs(self, command._commit_info, [1234], expected_stderr=expected_stderr)
+        self.assertEqual(commit_info, mock_commit_info)
 
 
 class DownloadCommandsTest(CommandsTest):
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
index 4d23a4c..3d0ddd1 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
@@ -39,37 +39,37 @@ class EarlyWarningSytemTest(QueuesTest):
         ews._can_build = lambda: True
         ews.review_patch(Mock())
 
-    def test_chromium_linux_ews(self):
+    def _default_expected_stderr(self, ews):
+        string_replacemnts = {
+            "name": ews.name,
+            "checkout_dir": os.getcwd(),  # FIXME: Use of os.getcwd() is wrong, should be scm.checkout_root
+        }
         expected_stderr = {
-            "begin_work_queue": "CAUTION: chromium-ews will discard all local changes in \"%s\"\nRunning WebKit chromium-ews.\n" % os.getcwd(),
+            "begin_work_queue": "CAUTION: %(name)s will discard all local changes in \"%(checkout_dir)s\"\nRunning WebKit %(name)s.\n" % string_replacemnts,
             "handle_unexpected_error": "Mock error message\n",
+            "process_work_item": "MOCK: update_status: %(name)s Pass\n" % string_replacemnts,
         }
-        self.assert_queue_outputs(ChromiumLinuxEWS(), expected_stderr=expected_stderr)
+        return expected_stderr
+
+    def _test_ews(self, ews):
+        self.assert_queue_outputs(ews, expected_stderr=self._default_expected_stderr(ews))
+
+    # FIXME: If all EWSes are going to output the same text, we
+    # could test them all in one method with a for loop over an array.
+    def test_chromium_linux_ews(self):
+        self._test_ews(ChromiumLinuxEWS())
 
     def test_chromium_windows_ews(self):
-        expected_stderr = {
-            "begin_work_queue": "CAUTION: cr-win-ews will discard all local changes in \"%s\"\nRunning WebKit cr-win-ews.\n" % os.getcwd(),
-            "handle_unexpected_error": "Mock error message\n",
-        }
-        self.assert_queue_outputs(ChromiumWindowsEWS(), expected_stderr=expected_stderr)
+        self._test_ews(ChromiumWindowsEWS())
 
     def test_qt_ews(self):
-        expected_stderr = {
-            "begin_work_queue": "CAUTION: qt-ews will discard all local changes in \"%s\"\nRunning WebKit qt-ews.\n" % os.getcwd(),
-            "handle_unexpected_error": "Mock error message\n",
-        }
-        self.assert_queue_outputs(QtEWS(), expected_stderr=expected_stderr)
+        self._test_ews(QtEWS())
 
     def test_gtk_ews(self):
-        expected_stderr = {
-            "begin_work_queue": "CAUTION: gtk-ews will discard all local changes in \"%s\"\nRunning WebKit gtk-ews.\n" % os.getcwd(),
-            "handle_unexpected_error": "Mock error message\n",
-        }
-        self.assert_queue_outputs(GtkEWS(), expected_stderr=expected_stderr)
+        self._test_ews(GtkEWS())
 
     def test_mac_ews(self):
-        expected_stderr = {
-            "begin_work_queue": "CAUTION: mac-ews will discard all local changes in \"%s\"\nRunning WebKit mac-ews.\n" % os.getcwd(),
-            "handle_unexpected_error": "Mock error message\n",
-        }
-        self.assert_queue_outputs(MacEWS(), expected_stderr=expected_stderr)
+        ews = MacEWS()
+        expected_stderr = self._default_expected_stderr(ews)
+        expected_stderr["process_work_item"] = "MOCK: update_status: mac-ews Error: mac-ews cannot process patches from non-committers :(\n"
+        self.assert_queue_outputs(ews, expected_stderr=expected_stderr)
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
index 775aa44..78ca729 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
@@ -46,7 +46,6 @@ from webkitpy.tool.multicommandtool import Command
 
 class AbstractQueue(Command, QueueEngineDelegate):
     watchers = [
-        "webkit-bot-watchers@googlegroups.com",
     ]
 
     _pass_status = "Pass"
@@ -168,16 +167,17 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
         # Not using BugzillaQueries.fetch_patches_from_commit_queue() so we can reject patches with invalid committers/reviewers.
         bug_ids = self.tool.bugs.queries.fetch_bug_ids_from_commit_queue()
         all_patches = sum([self.tool.bugs.fetch_bug(bug_id).commit_queued_patches(include_invalid=True) for bug_id in bug_ids], [])
-        valid_patches = self.committer_validator.patches_after_rejecting_invalid_commiters_and_reviewers(all_patches)
-        if not self._builders_are_green():
-            return filter(lambda patch: patch.is_rollout(), valid_patches)
-        return valid_patches
+        return self.committer_validator.patches_after_rejecting_invalid_commiters_and_reviewers(all_patches)
 
     def next_work_item(self):
         patches = self._validate_patches_in_commit_queue()
+        builders_are_green = self._builders_are_green()
+        if not builders_are_green:
+            patches = filter(lambda patch: patch.is_rollout(), patches)
         # FIXME: We could sort the patches in a specific order here, was suggested by https://bugs.webkit.org/show_bug.cgi?id=33395
         if not patches:
-            self._update_status("Empty queue")
+            queue_text = "queue" if builders_are_green else "rollout queue"
+            self._update_status("Empty %s" % queue_text)
             return None
         # Only bother logging if we have patches in the queue.
         self.log_progress([patch.id() for patch in patches])
@@ -211,7 +211,8 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
         if not patch.is_rollout():
             if not self._builders_are_green():
                 return False
-        self._update_status("Landing patch", patch)
+        patch_text = "rollout patch" if patch.is_rollout() else "patch"
+        self._update_status("Landing %s" % patch_text, patch)
         return True
 
     def _land(self, patch, first_run=False):
@@ -226,7 +227,6 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
                 "land-attachment",
                 "--force-clean",
                 "--build",
-                "--test",
                 "--non-interactive",
                 # The master process is responsible for checking the status
                 # of the builders (see above call to _builders_are_green).
@@ -235,6 +235,9 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
                 "--quiet",
                 patch.id()
             ]
+            # We don't bother to run tests for rollouts as that makes them too slow.
+            if not patch.is_rollout():
+                args.append("--test")
             if not first_run:
                 # The first time through, we don't reject the patch from the
                 # commit queue because we want to make sure we can build and
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
index 16eb053..0bd42fb 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
@@ -118,11 +118,13 @@ class CommitQueueTest(QueuesTest):
     def test_commit_queue(self):
         expected_stderr = {
             "begin_work_queue" : "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root,
+            "should_proceed_with_work_item": "MOCK: update_status: commit-queue Landing patch\n",
             # FIXME: The commit-queue warns about bad committers twice.  This is due to the fact that we access Attachment.reviewer() twice and it logs each time.
             "next_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)
 2 patches in commit-queue [197, 106]
 """,
+            "process_work_item" : "MOCK: update_status: commit-queue Pass\n",
         }
         self.assert_queue_outputs(CommitQueue(), expected_stderr=expected_stderr)
 
@@ -131,11 +133,14 @@ Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.c
         tool.buildbot.light_tree_on_fire()
         expected_stderr = {
             "begin_work_queue" : "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root,
+            "should_proceed_with_work_item": "MOCK: update_status: commit-queue Builders [\"Builder2\"] are red. See http://build.webkit.org\n",
             # FIXME: The commit-queue warns about bad committers twice.  This is due to the fact that we access Attachment.reviewer() twice and it logs each time.
             "next_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: update_status: commit-queue Builders ["Builder2"] are red. See http://build.webkit.org
 1 patch in commit-queue [106]
 """,
+            "process_work_item" : "MOCK: update_status: commit-queue Builders [\"Builder2\"] are red. See http://build.webkit.org\n",
         }
         self.assert_queue_outputs(CommitQueue(), tool=tool, expected_stderr=expected_stderr)
 
@@ -145,20 +150,33 @@ Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.c
         rollout_patch = MockPatch()
         expected_stderr = {
             "begin_work_queue": "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root,
+            "should_proceed_with_work_item": "MOCK: update_status: commit-queue Landing rollout patch\n",
             # FIXME: The commit-queue warns about bad committers twice.  This is due to the fact that we access Attachment.reviewer() twice and it logs each time.
             "next_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: update_status: commit-queue Builders ["Builder2"] are red. See http://build.webkit.org
 1 patch in commit-queue [106]
 """,
-            "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 76543]\n",
+            "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 76543]\nMOCK: update_status: commit-queue Pass\n",
         }
         self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_stderr=expected_stderr)
 
+    def test_can_build_and_test(self):
+        queue = CommitQueue()
+        tool = MockTool()
+        tool.executive = Mock()
+        queue.bind_to_tool(tool)
+        self.assertTrue(queue._can_build_and_test())
+        expected_run_args = ["echo", "--status-host=example.com", "build-and-test", "--force-clean", "--build", "--test", "--non-interactive", "--no-update", "--build-style=both", "--quiet"]
+        tool.executive.run_and_throw_if_fail.assert_called_with(expected_run_args)
+
 
 class StyleQueueTest(QueuesTest):
     def test_style_queue(self):
         expected_stderr = {
             "begin_work_queue" : "CAUTION: style-queue will discard all local changes in \"%s\"\nRunning WebKit style-queue.\n" % MockSCM.fake_checkout_root,
+            "should_proceed_with_work_item": "MOCK: update_status: style-queue Checking style\n",
+            "process_work_item" : "MOCK: update_status: style-queue Pass\n",
             "handle_unexpected_error" : "Mock error message\n",
         }
         self.assert_queue_outputs(StyleQueue(), expected_stderr=expected_stderr)
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py
index eb80d8f..24c8517 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py
@@ -57,17 +57,46 @@ class SheriffBot(AbstractQueue, StepSequenceErrorHandler):
     def work_item_log_path(self, new_failures):
         return os.path.join("%s-logs" % self.name, "%s.log" % new_failures.keys()[0])
 
-    def next_work_item(self):
-        self._irc_bot.process_pending_messages()
-        self._update()
+    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 = {}
-        revisions_causing_failures = self.tool.buildbot.revisions_causing_failures()
         for svn_revision, builders in revisions_causing_failures.items():
-            if self.tool.status_server.svn_revision(svn_revision):
+            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_failures[svn_revision] = builders
+            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 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.revisions_causing_failures()
+
+        # Similarly, we read once from our the status_server.
+        old_failing_svn_revisions = []
+        for svn_revision in revisions_causing_failures.keys():
+            if self.tool.status_server.svn_revision(svn_revision):
+                old_failing_svn_revisions.append(svn_revision)
+
+        new_failures = self._new_failures(revisions_causing_failures,
+                                          old_failing_svn_revisions)
+
         self._sheriff.provoke_flaky_builders(revisions_causing_failures)
         return new_failures
 
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
index f121eda..4b4b8b6 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
@@ -30,18 +30,44 @@ import os
 
 from webkitpy.tool.commands.queuestest import QueuesTest
 from webkitpy.tool.commands.sheriffbot import SheriffBot
-from webkitpy.tool.mocktool import mock_builder
+from webkitpy.tool.mocktool import MockBuilder
 
 
 class SheriffBotTest(QueuesTest):
+    builder1 = MockBuilder("Builder1")
+    builder2 = MockBuilder("Builder2")
+
     def test_sheriff_bot(self):
         mock_work_item = {
-            29837: [mock_builder],
+            29837: [self.builder1],
         }
         expected_stderr = {
             "begin_work_queue": "CAUTION: sheriff-bot will discard all local changes in \"%s\"\nRunning WebKit sheriff-bot.\n" % os.getcwd(),
             "next_work_item": "",
-            "process_work_item": "MOCK: irc.post: abarth, darin, eseidel: http://trac.webkit.org/changeset/29837 might have broken Mock builder name (Tests)\nMOCK bug comment: bug_id=42, cc=['webkit-bot-watchers@googlegroups.com', 'abarth@webkit.org', 'eric@webkit.org']\n--- Begin comment ---\\http://trac.webkit.org/changeset/29837 might have broken Mock builder name (Tests)\n--- End comment ---\n\n",
+            "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",
             "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.py b/WebKitTools/Scripts/webkitpy/tool/commands/upload.py
index 99d45a6..4797ef6 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/upload.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/upload.py
@@ -46,14 +46,23 @@ from webkitpy.tool.comments import bug_comment_from_svn_revision
 from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand
 from webkitpy.common.system.deprecated_logging import error, log
 
+
 class CommitMessageForCurrentDiff(AbstractDeclarativeCommand):
     name = "commit-message"
     help_text = "Print a commit message suitable for the uncommitted changes"
 
+    def __init__(self):
+        options = [
+            steps.Options.squash,
+            steps.Options.git_commit,
+        ]
+        AbstractDeclarativeCommand.__init__(self, options=options)
+
     def execute(self, options, args, tool):
         # This command is a useful test to make sure commit_message_for_this_commit
         # always returns the right value regardless of the current working directory.
-        print "%s" % tool.checkout().commit_message_for_this_commit().message()
+        print "%s" % tool.checkout().commit_message_for_this_commit(options.git_commit, options.squash).message()
+
 
 class CleanPendingCommit(AbstractDeclarativeCommand):
     name = "clean-pending-commit"
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py
index eec3751..8f6483a 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py
@@ -34,10 +34,7 @@ from webkitpy.tool.mocktool import MockTool
 class UploadCommandsTest(CommandsTest):
     def test_commit_message_for_current_diff(self):
         tool = MockTool()
-        mock_commit_message_for_this_commit = Mock()
-        mock_commit_message_for_this_commit.message = lambda: "Mock message"
-        tool._checkout.commit_message_for_this_commit = lambda: mock_commit_message_for_this_commit
-        expected_stdout = "Mock message\n"
+        expected_stdout = "This is a fake commit message that is at least 50 characters.\n"
         self.assert_execute_outputs(CommitMessageForCurrentDiff(), [], expected_stdout=expected_stdout, tool=tool)
 
     def test_clean_pending_commit(self):