Issue 7019 - [CMS] Refactor `test_server.py`

tests/utils.py

Index: tests/utils.py
diff --git a/tests/utils.py b/tests/utils.py
index ed21e24c650e59651cd2a6edcf9d1cd915c9a38e..ebc4038f5c28ba6109aa1ad8d50fc58457870efd 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -20,6 +20,8 @@ import subprocess
 import time
 import zipfile
 from io import BytesIO
+import urllib2
+import traceback
 
 import pytest
 
@@ -37,20 +39,71 @@ def get_dir_contents(path):
 
 
 @contextlib.contextmanager
-def run_test_server(site_path):
+def run_test_server(site_path, new_env=None):
     """Run test server, yield its URL. Terminate server on next iteration.
 
     This function is intended be used in a pytest fixture.
+
+    Parameters
+    ----------
+    site_path: str
+        The path to the website's source code.
+    new_env: dict
+        The environment under which the server will be run. If `None`, this
+        will be inherited from the main process.
+
+    Returns
+    -------
+    str
+        The url where the server runs.
+
     """
     args = ['python', 'runserver.py', site_path]
     # Werkzeug is a dependency of flask which we are using for the mock api
     # however there is an issue with Werkzeug that prevents it from properly
     # handling the SIGTERM sent by p.kill() or terminate()
     # Issue: https://github.com/pallets/werkzeug/issues/58
-    p = subprocess.Popen(args, stdout=subprocess.PIPE, preexec_fn=os.setsid)
-    time.sleep(0.5)
+    p = subprocess.Popen(args, stdout=subprocess.PIPE, preexec_fn=os.setsid,
+                         env=new_env)
+    works, err = _test_server_and_wait()
+    if not works:
+        os.killpg(os.getpgid(p.pid), signal.SIGINT)

[Vasily Kuznetsov, 2018/10/25 15:51:43]

We can get rid of the replication of this line using a try/finally block. Like
this:

try:
    _wait_for_server()
    yield 'http://.....'
finally:
    os.killpg(...)

and then _wait_for_server() would just reraise the exception once the timeout is
expired: I don't think the fancy handling adds that much value here.

[Tudor Avram, 2018/10/29 10:42:51]

Done.

+        raise Exception('Could not get the server running on `http://localhost'
+                        ':5000/`. See traceback:\n{}'.format(err))
     yield 'http://localhost:5000/'
-    os.killpg(os.getpgid(p.pid), signal.SIGTERM)
+    os.killpg(os.getpgid(p.pid), signal.SIGINT)
+
+
+def _test_server_and_wait():
+    """Test the server started by `run_test_server` and fail after 2s.
+
+    Expects the server to be active at `http://localhost:5000/`.
+
+    Returns
+    -------
+    bool
+        True - if the server works as expected
+        False - otherwise
+    str or None
+        The traceback of the error - if the server connection fails
+        None - otherwise
+
+    Raises
+    ------
+    Exception
+        If the server is not running after retry-ing for 2 seconds.
+
+    """
+    start_time = time.time()
+
+    while True:
+        try:
+            urllib2.urlopen('http://localhost:5000/')
+            return True, None
+        except Exception:
+            time.sleep(.1)
+            if time.time() - start_time > 2:
+                return False, traceback.format_exc()
 
 
 def create_in_memory_zip(file_names, file_data):