https://bugs.webkit.org/show_bug.cgi?id=79416
Reviewed by Eric Seidel.
We used to use this function to check whether the tree is red. Now, we
don't use external measures of whether to proceed with work items.
Instead, we analyze them with the idea in mind that the tree might be
red.
* Scripts/webkitpy/tool/bot/queueengine.py:
(QueueEngineDelegate.next_work_item):
(QueueEngine.run):
* Scripts/webkitpy/tool/bot/queueengine_unittest.py:
(LoggingDelegate):
(LoggingDelegate.next_work_item):
(RaisingDelegate.process_work_item):
(QueueEngineTest.test_terminating_error):
* Scripts/webkitpy/tool/commands/earlywarningsystem.py:
(AbstractEarlyWarningSystem.__init__):
* Scripts/webkitpy/tool/commands/queues.py:
(AbstractQueue.next_work_item):
(FeederQueue.next_work_item):
(CommitQueue.next_work_item):
(AbstractReviewQueue.next_work_item):
(StyleQueue.__init__):
* Scripts/webkitpy/tool/commands/queues_unittest.py:
(FeederQueueTest.test_feeder_queue):
(CommitQueueTest.test_commit_queue):
(test_commit_queue_failure):
(test_commit_queue_failure_with_failing_tests):
(test_rollout):
(test_rollout_lands):
(StyleQueueTest.test_style_queue_with_style_exception):
(test_style_queue_with_watch_list_exception):
* Scripts/webkitpy/tool/commands/queuestest.py:
(QueuesTest.assert_queue_outputs):
* Scripts/webkitpy/tool/commands/sheriffbot.py:
(SheriffBot.next_work_item):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@108737
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2012-02-23 Adam Barth <abarth@webkit.org>
+
+ should_proceed_with_work_item is unused and can be removed
+ https://bugs.webkit.org/show_bug.cgi?id=79416
+
+ Reviewed by Eric Seidel.
+
+ We used to use this function to check whether the tree is red. Now, we
+ don't use external measures of whether to proceed with work items.
+ Instead, we analyze them with the idea in mind that the tree might be
+ red.
+
+ * Scripts/webkitpy/tool/bot/queueengine.py:
+ (QueueEngineDelegate.next_work_item):
+ (QueueEngine.run):
+ * Scripts/webkitpy/tool/bot/queueengine_unittest.py:
+ (LoggingDelegate):
+ (LoggingDelegate.next_work_item):
+ (RaisingDelegate.process_work_item):
+ (QueueEngineTest.test_terminating_error):
+ * Scripts/webkitpy/tool/commands/earlywarningsystem.py:
+ (AbstractEarlyWarningSystem.__init__):
+ * Scripts/webkitpy/tool/commands/queues.py:
+ (AbstractQueue.next_work_item):
+ (FeederQueue.next_work_item):
+ (CommitQueue.next_work_item):
+ (AbstractReviewQueue.next_work_item):
+ (StyleQueue.__init__):
+ * Scripts/webkitpy/tool/commands/queues_unittest.py:
+ (FeederQueueTest.test_feeder_queue):
+ (CommitQueueTest.test_commit_queue):
+ (test_commit_queue_failure):
+ (test_commit_queue_failure_with_failing_tests):
+ (test_rollout):
+ (test_rollout_lands):
+ (StyleQueueTest.test_style_queue_with_style_exception):
+ (test_style_queue_with_watch_list_exception):
+ * Scripts/webkitpy/tool/commands/queuestest.py:
+ (QueuesTest.assert_queue_outputs):
+ * Scripts/webkitpy/tool/commands/sheriffbot.py:
+ (SheriffBot.next_work_item):
+
2012-02-23 Adrienne Walker <enne@google.com>
Unreviewed, add Stephen Chenney to committers.py as a contributor.
def next_work_item(self):
raise NotImplementedError, "subclasses must implement"
- def should_proceed_with_work_item(self, work_item):
- # returns (safe_to_proceed, waiting_message, patch)
- raise NotImplementedError, "subclasses must implement"
-
def process_work_item(self, work_item):
raise NotImplementedError, "subclasses must implement"
if not work_item:
self._sleep("No work item.")
continue
- if not self._delegate.should_proceed_with_work_item(work_item):
- self._sleep("Not proceeding with work item.")
- continue
# FIXME: Work logs should not depend on bug_id specificaly.
# This looks fixed, no?
'begin_work_queue',
'should_continue_work_queue',
'next_work_item',
- 'should_proceed_with_work_item',
'work_item_log_path',
'process_work_item',
'should_continue_work_queue',
self.record("next_work_item")
return "work_item"
- def should_proceed_with_work_item(self, work_item):
- self.record("should_proceed_with_work_item")
- self._test.assertEquals(work_item, "work_item")
- fake_patch = {'bug_id': 50000}
- return (True, "waiting_message", fake_patch)
-
def process_work_item(self, work_item):
self.record("process_work_item")
self._test.assertEquals(work_item, "work_item")
raise self._exception
-class NotSafeToProceedDelegate(LoggingDelegate):
- def should_proceed_with_work_item(self, work_item):
- self.record("should_proceed_with_work_item")
- self._test.assertEquals(work_item, "work_item")
- return False
-
-
class FastQueueEngine(QueueEngine):
def __init__(self, delegate):
QueueEngine.__init__(self, "fast-queue", delegate, threading.Event())
self._test_terminating_queue(KeyboardInterrupt(), "User terminated queue.")
self._test_terminating_queue(TerminateQueue(), "TerminateQueue exception received.")
- def test_not_safe_to_proceed(self):
- delegate = NotSafeToProceedDelegate(self)
- self._run_engine(delegate, engine=FastQueueEngine(delegate))
- expected_callbacks = LoggingDelegate.expected_callbacks[:]
- expected_callbacks.remove('work_item_log_path')
- expected_callbacks.remove('process_work_item')
- self.assertEquals(delegate._callbacks, expected_callbacks)
-
def test_now(self):
"""Make sure there are no typos in the QueueEngine.now() method."""
engine = QueueEngine("test", None, None)
AbstractReviewQueue.__init__(self, options=options)
self.port = DeprecatedPort.port(self.port_name)
- def should_proceed_with_work_item(self, patch):
- return True
-
def begin_work_queue(self):
# FIXME: This violates abstraction
self._tool._deprecated_port = self.port
def next_work_item(self):
raise NotImplementedError, "subclasses must implement"
- def should_proceed_with_work_item(self, work_item):
- raise NotImplementedError, "subclasses must implement"
-
def process_work_item(self, work_item):
raise NotImplementedError, "subclasses must implement"
# understands work items.
return "synthetic-work-item"
- def should_proceed_with_work_item(self, work_item):
- return True
-
def process_work_item(self, work_item):
for feeder in self.feeders:
feeder.feed()
def next_work_item(self):
return self._next_patch()
- def should_proceed_with_work_item(self, patch):
- patch_text = "rollout patch" if patch.is_rollout() else "patch"
- self._update_status("Processing %s" % patch_text, patch)
- return True
-
def process_work_item(self, patch):
self._cc_watchers(patch.bug_id())
task = CommitQueueTask(self, patch)
def next_work_item(self):
return self._next_patch()
- def should_proceed_with_work_item(self, patch):
- raise NotImplementedError("subclasses must implement")
-
def process_work_item(self, patch):
try:
if not self.review_patch(patch):
def __init__(self):
AbstractReviewQueue.__init__(self)
- def should_proceed_with_work_item(self, patch):
- return True
-
def review_patch(self, patch):
task = StyleQueueTask(self, patch)
try:
tool = MockTool(log_executive=True)
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("feeder-queue"),
- "should_proceed_with_work_item": "",
"next_work_item": "",
"process_work_item": """Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com)
Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com)
tool.filesystem.write_text_file('/mock-results/full_results.json', '') # Otherwise the commit-queue will hit a KeyError trying to read the results from the MockFileSystem.
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"),
- "should_proceed_with_work_item": "MOCK: update_status: commit-queue Processing patch\n",
"next_work_item": "",
"process_work_item": """MOCK: update_status: commit-queue Cleaned working directory
MOCK: update_status: commit-queue Updated working directory
def test_commit_queue_failure(self):
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"),
- "should_proceed_with_work_item": "MOCK: update_status: commit-queue Processing patch\n",
"next_work_item": "",
"process_work_item": """MOCK: update_status: commit-queue Cleaned working directory
MOCK: update_status: commit-queue Updated working directory
def test_commit_queue_failure_with_failing_tests(self):
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"),
- "should_proceed_with_work_item": "MOCK: update_status: commit-queue Processing patch\n",
"next_work_item": "",
"process_work_item": """MOCK: update_status: commit-queue Cleaned working directory
MOCK: update_status: commit-queue Updated working directory
tool.buildbot.light_tree_on_fire()
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"),
- "should_proceed_with_work_item": "MOCK: update_status: commit-queue Processing patch\n",
"next_work_item": "",
"process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean'], cwd=/mock-checkout
MOCK: update_status: commit-queue Cleaned working directory
assert(rollout_patch.is_rollout())
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"),
- "should_proceed_with_work_item": "MOCK: update_status: commit-queue Processing rollout patch\n",
"next_work_item": "",
"process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean'], cwd=/mock-checkout
MOCK: update_status: commit-queue Cleaned working directory
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("style-queue"),
"next_work_item": "",
- "should_proceed_with_work_item": "",
"process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean'], cwd=/mock-checkout
MOCK: update_status: style-queue Cleaned working directory
MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update'], cwd=/mock-checkout
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("style-queue"),
"next_work_item": "",
- "should_proceed_with_work_item": "",
"process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean'], cwd=/mock-checkout
MOCK: update_status: style-queue Cleaned working directory
MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update'], cwd=/mock-checkout
self.assert_outputs(queue.begin_work_queue, "begin_work_queue", [], expected_stdout, expected_stderr, expected_exceptions)
self.assert_outputs(queue.should_continue_work_queue, "should_continue_work_queue", [], expected_stdout, expected_stderr, expected_exceptions)
self.assert_outputs(queue.next_work_item, "next_work_item", [], expected_stdout, expected_stderr, expected_exceptions)
- self.assert_outputs(queue.should_proceed_with_work_item, "should_proceed_with_work_item", [work_item], expected_stdout, expected_stderr, expected_exceptions)
self.assert_outputs(queue.process_work_item, "process_work_item", [work_item], expected_stdout, expected_stderr, expected_exceptions)
self.assert_outputs(queue.handle_unexpected_error, "handle_unexpected_error", [work_item, "Mock error message"], expected_stdout, expected_stderr, expected_exceptions)
# Should we have a different function for testing StepSequenceErrorHandlers?
self._irc_bot.process_pending_messages()
return
- 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, failure_map):
return True