Attachment 152667 Details for Bug 88478 – Patch

[patch] Patch

bug-88478-20120716180613.patch (text/plain), 20.83 KB, created by Xianzhu Wang on 2012-07-16 18:06:14 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Xianzhu Wang

Created: 2012-07-16 18:06:14 PDT

Size: 20.83 KB

patch

obsolete

>Subversion Revision: 122789
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 499c146fd9951d3b89aa31d94cbb7fe651fd87b5..cb5401b01762354c38f03af1c6175b8a0fd2c619 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,16 @@
>+2012-07-16  Xianzhu Wang  <wangxianzhu@chromium.org>
>+
>+        remove ChromiumDriver from NRWT
>+        https://bugs.webkit.org/show_bug.cgi?id=88478
>+
>+        Now WebKitDriver has replaced ChromiumDriver since test_shell mode is deperecated.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * Scripts/webkitpy/layout_tests/port/chromium.py: Removed ChromiumDriver code.
>+        (ChromiumPort._driver_class): Removed. WebKitPort._driver_class() will be used.
>+        * Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
>+
> 2012-07-16  Don Olmstead  <don.olmstead@am.sony.com>
> 
>         NRWRT Should provide a VS project to work on
>diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium.py
>index 24f7efa0f5c978f1695f82e459a96b5b24814a1d..45298c6348ce2f00f0b04f77a60aae88b7980b80 100755
>--- a/Tools/Scripts/webkitpy/layout_tests/port/chromium.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium.py
>@@ -272,9 +272,6 @@ class ChromiumPort(WebKitPort):
>         except AssertionError:
>             return self._build_path(self.get_option('configuration'), 'layout-test-results')
> 
>-    def _driver_class(self):
>-        return ChromiumDriver
>-
>     def _missing_symbol_to_skipped_tests(self):
>         # FIXME: Should WebKitPort have these definitions also?
>         return {
>@@ -436,283 +433,3 @@ class ChromiumPort(WebKitPort):
>         if sys.platform == 'cygwin':
>             return cygpath(path)
>         return path
>-
>-
>-class ChromiumDriver(WebKitDriver):
>-    KILL_TIMEOUT_DEFAULT = 3.0
>-
>-    def __init__(self, port, worker_number, pixel_tests, no_timeout=False):
>-        WebKitDriver.__init__(self, port, worker_number, pixel_tests, no_timeout)
>-        self._proc = None
>-        self._image_path = None
>-
>-        # FIXME: Delete all of this driver code once we're satisfied that it's not needed any more.
>-        #if port.host.platform.os_version == 'snowleopard':
>-        #    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 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') + '"')
>-        if self._no_timeout:
>-            cmd.append("--no-timeout")
>-
>-        # FIXME: We should be able to build this list using only an array of
>-        # option names, the options (optparse.Values) object, and the orignal
>-        # list of options from the main method by looking up the option
>-        # text from the options list if the value is non-None.
>-        # FIXME: How many of these options are still used?
>-        option_mappings = {
>-            'startup_dialog': '--testshell-startup-dialog',
>-            'gp_fault_error_box': '--gp-fault-error-box',
>-            'stress_opt': '--stress-opt',
>-            'stress_deopt': '--stress-deopt',
>-            'threaded_compositing': '--enable-threaded-compositing',
>-            'accelerated_2d_canvas': '--enable-accelerated-2d-canvas',
>-            'accelerated_painting': '--enable-accelerated-painting',
>-            'accelerated_video': '--enable-accelerated-video',
>-            'enable_hardware_gpu': '--enable-hardware-gpu',
>-            'per_tile_painting': '--enable-per-tile-painting',
>-        }
>-        for nrwt_option, drt_option in option_mappings.items():
>-            if self._port.get_option(nrwt_option):
>-                cmd.append(drt_option)
>-
>-        cmd.extend(self._port.get_option('additional_drt_flag', []))
>-        return cmd
>-
>-    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())
>-        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.
>-        close_fds = sys.platform != 'win32'
>-        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
>-
>-    def _write_command_and_read_line(self, input=None):
>-        """Returns a tuple: (line, did_crash)"""
>-        try:
>-            if input:
>-                if isinstance(input, unicode):
>-                    # DRT expects utf-8
>-                    input = input.encode("utf-8")
>-                self._proc.stdin.write(input)
>-            # DumpRenderTree text output is always UTF-8.  However some tests
>-            # (e.g. webarchive) may spit out binary data instead of text so we
>-            # don't bother to decode the output.
>-            line = self._proc.stdout.readline()
>-            # We could assert() here that line correctly decodes as UTF-8.
>-            return (line, False)
>-        except IOError, e:
>-            _log.error("IOError communicating w/ DRT: " + str(e))
>-            return (None, True)
>-
>-    def _test_shell_command(self, uri, timeoutms, checksum):
>-        cmd = uri
>-        if timeoutms:
>-            cmd += ' ' + str(timeoutms)
>-        if checksum:
>-            cmd += ' ' + checksum
>-        cmd += "\n"
>-        return cmd
>-
>-    def _output_image(self):
>-        if self._image_path and self._port._filesystem.exists(self._image_path):
>-            return self._port._filesystem.read_binary_file(self._image_path)
>-        return None
>-
>-    def _output_image_with_retry(self):
>-        # Retry a few more times because open() sometimes fails on Windows,
>-        # raising "IOError: [Errno 13] Permission denied:"
>-        retry_num = 50
>-        timeout_seconds = 5.0
>-        for _ in range(retry_num):
>-            try:
>-                return self._output_image()
>-            except IOError, e:
>-                if e.errno != errno.EACCES:
>-                    raise e
>-            # FIXME: We should have a separate retry delay.
>-            # This implementation is likely to exceed the timeout before the expected number of retries.
>-            time.sleep(timeout_seconds / retry_num)
>-        return self._output_image()
>-
>-    def _clear_output_image(self):
>-        if self._image_path and self._port._filesystem.exists(self._image_path):
>-            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)
>-
>-        output = []
>-        error = []
>-        crash = False
>-        timeout = False
>-        actual_uri = None
>-        actual_checksum = None
>-        self._clear_output_image()
>-        start_time = time.time()
>-        has_audio = False
>-        has_base64 = False
>-
>-        uri = self.test_to_uri(driver_input.test_name)
>-        cmd = self._test_shell_command(uri, driver_input.timeout, driver_input.image_hash)
>-        line, crash = self._write_command_and_read_line(input=cmd)
>-
>-        while not crash and line.rstrip() != "#EOF":
>-            # Make sure we haven't crashed.
>-            if line == '' and self._proc.poll() is not None:
>-                # This is hex code 0xc000001d, which is used for abrupt
>-                # termination. This happens if we hit ctrl+c from the prompt
>-                # and we happen to be waiting on DRT.
>-                # sdoyon: Not sure for which OS and in what circumstances the
>-                # above code is valid. What works for me under Linux to detect
>-                # ctrl+c is for the subprocess returncode to be negative
>-                # SIGINT. And that agrees with the subprocess documentation.
>-                if (-1073741510 == self._proc.returncode or
>-                    - signal.SIGINT == self._proc.returncode):
>-                    raise KeyboardInterrupt
>-                crash = True
>-                break
>-
>-            # Don't include #URL lines in our output
>-            if line.startswith("#URL:"):
>-                actual_uri = line.rstrip()[5:]
>-                if uri != actual_uri:
>-                    # GURL capitalizes the drive letter of a file URL.
>-                    if (not re.search("^file:///[a-z]:", uri) or uri.lower() != actual_uri.lower()):
>-                        _log.fatal("Test got out of sync:\n|%s|\n|%s|" % (uri, actual_uri))
>-                        raise AssertionError("test out of sync")
>-            elif line.startswith("#MD5:"):
>-                actual_checksum = line.rstrip()[5:]
>-            elif line.startswith("#TEST_TIMED_OUT"):
>-                timeout = True
>-                # Test timed out, but we still need to read until #EOF.
>-            elif line.startswith("Content-Type: audio/wav"):
>-                has_audio = True
>-            elif line.startswith("Content-Transfer-Encoding: base64"):
>-                has_base64 = True
>-            elif line.startswith("Content-Length:"):
>-                pass
>-            elif actual_uri:
>-                output.append(line)
>-            else:
>-                error.append(line)
>-
>-            line, crash = self._write_command_and_read_line(input=None)
>-
>-        if crash and line is not None:
>-            error.append(line)
>-        run_time = time.time() - start_time
>-        output_image = self._output_image_with_retry()
>-
>-        audio_bytes = None
>-        text = None
>-        if has_audio:
>-            if has_base64:
>-                audio_bytes = base64.b64decode(''.join(output))
>-            else:
>-                audio_bytes = ''.join(output).rstrip()
>-        else:
>-            text = ''.join(output)
>-            if not text:
>-                text = None
>-
>-        error = ''.join(error)
>-        # 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:
>-            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, newer_than=start_time)
>-            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)
>-
>-    def start(self, pixel_tests, per_test_args):
>-        if not self._test_shell:
>-            return super(ChromiumDriver, self).start(pixel_tests, per_test_args)
>-
>-        if not self._proc:
>-            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
>-        self._proc.stdin.close()
>-        self._proc.stdout.close()
>-        if self._proc.stderr:
>-            self._proc.stderr.close()
>-        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_timeout_ms()
>-            kill_timeout_seconds = self.KILL_TIMEOUT_DEFAULT * timeout_ratio if timeout_ratio > 1.0 else self.KILL_TIMEOUT_DEFAULT
>-        else:
>-            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
>-        # the harness if DRT is buggy, so we wait a couple
>-        # seconds to give DRT a chance to clean up, but then
>-        # force-kill the process if necessary.
>-        timeout = time.time() + kill_timeout_seconds
>-        while self._proc.poll() is None and time.time() < timeout:
>-            time.sleep(0.1)
>-        if self._proc.poll() is None:
>-            _log.warning('stopping test driver timed out, killing it')
>-            self._port._executive.kill_process(self._proc.pid)
>-        # FIXME: This is sometime None. What is wrong? assert self._proc.poll() is not None
>-        if self._proc.poll() is not None:
>-            self._proc.wait()
>-        self._proc = None
>diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py
>index 0c49112baf9ba9d1cf09d65201500025c093e117..87de41c6cbf485b4ec3e138c0250fecbd76b6458 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py
>@@ -44,141 +44,6 @@ from webkitpy.layout_tests.port import chromium_port_testcase
> from webkitpy.layout_tests.port.driver import DriverInput
> 
> 
>-class ChromiumDriverTest(unittest.TestCase):
>-    def setUp(self):
>-        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"
>-        self.assertEqual(self.driver._test_shell_command("test.html", 2, "checksum"), expected_command)
>-
>-    def _assert_write_command_and_read_line(self, input=None, expected_line=None, expected_stdin=None, expected_crash=False):
>-        if not expected_stdin:
>-            if input:
>-                expected_stdin = input
>-            else:
>-                # We reset stdin, so we should expect stdin.getValue = ""
>-                expected_stdin = ""
>-        self.driver._proc.stdin = StringIO.StringIO()
>-        line, did_crash = self.driver._write_command_and_read_line(input)
>-        self.assertEqual(self.driver._proc.stdin.getvalue(), expected_stdin)
>-        self.assertEqual(line, expected_line)
>-        self.assertEqual(did_crash, expected_crash)
>-
>-    def test_write_command_and_read_line(self):
>-        self.driver._proc = Mock()  # FIXME: This should use a tighter mock.
>-        # Set up to read 3 lines before we get an IOError
>-        self.driver._proc.stdout = StringIO.StringIO("first\nsecond\nthird\n")
>-
>-        unicode_input = u"I \u2661 Unicode"
>-        utf8_input = unicode_input.encode("utf-8")
>-        # Test unicode input conversion to utf-8
>-        self._assert_write_command_and_read_line(input=unicode_input, expected_stdin=utf8_input, expected_line="first\n")
>-        # Test str() input.
>-        self._assert_write_command_and_read_line(input="foo", expected_line="second\n")
>-        # Test input=None
>-        self._assert_write_command_and_read_line(expected_line="third\n")
>-        # Test reading from a closed/empty stream.
>-        # reading from a StringIO does not raise IOError like a real file would, so raise IOError manually.
>-        def mock_readline():
>-            raise IOError
>-        self.driver._proc.stdout.readline = mock_readline
>-        self._assert_write_command_and_read_line(expected_crash=True)
>-
>-    def test_crash_log(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, newer_than: 'mockcrashlog'
>-        driver_output = self.driver.run_test(DriverInput(test_name='some/test.html', timeout=1, image_hash=None, should_run_pixel_test=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')
>-
>-    def test_stop(self):
>-        self.pid = None
>-        self.wait_called = False
>-        self.driver._proc = Mock()  # FIXME: This should use a tighter mock.
>-        self.driver._proc.pid = 1
>-        self.driver._proc.stdin = StringIO.StringIO()
>-        self.driver._proc.stdout = StringIO.StringIO()
>-        self.driver._proc.stderr = StringIO.StringIO()
>-        self.driver._proc.poll = lambda: None
>-
>-        def fake_wait():
>-            self.assertTrue(self.pid is not None)
>-            self.wait_called = True
>-
>-        self.driver._proc.wait = fake_wait
>-
>-        class FakeExecutive(object):
>-            def kill_process(other, pid):
>-                self.pid = pid
>-                self.driver._proc.poll = lambda: 2
>-
>-        self.driver._port._executive = FakeExecutive()
>-        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):
>-
>-        class MockDriver(chromium.ChromiumDriver):
>-            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.
>-        driver1 = MockDriver(self.port)
>-        driver1._start(False, [])
>-        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()
>-        driver1.stop()
>-        driver2.stop()
>-        self.assertTrue(time.time() - start_time < 20)
>-
>-    def test_stop_cleans_up_properly(self):
>-        self.driver._test_shell = False
>-        self.driver.start(True, [])
>-        last_tmpdir = self.port._filesystem.last_tmpdir
>-        self.assertNotEquals(last_tmpdir, None)
>-        self.driver.stop()
>-        self.assertFalse(self.port._filesystem.isdir(last_tmpdir))
>-
>-    def test_two_starts_cleans_up_properly(self):
>-        # clone the WebKitDriverTest tests here since we override start() and stop()
>-        self.driver._test_shell = False
>-        self.driver.start(True, [])
>-        last_tmpdir = self.port._filesystem.last_tmpdir
>-        self.driver._start(True, [])
>-        self.assertFalse(self.port._filesystem.isdir(last_tmpdir))
>-
>-    def test_expectations_dict(self):
>-        self.port._filesystem.write_text_file('/mock-checkout/LayoutTests/platform/chromium/TestExpectations', 'upstream')
>-        self.port._filesystem.write_text_file('/mock-checkout/Source/WebKit/chromium/webkit/tools/layout_tests/test_expectations.txt', 'downstream')
>-        self.assertEquals('\n'.join(self.port.expectations_dict().values()), 'upstream\ndownstream')
>-
>-        self.port._filesystem.write_text_file(self.port.path_from_chromium_base('skia', 'skia_test_expectations.txt'), 'skia')
>-        self.assertEquals('\n'.join(self.port.expectations_dict().values()), 'upstream\nskia\ndownstream')
>-
>-
> class ChromiumPortLoggingTest(logtesting.LoggingTestCase):
> 
>     # FIXME: put this someplace more useful

Actions: View | Formatted Diff | Diff

Attachments on bug 88478: 152667