Commit 21db6120 authored by Ben Joyce's avatar Ben Joyce Committed by Commit Bot

Fix flakey test_server.

See:
https://bugs.chromium.org/p/chromium/issues/detail?id=946475#c48

I believe the root cause of the flake is trying to test if the server
has been killed by trying to rebind to the port, although my test shows
that the port can get claimed for something else by the system.

Add a process.wait() to ensure server process has terminated and remove
the error from checking the port status. Could do a check on PID status
but system could reassign the PID, so that could lead to the same
flake as before except based on PID instead of port number.

Bug: 946475
Change-Id: I48911fffcab6b592ddf627e5999fc0609e0f0b52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339553Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Reviewed-by: default avatarDavid Benjamin <davidben@chromium.org>
Commit-Queue: benjamin joyce <bjoyce@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799803}
parent 840a403e
...@@ -270,6 +270,10 @@ class TestServerThread(threading.Thread): ...@@ -270,6 +270,10 @@ class TestServerThread(threading.Thread):
self.stop_event.wait() self.stop_event.wait()
if self.process.poll() is None: if self.process.poll() is None:
self.process.kill() self.process.kill()
# Wait for process to actually terminate.
# (crbug.com/946475)
self.process.wait()
self.port_forwarder.Unmap(self.forwarder_device_port) self.port_forwarder.Unmap(self.forwarder_device_port)
self.process = None self.process = None
self.is_ready = False self.is_ready = False
...@@ -387,8 +391,11 @@ class SpawningServerRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): ...@@ -387,8 +391,11 @@ class SpawningServerRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
self._SendResponse(200, 'OK', {}, 'killed') self._SendResponse(200, 'OK', {}, 'killed')
_logger.info('Test server on port %d is killed', port) _logger.info('Test server on port %d is killed', port)
else: else:
self._SendResponse(500, 'Test Server Error.', {}, '') # We expect the port to be free, but nothing stops the system from
_logger.info('Encounter problem during killing a test server.') # binding something else to that port, so don't throw error.
# (crbug.com/946475)
self._SendResponse(200, 'OK', {}, '')
_logger.warn('Port %s is not free after killing test server.' % port)
def log_message(self, format, *args): def log_message(self, format, *args):
# Suppress the default HTTP logging behavior if the logging level is higher # Suppress the default HTTP logging behavior if the logging level is higher
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment