From 627bc4840524ee36e319747a9d50d8b53522cbca Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Tue, 15 Aug 2023 11:20:06 +0200 Subject: [PATCH 1/3] Converge unit tests for test_language_server and test_notebook_document --- test/fixtures.py | 17 ++++++ test/test_language_server.py | 101 +++++++++------------------------ test/test_notebook_document.py | 50 ++-------------- test/test_utils.py | 55 ++++++++++++++++++ 4 files changed, 104 insertions(+), 119 deletions(-) diff --git a/test/fixtures.py b/test/fixtures.py index dde37ff4..03d0f824 100644 --- a/test/fixtures.py +++ b/test/fixtures.py @@ -5,6 +5,8 @@ from io import StringIO from unittest.mock import MagicMock +from test.test_utils import ClientServerPair + import pytest from pylsp_jsonrpc.dispatchers import MethodDispatcher from pylsp_jsonrpc.endpoint import Endpoint @@ -22,6 +24,7 @@ def main(): print sys.stdin.read() """ +CALL_TIMEOUT_IN_SECONDS = 30 class FakeEditorMethodsMixin: @@ -163,3 +166,17 @@ def create_file(name, content): return workspace return fn + + +@pytest.fixture +def client_server_pair(): + """A fixture that sets up a client/server pair and shuts down the server""" + client_server_pair_obj = ClientServerPair() + + yield (client_server_pair_obj.client, client_server_pair_obj.server) + + shutdown_response = client_server_pair_obj.client._endpoint.request( + "shutdown" + ).result(timeout=CALL_TIMEOUT_IN_SECONDS) + assert shutdown_response is None + client_server_pair_obj.client._endpoint.notify("exit") diff --git a/test/test_language_server.py b/test/test_language_server.py index 8aa8d2c5..280a62fa 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -3,74 +3,18 @@ import os import time -import multiprocessing import sys -from threading import Thread + +from test.test_utils import ClientServerPair from flaky import flaky from pylsp_jsonrpc.exceptions import JsonRpcMethodNotFound import pytest -from pylsp.python_lsp import start_io_lang_server, PythonLSPServer -CALL_TIMEOUT = 10 RUNNING_IN_CI = bool(os.environ.get("CI")) - -def start_client(client): - client.start() - - -class _ClientServer: - """A class to setup a client/server pair""" - - def __init__(self, check_parent_process=False): - # Client to Server pipe - csr, csw = os.pipe() - # Server to client pipe - scr, scw = os.pipe() - - if os.name == "nt": - ParallelKind = Thread - else: - if sys.version_info[:2] >= (3, 8): - ParallelKind = multiprocessing.get_context("fork").Process - else: - ParallelKind = multiprocessing.Process - - self.process = ParallelKind( - target=start_io_lang_server, - args=( - os.fdopen(csr, "rb"), - os.fdopen(scw, "wb"), - check_parent_process, - PythonLSPServer, - ), - ) - self.process.start() - - self.client = PythonLSPServer( - os.fdopen(scr, "rb"), os.fdopen(csw, "wb"), start_io_lang_server - ) - self.client_thread = Thread(target=start_client, args=[self.client]) - self.client_thread.daemon = True - self.client_thread.start() - - -@pytest.fixture -def client_server(): - """A fixture that sets up a client/server pair and shuts down the server - This client/server pair does not support checking parent process aliveness - """ - client_server_pair = _ClientServer() - - yield client_server_pair.client - - shutdown_response = client_server_pair.client._endpoint.request("shutdown").result( - timeout=CALL_TIMEOUT - ) - assert shutdown_response is None - client_server_pair.client._endpoint.notify("exit") +CALL_TIMEOUT_IN_SECONDS = 10 @pytest.fixture @@ -78,21 +22,21 @@ def client_exited_server(): """A fixture that sets up a client/server pair that support checking parent process aliveness and assert the server has already exited """ - client_server_pair = _ClientServer(True) + client_server_pair_obj = ClientServerPair(True, True) - # yield client_server_pair.client - yield client_server_pair + yield client_server_pair_obj - assert client_server_pair.process.is_alive() is False + assert client_server_pair_obj.server_process.is_alive() is False @flaky(max_runs=10, min_passes=1) @pytest.mark.skipif(sys.platform == "darwin", reason="Too flaky on Mac") -def test_initialize(client_server): # pylint: disable=redefined-outer-name - response = client_server._endpoint.request( +def test_initialize(client_server_pair): + client, _ = client_server_pair + response = client._endpoint.request( "initialize", {"rootPath": os.path.dirname(__file__), "initializationOptions": {}}, - ).result(timeout=CALL_TIMEOUT) + ).result(timeout=CALL_TIMEOUT_IN_SECONDS) assert "capabilities" in response @@ -104,7 +48,10 @@ def test_exit_with_parent_process_died( client_exited_server, ): # pylint: disable=redefined-outer-name # language server should have already exited before responding - lsp_server, mock_process = client_exited_server.client, client_exited_server.process + lsp_server, mock_process = ( + client_exited_server.client, + client_exited_server.server_process, + ) # with pytest.raises(Exception): lsp_server._endpoint.request( "initialize", @@ -113,31 +60,35 @@ def test_exit_with_parent_process_died( "rootPath": os.path.dirname(__file__), "initializationOptions": {}, }, - ).result(timeout=CALL_TIMEOUT) + ).result(timeout=CALL_TIMEOUT_IN_SECONDS) mock_process.terminate() - time.sleep(CALL_TIMEOUT) + time.sleep(CALL_TIMEOUT_IN_SECONDS) assert not client_exited_server.client_thread.is_alive() @flaky(max_runs=10, min_passes=1) @pytest.mark.skipif(sys.platform.startswith("linux"), reason="Fails on linux") def test_not_exit_without_check_parent_process_flag( - client_server, -): # pylint: disable=redefined-outer-name - response = client_server._endpoint.request( + client_server_pair, +): + client, _ = client_server_pair + response = client._endpoint.request( "initialize", { "processId": 1234, "rootPath": os.path.dirname(__file__), "initializationOptions": {}, }, - ).result(timeout=CALL_TIMEOUT) + ).result(timeout=CALL_TIMEOUT_IN_SECONDS) assert "capabilities" in response @flaky(max_runs=10, min_passes=1) @pytest.mark.skipif(RUNNING_IN_CI, reason="This test is hanging on CI") -def test_missing_message(client_server): # pylint: disable=redefined-outer-name +def test_missing_message(client_server_pair): + client, _ = client_server_pair with pytest.raises(JsonRpcMethodNotFound): - client_server._endpoint.request("unknown_method").result(timeout=CALL_TIMEOUT) + client._endpoint.request("unknown_method").result( + timeout=CALL_TIMEOUT_IN_SECONDS + ) diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index a85ff931..d632a660 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -2,17 +2,15 @@ import os import time -from threading import Thread from unittest.mock import patch, call +from test.fixtures import CALL_TIMEOUT_IN_SECONDS + import pytest from pylsp import IS_WIN -from pylsp.python_lsp import PythonLSPServer from pylsp.lsp import NotebookCellKind -CALL_TIMEOUT_IN_SECONDS = 30 - def wait_for_condition(condition, timeout=CALL_TIMEOUT_IN_SECONDS): """Wait for a condition to be true, or timeout.""" @@ -23,44 +21,8 @@ def wait_for_condition(condition, timeout=CALL_TIMEOUT_IN_SECONDS): raise TimeoutError("Timeout waiting for condition") -def start(obj): - obj.start() - - -class ClientServerPair: - """A class to setup a client/server pair""" - - def __init__(self): - # Client to Server pipe - csr, csw = os.pipe() - # Server to client pipe - scr, scw = os.pipe() - - self.server = PythonLSPServer(os.fdopen(csr, "rb"), os.fdopen(scw, "wb")) - self.server_thread = Thread(target=start, args=[self.server]) - self.server_thread.start() - - self.client = PythonLSPServer(os.fdopen(scr, "rb"), os.fdopen(csw, "wb")) - self.client_thread = Thread(target=start, args=[self.client]) - self.client_thread.start() - - -@pytest.fixture -def client_server_pair(): - """A fixture that sets up a client/server pair and shuts down the server""" - client_server_pair_obj = ClientServerPair() - - yield (client_server_pair_obj.client, client_server_pair_obj.server) - - shutdown_response = client_server_pair_obj.client._endpoint.request( - "shutdown" - ).result(timeout=CALL_TIMEOUT_IN_SECONDS) - assert shutdown_response is None - client_server_pair_obj.client._endpoint.notify("exit") - - @pytest.mark.skipif(IS_WIN, reason="Flaky on Windows") -def test_initialize(client_server_pair): # pylint: disable=redefined-outer-name +def test_initialize(client_server_pair): client, server = client_server_pair response = client._endpoint.request( "initialize", @@ -77,7 +39,7 @@ def test_initialize(client_server_pair): # pylint: disable=redefined-outer-name @pytest.mark.skipif(IS_WIN, reason="Flaky on Windows") def test_notebook_document__did_open( client_server_pair, -): # pylint: disable=redefined-outer-name +): client, server = client_server_pair client._endpoint.request( "initialize", @@ -241,7 +203,7 @@ def test_notebook_document__did_open( @pytest.mark.skipif(IS_WIN, reason="Flaky on Windows") def test_notebook_document__did_change( client_server_pair, -): # pylint: disable=redefined-outer-name +): client, server = client_server_pair client._endpoint.request( "initialize", @@ -513,7 +475,7 @@ def test_notebook_document__did_change( @pytest.mark.skipif(IS_WIN, reason="Flaky on Windows") def test_notebook__did_close( client_server_pair, -): # pylint: disable=redefined-outer-name +): client, server = client_server_pair client._endpoint.request( "initialize", diff --git a/test/test_utils.py b/test/test_utils.py index 50e3ca8d..7d01ba01 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -1,12 +1,67 @@ # Copyright 2017-2020 Palantir Technologies, Inc. # Copyright 2021- Python Language Server Contributors. +import multiprocessing +import os +import sys +from threading import Thread import time from unittest import mock from flaky import flaky from pylsp import _utils +from pylsp.python_lsp import PythonLSPServer, start_io_lang_server + + +def start(obj): + obj.start() + + +class ClientServerPair: + """ + A class to setup a client/server pair. + + args: + start_server_in_process: if True, the server will be started in a process. + check_parent_process: if True, the server_process will check if the parent process is alive. + """ + + def __init__(self, start_server_in_process=False, check_parent_process=False): + # Client to Server pipe + csr, csw = os.pipe() + # Server to client pipe + scr, scw = os.pipe() + + if start_server_in_process: + ParallelKind = self._get_parallel_kind() + self.server_process = ParallelKind( + target=start_io_lang_server, + args=( + os.fdopen(csr, "rb"), + os.fdopen(scw, "wb"), + check_parent_process, + PythonLSPServer, + ), + ) + self.server_process.start() + else: + self.server = PythonLSPServer(os.fdopen(csr, "rb"), os.fdopen(scw, "wb")) + self.server_thread = Thread(target=start, args=[self.server]) + self.server_thread.start() + + self.client = PythonLSPServer(os.fdopen(scr, "rb"), os.fdopen(csw, "wb")) + self.client_thread = Thread(target=start, args=[self.client]) + self.client_thread.start() + + def _get_parallel_kind(self): + if os.name == "nt": + return Thread + + if sys.version_info[:2] >= (3, 8): + return multiprocessing.get_context("fork").Process + + return multiprocessing.Process @flaky(max_runs=6, min_passes=1) From cab3e34f12cd455426d5277987d916f60abad67a Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Thu, 17 Aug 2023 11:36:43 +0200 Subject: [PATCH 2/3] rebase master into HEAD --- pylsp/plugins/rope_autoimport.py | 2 +- test/test_utils.py | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pylsp/plugins/rope_autoimport.py b/pylsp/plugins/rope_autoimport.py index be40fe41..dc400c92 100644 --- a/pylsp/plugins/rope_autoimport.py +++ b/pylsp/plugins/rope_autoimport.py @@ -20,7 +20,7 @@ log = logging.getLogger(__name__) _score_pow = 5 -_score_max = 10**_score_pow +_score_max = 10 ** _score_pow MAX_RESULTS = 1000 diff --git a/test/test_utils.py b/test/test_utils.py index 7d01ba01..28689835 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -138,10 +138,13 @@ def test_find_parents(tmpdir): def test_merge_dicts(): - assert _utils.merge_dicts( - {"a": True, "b": {"x": 123, "y": {"hello": "world"}}}, - {"a": False, "b": {"y": [], "z": 987}}, - ) == {"a": False, "b": {"x": 123, "y": [], "z": 987}} + assert ( + _utils.merge_dicts( + {"a": True, "b": {"x": 123, "y": {"hello": "world"}}}, + {"a": False, "b": {"y": [], "z": 987}}, + ) + == {"a": False, "b": {"x": 123, "y": [], "z": 987}} + ) def test_clip_column(): From b6383528a4e7913a9f587e5e3a10f379960c76ff Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Thu, 17 Aug 2023 11:43:43 +0200 Subject: [PATCH 3/3] black format --- pylsp/plugins/rope_autoimport.py | 2 +- test/test_utils.py | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pylsp/plugins/rope_autoimport.py b/pylsp/plugins/rope_autoimport.py index dc400c92..be40fe41 100644 --- a/pylsp/plugins/rope_autoimport.py +++ b/pylsp/plugins/rope_autoimport.py @@ -20,7 +20,7 @@ log = logging.getLogger(__name__) _score_pow = 5 -_score_max = 10 ** _score_pow +_score_max = 10**_score_pow MAX_RESULTS = 1000 diff --git a/test/test_utils.py b/test/test_utils.py index 28689835..7d01ba01 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -138,13 +138,10 @@ def test_find_parents(tmpdir): def test_merge_dicts(): - assert ( - _utils.merge_dicts( - {"a": True, "b": {"x": 123, "y": {"hello": "world"}}}, - {"a": False, "b": {"y": [], "z": 987}}, - ) - == {"a": False, "b": {"x": 123, "y": [], "z": 987}} - ) + assert _utils.merge_dicts( + {"a": True, "b": {"x": 123, "y": {"hello": "world"}}}, + {"a": False, "b": {"y": [], "z": 987}}, + ) == {"a": False, "b": {"x": 123, "y": [], "z": 987}} def test_clip_column():