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

tests/utils.py

 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-present eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import contextlib
 import os
 import signal
 import subprocess
 import time
 import zipfile
 from io import BytesIO
 import urllib2
-import traceback
 
 import pytest
 
 
 def get_dir_contents(path):
     # TODO: This function is duplicated in test_page_outputs.py.
     dirdata = {}
     for dirpath, dirnames, filenames in os.walk(path):
         for output_file in filenames:
             filepath = os.path.join(dirpath, output_file)
@@ skipping 23 matching lines @@
         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,
                          env=new_env)
-    works, err = _test_server_and_wait()
-    if not works:
+
+    try:
+        _wait_for_server()
+        yield 'http://localhost:5000/'
+    finally:
         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.SIGINT)
 
 
-def _test_server_and_wait():
+def _wait_for_server():
     """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.
+        This will be the exception raised by `urllib2.urlopen`.
 
     """
     start_time = time.time()
 
     while True:
         try:
             urllib2.urlopen('http://localhost:5000/')
-            return True, None
+            break
         except Exception:
             time.sleep(.1)
             if time.time() - start_time > 2:
-                return False, traceback.format_exc()
+                raise
 
 
 def create_in_memory_zip(file_names, file_data):
     """Create a BytesIO object with the contents of a zip file.
 
     Parameters
     ----------
     file_names: iterable
         Of file names. Should be the full paths of the file inside the zip.
     file_data: iterable
@@ skipping 13 matching lines @@
 
     memory_zip.seek(0)
     return memory_zip
 
 
 def exception_test(func, exception, exp_msg, *args, **kw):
     with pytest.raises(exception) as err:
         func(*args, **kw)
 
     assert exp_msg in str(err.value)