Title: [226056] releases/WebKitGTK/webkit-2.18/Tools
Revision
226056
Author
carlo...@webkit.org
Date
2017-12-18 09:40:54 -0800 (Mon, 18 Dec 2017)

Log Message

Merge r225740 - [GTK] WebDriver: run-webdriver-tests is leaking a DumpRenderTree directory in tmp
https://bugs.webkit.org/show_bug.cgi?id=180426

Reviewed by Michael Catanzaro.

This happens when running the tests with Xvfb driver, because _setup_environ_for_test() is called twice and the
DTR temp directory is created there every time. Only the last directory created is cleaned up by the driver
destructor. The DRT temp directory is only needed for layout tests, so we could even avoid its creation by
moving it to the start() method like other drivers do (included the base driver implementation). Since API and
WebDriver tests don't call start(), the directory is not even created, and the required env vars are not set
either in that case. Weston driver was behaving differently for some reason, it's now consistent with all other
drivers.

* Scripts/webkitpy/port/headlessdriver.py:
(HeadlessDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
(HeadlessDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
* Scripts/webkitpy/port/waylanddriver.py:
(WaylandDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
(WaylandDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
* Scripts/webkitpy/port/westondriver.py:
(WestonDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
(WestonDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
(WestonDriver.stop): Do not delete the temp directory, it's done by the parent class.
(WestonDriver._ensure_driver_tmpdir_subdirectory): Deleted.
* Scripts/webkitpy/port/westondriver_unittest.py:
(WestonDriverTest.make_driver):
(WestonDriverTest.test_stop):
* Scripts/webkitpy/port/xorgdriver.py:
(XorgDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
(XorgDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
* Scripts/webkitpy/port/xvfbdriver.py:
(XvfbDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
(XvfbDriver._start): Use Port._driver_tempdir() to create the driver temp directory.

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.18/Tools/ChangeLog (226055 => 226056)


--- releases/WebKitGTK/webkit-2.18/Tools/ChangeLog	2017-12-18 17:40:49 UTC (rev 226055)
+++ releases/WebKitGTK/webkit-2.18/Tools/ChangeLog	2017-12-18 17:40:54 UTC (rev 226056)
@@ -1,3 +1,39 @@
+2017-12-11  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        [GTK] WebDriver: run-webdriver-tests is leaking a DumpRenderTree directory in tmp
+        https://bugs.webkit.org/show_bug.cgi?id=180426
+
+        Reviewed by Michael Catanzaro.
+
+        This happens when running the tests with Xvfb driver, because _setup_environ_for_test() is called twice and the
+        DTR temp directory is created there every time. Only the last directory created is cleaned up by the driver
+        destructor. The DRT temp directory is only needed for layout tests, so we could even avoid its creation by
+        moving it to the start() method like other drivers do (included the base driver implementation). Since API and
+        WebDriver tests don't call start(), the directory is not even created, and the required env vars are not set
+        either in that case. Weston driver was behaving differently for some reason, it's now consistent with all other
+        drivers.
+
+        * Scripts/webkitpy/port/headlessdriver.py:
+        (HeadlessDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
+        (HeadlessDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
+        * Scripts/webkitpy/port/waylanddriver.py:
+        (WaylandDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
+        (WaylandDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
+        * Scripts/webkitpy/port/westondriver.py:
+        (WestonDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
+        (WestonDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
+        (WestonDriver.stop): Do not delete the temp directory, it's done by the parent class.
+        (WestonDriver._ensure_driver_tmpdir_subdirectory): Deleted.
+        * Scripts/webkitpy/port/westondriver_unittest.py:
+        (WestonDriverTest.make_driver):
+        (WestonDriverTest.test_stop):
+        * Scripts/webkitpy/port/xorgdriver.py:
+        (XorgDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
+        (XorgDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
+        * Scripts/webkitpy/port/xvfbdriver.py:
+        (XvfbDriver._setup_environ_for_test): Do not set DUMPRENDERTREE_TEMP and XDG_CACHE_HOME if _driver_tempdir is None.
+        (XvfbDriver._start): Use Port._driver_tempdir() to create the driver temp directory.
+
 2017-12-01  Carlos Garcia Campos  <cgar...@igalia.com>
 
         WebDriver: auto-install pytest instead of importing it from wpt tools directory

Modified: releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/waylanddriver.py (226055 => 226056)


--- releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/waylanddriver.py	2017-12-18 17:40:49 UTC (rev 226055)
+++ releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/waylanddriver.py	2017-12-18 17:40:54 UTC (rev 226056)
@@ -50,13 +50,14 @@
         self._port._copy_value_from_environ_if_set(driver_environment, 'WAYLAND_SOCKET')
         driver_environment['GDK_BACKEND'] = 'wayland'
         driver_environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir()
-        driver_environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
-        driver_environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
+        if self._driver_tempdir is not None:
+            driver_environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
+            driver_environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
         return driver_environment
 
     def _start(self, pixel_tests, per_test_args):
         super(WaylandDriver, self).stop()
-        self._driver_tempdir = self._port.host.filesystem.mkdtemp(prefix='%s-' % self._server_name)
+        self._driver_tempdir = self._port._driver_tempdir(self._target_host)
         self._crashed_process_name = None
         self._crashed_pid = None
         self._server_process = self._port._server_process_constructor(self._port, self._server_name, self.cmd_line(pixel_tests, per_test_args), self._setup_environ_for_test())

Modified: releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/westondriver.py (226055 => 226056)


--- releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/westondriver.py	2017-12-18 17:40:49 UTC (rev 226055)
+++ releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/westondriver.py	2017-12-18 17:40:54 UTC (rev 226056)
@@ -55,7 +55,6 @@
         self._xvfbdriver = XvfbDriver(*args, **kwargs)
 
     def _setup_environ_for_test(self):
-        self._driver_directory = self._port.host.filesystem.mkdtemp(prefix='%s-' % self._server_name)
         driver_environment = self._port.setup_environ_for_server(self._server_name)
         driver_environment['DISPLAY'] = ":%d" % self._xvfbdriver._xvfb_run(driver_environment)
         weston_socket = 'WKTesting-weston-%032x' % random.getrandbits(128)
@@ -69,10 +68,11 @@
         time.sleep(self._startup_delay_secs)
 
         driver_environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir()
-        # Currently on WebKit2, there is no API for setting the application cache directory.
-        # Each worker should have its own and it should be cleaned afterwards, when the worker stops.
-        driver_environment['XDG_CACHE_HOME'] = self._ensure_driver_tmpdir_subdirectory('appcache')
-        driver_environment['DUMPRENDERTREE_TEMP'] = self._ensure_driver_tmpdir_subdirectory('drt-temp')
+        if self._driver_tempdir is not None:
+            # Currently on WebKit2, there is no API for setting the application cache directory.
+            # Each worker should have its own and it should be cleaned afterwards, when the worker stops.
+            driver_environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
+            driver_environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
         driver_environment['WAYLAND_DISPLAY'] = weston_socket
         driver_environment['GDK_BACKEND'] = 'wayland'
         if driver_environment.get('DISPLAY'):
@@ -81,6 +81,7 @@
 
     def _start(self, pixel_tests, per_test_args):
         self.stop()
+        self._driver_tempdir = self._port._driver_tempdir(self._target_host)
         self._crashed_process_name = None
         self._crashed_pid = None
         self._server_process = self._port._server_process_constructor(self._port, self._server_name, self.cmd_line(pixel_tests, per_test_args), self._setup_environ_for_test())
@@ -92,11 +93,4 @@
             # The Weston process is terminated instead of killed, giving the Weston a chance to clean up after itself.
             self._weston_process.terminate()
             self._weston_process = None
-        if getattr(self, '_driver_directory', None):
-            self._port.host.filesystem.rmtree(str(self._driver_directory))
 
-    def _ensure_driver_tmpdir_subdirectory(self, subdirectory):
-        assert getattr(self, '_driver_directory', None)
-        directory_path = self._port.host.filesystem.join(str(self._driver_directory), subdirectory)
-        self._port.host.filesystem.maybe_make_directory(directory_path)
-        return directory_path

Modified: releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/westondriver_unittest.py (226055 => 226056)


--- releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/westondriver_unittest.py	2017-12-18 17:40:49 UTC (rev 226055)
+++ releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/westondriver_unittest.py	2017-12-18 17:40:54 UTC (rev 226056)
@@ -50,8 +50,8 @@
 
 
 class WestonDriverTest(unittest.TestCase):
-    def make_driver(self, filesystem=None):
-        port = Port(MockSystemHost(log_executive=True, filesystem=filesystem), 'westondrivertestport', options=MockOptions(configuration='Release'))
+    def make_driver(self):
+        port = Port(MockSystemHost(log_executive=True), 'westondrivertestport', options=MockOptions(configuration='Release'))
         port._config.build_directory = lambda configuration: "/mock_build"
         port._server_process_constructor = MockServerProcess
 
@@ -85,13 +85,10 @@
             def terminate(self):
                 _log.info("MOCK FakeWestonProcess.terminate")
 
-        filesystem = MockFileSystem(dirs=['/tmp/weston-driver-directory'])
-        driver = self.make_driver(filesystem)
+        driver = self.make_driver()
         driver._weston_process = FakeWestonProcess()
-        driver._driver_directory = '/tmp/weston-driver-directory'
 
         expected_logs = "MOCK FakeWestonProcess.terminate\n"
         OutputCapture().assert_outputs(self, driver.stop, [], expected_logs=expected_logs)
 
         self.assertIsNone(driver._weston_process)
-        self.assertFalse(driver._port._filesystem.exists(driver._driver_directory))

Modified: releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/xorgdriver.py (226055 => 226056)


--- releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/xorgdriver.py	2017-12-18 17:40:49 UTC (rev 226055)
+++ releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/xorgdriver.py	2017-12-18 17:40:54 UTC (rev 226056)
@@ -50,18 +50,19 @@
         self._port._copy_value_from_environ_if_set(server_environment, 'XAUTHORITY')
         server_environment['GDK_BACKEND'] = 'x11'
         server_environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir()
-        server_environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
-        # Currently on WebKit2, there is no API for setting the application
-        # cache directory. Each worker should have it's own and it should be
-        # cleaned afterwards, so we set it to inside the temporary folder by
-        # prepending XDG_CACHE_HOME with DUMPRENDERTREE_TEMP.
-        server_environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
+        if self._driver_tempdir is not None:
+            server_environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
+            # Currently on WebKit2, there is no API for setting the application
+            # cache directory. Each worker should have it's own and it should be
+            # cleaned afterwards, so we set it to inside the temporary folder by
+            # prepending XDG_CACHE_HOME with DUMPRENDERTREE_TEMP.
+            server_environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
         return server_environment
 
     def _start(self, pixel_tests, per_test_args):
         super(XorgDriver, self).stop()
 
-        self._driver_tempdir = self._port.host.filesystem.mkdtemp(prefix='%s-' % self._server_name)
+        self._driver_tempdir = self._port._driver_tempdir(self._target_host)
 
         self._crashed_process_name = None
         self._crashed_pid = None

Modified: releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/xvfbdriver.py (226055 => 226056)


--- releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/xvfbdriver.py	2017-12-18 17:40:49 UTC (rev 226055)
+++ releases/WebKitGTK/webkit-2.18/Tools/Scripts/webkitpy/port/xvfbdriver.py	2017-12-18 17:40:54 UTC (rev 226056)
@@ -99,19 +99,19 @@
         # We must do this here because the DISPLAY number depends on _worker_number
         environment['DISPLAY'] = ":%d" % display_id
         environment['GDK_BACKEND'] = 'x11'
-        self._driver_tempdir = self._port.host.filesystem.mkdtemp(prefix='%s-' % self._server_name)
-        environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
         environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir()
-
-        # Currently on WebKit2, there is no API for setting the application
-        # cache directory. Each worker should have it's own and it should be
-        # cleaned afterwards, so we set it to inside the temporary folder by
-        # prepending XDG_CACHE_HOME with DUMPRENDERTREE_TEMP.
-        environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
+        if self._driver_tempdir is not None:
+            environment['DUMPRENDERTREE_TEMP'] = str(self._driver_tempdir)
+            # Currently on WebKit2, there is no API for setting the application
+            # cache directory. Each worker should have it's own and it should be
+            # cleaned afterwards, so we set it to inside the temporary folder by
+            # prepending XDG_CACHE_HOME with DUMPRENDERTREE_TEMP.
+            environment['XDG_CACHE_HOME'] = self._port.host.filesystem.join(str(self._driver_tempdir), 'appcache')
         return environment
 
     def _start(self, pixel_tests, per_test_args):
         self.stop()
+        self._driver_tempdir = self._port._driver_tempdir(self._target_host)
         self._crashed_process_name = None
         self._crashed_pid = None
         self._server_process = self._port._server_process_constructor(self._port, self._server_name, self.cmd_line(pixel_tests, per_test_args), self._setup_environ_for_test())
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to