From 70b6e44f0ffba398acbe0a1cb7955eb940e54401 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 08:42:20 +0000 Subject: [PATCH] Fix the configuration-reference generator `extra/tools/render_configuration_reference.py` generates docs/includes/settingref.txt, which docs/userguide/settings.rst includes. Nothing validates its output, so a bug there silently corrupts the published configuration reference instead of failing something. Three did, and together they made the output depend on which interpreter ran `make configref` -- which is why regenerating produced a tree-wide diff rather than a clean one. ## Docstring dedent, broken by Python 3.13 `normalize_indent` dedented by scanning for the first *indented* line after the summary and stripping that much from every line. On 3.12 and older that was the body indent, so it worked. Python 3.13 strips the common leading whitespace from docstrings at compile time. From then on the body arrives flush left, and the first indented line the scan finds is the body of a `.. warning::` or `.. note::` -- whose indent it then removed, so the content escaped the directive and rendered as loose paragraphs instead of an admonition. Replaced with `inspect.cleandoc`, which dedents by the *minimum* indent: a no-op on an already-dedented docstring, and byte-identical output on every supported interpreter. That also fixes an off-by-one in `strip_space` (`i > n` where `i >= n` was meant), which is why directive bodies in the committed reference sit at three spaces rather than four. ## `pathlib._local` on 3.13+ Types were referenced through `__module__`. 3.13 moved `pathlib.Path` into `pathlib._local`, so the reference became `:class:`~pathlib._local.Path``, which resolves nowhere -- and changed depending on the interpreter. `public_module()` now walks up private components and takes the shallowest package that still exposes the same object. ## CLI options rendered one character at a time Three settings declared `related_cli_options={"faust": "--datadir"}`, a bare string where `Mapping[str, List[str]]` is declared. The renderer iterates the value, so `--datadir` became nine separate options: :option:`faust -`, :option:`faust -`, :option:`faust d`, ... `Section.setting()` takes `**kwargs: Any`, so the declared type is erased and mypy cannot catch it -- hence the test below rather than a type fix alone. ## Tests tests/unit/test_configref_renderer.py covers the normalization against both docstring forms, the directive-body indent, the module reference, and a smoke render of every real setting. It also asserts no setting declares its CLI options as a bare string. Verified in both directions: 6 of the 9 fail against the original generator on 3.12 and on 3.13; all 9 pass after. The generator now produces byte-identical output on 3.12, 3.13 and free-threaded 3.14. ## Not done here The committed settingref.txt is deliberately left untouched, because regenerating it needs two unrelated decisions first: * it contains a hand-written "OAuth2 Authentication" block that exists nowhere in the source docstrings, so regenerating would delete real documentation. It belongs in `broker_credentials.__doc__`. * `broker_client_id` defaults to `f"faust-{faust_version}"`, so the rendered default embeds whichever version generated it -- the committed file says `faust-0.8.9`, a dev tree renders `faust-0.1.dev193+...`. Until that is symbolic the file cannot be regenerated reproducibly, and no byte-exact CI drift check can work. Worth knowing meanwhile: `python extra/tools/...` puts the script's own directory on sys.path, not the repository root, so the generator renders whichever faust is *installed*. With a non-editable install it silently renders a stale copy. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01K8qT5E3rnSXvw7ibNLXrVr --- extra/tools/render_configuration_reference.py | 84 +++++---- faust/types/settings/settings.py | 6 +- tests/unit/test_configref_renderer.py | 162 ++++++++++++++++++ 3 files changed, 212 insertions(+), 40 deletions(-) create mode 100644 tests/unit/test_configref_renderer.py 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