webkitpy: clean up actually getting crash logs from DRT/WTR crashes
authorossy@webkit.org <ossy@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Mar 2012 08:18:39 +0000 (08:18 +0000)
committerossy@webkit.org <ossy@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Mar 2012 08:18:39 +0000 (08:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=81603

Unreviewed rolling out r111609 and part of r111615,
because it broke NRWT on Qt-WK2 platform.

* Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
(SingleTestRunner._handle_error):
* Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:
(write_test_result):
(TestResultWriter.write_crash_report):
* Scripts/webkitpy/layout_tests/port/base.py:
(Port.is_crash_reporter):
(Port._driver_class):
* Scripts/webkitpy/layout_tests/port/chromium.py:
(ChromiumDriver.run_test):
* Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
(ChromiumDriverTest.test_crashed_process_name):
* Scripts/webkitpy/layout_tests/port/driver.py:
(DriverOutput.__init__):
* Scripts/webkitpy/layout_tests/port/mac.py:
(MacPort):
(MacPort.is_crash_reporter):
(MacPort.release_http_lock):
* Scripts/webkitpy/layout_tests/port/test.py:
(TestDriver.run_test):
* Scripts/webkitpy/layout_tests/port/webkit.py:
(WebKitDriver.__init__):
(WebKitDriver._start):
(WebKitDriver.has_crashed):
(WebKitDriver._check_for_driver_crash):
(WebKitDriver.run_test):

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

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py
Tools/Scripts/webkitpy/layout_tests/port/base.py
Tools/Scripts/webkitpy/layout_tests/port/chromium.py
Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py
Tools/Scripts/webkitpy/layout_tests/port/driver.py
Tools/Scripts/webkitpy/layout_tests/port/mac.py
Tools/Scripts/webkitpy/layout_tests/port/test.py
Tools/Scripts/webkitpy/layout_tests/port/webkit.py

index b7a52ae..2ec4bd0 100644 (file)
@@ -1,5 +1,40 @@
 2012-03-22  Csaba Osztrogon√°c  <ossy@webkit.org>
 
+        webkitpy: clean up actually getting crash logs from DRT/WTR crashes
+        https://bugs.webkit.org/show_bug.cgi?id=81603
+
+        Unreviewed rolling out r111609 and part of r111615,
+        because it broke NRWT on Qt-WK2 platform.
+
+        * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
+        (SingleTestRunner._handle_error):
+        * Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:
+        (write_test_result):
+        (TestResultWriter.write_crash_report):
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        (Port.is_crash_reporter):
+        (Port._driver_class):
+        * Scripts/webkitpy/layout_tests/port/chromium.py:
+        (ChromiumDriver.run_test):
+        * Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
+        (ChromiumDriverTest.test_crashed_process_name):
+        * Scripts/webkitpy/layout_tests/port/driver.py:
+        (DriverOutput.__init__):
+        * Scripts/webkitpy/layout_tests/port/mac.py:
+        (MacPort):
+        (MacPort.is_crash_reporter):
+        (MacPort.release_http_lock):
+        * Scripts/webkitpy/layout_tests/port/test.py:
+        (TestDriver.run_test):
+        * Scripts/webkitpy/layout_tests/port/webkit.py:
+        (WebKitDriver.__init__):
+        (WebKitDriver._start):
+        (WebKitDriver.has_crashed):
+        (WebKitDriver._check_for_driver_crash):
+        (WebKitDriver.run_test):
+
+2012-03-22  Csaba Osztrogon√°c  <ossy@webkit.org>
+
         [Qt] Add full platforms to Qt buildslaves
         https://bugs.webkit.org/show_bug.cgi?id=81877
 
index 2f890b0..dcb10c1 100644 (file)
@@ -198,9 +198,7 @@ class SingleTestRunner:
             testname = self._test_name
 
         if driver_output.crash:
-            failures.append(test_failures.FailureCrash(bool(reference_filename),
-                                                       driver_output.crashed_process_name,
-                                                       driver_output.crashed_pid))
+            failures.append(test_failures.FailureCrash(bool(reference_filename)))
             if driver_output.error:
                 _log.debug("%s %s crashed, stack trace:" % (self._worker_name, testname))
             else:
index 70c3553..75d5278 100644 (file)
@@ -29,6 +29,7 @@
 
 import logging
 
+from webkitpy.common.system.crashlogs import CrashLogs
 from webkitpy.layout_tests.models import test_failures
 
 
@@ -62,7 +63,7 @@ def write_test_result(filesystem, port, test_name, driver_output,
             writer.write_audio_files(driver_output.audio, expected_driver_output.audio)
         elif isinstance(failure, test_failures.FailureCrash):
             crashed_driver_output = expected_driver_output if failure.is_reftest else driver_output
-            writer.write_crash_log(crashed_driver_output.crash_log)
+            writer.write_crash_report(crashed_driver_output.crashed_process_name, crashed_driver_output.error)
         elif isinstance(failure, test_failures.FailureReftestMismatch):
             writer.write_image_files(driver_output.image, expected_driver_output.image)
             # FIXME: This work should be done earlier in the pipeline (e.g., when we compare images for non-ref tests).
@@ -156,12 +157,15 @@ class TestResultWriter(object):
         fs.maybe_make_directory(fs.dirname(filename))
         fs.write_binary_file(filename, error)
 
-    def write_crash_log(self, crash_log):
+    def write_crash_report(self, crashed_process_name, error):
         fs = self._filesystem
         filename = self.output_filename(self.FILENAME_SUFFIX_CRASH_LOG + ".txt")
         fs.maybe_make_directory(fs.dirname(filename))
-        if crash_log is not None:
-            fs.write_text_file(filename, crash_log)
+        crash_logs = CrashLogs(fs)
+        log = crash_logs.find_newest_log(crashed_process_name)
+        # CrashLogs doesn't support every platform, so we fall back to
+        # including the stderr output, which is admittedly somewhat redundant.
+        fs.write_text_file(filename, log if log else error)
 
     def write_text_files(self, actual_text, expected_text):
         self.write_output_files(".txt", actual_text, expected_text)
index 482ebbe..0500bfc 100755 (executable)
@@ -290,6 +290,9 @@ class Port(object):
                                     actual_filename)
         return ''.join(diff)
 
+    def is_crash_reporter(self, process_name):
+        return False
+
     def check_for_leaks(self, process_name, process_pid):
         # Subclasses should check for leaks in the running process
         # and print any necessary warnings if leaks are found.
@@ -1051,14 +1054,6 @@ class Port(object):
         """Returns the port's driver implementation."""
         raise NotImplementedError('Port._driver_class')
 
-    def _get_crash_log(self, name, pid, stdout, stderr):
-        log = 'crash log for %s (pid %d)\n%s\n%s\n' % (
-            name or '??',
-            pid or '<unknown>',
-            '\n'.join('STDOUT: ' + line for line in stdout.decode('utf8').splitlines()) if stdout else '<empty>',
-            '\n'.join('STDERR: ' + line for line in stderr.decode('utf8').splitlines()) if stderr else '<empty>')
-        return log
-
     def virtual_test_suites(self):
         return []
 
index ada4a81..4275206 100755 (executable)
@@ -592,23 +592,16 @@ class ChromiumDriver(Driver):
                 text = None
 
         error = ''.join(error)
+        crashed_process_name = None
         # Currently the stacktrace is in the text output, not error, so append the two together so
         # that we can see stack in the output. See http://webkit.org/b/66806
         # FIXME: We really should properly handle the stderr output separately.
-        crash_log = ''
-        crashed_process_name = None
-        crashed_pid = None
         if crash:
+            error = error + str(text)
             crashed_process_name = self._port.driver_name()
-            if self._proc:
-                crashed_pid = self._proc.pid
-            crash_log = self._port._get_crash_log(crashed_process_name, crashed_pid, text, error)
-            if text:
-                error = error + text
 
         return DriverOutput(text, output_image, actual_checksum, audio=audio_bytes,
-            crash=crash, crashed_process_name=crashed_process_name, crashed_pid=crashed_pid, crash_log=crash_log,
-            test_time=run_time, timeout=timeout, error=error)
+            crash=crash, crashed_process_name=crashed_process_name, test_time=run_time, timeout=timeout, error=error)
 
     def start(self, pixel_tests, per_test_args):
         if not self._proc:
index 0c419d7..d3d311c 100644 (file)
@@ -91,23 +91,17 @@ class ChromiumDriverTest(unittest.TestCase):
         self.driver._proc.stdout.readline = mock_readline
         self._assert_write_command_and_read_line(expected_crash=True)
 
-    def test_crash_log(self):
+    def test_crashed_process_name(self):
         self.driver._proc = Mock()
 
         # Simulate a crash by having stdout close unexpectedly.
         def mock_readline():
             raise IOError
         self.driver._proc.stdout.readline = mock_readline
-        self.driver._proc.pid = 1234
 
         self.driver.test_to_uri = lambda test: 'mocktesturi'
-        self.driver._port.driver_name = lambda: 'mockdriver'
-        self.driver._port._get_crash_log = lambda name, pid, out, err: 'mockcrashlog'
         driver_output = self.driver.run_test(DriverInput(test_name='some/test.html', timeout=1, image_hash=None, is_reftest=False))
-        self.assertTrue(driver_output.crash)
-        self.assertEqual(driver_output.crashed_process_name, 'mockdriver')
-        self.assertEqual(driver_output.crashed_pid, 1234)
-        self.assertEqual(driver_output.crash_log, 'mockcrashlog')
+        self.assertEqual(self.driver._port.driver_name(), driver_output.crashed_process_name)
 
     def test_stop(self):
         self.pid = None
index e1d8a4b..fa6e81b 100644 (file)
@@ -64,8 +64,7 @@ class DriverOutput(object):
     strip_patterns.append((re.compile('scrollHeight [0-9]+'), 'scrollHeight'))
 
     def __init__(self, text, image, image_hash, audio, crash=False,
-            test_time=0, timeout=False, error='', crashed_process_name='??',
-            crashed_pid=None, crash_log=None):
+            test_time=0, timeout=False, error='', crashed_process_name=None):
         # FIXME: Args could be renamed to better clarify what they do.
         self.text = text
         self.image = image  # May be empty-string if the test crashes.
@@ -74,8 +73,6 @@ class DriverOutput(object):
         self.audio = audio  # Binary format is port-dependent.
         self.crash = crash
         self.crashed_process_name = crashed_process_name
-        self.crashed_pid = crashed_pid
-        self.crash_log = crash_log
         self.test_time = test_time
         self.timeout = timeout
         self.error = error  # stderr output
index a90f945..a90cb1c 100644 (file)
@@ -29,9 +29,7 @@
 import logging
 import re
 import subprocess
-import time
 
-from webkitpy.common.system.crashlogs import CrashLogs
 from webkitpy.layout_tests.port.apple import ApplePort
 from webkitpy.layout_tests.port.leakdetector import LeakDetector
 
@@ -98,6 +96,10 @@ class MacPort(ApplePort):
     def is_lion(self):
         return self._version == "lion"
 
+    # Belongs on a Platform object.
+    def is_crash_reporter(self, process_name):
+        return re.search(r'ReportCrash', process_name)
+
     def default_child_processes(self):
         if self.is_snowleopard():
             _log.warn("Cannot run tests in parallel on Snow Leopard due to rdar://problem/10621525.")
@@ -158,25 +160,6 @@ class MacPort(ApplePort):
     def release_http_lock(self):
         pass
 
-    def _get_crash_log(self, name, pid, stdout, stderr):
-        # Note that we do slow-spin here and wait, since it appears the time
-        # ReportCrash takes to actually write and flush the file varies when there are
-        # lots of simultaneous crashes going on.
-        # FIXME: Should most of this be moved into CrashLogs()?
-        crash_log = ''
-        crash_logs = CrashLogs(self._filesystem)
-        now = time.time()
-        deadline = now + 5 * int(self.get_option('child_processes'))
-        while not crash_log and now <= deadline:
-            crash_log = crash_logs.find_newest_log(name, pid, include_errors=True)
-            if not crash_log or not [line for line in crash_log.splitlines() if not line.startswith('ERROR')]:
-                time.sleep(0.1)
-                now = time.time()
-        if not crash_log:
-            crash_log = 'no crash log found for %s:%d' % (name, pid)
-            _log.warning(crash_log)
-        return crash_log
-
     def _path_to_helper(self):
         binary_name = 'LayoutTestHelper'
         return self._build_path(binary_name)
index f08c574..2163723 100644 (file)
@@ -35,7 +35,6 @@ from webkitpy.layout_tests.port import Port, Driver, DriverOutput
 from webkitpy.layout_tests.port.base import VirtualTestSuite
 from webkitpy.layout_tests.models.test_configuration import TestConfiguration
 from webkitpy.common.system.filesystem_mock import MockFileSystem
-from webkitpy.common.system.crashlogs import CrashLogs
 
 
 # This sets basic expectations for a test. Each individual expectation
@@ -520,22 +519,13 @@ class TestDriver(Driver):
         if test.actual_audio:
             audio = base64.b64decode(test.actual_audio)
         crashed_process_name = None
-        crashed_pid = None
         if test.crash:
             crashed_process_name = self._port.driver_name()
-            crashed_pid = 1
         elif test.web_process_crash:
             crashed_process_name = 'WebProcess'
-            crashed_pid = 2
-
-        crash_log = ''
-        if crashed_process_name:
-            crash_logs = CrashLogs(self._port._filesystem)
-            crash_log = crash_logs.find_newest_log(crashed_process_name, None) or ''
-
-        return DriverOutput(actual_text, test.actual_image, test.actual_checksum, audio,
-            crash=test.crash or test.web_process_crash, crashed_process_name=crashed_process_name,
-            crashed_pid=crashed_pid, crash_log=crash_log,
+        return DriverOutput(actual_text, test.actual_image,
+            test.actual_checksum, audio, crash=test.crash or test.web_process_crash,
+            crashed_process_name=crashed_process_name,
             test_time=time.time() - start_time, timeout=test.timeout, error=test.error)
 
     def start(self, pixel_tests, per_test_args):
index ef5b774..b998c47 100644 (file)
@@ -437,8 +437,7 @@ class WebKitDriver(Driver):
         # "#CRASHED - PROCESSNAME".  Since those can happen at any time
         # and ServerProcess won't be aware of them (since the actual tool
         # didn't crash, just a subprocess) we record the crashed subprocess name here.
-        self._crashed_process_name = None
-        self._crashed_pid = None
+        self._crashed_subprocess_name = None
 
         # stderr reading is scoped on a per-test (not per-block) basis, so we store the accumulated
         # stderr output, as well as if we've seen #EOF on this driver instance.
@@ -484,34 +483,28 @@ class WebKitDriver(Driver):
         # FIXME: We're assuming that WebKitTestRunner checks this DumpRenderTree-named environment variable.
         environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
         environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir()
-        self._crashed_process_name = None
-        self._crashed_pid = None
+        self._crashed_subprocess_name = None
         self._server_process = server_process.ServerProcess(self._port, server_name, self.cmd_line(pixel_tests, per_test_args), environment)
 
     def has_crashed(self):
         if self._server_process is None:
             return False
-        if self._crashed_process_name:
+        if self._crashed_subprocess_name:
             return True
         if self._server_process.has_crashed():
-            self._crashed_process_name = self._server_process.name()
-            self._crashed_pid = self._server_process.pid()
+            self._crashed_subprocess_name = self._port.driver_name()
             return True
+        return False
 
     def _check_for_driver_crash(self, error_line):
         if error_line == "#CRASHED\n":
             # This is used on Windows to report that the process has crashed
             # See http://trac.webkit.org/changeset/65537.
-            self._crashed_process_name = self._server_process.name()
-            self._crashed_pid = self._server_process.pid()
-        elif error_line.startswith("#CRASHED - WebProcess"):
+            self._crashed_subprocess_name = self._port.driver_name()
+            return True
+        if error_line == "#CRASHED - WebProcess\n":
             # WebKitTestRunner uses this to report that the WebProcess subprocess crashed.
-            pid = None
-            m = re.match('pid (\d+)', error_line)
-            if m:
-                pid = int(m.group(1))
-            self._crashed_process_name = 'WebProcess'
-            self._crashed_pid = pid
+            self._crashed_subprocess_name = "WebProcess"
             return True
         return self.has_crashed()
 
@@ -562,15 +555,10 @@ class WebKitDriver(Driver):
         # FIXME: We may need to also read stderr until the process dies?
         self.error_from_test += self._server_process.pop_all_buffered_stderr()
 
-        crash_log = ''
-        if self.has_crashed():
-            crash_log = self._port._get_crash_log(self._crashed_process_name, self._crashed_pid, text, self.error_from_test)
-
         return DriverOutput(text, image, actual_image_hash, audio,
             crash=self.has_crashed(), test_time=time.time() - start_time,
             timeout=self._server_process.timed_out, error=self.error_from_test,
-            crashed_process_name=self._crashed_process_name,
-            crashed_pid=self._crashed_pid, crash_log=crash_log)
+            crashed_process_name=self._crashed_subprocess_name)
 
     def _read_header(self, block, line, header_text, header_attr, header_filter=None):
         if line.startswith(header_text) and getattr(block, header_attr) is None: