[GTK] WebDriver: run-webdriver-tests is leaking a DumpRenderTree directory in tmp
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Dec 2017 08:31:38 +0000 (08:31 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Dec 2017 08:31:38 +0000 (08:31 +0000)
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.

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

Tools/ChangeLog
Tools/Scripts/webkitpy/port/headlessdriver.py
Tools/Scripts/webkitpy/port/waylanddriver.py
Tools/Scripts/webkitpy/port/westondriver.py
Tools/Scripts/webkitpy/port/westondriver_unittest.py
Tools/Scripts/webkitpy/port/xorgdriver.py
Tools/Scripts/webkitpy/port/xvfbdriver.py

index e8f7570..19ebec0 100644 (file)
@@ -1,3 +1,39 @@
+2017-12-11  Carlos Garcia Campos  <cgarcia@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-10  Zan Dobersek  <zdobersek@igalia.com>
 
         Unreviewed webkitpy fix on Linux platforms after r225721.
index 9ce57c2..e4684a3 100644 (file)
@@ -41,13 +41,14 @@ class HeadlessDriver(Driver):
         driver_environment = self._port.setup_environ_for_server(self._server_name)
         driver_environment['WPE_USE_HEADLESS_VIEW_BACKEND'] = "1"
         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(HeadlessDriver, 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())
index 91780cb..7023895 100644 (file)
@@ -50,13 +50,14 @@ class WaylandDriver(Driver):
         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())
index 168496c..5ad7e93 100644 (file)
@@ -55,7 +55,6 @@ class WestonDriver(Driver):
         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 @@ class WestonDriver(Driver):
         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 @@ class WestonDriver(Driver):
 
     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 @@ class WestonDriver(Driver):
             # 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
index 6361630..bc71ab9 100644 (file)
@@ -50,8 +50,8 @@ class WestonXvfbDriverDisplayTest():
 
 
 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 @@ class WestonDriverTest(unittest.TestCase):
             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))
index 4d7782c..5333957 100644 (file)
@@ -50,18 +50,19 @@ class XorgDriver(Driver):
         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
index a6f5ad1..ab33d3a 100644 (file)
@@ -99,19 +99,19 @@ class XvfbDriver(Driver):
         # 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())