webkitpy: Pass directory with crash logs into CrashLogs
authorjbedard@apple.com <jbedard@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 May 2017 19:52:18 +0000 (19:52 +0000)
committerjbedard@apple.com <jbedard@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 May 2017 19:52:18 +0000 (19:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172033
<rdar://problem/32157616>

Reviewed by Daniel Bates.

Refactor CrashLogs and the callers of CrashLogs so that the port object owns
the location of crash logs.

* Scripts/webkitpy/common/system/crashlogs.py:
(CrashLogs.__init__): Pass mandatory crash_log_directory when constructing.
(CrashLogs._find_newest_log_darwin): Use self._crash_log_directory instead of
generating one.
(CrashLogs._find_newest_log_win): Use self._crash_log_directory instead of
self._results_directory.
(CrashLogs._find_all_logs_darwin): Use self._crash_log_directory instead of
generating one.
(CrashLogs._log_directory_darwin): Moved to port.
* Scripts/webkitpy/common/system/crashlogs_unittest.py: Update tests since the path
to the crash log is no longer owned by CrashLogs.
* Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py: Use the general
directory for uploading crash logs. Note that crash logs are only supported on Mac
and Windows.
* Scripts/webkitpy/port/apple.py: Remove unneeded CrashLogs import.
* Scripts/webkitpy/port/base.py:
(Port.path_to_crash_logs): Unless ports declare otherwise, crash logs are assumed
to be in the results directory.
* Scripts/webkitpy/port/darwin.py:
(DarwinPort.path_to_crash_logs): Moved from CrashLogs._log_directory_darwin.
(DarwinPort._look_for_all_crash_logs_in_log_dir): Use port specific crash log path.
(DarwinPort._get_crash_log): Ditto.
* Scripts/webkitpy/port/darwin_testcase.py:
(DarwinTest.test_crashlog_path): Test that the Darwin ports are correctly calculating
the path to crash logs.
* Scripts/webkitpy/port/ios_device.py:
(IOSDevicePort.path_to_crash_logs): Currently, crash log retrieval is undefined for iOS.
* Scripts/webkitpy/port/ios_device_unittest.py:
(IOSDeviceTest.test_crashlog_path): Currently, crash log retrieval is undefined for iOS.
* Scripts/webkitpy/port/ios_simulator.py: Remove unneeded CrashLogs import.
* Scripts/webkitpy/port/mac.py: Ditto.
* Scripts/webkitpy/port/test.py:
(TestDriver.run_test): Use port specific crash log path when retrieving crash logs.
* Scripts/webkitpy/port/win.py:
(WinPort._get_crash_log): Ditto.
* Scripts/webkitpy/tool/commands/queries.py:
(execute): Construct a port object since this is the object which owns the path to crash logs.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@216776 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
Tools/ChangeLog
Tools/Scripts/webkitpy/common/system/crashlogs.py
Tools/Scripts/webkitpy/common/system/crashlogs_unittest.py
Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py
Tools/Scripts/webkitpy/port/apple.py
Tools/Scripts/webkitpy/port/base.py
Tools/Scripts/webkitpy/port/darwin.py
Tools/Scripts/webkitpy/port/darwin_testcase.py
Tools/Scripts/webkitpy/port/ios_device.py
Tools/Scripts/webkitpy/port/ios_device_unittest.py
Tools/Scripts/webkitpy/port/ios_simulator.py
Tools/Scripts/webkitpy/port/mac.py
Tools/Scripts/webkitpy/port/test.py
Tools/Scripts/webkitpy/port/win.py
Tools/Scripts/webkitpy/tool/commands/queries.py

index f6dc2da..4e655ee 100644 (file)
@@ -1,3 +1,52 @@
+2017-05-12  Jonathan Bedard  <jbedard@apple.com>
+
+        webkitpy: Pass directory with crash logs into CrashLogs
+        https://bugs.webkit.org/show_bug.cgi?id=172033
+        <rdar://problem/32157616>
+
+        Reviewed by Daniel Bates.
+
+        Refactor CrashLogs and the callers of CrashLogs so that the port object owns
+        the location of crash logs.
+
+        * Scripts/webkitpy/common/system/crashlogs.py:
+        (CrashLogs.__init__): Pass mandatory crash_log_directory when constructing.
+        (CrashLogs._find_newest_log_darwin): Use self._crash_log_directory instead of
+        generating one.
+        (CrashLogs._find_newest_log_win): Use self._crash_log_directory instead of
+        self._results_directory.
+        (CrashLogs._find_all_logs_darwin): Use self._crash_log_directory instead of
+        generating one.
+        (CrashLogs._log_directory_darwin): Moved to port.
+        * Scripts/webkitpy/common/system/crashlogs_unittest.py: Update tests since the path
+        to the crash log is no longer owned by CrashLogs.
+        * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py: Use the general
+        directory for uploading crash logs. Note that crash logs are only supported on Mac
+        and Windows.
+        * Scripts/webkitpy/port/apple.py: Remove unneeded CrashLogs import.
+        * Scripts/webkitpy/port/base.py:
+        (Port.path_to_crash_logs): Unless ports declare otherwise, crash logs are assumed
+        to be in the results directory.
+        * Scripts/webkitpy/port/darwin.py:
+        (DarwinPort.path_to_crash_logs): Moved from CrashLogs._log_directory_darwin.
+        (DarwinPort._look_for_all_crash_logs_in_log_dir): Use port specific crash log path.
+        (DarwinPort._get_crash_log): Ditto.
+        * Scripts/webkitpy/port/darwin_testcase.py:
+        (DarwinTest.test_crashlog_path): Test that the Darwin ports are correctly calculating
+        the path to crash logs.
+        * Scripts/webkitpy/port/ios_device.py:
+        (IOSDevicePort.path_to_crash_logs): Currently, crash log retrieval is undefined for iOS.
+        * Scripts/webkitpy/port/ios_device_unittest.py:
+        (IOSDeviceTest.test_crashlog_path): Currently, crash log retrieval is undefined for iOS.
+        * Scripts/webkitpy/port/ios_simulator.py: Remove unneeded CrashLogs import.
+        * Scripts/webkitpy/port/mac.py: Ditto.
+        * Scripts/webkitpy/port/test.py:
+        (TestDriver.run_test): Use port specific crash log path when retrieving crash logs.
+        * Scripts/webkitpy/port/win.py:
+        (WinPort._get_crash_log): Ditto.
+        * Scripts/webkitpy/tool/commands/queries.py:
+        (execute): Construct a port object since this is the object which owns the path to crash logs.
+
 2017-05-12  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed. Fix GTK+ test /webkit2/WebKitWebView/create-navigation-data after r216615.
index d5ee40a..6ee9098 100644 (file)
@@ -37,9 +37,9 @@ class CrashLogs(object):
     GLOBAL_PID_REGEX = re.compile(r'\s+Global\s+PID:\s+\[(?P<pid>\d+)\]')
     EXIT_PROCESS_PID_REGEX = re.compile(r'Exit process \d+:(?P<pid>\w+), code')
 
-    def __init__(self, host, results_directory=None):
+    def __init__(self, host, crash_log_directory):
         self._host = host
-        self._results_directory = results_directory
+        self._crash_log_directory = crash_log_directory
 
     def find_newest_log(self, process_name, pid=None, include_errors=False, newer_than=None):
         if self._host.platform.is_mac():
@@ -53,21 +53,11 @@ class CrashLogs(object):
             return self._find_all_logs_darwin(include_errors, newer_than)
         return None
 
-    def _log_directory_darwin(self):
-        log_directory = self._host.filesystem.expanduser("~")
-        log_directory = self._host.filesystem.join(log_directory, "Library", "Logs")
-        if self._host.filesystem.exists(self._host.filesystem.join(log_directory, "DiagnosticReports")):
-            log_directory = self._host.filesystem.join(log_directory, "DiagnosticReports")
-        else:
-            log_directory = self._host.filesystem.join(log_directory, "CrashReporter")
-        return log_directory
-
     def _find_newest_log_darwin(self, process_name, pid, include_errors, newer_than):
         def is_crash_log(fs, dirpath, basename):
             return basename.startswith(process_name + "_") and basename.endswith(".crash")
 
-        log_directory = self._log_directory_darwin()
-        logs = self._host.filesystem.files_under(log_directory, file_filter=is_crash_log)
+        logs = self._host.filesystem.files_under(self._crash_log_directory, file_filter=is_crash_log)
         first_line_regex = re.compile(r'^Process:\s+(?P<process_name>.*) \[(?P<pid>\d+)\]$')
         errors = ''
         for path in reversed(sorted(logs)):
@@ -92,7 +82,7 @@ class CrashLogs(object):
         def is_crash_log(fs, dirpath, basename):
             return basename.startswith("CrashLog")
 
-        logs = self._host.filesystem.files_under(self._results_directory, file_filter=is_crash_log)
+        logs = self._host.filesystem.files_under(self._crash_log_directory, file_filter=is_crash_log)
         errors = u''
         for path in reversed(sorted(logs)):
             try:
@@ -129,8 +119,7 @@ class CrashLogs(object):
         def is_crash_log(fs, dirpath, basename):
             return basename.endswith(".crash")
 
-        log_directory = self._log_directory_darwin()
-        logs = self._host.filesystem.files_under(log_directory, file_filter=is_crash_log)
+        logs = self._host.filesystem.files_under(self._crash_log_directory, file_filter=is_crash_log)
         first_line_regex = re.compile(r'^Process:\s+(?P<process_name>.*) \[(?P<pid>\d+)\]$')
         errors = ''
         crash_logs = {}
index 98e7232..08161a7 100644 (file)
@@ -235,6 +235,8 @@ quit:
 
 
 class CrashLogsTest(unittest.TestCase):
+    DARWIN_MOCK_CRASH_DIRECTORY = '/Users/mock/Library/Logs/DiagnosticReports'
+
     def create_crash_logs_darwin(self):
         if not SystemHost().platform.is_mac():
             return
@@ -252,7 +254,7 @@ class CrashLogsTest(unittest.TestCase):
         self.files['/Users/mock/Library/Logs/DiagnosticReports/DumpRenderTree_2011-06-13-150722_quadzen.crash'] = self.other_process_mock_crash_report
         self.files['/Users/mock/Library/Logs/DiagnosticReports/DumpRenderTree_2011-06-13-150723_quadzen.crash'] = self.misformatted_mock_crash_report
         self.filesystem = MockFileSystem(self.files)
-        crash_logs = CrashLogs(MockSystemHost(filesystem=self.filesystem))
+        crash_logs = CrashLogs(MockSystemHost(filesystem=self.filesystem), CrashLogsTest.DARWIN_MOCK_CRASH_DIRECTORY)
         logs = self.filesystem.files_under('/Users/mock/Library/Logs/DiagnosticReports/')
         for path in reversed(sorted(logs)):
             self.assertTrue(path in self.files.keys())
@@ -297,7 +299,7 @@ class CrashLogsTest(unittest.TestCase):
         self.assertIn('IOError: No such file or directory', log)
 
         self.filesystem = MockFileSystem(self.files)
-        crash_logs = CrashLogs(MockSystemHost(filesystem=self.filesystem))
+        crash_logs = CrashLogs(MockSystemHost(filesystem=self.filesystem), CrashLogsTest.DARWIN_MOCK_CRASH_DIRECTORY)
         self.filesystem.mtime = bad_mtime
         log = crash_logs.find_newest_log("DumpRenderTree", newer_than=1.0, include_errors=True)
         self.assertIn('OSError: No such file or directory', log)
@@ -344,7 +346,7 @@ class CrashLogsTest(unittest.TestCase):
             return
 
         crash_report = make_mock_crash_report_darwin('DumpRenderTree', 28528)
-        crash_logs = CrashLogs(MockSystemHost())
+        crash_logs = CrashLogs(MockSystemHost(), CrashLogsTest.DARWIN_MOCK_CRASH_DIRECTORY)
         crash_timestamp = crash_logs.get_timestamp_from_log(crash_report)
         self.assertIn('2011-12-07 13:27:34.816', str(crash_timestamp))
 
index b5885f0..7ab327a 100644 (file)
@@ -532,24 +532,24 @@ class RunTest(unittest.TestCase, StreamTestingMixin):
 
     def test_crash_log(self):
         # FIXME: Need to rewrite these tests to not be mac-specific, or move them elsewhere.
-        # Currently CrashLog uploading only works on Darwin.
-        if not self._platform.is_mac():
+        # Currently CrashLog uploading only works on Darwin and Windows.
+        if not self._platform.is_mac() or self._platform.is_win():
             return
         mock_crash_report = make_mock_crash_report_darwin('DumpRenderTree', 12345)
         host = MockHost()
-        host.filesystem.write_text_file('/Users/mock/Library/Logs/DiagnosticReports/DumpRenderTree_2011-06-13-150719_quadzen.crash', mock_crash_report)
+        host.filesystem.write_text_file('/tmp/layout-test-results/DumpRenderTree_2011-06-13-150719_quadzen.crash', mock_crash_report)
         _, regular_output, _ = logging_run(['failures/unexpected/crash-with-stderr.html', '--dump-render-tree'], tests_included=True, host=host)
         expected_crash_log = mock_crash_report
         self.assertEqual(host.filesystem.read_text_file('/tmp/layout-test-results/failures/unexpected/crash-with-stderr-crash-log.txt'), expected_crash_log)
 
     def test_web_process_crash_log(self):
         # FIXME: Need to rewrite these tests to not be mac-specific, or move them elsewhere.
-        # Currently CrashLog uploading only works on Darwin.
-        if not self._platform.is_mac():
+        # Currently CrashLog uploading only works on Darwin and Windows.
+        if not self._platform.is_mac() or self._platform.is_win():
             return
         mock_crash_report = make_mock_crash_report_darwin('WebProcess', 12345)
         host = MockHost()
-        host.filesystem.write_text_file('/Users/mock/Library/Logs/DiagnosticReports/WebProcess_2011-06-13-150719_quadzen.crash', mock_crash_report)
+        host.filesystem.write_text_file('/tmp/layout-test-results/WebProcess_2011-06-13-150719_quadzen.crash', mock_crash_report)
         logging_run(['failures/unexpected/web-process-crash-with-stderr.html'], tests_included=True, host=host)
         self.assertEqual(host.filesystem.read_text_file('/tmp/layout-test-results/failures/unexpected/web-process-crash-with-stderr-crash-log.txt'), mock_crash_report)
 
index 96ad09c..ca1b09e 100644 (file)
@@ -29,7 +29,6 @@
 import logging
 import os
 
-from webkitpy.common.system.crashlogs import CrashLogs
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.port.base import Port
 from webkitpy.layout_tests.models.test_configuration import TestConfiguration
index 113b5de..d819ba2 100644 (file)
@@ -1347,6 +1347,9 @@ class Port(object):
         """Returns the port's driver implementation."""
         return driver.Driver
 
+    def path_to_crash_logs(self):
+        return self.results_directory()
+
     def _get_crash_log(self, name, pid, stdout, stderr, newer_than):
         name_str = name or '<unknown process name>'
         pid_str = str(pid or '<unknown>')
index 9b94d94..1ebdf6d 100644 (file)
@@ -85,6 +85,15 @@ class DarwinPort(ApplePort):
         self._executive.popen([self.path_to_script('run-safari')] + self._arguments_for_configuration() + ['--no-saved-state', '-NSOpen', results_filename],
             cwd=self.webkit_base(), stdout=file(os.devnull), stderr=file(os.devnull))
 
+    @memoized
+    def path_to_crash_logs(self):
+        log_directory = self.host.filesystem.expanduser('~')
+        log_directory = self.host.filesystem.join(log_directory, 'Library', 'Logs')
+        diagnositc_reports_directory = self.host.filesystem.join(log_directory, 'DiagnosticReports')
+        if self.host.filesystem.exists(diagnositc_reports_directory):
+            return diagnositc_reports_directory
+        return self.host.filesystem.join(log_directory, 'CrashReporter')
+
     def _merge_crash_logs(self, logs, new_logs, crashed_processes):
         for test, crash_log in new_logs.iteritems():
             try:
@@ -98,7 +107,7 @@ class DarwinPort(ApplePort):
         return logs
 
     def _look_for_all_crash_logs_in_log_dir(self, newer_than):
-        crash_log = CrashLogs(self.host)
+        crash_log = CrashLogs(self.host, self.path_to_crash_logs())
         return crash_log.find_all_logs(include_errors=True, newer_than=newer_than)
 
     def _get_crash_log(self, name, pid, stdout, stderr, newer_than, time_fn=None, sleep_fn=None, wait_for_log=True):
@@ -108,7 +117,7 @@ class DarwinPort(ApplePort):
         time_fn = time_fn or time.time
         sleep_fn = sleep_fn or time.sleep
         crash_log = ''
-        crash_logs = CrashLogs(self.host)
+        crash_logs = CrashLogs(self.host, self.path_to_crash_logs())
         now = time_fn()
         deadline = now + 5 * int(self.get_option('child_processes', 1))
         while not crash_log and now <= deadline:
index 1e8da6b..f80c23b 100644 (file)
@@ -25,6 +25,7 @@ import time
 from webkitpy.port import port_testcase
 from webkitpy.common.system.outputcapture import OutputCapture
 from webkitpy.tool.mocktool import MockOptions
+from webkitpy.common.system.filesystem_mock import MockFileSystem
 from webkitpy.common.system.executive_mock import MockExecutive, MockExecutive2, MockProcess, ScriptError
 from webkitpy.common.system.systemhost_mock import MockSystemHost
 
@@ -88,6 +89,14 @@ class DarwinTest(port_testcase.PortTestCase):
         port.stop_helper()
         oc.restore_output()
 
+    def test_crashlog_path(self):
+        port = self.make_port()
+        self.assertEqual(port.path_to_crash_logs(), '/Users/mock/Library/Logs/CrashReporter')
+
+        host = MockSystemHost(filesystem=MockFileSystem(files=['/Users/mock/Library/Logs/DiagnosticReports/crashlog.crash']))
+        port = self.make_port(host)
+        self.assertEqual(port.path_to_crash_logs(), '/Users/mock/Library/Logs/DiagnosticReports')
+
     def test_spindump(self):
 
         def logging_run_command(args):
index 3f87b46..4ac8c90 100644 (file)
@@ -68,6 +68,9 @@ class IOSDevicePort(IOSPort):
         return port_name
 
     # FIXME: These need device implementations <rdar://problem/30497991>.
+    def path_to_crash_logs(self):
+        raise NotImplementedError
+
     def check_for_leaks(self, process_name, process_pid):
         pass
 
index f002f8b..72c653e 100644 (file)
@@ -38,6 +38,9 @@ class IOSDeviceTest(ios_testcase.IOSTest):
         self.assertEqual('ios-device', self.make_port().operating_system())
 
     # FIXME: Update tests when <rdar://problem/30497991> is completed.
+    def test_crashlog_path(self):
+        pass
+
     def test_spindump(self):
         pass
 
index 7681927..e449f82 100644 (file)
@@ -32,7 +32,6 @@ from webkitpy.common.memoized import memoized
 from webkitpy.port.device import Device
 from webkitpy.port.ios import IOSPort
 from webkitpy.xcode.simulator import Simulator, Runtime, DeviceType
-from webkitpy.common.system.crashlogs import CrashLogs
 
 
 _log = logging.getLogger(__name__)
index 06e9ffa..4f861e9 100644 (file)
@@ -32,7 +32,6 @@ import os
 import time
 import re
 
-from webkitpy.common.system.crashlogs import CrashLogs
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.port.darwin import DarwinPort
 
index ab31d93..2168cfe 100644 (file)
@@ -579,7 +579,7 @@ class TestDriver(Driver):
 
         crash_log = ''
         if crashed_process_name:
-            crash_logs = CrashLogs(self._port.host)
+            crash_logs = CrashLogs(self._port.host, self._port.path_to_crash_logs())
             crash_log = crash_logs.find_newest_log(crashed_process_name, None) or ''
 
         if stop_when_done:
index 3af2e0e..31dcec4 100644 (file)
@@ -377,7 +377,7 @@ class WinPort(ApplePort):
         time_fn = time_fn or time.time
         sleep_fn = sleep_fn or time.sleep
         crash_log = ''
-        crash_logs = CrashLogs(self.host, self.results_directory())
+        crash_logs = CrashLogs(self.host, self.path_to_crash_logs())
         now = time_fn()
         # FIXME: delete this after we're sure this code is working ...
         _log.debug('looking for crash log for %s:%s' % (name, str(pid)))
index bb5bba2..1e74698 100644 (file)
@@ -430,7 +430,8 @@ and PID and prints it to stdout."""
     argument_names = "PROCESS_NAME [PID]"
 
     def execute(self, options, args, tool):
-        crash_logs = CrashLogs(tool)
+        default_port = tool.port_factory.get()
+        crash_logs = CrashLogs(tool, default_port.path_to_crash_logs())
         pid = None
         if len(args) > 1:
             pid = int(args[1])