new-run-webkit-tests should only enable MallocStackLogging for DRT
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Jul 2011 22:56:06 +0000 (22:56 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Jul 2011 22:56:06 +0000 (22:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=64792

Reviewed by Adam Barth.

The previous code would enable it for all servers launched
by the port, which included Apache, the python websocket server
as well as ImageDiff.  Now only DumpRenderTree will have
MallocStackLogging enabled or the GuardMalloc library injected.

I also cleaned up the websocket_server code to use filesystem
while I was in it.

I also made DRT restart every 1000 tests when running with
--leaks enabled.  I believe this made the --leaks run slightly
faster, but it still takes over an hour on my machine. :(

* Scripts/webkitpy/layout_tests/controllers/worker.py:
* Scripts/webkitpy/layout_tests/port/base.py:
* Scripts/webkitpy/layout_tests/port/chromium_win.py:
* Scripts/webkitpy/layout_tests/port/gtk.py:
* Scripts/webkitpy/layout_tests/port/mac.py:
* Scripts/webkitpy/layout_tests/port/qt.py:
* Scripts/webkitpy/layout_tests/port/server_process.py:
* Scripts/webkitpy/layout_tests/port/webkit.py:
* Scripts/webkitpy/layout_tests/servers/http_server.py:
* Scripts/webkitpy/layout_tests/servers/websocket_server.py:

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

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/controllers/worker.py
Tools/Scripts/webkitpy/layout_tests/port/base.py
Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py
Tools/Scripts/webkitpy/layout_tests/port/gtk.py
Tools/Scripts/webkitpy/layout_tests/port/mac.py
Tools/Scripts/webkitpy/layout_tests/port/qt.py
Tools/Scripts/webkitpy/layout_tests/port/server_process.py
Tools/Scripts/webkitpy/layout_tests/port/webkit.py
Tools/Scripts/webkitpy/layout_tests/servers/http_server.py
Tools/Scripts/webkitpy/layout_tests/servers/websocket_server.py

index 76aadff..6c2fcde 100644 (file)
@@ -1,3 +1,33 @@
+2011-07-19  Eric Seidel  <eric@webkit.org>
+
+        new-run-webkit-tests should only enable MallocStackLogging for DRT
+        https://bugs.webkit.org/show_bug.cgi?id=64792
+
+        Reviewed by Adam Barth.
+
+        The previous code would enable it for all servers launched
+        by the port, which included Apache, the python websocket server
+        as well as ImageDiff.  Now only DumpRenderTree will have
+        MallocStackLogging enabled or the GuardMalloc library injected.
+
+        I also cleaned up the websocket_server code to use filesystem
+        while I was in it.
+
+        I also made DRT restart every 1000 tests when running with
+        --leaks enabled.  I believe this made the --leaks run slightly
+        faster, but it still takes over an hour on my machine. :(
+
+        * Scripts/webkitpy/layout_tests/controllers/worker.py:
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/layout_tests/port/chromium_win.py:
+        * Scripts/webkitpy/layout_tests/port/gtk.py:
+        * Scripts/webkitpy/layout_tests/port/mac.py:
+        * Scripts/webkitpy/layout_tests/port/qt.py:
+        * Scripts/webkitpy/layout_tests/port/server_process.py:
+        * Scripts/webkitpy/layout_tests/port/webkit.py:
+        * Scripts/webkitpy/layout_tests/servers/http_server.py:
+        * Scripts/webkitpy/layout_tests/servers/websocket_server.py:
+
 2011-07-19  Ojan Vafai  <ojan@chromium.org>
 
         remove the concept of platform fallbacks
index 5178042..a6dd94e 100644 (file)
@@ -68,8 +68,7 @@ class Worker(manager_worker_broker.AbstractWorker):
         self._filesystem = port.filesystem
         self._batch_count = 0
         self._batch_size = self._options.batch_size
-        tests_run_filename = self._filesystem.join(port.results_directory(),
-                                                   "tests_run%d.txt" % self._worker_number)
+        tests_run_filename = self._filesystem.join(port.results_directory(), "tests_run%d.txt" % self._worker_number)
         self._tests_run_file = self._filesystem.open_text_file_for_writing(tests_run_filename)
 
     def cancel(self):
index 360bd6d..82aa077 100755 (executable)
@@ -234,7 +234,8 @@ class Port(object):
             httpd_path = self._path_to_lighttpd()
 
         try:
-            env = self.setup_environ_for_server()
+            server_name = self._filesystem.basename(httpd_path)
+            env = self.setup_environ_for_server(server_name)
             return self._executive.run_command([httpd_path, "-v"], env=env, return_exit_code=True) == 0
         except OSError:
             _log.error("No httpd found. Cannot run http tests.")
@@ -662,7 +663,7 @@ class Port(object):
         """Perform port-specific work at the beginning of a test run."""
         pass
 
-    def setup_environ_for_server(self):
+    def setup_environ_for_server(self, server_name=None):
         """Perform port-specific work at the beginning of a server launch.
 
         Returns:
index 3512991..51b25b3 100755 (executable)
@@ -98,7 +98,7 @@ class ChromiumWinPort(chromium.ChromiumPort):
             assert self._version in self.SUPPORTED_VERSIONS, "%s is not in %s" % (self._version, self.SUPPORTED_VERSIONS)
         self._operating_system = 'win'
 
-    def setup_environ_for_server(self):
+    def setup_environ_for_server(self, server_name=None):
         env = chromium.ChromiumPort.setup_environ_for_server(self)
         # Put the cygwin directory first in the path to find cygwin1.dll.
         env["PATH"] = "%s;%s" % (self.path_from_chromium_base("third_party", "cygwin", "bin"), env["PATH"])
index 574c291..76b3e19 100644 (file)
@@ -44,10 +44,11 @@ class GtkDriver(webkit.WebKitDriver):
         display_id = self._worker_number + 1
         run_xvfb = ["Xvfb", ":%d" % (display_id), "-screen",  "0", "800x600x24", "-nolisten", "tcp"]
         self._xvfb_process = subprocess.Popen(run_xvfb)
-        environment = self._port.setup_environ_for_server()
+        server_name = self._port.driver_name()
+        environment = self._port.setup_environ_for_server(server_name)
         # We must do this here because the DISPLAY number depends on _worker_number
         environment['DISPLAY'] = ":%d" % (display_id)
-        self._server_process = server_process.ServerProcess(self._port, self._port.driver_name(), self.cmd_line(), environment)
+        self._server_process = server_process.ServerProcess(self._port, server_name, self.cmd_line(), environment)
 
     def stop(self):
         webkit.WebKitDriver.stop(self)
@@ -64,8 +65,8 @@ class GtkPort(webkit.WebKitPort):
     def create_driver(self, worker_number):
         return GtkDriver(self, worker_number)
 
-    def setup_environ_for_server(self):
-        environment = webkit.WebKitPort.setup_environ_for_server(self)
+    def setup_environ_for_server(self, server_name=None):
+        environment = webkit.WebKitPort.setup_environ_for_server(self, server_name)
         environment['GTK_MODULES'] = 'gail'
         environment['LIBOVERLAY_SCROLLBAR'] = '0'
         environment['WEBKIT_INSPECTOR_PATH'] = self._build_path('Programs/resources/inspector')
index 6ecfa7e..7cc5f3b 100644 (file)
@@ -118,7 +118,10 @@ class LeakDetector(object):
     def check_for_leaks(self, process_name, process_pid):
         _log.debug("Checking for leaks in %s" % process_name)
         try:
-            leaks_output = self._port._run_script("run-leaks", self._leaks_args(process_pid), include_configuration_arguments=False)
+            # Oddly enough, run-leaks (or the underlying leaks tool) does not seem to always output utf-8,
+            # thus we pass decode_output=False.  Without this code we've seen errors like:
+            # "UnicodeDecodeError: 'utf8' codec can't decode byte 0x88 in position 779874: unexpected code byte"
+            leaks_output = self._port._run_script("run-leaks", self._leaks_args(process_pid), include_configuration_arguments=False, decode_output=False)
         except ScriptError, e:
             _log.warn("Failed to run leaks tool: %s" % e.message_with_output())
             return
@@ -130,7 +133,7 @@ class LeakDetector(object):
 
         leaks_filename = self.leaks_file_name(process_name, process_pid)
         leaks_output_path = self._filesystem.join(self._port.results_directory(), leaks_filename)
-        self._filesystem.write_text_file(leaks_output_path, leaks_output)
+        self._filesystem.write_binary_file(leaks_output_path, leaks_output)
 
         # FIXME: Ideally we would not be logging from the worker process, but rather pass the leak
         # information back to the manager and have it log.
@@ -195,6 +198,10 @@ class MacPort(WebKitPort):
             assert self._version in self.SUPPORTED_VERSIONS, "%s is not in %s" % (self._version, self.SUPPORTED_VERSIONS)
         self._operating_system = 'mac'
         self._leak_detector = LeakDetector(self)
+        if self.get_option("leaks"):
+            # DumpRenderTree slows down noticably if we run more than about 1000 tests in a batch
+            # with MallocStackLogging enabled.
+            self.set_option_default("batch_size", 1000)
 
     def baseline_search_path(self):
         search_paths = self.FALLBACK_PATHS[self._version]
@@ -202,12 +209,13 @@ class MacPort(WebKitPort):
             search_paths.insert(0, self._wk2_port_name())
         return map(self._webkit_baseline_path, search_paths)
 
-    def setup_environ_for_server(self):
-        env = WebKitPort.setup_environ_for_server(self)
-        if self.get_option('leaks'):
-            env['MallocStackLogging'] = '1'
-        if self.get_option('guard_malloc'):
-            env['DYLD_INSERT_LIBRARIES'] = '/usr/lib/libgmalloc.dylib'
+    def setup_environ_for_server(self, server_name=None):
+        env = WebKitPort.setup_environ_for_server(self, server_name)
+        if server_name == self.driver_name():
+            if self.get_option('leaks'):
+                env['MallocStackLogging'] = '1'
+            if self.get_option('guard_malloc'):
+                env['DYLD_INSERT_LIBRARIES'] = '/usr/lib/libgmalloc.dylib'
         return env
 
     # Belongs on a Platform object.
@@ -244,7 +252,7 @@ class MacPort(WebKitPort):
         leaks_files = self._leak_detector.leaks_files_in_directory(self.results_directory())
         if not leaks_files:
             return
-        total_bytes, unique_leaks = self.parse_leak_files(leaks_files)
+        total_bytes, unique_leaks = self._leak_detector.parse_leak_files(leaks_files)
         _log.info("%s total leaks found for a total of %s!" % (self._total_leaks, total_bytes))
         _log.info("%s unique leaks found!" % unique_leaks)
 
index fdaba1c..793a60f 100644 (file)
@@ -82,8 +82,8 @@ class QtPort(WebKitPort):
     def _runtime_feature_list(self):
         return None
 
-    def setup_environ_for_server(self):
-        env = WebKitPort.setup_environ_for_server(self)
+    def setup_environ_for_server(self, server_name=None):
+        env = WebKitPort.setup_environ_for_server(self, server_name)
         env['QTWEBKIT_PLUGIN_PATH'] = self._build_path('lib/plugins')
         return env
 
index e6b161e..77cca6e 100644 (file)
@@ -124,6 +124,7 @@ class ServerProcess:
             self._proc.stdin.write(input)
         except IOError, e:
             self.stop()
+            # stop() calls _reset(), so we have to set crashed to True after calling stop().
             self.crashed = True
 
     def read_line(self, timeout):
@@ -160,8 +161,7 @@ class ServerProcess:
                 string is returned.
         """
         if size <= 0:
-            raise ValueError('ServerProcess.read() called with a '
-                             'non-positive size: %d ' % size)
+            raise ValueError('ServerProcess.read() called with a non-positive size: %d ' % size)
         return self._read(timeout, size)
 
     def _check_for_crash(self):
@@ -236,7 +236,9 @@ class ServerProcess:
         if not self._proc:
             return
 
-        self._port.check_for_leaks(self.name(), self.pid())
+        # Only bother to check for leaks if the process is still running.
+        if self.poll() is None:
+            self._port.check_for_leaks(self.name(), self.pid())
 
         pid = self._proc.pid
         self._proc.stdin.close()
index 1c47c64..a1395e9 100644 (file)
@@ -111,13 +111,13 @@ class WebKitPort(Port):
             config_args.append(port_flag)
         return config_args
 
-    def _run_script(self, script_name, args=None, include_configuration_arguments=True):
+    def _run_script(self, script_name, args=None, include_configuration_arguments=True, decode_output=True):
         run_script_command = [self._config.script_path(script_name)]
         if include_configuration_arguments:
             run_script_command.extend(self._arguments_for_configuration())
         if args:
             run_script_command.extend(args)
-        return self._executive.run_command(run_script_command, cwd=self._config.webkit_base_dir())
+        return self._executive.run_command(run_script_command, cwd=self._config.webkit_base_dir(), decode_output=decode_output)
 
     def _build_driver(self):
         try:
@@ -444,11 +444,12 @@ class WebKitDriver(Driver):
         return cmd
 
     def start(self):
-        environment = self._port.setup_environ_for_server()
+        server_name = self._port.driver_name()
+        environment = self._port.setup_environ_for_server(server_name)
         environment['DYLD_FRAMEWORK_PATH'] = self._port._build_path()
         # FIXME: We're assuming that WebKitTestRunner checks this DumpRenderTree-named environment variable.
         environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
-        self._server_process = server_process.ServerProcess(self._port, self._port.driver_name(), self.cmd_line(), environment)
+        self._server_process = server_process.ServerProcess(self._port, server_name, self.cmd_line(), environment)
 
     def poll(self):
         return self._server_process.poll()
index 009378c..701e7bd 100755 (executable)
@@ -180,7 +180,7 @@ class Lighttpd(http_server_base.HttpServerBase):
                                       os.path.join(tmp_module_path, lib_file))
 
         self._start_cmd = start_cmd
-        self._env = self._port_obj.setup_environ_for_server()
+        self._env = self._port_obj.setup_environ_for_server('lighttpd')
         self._mappings = mappings
 
     def _remove_stale_logs(self):
index e40bd10..e6a6352 100644 (file)
@@ -82,16 +82,15 @@ class PyWebSocket(http_server.Lighttpd):
             self._pid_file = self._filesystem.join(self._runtime_path, '%s.pid' % self._name)
 
         # Webkit tests
+        # FIXME: This is the wrong way to detect if we're in Chrome vs. WebKit!
+        # The port objects are supposed to abstract this.
         if self._root:
-            self._layout_tests = os.path.abspath(self._root)
-            self._web_socket_tests = os.path.abspath(
-                os.path.join(self._root, 'http', 'tests',
-                             'websocket', 'tests'))
+            self._layout_tests = self._filesystem.abspath(self._root)
+            self._web_socket_tests = self._filesystem.abspath(self._filesystem.join(self._root, 'http', 'tests', 'websocket', 'tests'))
         else:
             try:
                 self._layout_tests = self._port_obj.layout_tests_dir()
-                self._web_socket_tests = os.path.join(self._layout_tests,
-                     'http', 'tests', 'websocket', 'tests')
+                self._web_socket_tests = self._filesystem.join(self._layout_tests, 'http', 'tests', 'websocket', 'tests')
             except:
                 self._web_socket_tests = None
 
@@ -103,34 +102,32 @@ class PyWebSocket(http_server.Lighttpd):
     def _prepare_config(self):
         time_str = time.strftime('%d%b%Y-%H%M%S')
         log_file_name = self._log_prefix + time_str
+        # FIXME: Doesn't Executive have a devnull, so that we don't have to use os.devnull directly?
         self._wsin = open(os.devnull, 'r')
 
-        error_log = os.path.join(self._output_dir, log_file_name + "-err.txt")
-
-        output_log = os.path.join(self._output_dir, log_file_name + "-out.txt")
+        error_log = self._filesystem.join(self._output_dir, log_file_name + "-err.txt")
+        output_log = self._filesystem.join(self._output_dir, log_file_name + "-out.txt")
         self._wsout = self._filesystem.open_text_file_for_writing(output_log)
 
         from webkitpy.thirdparty.autoinstalled.pywebsocket import mod_pywebsocket
         python_interp = sys.executable
-        pywebsocket_base = os.path.join(
-            os.path.dirname(os.path.dirname(os.path.dirname(
-            os.path.abspath(__file__)))), 'thirdparty',
-            'autoinstalled', 'pywebsocket')
-        pywebsocket_script = os.path.join(pywebsocket_base, 'mod_pywebsocket',
-            'standalone.py')
+        # FIXME: Use self._filesystem.path_to_module(self.__module__) instead of __file__
+        # I think this is trying to get the chrome directory?  Doesn't the port object know that?
+        pywebsocket_base = self._filesystem.join(self._filesystem.dirname(self._filesystem.dirname(self._filesystem.dirname(self._filesystem.abspath(__file__)))), 'thirdparty', 'autoinstalled', 'pywebsocket')
+        pywebsocket_script = self._filesystem.join(pywebsocket_base, 'mod_pywebsocket', 'standalone.py')
         start_cmd = [
             python_interp, '-u', pywebsocket_script,
             '--server-host', '127.0.0.1',
             '--port', str(self._port),
-            '--document-root', os.path.join(self._layout_tests, 'http', 'tests'),
+            # FIXME: Don't we have a self._port_obj.layout_test_path?
+            '--document-root', self._filesystem.join(self._layout_tests, 'http', 'tests'),
             '--scan-dir', self._web_socket_tests,
             '--cgi-paths', '/websocket/tests',
             '--log-file', error_log,
         ]
 
-        handler_map_file = os.path.join(self._web_socket_tests,
-                                        'handler_map.txt')
-        if os.path.exists(handler_map_file):
+        handler_map_file = self._filesystem.join(self._web_socket_tests, 'handler_map.txt')
+        if self._filesystem.exists(handler_map_file):
             _log.debug('Using handler_map_file: %s' % handler_map_file)
             start_cmd.append('--websock-handlers-map-file')
             start_cmd.append(handler_map_file)
@@ -142,7 +139,8 @@ class PyWebSocket(http_server.Lighttpd):
                               '-c', self._certificate])
 
         self._start_cmd = start_cmd
-        self._env = self._port_obj.setup_environ_for_server()
+        server_name = self._filesystem.basename(pywebsocket_script)
+        self._env = self._port_obj.setup_environ_for_server(server_name)
         self._env['PYTHONPATH'] = (pywebsocket_base + os.path.pathsep + self._env.get('PYTHONPATH', ''))
 
     def _remove_stale_logs(self):