diff options
Diffstat (limited to 'WebKitTools/Scripts/webkitpy/tool/bot')
9 files changed, 157 insertions, 274 deletions
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py index a347972..02e203c 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py @@ -27,19 +27,39 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. from webkitpy.common.system.executive import ScriptError +from webkitpy.common.net.layouttestresults import LayoutTestResults + + +class CommitQueueTaskDelegate(object): + def run_command(self, command): + raise NotImplementedError("subclasses must implement") + + def command_passed(self, message, patch): + raise NotImplementedError("subclasses must implement") + + def command_failed(self, message, script_error, patch): + raise NotImplementedError("subclasses must implement") + + def refetch_patch(self, patch): + raise NotImplementedError("subclasses must implement") + + def layout_test_results(self): + raise NotImplementedError("subclasses must implement") + + def report_flaky_tests(self, patch, flaky_tests): + raise NotImplementedError("subclasses must implement") class CommitQueueTask(object): - def __init__(self, tool, commit_queue, patch): - self._tool = tool - self._commit_queue = commit_queue + def __init__(self, delegate, patch): + self._delegate = delegate 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()) + self._patch = self._delegate.refetch_patch(self._patch) if self._patch.is_obsolete(): return False if self._patch.bug().is_closed(): @@ -52,12 +72,12 @@ class CommitQueueTask(object): def _run_command(self, command, success_message, failure_message): try: - self._commit_queue.run_webkit_patch(command) - self._commit_queue.command_passed(success_message, patch=self._patch) + self._delegate.run_command(command) + self._delegate.command_passed(success_message, patch=self._patch) 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) + self.failure_status_id = self._delegate.command_failed(failure_message, script_error=self._script_error, patch=self._patch) return False def _apply(self): @@ -76,7 +96,6 @@ class CommitQueueTask(object): "build", "--no-clean", "--no-update", - "--build", "--build-style=both", "--quiet", ], @@ -88,7 +107,6 @@ class CommitQueueTask(object): "build", "--force-clean", "--no-update", - "--build", "--build-style=both", "--quiet", ], @@ -121,6 +139,12 @@ class CommitQueueTask(object): "Able to pass tests without patch", "Unable to pass tests without patch (tree is red?)") + def _failing_tests_from_last_run(self): + results = self._delegate.layout_test_results() + if not results: + return None + return results.failing_tests() + def _land(self): return self._run_command([ "land-attachment", @@ -134,6 +158,29 @@ class CommitQueueTask(object): "Landed patch", "Unable to land patch") + def _report_flaky_tests(self, flaky_tests): + self._delegate.report_flaky_tests(self._patch, flaky_tests) + + def _test_patch(self): + if self._patch.is_rollout(): + return True + if self._test(): + return True + + first_failing_tests = self._failing_tests_from_last_run() + if self._test(): + self._report_flaky_tests(first_failing_tests) + return True + + second_failing_tests = self._failing_tests_from_last_run() + if first_failing_tests != second_failing_tests: + self._report_flaky_tests(first_failing_tests + second_failing_tests) + return False + + if self._build_and_test_without_patch(): + raise self._script_error # The error from the previous ._test() run is real, report it. + return False # Tree must be red, just retry later. + def run(self): if not self._validate(): return False @@ -143,12 +190,8 @@ class CommitQueueTask(object): 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 + if not self._test_patch(): + return False # 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(): diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py index 8b46146..6fa0667 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py @@ -36,11 +36,11 @@ from webkitpy.tool.bot.commitqueuetask import * from webkitpy.tool.mocktool import MockTool -class MockCommitQueue: +class MockCommitQueue(CommitQueueTaskDelegate): def __init__(self, error_plan): self._error_plan = error_plan - def run_webkit_patch(self, command): + def run_command(self, command): log("run_webkit_patch: %s" % command) if self._error_plan: error = self._error_plan.pop(0) @@ -56,19 +56,28 @@ class MockCommitQueue: failure_message, script_error, patch.id())) return 3947 + def refetch_patch(self, patch): + return patch + + def layout_test_results(self): + return None + + def report_flaky_tests(self, patch, flaky_tests): + log("report_flaky_tests: patch='%s' flaky_tests='%s'" % (patch.id(), flaky_tests)) + 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) + task = CommitQueueTask(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] command_passed: success_message='Applied patch' patch='197' -run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet'] +run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both', '--quiet'] command_passed: success_message='Built patch' patch='197' run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive'] command_passed: success_message='Passed tests' patch='197' @@ -93,9 +102,9 @@ command_failed: failure_message='Patch does not apply' script_error='MOCK apply ]) expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197] command_passed: success_message='Applied patch' patch='197' -run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet'] +run_webkit_patch: ['build', '--no-clean', '--no-update', '--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'] +run_webkit_patch: ['build', '--force-clean', '--no-update', '--build-style=both', '--quiet'] command_passed: success_message='Able to build without patch' patch='197' """ self._run_through_task(commit_queue, expected_stderr, ScriptError) @@ -108,9 +117,9 @@ command_passed: success_message='Able to build without patch' patch='197' ]) expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197] command_passed: success_message='Applied patch' patch='197' -run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet'] +run_webkit_patch: ['build', '--no-clean', '--no-update', '--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'] +run_webkit_patch: ['build', '--force-clean', '--no-update', '--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) @@ -123,12 +132,13 @@ command_failed: failure_message='Unable to build without patch' script_error='MO ]) expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197] command_passed: success_message='Applied patch' patch='197' -run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet'] +run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both', '--quiet'] command_passed: success_message='Built patch' 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 tests failure' patch='197' run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive'] command_passed: success_message='Passed tests' patch='197' +report_flaky_tests: patch='197' flaky_tests='None' run_webkit_patch: ['land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 197] command_passed: success_message='Landed patch' patch='197' """ @@ -143,7 +153,7 @@ command_passed: success_message='Landed patch' patch='197' ]) expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197] command_passed: success_message='Applied patch' patch='197' -run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet'] +run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both', '--quiet'] command_passed: success_message='Built patch' 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' patch='197' @@ -164,7 +174,7 @@ command_passed: success_message='Able to pass tests without patch' patch='197' ]) expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197] command_passed: success_message='Applied patch' patch='197' -run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet'] +run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both', '--quiet'] command_passed: success_message='Built patch' 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' patch='197' @@ -184,7 +194,7 @@ command_failed: failure_message='Unable to pass tests without patch (tree is red ]) expected_stderr = """run_webkit_patch: ['apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197] command_passed: success_message='Applied patch' patch='197' -run_webkit_patch: ['build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet'] +run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both', '--quiet'] command_passed: success_message='Built patch' patch='197' run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive'] command_passed: success_message='Passed tests' patch='197' diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/feeders.py b/WebKitTools/Scripts/webkitpy/tool/bot/feeders.py index 15eaaf3..dc892a4 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/feeders.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/feeders.py @@ -28,18 +28,15 @@ from webkitpy.common.system.deprecated_logging import log from webkitpy.common.net.bugzilla import CommitterValidator +from webkitpy.tool.grammar import pluralize class AbstractFeeder(object): def __init__(self, tool): self._tool = tool - def feed(tool): - raise NotImplementedError, "subclasses must implement" - - def update_work_items(self, item_ids): - self._tool.status_server.update_work_items(self.queue_name, item_ids) - log("Feeding %s items %s" % (self.queue_name, item_ids)) + def feed(self): + raise NotImplementedError("subclasses must implement") class CommitQueueFeeder(AbstractFeeder): @@ -49,11 +46,17 @@ class CommitQueueFeeder(AbstractFeeder): AbstractFeeder.__init__(self, tool) self.committer_validator = CommitterValidator(self._tool.bugs) + def _update_work_items(self, item_ids): + # FIXME: This is the last use of update_work_items, the commit-queue + # should move to feeding patches one at a time like the EWS does. + self._tool.status_server.update_work_items(self.queue_name, item_ids) + log("Feeding %s items %s" % (self.queue_name, item_ids)) + def feed(self): patches = self._validate_patches() patches = sorted(patches, self._patch_cmp) patch_ids = [patch.id() for patch in patches] - self.update_work_items(patch_ids) + self._update_work_items(patch_ids) def _patches_for_bug(self, bug_id): return self._tool.bugs.fetch_bug(bug_id).commit_queued_patches(include_invalid=True) @@ -71,3 +74,17 @@ class CommitQueueFeeder(AbstractFeeder): if rollout_cmp != 0: return rollout_cmp return cmp(a.attach_date(), b.attach_date()) + + +class EWSFeeder(AbstractFeeder): + def __init__(self, tool): + self._ids_sent_to_server = set() + AbstractFeeder.__init__(self, tool) + + def feed(self): + ids_needing_review = set(self._tool.bugs.queries.fetch_attachment_ids_from_review_queue()) + new_ids = ids_needing_review.difference(self._ids_sent_to_server) + log("Feeding EWS (%s, %s new)" % (pluralize("r? patch", len(ids_needing_review)), len(new_ids))) + for attachment_id in new_ids: # Order doesn't really matter for the EWS. + self._tool.status_server.submit_to_ews(attachment_id) + self._ids_sent_to_server.add(attachment_id) diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/patchcollection.py b/WebKitTools/Scripts/webkitpy/tool/bot/patchcollection.py deleted file mode 100644 index 6100cf8..0000000 --- a/WebKitTools/Scripts/webkitpy/tool/bot/patchcollection.py +++ /dev/null @@ -1,78 +0,0 @@ -# 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. - - -class PersistentPatchCollectionDelegate: - def collection_name(self): - raise NotImplementedError, "subclasses must implement" - - def fetch_potential_patch_ids(self): - raise NotImplementedError, "subclasses must implement" - - def status_server(self): - raise NotImplementedError, "subclasses must implement" - - def is_terminal_status(self, status): - raise NotImplementedError, "subclasses must implement" - - -class PersistentPatchCollection: - def __init__(self, delegate): - self._delegate = delegate - self._name = self._delegate.collection_name() - self._status = self._delegate.status_server() - self._status_cache = {} - - def _cached_status(self, patch_id): - cached = self._status_cache.get(patch_id) - if cached: - return cached - status = self._status.patch_status(self._name, patch_id) - if status and self._delegate.is_terminal_status(status): - self._status_cache[patch_id] = status - return status - - def _is_active_patch_id(self, patch_id): - """Active patches are patches waiting to be processed from this collection.""" - status = self._cached_status(patch_id) - return not status or not self._delegate.is_terminal_status(status) - - def _fetch_active_patch_ids(self): - patch_ids = self._delegate.fetch_potential_patch_ids() - return filter(lambda patch_id: self._is_active_patch_id(patch_id), patch_ids) - - def next(self): - # Note: We only fetch all the ids so we can post them back to the server. - # This will go away once we have a feeder queue and all other queues are - # just pulling their next work item from the server. - patch_ids = self._fetch_active_patch_ids() - # FIXME: We're assuming self._name is a valid queue-name. - self._status.update_work_items(self._name, patch_ids) - if not patch_ids: - return None - return patch_ids[0] diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/patchcollection_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/patchcollection_unittest.py deleted file mode 100644 index 4ec6e25..0000000 --- a/WebKitTools/Scripts/webkitpy/tool/bot/patchcollection_unittest.py +++ /dev/null @@ -1,52 +0,0 @@ -# Copyright (c) 2009 Google Inc. All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are -# met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above -# copyright notice, this list of conditions and the following disclaimer -# in the documentation and/or other materials provided with the -# distribution. -# * Neither the name of Google Inc. nor the names of its -# contributors may be used to endorse or promote products derived from -# this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -import unittest - -from webkitpy.tool.bot.patchcollection import PersistentPatchCollection, PersistentPatchCollectionDelegate -from webkitpy.thirdparty.mock import Mock - - -class TestPersistentPatchCollectionDelegate(PersistentPatchCollectionDelegate): - def collection_name(self): - return "test-collection" - - def fetch_potential_patch_ids(self): - return [42, 192, 87] - - def status_server(self): - return Mock() - - def is_terminal_status(self, status): - return False - - -class PersistentPatchCollectionTest(unittest.TestCase): - def test_next(self): - collection = PersistentPatchCollection(TestPersistentPatchCollectionDelegate()) - collection.next() diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py index 8118653..8b016e8 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py @@ -125,8 +125,8 @@ class QueueEngine: traceback.print_exc() # Don't try tell the status bot, in case telling it causes an exception. self._sleep("Exception while preparing queue") - # Never reached. - self._ensure_work_log_closed() + self._stopping("Delegate terminated queue.") + return 0 def _stopping(self, message): log("\n%s" % message) diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py index bfec401..37d8502 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py @@ -43,6 +43,7 @@ class LoggingDelegate(QueueEngineDelegate): self._test = test self._callbacks = [] self._run_before = False + self.stop_message = None expected_callbacks = [ 'queue_log_path', @@ -52,7 +53,8 @@ class LoggingDelegate(QueueEngineDelegate): 'should_proceed_with_work_item', 'work_item_log_path', 'process_work_item', - 'should_continue_work_queue' + 'should_continue_work_queue', + 'stop_work_queue', ] def record(self, method_name): @@ -95,21 +97,20 @@ class LoggingDelegate(QueueEngineDelegate): self.record("handle_unexpected_error") self._test.assertEquals(work_item, "work_item") + def stop_work_queue(self, message): + self.record("stop_work_queue") + self.stop_message = message + class RaisingDelegate(LoggingDelegate): def __init__(self, test, exception): LoggingDelegate.__init__(self, test) self._exception = exception - self.stop_message = None def process_work_item(self, work_item): self.record("process_work_item") raise self._exception - def stop_work_queue(self, message): - self.record("stop_work_queue") - self.stop_message = message - class NotSafeToProceedDelegate(LoggingDelegate): def should_proceed_with_work_item(self, work_item): @@ -132,16 +133,15 @@ class FastQueueEngine(QueueEngine): class QueueEngineTest(unittest.TestCase): def test_trivial(self): delegate = LoggingDelegate(self) - work_queue = QueueEngine("trivial-queue", delegate, threading.Event()) - work_queue.run() + self._run_engine(delegate) + self.assertEquals(delegate.stop_message, "Delegate terminated queue.") self.assertEquals(delegate._callbacks, LoggingDelegate.expected_callbacks) self.assertTrue(os.path.exists(os.path.join(self.temp_dir, "queue_log_path"))) self.assertTrue(os.path.exists(os.path.join(self.temp_dir, "work_log_path", "work_item.log"))) def test_unexpected_error(self): delegate = RaisingDelegate(self, ScriptError(exit_code=3)) - work_queue = QueueEngine("error-queue", delegate, threading.Event()) - work_queue.run() + self._run_engine(delegate) expected_callbacks = LoggingDelegate.expected_callbacks[:] work_item_index = expected_callbacks.index('process_work_item') # The unexpected error should be handled right after process_work_item starts @@ -151,11 +151,18 @@ class QueueEngineTest(unittest.TestCase): def test_handled_error(self): delegate = RaisingDelegate(self, ScriptError(exit_code=QueueEngine.handled_error_code)) - work_queue = QueueEngine("handled-error-queue", delegate, threading.Event()) - work_queue.run() + self._run_engine(delegate) self.assertEquals(delegate._callbacks, LoggingDelegate.expected_callbacks) - def _test_terminating_queue(self, exception, expected_message): + def _run_engine(self, delegate, engine=None, termination_message=None): + if not engine: + engine = QueueEngine("test-queue", delegate, threading.Event()) + if not termination_message: + termination_message = "Delegate terminated queue." + expected_stderr = "\n%s\n" % termination_message + OutputCapture().assert_outputs(self, engine.run, [], expected_stderr=expected_stderr) + + def _test_terminating_queue(self, exception, termination_message): work_item_index = LoggingDelegate.expected_callbacks.index('process_work_item') # The terminating error should be handled right after process_work_item. # There should be no other callbacks after stop_work_queue. @@ -163,14 +170,10 @@ class QueueEngineTest(unittest.TestCase): expected_callbacks.append("stop_work_queue") delegate = RaisingDelegate(self, exception) - work_queue = QueueEngine("terminating-queue", delegate, threading.Event()) - - output = OutputCapture() - expected_stderr = "\n%s\n" % expected_message - output.assert_outputs(self, work_queue.run, [], expected_stderr=expected_stderr) + self._run_engine(delegate, termination_message=termination_message) self.assertEquals(delegate._callbacks, expected_callbacks) - self.assertEquals(delegate.stop_message, expected_message) + self.assertEquals(delegate.stop_message, termination_message) def test_terminating_error(self): self._test_terminating_queue(KeyboardInterrupt(), "User terminated queue.") @@ -178,15 +181,10 @@ class QueueEngineTest(unittest.TestCase): def test_not_safe_to_proceed(self): delegate = NotSafeToProceedDelegate(self) - work_queue = FastQueueEngine(delegate) - work_queue.run() + self._run_engine(delegate, engine=FastQueueEngine(delegate)) expected_callbacks = LoggingDelegate.expected_callbacks[:] - next_work_item_index = expected_callbacks.index('next_work_item') - # We slice out the common part of the expected callbacks. - # We add 2 here to include should_proceed_with_work_item, which is - # a pain to search for directly because it occurs twice. - expected_callbacks = expected_callbacks[:next_work_item_index + 2] - expected_callbacks.append('should_continue_work_queue') + expected_callbacks.remove('work_item_log_path') + expected_callbacks.remove('process_work_item') self.assertEquals(delegate._callbacks, expected_callbacks) def test_now(self): diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py b/WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py index a38c3cf..da506bc 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py @@ -77,55 +77,15 @@ class Sheriff(object): ]) return parse_bug_id(output) - def _rollout_reason(self, builders): - # FIXME: This should explain which layout tests failed - # however, that would require Build objects here, either passed - # in through failure_info, or through Builder.latest_build. - names = [builder.name() for builder in builders] - return "Caused builders %s to fail." % join_with_separators(names) - - def post_automatic_rollout_patch(self, commit_info, builders): - # For now we're only posting rollout patches for commit-queue patches. - commit_bot_email = "eseidel@chromium.org" - if commit_bot_email == commit_info.committer_email(): - try: - self.post_rollout_patch(commit_info.revision(), - self._rollout_reason(builders)) - except ScriptError, e: - log("Failed to create-rollout.") - - def post_blame_comment_on_bug(self, commit_info, builders, blame_list): + def post_blame_comment_on_bug(self, commit_info, builders, tests): if not commit_info.bug_id(): return comment = "%s might have broken %s" % ( view_source_url(commit_info.revision()), join_with_separators([builder.name() for builder in builders])) - if len(blame_list) > 1: - comment += "\nThe following changes are on the blame list:\n" - comment += "\n".join(map(view_source_url, blame_list)) + if tests: + comment += "\nThe following tests are not passing:\n" + comment += "\n".join(tests) self._tool.bugs.post_comment_to_bug(commit_info.bug_id(), comment, cc=self._sheriffbot.watchers) - - # FIXME: Should some of this logic be on BuildBot? - def provoke_flaky_builders(self, revisions_causing_failures): - # We force_build builders that are red but have not "failed" (i.e., - # been red twice). We do this to avoid a deadlock situation where a - # flaky test blocks the commit-queue and there aren't any other - # patches being landed to re-spin the builder. - failed_builders = sum([revisions_causing_failures[key] for - key in revisions_causing_failures.keys()], []) - failed_builder_names = \ - set([builder.name() for builder in failed_builders]) - idle_red_builder_names = \ - set([builder["name"] - for builder in self._tool.buildbot.idle_red_core_builders()]) - - # We only want to provoke these builders if they are idle and have not - # yet "failed" (i.e., been red twice) to avoid overloading the bots. - flaky_builder_names = idle_red_builder_names - failed_builder_names - - for name in flaky_builder_names: - flaky_builder = self._tool.buildbot.builder_with_name(name) - flaky_builder.force_build(username=self._sheriffbot.name, - comments="Probe for flakiness.") diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/sheriff_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/sheriff_unittest.py index c375ff9..690af1f 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/sheriff_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/sheriff_unittest.py @@ -47,15 +47,6 @@ class MockSheriffBot(object): class SheriffTest(unittest.TestCase): - def test_rollout_reason(self): - sheriff = Sheriff(MockTool(), MockSheriffBot()) - builders = [ - Builder("Foo", None), - Builder("Bar", None), - ] - reason = "Caused builders Foo and Bar to fail." - self.assertEquals(sheriff._rollout_reason(builders), reason) - def test_post_blame_comment_on_bug(self): def run(): sheriff = Sheriff(MockTool(), MockSheriffBot()) @@ -68,38 +59,32 @@ class SheriffTest(unittest.TestCase): commit_info.revision = lambda: 4321 # Should do nothing with no bug_id sheriff.post_blame_comment_on_bug(commit_info, builders, []) - sheriff.post_blame_comment_on_bug(commit_info, builders, [2468, 5646]) + sheriff.post_blame_comment_on_bug(commit_info, builders, ["mock-test-1", "mock-test-2"]) # Should try to post a comment to the bug, but MockTool.bugs does nothing. commit_info.bug_id = lambda: 1234 sheriff.post_blame_comment_on_bug(commit_info, builders, []) - sheriff.post_blame_comment_on_bug(commit_info, builders, [3432]) - sheriff.post_blame_comment_on_bug(commit_info, builders, [841, 5646]) + sheriff.post_blame_comment_on_bug(commit_info, builders, ["mock-test-1"]) + sheriff.post_blame_comment_on_bug(commit_info, builders, ["mock-test-1", "mock-test-2"]) - expected_stderr = u"MOCK bug comment: bug_id=1234, cc=['watcher@example.com']\n--- Begin comment ---\\http://trac.webkit.org/changeset/4321 might have broken Foo and Bar\n--- End comment ---\n\nMOCK bug comment: bug_id=1234, cc=['watcher@example.com']\n--- Begin comment ---\\http://trac.webkit.org/changeset/4321 might have broken Foo and Bar\n--- End comment ---\n\nMOCK bug comment: bug_id=1234, cc=['watcher@example.com']\n--- Begin comment ---\\http://trac.webkit.org/changeset/4321 might have broken Foo and Bar\nThe following changes are on the blame list:\nhttp://trac.webkit.org/changeset/841\nhttp://trac.webkit.org/changeset/5646\n--- End comment ---\n\n" - OutputCapture().assert_outputs(self, run, expected_stderr=expected_stderr) + expected_stderr = u"""MOCK bug comment: bug_id=1234, cc=['watcher@example.com'] +--- Begin comment --- +http://trac.webkit.org/changeset/4321 might have broken Foo and Bar +--- End comment --- - def test_provoke_flaky_builders(self): - def run(): - tool = MockTool() - tool.buildbot.light_tree_on_fire() - sheriff = Sheriff(tool, MockSheriffBot()) - revisions_causing_failures = {} - sheriff.provoke_flaky_builders(revisions_causing_failures) - expected_stderr = "MOCK: force_build: name=Builder2, username=mock-sheriff-bot, comments=Probe for flakiness.\n" - OutputCapture().assert_outputs(self, run, expected_stderr=expected_stderr) +MOCK bug comment: bug_id=1234, cc=['watcher@example.com'] +--- Begin comment --- +http://trac.webkit.org/changeset/4321 might have broken Foo and Bar +The following tests are not passing: +mock-test-1 +--- End comment --- - def test_post_blame_comment_on_bug(self): - sheriff = Sheriff(MockTool(), MockSheriffBot()) - builders = [ - Builder("Foo", None), - Builder("Bar", None), - ] - commit_info = Mock() - commit_info.bug_id = lambda: None - commit_info.revision = lambda: 4321 - commit_info.committer = lambda: None - commit_info.committer_email = lambda: "foo@example.com" - commit_info.reviewer = lambda: None - commit_info.author = lambda: None - sheriff.post_automatic_rollout_patch(commit_info, builders) +MOCK bug comment: bug_id=1234, cc=['watcher@example.com'] +--- Begin comment --- +http://trac.webkit.org/changeset/4321 might have broken Foo and Bar +The following tests are not passing: +mock-test-1 +mock-test-2 +--- End comment --- +""" + OutputCapture().assert_outputs(self, run, expected_stderr=expected_stderr) |