diff --git a/mslib/mscolab/mscolab.py b/mslib/mscolab/mscolab.py index ad6dfe46b..b0a79969d 100644 --- a/mslib/mscolab/mscolab.py +++ b/mslib/mscolab/mscolab.py @@ -45,15 +45,13 @@ def handle_start(args): - from mslib.mscolab.server import APP, initialize_managers, start_server + from mslib.mscolab.server import APP, sockio, cm, fm, start_server setup_logging(args) logging.info("MSS Version: %s", __version__) logging.info("Python Version: %s", sys.version) logging.info("Platform: %s (%s)", platform.platform(), platform.architecture()) logging.info("Launching MSColab Server") - - app, sockio, cm, fm = initialize_managers(APP) - start_server(app, sockio, cm, fm) + start_server(APP, sockio, cm, fm) def confirm_action(confirmation_prompt): diff --git a/mslib/mscolab/server.py b/mslib/mscolab/server.py index ebab735ad..90afff90b 100644 --- a/mslib/mscolab/server.py +++ b/mslib/mscolab/server.py @@ -48,7 +48,7 @@ from mslib.mscolab.conf import mscolab_settings, setup_saml2_backend from mslib.mscolab.models import Change, MessageType, User -from mslib.mscolab.sockets_manager import setup_managers +from mslib.mscolab.sockets_manager import _setup_managers from mslib.mscolab.utils import create_files, get_message_dict from mslib.utils import conditional_decorator from mslib.index import create_app @@ -126,8 +126,8 @@ def confirm_token(token, expiration=3600): return email -def initialize_managers(app): - sockio, cm, fm = setup_managers(app) +def _initialize_managers(app): + sockio, cm, fm = _setup_managers(app) # initializing socketio and db app.wsgi_app = socketio.Middleware(socketio.server, app.wsgi_app) sockio.init_app(app) @@ -135,7 +135,7 @@ def initialize_managers(app): return app, sockio, cm, fm -_app, sockio, cm, fm = initialize_managers(APP) +_app, sockio, cm, fm = _initialize_managers(APP) def check_login(emailid, password): diff --git a/mslib/mscolab/sockets_manager.py b/mslib/mscolab/sockets_manager.py index 85bf6b569..20b1e6b82 100644 --- a/mslib/mscolab/sockets_manager.py +++ b/mslib/mscolab/sockets_manager.py @@ -262,7 +262,7 @@ def emit_operation_delete(self, op_id): socketio.emit("operation-deleted", json.dumps({"op_id": op_id})) -def setup_managers(app): +def _setup_managers(app): """ takes app as parameter to extract config data, initializes ChatManager, FileManager, SocketManager and return them diff --git a/tests/_test_mscolab/test_server.py b/tests/_test_mscolab/test_server.py index 680364a4b..9ffb47a0c 100644 --- a/tests/_test_mscolab/test_server.py +++ b/tests/_test_mscolab/test_server.py @@ -30,7 +30,7 @@ from mslib.mscolab.conf import mscolab_settings from mslib.mscolab.models import User, Operation -from mslib.mscolab.server import initialize_managers, check_login, register_user +from mslib.mscolab.server import check_login, register_user from mslib.mscolab.file_manager import FileManager from mslib.mscolab.seed import add_user, get_user @@ -43,9 +43,9 @@ def setup(self, mscolab_app): with self.app.app_context(): yield - def test_initialize_managers(self): - app, sockio, cm, fm = initialize_managers(self.app) - assert app.config['MSCOLAB_DATA_DIR'] == mscolab_settings.MSCOLAB_DATA_DIR + def test_initialized_managers(self, mscolab_managers): + sockio, cm, fm = mscolab_managers + assert self.app.config['MSCOLAB_DATA_DIR'] == mscolab_settings.MSCOLAB_DATA_DIR assert 'Create a Flask-SocketIO server.' in sockio.__doc__ assert 'Class with handler functions for chat related functionalities' in cm.__doc__ assert 'Class with handler functions for file related functionalities' in fm.__doc__ diff --git a/tests/_test_mscolab/test_sockets_manager.py b/tests/_test_mscolab/test_sockets_manager.py index 574730c6e..84152f219 100644 --- a/tests/_test_mscolab/test_sockets_manager.py +++ b/tests/_test_mscolab/test_sockets_manager.py @@ -26,11 +26,7 @@ """ import os import pytest -import socket -import socketio import datetime -import requests -from urllib.parse import urljoin, urlparse from mslib.msui.icons import icons from mslib.mscolab.conf import mscolab_settings @@ -41,10 +37,10 @@ class Test_Socket_Manager: @pytest.fixture(autouse=True) - def setup(self, mscolab_app, mscolab_managers, mscolab_server): + def setup(self, mscolab_app, mscolab_managers): self.app = mscolab_app - _, self.cm, self.fm = mscolab_managers - self.url = mscolab_server + self.sockio, self.cm, self.fm = mscolab_managers + self.sm = self.sockio.sm self.sockets = [] self.userdata = 'UV10@uv10', 'UV10', 'uv10' self.anotheruserdata = 'UV20@uv20', 'UV20', 'uv20' @@ -57,28 +53,13 @@ def setup(self, mscolab_app, mscolab_managers, mscolab_server): self.anotheruser = get_user(self.anotheruserdata[0]) self.token = self.user.generate_auth_token() self.operation = get_operation(self.operation_name) - self.sm = SocketsManager(self.cm, self.fm) yield for sock in self.sockets: sock.disconnect() - def _can_ping_server(self): - parsed_url = urlparse(self.url) - host, port = parsed_url.hostname, parsed_url.port - try: - sock = socket.create_connection((host, port)) - success = True - except socket.error: - success = False - finally: - sock.close() - return success - def _connect(self): - sio = socketio.Client(reconnection_attempts=5) + sio = self.sockio.test_client(self.app) self.sockets.append(sio) - assert self._can_ping_server() - sio.connect(self.url, transports='polling') sio.emit('connect') return sio @@ -88,13 +69,8 @@ def _new_operation(self, operation_name, description): return operation def test_handle_connect(self): - sio = socketio.Client() - assert sio.sid is None - self.sockets.append(sio) - assert self._can_ping_server() - sio.connect(self.url, transports='polling') - sio.emit('connect') - assert len(sio.sid) > 5 + sio = self._connect() + assert len(sio.eio_sid) > 5 def test_join_creator_to_operatiom(self): sio = self._connect() @@ -122,14 +98,12 @@ def test_join_collaborator_to_operation(self): def test_remove_collaborator_from_operation(self): pytest.skip("get_session_id has None result") - sio = self._connect() operation = self._new_operation('new_operation', "example description") sm = SocketsManager(self.cm, self.fm) sm.join_collaborator_to_operation(self.anotheruser.id, operation.id) perms = Permission(self.anotheruser.id, operation.id, "collaborator") assert perms is not None sm.remove_collaborator_from_operation(self.anotheruser.id, operation.id) - sio.sleep(1) perms = Permission(self.anotheruser.id, operation.id, "collaborator") assert perms is None @@ -158,7 +132,6 @@ def test_send_message(self): "message_text": "® non ascii", "reply_id": -1 }) - sio.sleep(1) with self.app.app_context(): message = Message.query.filter_by(text="message from 1").first() @@ -185,7 +158,6 @@ def test_get_messages(self): "message_text": "message from 1", "reply_id": -1 }) - sio.sleep(5) with self.app.app_context(): messages = self.cm.get_messages(1) assert messages[0]["text"] == "message from 1" @@ -216,7 +188,6 @@ def test_get_messages_api(self): "message_text": "message from 1", "reply_id": -1 }) - sio.sleep(1) token = self.token data = { @@ -224,15 +195,14 @@ def test_get_messages_api(self): "op_id": self.operation.id, "timestamp": datetime.datetime(1970, 1, 1, tzinfo=datetime.timezone.utc).isoformat() } - # returns an array of messages - url = urljoin(self.url, 'messages') - res = requests.get(url, data=data, timeout=(2, 10)).json() - assert len(res["messages"]) == 2 + with self.app.test_client() as c: + res = c.get("/messages", data=data) + assert len(res.json["messages"]) == 2 - data["token"] = "dummy" - # returns False due to bad authorization - r = requests.get(url, data=data, timeout=(2, 10)) - assert r.text == "False" + data["token"] = "dummy" + # returns False due to bad authorization + r = c.get("/messages", data=data) + assert r.text == "False" def test_edit_message(self): sio = self._connect() @@ -244,7 +214,6 @@ def test_edit_message(self): "message_text": "Edit this message", "reply_id": -1 }) - sio.sleep(1) with self.app.app_context(): message = Message.query.filter_by(text="Edit this message").first() sio.emit('edit-message', { @@ -253,16 +222,14 @@ def test_edit_message(self): "op_id": message.op_id, "token": self.token }) - sio.sleep(1) token = self.token data = { "token": token, "op_id": self.operation.id, "timestamp": datetime.datetime(1970, 1, 1, tzinfo=datetime.timezone.utc).isoformat() } - # returns an array of messages - url = urljoin(self.url, 'messages') - res = requests.get(url, data=data, timeout=(2, 10)).json() + with self.app.test_client() as c: + res = c.get("messages", data=data).json assert len(res["messages"]) == 1 messages = res["messages"][0] assert messages["text"] == "I have updated the message" @@ -277,7 +244,7 @@ def test_delete_message(self): "message_text": "delete this message", "reply_id": -1 }) - sio.sleep(1) + with self.app.app_context(): message = Message.query.filter_by(text="delete this message").first() sio.emit('delete-message', { @@ -285,7 +252,6 @@ def test_delete_message(self): 'op_id': self.operation.id, 'token': self.token }) - sio.sleep(1) with self.app.app_context(): assert Message.query.filter_by(text="delete this message").count() == 0 @@ -293,14 +259,14 @@ def test_delete_message(self): def test_upload_file(self): sio = self._connect() sio.emit('start', {'token': self.token}) - files = {'file': open(icons('16x16'), 'rb')} data = { "token": self.token, "op_id": self.operation.id, - "message_type": int(MessageType.IMAGE) + "message_type": int(MessageType.IMAGE), + "file": open(icons('16x16'), 'rb'), } - url = urljoin(self.url, 'message_attachment') - requests.post(url, data=data, files=files, timeout=(2, 10)) + with self.app.test_client() as c: + c.post("message_attachment", data=data, content_type="multipart/form-data") upload_dir = os.path.join(mscolab_settings.UPLOAD_FOLDER, str(self.user.id)) assert os.path.exists(upload_dir) file = os.listdir(upload_dir)[0] diff --git a/tests/fixtures.py b/tests/fixtures.py index 4005225ec..dfd29a39e 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -28,6 +28,7 @@ import multiprocessing import time import urllib +import socketio import mslib.mswms.mswms import eventlet import eventlet.wsgi @@ -35,7 +36,7 @@ from PyQt5 import QtWidgets from contextlib import contextmanager from mslib.mscolab.conf import mscolab_settings -from mslib.mscolab.server import APP, initialize_managers +from mslib.mscolab.server import APP, sockio, cm, fm from mslib.mscolab.mscolab import handle_db_init, handle_db_reset from mslib.utils.config import modify_config_file from tests.utils import is_url_response_ok @@ -104,10 +105,17 @@ def mscolab_session_managers(mscolab_session_app): This fixture should not be used in tests. Instead use :func:`mscolab_managers`, which handles per-test cleanup as well. """ - return initialize_managers(mscolab_session_app)[1:] + return sockio, cm, fm -@pytest.fixture(scope="session") +# TODO: Having this fixture be autouse is a crutch. It seems like if it is not autouse some tests can bring the pytest +# processes objects into a state in which the MSColab server will have trouble starting the Flask-SocketIO server once +# it is forked. With autouse the fork happens first, before any test runs. After that, the pytest process can no longer +# affect the now-running server, thus mitigating the issue. This is my understanding at time of writing. +# +# This issue would also be avoided if the background server process wasn't started with multiprocessing and a fork, but +# with a real subprocess, which would solve some other issues (e.g. testing on Windows) as well. +@pytest.fixture(scope="session", autouse=True) def mscolab_session_server(mscolab_session_app, mscolab_session_managers): """Session-scoped fixture that provides a running MSColab server. @@ -115,6 +123,11 @@ def mscolab_session_server(mscolab_session_app, mscolab_session_managers): handles per-test cleanup as well. """ with _running_eventlet_server(mscolab_session_app) as url: + # Wait until the Flask-SocketIO server is ready for connections + sio = socketio.Client() + sio.connect(url, retry=True) + sio.disconnect() + del sio yield url @@ -141,8 +154,7 @@ def mscolab_app(mscolab_session_app, reset_mscolab): def mscolab_managers(mscolab_session_managers, reset_mscolab): """Fixture that provides the MSColab managers and does cleanup actions. - :returns: A tuple (SocketIO, ChatManager, FileManager) as returned by - initialize_managers. + :returns: A tuple (SocketIO, ChatManager, FileManager). """ return mscolab_session_managers