diff --git a/.github/workflows/changelog-check.yml b/.github/workflows/changelog-check.yml new file mode 100644 index 000000000000..945d6139f43b --- /dev/null +++ b/.github/workflows/changelog-check.yml @@ -0,0 +1,48 @@ +# 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 + +name: Changelog Fragment Check + +on: + pull_request: + types: [opened, synchronize, reopened] + workflow_dispatch: + inputs: + base_ref: + description: 'Base branch to diff against' + required: true + default: 'develop' + +concurrency: + group: changelog-check-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + check-fragments: + name: Check changelog fragments + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + # Full history needed to diff against the base branch + fetch-depth: 0 + + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: "3.12" + + - name: Resolve base ref + id: base + run: echo "ref=${{ github.event.inputs.base_ref || github.base_ref }}" >> "$GITHUB_OUTPUT" + + - name: Fetch base branch + run: git fetch origin ${{ steps.base.outputs.ref }} + + - name: Verify changelog fragments + run: python3 tools/changelog/cli.py check ${{ steps.base.outputs.ref }} diff --git a/.github/workflows/nightly-changelog.yml b/.github/workflows/nightly-changelog.yml new file mode 100644 index 000000000000..7e55ad7450c6 --- /dev/null +++ b/.github/workflows/nightly-changelog.yml @@ -0,0 +1,119 @@ +# 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 + +# Nightly auto-compile: rolls accumulated fragments under +# ``source//changelog.d/`` into per-package ``CHANGELOG.rst`` entries, +# bumps each ``extension.toml``, deletes consumed fragments, and pushes the +# result back to ``develop``. Keeps the develop branch's changelog current +# without requiring a maintainer to run ``compile`` by hand. +# +# The push uses ``CHANGELOG_PAT`` (a personal access token / fine-grained +# GitHub App token with ``contents:write`` on this repo) when it's +# available so downstream CI runs on the auto-commit. Falls back to +# ``GITHUB_TOKEN`` — sufficient for the push itself, but pushes signed +# with ``GITHUB_TOKEN`` do not trigger workflow runs on the resulting +# commit, which is by design (avoids infinite loops) but means the +# Docker / docs rebuild won't re-trigger off the nightly's auto-commit. + +name: Nightly Changelog Compilation + +on: + schedule: + # Run nightly at 5 AM UTC (one hour after daily-compatibility, so we + # don't compete for runner capacity). + - cron: '0 5 * * *' + workflow_dispatch: + inputs: + dry_run: + description: 'Preview only — do not commit / push' + required: false + type: boolean + default: false + +concurrency: + # Only one nightly compile may be in flight at a time. ``cancel-in-progress`` + # is intentionally false: if a previous run is still finishing its push, we + # queue rather than abort it mid-commit. + group: nightly-changelog + cancel-in-progress: false + +permissions: + contents: write + +jobs: + compile-changelog: + name: Compile changelog fragments + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + # Operate on develop, not the repo's default branch. Scheduled + # workflows fire from the default branch's workflow file by + # default, but we want the *checkout* to be develop so the + # compile sees develop's accumulated fragments and the push + # writes back to develop. + ref: develop + # Use a PAT so the auto-commit triggers downstream CI; falls back + # to GITHUB_TOKEN which is sufficient for the push itself. + token: ${{ secrets.CHANGELOG_PAT || secrets.GITHUB_TOKEN }} + # Full history so the compiler can resolve each fragment's merge + # time via ``git log --diff-filter=A --first-parent``. + fetch-depth: 0 + + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: "3.12" + + - name: Compile fragments + run: | + ARGS="--all" + if [ "${{ inputs.dry_run }}" = "true" ]; then + ARGS="$ARGS --dry-run" + fi + echo "Running: python3 tools/changelog/cli.py compile $ARGS" + python3 tools/changelog/cli.py compile $ARGS + + - name: Commit and push if fragments were compiled + if: inputs.dry_run != 'true' + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add source/*/changelog.d/ \ + source/*/docs/CHANGELOG.rst \ + source/*/config/extension.toml + if git diff --staged --quiet; then + echo "No changelog fragments found — nothing to commit." + else + # Convention for CI-driven auto-commits on this repo: + # [CI][] + # The leading ``[CI]`` tag groups every machine-driven commit + # (so future automations — auto image rebuilds, auto publish, + # etc. — all share the prefix and are easy to find/filter in + # ``git log --grep``). The second tag names the specific + # action. The trigger event suffix (``schedule`` vs + # ``workflow_dispatch``) is preserved for traceability. + # + # The body lists every package that bumped, derived from the + # staged ``extension.toml`` diff so the entries are accurate + # regardless of which packages happen to have pending + # fragments this run. + MSG_FILE=$(mktemp) + { + echo "[CI][Auto Version Bump] Compile changelog fragments (${{ github.event_name }})" + echo + echo "Bumped packages:" + for tom in $(git diff --staged --name-only -- 'source/*/config/extension.toml'); do + pkg=$(echo "$tom" | sed -E 's|source/([^/]+)/config/extension.toml|\1|') + old=$(git diff --staged "$tom" | awk -F'"' '/^-version/{print $2; exit}') + new=$(git diff --staged "$tom" | awk -F'"' '/^\+version/{print $2; exit}') + echo "- $pkg: $old → $new" + done + } > "$MSG_FILE" + git commit -F "$MSG_FILE" + # Push explicitly to develop so we don't accidentally write + # to the source ref of a workflow_dispatch run. + git push origin HEAD:develop + fi diff --git a/AGENTS.md b/AGENTS.md index 91f858751f50..f331c3c10988 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -89,23 +89,27 @@ Proper workflow: ## Changelog -- **Update `CHANGELOG.rst` for every change** targeting the source directory. Each extension has its own changelog at `source//docs/CHANGELOG.rst` (e.g. `source/isaaclab/docs/CHANGELOG.rst`, `source/isaaclab_physx/docs/CHANGELOG.rst`). -- **Always create a new version heading.** Never add entries to an existing version — they are released and immutable. Bump the patch version (e.g. `1.5.0` → `1.5.1`) and use today's date. -- **Bump `config/extension.toml` to match.** When creating a new changelog version, update the `version` field in `source//config/extension.toml` to the same version string. -- **Determine which changelog(s) to update** by looking at which `source//` directories your changes touch. A single PR may require entries in multiple changelogs. +- **Do not edit `CHANGELOG.rst` or `config/extension.toml` directly.** Each PR adds a fragment file under `source//changelog.d/`; the changelog and version are compiled by the nightly CI workflow. +- **Add one fragment per touched package.** Pick any short, unique slug for the filename — your branch name (with `/` replaced by `-`) is a good default. The filename suffix declares the bump tier; within a batch the highest tier wins for the package. + + | Filename | Effect | + |---|---| + | `source//changelog.d/.rst` | patch bump | + | `source//changelog.d/.minor.rst` | minor bump | + | `source//changelog.d/.major.rst` | major bump | + | `source//changelog.d/.skip` | no entry, no bump (CI / docs / test-only) | + - Use **past tense** matching the section header: "Added X", "Fixed Y", "Changed Z". - Place entries under the correct category: `Added`, `Changed`, `Deprecated`, `Removed`, or `Fixed`. - Avoid internal implementation details users wouldn't understand. - **For `Deprecated`, `Changed`, and `Removed` entries, include migration guidance.** - Example: "Deprecated `Articulation.A` in favor of `Articulation.B`." +- **Breaking changes** belong in `Changed`, prefixed with `**Breaking:**`. - Use Sphinx cross-reference roles for class/method/module names. ### RST formatting reference ``` -X.Y.Z (YYYY-MM-DD) -~~~~~~~~~~~~~~~~~~ - Added ^^^^^ @@ -119,10 +123,10 @@ Fixed ``` Key formatting rules: -- Version heading: underline with `~` (tildes), must be at least as long as the heading text. -- Category heading: underline with `^` (carets). +- Category heading: underline with `^` (carets), at least as long as the heading text. - Entries: `* ` prefix, continuation lines indented by 2 spaces. -- Blank line between the last entry and the next version heading. + +See `tools/changelog/test/integration/` for worked examples that double as integration-test fixtures. ## Commit and Pull Request Guidelines diff --git a/source/isaaclab/changelog.d/.gitkeep b/source/isaaclab/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_assets/changelog.d/.gitkeep b/source/isaaclab_assets/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_contrib/changelog.d/.gitkeep b/source/isaaclab_contrib/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_experimental/changelog.d/.gitkeep b/source/isaaclab_experimental/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_mimic/changelog.d/.gitkeep b/source/isaaclab_mimic/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_newton/changelog.d/.gitkeep b/source/isaaclab_newton/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_ov/changelog.d/.gitkeep b/source/isaaclab_ov/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_physx/changelog.d/.gitkeep b/source/isaaclab_physx/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_rl/changelog.d/.gitkeep b/source/isaaclab_rl/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_tasks/changelog.d/.gitkeep b/source/isaaclab_tasks/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_teleop/changelog.d/.gitkeep b/source/isaaclab_teleop/changelog.d/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tools/changelog/cli.py b/tools/changelog/cli.py new file mode 100644 index 000000000000..16ad551b4ca3 --- /dev/null +++ b/tools/changelog/cli.py @@ -0,0 +1,1009 @@ +# 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 + +"""Manage changelog fragments — single entry point with two subcommands. + +Each PR drops a fragment under ``source//changelog.d/.rst``. +The slug is any short, unique name — the contributor's branch name (with +``/`` replaced by ``-``) is the recommended default. The file mirrors +the RST that will appear in the changelog — one or more section headings +(``Added``, ``Changed``, ``Deprecated``, ``Removed``, ``Fixed``) each +underlined with ``^``. The **filename suffix** declares the bump tier: + +- ``.rst`` — patch bump. +- ``.minor.rst`` — minor bump. +- ``.major.rst`` — major bump. +- ``.skip`` — no entry, no bump. + +When a batch compiles together, the highest declared bump wins for the +package (one ``.major.rst`` anywhere → major). + +Subcommands: + + check PR gate. Verifies every modified package has a valid fragment. + compile Roll accumulated fragments into ``CHANGELOG.rst`` and bump + ``extension.toml``. Run by the nightly workflow + (``.github/workflows/nightly-changelog.yml``) on a cron and + by maintainers manually when cutting a release. + +Usage: + + # ── check ───────────────────────────────────────────────────── + # CI invocation on every pull_request: + cli.py check + + # ── compile ─────────────────────────────────────────────────── + # Normal release-time invocation — bump every managed package + # from accumulated fragments, write entries, delete fragments: + cli.py compile --all + + # Preview only (no writes, no deletes): + cli.py compile --all --dry-run + + # Pin one package to a specific version (single-package only — + # each managed package has its own version trajectory): + cli.py compile --package isaaclab --version 4.7.0 + + # Preview against a worked example without touching real packages: + cli.py compile --package isaaclab --dry-run \\ + --fragments-dir tools/changelog/test/integration/02_minor_bump/fragments + +For big version jumps (e.g. ``2.1`` → ``4.7``) edit +``source//config/extension.toml`` and prepend a manual entry to +``source//docs/CHANGELOG.rst``. The compiler is for fragment-driven +incremental bumps, not for jumps. +""" + +from __future__ import annotations + +import argparse +import re +import subprocess +import sys +from dataclasses import dataclass, field +from datetime import date +from functools import cached_property +from pathlib import Path +from typing import ClassVar + +# Walk three levels up: tools/changelog/cli.py -> tools/changelog/ -> tools/ -> repo root. +REPO_ROOT = Path(__file__).parent.parent.parent +PACKAGES_ROOT = REPO_ROOT / "source" + +# Recognised fragment filename patterns. ```` is any short identifier +# the contributor chose — typically their branch name with ``/`` replaced by +# ``-``. The slug must not contain ``.`` (reserved for the tier suffix) or +# ``/`` (path separator), but otherwise mirrors what git allows in a ref name. +# These regexes live at module level because Fragment, FragmentBatch, and +# PRDiff all match against them — they are the wire-format contract between +# contributors and the gate. +FRAGMENT_RE = re.compile(r"^(?P[^./][^./]*)(?:\.(?Pminor|major))?\.rst$") +SKIP_RE = re.compile(r"^(?P[^./][^./]*)\.skip$") + + +def _display_path(p: Path) -> str: + """Pretty-print a Path. Strips ``REPO_ROOT`` if ``p`` is inside the repo, + falls back to the absolute path otherwise (``--fragments-dir`` may + legitimately point at an external directory like ``/tmp/...``). + + Lives at module level because both :class:`Package` (writing on-disk + paths) and :class:`FragmentBatch` (warning about external fragment + paths) use it. + """ + try: + return str(p.relative_to(REPO_ROOT)) + except ValueError: + return str(p) + + +# --------------------------------------------------------------------------- +# Domain objects +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class Version: + """A semver-style version string ``X.Y.Z`` (optionally suffixed with ``.devN``). + + Models a version as a value object: immutable, comparable by its text, + knows how to produce a bumped successor. PEP 440 ``.devN`` suffixes + are tolerated on the way *in* (stripped before bumping) but never + written back out — :meth:`bumped` always returns a clean ``X.Y.Z``. + + Construction validates the format up front so that an invalid + ``--version`` flag from the CLI fails fast instead of silently writing + a malformed entry to ``CHANGELOG.rst``. + """ + + # ``X.Y.Z`` with an optional PEP 440 ``.devN`` suffix. The suffix is + # tolerated on the way *in* (e.g. when reading a stale dev version out + # of an existing ``extension.toml``) but :meth:`bumped` always strips + # it before producing a successor. + _SEMVER_RE: ClassVar[re.Pattern[str]] = re.compile(r"^\d+\.\d+\.\d+(\.dev\d+)?$") + + text: str + + def __post_init__(self) -> None: + if not self._SEMVER_RE.match(self.text): + raise ValueError(f"Invalid version {self.text!r}; expected X.Y.Z (optionally suffixed with .devN)") + + def bumped(self, tier: str) -> Version: + """Return a new Version one tier ahead of this one. + + ``tier`` is ``'major'``, ``'minor'``, or ``'patch'``. Major zeros + the minor and patch components; minor zeros patch. Any ``.devN`` + suffix on the current version is stripped before bumping. + """ + # __post_init__ guarantees the format, so this split is safe. + parts = self.text.split(".dev")[0].split(".") + if tier == "major": + return Version(f"{int(parts[0]) + 1}.0.0") + if tier == "minor": + return Version(f"{parts[0]}.{int(parts[1]) + 1}.0") + return Version(f"{parts[0]}.{parts[1]}.{int(parts[2]) + 1}") + + def __str__(self) -> str: + return self.text + + +@dataclass(frozen=True) +class Fragment: + """One fragment file in a package's ``changelog.d/`` (or an examples dir). + + A :class:`Fragment` instance is just a path plus methods that interpret + it as a changelog fragment. ``.gitkeep`` and ``*.skip`` files should + not be wrapped — only files matching :data:`FRAGMENT_RE`. + """ + + path: Path + + @property + def name(self) -> str: + return self.path.name + + @cached_property + def _match(self) -> re.Match[str] | None: + return FRAGMENT_RE.match(self.name) + + @property + def is_valid_filename(self) -> bool: + return self._match is not None + + @property + def bump(self) -> str: + """Bump tier declared by the filename suffix (defaults to ``'patch'``).""" + if self._match and self._match.group("bump"): + return self._match.group("bump") + return "patch" + + def parse(self) -> dict[str, list[str]]: + """Return ``{section: [lines]}`` from this fragment's content. + + Lines are kept as-is (including trailing newlines) so the compiled + output is byte-for-byte identical to what the contributor wrote. A + section heading is a non-empty line followed by ``^`` underline of + equal-or-greater length. + """ + text = self.path.read_text(encoding="utf-8") + lines = text.splitlines(keepends=True) + sections: dict[str, list[str]] = {} + current: str | None = None + buf: list[str] = [] + + i = 0 + while i < len(lines): + raw = lines[i] + stripped = raw.rstrip("\n") + if ( + i + 1 < len(lines) + and stripped + and re.fullmatch(r"\^+", lines[i + 1].rstrip("\n")) + and len(lines[i + 1].rstrip("\n")) >= len(stripped) + ): + if current is not None: + sections[current] = self._strip_trailing_blank(buf) + current = stripped + buf = [] + i += 2 # skip heading + underline + if i < len(lines) and not lines[i].strip(): + i += 1 + continue + if current is not None: + buf.append(raw) + i += 1 + + if current is not None: + sections[current] = self._strip_trailing_blank(buf) + + return sections + + @staticmethod + def _strip_trailing_blank(lines: list[str]) -> list[str]: + """Drop trailing blank lines from a section's raw line buffer.""" + while lines and not lines[-1].strip(): + lines.pop() + return lines + + @staticmethod + def parse_slug(filename: str) -> str | None: + """Return the slug declared by a fragment / skip filename, or ``None``. + + Used by :class:`PRDiff` to detect collisions between an added + fragment's slug and an existing fragment in the same directory, + without needing to materialise a :class:`Fragment` (the diff entry + may not exist on disk yet during a gate run). + """ + m = FRAGMENT_RE.match(filename) or SKIP_RE.match(filename) + return m.group("slug") if m else None + + def merge_time(self) -> int: + """Unix timestamp of the merge commit that introduced this fragment. + + Uses ``git log --diff-filter=A --first-parent`` to follow develop's + first-parent history, so the timestamp reflects when the PR's merge + commit landed (not the feature-branch commit that originally added + the file). Falls back to the file's most recent commit time when + not yet in first-parent history (e.g. local dry-runs on a feature + branch), and ultimately to ``0`` if git is unavailable. + """ + for cmd in ( + ["git", "log", "--diff-filter=A", "--first-parent", "-1", "--format=%ct", "--", str(self.path)], + ["git", "log", "-1", "--format=%ct", "--", str(self.path)], + ): + try: + result = subprocess.run(cmd, capture_output=True, text=True, check=True, cwd=REPO_ROOT) + ts = result.stdout.strip() + if ts: + return int(ts) + except (subprocess.CalledProcessError, ValueError): + continue + return 0 + + def validate(self) -> str | None: + """Return a human-readable error string if malformed, else ``None``. + + Filename rules: must match :data:`FRAGMENT_RE` (``.gitkeep`` and + ``*.skip`` files are filtered out at :meth:`FragmentBatch.from_dir` + level and never reach this method). Content rules (for ``*.rst`` + fragments only): non-empty file with at least one valid section + heading and at least one bullet point. + """ + if not self.is_valid_filename: + return ( + "invalid filename — must be .rst, .minor.rst, " + ".major.rst, or .skip (slug = your branch name " + "with `/` replaced by `-`, no dots)" + ) + if not self.path.exists(): + # Deleted fragments don't need validating (consumed by a previous compile). + return None + text = self.path.read_text(encoding="utf-8") + if not text.strip(): + return "fragment is empty" + sections = self.parse() + if not sections: + return ( + "no recognised section headings (expected one or more of " + "Added / Changed / Deprecated / Removed / Fixed, each underlined " + "with carets ``^`` of equal-or-greater length)" + ) + # Every declared section must carry at least one bullet — otherwise + # the compiled output emits a heading with no body, which is both + # ugly and almost certainly a contributor authoring mistake (typed + # the heading, forgot the bullet). + empty = [s for s, lines in sections.items() if not any(line.lstrip().startswith("*") for line in lines)] + if empty: + return ( + f"section(s) {', '.join(repr(s) for s in empty)} have no bullet entries — " + "use ``* `` to start each entry, or remove the heading" + ) + return None + + +@dataclass(frozen=True) +class FragmentBatch: + """A collection of fragments collected from a directory. + + ``valid`` are :class:`Fragment` instances sorted by merge time + (oldest first). ``invalid`` are paths that don't match any recognised + filename pattern — surfaced so the caller can warn or fail. ``.skip`` + and ``.gitkeep`` files are tolerated but excluded from both lists. + + Holds the pure-data class methods that turn a batch (or a synthetic + list of bumps / sections) into a compiled changelog entry. The + instance methods (:meth:`aggregate_bump`, :meth:`merged_sections`, + :meth:`compile_to_entry`) read the batch's own state; the + underscore-prefixed static methods (:meth:`_aggregate`, etc.) are + the underlying pure transformations and are used directly by tests + that exercise edge cases without a real fragments directory. + """ + + # Canonical ordering of section headings in compiled output. Anything + # not listed here keeps insertion order *after* these. + _SECTION_ORDER: ClassVar[list[str]] = ["Added", "Changed", "Deprecated", "Removed", "Fixed"] + + # Strict ordering of bump tiers (``major`` strictly outranks ``minor`` + # outranks ``patch``). Unrecognised tiers sort below ``patch``. + _BUMP_RANK: ClassVar[dict[str, int]] = {"patch": 0, "minor": 1, "major": 2} + + valid: list[Fragment] + invalid: list[Path] + skip_paths: list[Path] = field(default_factory=list) + + # ---- Construction -------------------------------------------------- + + @classmethod + def from_dir(cls, fragment_dir: Path) -> FragmentBatch: + if not fragment_dir.is_dir(): + return cls([], []) + valid: list[Fragment] = [] + invalid: list[Path] = [] + skips: list[Path] = [] + for p in fragment_dir.iterdir(): + if p.is_dir() or p.name == ".gitkeep": + continue + if SKIP_RE.match(p.name): + skips.append(p) + continue + f = Fragment(p) + if f.is_valid_filename: + valid.append(f) + else: + invalid.append(p) + # Sort by merge time, breaking ties on filename so the compiled output + # is deterministic when fragments share a merge commit (or when none + # are in git history yet — e.g. a local dry-run against test fixtures). + valid.sort(key=lambda f: (f.merge_time(), f.name)) + return cls(valid, invalid, skips) + + # ---- Queries against this batch's state --------------------------- + + def aggregate_bump(self) -> str: + """Highest bump tier declared by fragments that parsed to content. + + Empty fragments (which the compiler warns about and skips) are + excluded so they don't influence the version. Defaults to + ``patch`` if nothing parsed. + """ + return self._aggregate([f.bump for f, _ in self.parsed()]) + + def parsed(self) -> list[tuple[Fragment, dict[str, list[str]]]]: + """Return ``(fragment, sections)`` pairs, dropping fragments that parse empty.""" + return [(f, s) for f, s in ((f, f.parse()) for f in self.valid) if s] + + 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 compile_to_entry( + self, + current_version: Version, + *, + explicit_version: Version | None = None, + ) -> tuple[Version, str, str]: + """Return ``(new_version, bump_label, entry_text)`` for this batch. + + ``new_version`` is either ``explicit_version`` verbatim or the + result of bumping ``current_version`` by the aggregated tier. + ``bump_label`` is a human-readable suffix like ``" (bump: minor)"`` + for log lines (empty when ``explicit_version`` is used). + ``entry_text`` is the rendered RST block ready to prepend to a + ``CHANGELOG.rst``. Pure computation — no I/O. + """ + if explicit_version is not None: + new_version = explicit_version + bump_label = "" + else: + chosen_bump = self.aggregate_bump() + new_version = current_version.bumped(chosen_bump) + bump_label = f" (bump: {chosen_bump})" + entry = self._format_entry(new_version.text, self.merged_sections()) + return new_version, bump_label, entry + + # ---- Cleanup ------------------------------------------------------- + + def delete_all(self) -> tuple[int, int]: + """Delete every consumed fragment + skip file. Returns ``(n_frag, n_skip)``.""" + n_frag = self.delete_valid() + n_skip = self.delete_skips() + return n_frag, n_skip + + def delete_valid(self) -> int: + for f in self.valid: + f.path.unlink() + return len(self.valid) + + def delete_skips(self) -> int: + for p in self.skip_paths: + p.unlink() + return len(self.skip_paths) + + # ---- Pure transformations (the data class methods) ---------------- + # Static so callers and tests can exercise them with synthetic + # primitives — no FragmentBatch instance needed when the question + # is "given these tiers, which wins?" or "how do these dicts merge?" + + @classmethod + def _aggregate(cls, bumps: list[str]) -> str: + """Highest-ranked bump from ``bumps`` (``major > minor > patch``). + + An empty list defaults to ``'patch'``. + """ + if not bumps: + return "patch" + return max(bumps, key=lambda b: cls._BUMP_RANK.get(b, -1)) + + @staticmethod + def _merge_sections(fragments: list[dict[str, list[str]]]) -> dict[str, list[str]]: + """Merge multiple parsed fragments into a single section map. + + Bullets from different fragments that share a section heading are + concatenated directly (no blank line between them) to match the + dominant style in IsaacLab's existing ``CHANGELOG.rst`` files. + """ + merged: dict[str, list[str]] = {} + for frag in fragments: + for section, lines in frag.items(): + if section not in merged: + merged[section] = list(lines) + else: + merged[section].extend(lines) + return merged + + @classmethod + def _format_entry(cls, version: str, sections: dict[str, list[str]]) -> str: + """Return a complete RST version entry, ready to prepend to ``CHANGELOG.rst``. + + Sections appear in :attr:`_SECTION_ORDER` (Added, Changed, + Deprecated, Removed, Fixed). Anything else keeps insertion order + *after* the canonical ones. + """ + today = date.today().strftime("%Y-%m-%d") + heading = f"{version} ({today})" + out = [heading, "~" * len(heading), ""] + + ordered = [s for s in cls._SECTION_ORDER if s in sections] + extras = [s for s in sections if s not in cls._SECTION_ORDER] + + for section in ordered + extras: + out.append(section) + out.append("^" * len(section)) + out.append("") + for line in sections[section]: + out.append(line.rstrip("\n")) + out.append("") + + return "\n".join(out) + "\n" + + +@dataclass(frozen=True) +class Package: + """A source// directory the changelog tool can manage. + + A package is "managed" if it has both a ``config/extension.toml`` (the + version file the compiler bumps) and a ``docs/CHANGELOG.rst`` (the + file the compiler updates). :meth:`discover` returns only managed + packages; instances created directly may not be managed (use + :attr:`is_managed`). + """ + + root: Path + + @property + def name(self) -> str: + return self.root.name + + @property + def changelog_path(self) -> Path: + return self.root / "docs" / "CHANGELOG.rst" + + @property + def toml_path(self) -> Path: + return self.root / "config" / "extension.toml" + + @property + def default_fragment_dir(self) -> Path: + return self.root / "changelog.d" + + @property + def is_managed(self) -> bool: + return self.toml_path.is_file() and self.changelog_path.is_file() + + def current_version(self) -> Version: + for line in self.toml_path.read_text(encoding="utf-8").splitlines(): + m = re.match(r'^version\s*=\s*"([^"]+)"', line) + if m: + return Version(m.group(1)) + raise ValueError(f"No version field found in {self.toml_path}") + + def write_changelog_entry(self, entry: str, *, dry_run: bool) -> None: + text = self.changelog_path.read_text(encoding="utf-8") + m = re.search(r"^Changelog\n-+\s*\n\s*\n", text, re.MULTILINE) + if not m: + raise ValueError(f"Could not locate changelog header in {self.changelog_path}") + updated = text[: m.end()] + entry + "\n" + text[m.end() :] + if dry_run: + print(f"\n{'=' * 60}") + print(f"DRY RUN — would write to {_display_path(self.changelog_path)}") + print(f"{'=' * 60}") + print(entry) + else: + self.changelog_path.write_text(updated, encoding="utf-8") + + def write_version(self, new_version: Version, *, dry_run: bool) -> None: + text = self.toml_path.read_text(encoding="utf-8") + updated = re.sub(r'^version\s*=\s*"[^"]+"', f'version = "{new_version}"', text, flags=re.MULTILINE) + if dry_run: + print(f'DRY RUN — would set version = "{new_version}" in {_display_path(self.toml_path)}') + else: + self.toml_path.write_text(updated, encoding="utf-8") + + @classmethod + def from_name(cls, name: str, packages_root: Path = PACKAGES_ROOT) -> Package: + return cls(packages_root / name) + + @classmethod + def discover(cls, packages_root: Path = PACKAGES_ROOT) -> list[Package]: + """Return all managed packages under ``packages_root``, sorted by name.""" + if not packages_root.is_dir(): + return [] + return sorted( + (cls(child) for child in packages_root.iterdir() if child.is_dir() and cls(child).is_managed), + key=lambda p: p.name, + ) + + def compile( + self, + *, + fragments_dir: Path | None = None, + explicit_version: Version | None = None, + dry_run: bool = False, + ) -> bool: + """Compile fragments for this package. Returns True if any were compiled. + + There are exactly two modes: ``dry_run=True`` previews and writes + nothing; ``dry_run=False`` writes the new entry, bumps the version, + **and** deletes the consumed fragments. There is deliberately no + third "write but keep fragments" mode — leaving fragments in place + after a real compile is a footgun (the next compile would re-emit + them as a duplicate version block). + + Args: + fragments_dir: Read fragments from here instead of + :attr:`default_fragment_dir`. Useful for previewing against + example fixtures. + explicit_version: Pin the new version to this string (skips the + per-fragment bump inference). + dry_run: Preview only — no files are written or deleted. + """ + batch = FragmentBatch.from_dir(self._resolve_fragments_dir(fragments_dir)) + + for p in batch.invalid: + print( + f" WARNING: {_display_path(p)} does not match any recognised fragment " + "pattern (.rst, .minor.rst, .major.rst, .skip) — skipping.", + file=sys.stderr, + ) + + if not batch.valid: + if batch.skip_paths: + n = len(batch.skip_paths) + if dry_run: + print(f" {self.name}: would clean {n} stale skip file(s).") + else: + batch.delete_skips() + print(f" {self.name}: cleaned {n} stale skip file(s).") + else: + print(f" {self.name}: no fragments, skipping.") + return False + + # Apply the same content-validation rules the PR gate uses, so a + # malformed fragment that somehow reached this package (e.g. a + # stale fragment that predates a content-rule tightening, or a + # locally-edited file) doesn't silently produce a half-empty + # version block. Runs every fragment that survived filename + # validation in ``from_dir``. + validation_errors = [(f, err) for f in batch.valid if (err := f.validate()) is not None] + if validation_errors: + for f, err in validation_errors: + print(f" ERROR: {_display_path(f.path)}: {err}", file=sys.stderr) + raise ValueError( + f"{self.name}: {len(validation_errors)} fragment(s) failed content validation; " + "fix or remove them before compiling." + ) + + parsed_pairs = batch.parsed() + if not parsed_pairs: + print(f" {self.name}: all fragments empty after parsing, skipping.") + return False + + new_version, bump_label, entry = batch.compile_to_entry( + self.current_version(), explicit_version=explicit_version + ) + print(f" {self.name}: {len(parsed_pairs)} fragment(s) → version {new_version}{bump_label}") + + if not self.changelog_path.exists(): + # Should never happen with managed packages discovered via + # ``Package.discover()`` — defensive check for callers that + # construct a ``Package`` directly with an unmanaged root. + raise ValueError( + f"{_display_path(self.changelog_path)} does not exist; " + f"package {self.name!r} is not managed (missing CHANGELOG.rst)." + ) + self.write_changelog_entry(entry, dry_run=dry_run) + self.write_version(new_version, dry_run=dry_run) + + if not dry_run: + n_frag, n_skip = batch.delete_all() + msg = f" {self.name}: deleted {n_frag} fragment(s)" + if n_skip: + msg += f" and {n_skip} skip file(s)" + print(msg + ".") + + return True + + def _resolve_fragments_dir(self, override: Path | None) -> Path: + """Pick the directory ``compile`` should read fragments from. + + ``None`` means "use this package's own ``changelog.d/``"; an + absolute path is used as-is; a relative path is resolved against + ``REPO_ROOT`` so callers can pass things like + ``tools/changelog/test/integration/01_patch_bump/fragments`` without + worrying about the cwd. + """ + if override is None: + return self.default_fragment_dir + return override if override.is_absolute() else (REPO_ROOT / override).resolve() + + +@dataclass(frozen=True) +class PRDiff: + """A snapshot of "what this PR changed against its base branch." + + Wraps two views from the same git diff: ``changed`` (any file modified + or added) and ``added`` (the strict subset that's new on this branch). + Tests construct ``PRDiff`` directly with synthetic sets; + :meth:`from_git` runs the real ``git diff`` for production use. + """ + + changed: set[str] + added: set[str] + + @classmethod + def from_git(cls, base_ref: str) -> PRDiff: + """Run ``git diff`` against ``origin/...HEAD`` to populate the diff.""" + + def _diff(extra_args: list[str]) -> set[str]: + result = subprocess.run( + ["git", "diff", "--name-only", *extra_args, f"origin/{base_ref}...HEAD"], + capture_output=True, + text=True, + check=True, + cwd=REPO_ROOT, + ) + return {f for f in result.stdout.splitlines() if f} + + return cls(changed=_diff([]), added=_diff(["--diff-filter=A"])) + + def evaluate( + self, + packages: list[Package], + ) -> tuple[list[str], list[tuple[str, str]]]: + """Apply the PR-gate rules and return ``(missing_packages, invalid_fragments)``. + + Rules: + + 1. **Immutability** — every fragment file in the diff must be in + ``added`` (added on this branch). Modifying or renaming an existing + fragment is rejected with a hint to add a new one instead. + + 2. **Content validity** — every added ``*.rst`` fragment must parse + (recognised section headings + at least one bullet). ``.skip`` and + ``.gitkeep`` are exempt. + + 3. **Slug uniqueness** — within a package's ``changelog.d/``, no two + fragments may share the same slug. If an added fragment's slug + collides with an existing or co-added fragment, fail with a hint + to rename (e.g. append ``-2``). + + 4. **Required fragment per touched package** — for each managed + package the PR touches in ``source/`` (outside ``changelog.d/``), + the PR must *add* at least one valid fragment to that package's + ``changelog.d/``. Chained PRs (parent PR's fragment shows up in + the child's diff) naturally satisfy this — slug uniqueness is + the only constraint that matters. + """ + missing: list[str] = [] + invalid_fragments: list[tuple[str, str]] = [] + + for pkg in packages: + pkg_prefix = f"source/{pkg.name}/" + changelog_dir = f"source/{pkg.name}/changelog.d/" + + source_changed = [f for f in self.changed if f.startswith(pkg_prefix) and not f.startswith(changelog_dir)] + fragment_changes = [f for f in self.changed if f.startswith(changelog_dir)] + + # Map *pre-existing* fragments in the package's changelog.d/ by slug, + # for the uniqueness check below. The CI checkout contains both + # base-branch fragments and the PR's additions side by side, so we + # must explicitly exclude added files — otherwise an added file can + # overwrite the entry for a colliding pre-existing fragment with + # the same slug, hiding the very collision we're trying to detect. + # Skip ``.gitkeep`` and unrecognised filenames — they can't collide. + added_basenames = {Path(f).name for f in self.added if f.startswith(changelog_dir)} + existing_slugs: dict[str, str] = {} + existing_dir = pkg.default_fragment_dir + if existing_dir.is_dir(): + for p in existing_dir.iterdir(): + if p.is_dir() or p.name == ".gitkeep" or p.name in added_basenames: + continue + slug = Fragment.parse_slug(p.name) + if slug is not None: + existing_slugs[slug] = p.name + + added_slugs: dict[str, str] = {} + for f in fragment_changes: + path = Path(f) + if path.name == ".gitkeep": + continue + + # Rule 1: immutability — modifying an existing fragment is forbidden. + if f not in self.added: + invalid_fragments.append( + ( + f, + "fragments are immutable — add a new fragment with a different slug " + "instead of editing an existing one", + ) + ) + continue + + # Rule 2: content validity (only for *.rst, not *.skip). + if not SKIP_RE.match(path.name): + err = Fragment(REPO_ROOT / f).validate() + if err: + invalid_fragments.append((f, err)) + continue + + # Rule 3: slug uniqueness within the package's changelog.d/. + slug = Fragment.parse_slug(path.name) + if slug is None: + # Filename validation already flagged this above for *.rst, + # but a malformed *.skip would slip through. Surface it. + invalid_fragments.append( + (f, "invalid filename — must be .rst, .minor.rst, .major.rst, or .skip") + ) + continue + if slug in existing_slugs and existing_slugs[slug] != path.name: + invalid_fragments.append( + ( + f, + f"slug {slug!r} collides with existing fragment " + f"{existing_slugs[slug]!r} — rename to {slug}-2 (or any unused slug)", + ) + ) + continue + if slug in added_slugs and added_slugs[slug] != path.name: + invalid_fragments.append( + ( + f, + f"slug {slug!r} collides with another added fragment " + f"{added_slugs[slug]!r} — rename one to {slug}-2 (or any unused slug)", + ) + ) + continue + added_slugs[slug] = path.name + + if not source_changed: + continue + + # Rule 4: this PR must add at least one valid fragment for the package. + owned = [ + f + for f in fragment_changes + if f in self.added and (FRAGMENT_RE.match(Path(f).name) or SKIP_RE.match(Path(f).name)) + ] + if not owned: + missing.append(pkg.name) + + return missing, invalid_fragments + + +# --------------------------------------------------------------------------- +# Subcommand handlers +# --------------------------------------------------------------------------- + + +def cmd_compile(args: argparse.Namespace, parser: argparse.ArgumentParser) -> int: + if args.fragments_dir is not None and args.all: + parser.error("--fragments-dir requires --package (it cannot apply to all packages at once)") + if args.version is not None and args.all: + parser.error( + "--version requires --package (each managed package has its own version trajectory; " + "pin one with --package )" + ) + # Validate ``--version`` shape up front so a typo like ``--version 4.7`` + # fails at argument parsing instead of silently writing ``4.7`` into + # ``CHANGELOG.rst`` and ``extension.toml``. + explicit_version: Version | None = None + if args.version is not None: + try: + explicit_version = Version(args.version) + except ValueError as e: + parser.error(f"--version: {e}") + + if args.package: + pkg = Package.from_name(args.package) + if not pkg.root.is_dir(): + parser.error(f"--package {args.package!r}: directory not found at {pkg.root}") + if not pkg.is_managed: + parser.error( + f"--package {args.package!r} is not managed: missing config/extension.toml or " + f"docs/CHANGELOG.rst at {pkg.root}. Run with --all to see the discovered list." + ) + packages = [pkg] + else: + packages = Package.discover() + + any_compiled = False + for pkg in packages: + try: + compiled = pkg.compile( + fragments_dir=args.fragments_dir, + explicit_version=explicit_version, + dry_run=args.dry_run, + ) + except (FileNotFoundError, ValueError) as e: + print(f" ERROR: {e}", file=sys.stderr) + return 1 + any_compiled = any_compiled or compiled + + if not any_compiled: + print("No fragments found in any package.") + return 0 + + +def cmd_check(args: argparse.Namespace, _parser: argparse.ArgumentParser) -> int: + try: + diff = PRDiff.from_git(args.base_ref) + except subprocess.CalledProcessError as e: + print(f"ERROR: git diff failed: {e.stderr}", file=sys.stderr) + return 1 + + missing, invalid_fragments = diff.evaluate(Package.discover()) + + if invalid_fragments: + print("::error::Invalid changelog fragment(s) in this PR:") + for path, reason in invalid_fragments: + print(f" • {path}") + print(f" → {reason}") + print() + + if missing: + print("::error::Missing changelog fragments for the following packages:") + for pkg_name in missing: + print(f" • {pkg_name}") + print(f" → add source/{pkg_name}/changelog.d/.rst (patch bump)") + print(f" → or source/{pkg_name}/changelog.d/.minor.rst (minor bump)") + print(f" → or source/{pkg_name}/changelog.d/.major.rst (major bump)") + print(f" → or source/{pkg_name}/changelog.d/.skip (no entry, no bump)") + print() + print("Slug = your branch name with `/` replaced by `-` (or any short, unique name).") + print() + print("Fragment format (source//changelog.d/[.minor|.major].rst):") + print() + print(" Added") + print(" ^^^^^") + print() + print(" * Added :class:`~pkg.Foo` for feature X.") + print() + print(" Fixed") + print(" ^^^^^") + print() + print(" * Fixed edge case in :meth:`~pkg.Foo.bar`.") + print() + print("See AGENTS.md ## Changelog for full guidance.") + + if invalid_fragments or missing: + return 1 + + print("✓ All modified packages have valid changelog fragments.") + return 0 + + +# --------------------------------------------------------------------------- +# CLI entry point +# --------------------------------------------------------------------------- + + +def _build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + # The module docstring carries the full usage walkthrough — surfacing + # it as the parser description means ``cli.py --help`` shows the same + # guidance someone reading the source would see. + description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + sub = parser.add_subparsers(dest="cmd", required=True, metavar="{compile,check}") + + p_compile = sub.add_parser( + "compile", + help="Compile fragments into CHANGELOG.rst (maintainer release-time tool).", + description="Compile accumulated fragments into per-package CHANGELOG.rst entries and bump extension.toml.", + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + p_compile.set_defaults(func=cmd_compile) + + # ── Target: which packages to compile (required, mutually exclusive) ── + target = p_compile.add_argument_group("target", "Which package(s) to compile (required, mutually exclusive)") + target_group = target.add_mutually_exclusive_group(required=True) + target_group.add_argument("--package", metavar="NAME", help="Compile a single package.") + target_group.add_argument("--all", action="store_true", help="Compile all managed packages.") + + # ── Version source: by default inferred from filename suffixes ──────── + version_group = p_compile.add_argument_group( + "version (optional)", + "By default the new version is inferred from the filename suffixes of the consumed fragments.", + ) + version_group.add_argument( + "--version", + metavar="X.Y.Z", + help=( + "Pin the package to an explicit version, skipping the per-fragment bump inference. " + "Requires --package — each managed package has its own version trajectory and " + "applying a single version to all of them would corrupt their independent histories." + ), + ) + + # ── Execution mode: preview vs apply, where to read fragments from ──── + exec_group = p_compile.add_argument_group("execution") + exec_group.add_argument( + "--dry-run", + action="store_true", + help=( + "Preview only — no files are written or deleted. Without this flag, " + "the compile writes the new entry, bumps the version, and deletes " + "the consumed fragments." + ), + ) + exec_group.add_argument( + "--fragments-dir", + type=Path, + default=None, + metavar="PATH", + help=( + "Override the directory to read fragments from " + "(default: source//changelog.d/). " + "Useful for previewing against example fragments without touching real ones. " + "Only valid with --package." + ), + ) + + p_check = sub.add_parser( + "check", + help="Verify each modified package has a valid fragment (PR gate).", + description="Verify each modified package has a valid changelog fragment.", + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + p_check.set_defaults(func=cmd_check) + p_check.add_argument( + "base_ref", + help=( + "Base branch to diff against (e.g. 'main' or 'develop'). " + "The diff is taken against ``origin/...HEAD``." + ), + ) + + return parser + + +def main() -> None: + parser = _build_parser() + args = parser.parse_args() + sys.exit(args.func(args, parser)) + + +if __name__ == "__main__": + main() diff --git a/tools/changelog/pyproject.toml b/tools/changelog/pyproject.toml new file mode 100644 index 000000000000..6033f35f178c --- /dev/null +++ b/tools/changelog/pyproject.toml @@ -0,0 +1,13 @@ +# Scopes pytest to this self-contained tool: the ``[tool.pytest.ini_options]`` +# section makes pytest treat ``tools/changelog/`` as its rootdir, so: +# +# 1. ``pythonpath = ["."]`` adds ``tools/changelog/`` to ``sys.path``, +# making ``import cli`` work from the test files without any shim. +# 2. ``tools/conftest.py`` (a session-takeover hook for the IsaacLab +# source/ test suite) sits *above* rootdir and is therefore not +# loaded — no ``--noconftest`` flag required. +# +# Run with: ``./isaaclab.sh -p -m pytest tools/changelog/`` +[tool.pytest.ini_options] +pythonpath = ["."] +testpaths = ["test"] diff --git a/tools/changelog/test/integration/01_patch_bump/changelog_after.rst b/tools/changelog/test/integration/01_patch_bump/changelog_after.rst new file mode 100644 index 000000000000..199771149d62 --- /dev/null +++ b/tools/changelog/test/integration/01_patch_bump/changelog_after.rst @@ -0,0 +1,25 @@ +Changelog +--------- + +1.2.4 (2026-04-30) +~~~~~~~~~~~~~~~~~~ + +Changed +^^^^^^^ + +* Tightened error message in :class:`~example.Foo` when a required argument is missing. + +Fixed +^^^^^ + +* Fixed missing GPU sync in :func:`~example.refresh_buffers` that occasionally returned stale data. +* Fixed off-by-one in :meth:`~example.Foo.bar` when the input list was empty. + + +1.2.3 (2026-01-15) +~~~~~~~~~~~~~~~~~~ + +Added +^^^^^ + +* Added :class:`~example.OldThing` for an earlier feature. diff --git a/tools/changelog/test/integration/01_patch_bump/changelog_before.rst b/tools/changelog/test/integration/01_patch_bump/changelog_before.rst new file mode 100644 index 000000000000..81dc30f240c3 --- /dev/null +++ b/tools/changelog/test/integration/01_patch_bump/changelog_before.rst @@ -0,0 +1,10 @@ +Changelog +--------- + +1.2.3 (2026-01-15) +~~~~~~~~~~~~~~~~~~ + +Added +^^^^^ + +* Added :class:`~example.OldThing` for an earlier feature. diff --git a/tools/changelog/test/integration/01_patch_bump/fragments/asmith-fix-collision-margin.rst b/tools/changelog/test/integration/01_patch_bump/fragments/asmith-fix-collision-margin.rst new file mode 100644 index 000000000000..2f4c9e39a432 --- /dev/null +++ b/tools/changelog/test/integration/01_patch_bump/fragments/asmith-fix-collision-margin.rst @@ -0,0 +1,9 @@ +Fixed +^^^^^ + +* Fixed missing GPU sync in :func:`~example.refresh_buffers` that occasionally returned stale data. + +Changed +^^^^^^^ + +* Tightened error message in :class:`~example.Foo` when a required argument is missing. diff --git a/tools/changelog/test/integration/01_patch_bump/fragments/jdoe-fix-mass-units.rst b/tools/changelog/test/integration/01_patch_bump/fragments/jdoe-fix-mass-units.rst new file mode 100644 index 000000000000..f3a7a66a215a --- /dev/null +++ b/tools/changelog/test/integration/01_patch_bump/fragments/jdoe-fix-mass-units.rst @@ -0,0 +1,4 @@ +Fixed +^^^^^ + +* Fixed off-by-one in :meth:`~example.Foo.bar` when the input list was empty. diff --git a/tools/changelog/test/integration/02_minor_bump/changelog_after.rst b/tools/changelog/test/integration/02_minor_bump/changelog_after.rst new file mode 100644 index 000000000000..bc5a3fee0e35 --- /dev/null +++ b/tools/changelog/test/integration/02_minor_bump/changelog_after.rst @@ -0,0 +1,30 @@ +Changelog +--------- + +1.3.0 (2026-04-30) +~~~~~~~~~~~~~~~~~~ + +Added +^^^^^ + +* Added :class:`~example.NewSensor` for IMU-based proprioception. +* Added :func:`~example.helper` utility for batched coordinate transforms. + +Changed +^^^^^^^ + +* Documented thread-safety guarantees for :class:`~example.Worker`. + +Fixed +^^^^^ + +* Fixed a NaN propagation in :meth:`~example.Sensor.update`. + + +1.2.3 (2026-01-15) +~~~~~~~~~~~~~~~~~~ + +Added +^^^^^ + +* Added :class:`~example.OldThing` for an earlier feature. diff --git a/tools/changelog/test/integration/02_minor_bump/changelog_before.rst b/tools/changelog/test/integration/02_minor_bump/changelog_before.rst new file mode 100644 index 000000000000..81dc30f240c3 --- /dev/null +++ b/tools/changelog/test/integration/02_minor_bump/changelog_before.rst @@ -0,0 +1,10 @@ +Changelog +--------- + +1.2.3 (2026-01-15) +~~~~~~~~~~~~~~~~~~ + +Added +^^^^^ + +* Added :class:`~example.OldThing` for an earlier feature. diff --git a/tools/changelog/test/integration/02_minor_bump/fragments/asmith-add-multi-asset-spawner.minor.rst b/tools/changelog/test/integration/02_minor_bump/fragments/asmith-add-multi-asset-spawner.minor.rst new file mode 100644 index 000000000000..5f623cf02ad5 --- /dev/null +++ b/tools/changelog/test/integration/02_minor_bump/fragments/asmith-add-multi-asset-spawner.minor.rst @@ -0,0 +1,4 @@ +Added +^^^^^ + +* Added :class:`~example.NewSensor` for IMU-based proprioception. diff --git a/tools/changelog/test/integration/02_minor_bump/fragments/blee-add-camera-output-contract.minor.rst b/tools/changelog/test/integration/02_minor_bump/fragments/blee-add-camera-output-contract.minor.rst new file mode 100644 index 000000000000..a3feda328623 --- /dev/null +++ b/tools/changelog/test/integration/02_minor_bump/fragments/blee-add-camera-output-contract.minor.rst @@ -0,0 +1,9 @@ +Added +^^^^^ + +* Added :func:`~example.helper` utility for batched coordinate transforms. + +Changed +^^^^^^^ + +* Documented thread-safety guarantees for :class:`~example.Worker`. diff --git a/tools/changelog/test/integration/02_minor_bump/fragments/jdoe-fix-rotation-frame.rst b/tools/changelog/test/integration/02_minor_bump/fragments/jdoe-fix-rotation-frame.rst new file mode 100644 index 000000000000..e103861d0d98 --- /dev/null +++ b/tools/changelog/test/integration/02_minor_bump/fragments/jdoe-fix-rotation-frame.rst @@ -0,0 +1,4 @@ +Fixed +^^^^^ + +* Fixed a NaN propagation in :meth:`~example.Sensor.update`. diff --git a/tools/changelog/test/integration/03_major_bump/changelog_after.rst b/tools/changelog/test/integration/03_major_bump/changelog_after.rst new file mode 100644 index 000000000000..cec9e3221263 --- /dev/null +++ b/tools/changelog/test/integration/03_major_bump/changelog_after.rst @@ -0,0 +1,34 @@ +Changelog +--------- + +2.0.0 (2026-04-30) +~~~~~~~~~~~~~~~~~~ + +Added +^^^^^ + +* Added :class:`~example.AnotherSensor` for proximity detection. + +Changed +^^^^^^^ + +* **Breaking:** :meth:`~example.Foo.bar` now returns a tuple ``(value, error)`` instead of raising. + +Removed +^^^^^^^ + +* Removed deprecated module ``example.old_api`` (use :mod:`~example.api` instead). + +Fixed +^^^^^ + +* Fixed a deadlock in :class:`~example.Pool` under high concurrency. + + +1.2.3 (2026-01-15) +~~~~~~~~~~~~~~~~~~ + +Added +^^^^^ + +* Added :class:`~example.OldThing` for an earlier feature. diff --git a/tools/changelog/test/integration/03_major_bump/changelog_before.rst b/tools/changelog/test/integration/03_major_bump/changelog_before.rst new file mode 100644 index 000000000000..81dc30f240c3 --- /dev/null +++ b/tools/changelog/test/integration/03_major_bump/changelog_before.rst @@ -0,0 +1,10 @@ +Changelog +--------- + +1.2.3 (2026-01-15) +~~~~~~~~~~~~~~~~~~ + +Added +^^^^^ + +* Added :class:`~example.OldThing` for an earlier feature. diff --git a/tools/changelog/test/integration/03_major_bump/fragments/asmith-add-warp-contact-stream.minor.rst b/tools/changelog/test/integration/03_major_bump/fragments/asmith-add-warp-contact-stream.minor.rst new file mode 100644 index 000000000000..864d48ce0cef --- /dev/null +++ b/tools/changelog/test/integration/03_major_bump/fragments/asmith-add-warp-contact-stream.minor.rst @@ -0,0 +1,4 @@ +Added +^^^^^ + +* Added :class:`~example.AnotherSensor` for proximity detection. diff --git a/tools/changelog/test/integration/03_major_bump/fragments/blee-rename-articulation-api.major.rst b/tools/changelog/test/integration/03_major_bump/fragments/blee-rename-articulation-api.major.rst new file mode 100644 index 000000000000..d392bcd339c2 --- /dev/null +++ b/tools/changelog/test/integration/03_major_bump/fragments/blee-rename-articulation-api.major.rst @@ -0,0 +1,9 @@ +Removed +^^^^^^^ + +* Removed deprecated module ``example.old_api`` (use :mod:`~example.api` instead). + +Changed +^^^^^^^ + +* **Breaking:** :meth:`~example.Foo.bar` now returns a tuple ``(value, error)`` instead of raising. diff --git a/tools/changelog/test/integration/03_major_bump/fragments/jdoe-fix-articulation-state.rst b/tools/changelog/test/integration/03_major_bump/fragments/jdoe-fix-articulation-state.rst new file mode 100644 index 000000000000..b184ab45c6e2 --- /dev/null +++ b/tools/changelog/test/integration/03_major_bump/fragments/jdoe-fix-articulation-state.rst @@ -0,0 +1,4 @@ +Fixed +^^^^^ + +* Fixed a deadlock in :class:`~example.Pool` under high concurrency. diff --git a/tools/changelog/test/integration/README.md b/tools/changelog/test/integration/README.md new file mode 100644 index 000000000000..1f55d0438705 --- /dev/null +++ b/tools/changelog/test/integration/README.md @@ -0,0 +1,35 @@ +# Changelog integration fixtures + +End-to-end test fixtures for `tools/changelog/cli.py compile`. Each +subdirectory holds a worked example: input fragments, the starting +`CHANGELOG.rst`, and the expected compiled output. + +`tools/changelog/test/test_integration.py` runs the compiler +against each one and asserts the output matches `changelog_after.rst`. +The fixtures double as human-readable demos — read alongside the PR +description to see how the system handles patch / minor / major bumps +and cross-fragment section merges. + +## Layout + +| Demo | Fragments | Bump | Resulting version | +|---|---|---|---| +| `01_patch_bump/` | 2 × `.rst` | patch | `1.2.3 → 1.2.4` | +| `02_minor_bump/` | 1 × `.rst` + 2 × `.minor.rst` | minor | `1.2.3 → 1.3.0` | +| `03_major_bump/` | 1 × `.rst` + 1 × `.minor.rst` + 1 × `.major.rst` | major | `1.2.3 → 2.0.0` | + +Each demo includes a `changelog_before.rst` (initial state) and a +`changelog_after.rst` (expected post-compile state). The bump tier is the +**max** of every fragment's filename suffix in the batch. + +## Run the compiler against a demo + +```bash +./isaaclab.sh -p tools/changelog/cli.py compile --package isaaclab \ + --fragments-dir tools/changelog/test/integration/02_minor_bump/fragments \ + --dry-run +``` + +`--dry-run` prevents the compile from consuming (deleting) the fixture +fragments. The output should match `02_minor_bump/changelog_after.rst` +modulo today's date. diff --git a/tools/changelog/test/invalid_content/3001.rst b/tools/changelog/test/invalid_content/3001.rst new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tools/changelog/test/invalid_content/3002.rst b/tools/changelog/test/invalid_content/3002.rst new file mode 100644 index 000000000000..190621a3aaf3 --- /dev/null +++ b/tools/changelog/test/invalid_content/3002.rst @@ -0,0 +1 @@ +Just a free-form note with no section headings. diff --git a/tools/changelog/test/invalid_content/3003.rst b/tools/changelog/test/invalid_content/3003.rst new file mode 100644 index 000000000000..4470915bae8e --- /dev/null +++ b/tools/changelog/test/invalid_content/3003.rst @@ -0,0 +1,2 @@ +Added +^^^^^ diff --git a/tools/changelog/test/invalid_filenames/1234.notabump.rst b/tools/changelog/test/invalid_filenames/1234.notabump.rst new file mode 100644 index 000000000000..f70a86344dc6 --- /dev/null +++ b/tools/changelog/test/invalid_filenames/1234.notabump.rst @@ -0,0 +1,4 @@ +Added +^^^^^ + +* This file has an unrecognised bump tier and should be rejected. diff --git a/tools/changelog/test/invalid_filenames/multi.dot.slug.rst b/tools/changelog/test/invalid_filenames/multi.dot.slug.rst new file mode 100644 index 000000000000..c9ea00434158 --- /dev/null +++ b/tools/changelog/test/invalid_filenames/multi.dot.slug.rst @@ -0,0 +1,4 @@ +Added +^^^^^ + +* This file's slug contains dots (reserved for the tier suffix) and should be rejected. diff --git a/tools/changelog/test/test_bump_suffix.py b/tools/changelog/test/test_bump_suffix.py new file mode 100644 index 000000000000..1c191c6b33d9 --- /dev/null +++ b/tools/changelog/test/test_bump_suffix.py @@ -0,0 +1,135 @@ +# 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 + +"""Bump-tier inference: filename suffix → bump, and aggregating across a batch. + +These tests use the worked examples under :file:`tools/changelog/examples/` +as fixtures so the same files double as human-readable demos and as +inputs the test suite verifies. +""" + +from __future__ import annotations + +from pathlib import Path + +import cli +import pytest + +EXAMPLES = Path(__file__).parent / "integration" + + +# --------------------------------------------------------------------------- +# Filename → bump tier (one demo per tier, tested separately) +# --------------------------------------------------------------------------- + + +def test_patch_bump_demo_aggregates_to_patch(): + """``examples/01_patch_bump/`` has two ``.rst`` files (no suffix) → patch.""" + batch = cli.FragmentBatch.from_dir(EXAMPLES / "01_patch_bump" / "fragments") + assert batch.invalid == [] + assert {f.name for f in batch.valid} == { + "jdoe-fix-mass-units.rst", + "asmith-fix-collision-margin.rst", + } + assert all(f.bump == "patch" for f in batch.valid) + assert batch.aggregate_bump() == "patch" + + +def test_minor_bump_demo_aggregates_to_minor(): + """``examples/02_minor_bump/`` mixes patch + minor fragments → minor wins.""" + batch = cli.FragmentBatch.from_dir(EXAMPLES / "02_minor_bump" / "fragments") + assert batch.invalid == [] + assert {f.name for f in batch.valid} == { + "jdoe-fix-rotation-frame.rst", + "asmith-add-multi-asset-spawner.minor.rst", + "blee-add-camera-output-contract.minor.rst", + } + bumps = sorted(f.bump for f in batch.valid) + assert bumps == ["minor", "minor", "patch"] + assert batch.aggregate_bump() == "minor" + + +def test_major_bump_demo_aggregates_to_major(): + """``examples/03_major_bump/`` mixes patch + minor + major → major wins.""" + batch = cli.FragmentBatch.from_dir(EXAMPLES / "03_major_bump" / "fragments") + assert batch.invalid == [] + assert {f.name for f in batch.valid} == { + "jdoe-fix-articulation-state.rst", + "asmith-add-warp-contact-stream.minor.rst", + "blee-rename-articulation-api.major.rst", + } + bumps = sorted(f.bump for f in batch.valid) + assert bumps == ["major", "minor", "patch"] + assert batch.aggregate_bump() == "major" + + +# --------------------------------------------------------------------------- +# Pure aggregation logic (no filesystem) +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "bumps,expected", + [ + ([], "patch"), + (["patch"], "patch"), + (["patch", "patch"], "patch"), + (["patch", "minor"], "minor"), + (["minor", "patch", "minor"], "minor"), + (["patch", "minor", "major"], "major"), + (["major", "patch"], "major"), + ], +) +def test_aggregate_bump_logic(bumps, expected): + assert cli.FragmentBatch._aggregate(bumps) == expected + + +# --------------------------------------------------------------------------- +# Filename regex — what the gate and compiler agree to accept +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "name,is_fragment,is_skip", + [ + ("1234.rst", True, False), + ("1234.minor.rst", True, False), + ("1234.major.rst", True, False), + ("1234.skip", False, True), + ("jdoe-fix-bug.rst", True, False), + ("jdoe-add-feature.minor.rst", True, False), + ("jdoe-rename-api.major.rst", True, False), + ("jdoe-ci-only.skip", False, True), + (".gitkeep", False, False), + ("README.md", False, False), + ("1234.patch.rst", False, False), # only minor/major are recognised tiers + ("foo.bar.rst", False, False), # extra dots in slug are reserved for tier suffix + ("1234.minor", False, False), # missing .rst extension + ("1234.rst.bak", False, False), + ], +) +def test_fragment_filename_regexes(name, is_fragment, is_skip): + assert bool(cli.FRAGMENT_RE.match(name)) is is_fragment + assert bool(cli.SKIP_RE.match(name)) is is_skip + + +# --------------------------------------------------------------------------- +# Fragment.parse_slug — derived from filename for collision detection +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "name,expected_slug", + [ + ("1234.rst", "1234"), + ("jdoe-add-feature.minor.rst", "jdoe-add-feature"), + ("blee-rename-api.major.rst", "blee-rename-api"), + ("ci-only.skip", "ci-only"), + ("README.md", None), + (".gitkeep", None), + ], +) +def test_parse_slug_for_filenames(name, expected_slug): + assert cli.Fragment.parse_slug(name) == expected_slug diff --git a/tools/changelog/test/test_format.py b/tools/changelog/test/test_format.py new file mode 100644 index 000000000000..24a249992f7b --- /dev/null +++ b/tools/changelog/test/test_format.py @@ -0,0 +1,94 @@ +# 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 + +"""FragmentBatch._merge_sections + ._format_entry + Version.bumped — the rendering pipeline.""" + +from __future__ import annotations + +import cli +import pytest + +# --------------------------------------------------------------------------- +# merge_fragments — collapses bullets across fragments under the same section +# --------------------------------------------------------------------------- + + +def test_merge_fragments_collapses_same_section_across_fragments(): + f1 = {"Added": ["* a1\n"]} + f2 = {"Added": ["* a2\n"], "Fixed": ["* f1\n"]} + merged = cli.FragmentBatch._merge_sections([f1, f2]) + # Bullets from separate fragments concatenate with no blank line in between + # (matching IsaacLab's repo convention, where successive bullets are run-on). + assert merged["Added"] == ["* a1\n", "* a2\n"] + assert merged["Fixed"] == ["* f1\n"] + + +# --------------------------------------------------------------------------- +# format_entry — section ordering + version heading +# --------------------------------------------------------------------------- + + +def test_format_entry_orders_canonical_sections(): + sections = { + "Fixed": ["* f1\n"], + "Added": ["* a1\n"], + "Removed": ["* r1\n"], + } + out = cli.FragmentBatch._format_entry("1.2.4", sections) + # Canonical order is Added, Changed, Deprecated, Removed, Fixed. + a_pos = out.index("Added") + r_pos = out.index("Removed") + f_pos = out.index("Fixed") + assert a_pos < r_pos < f_pos + + +def test_format_entry_includes_version_heading(): + out = cli.FragmentBatch._format_entry("9.9.9", {"Added": ["* x\n"]}) + assert "9.9.9 (" in out + assert "~~~~~~" in out # tilde underline + + +def test_format_entry_unknown_sections_appear_after_canonical(): + sections = {"Performance": ["* p1\n"], "Added": ["* a1\n"]} + out = cli.FragmentBatch._format_entry("1.0.0", sections) + assert out.index("Added") < out.index("Performance") + + +# --------------------------------------------------------------------------- +# bump_version — semver maths +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "current,part,expected", + [ + ("1.2.3", "patch", "1.2.4"), + ("1.2.3", "minor", "1.3.0"), # minor bump zeros patch + ("1.2.3", "major", "2.0.0"), # major bump zeros minor and patch + ("4.6.21", "patch", "4.6.22"), + ("4.6.21.dev20260301", "patch", "4.6.22"), # dev suffix stripped + ], +) +def test_version_bumped(current, part, expected): + assert cli.Version(current).bumped(part).text == expected + assert str(cli.Version(current).bumped(part)) == expected + + +def test_version_bumped_rejects_non_semver(): + # Construction itself rejects malformed input — fail-fast for bad ``--version``. + with pytest.raises(ValueError): + cli.Version("1.2") + with pytest.raises(ValueError): + cli.Version("not-semver") + with pytest.raises(ValueError): + cli.Version("1.2.3.4.5") + + +def test_version_accepts_dev_suffix(): + """PEP 440 ``.devN`` suffixes are tolerated on construction (they appear in + real ``extension.toml`` files between releases) and stripped on bump.""" + v = cli.Version("4.6.21.dev20260301") + assert v.text == "4.6.21.dev20260301" + assert v.bumped("patch").text == "4.6.22" diff --git a/tools/changelog/test/test_integration.py b/tools/changelog/test/test_integration.py new file mode 100644 index 000000000000..0e48e4a874aa --- /dev/null +++ b/tools/changelog/test/test_integration.py @@ -0,0 +1,69 @@ +# 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 + +"""End-to-end checks: run the compiler against each worked example and verify +the resulting changelog matches the checked-in :file:`changelog_after.rst`. + +This is what makes the examples *living docs* — if anything in the compile +pipeline drifts, an example's ``changelog_after.rst`` stops matching and +the corresponding test fails immediately. +""" + +from __future__ import annotations + +import re +import shutil +from pathlib import Path + +import cli +import pytest + +EXAMPLES = Path(__file__).parent / "integration" + +# Strip the ``(YYYY-MM-DD)`` suffix from version headings so the fixed example +# files don't drift when the compiler stamps today's date. +_DATE_RE = re.compile(r"\(\d{4}-\d{2}-\d{2}\)") + + +def _normalize(text: str) -> str: + return _DATE_RE.sub("(YYYY-MM-DD)", text) + + +@pytest.mark.parametrize( + "demo,expected_version", + [ + ("01_patch_bump", "1.2.4"), + ("02_minor_bump", "1.3.0"), + ("03_major_bump", "2.0.0"), + ], +) +def test_demo_compile_matches_changelog_after(tmp_path, demo, expected_version): + """Stage a fake package whose CHANGELOG.rst matches the demo's ``before``, + run the compiler against the demo's fragments, and verify the file ends + up byte-equal to the demo's ``after`` (modulo today's date).""" + demo_dir = EXAMPLES / demo + + # Build a minimal package layout the compiler will accept. + pkg_root = tmp_path / "demo_pkg" + (pkg_root / "config").mkdir(parents=True) + (pkg_root / "docs").mkdir(parents=True) + (pkg_root / "config" / "extension.toml").write_text('version = "1.2.3"\n', encoding="utf-8") + shutil.copy(demo_dir / "changelog_before.rst", pkg_root / "docs" / "CHANGELOG.rst") + + # Copy fragments into tmp_path so the compile's auto-clean doesn't + # delete the live checked-in examples directory. + fragments_tmp = tmp_path / "fragments" + shutil.copytree(demo_dir / "fragments", fragments_tmp) + + # Run the compiler against the (copied) fragments. + pkg = cli.Package(pkg_root) + pkg.compile(fragments_dir=fragments_tmp) + + actual = (pkg_root / "docs" / "CHANGELOG.rst").read_text(encoding="utf-8") + expected = (demo_dir / "changelog_after.rst").read_text(encoding="utf-8") + assert _normalize(actual) == _normalize(expected) + + # Version should have bumped exactly as the demo name suggests. + assert str(pkg.current_version()) == expected_version diff --git a/tools/changelog/test/test_parse.py b/tools/changelog/test/test_parse.py new file mode 100644 index 000000000000..fe8123c2f14c --- /dev/null +++ b/tools/changelog/test/test_parse.py @@ -0,0 +1,143 @@ +# 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 + +"""Fragment.parse + FragmentBatch.from_dir + Package.discover — directory scanning.""" + +from __future__ import annotations + +from pathlib import Path + +import cli + +FIXTURES = Path(__file__).parent + + +def _write(path: Path, body: str) -> Path: + path.write_text(body, encoding="utf-8") + return path + + +# --------------------------------------------------------------------------- +# parse_fragment — section header detection (pure function) +# --------------------------------------------------------------------------- + + +def test_parse_fragment_single_section(tmp_path): + p = _write(tmp_path / "1.rst", "Added\n^^^^^\n\n* Added :class:`~pkg.Foo`.\n") + sections = cli.Fragment(p).parse() + assert list(sections.keys()) == ["Added"] + assert sections["Added"] == ["* Added :class:`~pkg.Foo`.\n"] + + +def test_parse_fragment_multiple_sections_preserves_dict_order(tmp_path): + p = _write(tmp_path / "1.rst", "Added\n^^^^^\n\n* a1\n\nFixed\n^^^^^\n\n* f1\n* f2\n") + sections = cli.Fragment(p).parse() + assert list(sections.keys()) == ["Added", "Fixed"] + assert sections["Added"] == ["* a1\n"] + assert sections["Fixed"] == ["* f1\n", "* f2\n"] + + +def test_parse_fragment_underline_must_be_at_least_heading_length(tmp_path): + """Heading 'Added' (5 chars) needs >=5 carets; '^^' must not match.""" + p = _write(tmp_path / "1.rst", "Added\n^^\n\n* a1\n") + assert cli.Fragment(p).parse() == {} + + +def test_parse_fragment_empty_file(tmp_path): + p = _write(tmp_path / "1.rst", "") + assert cli.Fragment(p).parse() == {} + + +def test_parse_fragment_no_section_headings(tmp_path): + p = _write(tmp_path / "1.rst", "Just a free-form note with no headings.\n") + assert cli.Fragment(p).parse() == {} + + +# --------------------------------------------------------------------------- +# Fragment.parse — same logic, exposed as a method on the wrapper +# --------------------------------------------------------------------------- + + +# --------------------------------------------------------------------------- +# FragmentBatch.from_dir — separates valid filenames from the rest +# --------------------------------------------------------------------------- + + +def test_fragment_batch_flags_invalid_filenames_from_fixture(): + """Files with dotted slugs or unknown bump tiers go in ``invalid``.""" + batch = cli.FragmentBatch.from_dir(FIXTURES / "invalid_filenames") + assert batch.valid == [] + assert {p.name for p in batch.invalid} == {"multi.dot.slug.rst", "1234.notabump.rst"} + + +def test_fragment_batch_missing_directory(tmp_path): + """A non-existent directory is treated as empty, not an error.""" + batch = cli.FragmentBatch.from_dir(tmp_path / "does-not-exist") + assert batch.valid == [] + assert batch.invalid == [] + assert batch.skip_paths == [] + + +def test_fragment_batch_collects_skip_files_separately(tmp_path): + """``.skip`` files are tolerated — exposed via ``skip_paths``, not ``valid``.""" + (tmp_path / "1234.skip").write_text("", encoding="utf-8") + (tmp_path / "1235.rst").write_text("Added\n^^^^^\n\n* x\n", encoding="utf-8") + batch = cli.FragmentBatch.from_dir(tmp_path) + assert {f.name for f in batch.valid} == {"1235.rst"} + assert {p.name for p in batch.skip_paths} == {"1234.skip"} + + +# --------------------------------------------------------------------------- +# Package.discover — a package is "managed" iff it has both +# config/extension.toml and docs/CHANGELOG.rst +# --------------------------------------------------------------------------- + + +def _make_pkg(root: Path, name: str, *, has_ext: bool = True, has_changelog: bool = True) -> None: + pkg = root / name + if has_ext: + (pkg / "config").mkdir(parents=True, exist_ok=True) + (pkg / "config" / "extension.toml").write_text('version = "0.0.0"\n', encoding="utf-8") + if has_changelog: + (pkg / "docs").mkdir(parents=True, exist_ok=True) + (pkg / "docs" / "CHANGELOG.rst").write_text("Changelog\n---------\n\n", encoding="utf-8") + + +def test_package_discover_includes_complete_packages(tmp_path): + _make_pkg(tmp_path, "complete_a") + _make_pkg(tmp_path, "complete_b") + pkgs = cli.Package.discover(tmp_path) + assert [p.name for p in pkgs] == ["complete_a", "complete_b"] + assert all(p.is_managed for p in pkgs) + + +def test_package_discover_excludes_packages_missing_changelog(tmp_path): + _make_pkg(tmp_path, "complete") + _make_pkg(tmp_path, "no_changelog", has_changelog=False) + assert [p.name for p in cli.Package.discover(tmp_path)] == ["complete"] + + +def test_package_discover_excludes_packages_missing_extension_toml(tmp_path): + _make_pkg(tmp_path, "complete") + _make_pkg(tmp_path, "no_extension", has_ext=False) + assert [p.name for p in cli.Package.discover(tmp_path)] == ["complete"] + + +def test_package_discover_returns_sorted_alphabetically(tmp_path): + _make_pkg(tmp_path, "zebra") + _make_pkg(tmp_path, "alpha") + _make_pkg(tmp_path, "mango") + assert [p.name for p in cli.Package.discover(tmp_path)] == ["alpha", "mango", "zebra"] + + +def test_package_discover_missing_root_returns_empty(tmp_path): + assert cli.Package.discover(tmp_path / "does-not-exist") == [] + + +def test_package_is_managed_property(tmp_path): + _make_pkg(tmp_path, "complete") + _make_pkg(tmp_path, "no_changelog", has_changelog=False) + assert cli.Package(tmp_path / "complete").is_managed is True + assert cli.Package(tmp_path / "no_changelog").is_managed is False diff --git a/tools/changelog/test/test_validate.py b/tools/changelog/test/test_validate.py new file mode 100644 index 000000000000..e02e1b95e990 --- /dev/null +++ b/tools/changelog/test/test_validate.py @@ -0,0 +1,323 @@ +# 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 + +"""Fragment.validate — PR-gate filename + content rules.""" + +from __future__ import annotations + +from pathlib import Path + +import cli +import pytest + +FIXTURES = Path(__file__).parent + + +def _write(path: Path, body: str) -> Path: + path.write_text(body, encoding="utf-8") + return path + + +# --------------------------------------------------------------------------- +# Acceptance — well-formed fragments +# --------------------------------------------------------------------------- + + +def test_validate_accepts_well_formed(tmp_path): + p = _write(tmp_path / "1234.rst", "Added\n^^^^^\n\n* Added X.\n") + assert cli.Fragment(p).validate() is None + + +def test_validate_accepts_minor_suffix(tmp_path): + p = _write(tmp_path / "1234.minor.rst", "Added\n^^^^^\n\n* Added X.\n") + assert cli.Fragment(p).validate() is None + + +def test_validate_accepts_major_suffix(tmp_path): + p = _write(tmp_path / "1234.major.rst", "Removed\n^^^^^^^\n\n* Removed X.\n") + assert cli.Fragment(p).validate() is None + + +# --------------------------------------------------------------------------- +# Rejection — uses checked-in fixtures so the malformed inputs are reviewable +# --------------------------------------------------------------------------- + + +def test_validate_rejects_unknown_filename_from_fixture(): + err = cli.Fragment(FIXTURES / "invalid_filenames" / "multi.dot.slug.rst").validate() + assert err is not None and "invalid filename" in err + + +def test_validate_rejects_unknown_bump_tier_from_fixture(): + err = cli.Fragment(FIXTURES / "invalid_filenames" / "1234.notabump.rst").validate() + assert err is not None and "invalid filename" in err + + +def test_validate_rejects_empty_file_from_fixture(): + err = cli.Fragment(FIXTURES / "invalid_content" / "3001.rst").validate() + assert err is not None and "empty" in err + + +def test_validate_rejects_missing_section_heading_from_fixture(): + err = cli.Fragment(FIXTURES / "invalid_content" / "3002.rst").validate() + assert err is not None and "section" in err.lower() + + +def test_validate_rejects_section_without_bullets_from_fixture(): + err = cli.Fragment(FIXTURES / "invalid_content" / "3003.rst").validate() + assert err is not None and "bullet" in err.lower() + + +# --------------------------------------------------------------------------- +# check_fragments — gate orchestration: immutability, slug uniqueness, and +# the "PR must add at least one fragment per touched package" rule +# --------------------------------------------------------------------------- + + +def _pkg_under(tmp_path: Path, name: str) -> cli.Package: + """Build a managed-looking Package rooted 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_check_fragments_immutability_rejects_modified_fragment(tmp_path): + """Modifying an existing fragment is forbidden — must add a new one instead.""" + pkg = _pkg_under(tmp_path, "isaaclab") + changed = {"source/isaaclab/code.py", "source/isaaclab/changelog.d/jdoe-fix-bug.rst"} + added = {"source/isaaclab/code.py"} # fragment exists already; the PR only modified it + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + assert missing == ["isaaclab"] + invalid_map = dict(invalid) + assert "source/isaaclab/changelog.d/jdoe-fix-bug.rst" in invalid_map + assert "immutable" in invalid_map["source/isaaclab/changelog.d/jdoe-fix-bug.rst"] + + +def test_check_fragments_chain_allows_other_pr_fragment(tmp_path): + """A chained PR (B based on A's branch, A still open) sees A's fragment in + its diff. That should pass — both fragments have distinct slugs and B + contributes its own fragment for the touched package.""" + pkg = _pkg_under(tmp_path, "isaaclab") + (pkg.root / "changelog.d").mkdir() + (pkg.root / "changelog.d" / "alice-feature-a.rst").write_text("Fixed\n^^^^^\n\n* x\n", encoding="utf-8") + (pkg.root / "changelog.d" / "bob-feature-b.rst").write_text("Added\n^^^^^\n\n* y\n", encoding="utf-8") + changed = { + "source/isaaclab/code.py", + "source/isaaclab/changelog.d/alice-feature-a.rst", # parent PR's fragment + "source/isaaclab/changelog.d/bob-feature-b.rst", # this PR's own fragment + } + added = changed + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + assert missing == [] + assert invalid == [] + + +def test_check_fragments_slug_collision_with_existing(tmp_path): + """Adding a fragment whose slug collides with one already in changelog.d/ fails.""" + pkg = _pkg_under(tmp_path, "isaaclab") + (pkg.root / "changelog.d").mkdir() + # Pre-existing fragment on develop with the same slug as the one this PR adds. + (pkg.root / "changelog.d" / "jdoe-fix-bug.rst").write_text("Fixed\n^^^^^\n\n* x\n", encoding="utf-8") + # PR adds a fresh fragment whose slug collides — different tier, same slug. + (pkg.root / "changelog.d" / "jdoe-fix-bug.minor.rst").write_text("Added\n^^^^^\n\n* y\n", encoding="utf-8") + changed = {"source/isaaclab/code.py", "source/isaaclab/changelog.d/jdoe-fix-bug.minor.rst"} + added = changed + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + invalid_map = dict(invalid) + assert "source/isaaclab/changelog.d/jdoe-fix-bug.minor.rst" in invalid_map + assert "collides" in invalid_map["source/isaaclab/changelog.d/jdoe-fix-bug.minor.rst"] + + +def test_check_fragments_collision_independent_of_iterdir_order(tmp_path, monkeypatch): + """Regression: an added file must not be allowed to *replace* a colliding + pre-existing fragment in the existing-slug map. The CI checkout contains + both, and depending on filesystem iteration order the added file could + end up as the "existing" entry, hiding the collision.""" + pkg = _pkg_under(tmp_path, "isaaclab") + (pkg.root / "changelog.d").mkdir() + (pkg.root / "changelog.d" / "jdoe-foo.rst").write_text("Fixed\n^^^^^\n\n* x\n", encoding="utf-8") + (pkg.root / "changelog.d" / "jdoe-foo.minor.rst").write_text("Added\n^^^^^\n\n* y\n", encoding="utf-8") + changed = {"source/isaaclab/code.py", "source/isaaclab/changelog.d/jdoe-foo.minor.rst"} + added = changed + + # Force iterdir() to return the added file *last* so it would overwrite + # the pre-existing entry in a buggy implementation. Sort with the added + # file ranked highest, so it lands at the tail regardless of natural + # alphabetical order. + real_iterdir = Path.iterdir + added_name = "jdoe-foo.minor.rst" + + def ordered_iterdir(self): + if self == pkg.root / "changelog.d": + return iter(sorted(real_iterdir(self), key=lambda p: (p.name == added_name, p.name))) + return real_iterdir(self) + + monkeypatch.setattr(Path, "iterdir", ordered_iterdir) + + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + invalid_map = dict(invalid) + assert "source/isaaclab/changelog.d/jdoe-foo.minor.rst" in invalid_map + assert "collides" in invalid_map["source/isaaclab/changelog.d/jdoe-foo.minor.rst"] + + +def test_check_fragments_slug_collision_within_pr(tmp_path): + """Two added fragments in the same PR that share a slug (e.g. across tiers) fail.""" + pkg = _pkg_under(tmp_path, "isaaclab") + (pkg.root / "changelog.d").mkdir() + (pkg.root / "changelog.d" / "jdoe-fix.rst").write_text("Fixed\n^^^^^\n\n* x\n", encoding="utf-8") + (pkg.root / "changelog.d" / "jdoe-fix.minor.rst").write_text("Added\n^^^^^\n\n* y\n", encoding="utf-8") + changed = { + "source/isaaclab/code.py", + "source/isaaclab/changelog.d/jdoe-fix.rst", + "source/isaaclab/changelog.d/jdoe-fix.minor.rst", + } + added = changed + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + # One of the two is the offender; the other is the first-seen "winner". + invalid_paths = [p for p, _ in invalid] + assert any("jdoe-fix" in p for p in invalid_paths) + assert any("collides" in r for _, r in invalid) + + +def test_check_fragments_skip_file_satisfies_requirement(tmp_path): + """A ``.skip`` opt-out is a valid form of "PR owns a fragment for this pkg".""" + pkg = _pkg_under(tmp_path, "isaaclab") + (pkg.root / "changelog.d").mkdir() + (pkg.root / "changelog.d" / "ci-only.skip").write_text("", encoding="utf-8") + changed = {"source/isaaclab/code.py", "source/isaaclab/changelog.d/ci-only.skip"} + added = changed + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + assert missing == [] + assert invalid == [] + + +def test_check_fragments_no_source_changes_means_no_required_fragment(tmp_path): + """Pure docs / CI / changelog-tooling PRs don't trigger the requirement.""" + pkg = _pkg_under(tmp_path, "isaaclab") + changed = {"docs/something.rst"} # not under source/isaaclab/ + added = changed + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + assert missing == [] + assert invalid == [] + + +def test_check_fragments_missing_when_source_touched_without_fragment(tmp_path): + """If the PR touches a package's source but adds no fragment, the package is missing.""" + pkg = _pkg_under(tmp_path, "isaaclab") + changed = {"source/isaaclab/code.py"} + added = changed + missing, invalid = cli.PRDiff(changed=changed, added=added).evaluate([pkg]) + assert missing == ["isaaclab"] + assert invalid == [] + + +# --------------------------------------------------------------------------- +# _display_path — handles paths inside *and* outside REPO_ROOT +# --------------------------------------------------------------------------- + + +def test_display_path_strips_repo_root_for_internal_paths(): + """Inside-repo paths are shown relative for terse log lines.""" + p = cli.REPO_ROOT / "tools" / "changelog" / "cli.py" + assert cli._display_path(p) == "tools/changelog/cli.py" + + +def test_display_path_falls_back_to_absolute_for_external(tmp_path): + """External paths (e.g. ``--fragments-dir /tmp/foo`` outside the repo) + used to crash on ``relative_to(REPO_ROOT)``; the helper now returns the + absolute path in that case.""" + external = tmp_path / "external_fragments" / "1234.rst" + external.parent.mkdir(parents=True) + external.write_text("", encoding="utf-8") + assert cli._display_path(external) == str(external) + + +# --------------------------------------------------------------------------- +# Package.compile bails on unmanaged packages instead of silently warning +# --------------------------------------------------------------------------- + + +def test_compile_raises_on_package_missing_changelog(tmp_path): + """Constructing a Package directly at an unmanaged root and calling + ``compile()`` must raise (not silently warn-and-write a stale toml).""" + pkg_root = tmp_path / "pkg" + (pkg_root / "config").mkdir(parents=True) + (pkg_root / "config" / "extension.toml").write_text('version = "1.2.3"\n', encoding="utf-8") + # No docs/CHANGELOG.rst — package is not managed. + pkg = cli.Package(pkg_root) + assert pkg.is_managed is False + + fragments = tmp_path / "fragments" + fragments.mkdir() + (fragments / "1234.rst").write_text("Fixed\n^^^^^\n\n* x\n", encoding="utf-8") + + with pytest.raises(ValueError, match="not managed"): + pkg.compile(fragments_dir=fragments, dry_run=True) + + +# --------------------------------------------------------------------------- +# cmd_compile parser guards — argparse-level errors fire as SystemExit +# --------------------------------------------------------------------------- + + +def _parse_compile(argv: list[str]): + """Build the parser and parse a compile invocation. Returns (parser, args).""" + parser = cli._build_parser() + return parser, parser.parse_args(argv) + + +def test_compile_guard_version_with_all_errors(): + """``--version`` with ``--all`` is meaningless — each package has its own version.""" + parser, args = _parse_compile(["compile", "--all", "--version", "1.2.3"]) + with pytest.raises(SystemExit): + cli.cmd_compile(args, parser) + + +def test_compile_guard_fragments_dir_with_all_errors(): + """``--fragments-dir`` with ``--all`` is meaningless — different dirs per package.""" + parser, args = _parse_compile(["compile", "--all", "--fragments-dir", "/tmp/x"]) + with pytest.raises(SystemExit): + cli.cmd_compile(args, parser) + + +def test_compile_guard_malformed_version_errors(): + """A garbage ``--version`` value fails before any file is touched.""" + parser, args = _parse_compile(["compile", "--package", "isaaclab", "--version", "not-semver"]) + with pytest.raises(SystemExit): + cli.cmd_compile(args, parser) + + +def test_compile_guard_nonexistent_package_errors(): + """A ``--package`` that doesn't exist on disk fails fast.""" + parser, args = _parse_compile(["compile", "--package", "definitely_not_a_real_package_xyz"]) + with pytest.raises(SystemExit): + cli.cmd_compile(args, parser) + + +def test_compile_rejects_fragments_that_check_would_reject(tmp_path): + """``compile`` must enforce the same content rules as ``check``. + + Regression: a fragment with a section heading but no bullet body + used to slip past compile (parsed to ``{"Added": []}``, emitted an + empty Added section), while check correctly rejected it. The two + paths must agree on what a valid fragment looks like. + """ + pkg_root = tmp_path / "pkg" + (pkg_root / "config").mkdir(parents=True) + (pkg_root / "docs").mkdir(parents=True) + (pkg_root / "config" / "extension.toml").write_text('version = "1.2.3"\n', encoding="utf-8") + (pkg_root / "docs" / "CHANGELOG.rst").write_text("Changelog\n---------\n\n", encoding="utf-8") + pkg = cli.Package(pkg_root) + + fragments = tmp_path / "fragments" + fragments.mkdir() + # Header but no bullets — same shape as fixtures/invalid_content/3003.rst. + (fragments / "1234.rst").write_text("Added\n^^^^^\n\n", encoding="utf-8") + + with pytest.raises(ValueError, match="failed content validation"): + pkg.compile(fragments_dir=fragments, dry_run=True)