From 110ce6c335c6e9bc50f26eb3e5003bf9e20b5bd0 Mon Sep 17 00:00:00 2001 From: Alex Carney Date: Sun, 27 Aug 2023 19:03:47 +0100 Subject: [PATCH 1/3] lsp: Initial pytest-lsp integration, add test cases --- .../tests/sphinx-agent/test_sa_build.py | 216 ++++++++++++++---- .../tests/sphinx-agent/test_sa_create_app.py | 55 ++--- .../tests/sphinx-agent/test_sa_unit.py | 4 + .../tests/sphinx-default/workspace/index.rst | 4 +- 4 files changed, 211 insertions(+), 68 deletions(-) diff --git a/lib/esbonio/tests/sphinx-agent/test_sa_build.py b/lib/esbonio/tests/sphinx-agent/test_sa_build.py index 8c3a89344..41321cdf7 100644 --- a/lib/esbonio/tests/sphinx-agent/test_sa_build.py +++ b/lib/esbonio/tests/sphinx-agent/test_sa_build.py @@ -1,30 +1,37 @@ import logging +import pathlib +import re import sys import pytest -import pytest_asyncio +import pytest_lsp from lsprotocol.types import WorkspaceFolder +from pygls import IS_WIN from pygls.workspace import Workspace +from pytest_lsp import ClientServerConfig from esbonio.server import Uri from esbonio.server.features.sphinx_manager.client_subprocess import ( SubprocessSphinxClient, ) +from esbonio.server.features.sphinx_manager.client_subprocess import ( + make_test_sphinx_client, +) from esbonio.server.features.sphinx_manager.config import SphinxConfig +from esbonio.sphinx_agent import types -@pytest_asyncio.fixture(scope="module") -async def sphinx_client(uri_for, tmp_path_factory): - # TODO: Integrate with `pytest_lsp` - logger = logging.getLogger("sphinx_client") - logger.setLevel(logging.INFO) - - client = SubprocessSphinxClient() - - @client.feature("window/logMessage") - def _(params): - logger.info("%s", params.message) - +@pytest_lsp.fixture( + scope="module", + params=["html", "dirhtml"], + config=ClientServerConfig( + server_command=[sys.executable, "-m", "esbonio.sphinx_agent"], + client_factory=make_test_sphinx_client, + ), +) +async def client( + request, sphinx_client: SubprocessSphinxClient, uri_for, tmp_path_factory +): build_dir = tmp_path_factory.mktemp("build") test_uri = uri_for("sphinx-default", "workspace", "index.rst") sd_workspace = uri_for("sphinx-default", "workspace") @@ -36,45 +43,176 @@ def _(params): ], ) config = SphinxConfig( - build_command=["sphinx-build", "-M", "html", ".", str(build_dir)], - python_command=[sys.executable], - env_passthrough=["PYTHONPATH"], + build_command=[ + "sphinx-build", + "-M", + request.param, + sd_workspace.fs_path, + str(build_dir), + ], ) - resolved = config.resolve(test_uri, workspace, client.logger) + resolved = config.resolve(test_uri, workspace, sphinx_client.logger) assert resolved is not None - info = await client.create_application(resolved) + info = await sphinx_client.create_application(resolved) assert info is not None - yield client + yield - if not client.stopped: - await client.stop() + +@pytest.mark.asyncio +async def test_build_includes_webview_js(client: SubprocessSphinxClient, uri_for): + """Ensure that builds include the ``webview.js`` script.""" + + out = client.build_uri + src = client.src_uri + assert out is not None and src is not None + + await client.build() + + # Ensure the script is included in the build output + webview_js = pathlib.Path(out / "_static" / "webview.js") + assert webview_js.exists() + assert "editor/scroll" in webview_js.read_text() + + # Ensure the script is included in the page + index_html = pathlib.Path(out / "index.html") + pattern = re.compile(r'') + assert pattern.search(index_html.read_text()) is not None @pytest.mark.asyncio -async def test_build_file_map(sphinx_client, uri_for): +async def test_diagnostics(client: SubprocessSphinxClient, uri_for): + """Ensure that the sphinx agent reports diagnostics collected during the build""" + expected = { + uri_for("sphinx-default/workspace/definitions.rst").fs_path: [ + types.Diagnostic( + message="unknown document: '/changelog'", + severity=types.DiagnosticSeverity.Warning, + range=types.Range( + start=types.Position(line=13, character=0), + end=types.Position(line=14, character=0), + ), + ), + types.Diagnostic( + message="image file not readable: _static/bad.png", + severity=types.DiagnosticSeverity.Warning, + range=types.Range( + start=types.Position(line=28, character=0), + end=types.Position(line=29, character=0), + ), + ), + ], + uri_for("sphinx-default/workspace/directive_options.rst").fs_path: [ + types.Diagnostic( + message="document isn't included in any toctree", + severity=types.DiagnosticSeverity.Warning, + range=types.Range( + start=types.Position(line=0, character=0), + end=types.Position(line=1, character=0), + ), + ), + types.Diagnostic( + message="image file not readable: filename.png", + severity=types.DiagnosticSeverity.Warning, + range=types.Range( + start=types.Position(line=0, character=0), + end=types.Position(line=1, character=0), + ), + ), + ], + } + result = await client.build() + + assert set(result.diagnostics.keys()) == set(expected.keys()) + + for k, ex_diags in expected.items(): + # Order of results is not important + assert set(result.diagnostics[k]) == set(ex_diags) + + +@pytest.mark.asyncio +async def test_build_file_map(client: SubprocessSphinxClient): """Ensure that we can trigger a Sphinx build correctly and the returned build_file_map is correct.""" - src = sphinx_client.src_uri + src = client.src_uri assert src is not None - build_file_map = { - (src / "index.rst").fs_path: "index.html", - (src / "definitions.rst").fs_path: "definitions.html", - (src / "directive_options.rst").fs_path: "directive_options.html", - (src / "glossary.rst").fs_path: "glossary.html", - (src / "math.rst").fs_path: "theorems/pythagoras.html", - (src / "code" / "cpp.rst").fs_path: "code/cpp.html", - (src / "theorems" / "index.rst").fs_path: "theorems/index.html", - (src / "theorems" / "pythagoras.rst").fs_path: "theorems/pythagoras.html", - # It looks like non-existant files show up in this mapping as well - (src / ".." / "badfile.rst").fs_path: "definitions.html", - } + if client.builder == "dirhtml": + build_file_map = { + (src / "index.rst").fs_path: "", + (src / "definitions.rst").fs_path: "definitions/", + (src / "directive_options.rst").fs_path: "directive_options/", + (src / "glossary.rst").fs_path: "glossary/", + (src / "math.rst").fs_path: "theorems/pythagoras/", + (src / "code" / "cpp.rst").fs_path: "code/cpp/", + (src / "theorems" / "index.rst").fs_path: "theorems/", + (src / "theorems" / "pythagoras.rst").fs_path: "theorems/pythagoras/", + # It looks like non-existant files show up in this mapping as well + (src / ".." / "badfile.rst").fs_path: "definitions/", + } + else: + build_file_map = { + (src / "index.rst").fs_path: "index.html", + (src / "definitions.rst").fs_path: "definitions.html", + (src / "directive_options.rst").fs_path: "directive_options.html", + (src / "glossary.rst").fs_path: "glossary.html", + (src / "math.rst").fs_path: "theorems/pythagoras.html", + (src / "code" / "cpp.rst").fs_path: "code/cpp.html", + (src / "theorems" / "index.rst").fs_path: "theorems/index.html", + (src / "theorems" / "pythagoras.rst").fs_path: "theorems/pythagoras.html", + # It looks like non-existant files show up in this mapping as well + (src / ".." / "badfile.rst").fs_path: "definitions.html", + } + + result = await client.build() + + # Paths are case insensitive on windows. + if IS_WIN: + actual_map = {f.lower(): uri for f, uri in result.build_file_map.items()} + expected_map = {f.lower(): uri for f, uri in build_file_map.items()} + assert actual_map == expected_map + + actual_uri_map = { + Uri.parse(str(src).lower()): out + for src, out in client.build_file_map.items() + } + expected_uri_map = { + Uri.for_file(src.lower()): out for src, out in build_file_map.items() + } + assert actual_uri_map == expected_uri_map + + else: + assert result.build_file_map == build_file_map + + build_uri_map = {Uri.for_file(src): out for src, out in build_file_map.items()} + assert client.build_file_map == build_uri_map - result = await sphinx_client.build() - assert result.build_file_map == build_file_map - build_uri_map = {Uri.for_file(src): out for src, out in build_file_map.items()} - assert sphinx_client.build_file_map == build_uri_map +@pytest.mark.asyncio +async def test_build_content_override(client: SubprocessSphinxClient, uri_for): + """Ensure that we can override the contents of a given src file when + required.""" + + out = client.build_uri + src = client.src_uri + assert out is not None and src is not None + + await client.build() + + # Before + expected = "Welcome to the documentation!" + index_html = pathlib.Path(out / "index.html") + assert expected in index_html.read_text() + + await client.build( + content_overrides={ + (src / "index.rst").fs_path: "My Custom Title\n===============" + } + ) + + # Ensure the override was applied + expected = "My Custom Title" + print(index_html.read_text()) + assert expected in index_html.read_text() diff --git a/lib/esbonio/tests/sphinx-agent/test_sa_create_app.py b/lib/esbonio/tests/sphinx-agent/test_sa_create_app.py index 928b1b401..af0850d86 100644 --- a/lib/esbonio/tests/sphinx-agent/test_sa_create_app.py +++ b/lib/esbonio/tests/sphinx-agent/test_sa_create_app.py @@ -1,33 +1,35 @@ +import asyncio import logging import sys import pytest +import pytest_lsp from lsprotocol.types import WorkspaceFolder +from pygls import IS_WIN from pygls.workspace import Workspace +from pytest_lsp import ClientServerConfig from esbonio.server.features.sphinx_manager.client_subprocess import ( SubprocessSphinxClient, ) +from esbonio.server.features.sphinx_manager.client_subprocess import ( + make_test_sphinx_client, +) from esbonio.server.features.sphinx_manager.config import SphinxConfig -@pytest.fixture() -def sphinx_client(): - # TODO: Integrate with `pytest_lsp` - logger = logging.getLogger("sphinx_client") - logger.setLevel(logging.INFO) - - client = SubprocessSphinxClient() - - @client.feature("window/logMessage") - def _(params): - logger.info("%s", params.message) - - return client +@pytest_lsp.fixture( + config=ClientServerConfig( + server_command=[sys.executable, "-m", "esbonio.sphinx_agent"], + client_factory=make_test_sphinx_client, + ) +) +async def client(sphinx_client: SubprocessSphinxClient): + yield @pytest.mark.asyncio -async def test_create_application(sphinx_client, uri_for): +async def test_create_application(client: SubprocessSphinxClient, uri_for): """Ensure that we can create a Sphinx application instance correctly.""" test_uri = uri_for("sphinx-default", "workspace", "index.rst") @@ -41,21 +43,20 @@ async def test_create_application(sphinx_client, uri_for): WorkspaceFolder(uri=str(sd_workspace), name="sphinx-default"), ], ) - config = SphinxConfig( - python_command=[sys.executable], - env_passthrough=["PYTHONPATH"], - ) - resolved = config.resolve(test_uri, workspace, sphinx_client.logger) + config = SphinxConfig() + resolved = config.resolve(test_uri, workspace, client.logger) assert resolved is not None - try: - info = await sphinx_client.create_application(resolved) - assert info is not None + info = await client.create_application(resolved) + assert info is not None + assert info.builder_name == "dirhtml" - assert info.builder_name == "dirhtml" + # Paths are case insensitive on Windows + if IS_WIN: + assert info.src_dir.lower() == sd_workspace.fs_path.lower() + assert info.conf_dir.lower() == sd_workspace.fs_path.lower() + assert "cache" in info.build_dir.lower() + else: assert info.src_dir == sd_workspace.fs_path assert info.conf_dir == sd_workspace.fs_path - assert "cache" in info.build_dir.lower() - finally: - if not sphinx_client.stopped: - await sphinx_client.stop() + assert "cache" in info.build_dir diff --git a/lib/esbonio/tests/sphinx-agent/test_sa_unit.py b/lib/esbonio/tests/sphinx-agent/test_sa_unit.py index ed1ae73af..b731c0237 100644 --- a/lib/esbonio/tests/sphinx-agent/test_sa_unit.py +++ b/lib/esbonio/tests/sphinx-agent/test_sa_unit.py @@ -36,6 +36,10 @@ def application_args(**kwargs) -> Dict[str, Any]: "warningiserror": False, } + for arg in {"srcdir", "outdir", "confdir", "doctreedir"}: + if arg in kwargs: + kwargs[arg] = str(pathlib.Path(kwargs[arg]).resolve()) + # Order matters, kwargs will override any keys found in defaults. return {**defaults, **kwargs} diff --git a/lib/esbonio/tests/sphinx-default/workspace/index.rst b/lib/esbonio/tests/sphinx-default/workspace/index.rst index 028699a64..e10ab9e59 100644 --- a/lib/esbonio/tests/sphinx-default/workspace/index.rst +++ b/lib/esbonio/tests/sphinx-default/workspace/index.rst @@ -5,8 +5,8 @@ .. _welcome: -Welcome to Defaults's documentation! -==================================== +Welcome to the documentation! +============================= .. toctree:: :maxdepth: 2 From f8481aad2fc48fcc79af1eea742a4d0c9495d72f Mon Sep 17 00:00:00 2001 From: Alex Carney Date: Tue, 12 Sep 2023 20:26:40 +0100 Subject: [PATCH 2/3] tox: Run pytest verbosely by default --- lib/esbonio/tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/esbonio/tox.ini b/lib/esbonio/tox.ini index ffc1fa615..db4a63a3f 100644 --- a/lib/esbonio/tox.ini +++ b/lib/esbonio/tox.ini @@ -50,7 +50,7 @@ commands_pre = coverage erase commands = python ../../scripts/check-sphinx-version.py - pytest tests/sphinx-agent {posargs} + pytest tests/sphinx-agent {posargs:-vv} commands_post = coverage combine coverage report From 48ede3eda38a13d9fe595992269899433dbccba5 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 7 Oct 2023 19:12:02 +0000 Subject: [PATCH 3/3] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- docs/lsp/editors/nvim-lspconfig/init.vim | 2 +- lib/esbonio/CHANGES.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/lsp/editors/nvim-lspconfig/init.vim b/docs/lsp/editors/nvim-lspconfig/init.vim index e6cf56494..c736d7922 100644 --- a/docs/lsp/editors/nvim-lspconfig/init.vim +++ b/docs/lsp/editors/nvim-lspconfig/init.vim @@ -48,7 +48,7 @@ end -- Attempt to find a virtualenv that the server can use to build the docs. local function find_venv() - + -- If there is an active virtual env, use that if vim.env.VIRTUAL_ENV then return { vim.env.VIRTUAL_ENV .. "/bin/python" } diff --git a/lib/esbonio/CHANGES.rst b/lib/esbonio/CHANGES.rst index ee28f5440..e2d4cad23 100644 --- a/lib/esbonio/CHANGES.rst +++ b/lib/esbonio/CHANGES.rst @@ -6,7 +6,7 @@ This somewhat quiet release marks the end of the ``0.x`` series as development h In fact this release includes a sneaky preview of the ``1.0`` version of the server - which includes support for multi-root projects! If you are feeling adventurous and want to try it out - change the command you use to launch ``esbonio`` to ``python -m esbonio.server`` -However, to set expectations there are many missing features from the preview server. +However, to set expectations there are many missing features from the preview server. The only features currently available are sphinx builds, diagnostics, document symbols and live preview/sync scrolling - but they should all work across multiple roots/projects! See `this issue `__ for more information and if you want to submit any feedback and keep an eye out for some beta releases in the not-to-distant-future!