[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
abarth at webkit.org
abarth at webkit.org
Wed Dec 22 13:40:20 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 47a19597e453d085dab154b6e58cac710470e9df
Author: abarth at webkit.org <abarth at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Thu Sep 23 00:37:41 2010 +0000
2010-09-22 Adam Barth <abarth at webkit.org>
Reviewed by Eric Seidel.
Optimize commit-queue performance for green trees
https://bugs.webkit.org/show_bug.cgi?id=46254
This patch redesigns the controller logic for the commit-queue. In the
new design, the controller exercises much finer-grained control over
the landing process. In particular:
- Patches that fail to apply now get rejected almost immediately.
- Patches that fail to build get rejects after two builds (instead of
three builds and one test run).
- Patches that run into a flaky test now get accepted after one build
and two test runs instead of three full build-and-test runs.
The main cost of these optimizations is that we don't find out the tree
has a failing test until the very end of the process, but if the tree
has a busted test, there's not much we can do anyway. We might as well
burn commit-queue resources spinning optimisticly.
* Scripts/webkitpy/tool/bot/commitqueuetask.py: Added.
* Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py: Added.
* Scripts/webkitpy/tool/commands/queues.py:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@68102 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 265516d..6bba797 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,29 @@
+2010-09-22 Adam Barth <abarth at webkit.org>
+
+ Reviewed by Eric Seidel.
+
+ Optimize commit-queue performance for green trees
+ https://bugs.webkit.org/show_bug.cgi?id=46254
+
+ This patch redesigns the controller logic for the commit-queue. In the
+ new design, the controller exercises much finer-grained control over
+ the landing process. In particular:
+
+ - Patches that fail to apply now get rejected almost immediately.
+ - Patches that fail to build get rejects after two builds (instead of
+ three builds and one test run).
+ - Patches that run into a flaky test now get accepted after one build
+ and two test runs instead of three full build-and-test runs.
+
+ The main cost of these optimizations is that we don't find out the tree
+ has a failing test until the very end of the process, but if the tree
+ has a busted test, there's not much we can do anyway. We might as well
+ burn commit-queue resources spinning optimisticly.
+
+ * Scripts/webkitpy/tool/bot/commitqueuetask.py: Added.
+ * Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py: Added.
+ * Scripts/webkitpy/tool/commands/queues.py:
+
2010-09-22 Brent Fulgham <bfulgham at webkit.org>
Reviewed by Martin Robinson.
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py
new file mode 100644
index 0000000..9b690fb
--- /dev/null
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py
@@ -0,0 +1,144 @@
+# 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.
+
+from webkitpy.common.system.executive import ScriptError
+
+
+class CommitQueueTask(object):
+ def __init__(self, tool, commit_queue, patch):
+ self._tool = tool
+ self._commit_queue = commit_queue
+ self._patch = patch
+ self._script_error = None
+
+ def _validate(self):
+ # Bugs might get closed, or patches might be obsoleted or r-'d while the
+ # commit-queue is processing.
+ self._patch = self._tool.bugs.fetch_attachment(self._patch.id())
+ if self._patch.is_obsolete():
+ return False
+ if self._patch.bug().is_closed():
+ return False
+ if not self._patch.committer():
+ return False
+ # Reviewer is not required. Missing reviewers will be caught during
+ # the ChangeLog check during landing.
+ return True
+
+ def _run_command(self, command, failure_message):
+ try:
+ self._commit_queue.run_webkit_patch(command)
+ return True
+ except ScriptError, e:
+ self._script_error = e
+ self.failure_status_id = self._commit_queue.command_failed(failure_message, script_error=self._script_error, patch=self._patch)
+ return False
+
+ def _apply(self):
+ return self._run_command([
+ "apply-attachment",
+ "--force-clean",
+ "--non-interactive",
+ "--quiet",
+ self._patch.id(),
+ ], "Patch does not apply")
+
+ def _build(self):
+ return self._run_command([
+ "build",
+ "--no-clean",
+ "--no-update",
+ "--build",
+ "--build-style=both",
+ "--quiet",
+ ], "Patch does not build")
+
+ def _build_without_patch(self):
+ return self._run_command([
+ "build",
+ "--force-clean",
+ "--no-update",
+ "--build",
+ "--build-style=both",
+ "--quiet",
+ ], "Unable to build without patch")
+
+ def _test(self):
+ return self._run_command([
+ "build-and-test",
+ "--no-clean",
+ "--no-update",
+ # Notice that we don't pass --build, which means we won't build!
+ "--test",
+ "--quiet",
+ "--non-interactive",
+ ], "Patch does not pass tests")
+
+ def _build_and_test_without_patch(self):
+ return self._run_command([
+ "build-and-test",
+ "--force-clean",
+ "--no-update",
+ "--build",
+ "--test",
+ "--quiet",
+ "--non-interactive",
+ ], "Unable to pass tests without patch")
+
+ def _land(self):
+ return self._run_command([
+ "land-attachment",
+ "--force-clean",
+ "--ignore-builders",
+ "--quiet",
+ "--non-interactive",
+ "--parent-command=commit-queue",
+ self._patch.id(),
+ ], "Unable to land patch")
+
+ def run(self):
+ if not self._validate():
+ return False
+ if not self._apply():
+ raise self._script_error
+ if not self._build():
+ if not self._build_without_patch():
+ return False
+ raise self._script_error
+ if not self._patch.is_rollout():
+ if not self._test():
+ if not self._test():
+ if not self._build_and_test_without_patch():
+ return False
+ raise self._script_error
+ # Make sure the patch is still valid before landing (e.g., make sure
+ # no one has set commit-queue- since we started working on the patch.)
+ if not self._validate():
+ return False
+ self._land()
+ return True
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py
new file mode 100644
index 0000000..1b933eb
--- /dev/null
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py
@@ -0,0 +1,156 @@
+# 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.
+
+from datetime import datetime
+import unittest
+
+from webkitpy.common.system.deprecated_logging import error, log
+from webkitpy.common.system.outputcapture import OutputCapture
+from webkitpy.thirdparty.mock import Mock
+from webkitpy.tool.bot.commitqueuetask import *
+from webkitpy.tool.mocktool import MockTool
+
+
+class MockCommitQueue:
+ def __init__(self, error_plan):
+ self._error_plan = error_plan
+
+ def run_webkit_patch(self, command):
+ log("run_webkit_patch: %s" % command)
+ if self._error_plan:
+ error = self._error_plan.pop(0)
+ if error:
+ raise error
+
+ def command_failed(self, failure_message, script_error, patch):
+ log("command_failed: failure_message='%s' script_error='%s' patch='%s'" % (
+ failure_message, script_error, patch.id()))
+ return 3947
+
+
+class CommitQueueTaskTest(unittest.TestCase):
+ def _run_through_task(self, commit_queue, expected_stderr, expected_exception=None):
+ tool = MockTool(log_executive=True)
+ patch = tool.bugs.fetch_attachment(197)
+ task = CommitQueueTask(tool, commit_queue, patch)
+ OutputCapture().assert_outputs(self, task.run, expected_stderr=expected_stderr, expected_exception=expected_exception)
+
+ def test_success_case(self):
+ commit_queue = MockCommitQueue([])
+ expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
+run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet']
+run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
+run_webkit_patch: ['land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 197]
+"""
+ self._run_through_task(commit_queue, expected_stderr)
+
+ def test_apply_failure(self):
+ commit_queue = MockCommitQueue([
+ ScriptError("MOCK apply failure"),
+ ])
+ expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
+command_failed: failure_message='Patch does not apply' script_error='MOCK apply failure' patch='197'
+"""
+ self._run_through_task(commit_queue, expected_stderr, ScriptError)
+
+ def test_build_failure(self):
+ commit_queue = MockCommitQueue([
+ None,
+ ScriptError("MOCK build failure"),
+ ])
+ expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
+run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet']
+command_failed: failure_message='Patch does not build' script_error='MOCK build failure' patch='197'
+run_webkit_patch: ['build', '--force-clean', '--no-update', '--build', '--build-style=both', '--quiet']
+"""
+ self._run_through_task(commit_queue, expected_stderr, ScriptError)
+
+ def test_red_build_failure(self):
+ commit_queue = MockCommitQueue([
+ None,
+ ScriptError("MOCK build failure"),
+ ScriptError("MOCK clean build failure"),
+ ])
+ expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
+run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet']
+command_failed: failure_message='Patch does not build' script_error='MOCK build failure' patch='197'
+run_webkit_patch: ['build', '--force-clean', '--no-update', '--build', '--build-style=both', '--quiet']
+command_failed: failure_message='Unable to build without patch' script_error='MOCK clean build failure' patch='197'
+"""
+ self._run_through_task(commit_queue, expected_stderr)
+
+ def test_flaky_test_failure(self):
+ commit_queue = MockCommitQueue([
+ None,
+ None,
+ ScriptError("MOCK tests failure"),
+ ])
+ expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
+run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet']
+run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
+command_failed: failure_message='Patch does not pass tests' script_error='MOCK tests failure' patch='197'
+run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
+run_webkit_patch: ['land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 197]
+"""
+ self._run_through_task(commit_queue, expected_stderr)
+
+ def test_test_failure(self):
+ commit_queue = MockCommitQueue([
+ None,
+ None,
+ ScriptError("MOCK test failure"),
+ ScriptError("MOCK test failure again"),
+ ])
+ expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
+run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet']
+run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
+command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure' patch='197'
+run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
+command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure again' patch='197'
+run_webkit_patch: ['build-and-test', '--force-clean', '--no-update', '--build', '--test', '--quiet', '--non-interactive']
+"""
+ self._run_through_task(commit_queue, expected_stderr, ScriptError)
+
+ def test_red_test_failure(self):
+ commit_queue = MockCommitQueue([
+ None,
+ None,
+ ScriptError("MOCK test failure"),
+ ScriptError("MOCK test failure again"),
+ ScriptError("MOCK clean test failure"),
+ ])
+ expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
+run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet']
+run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
+command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure' patch='197'
+run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
+command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure again' patch='197'
+run_webkit_patch: ['build-and-test', '--force-clean', '--no-update', '--build', '--test', '--quiet', '--non-interactive']
+command_failed: failure_message='Unable to pass tests without patch' script_error='MOCK clean test failure' patch='197'
+"""
+ self._run_through_task(commit_queue, expected_stderr)
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
index ff5b19f..374a7fd 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
@@ -40,6 +40,7 @@ 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.queueengine import QueueEngine, QueueEngineDelegate
@@ -52,6 +53,7 @@ class AbstractQueue(Command, QueueEngineDelegate):
_pass_status = "Pass"
_fail_status = "Fail"
+ _retry_status = "Retry"
_error_status = "Error"
def __init__(self, options=None): # Default values should never be collections (like []) as default values are shared between invocations
@@ -183,7 +185,7 @@ class FeederQueue(AbstractQueue):
class AbstractPatchQueue(AbstractQueue):
def _update_status(self, message, patch=None, results_file=None):
- self._tool.status_server.update_status(self.name, message, patch, results_file)
+ return self._tool.status_server.update_status(self.name, message, patch, results_file)
def _fetch_next_work_item(self):
return self._tool.status_server.next_work_item(self.name)
@@ -194,6 +196,9 @@ class AbstractPatchQueue(AbstractQueue):
def _did_fail(self, patch):
self._update_status(self._fail_status, patch)
+ def _did_retry(self, patch):
+ self._update_status(self._retry_status, patch)
+
def _did_error(self, patch, reason):
message = "%s: %s" % (self._error_status, reason)
self._update_status(message, patch)
@@ -217,134 +222,44 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
return None
return self._tool.bugs.fetch_attachment(patch_id)
- def _can_build_and_test_without_patch(self):
- try:
- self.run_webkit_patch([
- "build-and-test",
- "--force-clean",
- "--build",
- "--test",
- "--non-interactive",
- "--no-update",
- "--build-style=both",
- "--quiet"])
- return True
- except ScriptError, e:
- failure_log = self._log_from_script_error_for_upload(e)
- self._update_status("Unable to build and test without patch", results_file=failure_log)
- return False
-
def should_proceed_with_work_item(self, patch):
patch_text = "rollout patch" if patch.is_rollout() else "patch"
self._update_status("Landing %s" % patch_text, patch)
return True
- def _build_and_test_patch(self, patch, first_run=False):
- try:
- args = [
- "build-and-test-attachment",
- "--force-clean",
- "--build",
- "--non-interactive",
- "--build-style=both",
- "--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
- # test ourselves. However, the second time through, we
- # register ourselves as the parent-command so we can reject
- # the patch on failure.
- args.append("--parent-command=commit-queue")
- # The second time through, we also don't want to update so we
- # know we're testing the same revision that we successfully
- # built and tested.
- args.append("--no-update")
- self.run_webkit_patch(args)
- return True
- except ScriptError, e:
- failure_log = self._log_from_script_error_for_upload(e)
- self._update_status("Unable to build and test patch", patch=patch, results_file=failure_log)
- if first_run:
- return False
- self._did_fail(patch)
- raise
-
- def _revalidate_patch(self, patch):
- # Bugs might get closed, or patches might be obsoleted or r-'d while the
- # commit-queue is processing. Do one last minute check before landing.
- patch = self._tool.bugs.fetch_attachment(patch.id())
- if patch.is_obsolete():
- return None
- if patch.bug().is_closed():
- return None
- if not patch.committer():
- return None
- # Reviewer is not required. Misisng reviewers will be caught during the ChangeLog check during landing.
- return patch
-
- def _land(self, patch):
+ def process_work_item(self, patch):
+ self._cc_watchers(patch.bug_id())
+ task = CommitQueueTask(self._tool, self, patch)
try:
- args = [
- "land-attachment",
- "--force-clean",
- "--non-interactive",
- "--ignore-builders",
- "--quiet",
- "--parent-command=commit-queue",
- patch.id(),
- ]
- self.run_webkit_patch(args)
- self._did_pass(patch)
+ if task.run():
+ self._did_pass(patch)
+ return True
+ self._did_retry(patch)
except ScriptError, e:
- failure_log = self._log_from_script_error_for_upload(e)
- self._update_status("Unable to land patch", patch=patch, results_file=failure_log)
+ 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)
- raise
-
- def process_work_item(self, patch):
- self._cc_watchers(patch.bug_id())
- if not self._build_and_test_patch(patch, first_run=True):
- self._update_status("Building and testing without the patch as a sanity check", patch)
- # The patch failed to build and test. It's possible that the
- # tree is busted. To check that case, we try to build and test
- # without the patch.
- if not self._can_build_and_test_without_patch():
- return False
- self._update_status("Build and test succeeded, trying again with patch", patch)
- # Hum, looks like the patch is actually bad. Of course, we could
- # have been bitten by a flaky test the first time around. We try
- # to build and test again. If it fails a second time, we're pretty
- # sure its a bad test and re can reject it outright.
- self._build_and_test_patch(patch)
- # Do one last check to catch any bug changes (cq-, closed, reviewer changed, etc.)
- # This helps catch races between the bots if locks expire.
- patch = self._revalidate_patch(patch)
- if not patch:
- return False
- self._land(patch)
- return True
def handle_unexpected_error(self, patch, message):
self.committer_validator.reject_patch_from_commit_queue(patch.id(), message)
- # StepSequenceErrorHandler methods
- @staticmethod
- def _error_message_for_bug(tool, status_id, script_error):
+ def command_failed(message, script_error, patch):
+ 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 = tool.status_server.results_url_for_status(status_id)
+ results_link = self._tool.status_server.results_url_for_status(status_id)
return "%s\nFull output: %s" % (script_error.message_with_output(), results_link)
- @classmethod
+ # StepSequenceErrorHandler methods
+
def handle_script_error(cls, tool, state, script_error):
- status_id = cls._update_status_for_script_error(tool, state, script_error)
- validator = CommitterValidator(tool.bugs)
- validator.reject_patch_from_commit_queue(state["patch"].id(), cls._error_message_for_bug(tool, status_id, script_error))
+ # Hitting this error handler should be pretty rare. It does occur,
+ # however, when a patch no longer applies to top-of-tree in the final
+ # land step.
+ log(script_error.message_with_output())
@classmethod
def handle_checkout_needs_update(cls, tool, state, options, error):
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
index 606a8a3..67ed054 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
@@ -195,11 +195,10 @@ class CommitQueueTest(QueuesTest):
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("commit-queue", 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": "",
"process_work_item": "MOCK: update_status: commit-queue Pass\n",
"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": "MOCK: update_status: commit-queue ScriptError error message\nMOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting patch 197 from commit-queue.' and additional comment 'ScriptError error message'\n",
+ "handle_script_error": "ScriptError error message\n",
}
self.assert_queue_outputs(CommitQueue(), expected_stderr=expected_stderr)
@@ -209,11 +208,15 @@ class CommitQueueTest(QueuesTest):
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("commit-queue", 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": "",
- "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build-and-test-attachment', '--force-clean', '--build', '--non-interactive', '--build-style=both', '--quiet', 197, '--test']\nMOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--non-interactive', '--ignore-builders', '--quiet', '--parent-command=commit-queue', 197]\nMOCK: update_status: commit-queue Pass\n",
+ "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
+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-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
+MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 197]
+MOCK: update_status: commit-queue Pass
+""",
"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": "MOCK: update_status: commit-queue ScriptError error message\nMOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting patch 197 from commit-queue.' and additional comment 'ScriptError error message'\n",
+ "handle_script_error": "ScriptError error message\n",
}
self.assert_queue_outputs(CommitQueue(), tool=tool, expected_stderr=expected_stderr)
@@ -224,23 +227,18 @@ class CommitQueueTest(QueuesTest):
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("commit-queue", 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": "",
- "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build-and-test-attachment', '--force-clean', '--build', '--non-interactive', '--build-style=both', '--quiet', 197]\nMOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--non-interactive', '--ignore-builders', '--quiet', '--parent-command=commit-queue', 197]\nMOCK: update_status: commit-queue Pass\n",
+ "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
+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-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
+MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 197]
+MOCK: update_status: commit-queue Pass
+""",
"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": "MOCK: update_status: commit-queue ScriptError error message\nMOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting patch 197 from commit-queue.' and additional comment 'ScriptError 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)
- 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_without_patch())
- 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)
-
def test_auto_retry(self):
queue = CommitQueue()
options = Mock()
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list