Title: [108737] trunk/Tools
Revision
108737
Author
[email protected]
Date
2012-02-23 23:52:51 -0800 (Thu, 23 Feb 2012)

Log Message

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):

Modified Paths

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
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to