From 5136236c73431230343b2c3c511ec094eada0e48 Mon Sep 17 00:00:00 2001 From: Prashanth Radhakrishnan Date: Thu, 11 Jul 2024 10:23:12 -0700 Subject: [PATCH 1/2] Implement "spawn" --- server/integration_tests/__init__.py | 4 +- server/requirements.txt | 1 - server/webdriver/base.py | 8 +-- .../tests/standalone/cdc/__init__.py | 4 +- shared/lib/test_server.py | 64 +++++++++++-------- shared/lib/test_setup.py | 15 ++--- 6 files changed, 51 insertions(+), 45 deletions(-) diff --git a/server/integration_tests/__init__.py b/server/integration_tests/__init__.py index 63486ab12a..a47ec35c15 100644 --- a/server/integration_tests/__init__.py +++ b/server/integration_tests/__init__.py @@ -12,6 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from shared.lib.test_setup import set_up_macos_for_tests +from shared.lib.test_setup import set_up_multiprocessing_for_tests -set_up_macos_for_tests() +set_up_multiprocessing_for_tests() diff --git a/server/requirements.txt b/server/requirements.txt index 20c2ae77c7..9f99c7a9b6 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -5,7 +5,6 @@ Flask==2.3.2 Flask-Babel==2.0.0 Flask-Caching==2.0.1 flask_cors==3.0.10 -flask_testing==0.8.1 frozendict==2.3.4 geojson_rewind==1.0.1 google-auth==2.28.1 diff --git a/server/webdriver/base.py b/server/webdriver/base.py index 7226c4c21f..4f814d9057 100644 --- a/server/webdriver/base.py +++ b/server/webdriver/base.py @@ -12,16 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -import multiprocessing -import os -import sys - from server.webdriver import shared from server.webdriver.base_utils import create_driver from shared.lib.test_server import NLWebServerTestCase -from shared.lib.test_setup import set_up_macos_for_tests +from shared.lib.test_setup import set_up_multiprocessing_for_tests -set_up_macos_for_tests() +set_up_multiprocessing_for_tests() # Base test class to setup the server. diff --git a/server/webdriver/tests/standalone/cdc/__init__.py b/server/webdriver/tests/standalone/cdc/__init__.py index cd9c6b6f91..1ba37caf02 100644 --- a/server/webdriver/tests/standalone/cdc/__init__.py +++ b/server/webdriver/tests/standalone/cdc/__init__.py @@ -12,6 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from shared.lib.test_setup import set_up_macos_for_tests +from shared.lib.test_setup import set_up_multiprocessing_for_tests -set_up_macos_for_tests() +set_up_multiprocessing_for_tests() diff --git a/shared/lib/test_server.py b/shared/lib/test_server.py index 2da4d74124..426c1d52d0 100644 --- a/shared/lib/test_server.py +++ b/shared/lib/test_server.py @@ -16,23 +16,27 @@ import os import platform import socket +import unittest import warnings -from flask_testing import LiveServerTestCase - from nl_server.flask import create_app as create_nl_app from server.__init__ import create_app as create_web_app import server.lib.util as libutil -def find_open_port(): +def find_open_port(skip_port: int | None = None): for port in range(12000, 13000): with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + if port == skip_port: + continue res = sock.connect_ex(('localhost', port)) if res != 0: return port +web_port = find_open_port() + +nl_port = None is_nl_mode = os.environ.get('ENABLE_MODEL') == 'true' if is_nl_mode: # Start NL server on an unused port, so multiple integration tests can @@ -45,41 +49,51 @@ def find_open_port(): nl_port = 6060 should_start_nl_server = False else: - nl_port = find_open_port() + nl_port = find_open_port(web_port) should_start_nl_server = True -class NLWebServerTestCase(LiveServerTestCase): +def start_nl_server(port): + nl_app = create_nl_app() + nl_app.run(port=port, debug=False, use_reloader=False, threaded=True) + + +def start_web_server(web_port, nl_port=None): + if nl_port: + web_app = create_web_app(f'http://127.0.0.1:{nl_port}') + else: + web_app = create_web_app() + web_app.run(port=web_port, use_reloader=False) + + +class NLWebServerTestCase(unittest.TestCase): @classmethod def setUpClass(cls): if is_nl_mode: if should_start_nl_server: - def start_nl_server(app): - app.run(port=nl_port, debug=False, use_reloader=False, threaded=True) - - nl_app = create_nl_app() # Create a thread that will contain our running server - cls.proc = multiprocessing.Process(target=start_nl_server, - args=(nl_app,), - daemon=True) - cls.proc.start() + cls.nl_proc = multiprocessing.Process(target=start_nl_server, + args=(nl_port,), + daemon=True) + cls.nl_proc.start() else: - cls.proc = None + cls.nl_proc = None libutil.check_backend_ready( ['http://127.0.0.1:{}/healthz'.format(nl_port)]) + # Start web app. + cls.web_proc = multiprocessing.Process(target=start_web_server, + args=(web_port, nl_port)) + cls.web_proc.start() + libutil.check_backend_ready([f'http://127.0.0.1:{web_port}/healthz']) + + def get_server_url(cls): + return f'http://localhost:{web_port}' + @classmethod def tearDownClass(cls): - if is_nl_mode and cls.proc: - cls.proc.terminate() - - def create_app(self): - """Returns the Flask Server running Data Commons.""" - if is_nl_mode: - app = create_web_app('http://127.0.0.1:{}'.format(nl_port)) - else: - app = create_web_app() - app.config['LIVESERVER_PORT'] = 0 - return app + if is_nl_mode and cls.nl_proc: + cls.nl_proc.terminate() + cls.web_proc.terminate() diff --git a/shared/lib/test_setup.py b/shared/lib/test_setup.py index 1a557632d2..95be5b728e 100644 --- a/shared/lib/test_setup.py +++ b/shared/lib/test_setup.py @@ -14,18 +14,15 @@ import multiprocessing import os -import sys -def set_up_macos_for_tests(): - """Ensure Python tests work on MacOS. +def set_up_multiprocessing_for_tests(): + """Ensure Python tests work with multiprocessing. - Explicitly sets multiprocessing start method to 'fork' so tests work with - python3.8+ on MacOS: - https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods + Explicitly sets to "spawn" as the default can be "fork" in certain Linux, + and that does not work consistently well. This code must only be run once per execution. """ - if sys.version_info >= (3, 8) and sys.platform == "darwin": - multiprocessing.set_start_method("fork") - os.environ['no_proxy'] = '*' + multiprocessing.set_start_method("spawn") + os.environ['no_proxy'] = '*' \ No newline at end of file From a16da119906d328897cc169d32bf33f13a012823 Mon Sep 17 00:00:00 2001 From: Prashanth Radhakrishnan Date: Fri, 12 Jul 2024 10:56:47 -0700 Subject: [PATCH 2/2] Tweak webdriver --- server/webdriver/base_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/webdriver/base_utils.py b/server/webdriver/base_utils.py index fa2397b2ac..9f5e49ed79 100644 --- a/server/webdriver/base_utils.py +++ b/server/webdriver/base_utils.py @@ -15,6 +15,7 @@ from selenium import webdriver from selenium.webdriver.chrome.options import Options +from selenium.webdriver.chrome.service import Service DEFAULT_HEIGHT = 1200 DEFAULT_WIDTH = 1200 @@ -29,7 +30,7 @@ def create_driver(preferences=None): chrome_options.add_argument('--hide-scrollbars') if preferences: chrome_options.add_experimental_option("prefs", preferences) - driver = webdriver.Chrome(options=chrome_options) + driver = webdriver.Chrome(options=chrome_options, service=Service(port=0)) # Set a reliable window size for all tests (can be overwritten though) driver.set_window_size(DEFAULT_WIDTH, DEFAULT_HEIGHT) return driver