From 8229d4375b9eb7516404ed69429958d26f563242 Mon Sep 17 00:00:00 2001 From: jichuanh Date: Thu, 14 May 2026 18:12:16 +0000 Subject: [PATCH] [Changelog] Parse the rendered fragment-merge with docutils to mirror doc build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR gate now compiles every accumulated fragment for a touched package through the same FragmentBatch._merge_sections + _format_entry the nightly bot uses, then parses the resulting RST block with the same docutils engine that backs sphinx-build. Any WARNING+ on the merged output fails the gate with the exact diagnostic Sphinx would emit at doc-build time, so structural drift between the gate's checks and the doc build's checks is bounded by Sphinx-specific extensions only (currently empty for fragment content). The previous regex check rejected orphan paragraphs anywhere inside a section body — over-strict in isolation (false-positive on prose-intro shapes that compile cleanly) and incomplete in coverage (silent on unclosed inline literals, under-indented continuations, and other RST defects that the regex shape can't see). The docutils-backed lint is both narrower (only flags what the doc build would flag) and broader (catches every structural class, not just the orphan-paragraph one). Validator helpers (_register_sphinx_role_stubs + lint) are grouped on FragmentBatch._CompiledLinter because they exist solely to support FragmentBatch.validate_compile_output — the role-stub registration is a required precondition of the lint, never called independently. Keeping them in a shared namespace makes the relationship explicit. Verified: - 105 tests pass (24 prior validate + 18 new compile-lint cases incl. empty/skip-only/clean-single/seam/inline-defect/role-stub coverage + the rest of the changelog suite). - Regression: validator on the actual bot-input state at b65a1ac2b73^ flags 'line 35: Unexpected indentation' — the exact diagnostic sphinx-build -W emitted on the merged CHANGELOG.rst. - False-positive scan across 49 historical bot-input batches: only the one known-bad batch (b65a1ac2b73, the #5400 incident) is flagged. - pre-commit clean on all touched files. --- .github/workflows/changelog-check.yml | 6 + tools/changelog/cli.py | 164 ++++++-- tools/changelog/test/invalid_content/3004.rst | 9 - tools/changelog/test/test_compile_lint.py | 370 ++++++++++++++++++ tools/changelog/test/test_validate.py | 27 +- 5 files changed, 538 insertions(+), 38 deletions(-) delete mode 100644 tools/changelog/test/invalid_content/3004.rst create mode 100644 tools/changelog/test/test_compile_lint.py diff --git a/.github/workflows/changelog-check.yml b/.github/workflows/changelog-check.yml index 945d6139f43b..dbf4dd570146 100644 --- a/.github/workflows/changelog-check.yml +++ b/.github/workflows/changelog-check.yml @@ -44,5 +44,11 @@ jobs: - name: Fetch base branch run: git fetch origin ${{ steps.base.outputs.ref }} + - name: Install docutils + # Used by tools/changelog/cli.py to parse the rendered fragment-merge + # with the same engine ``sphinx-build`` uses, so structural RST issues + # that would fail the doc build are caught at the PR gate. + run: pip install docutils + - name: Verify changelog fragments run: python3 tools/changelog/cli.py check ${{ steps.base.outputs.ref }} diff --git a/tools/changelog/cli.py b/tools/changelog/cli.py index d316a416f89f..95e8e8929fe4 100644 --- a/tools/changelog/cli.py +++ b/tools/changelog/cli.py @@ -299,28 +299,6 @@ def validate(self) -> str | None: f"section(s) {', '.join(repr(s) for s in empty)} have no bullet entries — " "use ``* `` to start each entry, or remove the heading" ) - # Every line inside a section body must be a bullet (``* ``), a - # continuation (leading whitespace), or blank. A column-0 non-blank - # line that isn't a bullet terminates the list under RST rules and - # then sits as a paragraph adjacent to the next ``* `` — which the - # compile step splices into ``CHANGELOG.rst`` under the same - # ``^^^`` subheading and Sphinx then rejects with - # ``Unexpected indentation``. Catch it here before merge. - for section, lines in sections.items(): - for offset, line in enumerate(lines): - if not line.strip(): - continue - if line[0].isspace() or line.lstrip().startswith("*"): - continue - snippet = line.strip()[:80] - return ( - f"section {section!r} contains an orphan paragraph " - f"(non-bullet line {offset + 1}: {snippet!r}). Every line under " - "a section heading must start with ``* `` (new bullet) or whitespace " - "(continuation of the previous bullet). A flush-left paragraph here " - "splits the bullet list and Sphinx fails the doc build with " - "``Unexpected indentation``." - ) return None @@ -399,6 +377,136 @@ def merged_sections(self) -> dict[str, list[str]]: """Cross-fragment merged section map for this batch.""" return self._merge_sections([s for _, s in self.parsed()]) + def validate_compile_output(self, package_name: str) -> str | None: + """Lint the rendered fragment-merge output with docutils. + + Compiles every parsed fragment via :meth:`_merge_sections` + + :meth:`_format_entry` into the exact RST block the nightly bot + would prepend to ``CHANGELOG.rst``, then parses that block with + the same engine the doc build uses. Any docutils WARNING+ on the + compiled output would also fail ``sphinx-build -W`` downstream, + so flagging here keeps doc-build green. + + Returns ``None`` on a clean parse, or a human-readable error + string naming the package and listing each diagnostic with its + line number in the would-be entry. + """ + if not self.valid: + return None # nothing to compile yet + entry = self._format_entry("0.0.0", self.merged_sections()) + errors = self._CompiledLinter.lint(entry, f"<{package_name} compiled fragments>") + if not errors: + return None + joined = "\n ".join(errors) + return ( + f"package {package_name!r}: compiled fragment output would fail " + f"the doc build (docutils WARNING+ on the merged section text). " + f"This usually means two fragments collide at a section seam (e.g. " + f"one ends a section with a paragraph, the next starts with a " + f"bullet) or a single fragment has malformed RST (unclosed ``...``, " + f"under-indented continuation, etc.). Diagnostics:\n {joined}" + ) + + class _CompiledLinter: + """docutils-based lint of the rendered fragment-merge output. + + Nested on :class:`FragmentBatch` to keep the two cooperating helpers + (:meth:`_register_sphinx_role_stubs`, :meth:`lint`) in the same + namespace — they serve a single purpose (parse a compiled fragment + block exactly the way ``sphinx-build`` would) and the role-stub + registration must run before :meth:`lint` parses anything. + """ + + # Sphinx interpreted-text roles used across IsaacLab fragments. + # Vanilla docutils doesn't know any of these, so without stubs every + # ``:class:`` / ``:meth:`` / ``:attr:`` etc. would emit a spurious + # ``Unknown interpreted text role`` warning that drowns out the real + # structural issues we want to catch. + SPHINX_STUB_ROLES = ( + "attr", + "class", + "meth", + "func", + "mod", + "data", + "exc", + "obj", + "paramref", + "ref", + "doc", + "term", + "kbd", + "file", + "envvar", + ) + + @classmethod + def _register_sphinx_role_stubs(cls) -> None: + """Register Sphinx roles as no-op literals so docutils doesn't warn. + + Idempotent — docutils' registry stores the latest registration per + name and re-registering the same stub is harmless. + """ + from docutils import nodes + from docutils.parsers.rst import roles + + def _stub(role, rawtext, text, lineno, inliner, options=None, content=None): + return [nodes.literal(rawtext, text)], [] + + for name in cls.SPHINX_STUB_ROLES: + roles.register_local_role(name, _stub) + + @classmethod + def lint(cls, text: str, source_label: str) -> list[str]: + """Parse ``text`` as RST and return WARNING+ system messages. + + The parser is the same engine that backs ``sphinx-build``, so + structural issues docutils flags here would also fail the doc + build under ``-W``. Sphinx roles are pre-registered as no-ops + via :meth:`_register_sphinx_role_stubs`. + + Args: + text: RST source — typically the output of + :meth:`FragmentBatch._format_entry` (the would-be + ``CHANGELOG.rst`` entry the bot will write next). + source_label: Human-readable label that appears in + docutils' error messages. + + Returns: + Deduplicated list of error strings ``"line N: "``; + empty list means clean. + """ + import io # local — keep cli.py importable when docutils is absent + + from docutils import frontend, utils + from docutils.parsers.rst import Parser + + cls._register_sphinx_role_stubs() + + settings = frontend.OptionParser(components=(Parser,)).get_default_values() + settings.report_level = 0 # collect every diagnostic; filter below + settings.halt_level = 5 # never halt + settings.warning_stream = io.StringIO() # suppress stderr; we observe + + doc = utils.new_document(source_label, settings) + collected: list = [] + # Observe every system_message the reporter emits, even ones the + # parser swallows during recovery (e.g. inline literal warnings). + doc.reporter.attach_observer(lambda msg: collected.append(msg)) + Parser().parse(text, doc) + + seen: set[str] = set() + out: list[str] = [] + for msg in collected: + if int(msg["level"]) < 2: + continue # WARNING (2), ERROR (3), SEVERE (4) only + line = msg.get("line", "?") + entry = f"line {line}: {msg.astext()}" + if entry not in seen: + seen.add(entry) + out.append(entry) + return out + def compile_to_entry( self, current_version: Version, @@ -818,10 +926,20 @@ def evaluate( continue added_slugs[slug] = path.name + # Rule 4: the rendered fragment-merge must parse cleanly with docutils. + # Only run when this PR touches the package's changelog.d/ — otherwise + # the compiled output is identical to what already shipped on the base + # branch and is the next bot run's problem, not this PR's. Run after + # all per-fragment checks so the lint sees only well-formed inputs. + if added_slugs: + lint_err = FragmentBatch.from_dir(existing_dir).validate_compile_output(pkg.name) + if lint_err: + invalid_fragments.append((f"source/{pkg.name}/changelog.d/", lint_err)) + if not source_changed: continue - # Rule 4: this PR must add at least one valid fragment for the package. + # Rule 5: this PR must add at least one valid fragment for the package. owned = [ f for f in fragment_changes diff --git a/tools/changelog/test/invalid_content/3004.rst b/tools/changelog/test/invalid_content/3004.rst deleted file mode 100644 index 57aa8df9bb87..000000000000 --- a/tools/changelog/test/invalid_content/3004.rst +++ /dev/null @@ -1,9 +0,0 @@ -Added -^^^^^ - -* Added ``foo()`` to support feature X. - -This is a free-form paragraph that lives at column 0 inside the Added -section, neither a bullet nor a continuation of the previous bullet. -The bot would splice this verbatim into ``CHANGELOG.rst`` and Sphinx -then rejects the build with ``Unexpected indentation [docutils]``. diff --git a/tools/changelog/test/test_compile_lint.py b/tools/changelog/test/test_compile_lint.py new file mode 100644 index 000000000000..6ec5f7e5d3df --- /dev/null +++ b/tools/changelog/test/test_compile_lint.py @@ -0,0 +1,370 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Compile-output lint — the docutils-backed PR-gate check. + +These tests exercise :meth:`cli.FragmentBatch._CompiledLinter.lint` and +:meth:`cli.FragmentBatch.validate_compile_output` at two layers: + +1. **Direct unit tests** that feed synthetic RST text into the helper and + assert what docutils flags. These are fast and isolate the lint logic + from filesystem / git state. +2. **Integration tests** that build a real :class:`cli.FragmentBatch` + from a temp directory and verify the wired-up batch method. The + batch tests pass parsed-section dicts directly through + :meth:`cli.FragmentBatch._merge_sections` to bypass the git-merge-time + sort (which is unavailable in a temp dir without git history). +""" + +from __future__ import annotations + +from pathlib import Path + +import cli +import pytest + +# --------------------------------------------------------------------------- +# Helper-level tests — direct docutils integration +# --------------------------------------------------------------------------- + + +def test_lint_accepts_clean_compiled_output(): + """A well-formed version block parses cleanly.""" + text = ( + "1.2.3 (2026-05-14)\n" + "~~~~~~~~~~~~~~~~~~\n\n" + "Added\n^^^^^\n\n" + "* Added :class:`~pkg.Foo`.\n" + "* Added :meth:`~pkg.Foo.bar`.\n\n" + "Changed\n^^^^^^^\n\n" + "* Renamed :attr:`old` to :attr:`new`.\n" + ) + assert cli.FragmentBatch._CompiledLinter.lint(text, "test") == [] + + +def test_lint_rejects_orphan_paragraph_seam(): + """The exact #5400 incident shape: bullet list, blank line, paragraph, + then a multi-line bullet directly underneath the paragraph (no blank line). + The post-paragraph bullet must have a 2-space-indented continuation — + that continuation line is what docutils reads as ``Unexpected + indentation`` because the parser is treating the paragraph as the active + block and the indented continuation breaks list-vs-paragraph parsing.""" + text = ( + "1.2.3 (2026-05-14)\n" + "~~~~~~~~~~~~~~~~~~\n\n" + "Added\n^^^^^\n\n" + "* First bullet.\n" + "* Second bullet.\n\n" + "Orphan paragraph between bullets and the next bullet group.\n" + "* Third bullet with multi-line content where the\n" + " continuation line is indented two spaces.\n" + ) + errors = cli.FragmentBatch._CompiledLinter.lint(text, "test") + assert errors, "expected docutils to flag the orphan-then-bullet seam" + assert any("indentation" in e.lower() for e in errors) + + +def test_lint_rejects_unclosed_inline_literal(): + """An unclosed ``...`` literal in a bullet — Layer 1's regex misses this + because the bullet shape is fine; docutils does catch it.""" + text = ( + "1.2.3 (2026-05-14)\n" + "~~~~~~~~~~~~~~~~~~\n\n" + "Added\n^^^^^\n\n" + "* Added support for ``BaseArticulation.body_link_jacobian_w property\n" + " for task-space controllers.\n" + "* Added other thing.\n" + ) + errors = cli.FragmentBatch._CompiledLinter.lint(text, "test") + assert errors, "expected docutils to flag the unclosed literal" + assert any("literal" in e.lower() and "end-string" in e.lower() for e in errors) + + +def test_lint_rejects_under_indented_continuation(): + """A continuation line with 1-space indent instead of 2 breaks list parsing.""" + text = ( + "1.2.3 (2026-05-14)\n" + "~~~~~~~~~~~~~~~~~~\n\n" + "Added\n^^^^^\n\n" + "* Added a feature with a long description that wraps to a continuation\n" + " line indented only one space instead of two.\n" + ) + errors = cli.FragmentBatch._CompiledLinter.lint(text, "test") + assert errors, "expected docutils to flag the under-indented continuation" + assert any(("unindent" in e.lower() or "indentation" in e.lower()) for e in errors) + + +def test_lint_accepts_sphinx_roles_as_stubs(): + """Sphinx roles (:attr:, :class:, :meth:, :paramref:, etc.) are pre-registered + as no-op stubs so docutils doesn't emit ``Unknown interpreted text role`` noise. + Without the stubs, every fragment using these roles would false-positive.""" + text = ( + "1.2.3 (2026-05-14)\n" + "~~~~~~~~~~~~~~~~~~\n\n" + "Added\n^^^^^\n\n" + "* Added :attr:`~pkg.Foo.bar`, :class:`~pkg.Foo`, :meth:`~pkg.Foo.qux`,\n" + " :func:`~pkg.helper`, :mod:`pkg.sub`, :data:`pkg.CONST`,\n" + " :exc:`pkg.Err`, :obj:`pkg.thing`, :paramref:`~pkg.Foo.bar`, :ref:`label`.\n" + ) + assert cli.FragmentBatch._CompiledLinter.lint(text, "test") == [] + + +def test_lint_accepts_prose_intro_then_bullets(): + """The ``isaaclab_rl 0.1.0`` initial-version shape: prose paragraph, + blank line, then a bullet list. Valid RST; Layer 1's regex would have + rejected this but Layer 2 correctly accepts it.""" + text = ( + "0.1.0 (2024-12-27)\n" + "~~~~~~~~~~~~~~~~~~\n\n" + "Added\n^^^^^\n\n" + "Initial version of the extension.\n" + "This extension is split off from ``other_pkg`` to include the wrapper\n" + "scripts for the supported RL libraries.\n\n" + "Supported RL libraries are:\n\n" + "* RL Games\n" + "* RSL RL\n" + "* SKRL\n" + "* Stable Baselines3\n" + ) + assert cli.FragmentBatch._CompiledLinter.lint(text, "test") == [] + + +def test_lint_returns_deduplicated_messages(): + """Multiple emissions of the same diagnostic collapse to a single entry.""" + text = ( + "1.2.3 (2026-05-14)\n" + "~~~~~~~~~~~~~~~~~~\n\n" + "Added\n^^^^^\n\n" + "* First with ``unclosed and\n" + "* Second with ``also unclosed and\n" + ) + errors = cli.FragmentBatch._CompiledLinter.lint(text, "test") + # We don't assert an exact count (docutils' emission count is internal) + # but the same (line, text) pair must never appear twice. + assert len(errors) == len(set(errors)) + + +# --------------------------------------------------------------------------- +# Batch-level tests — FragmentBatch.validate_compile_output +# --------------------------------------------------------------------------- + + +def _write(d: Path, name: str, body: str) -> Path: + p = d / name + p.write_text(body, encoding="utf-8") + return p + + +def test_validate_compile_output_empty_batch_is_clean(tmp_path): + """A directory with only ``.gitkeep`` / no fragments produces no entry to lint.""" + (tmp_path / ".gitkeep").touch() + batch = cli.FragmentBatch.from_dir(tmp_path) + assert batch.validate_compile_output("pkg") is None + + +def test_validate_compile_output_clean_single_fragment(tmp_path): + """A well-formed single fragment compiles to clean RST.""" + _write( + tmp_path, + "1234.rst", + "Added\n^^^^^\n\n* Added :class:`~pkg.Foo`.\n", + ) + batch = cli.FragmentBatch.from_dir(tmp_path) + assert batch.validate_compile_output("pkg") is None + + +def test_validate_compile_output_isolated_orphan_paragraph_is_clean(tmp_path): + """The #5400 fragment *alone* compiles to a valid block (paragraph followed + by next section underline = legal RST). The bug only forms when a sibling + fragment adds bullets to the same section.""" + _write( + tmp_path, + "1234.rst", + ( + "Added\n^^^^^\n\n" + "* First bullet.\n" + "* Second bullet.\n\n" + "Trailing paragraph at column 0.\n\n" + "Changed\n^^^^^^^\n\n" + "* Some change.\n" + ), + ) + batch = cli.FragmentBatch.from_dir(tmp_path) + assert batch.validate_compile_output("pkg") is None + + +def test_validate_compile_output_catches_merge_seam(): + """Two fragments individually OK, merged output broken — the actual #5400 + failure mode. Tests :meth:`_merge_sections` + :meth:`_format_entry` + + :meth:`_CompiledLinter.lint` end-to-end without depending on git merge-time + ordering (we hand the section dicts directly to :meth:`_merge_sections`).""" + frag_a_sections = { + "Added": [ + "* First bullet from A.", + "* Second bullet from A.", + "", + "Trailing paragraph from fragment A.", + "Second line of that paragraph.", + ], + } + frag_b_sections = { + "Added": [ + # Must be multi-line — the continuation indent is what docutils + # actually flags as ``Unexpected indentation``. + "* Bullet from B that has multi-line content with a", + " continuation line indented two spaces.", + ], + } + merged = cli.FragmentBatch._merge_sections([frag_a_sections, frag_b_sections]) + entry = cli.FragmentBatch._format_entry("1.2.3", merged) + errors = cli.FragmentBatch._CompiledLinter.lint(entry, "") + assert errors, "expected docutils to flag the orphan-then-bullet seam" + assert any("indentation" in e.lower() for e in errors) + + +def test_validate_compile_output_catches_single_fragment_with_inline_defect(tmp_path): + """A single fragment with an unclosed inline literal — Layer 1's regex + accepts it (bullet shape is fine), Layer 2 rejects via docutils.""" + _write( + tmp_path, + "1234.rst", + ( + "Added\n^^^^^\n\n" + "* Added support for ``BaseArticulation.body_link_jacobian_w property\n" + " for task-space controllers.\n" + ), + ) + batch = cli.FragmentBatch.from_dir(tmp_path) + err = batch.validate_compile_output("pkg") + assert err is not None + assert "pkg" in err + assert "literal" in err.lower() + + +def test_validate_compile_output_error_names_package(tmp_path): + """Error message includes the package name so authors can attribute it.""" + _write( + tmp_path, + "1234.rst", + "Added\n^^^^^\n\n* First.\n* Second with ``unclosed literal\n", + ) + batch = cli.FragmentBatch.from_dir(tmp_path) + err = batch.validate_compile_output("my_specific_package") + assert err is not None + assert "'my_specific_package'" in err + + +def test_validate_compile_output_skip_only_batch_is_clean(tmp_path): + """A batch with only ``.skip`` fragments has nothing to compile.""" + _write(tmp_path, "ci-only.skip", "") + batch = cli.FragmentBatch.from_dir(tmp_path) + assert batch.valid == [] + assert batch.validate_compile_output("pkg") is None + + +# --------------------------------------------------------------------------- +# Gate-orchestration integration — PRDiff.evaluate wires the new check in +# --------------------------------------------------------------------------- + + +def _pkg(tmp_path: Path, name: str) -> cli.Package: + """Build a managed Package at ``tmp_path/source//``.""" + root = tmp_path / "source" / name + (root / "config").mkdir(parents=True) + (root / "docs").mkdir(parents=True) + (root / "config" / "extension.toml").write_text('version = "0.0.0"\n', encoding="utf-8") + (root / "docs" / "CHANGELOG.rst").write_text("Changelog\n---------\n\n", encoding="utf-8") + return cli.Package(root) + + +def test_evaluate_passes_a_clean_pr_fragment(tmp_path): + """Sanity: a clean PR fragment + clean existing fragments → no errors.""" + pkg = _pkg(tmp_path, "isaaclab") + (pkg.root / "changelog.d").mkdir() + _write(pkg.root / "changelog.d", "alice-feature.rst", "Added\n^^^^^\n\n* Alice feature.\n") + _write(pkg.root / "changelog.d", "bob-feature.rst", "Added\n^^^^^\n\n* Bob feature.\n") + changed = { + "source/isaaclab/code.py", + "source/isaaclab/changelog.d/bob-feature.rst", + } + added = {"source/isaaclab/changelog.d/bob-feature.rst"} + # Monkey-patch REPO_ROOT so Fragment(REPO_ROOT / f).validate() reads our temp tree. + import unittest.mock + + with unittest.mock.patch.object(cli, "REPO_ROOT", tmp_path): + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + assert missing == [] + assert invalid == [] + + +def test_evaluate_flags_compiled_output_failure(tmp_path): + """When the PR-merged fragment shape would break the doc build, the gate + rejects with the new compile-output-failure error path.""" + pkg = _pkg(tmp_path, "isaaclab") + (pkg.root / "changelog.d").mkdir() + _write( + pkg.root / "changelog.d", + "bad-pr.rst", + "Added\n^^^^^\n\n* Has ``unclosed literal in this bullet.\n", + ) + changed = { + "source/isaaclab/code.py", + "source/isaaclab/changelog.d/bad-pr.rst", + } + added = {"source/isaaclab/changelog.d/bad-pr.rst"} + import unittest.mock + + with unittest.mock.patch.object(cli, "REPO_ROOT", tmp_path): + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + # The bad fragment isn't malformed at the per-fragment level (shape is OK), + # so it slips past Rule 2 and is caught by the new Rule 4 (compile-output lint). + assert any("compiled fragment output" in r for _, r in invalid), ( + f"expected compile-output error in invalid_fragments, got {invalid!r}" + ) + + +def test_evaluate_skips_compile_lint_when_pr_does_not_touch_fragments(tmp_path): + """Source-only PRs that don't add fragments don't trigger the compile lint + (the existing on-base fragments are not this PR's responsibility).""" + pkg = _pkg(tmp_path, "isaaclab") + (pkg.root / "changelog.d").mkdir() + # Pre-existing fragment with a defect; PR doesn't add any fragment. + _write( + pkg.root / "changelog.d", + "preexisting-bad.rst", + "Added\n^^^^^\n\n* ``unclosed.\n", + ) + changed = {"source/isaaclab/code.py"} # no changelog.d changes + added = set() + import unittest.mock + + with unittest.mock.patch.object(cli, "REPO_ROOT", tmp_path): + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + # The defective preexisting fragment is not flagged; only "missing fragment for + # this package" is reported (Rule 5). + assert missing == ["isaaclab"] + assert invalid == [] # compile-lint did not run + + +@pytest.mark.parametrize( + "section_body, expected_clean", + [ + # Valid shapes + (["* one", "* two"], True), + (["* one", " continuation", "* two"], True), + (["* one with :class:`~pkg.Foo` ref."], True), + # Invalid shapes + (["* one", "* two with ``unclosed and", " continuation"], False), + (["* one", " under-indented continuation"], False), + ], +) +def test_lint_compiled_output_parametrized(section_body, expected_clean): + """Compact matrix of known-good / known-bad shapes against the compile lint.""" + text = "1.2.3 (2026-05-14)\n~~~~~~~~~~~~~~~~~~\n\nAdded\n^^^^^\n\n" + "\n".join(section_body) + "\n" + errors = cli.FragmentBatch._CompiledLinter.lint(text, "test") + if expected_clean: + assert errors == [], f"expected clean parse, got {errors}" + else: + assert errors, f"expected docutils to flag, got clean parse for: {section_body}" diff --git a/tools/changelog/test/test_validate.py b/tools/changelog/test/test_validate.py index fd1af7a31dfe..e4f7a07f2eca 100644 --- a/tools/changelog/test/test_validate.py +++ b/tools/changelog/test/test_validate.py @@ -70,12 +70,27 @@ def test_validate_rejects_section_without_bullets_from_fixture(): assert err is not None and "bullet" in err.lower() -def test_validate_rejects_orphan_paragraph_from_fixture(): - """A flush-left paragraph between bullets / after the last bullet must be - rejected — the compile step would splice it verbatim into ``CHANGELOG.rst`` - and Sphinx then fails the doc build with ``Unexpected indentation``.""" - err = cli.Fragment(FIXTURES / "invalid_content" / "3004.rst").validate() - assert err is not None and "orphan" in err.lower() +def test_validate_accepts_orphan_paragraph_in_isolation(): + """A single fragment with a paragraph between its bullets and the next + section header is valid RST (Sphinx accepts it). The structural bug + only appears when the merged ``CHANGELOG.rst`` chains the orphan-tail + of fragment A onto the bullet-head of fragment B with no blank line; + that case lives in the batch-level check, not here.""" + body = ( + "Added\n^^^^^\n\n" + "* First bullet.\n" + "* Second bullet.\n\n" + "This is a stand-alone note about the additions above.\n" + "\n" + "Changed\n^^^^^^^\n\n" + "* Something else.\n" + ) + import tempfile + + with tempfile.TemporaryDirectory() as d: + p = Path(d) / "1234.rst" + p.write_text(body, encoding="utf-8") + assert cli.Fragment(p).validate() is None # ---------------------------------------------------------------------------