Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from "fork" to "spawn" for multiprocessing in tests #4469

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions server/integration_tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
1 change: 0 additions & 1 deletion server/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions server/webdriver/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion server/webdriver/base_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
4 changes: 2 additions & 2 deletions server/webdriver/cdc_tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
64 changes: 39 additions & 25 deletions shared/lib/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
15 changes: 6 additions & 9 deletions shared/lib/test_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'] = '*'
Loading