[chromium] use "drt-style" output, not "test-shell-style" output, on mac and linux DRT
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Apr 2012 21:03:29 +0000 (21:03 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Apr 2012 21:03:29 +0000 (21:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=84917

Unreviewed, build fix.

Tools:

Reland the change in r115453 with a fix for chromium win.
The logic in ChromiumDriver was busted on windows, causing DRT
to get launched without the --test-shell flag.

* Scripts/webkitpy/layout_tests/port/chromium.py:
(ChromiumDriver):
(ChromiumDriver.__init__):
(ChromiumDriver._wrapper_options):
(ChromiumDriver.cmd_line):
(ChromiumDriver._start):
(ChromiumDriver.has_crashed):
(ChromiumDriver.run_test):
(ChromiumDriver.stop):
* Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
(ChromiumDriverTest.setUp):
(ChromiumDriverTest.test_stop):
(ChromiumDriverTest.test_two_drivers.MockDriver.__init__):
(ChromiumDriverTest.test_two_drivers):

LayoutTests:

Reland the change in r115453 with a fix for chromium win.

* platform/chromium/test_expectations.txt:

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

LayoutTests/ChangeLog
LayoutTests/platform/chromium/test_expectations.txt
Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/port/chromium.py
Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py

index c01d866..209adbf 100644 (file)
@@ -1,3 +1,14 @@
+2012-04-27  Dirk Pranke  <dpranke@chromium.org>
+
+        [chromium] use "drt-style" output, not "test-shell-style" output, on mac and linux DRT
+        https://bugs.webkit.org/show_bug.cgi?id=84917
+
+        Unreviewed, build fix.
+
+        Reland the change in r115453 with a fix for chromium win.
+
+        * platform/chromium/test_expectations.txt:
+
 2012-04-27  Eric Carlson  <eric.carlson@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=85038
index 03ad6f8..ad60baa 100644 (file)
@@ -3696,6 +3696,9 @@ BUGWK85038 : http/tests/inspector/network/network-initiator.html = TEXT TIMEOUT
 BUGWK85073 : fast/images/gif-large-checkerboard.html = CRASH PASS
 BUGWK85082 XP DEBUG : platform/chromium/media/video-capture-preview.html = CRASH PASS
 
+// This appears to be sensitive to --test-shell vs "drt mode" in DRT for some reason
+BUGWK85085 MAC LINUX : svg/custom/clip-path-referencing-use2.svg = TEXT
+
 BUGWK85090 : fast/js/dfg-float64array.html = PASS TIMEOUT
 BUGWK85090 : fast/js/dfg-uint16array.html = PASS TIMEOUT
 BUGWK85090 : fast/js/dfg-uint8array.html = PASS TIMEOUT
index 6f4b945..0d91a5f 100644 (file)
@@ -1,3 +1,29 @@
+2012-04-27  Dirk Pranke  <dpranke@chromium.org>
+
+        [chromium] use "drt-style" output, not "test-shell-style" output, on mac and linux DRT
+        https://bugs.webkit.org/show_bug.cgi?id=84917
+
+        Unreviewed, build fix.
+
+        Reland the change in r115453 with a fix for chromium win.
+        The logic in ChromiumDriver was busted on windows, causing DRT
+        to get launched without the --test-shell flag.
+
+        * Scripts/webkitpy/layout_tests/port/chromium.py:
+        (ChromiumDriver):
+        (ChromiumDriver.__init__):
+        (ChromiumDriver._wrapper_options):
+        (ChromiumDriver.cmd_line):
+        (ChromiumDriver._start):
+        (ChromiumDriver.has_crashed):
+        (ChromiumDriver.run_test):
+        (ChromiumDriver.stop):
+        * Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
+        (ChromiumDriverTest.setUp):
+        (ChromiumDriverTest.test_stop):
+        (ChromiumDriverTest.test_two_drivers.MockDriver.__init__):
+        (ChromiumDriverTest.test_two_drivers):
+
 2012-04-27  Peter Beverloo  <peter@chromium.org>
 
         Add John Grabowski as a non-committer to committers.py
index 61156a2..1a5ca01 100755 (executable)
@@ -46,7 +46,8 @@ from webkitpy.layout_tests.controllers.manager import Manager
 from webkitpy.layout_tests.models import test_expectations
 from webkitpy.layout_tests.models.test_configuration import TestConfiguration
 from webkitpy.layout_tests.port.base import Port, VirtualTestSuite
-from webkitpy.layout_tests.port.driver import Driver, DriverOutput
+from webkitpy.layout_tests.port.driver import DriverOutput
+from webkitpy.layout_tests.port.webkit import WebKitDriver
 from webkitpy.layout_tests.port import builders
 from webkitpy.layout_tests.servers import http_server
 from webkitpy.layout_tests.servers import websocket_server
@@ -403,20 +404,34 @@ class ChromiumPort(Port):
         return self._build_path(self.get_option('configuration'), binary_name)
 
 
-# FIXME: This should inherit from WebKitDriver now that Chromium has a DumpRenderTree process like the rest of WebKit.
-class ChromiumDriver(Driver):
+class ChromiumDriver(WebKitDriver):
+    KILL_TIMEOUT_DEFAULT = 3.0
+
     def __init__(self, port, worker_number, pixel_tests, no_timeout=False):
-        Driver.__init__(self, port, worker_number, pixel_tests, no_timeout)
+        WebKitDriver.__init__(self, port, worker_number, pixel_tests, no_timeout)
         self._proc = None
         self._image_path = None
 
+        # FIXME: Make the regular webkit driver work on win as well so we can delete all of this driver code.
+        if port.host.platform.is_win():
+            if not hasattr(port._options, 'additional_drt_flag'):
+                port._options.additional_drt_flag = []
+            if not '--test-shell' in port._options.additional_drt_flag:
+                port._options.additional_drt_flag.append('--test-shell')
+
+        self._test_shell = '--test-shell' in port.get_option('additional_drt_flag', [])
+
     def _wrapper_options(self, pixel_tests):
         cmd = []
         if pixel_tests:
-            if not self._image_path:
-                self._image_path = self._port._filesystem.join(self._port.results_directory(), 'png_result%s.png' % self._worker_number)
-            # See note above in diff_image() for why we need _convert_path().
-            cmd.append("--pixel-tests=" + self._port._convert_path(self._image_path))
+            if self._test_shell:
+                if not self._image_path:
+                    self._image_path = self._port._filesystem.join(self._port.results_directory(), 'png_result%s.png' % self._worker_number)
+                 # See note above in diff_image() for why we need _convert_path().
+                cmd.append("--pixel-tests=" + self._port._convert_path(self._image_path))
+            else:
+                cmd.append('--pixel-tests')
+
         # FIXME: This is not None shouldn't be necessary, unless --js-flags="''" changes behavior somehow?
         if self._port.get_option('js_flags') is not None:
             cmd.append('--js-flags="' + self._port.get_option('js_flags') + '"')
@@ -450,15 +465,18 @@ class ChromiumDriver(Driver):
     def cmd_line(self, pixel_tests, per_test_args):
         cmd = self._command_wrapper(self._port.get_option('wrapper'))
         cmd.append(self._port._path_to_driver())
-        # FIXME: Why does --test-shell exist?  TestShell is dead, shouldn't this be removed?
-        # It seems it's still in use in Tools/DumpRenderTree/chromium/DumpRenderTree.cpp as of 8/10/11.
-        cmd.append('--test-shell')
         cmd.extend(self._wrapper_options(pixel_tests))
         cmd.extend(per_test_args)
 
+        if not self._test_shell:
+            cmd.append('-')
+
         return cmd
 
     def _start(self, pixel_tests, per_test_args):
+        if not self._test_shell:
+            return super(ChromiumDriver, self)._start(pixel_tests, per_test_args)
+
         assert not self._proc
         # FIXME: This should use ServerProcess like WebKitDriver does.
         # FIXME: We should be reading stderr and stdout separately like how WebKitDriver does.
@@ -466,6 +484,9 @@ class ChromiumDriver(Driver):
         self._proc = subprocess.Popen(self.cmd_line(pixel_tests, per_test_args), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, close_fds=close_fds)
 
     def has_crashed(self):
+        if not self._test_shell:
+            return super(ChromiumDriver, self).has_crashed()
+
         if self._proc is None:
             return False
         return self._proc.poll() is not None
@@ -523,6 +544,9 @@ class ChromiumDriver(Driver):
             self._port._filesystem.remove(self._image_path)
 
     def run_test(self, driver_input):
+        if not self._test_shell:
+            return super(ChromiumDriver, self).run_test(driver_input)
+
         if not self._proc:
             self._start(driver_input.should_run_pixel_test, driver_input.args)
 
@@ -624,9 +648,11 @@ class ChromiumDriver(Driver):
             self._start(pixel_tests, per_test_args)
 
     def stop(self):
+        if not self._test_shell:
+            return super(ChromiumDriver, self).stop()
+
         if not self._proc:
             return
-        # FIXME: If we used ServerProcess all this would happen for free with ServerProces.stop()
         self._proc.stdin.close()
         self._proc.stdout.close()
         if self._proc.stderr:
@@ -634,9 +660,9 @@ class ChromiumDriver(Driver):
         time_out_ms = self._port.get_option('time_out_ms')
         if time_out_ms and not self._no_timeout:
             timeout_ratio = float(time_out_ms) / self._port.default_test_timeout_ms()
-            kill_timeout_seconds = 3.0 * timeout_ratio if timeout_ratio > 1.0 else 3.0
+            kill_timeout_seconds = self.KILL_TIMEOUT_DEFAULT * timeout_ratio if timeout_ratio > 1.0 else self.KILL_TIMEOUT_DEFAULT
         else:
-            kill_timeout_seconds = 3.0
+            kill_timeout_seconds = self.KILL_TIMEOUT_DEFAULT
 
         # Closing stdin/stdout/stderr hangs sometimes on OS X,
         # (see __init__(), above), and anyway we don't want to hang
index 0cf04ac..026f811 100644 (file)
@@ -35,6 +35,7 @@ from webkitpy.common.system import logtesting
 from webkitpy.common.system.executive_mock import MockExecutive, MockExecutive2
 from webkitpy.common.system.filesystem_mock import MockFileSystem
 from webkitpy.common.system.systemhost_mock import MockSystemHost
+from webkitpy.layout_tests.port.config_mock import MockConfig
 from webkitpy.thirdparty.mock import Mock
 from webkitpy.tool.mocktool import MockOptions
 
@@ -51,9 +52,11 @@ from webkitpy.layout_tests.port.driver import DriverInput
 
 class ChromiumDriverTest(unittest.TestCase):
     def setUp(self):
-        mock_port = Mock()  # FIXME: This should use a tighter mock.
-        mock_port.default_test_timeout_ms = lambda: 1000
-        self.driver = chromium.ChromiumDriver(mock_port, worker_number=0, pixel_tests=True)
+        host = MockSystemHost()
+        options = MockOptions(configuration='Release', additional_drt_flag=['--test-shell'])
+        config = MockConfig(filesystem=host.filesystem, default_configuration='Release')
+        self.port = chromium_mac.ChromiumMacPort(host, 'chromium-mac-snowleopard', options=options, config=config)
+        self.driver = chromium.ChromiumDriver(self.port, worker_number=0, pixel_tests=True)
 
     def test_test_shell_command(self):
         expected_command = "test.html 2 checksum\n"
@@ -132,28 +135,24 @@ class ChromiumDriverTest(unittest.TestCase):
                 self.driver._proc.poll = lambda: 2
 
         self.driver._port._executive = FakeExecutive()
-        # Override the kill timeout (ms) so the test runs faster.
-        self.driver._port.get_option = lambda name: 1
+        self.driver.KILL_TIMEOUT_DEFAULT = 0.01
         self.driver.stop()
         self.assertTrue(self.wait_called)
         self.assertEquals(self.pid, 1)
 
     def test_two_drivers(self):
-        mock_port = Mock()
 
         class MockDriver(chromium.ChromiumDriver):
-            def __init__(self):
-                chromium.ChromiumDriver.__init__(self, mock_port, worker_number=0, pixel_tests=False)
+            def __init__(self, port):
+                chromium.ChromiumDriver.__init__(self, port, worker_number=0, pixel_tests=False)
 
             def cmd_line(self, pixel_test, per_test_args):
                 return 'python'
 
         # get_option is used to get the timeout (ms) for a process before we kill it.
-        mock_port.get_option = lambda name: 60 * 1000
-        mock_port.default_test_timeout_ms = lambda: 1000
-        driver1 = MockDriver()
+        driver1 = MockDriver(self.port)
         driver1._start(False, [])
-        driver2 = MockDriver()
+        driver2 = MockDriver(self.port)
         driver2._start(False, [])
         # It's possible for driver1 to timeout when stopping if it's sharing stdin with driver2.
         start_time = time.time()