REGRESSION (r217572): run-webkit-tests exits without emitting newline character
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Feb 2018 23:10:58 +0000 (23:10 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Feb 2018 23:10:58 +0000 (23:10 +0000)
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().

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

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner_unittest.py
Tools/Scripts/webkitpy/layout_tests/servers/websocket_server.py
Tools/Scripts/webkitpy/port/base.py

index cafd89e..e8b2216 100644 (file)
@@ -1,5 +1,53 @@
 2018-02-05  Daniel Bates  <dabates@apple.com>
 
+        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().
+
+2018-02-05  Daniel Bates  <dabates@apple.com>
+
         prepare-ChangeLog gets confused about Python docstrings that contain the word "class" or "def"
         https://bugs.webkit.org/show_bug.cgi?id=182405
 
index 05f9051..3e52e61 100644 (file)
@@ -79,6 +79,9 @@ class LayoutTestRunner(object):
         self._test_inputs = []
         self._retrying = False
         self._current_run_results = None
+        self._did_start_http_server = False
+        self._did_start_websocket_server = False
+        self._did_start_wpt_server = False
 
         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"):
             self.start_servers()
@@ -190,26 +193,32 @@ class LayoutTestRunner(object):
         self._interrupt_if_at_failure_limits(run_results)
 
     def start_servers(self):
-        if self._needs_http and not self._port.is_http_server_running():
+        if self._needs_http and not self._did_start_http_server and not self._port.is_http_server_running():
             self._printer.write_update('Starting HTTP server ...')
             self._port.start_http_server()
-        if self._needs_websockets and not self._port.is_websocket_servers_running():
+            self._did_start_http_server = True
+        if self._needs_websockets and not self._did_start_websocket_server and not self._port.is_websocket_server_running():
             self._printer.write_update('Starting WebSocket server ...')
             self._port.start_websocket_server()
-        if self._needs_web_platform_test_server and not self._port.is_wpt_server_running():
+            self._did_start_websocket_server = True
+        if self._needs_web_platform_test_server and not self._did_start_wpt_server and not self._port.is_wpt_server_running():
             self._printer.write_update('Starting Web Platform Test server ...')
             self._port.start_web_platform_test_server()
+            self._did_start_wpt_server = True
 
     def stop_servers(self):
-        if self._needs_http:
+        if self._did_start_http_server:
             self._printer.write_update('Stopping HTTP server ...')
             self._port.stop_http_server()
-        if self._needs_websockets:
+            self._did_start_http_server = False
+        if self._did_start_websocket_server:
             self._printer.write_update('Stopping WebSocket server ...')
             self._port.stop_websocket_server()
-        if self._needs_web_platform_test_server:
+            self._did_start_websocket_server = False
+        if self._did_start_wpt_server:
             self._printer.write_update('Stopping Web Platform Test server ...')
             self._port.stop_web_platform_test_server()
+            self._did_start_wpt_server = False
 
     def handle(self, name, source, *args):
         method = getattr(self, '_handle_' + name)
index 1837148..fff96df 100644 (file)
@@ -162,7 +162,7 @@ class LayoutTestRunnerTests(unittest.TestCase):
         def is_http_server_running():
             return self.http_started and not self.http_stopped
 
-        def is_websocket_servers_running():
+        def is_websocket_server_running():
             return self.websocket_started and not self.websocket_stopped
 
         def is_wpt_server_running():
@@ -177,7 +177,7 @@ class LayoutTestRunnerTests(unittest.TestCase):
         port.stop_websocket_server = stop_websocket_server
         port.stop_web_platform_test_server = stop_web_platform_test_server
         port.is_http_server_running = is_http_server_running
-        port.is_websocket_servers_running = is_websocket_servers_running
+        port.is_websocket_server_running = is_websocket_server_running
         port.is_wpt_server_running = is_wpt_server_running
 
         self.http_started = self.http_stopped = self.websocket_started = self.websocket_stopped = False
index 4d4f5b7..944f9cb 100644 (file)
 
 """A class to help start/stop the PyWebSocket server used by layout tests."""
 
-import errno
 import logging
 import os
-import socket
 import sys
 import time
 
-from webkitpy.layout_tests.servers import http_server
-from webkitpy.layout_tests.servers import http_server_base
+from webkitpy.layout_tests.servers import http_server, http_server_base
 
 _log = logging.getLogger(__name__)
 
@@ -108,18 +105,6 @@ class PyWebSocket(http_server.Lighttpd):
         else:
             self._log_prefix = _WS_LOG_NAME
 
-    def is_running(self):
-        s = socket.socket()
-        try:
-            s.connect(('localhost', self._port))
-        except IOError as e:
-            if e.errno not in (errno.ECONNREFUSED, errno.ECONNRESET):
-                raise
-            return False
-        finally:
-            s.close()
-        return True
-
     def ports_to_forward(self):
         return [self._port]
 
@@ -190,3 +175,7 @@ class PyWebSocket(http_server.Lighttpd):
         if self._wsout:
             self._wsout.close()
             self._wsout = None
+
+
+def is_web_socket_server_running():
+    return http_server_base.HttpServerBase._is_running_on_port(PyWebSocket.DEFAULT_WS_PORT)
index 015771f..c166cf4 100644 (file)
@@ -974,16 +974,18 @@ class Port(object):
         self._http_server = server
 
     def is_http_server_running(self):
+        if self._http_server:
+            return True
         return http_server_base.is_http_server_running()
 
-    def is_websocket_servers_running(self):
-        if self._websocket_secure_server and self._websocket_server:
-            return self._websocket_secure_server.is_running() and self._websocket_server.is_running()
-        elif self._websocket_server:
-            return self._websocket_server.is_running()
-        return False
+    def is_websocket_server_running(self):
+        if self._websocket_server:
+            return True
+        return websocket_server.is_web_socket_server_running()
 
     def is_wpt_server_running(self):
+        if self._web_platform_test_server:
+            return True
         return web_platform_test_server.is_wpt_server_running(self)
 
     def _extract_certificate_from_pem(self, pem_file, destination_certificate_file):