nrwt: clean up server process, webkit driver so chromium can use it
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Apr 2012 22:10:07 +0000 (22:10 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Apr 2012 22:10:07 +0000 (22:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=84910

Reviewed by Ojan Vafai.

This change moves the "sample a process" logic out of
server_process.py and into a port-specific class (where really
only the mac has an implementation), and also preemptively kills
DRT when a test times out in WebKitDriver (rather than waiting through the
additional delays caused by calling stop() when we would want to
restart the driver generically in worker.py).

These changes will make it possible for the chromium port to
switch over to the stock WebKitDriver implementation, at least
on mac and linux.

* Scripts/webkitpy/layout_tests/port/base.py:
(Port.sample_process):
* Scripts/webkitpy/layout_tests/port/mac.py:
(MacPort.sample_process):
* Scripts/webkitpy/layout_tests/port/mac_unittest.py:
(test_helper_fails_to_stop):
(test_sample_process):
(test_sample_process.logging_run_command):
(test_sample_process_throws_exception):
(test_sample_process_throws_exception.throwing_run_command):
* Scripts/webkitpy/layout_tests/port/server_process.py:
(ServerProcess._log):
(ServerProcess._handle_timeout):
(ServerProcess.stop):
(ServerProcess):
(ServerProcess.kill): Here we add a method to immediately stop
the process rather than trying to shut it down cleanly.
* Scripts/webkitpy/layout_tests/port/server_process_unittest.py:
(TestServerProcess.test_broken_pipe):
* Scripts/webkitpy/layout_tests/port/webkit.py:
(WebKitDriver.run_test): Fix an issue where we weren't passing
along any per-test args (only needed for Chromium, but still).
Also, kill the driver immediately if we time out a test.

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

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/port/base.py
Tools/Scripts/webkitpy/layout_tests/port/mac.py
Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py
Tools/Scripts/webkitpy/layout_tests/port/server_process.py
Tools/Scripts/webkitpy/layout_tests/port/server_process_unittest.py
Tools/Scripts/webkitpy/layout_tests/port/webkit.py

index 33ff88c..c2a9522 100644 (file)
@@ -1,3 +1,45 @@
+2012-04-26  Dirk Pranke  <dpranke@chromium.org>
+
+        nrwt: clean up server process, webkit driver so chromium can use it
+        https://bugs.webkit.org/show_bug.cgi?id=84910
+
+        Reviewed by Ojan Vafai.
+
+        This change moves the "sample a process" logic out of
+        server_process.py and into a port-specific class (where really
+        only the mac has an implementation), and also preemptively kills
+        DRT when a test times out in WebKitDriver (rather than waiting through the
+        additional delays caused by calling stop() when we would want to
+        restart the driver generically in worker.py).
+
+        These changes will make it possible for the chromium port to
+        switch over to the stock WebKitDriver implementation, at least
+        on mac and linux.
+
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        (Port.sample_process):
+        * Scripts/webkitpy/layout_tests/port/mac.py:
+        (MacPort.sample_process):
+        * Scripts/webkitpy/layout_tests/port/mac_unittest.py:
+        (test_helper_fails_to_stop):
+        (test_sample_process):
+        (test_sample_process.logging_run_command):
+        (test_sample_process_throws_exception):
+        (test_sample_process_throws_exception.throwing_run_command):
+        * Scripts/webkitpy/layout_tests/port/server_process.py:
+        (ServerProcess._log):
+        (ServerProcess._handle_timeout):
+        (ServerProcess.stop):
+        (ServerProcess):
+        (ServerProcess.kill): Here we add a method to immediately stop
+        the process rather than trying to shut it down cleanly.
+        * Scripts/webkitpy/layout_tests/port/server_process_unittest.py:
+        (TestServerProcess.test_broken_pipe):
+        * Scripts/webkitpy/layout_tests/port/webkit.py:
+        (WebKitDriver.run_test): Fix an issue where we weren't passing
+        along any per-test args (only needed for Chromium, but still).
+        Also, kill the driver immediately if we time out a test.
+
 2012-04-26  Benjamin Poulain  <bpoulain@apple.com>
 
         ObjcClass::methodsNamed() can leak if buffer is dynamically allocated
index 6c45d9d..ea9225b 100755 (executable)
@@ -1081,6 +1081,9 @@ class Port(object):
             '\n'.join(('STDOUT: ' + l) for l in stdout_lines),
             '\n'.join(('STDERR: ' + l) for l in stderr_lines))
 
+    def sample_process(self, name, pid):
+        pass
+
     def virtual_test_suites(self):
         return []
 
index a611458..a8128d8 100644 (file)
@@ -33,6 +33,7 @@ import subprocess
 import time
 
 from webkitpy.common.system.crashlogs import CrashLogs
+from webkitpy.common.system.executive import ScriptError
 from webkitpy.layout_tests.port.apple import ApplePort
 from webkitpy.layout_tests.port.leakdetector import LeakDetector
 
@@ -198,6 +199,20 @@ class MacPort(ApplePort):
             _log.warning(crash_log)
         return crash_log
 
+    def sample_process(self, name, pid):
+        try:
+            hang_report = self._filesystem.join(self.results_directory(), "%s-%s.sample.txt" % (name, pid))
+            self._executive.run_command([
+                "/usr/bin/sample",
+                pid,
+                10,
+                10,
+                "-file",
+                hang_report,
+            ])
+        except ScriptError, e:
+            _log.warning('Unable to sample process.')
+
     def _path_to_helper(self):
         binary_name = 'LayoutTestHelper'
         return self._build_path(binary_name)
index ae2eee5..0b9bca9 100644 (file)
@@ -31,7 +31,7 @@ from webkitpy.layout_tests.port import port_testcase
 from webkitpy.common.system.filesystem_mock import MockFileSystem
 from webkitpy.common.system.outputcapture import OutputCapture
 from webkitpy.tool.mocktool import MockOptions
-from webkitpy.common.system.executive_mock import MockExecutive, MockProcess
+from webkitpy.common.system.executive_mock import MockExecutive, MockExecutive2, MockProcess, ScriptError
 from webkitpy.common.system.systemhost_mock import MockSystemHost
 
 
@@ -234,3 +234,22 @@ java/
         port.start_helper()
         port.stop_helper()
         oc.restore_output()
+
+    def test_sample_process(self):
+
+        def logging_run_command(args):
+            print args
+
+        port = self.make_port()
+        port._executive = MockExecutive2(run_command_fn=logging_run_command)
+        expected_stdout = "['/usr/bin/sample', 42, 10, 10, '-file', '/mock-build/layout-test-results/test-42.sample.txt']\n"
+        OutputCapture().assert_outputs(self, port.sample_process, args=['test', 42], expected_stdout=expected_stdout)
+
+    def test_sample_process_throws_exception(self):
+
+        def throwing_run_command(args):
+            raise ScriptError("MOCK script error")
+
+        port = self.make_port()
+        port._executive = MockExecutive2(run_command_fn=throwing_run_command)
+        OutputCapture().assert_outputs(self, port.sample_process, args=['test', 42])
index 245b77a..c50c0e6 100644 (file)
@@ -179,25 +179,10 @@ class ServerProcess:
         _log.info('')
         _log.info(message)
 
-    def _sample(self):
-        if sys.platform != "darwin":
-            return
-        try:
-            hang_report = os.path.join(self._port.results_directory(), "%s-%s.sample.txt" % (self._name, self._proc.pid))
-            self._executive.run_command([
-                "/usr/bin/sample",
-                self._proc.pid,
-                10,
-                10,
-                "-file",
-                hang_report,
-            ])
-        except ScriptError, e:
-            self._log('Unable to sample process.')
 
     def _handle_timeout(self):
         self.timed_out = True
-        self._sample()
+        self._port.sample_process(self._name, self._proc.pid)
 
     def _split_string_after_index(self, string, index):
         return string[:index], string[index:]
@@ -277,6 +262,13 @@ class ServerProcess:
                 time.sleep(0.01)
             if self._proc.poll() is None:
                 _log.warning('stopping %s timed out, killing it' % self._name)
-                self._executive.kill_process(self._proc.pid)
+                self.kill()
                 _log.warning('killed')
         self._reset()
+
+    def kill(self):
+        if self._proc:
+            self._executive.kill_process(self._proc.pid)
+            if self._proc.poll() is not None:
+                self._proc.wait()
+            self._reset()
index f2bc996..b45bad3 100644 (file)
@@ -35,13 +35,6 @@ from webkitpy.common.system.executive_mock import MockExecutive2
 from webkitpy.common.system.outputcapture import OutputCapture
 
 
-def _logging_run_command(args):
-    print args
-
-
-def _throwing_run_command(args):
-    raise ScriptError("MOCK script error")
-
 
 class TrivialMockPort(object):
     def results_directory(self):
@@ -91,20 +84,3 @@ class TestServerProcess(unittest.TestCase):
         self.assertTrue(server_process.has_crashed())
         self.assertEquals(server_process._proc, None)
         self.assertEquals(server_process.broken_pipes, [server_process.stdin])
-
-    def test_sample_process(self):
-        # Currently, sample-on-timeout only works on Darwin.
-        if sys.platform != "darwin":
-            return
-        server_process = FakeServerProcess(port_obj=TrivialMockPort(), name="test", cmd=["test"], executive=MockExecutive2(run_command_fn=_logging_run_command))
-        server_process._proc = MockProc(server_process)
-        expected_stdout = "['/usr/bin/sample', 1, 10, 10, '-file', '/mock-results/test-1.sample.txt']\n"
-        OutputCapture().assert_outputs(self, server_process._sample, expected_stdout=expected_stdout)
-
-    def test_sample_process_throws_exception(self):
-        # Currently, sample-on-timeout only works on Darwin.
-        if sys.platform != "darwin":
-            return
-        server_process = FakeServerProcess(port_obj=TrivialMockPort(), name="test", cmd=["test"], executive=MockExecutive2(run_command_fn=_throwing_run_command))
-        server_process._proc = MockProc(server_process)
-        OutputCapture().assert_outputs(self, server_process._sample)
index a91af9a..79f9ad0 100644 (file)
@@ -543,7 +543,7 @@ class WebKitDriver(Driver):
     def run_test(self, driver_input):
         start_time = time.time()
         if not self._server_process:
-            self._start(driver_input.should_run_pixel_test, [])
+            self._start(driver_input.should_run_pixel_test, driver_input.args)
         self.error_from_test = str()
         self.err_seen_eof = False
 
@@ -565,9 +565,15 @@ class WebKitDriver(Driver):
             crash_log = self._port._get_crash_log(self._crashed_process_name, self._crashed_pid, text, self.error_from_test,
                                                   newer_than=start_time)
 
+        timeout = self._server_process.timed_out
+        if timeout:
+            # DRT doesn't have a built in timer to abort the test, so we might as well
+            # kill the process directly and not wait for it to shut down cleanly (since it may not).
+            self._server_process.kill()
+
         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,
+            timeout=timeout, error=self.error_from_test,
             crashed_process_name=self._crashed_process_name,
             crashed_pid=self._crashed_pid, crash_log=crash_log)