diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 60f1a175e..c3f5a729e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -3,8 +3,8 @@ on: push: branches: [master] pull_request: -jobs: +jobs: lint: name: Lint runs-on: ubuntu-latest @@ -23,18 +23,25 @@ jobs: fail-fast: false # Set on "false" to get the results of ALL builds matrix: os: ["ubuntu-latest"] - python-version: ["3.10", "3.12"] - sphinx-version: ["7.0", "8.0"] + python-version: ["3.9", "3.12", "3.13"] + sphinx-version: ["7.4", "8.2"] include: - - os: "ubuntu-latest" - python-version: "3.9" - sphinx-version: "7.0" + # corner cases for Windows - os: "windows-latest" python-version: "3.9" - sphinx-version: "7.0" + sphinx-version: "7.4" - os: "windows-latest" python-version: "3.12" - sphinx-version: "8.0" + sphinx-version: "8.2" + - os: "windows-latest" + python-version: "3.13" + sphinx-version: "8.2" + exclude: + # Sphinx 8.2 only supports py3.11+ + - os: "ubuntu-latest" + python-version: "3.9" + sphinx-version: "8.2" + steps: - uses: actions/checkout@v4 - name: Install graphviz (linux) @@ -75,10 +82,10 @@ jobs: include: - os: "ubuntu-latest" python-version: "3.9" - sphinx-version: "7.0" + sphinx-version: "7.4" - os: "ubuntu-latest" - python-version: "3.12" - sphinx-version: "8.0" + python-version: "3.13" + sphinx-version: "8.2" steps: - uses: actions/checkout@v4 - name: Use Node.js diff --git a/docs/directives/needimport.rst b/docs/directives/needimport.rst index e7dd78b42..afcfa6acb 100644 --- a/docs/directives/needimport.rst +++ b/docs/directives/needimport.rst @@ -26,26 +26,34 @@ The directive argument can be one of the following formats: - A remote URL from which to download the ``needs.json``: .. code-block:: rst - + .. needimport:: https://my_company.com/docs/remote-needs.json - A local path relative to the containing document: .. code-block:: rst - + .. needimport:: needs.json - A local path starting with ``/`` is relative to the Sphinx source directory: .. code-block:: rst - + .. needimport:: /path/to/needs.json -- For an absolute path, make sure to start with two ``//`` (on Linux/OSX): +- For an absolute path on Linux/OSX, make sure to start with two ``//``: .. code-block:: rst - - .. needimport:: //absoulte/path/to/needs.json + + .. needimport:: //absolute/path/to/needs.json + +- For an absolute path on Windows, just use the normal drive letters with either forward or backward slashes: + + .. code-block:: rst + + .. needimport:: c:/absolute/path/to/needs.json + + .. needimport:: c:\absolute\path\to\needs.json Options ------- diff --git a/pyproject.toml b/pyproject.toml index baf01189d..c9f4635b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,13 +21,14 @@ classifiers = [ 'Programming Language :: Python :: 3.10', 'Programming Language :: Python :: 3.11', 'Programming Language :: Python :: 3.12', + 'Programming Language :: Python :: 3.13', 'Topic :: Documentation', 'Topic :: Utilities', 'Framework :: Sphinx :: Extension', ] requires-python = ">=3.9,<4" dependencies = [ - "sphinx>=7.0,<9", + "sphinx>=7.4,<9", "requests-file~=2.1", # external links "requests~=2.32", # external links "jsonschema>=3.2.0", # needsimport schema validation diff --git a/sphinx_needs/directives/needimport.py b/sphinx_needs/directives/needimport.py index 66454e2c0..39f5ac70d 100644 --- a/sphinx_needs/directives/needimport.py +++ b/sphinx_needs/directives/needimport.py @@ -81,34 +81,9 @@ def run(self) -> Sequence[nodes.Node]: else: logger.info(f"Importing needs from {need_import_path}") - if not os.path.isabs(need_import_path): - # Relative path should start from current rst file directory - curr_dir = os.path.dirname(self.docname) - new_need_import_path = os.path.join( - self.env.app.srcdir, curr_dir, need_import_path - ) - - correct_need_import_path = new_need_import_path - if not os.path.exists(new_need_import_path): - # Check the old way that calculates relative path starting from conf.py directory - old_need_import_path = os.path.join( - self.env.app.srcdir, need_import_path - ) - if os.path.exists(old_need_import_path): - correct_need_import_path = old_need_import_path - log_warning( - logger, - "Deprecation warning: Relative path must be relative to the current document in future, " - "not to the conf.py location. Use a starting '/', like '/needs.json', to make the path " - "relative to conf.py.", - "deprecated", - location=(self.env.docname, self.lineno), - ) - else: - # Absolute path starts with /, based on the source directory. The / need to be striped - correct_need_import_path = os.path.join( - self.env.app.srcdir, need_import_path[1:] - ) + correct_need_import_path = self.env.relfn2path( + need_import_path, self.env.docname + )[1] if not os.path.exists(correct_need_import_path): raise ReferenceError( diff --git a/sphinx_needs/directives/needuml.py b/sphinx_needs/directives/needuml.py index 92e15ebf1..dfb2282b5 100644 --- a/sphinx_needs/directives/needuml.py +++ b/sphinx_needs/directives/needuml.py @@ -4,6 +4,7 @@ import os import time from collections.abc import Sequence +from pathlib import PurePosixPath from typing import TYPE_CHECKING, Any, TypedDict from docutils import nodes @@ -107,9 +108,9 @@ def run(self) -> Sequence[nodes.Node]: save_path = self.options.get("save") plantuml_code_out_path = None if save_path: - if os.path.isabs(save_path): + if PurePosixPath(save_path).is_absolute(): raise NeedumlException( - f"Given save path: {save_path}, is not a relative path." + f"Given save path: {save_path}, is not a relative posix path." ) else: plantuml_code_out_path = save_path diff --git a/sphinx_needs/needs.py b/sphinx_needs/needs.py index c684738fa..d88737775 100644 --- a/sphinx_needs/needs.py +++ b/sphinx_needs/needs.py @@ -8,12 +8,14 @@ from docutils import nodes from docutils.parsers.rst import directives from sphinx.application import Sphinx +from sphinx.builders import Builder from sphinx.config import Config from sphinx.environment import BuildEnvironment from sphinx.errors import SphinxError import sphinx_needs.debug as debug # Need to set global var in it for timeing measurements from sphinx_needs import __version__ +from sphinx_needs.api import get_needs_view from sphinx_needs.api.need import _split_list_with_dyn_funcs from sphinx_needs.builder import ( NeedsBuilder, @@ -305,6 +307,8 @@ def setup(app: Sphinx) -> dict[str, Any]: app.connect("doctree-resolved", process_need_nodes) app.connect("doctree-resolved", process_creator(NODE_TYPES)) + app.connect("write-started", ensure_post_process_needs_data) + app.connect("build-finished", process_warnings) app.connect("build-finished", build_needs_json) app.connect("build-finished", build_needs_id_json) @@ -324,6 +328,15 @@ def setup(app: Sphinx) -> dict[str, Any]: } +def ensure_post_process_needs_data(app: Sphinx, builder: Builder) -> None: + """ + Make sure post_process_needs_data is called at least once. + + Warnings are emitted in that step, even when no docs are updated. + """ + get_needs_view(app) + + def process_creator( node_list: _NODE_TYPES_T, doc_category: str = "all" ) -> Callable[[Sphinx, nodes.document, str], None]: diff --git a/tests/__snapshots__/test_needimport.ambr b/tests/__snapshots__/test_needimport.ambr index b73fb2dde..c990d4448 100644 --- a/tests/__snapshots__/test_needimport.ambr +++ b/tests/__snapshots__/test_needimport.ambr @@ -78,7 +78,7 @@ 'external_css': 'external_link', 'id': 'REQ_1', 'layout': '', - 'lineno': 48, + 'lineno': 47, 'section_name': 'FILTERED', 'sections': list([ 'FILTERED', @@ -193,7 +193,7 @@ 'external_css': 'external_link', 'id': 'SPEC_1', 'layout': '', - 'lineno': 51, + 'lineno': 50, 'section_name': 'FILTERED', 'sections': list([ 'FILTERED', @@ -1250,21 +1250,6 @@ 'type': 'req', 'type_name': 'Requirement', }), - 'small_depr_rel_path_TEST_01': dict({ - 'content': 'small_depr_rel_path_TEST_01', - 'docname': 'subdoc/deprecated_rel_path_import', - 'external_css': 'external_link', - 'id': 'small_depr_rel_path_TEST_01', - 'is_import': True, - 'lineno': 6, - 'section_name': 'Deprecated Relative path import test', - 'sections': list([ - 'Deprecated Relative path import test', - ]), - 'title': 'TEST_01 DESCRIPTION', - 'type': 'impl', - 'type_name': 'Implementation', - }), 'small_rel_path_TEST_01': dict({ 'content': 'small_rel_path_TEST_01', 'docname': 'subdoc/rel_path_import', @@ -1635,7 +1620,7 @@ 'type_name': 'Test Case', }), }), - 'needs_amount': 66, + 'needs_amount': 65, 'needs_defaults_removed': True, 'needs_schema': dict({ '$schema': 'http://json-schema.org/draft-07/schema#', diff --git a/tests/conftest.py b/tests/conftest.py index c3c93af8d..a3108410a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -54,6 +54,19 @@ def copy_srcdir_to_tmpdir(srcdir: path, tmp: path) -> path: return tmproot +def create_src_files_in_tmpdir(files: list[tuple[Path, str]], tmp: path) -> path: + """Create source files in a temporary directory under the subdir src.""" + subdir = path("src") + tmproot = tmp.joinpath(generate_random_string()) / subdir + tmproot.makedirs(exist_ok=True) + for file in files: + file_path, content = file + file_abs = tmproot.joinpath(str(file_path)) + file_abs.parent.makedirs(exist_ok=True) + file_abs.write_text(content) + return tmproot + + def get_abspath(relpath: str) -> str: """ Get the absolute path from a relative path. @@ -264,9 +277,18 @@ def test_app(make_app, sphinx_test_tempdir, request): ) sphinx_conf_overrides.update(plantuml=plantuml) - # copy test srcdir to test temporary directory sphinx_test_tempdir srcdir = builder_params.get("srcdir") - src_dir = copy_srcdir_to_tmpdir(srcdir, sphinx_test_tempdir) + files = builder_params.get("files") + if (srcdir is None) == (files is None): + raise ValueError("Exactly one of srcdir, files must not be None") + + if srcdir is not None: + # copy test srcdir to test temporary directory sphinx_test_tempdir + src_dir = copy_srcdir_to_tmpdir(srcdir, sphinx_test_tempdir) + else: + # create given files in tmpdir + src_dir = create_src_files_in_tmpdir(files, sphinx_test_tempdir) + parent_path = Path(str(src_dir.parent.abspath())) if version_info >= (7, 2): diff --git a/tests/doc_test/import_doc/index.rst b/tests/doc_test/import_doc/index.rst index 6c2e2f108..0f9bb2122 100644 --- a/tests/doc_test/import_doc/index.rst +++ b/tests/doc_test/import_doc/index.rst @@ -43,7 +43,6 @@ FILTERED subdoc/filter subdoc/abs_path_import subdoc/rel_path_import - subdoc/deprecated_rel_path_import .. req:: Test requirement 1 :id: REQ_1 diff --git a/tests/doc_test/import_doc/subdoc/deprecated_rel_path_import.rst b/tests/doc_test/import_doc/subdoc/deprecated_rel_path_import.rst deleted file mode 100644 index 4de3fc026..000000000 --- a/tests/doc_test/import_doc/subdoc/deprecated_rel_path_import.rst +++ /dev/null @@ -1,8 +0,0 @@ -Deprecated Relative path import test -==================================== - -Test import file with relative path, which based on directory of conf.py. - -.. needimport:: subdoc/needs_test_small.json - :id_prefix: small_depr_rel_path_ - :filter: "impl" in type diff --git a/tests/test_needimport.py b/tests/test_needimport.py index ca76932cc..14cb018e7 100644 --- a/tests/test_needimport.py +++ b/tests/test_needimport.py @@ -1,5 +1,6 @@ import json import os +import sys from pathlib import Path import pytest @@ -20,17 +21,8 @@ def test_import_json(test_app): app = test_app app.build() - warnings = app._warning.getvalue() - warnings_list = strip_colors( - warnings.replace(str(app.srcdir) + os.sep + "subdoc" + os.sep, "srcdir/subdoc/") - ).splitlines() - - if os.name != "nt": - assert warnings_list == [ - "srcdir/subdoc/deprecated_rel_path_import.rst:6: WARNING: Deprecation warning: Relative path must be relative to the current document in future, not to the conf.py location. Use a starting '/', like '/needs.json', to make the path relative to conf.py. [needs.deprecated]" - ] - else: - assert "Deprecation" in warnings + assert app.statuscode == 0 + assert not app.warning_list html = Path(app.outdir, "index.html").read_text() assert "TEST IMPORT TITLE" in html @@ -61,11 +53,130 @@ def test_import_json(test_app): rel_path_import_html = Path(app.outdir, "subdoc/rel_path_import.html").read_text() assert "small_rel_path_TEST_01" in rel_path_import_html - # Check deprecated relative path import based on conf.py - deprec_rel_path_import_html = Path( - app.outdir, "subdoc/deprecated_rel_path_import.html" - ).read_text() - assert "small_depr_rel_path_TEST_01" in deprec_rel_path_import_html + +needs_json = """ +{ + "current_version": "1", + "versions": { + "1": { + "needs": { + "TEST_01": { + "id": "TEST_01", + "title": "TEST IMPORT TITLE", + "type": "impl" + } + }, + "needs_amount": 1 + } + } +} +""" + + +@pytest.mark.parametrize( + "index_content", + [ + ".. needimport:: needs.json", + ".. needimport:: nested/needs.json", + ".. needimport:: /needs.json", + ".. needimport:: /nested/needs.json", + ], +) +@pytest.mark.parametrize( + "test_app", + [ + { + "buildername": "html", + "files": [ + ("conf.py", 'extensions = ["sphinx_needs"]'), + ("needs.json", needs_json), + ("nested/needs.json", needs_json), + ], + "no_plantuml": True, + } + ], + indirect=True, +) +def test_import_rel_abs_sphinx_paths(test_app, index_content): + """Test various relative and absolute import file paths.""" + # write the parametrized index.rst content + index_path = Path(test_app.srcdir, "index.rst") + index_path.write_text(index_content) + + app = test_app + app.build() + assert app.statuscode == 0 + assert not app.warning_list + + html = Path(app.outdir, "index.html").read_text() + assert "TEST IMPORT TITLE" in html + assert "TEST_01" in html + + +@pytest.mark.parametrize("path_sep", ["/", "\\", "\\\\"]) +@pytest.mark.parametrize( + "test_app", + [ + { + "buildername": "html", + "files": [ + ("conf.py", 'extensions = ["sphinx_needs"]'), + ("needs.json", needs_json), + ], + "no_plantuml": True, + } + ], + indirect=True, +) +@pytest.mark.skipif(sys.platform != "win32", reason="Test only runs on Windows") +def test_import_abs_paths_win(test_app, path_sep): + """Test various relative and absolute import file paths.""" + # write the parametrized index.rst content + index_path = Path(test_app.srcdir, "index.rst") + json_abs_path = str((Path(test_app.srcdir).resolve() / "needs.json").absolute()) + json_abs_path = json_abs_path.replace(os.path.sep, path_sep) + index_path.write_text(f".. needimport:: {json_abs_path}") + + app = test_app + app.build() + assert app.statuscode == 0 + assert not app.warning_list + + html = Path(app.outdir, "index.html").read_text() + assert "TEST IMPORT TITLE" in html + assert "TEST_01" in html + + +@pytest.mark.parametrize( + "test_app", + [ + { + "buildername": "html", + "files": [ + ("conf.py", 'extensions = ["sphinx_needs"]'), + ("needs.json", needs_json), + ], + "no_plantuml": True, + } + ], + indirect=True, +) +@pytest.mark.skipif(sys.platform == "win32", reason="Test only runs on Linux and MacOS") +def test_import_abs_paths_lin_mac(test_app): + """Test various relative and absolute import file paths.""" + # write the parametrized index.rst content + index_path = Path(test_app.srcdir, "index.rst") + json_abs_path = str((Path(test_app.srcdir).resolve() / "needs.json").absolute()) + index_path.write_text(f".. needimport:: /{json_abs_path}") + + app = test_app + app.build() + assert app.statuscode == 0 + assert not app.warning_list + + html = Path(app.outdir, "index.html").read_text() + assert "TEST IMPORT TITLE" in html + assert "TEST_01" in html @pytest.mark.parametrize( diff --git a/tests/test_needs_external_needs_build.py b/tests/test_needs_external_needs_build.py index 408f21d72..45a95dddf 100644 --- a/tests/test_needs_external_needs_build.py +++ b/tests/test_needs_external_needs_build.py @@ -5,6 +5,8 @@ import pytest import responses from docutils import __version__ as doc_ver +from sphinx import version_info +from sphinx.testing.util import SphinxTestApp from sphinx.util.console import strip_colors @@ -13,13 +15,11 @@ [{"buildername": "html", "srcdir": "doc_test/doc_needs_external_needs"}], indirect=True, ) -def test_doc_build_html(test_app, sphinx_test_tempdir): +def test_doc_build_html(test_app: SphinxTestApp, sphinx_test_tempdir): import subprocess - app = test_app - - src_dir = Path(app.srcdir) - out_dir = Path(app.outdir) + src_dir = Path(test_app.srcdir) + out_dir = Path(test_app.outdir) plantuml = r"java -Djava.awt.headless=true -jar {}".format( os.path.join(sphinx_test_tempdir, "utils", "plantuml.jar") ) @@ -27,17 +27,27 @@ def test_doc_build_html(test_app, sphinx_test_tempdir): ["sphinx-build", "-b", "html", "-D", rf"plantuml={plantuml}", src_dir, out_dir], capture_output=True, ) - assert strip_colors(output.stderr.decode("utf-8")).splitlines() == [ + expected_warnings = [ "WARNING: http://my_company.com/docs/v1/index.html#TEST_01: Need 'EXT_TEST_01' has unknown outgoing link 'SPEC_1' in field 'links' [needs.external_link_outgoing]", "WARNING: ../../_build/html/index.html#TEST_01: Need 'EXT_REL_PATH_TEST_01' has unknown outgoing link 'SPEC_1' in field 'links' [needs.external_link_outgoing]", ] + assert strip_colors(output.stderr.decode("utf-8")).splitlines() == expected_warnings # run second time and check output_second = subprocess.run( ["sphinx-build", "-b", "html", "-D", rf"plantuml={plantuml}", src_dir, out_dir], capture_output=True, ) - assert not output_second.stderr + + # Sphinx 8.2 removed an early return in case no documents were updated in + # https://github.com/sphinx-doc/sphinx/pull/13236 + # which leads to some SN warnings not being emitted for incremental builds + if version_info < (8, 2): + expected_warnings = [] + assert ( + strip_colors(output_second.stderr.decode("utf-8")).splitlines() + == expected_warnings + ) # check if incremental build used # first build output @@ -46,6 +56,7 @@ def test_doc_build_html(test_app, sphinx_test_tempdir): in strip_colors(output.stdout.decode("utf-8")) ) # second build output + # TODO(Marco) check why 3 added configs are expected, should be 0 for incremental builds without changes assert "loading pickled environment" in output_second.stdout.decode("utf-8") assert ( "updating environment: [new config] 3 added, 0 changed, 0 removed" diff --git a/tests/test_needuml.py b/tests/test_needuml.py index 04b6c5ea3..801281992 100644 --- a/tests/test_needuml.py +++ b/tests/test_needuml.py @@ -126,6 +126,7 @@ def test_needuml_save_with_abs_path(test_app): srcdir = Path(app.srcdir) out_dir = srcdir / "_build" + # this fails before plantuml is required, so the plantuml path is not provided out = subprocess.run( ["sphinx-build", "-M", "html", srcdir, out_dir], capture_output=True ) @@ -133,7 +134,7 @@ def test_needuml_save_with_abs_path(test_app): assert ( "sphinx_needs.directives.needuml.NeedumlException: " - "Given save path: /_out/my_needuml.puml, is not a relative path." + "Given save path: /_out/my_needuml.puml, is not a relative posix path." in out.stderr.decode("utf-8") )