Changeset 228133 in webkit


Ignore:
Timestamp:
Feb 5, 2018 3:10:58 PM (6 years ago)
Author:
dbates@webkit.org
Message:

REGRESSION (r217572): run-webkit-tests exits without emitting newline character
​https://bugs.webkit.org/show_bug.cgi?id=182360

Rubber-stamped by Aakash Jain.

Fixes an annoyance where run-webkit-tests always exits without printing a newline character.
In the terminal this looks like:

$ Tools/Scripts/run-webkit-tests
Expected to fail, but passed: (7)
...
Stopping WebSocket server ...$

This bug was caused by code added in r217572 to stop all run-webkit-tests started servers (e.g. an HTTP
server) from an at-exit handler. When run-webkit-tests runs successfully (i.e. without error or
control-C interruption) we would stop all such servers twice: once as part of ending the test
run and once from the at-exit handler. The latter never prints a trailing newline character hence
the state of the terminal (as depicted above). Instead LayoutTestRunner.stop_servers() should only
stop servers that it started in LayoutTestRunner.start_servers().

  • Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:

(LayoutTestRunner.init):
(LayoutTestRunner.start_servers):
(LayoutTestRunner.stop_servers):
Only start servers that run-webkit-tests has not already started and only stop servers that
run-webkit-tests started.

  • Scripts/webkitpy/layout_tests/controllers/layout_test_runner_unittest.py:

(LayoutTestRunnerTests.test_servers_started.is_websocket_server_running):
(LayoutTestRunnerTests.test_servers_started):
(LayoutTestRunnerTests.test_servers_started.is_websocket_servers_running): Deleted.
Update due to rename below.

  • Scripts/webkitpy/layout_tests/servers/websocket_server.py:

(is_web_socket_server_running): Added.
(PyWebSocket.is_running): Deleted.

  • Scripts/webkitpy/port/base.py:

(Port.is_http_server_running): Check if we already started the server ourself.
(Port.is_websocket_server_running): Formerly named is_websocket_servers_running. Modified
to check if we already started the server ourself. Take a similar approach as the other
Port.is_*_running methods and only check if an existing WebSocket server is running on the
non-secure server port. This is a simple heuristic and should be sufficient in practice.
(Port.is_wpt_server_running): Check if we already started the server ourself.
(Port.is_websocket_servers_running): Deleted; renamed to is_websocket_server_running().

Location:
trunk/Tools
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r228131 r228133  
     12018-02-05  Daniel Bates  <dabates@apple.com>
     2
     3        REGRESSION (r217572): run-webkit-tests exits without emitting newline character
     4        https://bugs.webkit.org/show_bug.cgi?id=182360
     5
     6        Rubber-stamped by Aakash Jain.
     7
     8        Fixes an annoyance where run-webkit-tests always exits without printing a newline character.
     9        In the terminal this looks like:
     10
     11            $ Tools/Scripts/run-webkit-tests
     12            Expected to fail, but passed: (7)
     13            ...
     14            Stopping WebSocket server ...$
     15
     16        This bug was caused by code added in r217572 to stop all run-webkit-tests started servers (e.g. an HTTP
     17        server) from an at-exit handler. When run-webkit-tests runs successfully (i.e. without error or
     18        control-C interruption) we would stop all such servers twice: once as part of ending the test
     19        run and once from the at-exit handler. The latter never prints a trailing newline character hence
     20        the state of the terminal (as depicted above). Instead LayoutTestRunner.stop_servers() should only
     21        stop servers that it started in LayoutTestRunner.start_servers().
     22
     23        * Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:
     24        (LayoutTestRunner.__init__):
     25        (LayoutTestRunner.start_servers):
     26        (LayoutTestRunner.stop_servers):
     27        Only start servers that run-webkit-tests has not already started and only stop servers that
     28        run-webkit-tests started.
     29
     30        * Scripts/webkitpy/layout_tests/controllers/layout_test_runner_unittest.py:
     31        (LayoutTestRunnerTests.test_servers_started.is_websocket_server_running):
     32        (LayoutTestRunnerTests.test_servers_started):
     33        (LayoutTestRunnerTests.test_servers_started.is_websocket_servers_running): Deleted.
     34        Update due to rename below.
     35
     36        * Scripts/webkitpy/layout_tests/servers/websocket_server.py:
     37        (is_web_socket_server_running): Added.
     38        (PyWebSocket.is_running): Deleted.
     39
     40        * Scripts/webkitpy/port/base.py:
     41        (Port.is_http_server_running): Check if we already started the server ourself.
     42        (Port.is_websocket_server_running): Formerly named is_websocket_servers_running. Modified
     43        to check if we already started the server ourself. Take a similar approach as the other
     44        Port.is_*_running methods and only check if an existing WebSocket server is running on the
     45        non-secure server port. This is a simple heuristic and should be sufficient in practice.
     46        (Port.is_wpt_server_running): Check if we already started the server ourself.
     47        (Port.is_websocket_servers_running): Deleted; renamed to is_websocket_server_running().
     48
    1492018-02-05  Daniel Bates  <dabates@apple.com>
    250

  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py

    r225733 r228133  
    8080        self._retrying = False
    8181        self._current_run_results = None
     82        self._did_start_http_server = False
     83        self._did_start_websocket_server = False
     84        self._did_start_wpt_server = False
    8285
    8386        if ((self._needs_http and self._options.http) or self._needs_web_platform_test_server) and self._port.get_option("start_http_servers_if_needed"):
    … …  
    191194
    192195    def start_servers(self):
    193         if self._needs_http and not self._port.is_http_server_running():
     196        if self._needs_http and not self._did_start_http_server and not self._port.is_http_server_running():
    194197            self._printer.write_update('Starting HTTP server ...')
    195198            self._port.start_http_server()
    196         if self._needs_websockets and not self._port.is_websocket_servers_running():
     199            self._did_start_http_server = True
     200        if self._needs_websockets and not self._did_start_websocket_server and not self._port.is_websocket_server_running():
    197201            self._printer.write_update('Starting WebSocket server ...')
    198202            self._port.start_websocket_server()
    199         if self._needs_web_platform_test_server and not self._port.is_wpt_server_running():
     203            self._did_start_websocket_server = True
     204        if self._needs_web_platform_test_server and not self._did_start_wpt_server and not self._port.is_wpt_server_running():
    200205            self._printer.write_update('Starting Web Platform Test server ...')
    201206            self._port.start_web_platform_test_server()
     207            self._did_start_wpt_server = True
    202208
    203209    def stop_servers(self):
    204         if self._needs_http:
     210        if self._did_start_http_server:
    205211            self._printer.write_update('Stopping HTTP server ...')
    206212            self._port.stop_http_server()
    207         if self._needs_websockets:
     213            self._did_start_http_server = False
     214        if self._did_start_websocket_server:
    208215            self._printer.write_update('Stopping WebSocket server ...')
    209216            self._port.stop_websocket_server()
    210         if self._needs_web_platform_test_server:
     217            self._did_start_websocket_server = False
     218        if self._did_start_wpt_server:
    211219            self._printer.write_update('Stopping Web Platform Test server ...')
    212220            self._port.stop_web_platform_test_server()
     221            self._did_start_wpt_server = False
    213222
    214223    def handle(self, name, source, *args):

  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner_unittest.py

    r226820 r228133  
    163163            return self.http_started and not self.http_stopped
    164164
    165         def is_websocket_servers_running():
     165        def is_websocket_server_running():
    166166            return self.websocket_started and not self.websocket_stopped
    167167
    … …  
    178178        port.stop_web_platform_test_server = stop_web_platform_test_server
    179179        port.is_http_server_running = is_http_server_running
    180         port.is_websocket_servers_running = is_websocket_servers_running
     180        port.is_websocket_server_running = is_websocket_server_running
    181181        port.is_wpt_server_running = is_wpt_server_running
    182182

  • trunk/Tools/Scripts/webkitpy/layout_tests/servers/websocket_server.py

    r225733 r228133  
    3030"""A class to help start/stop the PyWebSocket server used by layout tests."""
    3131
    32 import errno
    3332import logging
    3433import os
    35 import socket
    3634import sys
    3735import time
    3836
    39 from webkitpy.layout_tests.servers import http_server
    40 from webkitpy.layout_tests.servers import http_server_base
     37from webkitpy.layout_tests.servers import http_server, http_server_base
    4138
    4239_log = logging.getLogger(__name__)
    … …  
    108105        else:
    109106            self._log_prefix = _WS_LOG_NAME
    110 
    111     def is_running(self):
    112         s = socket.socket()
    113         try:
    114             s.connect(('localhost', self._port))
    115         except IOError as e:
    116             if e.errno not in (errno.ECONNREFUSED, errno.ECONNRESET):
    117                 raise
    118             return False
    119         finally:
    120             s.close()
    121         return True
    122107
    123108    def ports_to_forward(self):
    … …  
    191176            self._wsout.close()
    192177            self._wsout = None
     178
     179
     180def is_web_socket_server_running():
     181    return http_server_base.HttpServerBase._is_running_on_port(PyWebSocket.DEFAULT_WS_PORT)

  • trunk/Tools/Scripts/webkitpy/port/base.py

    r227427 r228133  
    975975
    976976    def is_http_server_running(self):
     977        if self._http_server:
     978            return True
    977979        return http_server_base.is_http_server_running()
    978980
    979     def is_websocket_servers_running(self):
    980         if self._websocket_secure_server and self._websocket_server:
    981             return self._websocket_secure_server.is_running() and self._websocket_server.is_running()
    982         elif self._websocket_server:
    983             return self._websocket_server.is_running()
    984         return False
     981    def is_websocket_server_running(self):
     982        if self._websocket_server:
     983            return True
     984        return websocket_server.is_web_socket_server_running()
    985985
    986986    def is_wpt_server_running(self):
     987        if self._web_platform_test_server:
     988            return True
    987989        return web_platform_test_server.is_wpt_server_running(self)
    988990
Note: See TracChangeset for help on using the changeset viewer.