Diff
Modified: trunk/Tools/ChangeLog (204288 => 204289)
--- trunk/Tools/ChangeLog 2016-08-09 18:52:11 UTC (rev 204288)
+++ trunk/Tools/ChangeLog 2016-08-09 19:30:16 UTC (rev 204289)
@@ -1,3 +1,44 @@
+2016-08-09 Aakash Jain <aakash_j...@apple.com>
+
+ EWS logging should ensure the logging to file is stopped on queue termination
+ https://bugs.webkit.org/show_bug.cgi?id=160698
+ rdar://problem/24464570
+
+ Reviewed by Daniel Bates.
+
+ * Scripts/webkitpy/tool/bot/queueengine.py:
+ (QueueEngine._stopping): Stop logging to file on queue termination.
+ (QueueEngine._begin_logging): Configure the Python logger to log to file.
+ * Scripts/webkitpy/common/system/logutils.py:
+ (configure_logger_to_log_to_file): Return the handler so as to enable caller to remove it later.
+ * Scripts/webkitpy/tool/bot/queueengine_unittest.py:
+ (QueueEngineTest._run_engine): Removed extra newline character to improve log readability.
+ * Scripts/webkitpy/tool/commands/queues.py:
+ (AbstractQueue._log_directory): Reverting to os.path.join as we don't have host object.
+ (AbstractQueue.queue_log_path): Same.
+ (AbstractQueue.begin_work_queue): Removed logging initialization, it is now being done in QueueEngine.
+ (AbstractQueue.__init__): Removed host parameter, not required anymore, it was required by logging initialization
+ which moved to QueueEngine now.
+ (PatchProcessingQueue.__init__): Same.
+ (CommitQueue.__init__): Same.
+ (AbstractReviewQueue.__init__): Same.
+ (StyleQueue.__init__): Same.
+ * Scripts/webkitpy/tool/commands/queues_unittest.py:
+ (TestCommitQueue): Removed host parameter.
+ (TestCommitQueue.__init__): Same.
+ (AbstractPatchQueueTest.test_next_patch): Same.
+ (PatchProcessingQueueTest.test_upload_results_archive_for_patch): Same.
+ (test_commit_queue_failure): Same.
+ (mock_run_webkit_patch):
+ (MockCommitQueueTask.results_from_patch_test_run): Same.
+ (test_rollout_lands): Same.
+ (test_non_valid_patch): Same.
+ (test_auto_retry): Same.
+ (test_style_queue_with_watch_list_exception): Same.
+ (TestQueue.__init__): Deleted.
+ (TestReviewQueue.__init__): Deleted.
+ (TestFeederQueue.__init__): Deleted.
+
2016-08-09 Konstantin Tokarev <annu...@yandex.ru>
webkit-gtk tarball fails to build due to missing files
Modified: trunk/Tools/Scripts/webkitpy/common/system/logutils.py (204288 => 204289)
--- trunk/Tools/Scripts/webkitpy/common/system/logutils.py 2016-08-09 18:52:11 UTC (rev 204288)
+++ trunk/Tools/Scripts/webkitpy/common/system/logutils.py 2016-08-09 19:30:16 UTC (rev 204289)
@@ -223,6 +223,7 @@
handler.setFormatter(formatter)
logger.addHandler(handler)
+ return handler
class FileSystemHandler(FileHandler):
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/queueengine.py (204288 => 204289)
--- trunk/Tools/Scripts/webkitpy/tool/bot/queueengine.py 2016-08-09 18:52:11 UTC (rev 204288)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/queueengine.py 2016-08-09 19:30:16 UTC (rev 204289)
@@ -33,6 +33,8 @@
from datetime import datetime, timedelta
+from webkitpy.common.host import Host
+from webkitpy.common.system import logutils
from webkitpy.common.system.executive import ScriptError
from webkitpy.common.system.outputtee import OutputTee
@@ -124,14 +126,18 @@
return 0
def _stopping(self, message):
- _log.info("\n%s" % message)
+ _log.info(message)
self._delegate.stop_work_queue(message)
+ logging.getLogger("webkitpy").removeHandler(self._log_handler)
# 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())
+ _queue_log_path = self._delegate.queue_log_path()
+ # We are using logging.getLogger("webkitpy") instead of _log since we want to capture all messages logged from webkitpy modules.
+ self._log_handler = logutils.configure_logger_to_log_to_file(logging.getLogger("webkitpy"), _queue_log_path, Host().filesystem)
+ self._queue_log = self._output_tee.add_log(_queue_log_path)
self._work_log = None
def _open_work_log(self, work_item):
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py (204288 => 204289)
--- trunk/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py 2016-08-09 18:52:11 UTC (rev 204288)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py 2016-08-09 19:30:16 UTC (rev 204289)
@@ -146,7 +146,7 @@
engine = QueueEngine("test-queue", delegate, threading.Event())
if not termination_message:
termination_message = "Delegate terminated queue."
- expected_logs = "\n%s\n" % termination_message
+ expected_logs = "%s\n" % termination_message
OutputCapture().assert_outputs(self, engine.run, expected_logs=expected_logs)
def _test_terminating_queue(self, exception, termination_message):
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues.py (204288 => 204289)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2016-08-09 18:52:11 UTC (rev 204288)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2016-08-09 19:30:16 UTC (rev 204289)
@@ -41,10 +41,8 @@
from webkitpy.common.config.committervalidator import CommitterValidator
from webkitpy.common.config.ports import DeprecatedPort
-from webkitpy.common.host import Host
from webkitpy.common.net.bugzilla import Attachment
from webkitpy.common.net.statusserver import StatusServer
-from webkitpy.common.system import logutils
from webkitpy.common.system.executive import ScriptError
from webkitpy.tool.bot.botinfo import BotInfo
from webkitpy.tool.bot.commitqueuetask import CommitQueueTask, CommitQueueTaskDelegate
@@ -68,7 +66,7 @@
_fail_status = "Fail"
_error_status = "Error"
- def __init__(self, options=None, host=Host()): # Default values should never be collections (like []) as default values are shared between invocations
+ def __init__(self, options=None): # Default values should never be collections (like []) as default values are shared between invocations
options_list = (options or []) + [
make_option("--no-confirm", action="" dest="confirm", default=True, help="Do not ask the user for confirmation before running the queue. Dangerous!"),
make_option("--exit-after-iteration", action="" type="int", dest="iterations", default=None, help="Stop running the queue after iterating this number of times."),
@@ -78,7 +76,6 @@
self._iteration_count = 0
if not hasattr(self, 'architecture'):
self.architecture = None
- self.host = host
def _cc_watchers(self, bug_id):
try:
@@ -112,20 +109,17 @@
return command_output
def _log_directory(self):
- return self.host.filesystem.join("..", "%s-logs" % self.name)
+ return os.path.join("..", "%s-logs" % self.name)
# QueueEngineDelegate methods
def queue_log_path(self):
- return self.host.filesystem.join(self._log_directory(), "%s.log" % self.name)
+ return os.path.join(self._log_directory(), "%s.log" % self.name)
def work_item_log_path(self, work_item):
raise NotImplementedError, "subclasses must implement"
def begin_work_queue(self):
- # FIXME: We should stop the logging as well when the queue stops.
- # We are using logging.getLogger("webkitpy") instead of _log since we want to capture all messages logged from webkitpy modules.
- logutils.configure_logger_to_log_to_file(logging.getLogger("webkitpy"), self.queue_log_path(), self.host.filesystem)
_log.info("CAUTION: %s will discard all local changes in \"%s\"" % (self.name, self._tool.scm().checkout_root))
if self._options.confirm:
response = self._tool.user.prompt("Are you sure? Type \"yes\" to continue: ")
@@ -265,10 +259,9 @@
# Subclasses must override.
port_name = None
- def __init__(self, options=None, host=Host()):
+ def __init__(self, options=None):
self._port = None # We can't instantiate port here because tool isn't avaialble.
- self.host = host
- AbstractPatchQueue.__init__(self, options, host=host)
+ AbstractPatchQueue.__init__(self, options)
# FIXME: This is a hack to map between the old port names and the new port names.
def _new_port_name_from_old(self, port_name, platform):
@@ -320,10 +313,9 @@
class CommitQueue(PatchProcessingQueue, StepSequenceErrorHandler, CommitQueueTaskDelegate):
- def __init__(self, commit_queue_task_class=CommitQueueTask, host=Host()):
- self.host = host
+ def __init__(self, commit_queue_task_class=CommitQueueTask):
self._commit_queue_task_class = commit_queue_task_class
- PatchProcessingQueue.__init__(self, host=host)
+ PatchProcessingQueue.__init__(self)
name = "commit-queue"
port_name = "mac"
@@ -438,9 +430,8 @@
class AbstractReviewQueue(PatchProcessingQueue, StepSequenceErrorHandler):
"""This is the base-class for the EWS queues and the style-queue."""
- def __init__(self, options=None, host=Host()):
- self.host = host
- PatchProcessingQueue.__init__(self, options, host=host)
+ def __init__(self, options=None):
+ PatchProcessingQueue.__init__(self, options)
def review_patch(self, patch):
raise NotImplementedError("subclasses must implement")
@@ -472,9 +463,8 @@
class StyleQueue(AbstractReviewQueue, StyleQueueTaskDelegate):
name = "style-queue"
- def __init__(self, host=Host()):
- self.host = host
- AbstractReviewQueue.__init__(self, host=host)
+ def __init__(self):
+ AbstractReviewQueue.__init__(self)
def review_patch(self, patch):
task = StyleQueueTask(self, patch)
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py (204288 => 204289)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2016-08-09 18:52:11 UTC (rev 204288)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2016-08-09 19:30:16 UTC (rev 204289)
@@ -31,7 +31,6 @@
from webkitpy.common.checkout.scm import CheckoutNeedsUpdate
from webkitpy.common.checkout.scm.scm_mock import MockSCM
-from webkitpy.common.host_mock import MockHost
from webkitpy.common.net.layouttestresults import LayoutTestResults
from webkitpy.common.net.bugzilla import Attachment
from webkitpy.common.system.outputcapture import OutputCapture
@@ -48,7 +47,7 @@
class TestCommitQueue(CommitQueue):
def __init__(self, tool=None):
- CommitQueue.__init__(self, host=MockHost())
+ CommitQueue.__init__(self)
if tool:
self.bind_to_tool(tool)
self._options = MockOptions(confirm=False, parent_command="commit-queue", port=None)
@@ -63,24 +62,15 @@
class TestQueue(AbstractPatchQueue):
name = "test-queue"
- def __init__(self):
- AbstractPatchQueue.__init__(self, host=MockHost())
-
class TestReviewQueue(AbstractReviewQueue):
name = "test-review-queue"
- def __init__(self):
- AbstractReviewQueue.__init__(self, host=MockHost())
-
class TestFeederQueue(FeederQueue):
_sleep_duration = 0
- def __init__(self):
- FeederQueue.__init__(self, host=MockHost())
-
class AbstractQueueTest(CommandsTest):
def test_log_directory(self):
self.assertEqual(TestQueue()._log_directory(), os.path.join("..", "test-queue-logs"))
@@ -163,7 +153,7 @@
class AbstractPatchQueueTest(CommandsTest):
def test_next_patch(self):
- queue = AbstractPatchQueue(host=MockHost())
+ queue = AbstractPatchQueue()
tool = MockTool()
queue.bind_to_tool(tool)
queue._options = Mock()
@@ -181,7 +171,7 @@
class PatchProcessingQueueTest(CommandsTest):
def test_upload_results_archive_for_patch(self):
- queue = PatchProcessingQueue(host=MockHost())
+ queue = PatchProcessingQueue()
queue.name = "mock-queue"
tool = MockTool()
queue.bind_to_tool(tool)
@@ -270,7 +260,7 @@
"handle_script_error": "ScriptError error message\n\nMOCK output\n",
"handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.\n\nMock error message'\n",
}
- self.assert_queue_outputs(CommitQueue(host=MockHost()), tool=tool, expected_logs=expected_logs)
+ self.assert_queue_outputs(CommitQueue(), tool=tool, expected_logs=expected_logs)
def test_commit_queue_failure(self):
expected_logs = {
@@ -286,7 +276,7 @@
"handle_script_error": "ScriptError error message\n\nMOCK output\n",
"handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.\n\nMock error message'\n",
}
- queue = CommitQueue(host=MockHost())
+ queue = CommitQueue()
def mock_run_webkit_patch(command):
if command[0] == 'clean' or command[0] == 'update':
@@ -318,7 +308,7 @@
def results_from_patch_test_run(self, patch):
return LayoutTestResults([test_results.TestResult("mock_test_name.html", failures=[test_failures.FailureTextMismatch()])], did_exceed_test_failure_limit=False)
- queue = CommitQueue(MockCommitQueueTask, host=MockHost())
+ queue = CommitQueue(MockCommitQueueTask)
def mock_run_webkit_patch(command):
if command[0] == 'clean' or command[0] == 'update':
@@ -357,7 +347,7 @@
"handle_script_error": "ScriptError error message\n\nMOCK output\n",
"handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.\n\nMock error message'\n",
}
- self.assert_queue_outputs(CommitQueue(host=MockHost()), tool=tool, expected_logs=expected_logs)
+ self.assert_queue_outputs(CommitQueue(), tool=tool, expected_logs=expected_logs)
def test_rollout_lands(self):
tool = MockTool()
@@ -382,7 +372,7 @@
"handle_script_error": "ScriptError error message\n\nMOCK output\n",
"handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10005' with comment 'Rejecting attachment 10005 from commit-queue.\n\nMock error message'\n",
}
- self.assert_queue_outputs(CommitQueue(host=MockHost()), tool=tool, work_item=rollout_patch, expected_logs=expected_logs)
+ self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_logs=expected_logs)
def test_non_valid_patch(self):
tool = MockTool()
@@ -393,10 +383,10 @@
MOCK: release_work_item: commit-queue 10007
""",
}
- self.assert_queue_outputs(CommitQueue(host=MockHost()), tool=tool, work_item=patch, expected_logs=expected_logs)
+ self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=patch, expected_logs=expected_logs)
def test_auto_retry(self):
- queue = CommitQueue(host=MockHost())
+ queue = CommitQueue()
options = Mock()
options.parent_command = "commit-queue"
tool = AlwaysCommitQueueTool()
@@ -508,7 +498,7 @@
"handle_script_error": "MOCK output\n",
}
tool = MockTool(executive_throws_when_run=set(['check-style']))
- self.assert_queue_outputs(StyleQueue(host=MockHost()), expected_logs=expected_logs, tool=tool)
+ self.assert_queue_outputs(StyleQueue(), expected_logs=expected_logs, tool=tool)
def test_style_queue_with_watch_list_exception(self):
expected_logs = {
@@ -533,7 +523,7 @@
"handle_script_error": "MOCK output\n",
}
tool = MockTool(executive_throws_when_run=set(['apply-watchlist-local']))
- self.assert_queue_outputs(StyleQueue(host=MockHost()), expected_logs=expected_logs, tool=tool)
+ self.assert_queue_outputs(StyleQueue(), expected_logs=expected_logs, tool=tool)
def test_non_valid_patch(self):
tool = MockTool()
@@ -544,4 +534,4 @@
MOCK: release_work_item: style-queue 10007
""",
}
- self.assert_queue_outputs(StyleQueue(host=MockHost()), tool=tool, work_item=patch, expected_logs=expected_logs)
+ self.assert_queue_outputs(StyleQueue(), tool=tool, work_item=patch, expected_logs=expected_logs)
