[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

eric at webkit.org eric at webkit.org
Wed Dec 22 13:27:12 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit 4a99af63879963359fa2c5eea33c82487c065910
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Thu Sep 16 00:04:16 2010 +0000

    2010-09-15  Eric Seidel  <eric at webkit.org>
    
            Reviewed by Adam Barth.
    
            Add queue start/stop messages
            https://bugs.webkit.org/show_bug.cgi?id=45853
    
            I ended up needing to clean up a bunch of our unit testing
            in order to test this new code path nicely.
    
            There are also a few PEP8 changes needed to pass check-webkit-style.
    
            * Scripts/webkitpy/tool/bot/queueengine.py:
            * Scripts/webkitpy/tool/bot/queueengine_unittest.py:
            * Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
            * Scripts/webkitpy/tool/commands/queues.py:
            * Scripts/webkitpy/tool/commands/queues_unittest.py:
            * Scripts/webkitpy/tool/commands/queuestest.py:
            * Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@67582 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 2393228..3c1d4eb 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,23 @@
+2010-09-15  Eric Seidel  <eric at webkit.org>
+
+        Reviewed by Adam Barth.
+
+        Add queue start/stop messages
+        https://bugs.webkit.org/show_bug.cgi?id=45853
+
+        I ended up needing to clean up a bunch of our unit testing
+        in order to test this new code path nicely.
+
+        There are also a few PEP8 changes needed to pass check-webkit-style.
+
+        * Scripts/webkitpy/tool/bot/queueengine.py:
+        * Scripts/webkitpy/tool/bot/queueengine_unittest.py:
+        * Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
+        * Scripts/webkitpy/tool/commands/queues.py:
+        * Scripts/webkitpy/tool/commands/queues_unittest.py:
+        * Scripts/webkitpy/tool/commands/queuestest.py:
+        * Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:
+
 2010-09-15  Simon Fraser  <simon.fraser at apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=45849
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py
index 36cbc5f..289dc4a 100644
--- a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py
@@ -33,7 +33,6 @@ import traceback
 
 from datetime import datetime, timedelta
 
-from webkitpy.common.net.statusserver import StatusServer
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.common.system.deprecated_logging import log, OutputTee
 
@@ -117,10 +116,10 @@ class QueueEngine:
                     message = "Unexpected failure when processing patch!  Please file a bug against webkit-patch.\n%s" % e.message_with_output()
                     self._delegate.handle_unexpected_error(work_item, message)
             except TerminateQueue, e:
-                log("\nTerminateQueue exception received.")
+                self._stopping("TerminateQueue exception received.")
                 return 0
             except KeyboardInterrupt, e:
-                log("\nUser terminated queue.")
+                self._stopping("User terminated queue.")
                 return 1
             except Exception, e:
                 traceback.print_exc()
@@ -129,6 +128,13 @@ class QueueEngine:
         # Never reached.
         self._ensure_work_log_closed()
 
+    def _stopping(self, message):
+        log("\n%s" % message)
+        self._delegate.stop_work_queue(message)
+        # Be careful to shut down our OutputTee or the unit tests will be unhappy.
+        self._ensure_work_log_closed()
+        self._output_tee.remove_log(self._queue_log)
+
     def _begin_logging(self):
         self._queue_log = self._output_tee.add_log(self._delegate.queue_log_path())
         self._work_log = None
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py
index ec91bdb..bfec401 100644
--- a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py
@@ -34,7 +34,9 @@ import threading
 import unittest
 
 from webkitpy.common.system.executive import ScriptError
-from webkitpy.tool.bot.queueengine import QueueEngine, QueueEngineDelegate
+from webkitpy.common.system.outputcapture import OutputCapture
+from webkitpy.tool.bot.queueengine import QueueEngine, QueueEngineDelegate, TerminateQueue
+
 
 class LoggingDelegate(QueueEngineDelegate):
     def __init__(self, test):
@@ -94,14 +96,19 @@ class LoggingDelegate(QueueEngineDelegate):
         self._test.assertEquals(work_item, "work_item")
 
 
-class ThrowErrorDelegate(LoggingDelegate):
-    def __init__(self, test, error_code):
+class RaisingDelegate(LoggingDelegate):
+    def __init__(self, test, exception):
         LoggingDelegate.__init__(self, test)
-        self.error_code = error_code
+        self._exception = exception
+        self.stop_message = None
 
     def process_work_item(self, work_item):
         self.record("process_work_item")
-        raise ScriptError(exit_code=self.error_code)
+        raise self._exception
+
+    def stop_work_queue(self, message):
+        self.record("stop_work_queue")
+        self.stop_message = message
 
 
 class NotSafeToProceedDelegate(LoggingDelegate):
@@ -132,7 +139,7 @@ class QueueEngineTest(unittest.TestCase):
         self.assertTrue(os.path.exists(os.path.join(self.temp_dir, "work_log_path", "work_item.log")))
 
     def test_unexpected_error(self):
-        delegate = ThrowErrorDelegate(self, 3)
+        delegate = RaisingDelegate(self, ScriptError(exit_code=3))
         work_queue = QueueEngine("error-queue", delegate, threading.Event())
         work_queue.run()
         expected_callbacks = LoggingDelegate.expected_callbacks[:]
@@ -143,11 +150,32 @@ class QueueEngineTest(unittest.TestCase):
         self.assertEquals(delegate._callbacks, expected_callbacks)
 
     def test_handled_error(self):
-        delegate = ThrowErrorDelegate(self, QueueEngine.handled_error_code)
+        delegate = RaisingDelegate(self, ScriptError(exit_code=QueueEngine.handled_error_code))
         work_queue = QueueEngine("handled-error-queue", delegate, threading.Event())
         work_queue.run()
         self.assertEquals(delegate._callbacks, LoggingDelegate.expected_callbacks)
 
+    def _test_terminating_queue(self, exception, expected_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.
+        expected_callbacks = LoggingDelegate.expected_callbacks[:work_item_index + 1]
+        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.assertEquals(delegate._callbacks, expected_callbacks)
+        self.assertEquals(delegate.stop_message, expected_message)
+
+    def test_terminating_error(self):
+        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)
         work_queue = FastQueueEngine(delegate)
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
index 1f04923..a011351 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
@@ -42,12 +42,11 @@ class EarlyWarningSytemTest(QueuesTest):
     def _default_expected_stderr(self, ews):
         string_replacemnts = {
             "name": ews.name,
-            "checkout_dir": os.getcwd(),  # FIXME: Use of os.getcwd() is wrong, should be scm.checkout_root
             "port": ews.port_name,
             "watchers": ews.watchers,
         }
         expected_stderr = {
-            "begin_work_queue": "CAUTION: %(name)s will discard all local changes in \"%(checkout_dir)s\"\nRunning WebKit %(name)s.\n" % string_replacemnts,
+            "begin_work_queue": self._default_begin_work_queue_stderr(ews.name, os.getcwd()),  # FIXME: Use of os.getcwd() is wrong, should be scm.checkout_root
             "handle_unexpected_error": "Mock error message\n",
             "next_work_item": "MOCK: update_work_items: %(name)s [103]\n" % string_replacemnts,
             "process_work_item": "MOCK: update_status: %(name)s Pass\n" % string_replacemnts,
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
index 1c64e82..8992407 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
@@ -95,6 +95,10 @@ class AbstractQueue(Command, QueueEngineDelegate):
             if (response != "yes"):
                 error("User declined.")
         log("Running WebKit %s." % self.name)
+        self.tool.status_server.update_status(self.name, "Starting Queue")
+
+    def stop_work_queue(self, reason):
+        self.tool.status_server.update_status(self.name, "Stopping Queue, reason: %s" % reason)
 
     def should_continue_work_queue(self):
         self._iteration_count += 1
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
index 1f0aefe..81808b6 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
@@ -161,17 +161,17 @@ class AlwaysCommitQueueTool(object):
 class CommitQueueTest(QueuesTest):
     def test_commit_queue(self):
         expected_stderr = {
-            "begin_work_queue" : "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root,
+            "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" : """Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
+            "next_work_item": """Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
 Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
 MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Rejecting patch 128 from commit-queue.' and additional comment 'non-committer at example.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.\n\n- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.\n\n- If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.'
 MOCK: update_work_items: commit-queue [106, 197]
 2 patches in commit-queue [106, 197]
 """,
-            "process_work_item" : "MOCK: update_status: commit-queue Pass\n",
-            "handle_unexpected_error" : "MOCK setting flag 'commit-queue' to '-' on attachment '1234' with comment 'Rejecting patch 1234 from commit-queue.' and additional comment 'Mock error message'\n",
+            "process_work_item": "MOCK: update_status: commit-queue Pass\n",
+            "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '1234' with comment 'Rejecting patch 1234 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 '1234' with comment 'Rejecting patch 1234 from commit-queue.' and additional comment 'ScriptError error message'\n",
         }
         self.assert_queue_outputs(CommitQueue(), expected_stderr=expected_stderr)
@@ -180,10 +180,10 @@ MOCK: update_work_items: commit-queue [106, 197]
         tool = MockTool(log_executive=True)
         tool.buildbot.light_tree_on_fire()
         expected_stderr = {
-            "begin_work_queue" : "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root,
+            "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" : """Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
+            "next_work_item": """Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
 Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
 MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Rejecting patch 128 from commit-queue.' and additional comment 'non-committer at example.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.
 
@@ -193,8 +193,8 @@ MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Reject
 MOCK: update_work_items: commit-queue [106, 197]
 2 patches in commit-queue [106, 197]
 """,
-            "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', 1234, '--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', 1234]\nMOCK: update_status: commit-queue Pass\n",
-            "handle_unexpected_error" : "MOCK setting flag 'commit-queue' to '-' on attachment '1234' with comment 'Rejecting patch 1234 from commit-queue.' and additional comment 'Mock error message'\n",
+            "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', 1234, '--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', 1234]\nMOCK: update_status: commit-queue Pass\n",
+            "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '1234' with comment 'Rejecting patch 1234 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 '1234' with comment 'Rejecting patch 1234 from commit-queue.' and additional comment 'ScriptError error message'\n",
         }
         self.assert_queue_outputs(CommitQueue(), tool=tool, expected_stderr=expected_stderr)
@@ -204,7 +204,7 @@ MOCK: update_work_items: commit-queue [106, 197]
         tool.buildbot.light_tree_on_fire()
         rollout_patch = MockPatch()
         expected_stderr = {
-            "begin_work_queue": "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root,
+            "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": """Warning, attachment 128 on bug 42 has invalid committer (non-committer at example.com)
@@ -270,7 +270,7 @@ MOCK: update_work_items: commit-queue [106, 197]
 class RietveldUploadQueueTest(QueuesTest):
     def test_rietveld_upload_queue(self):
         expected_stderr = {
-            "begin_work_queue": "CAUTION: rietveld-upload-queue will discard all local changes in \"%s\"\nRunning WebKit rietveld-upload-queue.\n" % MockSCM.fake_checkout_root,
+            "begin_work_queue": self._default_begin_work_queue_stderr("rietveld-upload-queue", MockSCM.fake_checkout_root),
             "should_proceed_with_work_item": "MOCK: update_status: rietveld-upload-queue Uploading patch\n",
             "process_work_item": "MOCK: update_status: rietveld-upload-queue Pass\n",
             "handle_unexpected_error": "Mock error message\nMOCK setting flag 'in-rietveld' to '-' on attachment '1234' with comment 'None' and additional comment 'None'\n",
@@ -282,11 +282,11 @@ class RietveldUploadQueueTest(QueuesTest):
 class StyleQueueTest(QueuesTest):
     def test_style_queue(self):
         expected_stderr = {
-            "begin_work_queue" : "CAUTION: style-queue will discard all local changes in \"%s\"\nRunning WebKit style-queue.\n" % MockSCM.fake_checkout_root,
+            "begin_work_queue": self._default_begin_work_queue_stderr("style-queue", MockSCM.fake_checkout_root),
             "next_work_item": "MOCK: update_work_items: style-queue [103]\n",
             "should_proceed_with_work_item": "MOCK: update_status: style-queue Checking style\n",
-            "process_work_item" : "MOCK: update_status: style-queue Pass\n",
-            "handle_unexpected_error" : "Mock error message\n",
+            "process_work_item": "MOCK: update_status: style-queue Pass\n",
+            "handle_unexpected_error": "Mock error message\n",
             "handle_script_error": "MOCK: update_status: style-queue ScriptError error message\nMOCK bug comment: bug_id=345, cc=[]\n--- Begin comment ---\\Attachment 1234 did not pass style-queue:\n\nScriptError error message\n\nIf any of these errors are false positives, please file a bug against check-webkit-style.\n--- End comment ---\n\n",
         }
         expected_exceptions = {
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py
index 9e17c5c..df73ec3 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py
@@ -71,6 +71,10 @@ class QueuesTest(unittest.TestCase):
                 expected_stderr=expected_stderr.get(func_name, ""),
                 expected_exception=exception)
 
+    def _default_begin_work_queue_stderr(self, name, checkout_dir):
+        string_replacements = {"name": name, 'checkout_dir': checkout_dir}
+        return "CAUTION: %(name)s will discard all local changes in \"%(checkout_dir)s\"\nRunning WebKit %(name)s.\nMOCK: update_status: %(name)s Starting Queue\n" % string_replacements
+
     def assert_queue_outputs(self, queue, args=None, work_item=None, expected_stdout=None, expected_stderr=None, expected_exceptions=None, options=Mock(), tool=MockTool()):
         if not expected_stdout:
             expected_stdout = {}
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
index 4b4b8b6..a63ec24 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
@@ -42,7 +42,7 @@ class SheriffBotTest(QueuesTest):
             29837: [self.builder1],
         }
         expected_stderr = {
-            "begin_work_queue": "CAUTION: sheriff-bot will discard all local changes in \"%s\"\nRunning WebKit sheriff-bot.\n" % os.getcwd(),
+            "begin_work_queue": self._default_begin_work_queue_stderr("sheriff-bot", os.getcwd()),
             "next_work_item": "",
             "process_work_item": "MOCK: irc.post: abarth, darin, eseidel: http://trac.webkit.org/changeset/29837 might have broken Builder1\nMOCK bug comment: bug_id=42, cc=['abarth at webkit.org', 'eric at webkit.org']\n--- Begin comment ---\\http://trac.webkit.org/changeset/29837 might have broken Builder1\n--- End comment ---\n\n",
             "handle_unexpected_error": "Mock error message\n"

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list