Diff
Modified: trunk/Tools/ChangeLog (108736 => 108737)
--- trunk/Tools/ChangeLog 2012-02-24 07:49:53 UTC (rev 108736)
+++ trunk/Tools/ChangeLog 2012-02-24 07:52:51 UTC (rev 108737)
@@ -1,3 +1,45 @@
+2012-02-23 Adam Barth <[email protected]>
+
+ 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 <[email protected]>
Unreviewed, add Stephen Chenney to committers.py as a contributor.
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/queueengine.py (108736 => 108737)
--- trunk/Tools/Scripts/webkitpy/tool/bot/queueengine.py 2012-02-24 07:49:53 UTC (rev 108736)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/queueengine.py 2012-02-24 07:52:51 UTC (rev 108737)
@@ -58,10 +58,6 @@
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"
@@ -98,9 +94,6 @@
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?
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py (108736 => 108737)
--- trunk/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py 2012-02-24 07:49:53 UTC (rev 108736)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py 2012-02-24 07:52:51 UTC (rev 108737)
@@ -50,7 +50,6 @@
'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',
@@ -82,12 +81,6 @@
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")
@@ -112,13 +105,6 @@
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())
@@ -179,14 +165,6 @@
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)
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py (108736 => 108737)
--- trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py 2012-02-24 07:49:53 UTC (rev 108736)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py 2012-02-24 07:52:51 UTC (rev 108737)
@@ -50,9 +50,6 @@
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
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues.py (108736 => 108737)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2012-02-24 07:49:53 UTC (rev 108736)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2012-02-24 07:52:51 UTC (rev 108737)
@@ -128,9 +128,6 @@
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"
@@ -185,9 +182,6 @@
# 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()
@@ -275,11 +269,6 @@
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)
@@ -382,9 +371,6 @@
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):
@@ -416,9 +402,6 @@
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:
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py (108736 => 108737)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2012-02-24 07:49:53 UTC (rev 108736)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2012-02-24 07:52:51 UTC (rev 108737)
@@ -132,7 +132,6 @@
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 ([email protected])
Warning, attachment 10001 on bug 50000 has invalid committer ([email protected])
@@ -235,7 +234,6 @@
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
@@ -255,7 +253,6 @@
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
@@ -283,7 +280,6 @@
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
@@ -317,7 +313,6 @@
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
@@ -348,7 +343,6 @@
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
@@ -448,7 +442,6 @@
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
@@ -481,7 +474,6 @@
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
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queuestest.py (108736 => 108737)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queuestest.py 2012-02-24 07:49:53 UTC (rev 108736)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queuestest.py 2012-02-24 07:52:51 UTC (rev 108737)
@@ -91,7 +91,6 @@
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?
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/sheriffbot.py (108736 => 108737)
--- trunk/Tools/Scripts/webkitpy/tool/commands/sheriffbot.py 2012-02-24 07:49:53 UTC (rev 108736)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/sheriffbot.py 2012-02-24 07:52:51 UTC (rev 108737)
@@ -58,10 +58,6 @@
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