Attempt to fix Ubuntu CI failure by improving LS lifecycle management in tests #788

test/conftest.py

@@ -1,9 +1,11 @@
 import logging
 import os
 import platform
+from collections.abc import Iterator
 from pathlib import Path
 
 import pytest
+from blib2to3.pgen2.parse import contextmanager
 from sensai.util.logging import configure
 
 from serena.constants import SERENA_MANAGED_DIR_IN_HOME, SERENA_MANAGED_DIR_NAME
@@ -15,6 +17,8 @@ from solidlsp.settings import SolidLSPSettings
 
 configure(level=logging.ERROR)
 
+log = logging.getLogger(__name__)
+
 
 @pytest.fixture(scope="session")
 def resources_dir() -> Path:
@@ -31,12 +35,8 @@ def get_repo_path(language: Language) -> Path:
     return Path(__file__).parent / "resources" / "repos" / language / "test_repo"
 
 
-def create_ls(
-    language: Language,
-    repo_path: str | None = None,
-    ignored_paths: list[str] | None = None,
-    trace_lsp_communication: bool = False,
-    log_level: int = logging.ERROR,
+def _create_ls(
+    language: Language, repo_path: str | None = None, ignored_paths: list[str] | None = None, trace_lsp_communication: bool = False
 ) -> SolidLanguageServer:
     ignored_paths = ignored_paths or []
     if repo_path is None:
@@ -52,9 +52,34 @@ def create_ls(
     )
 
 
-def create_default_ls(language: Language) -> SolidLanguageServer:
-    repo_path = str(get_repo_path(language))
-    return create_ls(language, repo_path)
+@contextmanager
+def start_ls_context(
+    language: Language, repo_path: str | None = None, ignored_paths: list[str] | None = None, trace_lsp_communication: bool = False
+) -> Iterator[SolidLanguageServer]:
+    ls = _create_ls(language, repo_path, ignored_paths, trace_lsp_communication)
+    log.info(f"Starting language server for {language} {repo_path}")
+    ls.start()
+    try:
+        log.info(f"Language server started for {language} {repo_path}")
+        yield ls
+    finally:
+        log.info(f"Stopping language server for {language} {repo_path}")
+        try:
+            ls.stop()
+        except Exception as e:
+            log.warning(f"Warning: Error stopping language server: {e}")
+            # try to force cleanup
+            if hasattr(ls, "server") and hasattr(ls.server, "process"):
+                try:
+                    ls.server.process.terminate()
+                except:
+                    pass
+
+
+@contextmanager
+def start_default_ls_context(language: Language) -> Iterator[SolidLanguageServer]:
+    with start_ls_context(language) as ls:
+        yield ls
 
 
 def create_default_project(language: Language) -> Project:
@@ -83,7 +108,8 @@ def repo_path(request: LanguageParamRequest) -> Path:
     return get_repo_path(language)
 
 
-@pytest.fixture(scope="session")
+# Note: using module scope here to avoid restarting LS for each test function but still terminate between test modules
+@pytest.fixture(scope="module")
 def language_server(request: LanguageParamRequest):
     """Create a language server instance configured for the specified language.
 
@@ -110,12 +136,8 @@ def language_server(request: LanguageParamRequest):
         raise ValueError("Language parameter must be provided via pytest.mark.parametrize")
 
     language = request.param
-    server = create_default_ls(language)
-    server.start()
-    try:
-        yield server
-    finally:
-        server.stop()
+    with start_default_ls_context(language) as ls:
+        yield ls
 
 
 @pytest.fixture(scope="session")

test/serena/test_symbol_editing.py

@@ -24,7 +24,7 @@ from syrupy import SnapshotAssertion
 from serena.code_editor import CodeEditor, LanguageServerCodeEditor
 from solidlsp.ls_config import Language
 from src.serena.symbol import LanguageServerSymbolRetriever
-from test.conftest import create_ls, get_repo_path
+from test.conftest import get_repo_path, start_ls_context
 
 pytestmark = pytest.mark.snapshot
 
@@ -201,19 +201,9 @@ class EditingTest(ABC):
             if os.name == "nt":
                 time.sleep(0.1)
             log.info(f"Creating language server for {self.language} {self.rel_path}")
-            language_server = create_ls(self.language, str(self.repo_path))
-            log.info(f"Starting language server for {self.language} {self.rel_path}")
-            language_server.start()
-            log.info(f"Language server started for {self.language} {self.rel_path}")
-            yield LanguageServerSymbolRetriever(ls=language_server)
+            with start_ls_context(self.language, str(self.repo_path)) as language_server:
+                yield LanguageServerSymbolRetriever(ls=language_server)
         finally:
-            if language_server is not None and language_server.is_running():
-                log.info(f"Stopping language server for {self.language} {self.rel_path}")
-                language_server.stop()
-                # attempt at trigger of garbage collection
-                language_server = None
-                log.info(f"Language server stopped for {self.language} {self.rel_path}")
-
             # prevent deadlock on Windows due to lingering file locks
             if os.name == "nt":
                 time.sleep(0.1)

test/solidlsp/elixir/test_elixir_ignored_dirs.py

@@ -5,7 +5,7 @@ import pytest
 
 from solidlsp import SolidLanguageServer
 from solidlsp.ls_config import Language
-from test.conftest import create_ls
+from test.conftest import start_ls_context
 
 from . import NEXTLS_UNAVAILABLE, NEXTLS_UNAVAILABLE_REASON
 
@@ -17,12 +17,8 @@ pytestmark = [pytest.mark.elixir, pytest.mark.skipif(NEXTLS_UNAVAILABLE, reason=
 def ls_with_ignored_dirs() -> Generator[SolidLanguageServer, None, None]:
     """Fixture to set up an LS for the elixir test repo with the 'scripts' directory ignored."""
     ignored_paths = ["scripts", "ignored_dir"]
-    ls = create_ls(ignored_paths=ignored_paths, language=Language.ELIXIR)
-    ls.start()
-    try:
+    with start_ls_context(language=Language.ELIXIR, ignored_paths=ignored_paths) as ls:
         yield ls
-    finally:
-        ls.stop()
 
 
 @pytest.mark.parametrize("ls_with_ignored_dirs", [Language.ELIXIR], indirect=True)
@@ -68,10 +64,8 @@ def test_find_references_ignores_dir(ls_with_ignored_dirs: SolidLanguageServer):
 def test_refs_and_symbols_with_glob_patterns(repo_path: Path) -> None:
     """Tests that refs and symbols with glob patterns are ignored."""
     ignored_paths = ["*cripts", "ignored_*"]  # codespell:ignore cripts
-    ls = create_ls(ignored_paths=ignored_paths, repo_path=str(repo_path), language=Language.ELIXIR)
-    ls.start()
+    with start_ls_context(language=Language.ELIXIR, repo_path=str(repo_path), ignored_paths=ignored_paths) as ls:
 
-    try:
         # Same as in the above tests
         root = ls.request_full_symbol_tree()[0]
         root_children = root["children"]
@@ -101,8 +95,6 @@ def test_refs_and_symbols_with_glob_patterns(repo_path: Path) -> None:
             # Assert that scripts and ignored_dir do not appear in references
             assert not any("scripts" in ref["relativePath"] for ref in references), "scripts should be ignored (glob)"
             assert not any("ignored_dir" in ref["relativePath"] for ref in references), "ignored_dir should be ignored (glob)"
-    finally:
-        ls.stop()
 
 
 @pytest.mark.parametrize("language_server", [Language.ELIXIR], indirect=True)

test/solidlsp/erlang/test_erlang_ignored_dirs.py

@@ -5,7 +5,7 @@ import pytest
 
 from solidlsp import SolidLanguageServer
 from solidlsp.ls_config import Language
-from test.conftest import create_ls
+from test.conftest import start_ls_context
 
 from . import ERLANG_LS_UNAVAILABLE, ERLANG_LS_UNAVAILABLE_REASON
 
@@ -20,21 +20,8 @@ pytestmark = [
 def ls_with_ignored_dirs() -> Generator[SolidLanguageServer, None, None]:
     """Fixture to set up an LS for the erlang test repo with the 'ignored_dir' directory ignored."""
     ignored_paths = ["_build", "ignored_dir"]
-    ls = create_ls(ignored_paths=ignored_paths, language=Language.ERLANG)
-    ls.start()
-    try:
+    with start_ls_context(language=Language.ERLANG, ignored_paths=ignored_paths) as ls:
         yield ls
-    finally:
-        try:
-            ls.stop(shutdown_timeout=1.0)  # Shorter timeout for CI
-        except Exception as e:
-            print(f"Warning: Error stopping language server: {e}")
-            # Force cleanup if needed
-            if hasattr(ls, "server") and hasattr(ls.server, "process"):
-                try:
-                    ls.server.process.terminate()
-                except:
-                    pass
 
 
 @pytest.mark.timeout(60)  # Add 60 second timeout
@@ -87,10 +74,7 @@ def test_find_references_ignores_dir(ls_with_ignored_dirs: SolidLanguageServer):
 def test_refs_and_symbols_with_glob_patterns(repo_path: Path) -> None:
     """Tests that refs and symbols with glob patterns are ignored."""
     ignored_paths = ["_build*", "ignored_*", "*.tmp"]
-    ls = create_ls(ignored_paths=ignored_paths, repo_path=str(repo_path), language=Language.ERLANG)
-    ls.start()
-
-    try:
+    with start_ls_context(language=Language.ERLANG, repo_path=str(repo_path), ignored_paths=ignored_paths) as ls:
         # Same as in the above tests
         root = ls.request_full_symbol_tree()[0]
         root_children = root["children"]
@@ -121,17 +105,6 @@ def test_refs_and_symbols_with_glob_patterns(repo_path: Path) -> None:
             # Assert that _build and ignored_dir do not appear in references
             assert not any("_build" in ref["relativePath"] for ref in references), "_build should be ignored (glob)"
             assert not any("ignored_dir" in ref["relativePath"] for ref in references), "ignored_dir should be ignored (glob)"
-    finally:
-        try:
-            ls.stop(shutdown_timeout=1.0)  # Shorter timeout for CI
-        except Exception as e:
-            print(f"Warning: Error stopping glob pattern test LS: {e}")
-            # Force cleanup if needed
-            if hasattr(ls, "server") and hasattr(ls.server, "process"):
-                try:
-                    ls.server.process.terminate()
-                except:
-                    pass
 
 
 @pytest.mark.parametrize("language_server", [Language.ERLANG], indirect=True)

test/solidlsp/python/test_retrieval_with_ignored_dirs.py

@@ -5,7 +5,7 @@ import pytest
 
 from solidlsp import SolidLanguageServer
 from solidlsp.ls_config import Language
-from test.conftest import create_ls
+from test.conftest import start_ls_context
 
 # This mark will be applied to all tests in this module
 pytestmark = pytest.mark.python
@@ -15,12 +15,8 @@ pytestmark = pytest.mark.python
 def ls_with_ignored_dirs() -> Generator[SolidLanguageServer, None, None]:
     """Fixture to set up an LS for the python test repo with the 'scripts' directory ignored."""
     ignored_paths = ["scripts", "custom_test"]
-    ls = create_ls(ignored_paths=ignored_paths, language=Language.PYTHON)
-    ls.start()
-    try:
+    with start_ls_context(language=Language.PYTHON, ignored_paths=ignored_paths) as ls:
         yield ls
-    finally:
-        ls.stop()
 
 
 @pytest.mark.parametrize("ls_with_ignored_dirs", [Language.PYTHON], indirect=True)
@@ -50,18 +46,17 @@ def test_find_references_ignores_dir(ls_with_ignored_dirs: SolidLanguageServer):
 def test_refs_and_symbols_with_glob_patterns(repo_path: Path) -> None:
     """Tests that refs and symbols with glob patterns are ignored."""
     ignored_paths = ["*ipts", "custom_t*"]
-    ls = create_ls(ignored_paths=ignored_paths, repo_path=str(repo_path), language=Language.PYTHON)
-    ls.start()
-    # same as in the above tests
-    root = ls.request_full_symbol_tree()[0]
-    root_children = root["children"]
-    children_names = {child["name"] for child in root_children}
-    assert children_names == {"test_repo", "examples"}
+    with start_ls_context(language=Language.PYTHON, repo_path=str(repo_path), ignored_paths=ignored_paths) as ls:
+        # same as in the above tests
+        root = ls.request_full_symbol_tree()[0]
+        root_children = root["children"]
+        children_names = {child["name"] for child in root_children}
+        assert children_names == {"test_repo", "examples"}
 
-    # test that the refs and symbols with glob patterns are ignored
-    definition_file = "test_repo/models.py"
-    definition_line = 56
-    definition_col = 6
+        # test that the refs and symbols with glob patterns are ignored
+        definition_file = "test_repo/models.py"
+        definition_line = 56
+        definition_col = 6
 
-    references = ls.request_references(definition_file, definition_line, definition_col)
-    assert not any("scripts" in ref["relativePath"] for ref in references)
+        references = ls.request_references(definition_file, definition_line, definition_col)
+        assert not any("scripts" in ref["relativePath"] for ref in references)

test/solidlsp/rust/test_rust_2024_edition.py

@@ -1,37 +1,36 @@
 import os
+from collections.abc import Iterator
 from pathlib import Path
 
 import pytest
 
+from solidlsp import SolidLanguageServer
 from solidlsp.ls_config import Language
 from solidlsp.ls_utils import SymbolUtils
-from test.conftest import create_ls
+from test.conftest import start_ls_context
+
+
+@pytest.fixture(scope="class")
+def rust_language_server() -> Iterator[SolidLanguageServer]:
+    """Set up the test class with the Rust 2024 edition test repository."""
+    test_repo_2024_path = TestRust2024EditionLanguageServer.test_repo_2024_path
+
+    if not test_repo_2024_path.exists():
+        pytest.skip("Rust 2024 edition test repository not found")
+
+    # Create and start the language server for the 2024 edition repo
+    with start_ls_context(Language.RUST, str(test_repo_2024_path)) as ls:
+        yield ls
 
 
 @pytest.mark.rust
 class TestRust2024EditionLanguageServer:
-    @classmethod
-    def setup_class(cls):
-        """Set up the test class with the Rust 2024 edition test repository."""
-        cls.test_repo_2024_path = Path(__file__).parent.parent.parent / "resources" / "repos" / "rust" / "test_repo_2024"
+    test_repo_2024_path = Path(__file__).parent.parent.parent / "resources" / "repos" / "rust" / "test_repo_2024"
 
-        if not cls.test_repo_2024_path.exists():
-            pytest.skip("Rust 2024 edition test repository not found")
-
-        # Create and start the language server for the 2024 edition repo
-        cls.language_server = create_ls(Language.RUST, str(cls.test_repo_2024_path))
-        cls.language_server.start()
-
-    @classmethod
-    def teardown_class(cls):
-        """Clean up the language server."""
-        if hasattr(cls, "language_server"):
-            cls.language_server.stop()
-
-    def test_find_references_raw(self) -> None:
+    def test_find_references_raw(self, rust_language_server) -> None:
         # Test finding references to the 'add' function defined in main.rs
         file_path = os.path.join("src", "main.rs")
-        symbols = self.language_server.request_document_symbols(file_path).get_all_symbols_and_roots()
+        symbols = rust_language_server.request_document_symbols(file_path).get_all_symbols_and_roots()
         add_symbol = None
         for sym in symbols[0]:
             if sym.get("name") == "add":
@@ -39,21 +38,21 @@ class TestRust2024EditionLanguageServer:
                 break
         assert add_symbol is not None, "Could not find 'add' function symbol in main.rs"
         sel_start = add_symbol["selectionRange"]["start"]
-        refs = self.language_server.request_references(file_path, sel_start["line"], sel_start["character"])
+        refs = rust_language_server.request_references(file_path, sel_start["line"], sel_start["character"])
         # The add function should be referenced within main.rs itself (in the main function)
         assert any("main.rs" in ref.get("relativePath", "") for ref in refs), "main.rs should reference add function"
 
-    def test_find_symbol(self) -> None:
-        symbols = self.language_server.request_full_symbol_tree()
+    def test_find_symbol(self, rust_language_server) -> None:
+        symbols = rust_language_server.request_full_symbol_tree()
         assert SymbolUtils.symbol_tree_contains_name(symbols, "main"), "main function not found in symbol tree"
         assert SymbolUtils.symbol_tree_contains_name(symbols, "add"), "add function not found in symbol tree"
         assert SymbolUtils.symbol_tree_contains_name(symbols, "multiply"), "multiply function not found in symbol tree"
         assert SymbolUtils.symbol_tree_contains_name(symbols, "Calculator"), "Calculator struct not found in symbol tree"
 
-    def test_find_referencing_symbols_multiply(self) -> None:
+    def test_find_referencing_symbols_multiply(self, rust_language_server) -> None:
         # Find references to 'multiply' function defined in lib.rs
         file_path = os.path.join("src", "lib.rs")
-        symbols = self.language_server.request_document_symbols(file_path).get_all_symbols_and_roots()
+        symbols = rust_language_server.request_document_symbols(file_path).get_all_symbols_and_roots()
         multiply_symbol = None
         for sym in symbols[0]:
             if sym.get("name") == "multiply":
@@ -61,15 +60,15 @@ class TestRust2024EditionLanguageServer:
                 break
         assert multiply_symbol is not None, "Could not find 'multiply' function symbol in lib.rs"
         sel_start = multiply_symbol["selectionRange"]["start"]
-        refs = self.language_server.request_references(file_path, sel_start["line"], sel_start["character"])
+        refs = rust_language_server.request_references(file_path, sel_start["line"], sel_start["character"])
         # The multiply function exists but may not be referenced anywhere, which is fine
         # This test just verifies we can find the symbol and request references without error
         assert isinstance(refs, list), "Should return a list of references (even if empty)"
 
-    def test_find_calculator_struct_and_impl(self) -> None:
+    def test_find_calculator_struct_and_impl(self, rust_language_server) -> None:
         # Test finding the Calculator struct and its impl block
         file_path = os.path.join("src", "lib.rs")
-        symbols = self.language_server.request_document_symbols(file_path).get_all_symbols_and_roots()
+        symbols = rust_language_server.request_document_symbols(file_path).get_all_symbols_and_roots()
 
         # Find the Calculator struct
         calculator_struct = None
@@ -95,8 +94,8 @@ class TestRust2024EditionLanguageServer:
             assert "add" in method_names, "Calculator impl should have 'add' method"
             assert "get_result" in method_names, "Calculator impl should have 'get_result' method"
 
-    def test_overview_methods(self) -> None:
-        symbols = self.language_server.request_full_symbol_tree()
+    def test_overview_methods(self, rust_language_server) -> None:
+        symbols = rust_language_server.request_full_symbol_tree()
         assert SymbolUtils.symbol_tree_contains_name(symbols, "main"), "main missing from overview"
         assert SymbolUtils.symbol_tree_contains_name(symbols, "add"), "add missing from overview"
         assert SymbolUtils.symbol_tree_contains_name(symbols, "multiply"), "multiply missing from overview"