diff --git a/extra/tools/render_configuration_reference.py b/extra/tools/render_configuration_reference.py index 8a791f82f..f07ee244e 100644 --- a/extra/tools/render_configuration_reference.py +++ b/extra/tools/render_configuration_reference.py @@ -1,4 +1,5 @@ #!/usr/bin/env python +import inspect import sys from typing import Any, Iterator, List, Type from faust.types.settings import Settings @@ -27,9 +28,31 @@ class Rst: + def public_module(self, t: Type) -> str: + """The importable module a type should be referenced through. + + ``__module__`` is not always the name the documentation uses. Python + 3.13 moved :class:`pathlib.Path` into ``pathlib._local``, so + ``__module__`` became a private submodule and the rendered + ``:class:`~pathlib._local.Path``` resolved nowhere -- and the reference + changed depending on which interpreter ran the generator. + + Walk up the private components and take the shallowest package that + still exposes the very same object, so the reference stays public and + the output stays identical across versions. + """ + parts = t.__module__.split('.') + while len(parts) > 1 and parts[-1].startswith('_'): + parts.pop() + candidate = '.'.join(parts) + module = sys.modules.get(candidate) + if module is not None and getattr(module, t.__name__, None) is t: + return candidate + return t.__module__ + def to_ref(self, t: Type) -> str: name: str - module = t.__module__ + module = self.public_module(t) if module == 'builtins': return self._class(t.__name__) elif module == 'typing': @@ -108,42 +131,29 @@ def inforow(self, name: str, value: str) -> str: return f':{name}: {value}' def normalize_docstring_indent(self, text: str) -> str: - # docstring indentation starts at the second line - return self.normalize_indent(text, line_start=1) - - def normalize_indent(self, text: str, line_start: int = 0) -> str: - lines = text.splitlines() - if len(lines) <= 1: - return text - # take indent to remove from second line, - # since first line of docstring is not indented - non_whitespace_index: int = 0 - # find first line with text in it that is not whitespace - for line in lines[line_start:]: - if line and not line.isspace(): - # find index of first non-whitespace character - for i, c in enumerate(line): - if not c.isspace(): - non_whitespace_index = i - break - if non_whitespace_index: - break - if not non_whitespace_index: - return text - return '\n'.join( - self.strip_space(non_whitespace_index, line) - for line in lines - ) - - def strip_space(self, n: int, line: str) -> str: - sentinel = False - result = [] - for i, c in enumerate(line): - if not c.isspace() or i > n: - sentinel = True - if sentinel: - result.append(c) - return ''.join(result) + """Dedent a docstring, leaving relative indentation intact. + + :func:`inspect.cleandoc` rather than the hand-rolled scan this used to + do, which was wrong in two ways. + + It looked for the first *indented* line after the summary and stripped + that much from every line. On Python 3.12 and older that happened to + be the body indent, so it worked. Python 3.13 strips the common + leading whitespace from docstrings at compile time, so by the time the + scan runs the body is already flush left and the first indented line it + finds is the body of a ``.. warning::`` or ``.. note::`` -- whose + indent it then removed, breaking the directive: the content escaped the + admonition and rendered as ordinary paragraphs. + + It also stripped one character too many (``i > n`` where ``i >= n`` was + meant), which is why directive bodies in the committed reference sit at + three spaces rather than four. + + ``cleandoc`` dedents by the *minimum* indent instead, so it is a no-op + on an already-dedented docstring and produces byte-identical output on + every supported interpreter. + """ + return inspect.cleandoc(text) def reindent(self, new_indent: int, text: str) -> str: return '\n'.join( diff --git a/faust/types/settings/settings.py b/faust/types/settings/settings.py index 8f68b2cf4..04d54e558 100644 --- a/faust/types/settings/settings.py +++ b/faust/types/settings/settings.py @@ -414,7 +414,7 @@ def get_all_packages_to_scan(): params.Path, env_name="APP_DATADIR", default=DATADIR, - related_cli_options={"faust": "--datadir"}, + related_cli_options={"faust": ["--datadir"]}, ) def datadir(self, path: Path) -> Path: """Application data directory. @@ -462,7 +462,7 @@ def _prepare_tabledir(self, path: Path) -> Path: params.Bool, env_name="APP_DEBUG", default=False, - related_cli_options={"faust": "--debug"}, + related_cli_options={"faust": ["--debug"]}, ) def debug(self) -> bool: """Use in development to expose sensor information endpoint. @@ -608,7 +608,7 @@ def agent_supervisor(self) -> Type[SupervisorStrategyT]: params.Seconds, env_name="BLOCKING_TIMEOUT", default=None, - related_cli_options={"faust": "--blocking-timeout"}, + related_cli_options={"faust": ["--blocking-timeout"]}, ) def blocking_timeout(self) -> Optional[float]: """Blocking timeout (in seconds). diff --git a/tests/unit/test_configref_renderer.py b/tests/unit/test_configref_renderer.py new file mode 100644 index 000000000..4e99b3a3f --- /dev/null +++ b/tests/unit/test_configref_renderer.py @@ -0,0 +1,162 @@ +"""Tests for ``extra/tools/render_configuration_reference.py``. + +The script generates ``docs/includes/settingref.txt``, which +``docs/userguide/settings.rst`` includes -- so a bug in it silently corrupts +the published configuration reference rather than failing anything. Three did: + +* it dedented docstrings by scanning for the first *indented* line, which broke + once Python 3.13 started stripping common leading whitespace from docstrings + at compile time -- from then on the first indented line it found was the body + of a ``.. warning::``, whose indent it removed, so the content escaped the + admonition; +* it stripped one character too many while doing it; and +* it rendered types through ``__module__``, which for :class:`pathlib.Path` + became the private ``pathlib._local`` on 3.13. + +All three made the output depend on which interpreter ran the script, which is +what these tests exist to prevent. +""" + +import importlib.util +import pathlib + +import pytest + +RENDERER = ( + pathlib.Path(__file__).parents[2] + / "extra" + / "tools" + / "render_configuration_reference.py" +) + + +def _load_renderer(): + spec = importlib.util.spec_from_file_location("_configref", RENDERER) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.fixture(scope="module") +def rst(): + if not RENDERER.exists(): # pragma: no cover + pytest.skip("renderer not present") + return _load_renderer().Rst() + + +#: The same docstring as CPython <=3.12 and >=3.13 present it. 3.13 strips the +#: common leading whitespace at compile time, so the body arrives flush left. +DOCSTRING_INDENTED = ( + "Summary line.\n" + "\n" + " Body paragraph.\n" + "\n" + " .. warning::\n" + "\n" + " Nested directive body.\n" + " " +) +DOCSTRING_DEDENTED = ( + "Summary line.\n" + "\n" + "Body paragraph.\n" + "\n" + ".. warning::\n" + "\n" + " Nested directive body.\n" +) + + +def test_normalize_is_interpreter_independent(rst) -> None: + """The two docstring forms must render identically. + + Otherwise the generated reference depends on which Python ran `make + configref`, and every regeneration produces a spurious diff. + """ + assert rst.normalize_docstring_indent( + DOCSTRING_INDENTED + ) == rst.normalize_docstring_indent(DOCSTRING_DEDENTED) + + +@pytest.mark.parametrize( + "docstring", [DOCSTRING_INDENTED, DOCSTRING_DEDENTED], ids=["indented", "dedented"] +) +def test_directive_body_keeps_its_indent(rst, docstring) -> None: + """A ``.. warning::`` body must stay indented under the directive. + + This is the failure that mattered: flush-left content after a directive is + not part of it, so the admonition renders empty and its text becomes loose + paragraphs. + """ + out = rst.normalize_docstring_indent(docstring).splitlines() + + body = out.index(".. warning::") + nested = next(line for line in out[body + 1 :] if line.strip()) + indent = len(nested) - len(nested.lstrip()) + rendered = "\n".join(out) + assert ( + indent > 0 + ), f"directive body is flush left, so it escaped the warning:\n{rendered}" + + +@pytest.mark.parametrize( + "docstring", [DOCSTRING_INDENTED, DOCSTRING_DEDENTED], ids=["indented", "dedented"] +) +def test_body_is_dedented_to_column_zero(rst, docstring) -> None: + """Ordinary paragraphs must end up flush left. + + They are spliced into the page at top level; leaving them indented would + make RST read them as a block quote. + """ + out = rst.normalize_docstring_indent(docstring).splitlines() + assert out[0] == "Summary line." + assert out[2] == "Body paragraph." + + +def test_public_module_prefers_the_documented_name(rst) -> None: + """`pathlib.Path` must render as `pathlib.Path` on every version. + + 3.13 moved it to ``pathlib._local``; a reference to that resolves nowhere. + """ + assert rst.public_module(pathlib.Path) == "pathlib" + assert "._local" not in rst.to_ref(pathlib.Path) + + +def test_public_module_leaves_ordinary_types_alone(rst) -> None: + assert rst.public_module(int) == "builtins" + assert rst.public_module(pytest.ExceptionInfo).startswith("_pytest") + + +def test_related_cli_options_are_lists(rst) -> None: + """Every setting must declare its CLI options as a list, not a string. + + The renderer iterates the value, so a bare string is rendered one character + at a time -- ``:option:`faust -`, :option:`faust -`, :option:`faust d``` + and so on. The declared type is ``Mapping[str, List[str]]``, but the + decorator takes ``**kwargs: Any``, so nothing else catches this. + """ + from faust.types.settings import Settings + + bad = { + name: param.related_cli_options + for name, param in Settings.SETTINGS.items() + if param.related_cli_options + and any(isinstance(opts, str) for opts in param.related_cli_options.values()) + } + assert not bad, f"settings declaring CLI options as a bare string: {bad}" + + +def test_renders_without_error() -> None: + """The whole reference renders -- a smoke test over every real setting.""" + import io + + module = _load_renderer() + out = io.StringIO() + module.render(fh=out) + text = out.getvalue() + + assert ".. setting:: broker" in text + # The per-character regression, spelled out: `faust -` would appear if any + # setting's options were iterated as a string. + assert ":option:`faust -`" not in text