Title: [281874] trunk/Tools
Revision
281874
Author
clo...@igalia.com
Date
2021-09-01 13:56:14 -0700 (Wed, 01 Sep 2021)

Log Message

Unreviewed, reverting r281870.

It broke the GTK api test runner.

Reverted changeset:

"[GTK] The Xvfb display server may fail to start sometimes
causing tests to randomly crash"
https://bugs.webkit.org/show_bug.cgi?id=229758
https://commits.webkit.org/r281870

Modified Paths

Removed Paths

Diff

Modified: trunk/Tools/ChangeLog (281873 => 281874)


--- trunk/Tools/ChangeLog	2021-09-01 20:44:40 UTC (rev 281873)
+++ trunk/Tools/ChangeLog	2021-09-01 20:56:14 UTC (rev 281874)
@@ -1,3 +1,16 @@
+2021-09-01  Carlos Alberto Lopez Perez  <clo...@igalia.com>
+
+        Unreviewed, reverting r281870.
+
+        It broke the GTK api test runner.
+
+        Reverted changeset:
+
+        "[GTK] The Xvfb display server may fail to start sometimes
+        causing tests to randomly crash"
+        https://bugs.webkit.org/show_bug.cgi?id=229758
+        https://commits.webkit.org/r281870
+
 2021-09-01  Jonathan Bedard  <jbed...@apple.com>
 
         [git-webkit] Automatic rebasing or pull-requests (Follow-up fix.)

Modified: trunk/Tools/Scripts/webkitpy/common/system/executive_mock.py (281873 => 281874)


--- trunk/Tools/Scripts/webkitpy/common/system/executive_mock.py	2021-09-01 20:44:40 UTC (rev 281873)
+++ trunk/Tools/Scripts/webkitpy/common/system/executive_mock.py	2021-09-01 20:56:14 UTC (rev 281874)
@@ -37,12 +37,12 @@
 
 
 class MockProcess(object):
-    def __init__(self, stdout='MOCK STDOUT\n', stderr='', returncode=0):
+    def __init__(self, stdout='MOCK STDOUT\n', stderr=''):
         self.pid = 42
         self.stdout = BytesIO(string_utils.encode(stdout))
         self.stderr = BytesIO(string_utils.encode(stderr))
         self.stdin = BytesIO()
-        self.returncode = returncode
+        self.returncode = 0
         self._is_running = False
 
     def wait(self):
@@ -49,11 +49,9 @@
         self._is_running = False
         return self.returncode
 
-    def communicate(self, input=None, timeout=None):
+    def communicate(self, input=None):
         self._is_running = False
-        stdout = self.stdout.read() if isinstance(self.stdout, BytesIO) else self.stdout
-        stderr = self.stderr.read() if isinstance(self.stderr, BytesIO) else self.stderr
-        return (stdout, stderr)
+        return (self.stdout, self.stderr)
 
     def poll(self):
         if self._is_running:

Modified: trunk/Tools/Scripts/webkitpy/port/westondriver.py (281873 => 281874)


--- trunk/Tools/Scripts/webkitpy/port/westondriver.py	2021-09-01 20:44:40 UTC (rev 281873)
+++ trunk/Tools/Scripts/webkitpy/port/westondriver.py	2021-09-01 20:56:14 UTC (rev 281874)
@@ -56,18 +56,7 @@
 
     def _setup_environ_for_test(self):
         driver_environment = super(WestonDriver, self)._setup_environ_for_test()
-        xvfb_display_id = self._xvfbdriver._xvfb_run(driver_environment)
-        driver_environment['DISPLAY'] = ':%d' % xvfb_display_id
-
-        # Ensure that Xvfb is ready and replying and expected before continuing, give it 3 tries.
-        if not self._xvfbdriver._xvfb_check_if_ready(xvfb_display_id):
-            self._xvfbdriver._current_retry_start_xvfb += 1
-            if self._xvfbdriver._current_retry_start_xvfb > 3:
-                _log.error('Failed to start Xvfb display server ... giving up after 3 retries.')
-                raise RuntimeError('Unable to start Xvfb display server')
-            _log.error('Failed to start Xvfb display server ... retrying [ %s of 3 ].' % self._xvfbdriver._current_retry_start_xvfb)
-            return self._setup_environ_for_test()
-
+        driver_environment['DISPLAY'] = ":%d" % self._xvfbdriver._xvfb_run(driver_environment)
         weston_socket = 'WKTesting-weston-%032x' % random.getrandbits(128)
         weston_command = ['weston', '--socket=%s' % weston_socket, '--width=1024', '--height=768', '--use-pixman']
         if self._port._should_use_jhbuild():

Modified: trunk/Tools/Scripts/webkitpy/port/westondriver_unittest.py (281873 => 281874)


--- trunk/Tools/Scripts/webkitpy/port/westondriver_unittest.py	2021-09-01 20:44:40 UTC (rev 281873)
+++ trunk/Tools/Scripts/webkitpy/port/westondriver_unittest.py	2021-09-01 20:56:14 UTC (rev 281874)
@@ -50,10 +50,7 @@
     def _xvfb_run(self, environment):
         return self._expected_xvfbdisplay
 
-    def _xvfb_check_if_ready(self, display_id):
-        return True
 
-
 class WestonDriverTest(unittest.TestCase):
     def make_driver(self):
         port = Port(MockSystemHost(log_executive=True), 'westondrivertestport', options=MockOptions(configuration='Release'))

Modified: trunk/Tools/Scripts/webkitpy/port/xvfbdriver.py (281873 => 281874)


--- trunk/Tools/Scripts/webkitpy/port/xvfbdriver.py	2021-09-01 20:44:40 UTC (rev 281873)
+++ trunk/Tools/Scripts/webkitpy/port/xvfbdriver.py	2021-09-01 20:56:14 UTC (rev 281874)
@@ -1,5 +1,5 @@
 # Copyright (C) 2010 Google Inc. All rights reserved.
-# Copyright (C) 2014-2021 Igalia S.L.
+# Copyright (C) 2014 Igalia S.L.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
@@ -29,7 +29,6 @@
 
 import logging
 import os
-import subprocess
 import sys
 import re
 import time
@@ -42,13 +41,6 @@
 
 
 class XvfbDriver(Driver):
-
-    def __init__(self, *args, **kwargs):
-        Driver.__init__(self, *args, **kwargs)
-        self._xvfb_process = None
-        self._current_retry_start_xvfb = 0
-        self._print_screen_size_process_for_testing = None  # required for unit tests
-
     @staticmethod
     def check_driver(port):
         xvfb_findcmd = ['which', 'Xvfb']
@@ -56,7 +48,7 @@
                 xvfb_findcmd = port._jhbuild_wrapper + xvfb_findcmd
         xvfb_found = port.host.executive.run_command(xvfb_findcmd, return_exit_code=True) == 0
         if not xvfb_found:
-            _log.error('No Xvfb found. Cannot run layout tests.')
+            _log.error("No Xvfb found. Cannot run layout tests.")
         return xvfb_found
 
     def _xvfb_pipe(self):
@@ -94,73 +86,21 @@
 
     def _xvfb_run(self, environment):
         read_fd, write_fd = self._xvfb_pipe()
-        run_xvfb = ['Xvfb', '-displayfd', str(write_fd), '-nolisten', 'tcp',
-                    '+extension', 'GLX', '-ac', '-screen', '0',
-                    '%sx%s' % (self._xvfb_screen_size(), self._xvfb_screen_depth())]
+        run_xvfb = ["Xvfb", "-displayfd", str(write_fd), "-screen",  "0", "1024x768x%s" % self._xvfb_screen_depth(), "-nolisten", "tcp"]
         if self._port._should_use_jhbuild():
             run_xvfb = self._port._jhbuild_wrapper + run_xvfb
-        self._xvfb_process = self._port.host.executive.popen(run_xvfb, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, env=environment, close_fds=False)
-        display_id = self._xvfb_read_display_id(read_fd)
+        with open(os.devnull, 'w') as devnull:
+            # python3 will try to close the file descriptors by default
+            self._xvfb_process = self._port.host.executive.popen(run_xvfb, stderr=devnull, env=environment, close_fds=False)
+            display_id = self._xvfb_read_display_id(read_fd)
+
         self._xvfb_close_pipe((read_fd, write_fd))
+
         return display_id
 
     def _xvfb_screen_depth(self):
         return os.environ.get('XVFB_SCREEN_DEPTH', '24')
 
-    def _xvfb_screen_size(self):
-        return os.environ.get('XVFB_SCREEN_SIZE', '1024x768')
-
-    def _xvfb_stop(self):
-        if self._xvfb_process:
-            self._port.host.executive.kill_process(self._xvfb_process.pid)
-            self._xvfb_process = None
-
-    def _xvfb_check_if_ready(self, display_id):
-        environment_print_screen_size_process = super(XvfbDriver, self)._setup_environ_for_test()
-        environment_print_screen_size_process['DISPLAY'] = ':%d' % display_id
-        environment_print_screen_size_process['GDK_BACKEND'] = 'x11'
-        waited_seconds_for_xvfb_ready = 0
-        xvfb_server_replying_as_expected = False
-        while True:
-            try:
-                timeout_expired = False
-                query_failed = False
-                print_screen_size_process = self._print_screen_size_process_for_testing if self._print_screen_size_process_for_testing else \
-                    self._port.host.executive.popen([self._port.path_from_webkit_base('Tools', 'gtk', 'print-screen-size')],
-                                                    stdout=subprocess.PIPE, stderr=subprocess.STDOUT, env=environment_print_screen_size_process)
-                stdout, stderr = print_screen_size_process.communicate()
-            except subprocess.TimeoutExpired:
-                _log.debug('Timeout expired trying to query the Xvfb display server.')
-                timeout_expired = True
-                print_screen_size_process.kill()
-                stdout, stderr = print_screen_size_process.communicate()
-                waited_seconds_for_xvfb_ready += 2
-            if not timeout_expired:
-                if print_screen_size_process.returncode == 0:
-                    queried_screen_size = stdout.decode('UTF-8').strip()
-                    if queried_screen_size == self._xvfb_screen_size():
-                        xvfb_server_replying_as_expected = True
-                        _log.debug('The Xvfb display server ":%d" is ready and replying as expected.' % display_id)
-                        break
-                    else:
-                        _log.warning('The queried Xvfb screen size "%s" does not match the expectation "%s".' % (queried_screen_size, self._xvfb_screen_size()))
-                else:
-                    _log.warning('The print-screen-size tool returned non-zero status. stdout is "%s" and stderr is "%s"' % (stdout, stderr))
-                    query_failed = True
-            if timeout_expired or query_failed:
-                if self._xvfb_process.poll():
-                    xvfb_stdout, xvfb_stderr = self._xvfb_process.communicate()
-                    _log.error('The Xvfb display server has exited unexpectedly with a return code of %s. stdout is "%s" and stderr is "%s"' % (self._xvfb_process.poll(), xvfb_stdout, xvfb_stderr))
-                    break
-            if waited_seconds_for_xvfb_ready > 5:
-                _log.error('Timeout reached meanwhile waiting for the Xvfb display server to be ready')
-                break
-            _log.debug('Waiting for Xvfb display server to be ready.')
-            if not self._print_screen_size_process_for_testing:
-                time.sleep(1)  # only wait when not running unit tests
-            waited_seconds_for_xvfb_ready += 1
-        return xvfb_server_replying_as_expected
-
     def _setup_environ_for_test(self):
         port_server_environment = self._port.setup_environ_for_server(self._server_name)
         driver_environment = super(XvfbDriver, self)._setup_environ_for_test()
@@ -171,25 +111,10 @@
         driver_environment['UNDER_XVFB'] = 'yes'
         driver_environment['GDK_BACKEND'] = 'x11'
         driver_environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir()
-
-        # Ensure that Xvfb is ready and replying and expected before continuing, give it 3 tries.
-        if not self._xvfb_check_if_ready(display_id):
-            self._current_retry_start_xvfb += 1
-            if self._current_retry_start_xvfb > 3:
-                _log.error('Failed to start Xvfb display server ... giving up after 3 retries.')
-                raise RuntimeError('Unable to start Xvfb display server')
-            _log.error('Failed to start Xvfb display server ... retrying [ %s of 3 ].' % self._current_retry_start_xvfb)
-            return self._setup_environ_for_test()
-
         return driver_environment
 
-    def has_crashed(self):
-        if self._xvfb_process and self._xvfb_process.poll():
-            self._crashed_process_name = 'Xvfb'
-            self._crashed_pid = self._xvfb_process.pid
-            return True
-        return super(XvfbDriver, self).has_crashed()
-
     def stop(self):
         super(XvfbDriver, self).stop()
-        self._xvfb_stop()
+        if getattr(self, '_xvfb_process', None):
+            self._port.host.executive.kill_process(self._xvfb_process.pid)
+            self._xvfb_process = None

Modified: trunk/Tools/Scripts/webkitpy/port/xvfbdriver_unittest.py (281873 => 281874)


--- trunk/Tools/Scripts/webkitpy/port/xvfbdriver_unittest.py	2021-09-01 20:44:40 UTC (rev 281873)
+++ trunk/Tools/Scripts/webkitpy/port/xvfbdriver_unittest.py	2021-09-01 20:56:14 UTC (rev 281874)
@@ -31,7 +31,7 @@
 import unittest
 
 from webkitpy.common.system.filesystem_mock import MockFileSystem
-from webkitpy.common.system.executive_mock import MockProcess
+from webkitpy.common.system.executive_mock import MockExecutive2
 from webkitpy.common.system.systemhost_mock import MockSystemHost
 from webkitpy.port import Port
 from webkitpy.port.server_process_mock import MockServerProcess
@@ -44,7 +44,7 @@
 
 
 class XvfbDriverTest(unittest.TestCase):
-    def make_driver(self, worker_number=0, xorg_running=False, executive=None, print_screen_size_process=None):
+    def make_driver(self, worker_number=0, xorg_running=False, executive=None):
         port = Port(MockSystemHost(log_executive=True, executive=executive), 'xvfbdrivertestport', options=MockOptions(configuration='Release'))
         port._config.build_directory = lambda configuration: "/mock-build"
         port._test_runner_process_constructor = MockServerProcess
@@ -57,7 +57,6 @@
         driver._xvfb_pipe = lambda: (3, 4)
         driver._xvfb_read_display_id = lambda x: 1
         driver._xvfb_close_pipe = lambda p: None
-        driver._print_screen_size_process_for_testing = print_screen_size_process if print_screen_size_process else MockProcess(driver._xvfb_screen_size())
         driver._port_server_environment = port.setup_environ_for_server(port.driver_name())
         return driver
 
@@ -68,7 +67,7 @@
         driver._xvfb_process = None
 
     def assertDriverStartSuccessful(self, driver, expected_logs, expected_display, pixel_tests=False):
-        with OutputCapture(level=logging.DEBUG) as captured:
+        with OutputCapture(level=logging.INFO) as captured:
             driver.start(pixel_tests, [])
         self.assertEqual(captured.root.log.getvalue(), expected_logs)
 
@@ -76,35 +75,18 @@
         self.assertEqual(driver._server_process.env['DISPLAY'], expected_display)
         self.assertEqual(driver._server_process.env['GDK_BACKEND'], 'x11')
 
-    def test_xvfb_start_and_ready(self):
+    def test_start(self):
         driver = self.make_driver()
-        expected_display = ':1'
-        expected_logs = ("MOCK popen: ['Xvfb', '-displayfd', '4', '-nolisten', 'tcp', '+extension', 'GLX', '-ac', '-screen', '0', '1024x768x24'], env=%s\n" % driver._port_server_environment)
-        expected_logs += ('The Xvfb display server "%s" is ready and replying as expected.\n' % expected_display)
-        self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=expected_display)
+        expected_logs = ("MOCK popen: ['Xvfb', '-displayfd', '4', '-screen', '0', '1024x768x24', '-nolisten', 'tcp'], env=%s\n" % driver._port_server_environment)
+        self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":1")
         self.cleanup_driver(driver)
 
-    def test_xvfb_start_arbitrary_worker_number(self):
+    def test_start_arbitrary_worker_number(self):
         driver = self.make_driver(worker_number=17)
-        expected_display = ':1'
-        expected_logs = ("MOCK popen: ['Xvfb', '-displayfd', '4', '-nolisten', 'tcp', '+extension', 'GLX', '-ac', '-screen', '0', '1024x768x24'], env=%s\n" % driver._port_server_environment)
-        expected_logs += ('The Xvfb display server "%s" is ready and replying as expected.\n' % expected_display)
+        expected_logs = ("MOCK popen: ['Xvfb', '-displayfd', '4', '-screen', '0', '1024x768x24', '-nolisten', 'tcp'], env=%s\n" % driver._port_server_environment)
         self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":1", pixel_tests=True)
         self.cleanup_driver(driver)
 
-    def test_xvfb_not_replying(self):
-        failing_print_screen_size_process = MockProcess(returncode=1)
-        driver = self.make_driver(print_screen_size_process=failing_print_screen_size_process)
-        with OutputCapture(level=logging.INFO) as captured:
-            self.assertRaisesRegexp(RuntimeError, 'Unable to start Xvfb display server', driver.start, False, [])
-            captured_log = captured.root.log.getvalue()
-            for retry in [1, 2, 3]:
-                self.assertTrue('Failed to start Xvfb display server ... retrying [ {} of 3 ].'.format(retry) in captured_log)
-                self.assertFalse('Failed to start Xvfb display server ... retrying [ 4 of 3 ].' in captured_log)
-            self.assertTrue('Failed to start Xvfb display server ... giving up after 3 retries.' in captured_log)
-            self.assertTrue('The print-screen-size tool returned non-zero status' in captured_log)
-        self.cleanup_driver(driver)
-
     def test_stop(self):
         port = Port(MockSystemHost(log_executive=True), 'xvfbdrivertestport', options=MockOptions(configuration='Release'))
         port._executive.kill_process = lambda x: _log.info("MOCK kill_process pid: " + str(x))

Deleted: trunk/Tools/gtk/print-screen-size (281873 => 281874)


--- trunk/Tools/gtk/print-screen-size	2021-09-01 20:44:40 UTC (rev 281873)
+++ trunk/Tools/gtk/print-screen-size	2021-09-01 20:56:14 UTC (rev 281874)
@@ -1,54 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (C) 2021 Igalia S.L.
-#
-# Redistribution and use in source and binary forms, with or without
-# modification, are permitted provided that the following conditions are met:
-#
-# 1. Redistributions of source code must retain the above copyright notice, this
-#    list of conditions and the following disclaimer.
-# 2. Redistributions in binary form must reproduce the above copyright notice,
-#    this list of conditions and the following disclaimer in the documentation
-#    and/or other materials provided with the distribution.
-#
-# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
-# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
-# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
-# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
-# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
-# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
-# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
-# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-import argparse
-import sys
-import gi
-gi.require_version('Gtk', '3.0')
-gi.require_version('Gdk', '3.0')
-from gi.repository import Gtk, Gdk
-
-
-def print_screen_size(monitor_id):
-    display = Gdk.Display.get_default()
-    if not display:
-        print('Unable to find a display')
-        return 1
-    monitor = display.get_monitor(monitor_id)
-    if not monitor:
-        print('Unable to find a monitor')
-        return 1
-    geometry = monitor.get_geometry()
-    print('{}x{}'.format(geometry.width,  geometry.height))
-    return 0
-
-
-# This tool is used from the layout tests when running with the Xvfb driver,
-# in order to check that the Xvfb server is alive and configured as expected.
-# Please, don't modify this tool without checking first how it is used at webkitpy/port/xvfbdriver.py
-if __name__ == '__main__':
-    parser = argparse.ArgumentParser(description='Print the current display size (widthxheight) of a given monitor')
-    parser.add_argument("-n", type=int, help='Number of the monitor the query', default=0)
-    args = parser.parse_args()
-    sys.exit(print_screen_size(args.n))
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to