diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 34d4b526..12dccd31 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -84,6 +84,14 @@ Testing - Must not access private members (names starting with `_`) of the class under test directly in tests. - Must place shared test helper functions and factory fixtures in the nearest `conftest.py` and reuse them across tests. - Must annotate pytest fixture parameters with `MockerFixture` (from `pytest_mock`) and return types with `Callable[..., T]` (from `collections.abc`) when the fixture returns a factory function. +- Prefer TDD workflow: + - Must create or update `SPEC.md` in the relevant package directory before writing any code, listing scenarios, inputs, and expected outputs. + - Must propose the full set of test cases (name + one-line intent + input summary + expected output summary) and wait for user confirmation before writing any code. + - Must be ready to add, remove, or rename tests based on user feedback before proceeding. + - Must write all failing tests first (red), then implement until all pass (green). + - Must cover all distinct combinations; each test must state its scenario in the docstring. + - Must update `SPEC.md` after all tests pass with the confirmed test case table (name + intent + input + expected output). +- Must not add comments outside test methods in `test_*.py` files; use section-header comments (`# --- section ---`) only to separate logical groups of tests. Tooling - Must format with Black (pyproject.toml). diff --git a/action.yml b/action.yml index 936c16a2..a3a6edd8 100644 --- a/action.yml +++ b/action.yml @@ -29,6 +29,14 @@ inputs: At most one `catch-open-hierarchy` chapter allowed; duplicates are warned and ignored. required: false default: '' + super-chapters: + description: | + YAML array of super-chapter definitions that group regular chapters by label. + Required keys: `title`, `label` or `labels`. + Records matching a super-chapter label are rendered inside that super-chapter. + A record can appear in multiple super-chapters. + required: false + default: '' duplicity-scope: description: 'Allow duplicity of issue lines in chapters. Scopes: custom, service, both, none.' required: false @@ -181,6 +189,7 @@ runs: INPUT_GITHUB_TOKEN: ${{ env.GITHUB_TOKEN }} INPUT_TAG_NAME: ${{ inputs.tag-name }} INPUT_CHAPTERS: ${{ inputs.chapters }} + INPUT_SUPER_CHAPTERS: ${{ inputs.super-chapters }} INPUT_FROM_TAG_NAME: ${{ inputs.from-tag-name }} INPUT_HIERARCHY: ${{ inputs.hierarchy }} INPUT_DUPLICITY_SCOPE: ${{ inputs.duplicity-scope }} diff --git a/docs/configuration_reference.md b/docs/configuration_reference.md index 2a19275b..02b778f8 100644 --- a/docs/configuration_reference.md +++ b/docs/configuration_reference.md @@ -27,6 +27,7 @@ This page lists all action inputs and outputs with defaults. Grouped for readabi | `row-format-issue` | No | `{type}: {number} _{title}_ developed by {developers} in {pull-requests}` | Template for issue rows. | | `row-format-pr` | No | `{number} _{title}_ developed by {developers}` | Template for PR rows. | | `row-format-link-pr` | No | `true` | If true adds `PR:` prefix when a PR is listed without an issue. | +| `super-chapters` | No | "" | YAML multi-line list of super-chapter entries (`title` + `label`/`labels`). Groups regular chapters under higher-level headings by label. See [Super Chapters](features/custom_chapters.md#super-chapters). | > CodeRabbit summaries must already be present in the PR body (produced by your own CI/App setup). This action only parses existing summaries; it does not configure or call CodeRabbit. diff --git a/docs/features/custom_chapters.md b/docs/features/custom_chapters.md index f666aeec..a6827563 100644 --- a/docs/features/custom_chapters.md +++ b/docs/features/custom_chapters.md @@ -215,6 +215,116 @@ A `catch-open-hierarchy` chapter can also be `hidden: true` to silently track op - Duplicate `catch-open-hierarchy: true` chapters are reduced to the first; a warning is logged for the rest. - When `hierarchy: false`, a warning is logged once at populate time: `"catch-open-hierarchy has no effect when hierarchy is disabled"`. +## Super Chapters + +**Super chapters** group regular chapters under higher-level headings based on a separate label. This is useful in monorepo or multi-module projects where the same chapter structure (Features, Bugfixes, …) should appear once per component. + +### Configuration + +Define super chapters via the `super-chapters` input — a YAML array with `title` and `label`/`labels`: + +```yaml +with: + super-chapters: | + - title: "Atum Server" + label: "atum-server" + - title: "Atum Agent" + labels: "atum-agent, atum-agent-spark" + chapters: | + - {"title": "Enhancements", "label": "enhancement"} + - {"title": "Bugfixes", "label": "bug"} +``` + +### Rendering + +When super chapters are configured the output uses `##` headings for super chapters and `###` for regular chapters nested inside: + +```markdown +## Atum Server +### Enhancements +- #10 _Streaming API_ developed by @alice in #12 + +### Bugfixes +- #15 _Fix timeout_ developed by @bob in #16 + +## Atum Agent +### Enhancements +- #20 _Checkpointing_ developed by @carol in #22 +``` + +### Behavior +- A record is placed under a super-chapter if it carries at least one label matching the super-chapter's labels. +- A record can appear in **multiple** super-chapters if its labels match more than one. +- Within each super-chapter, records are routed to regular chapters by the normal label-matching rules. +- Empty super chapters (no matching records) respect the `print-empty-chapters` setting: + - `print-empty-chapters: true` → header is printed with `No entries detected.` + - `print-empty-chapters: false` → header is omitted entirely +- `## Uncategorized` is only emitted when there are actually unmatched records; `print-empty-chapters` has no effect on it. +- When no super chapters are configured, output is flat (unchanged from previous behavior). + +### Hierarchy Split (with `hierarchy: true`) + +With `hierarchy: true`, super-chapter matching uses each hierarchy record's **full aggregated label set** — own labels plus all descendant labels recursively at every depth. So an Epic matches a super chapter even when the relevant label lives only on a grandchild Task (e.g. Epic → Feature → Task). + +The record is then split by which descendants belong to which super chapter: + +| Descendants | Output | +|---|---| +| All match one super chapter | Record appears in that super chapter only | +| None match any super chapter | Record appears in `## Uncategorized` only | +| Some match, some don't | Record appears in the matching super chapter **and** in `## Uncategorized`, each showing only its own subset | +| Match multiple super chapters | Record appears in each matching super chapter with its relevant subset | +| Match multiple SCs + some unmatched | Record appears in each matching super chapter and in `## Uncategorized` | + +#### Example + +Epic #1 has Task #2 (`scope:frontend`) and Task #3 (`scope:backend`): + +```yaml +super-chapters: | + - title: "Frontend" + label: "scope:frontend" + - title: "Backend" + label: "scope:backend" +chapters: | + - {"title": "New Features", "labels": "feature"} +``` + +```markdown +## Frontend +### New Features +- Epic: _Add user authentication_ #1 + - #2 _Build login form_ + +## Backend +### New Features +- Epic: _Add user authentication_ #1 + - #3 _Add JWT endpoint_ +``` + +If Epic #1 also had Task #4 with no super-chapter label, it would additionally appear in `## Uncategorized` with only Task #4. + +#### 3-level depth + +The same split works when the matching label is on a grandchild. Epic #1 → Feature #2 → Task #3 (`scope:security`): + +```markdown +## Security +### New Features +- Epic: _Add authentication_ #1 + - Feature: _Auth flow_ #2 + - #3 _Add JWT endpoint_ +``` + +Feature #2 has no `scope:security` label of its own, but its aggregated set includes it via Task #3, so it is routed to the Security super chapter. + +Children within each rendered node are sorted **ascending by issue number**. + +### Validation +- Entries missing `title` or `label`/`labels` are skipped with a warning. +- Non-dict entries are skipped with a warning. +- Empty labels after normalization cause the entry to be skipped with a warning. + ## Related Features - [Duplicity Handling](./duplicity_handling.md) – governs multi-chapter visibility. - [Release Notes Extraction](./release_notes_extraction.md) – provides the change increment lines. diff --git a/docs/features/issue_hierarchy_support.md b/docs/features/issue_hierarchy_support.md index 9cf6efb3..484e4d02 100644 --- a/docs/features/issue_hierarchy_support.md +++ b/docs/features/issue_hierarchy_support.md @@ -75,6 +75,7 @@ Resulting output (hierarchy issues only — sub-issue rows use `row-format-issue - [Custom Row Formats](./custom_row_formats.md) – controls hierarchy line rendering. - [Service Chapters](./service_chapters.md) – flags missing change increments if hierarchy parents lack qualifying sub-issues. - [Duplicity Handling](./duplicity_handling.md) – duplicate hierarchy items can be icon-prefixed if allowed. +- [Super Chapters](./custom_chapters.md#super-chapters) – hierarchy records split across super chapters when descendants carry different super-chapter labels. ← [Back to Feature Tutorials](../../README.md#feature-tutorials) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index 8b6bc841..0e51ad70 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -22,7 +22,7 @@ import os import sys import re - +from typing import Any import yaml from release_notes_generator.utils.constants import ( @@ -30,6 +30,7 @@ GITHUB_TOKEN, TAG_NAME, CHAPTERS, + SUPER_CHAPTERS, PUBLISHED_AT, VERBOSE, WARNINGS, @@ -59,7 +60,7 @@ ) from release_notes_generator.utils.enums import DuplicityScopeEnum from release_notes_generator.utils.gh_action import get_action_input -from release_notes_generator.utils.utils import normalize_version_tag +from release_notes_generator.utils.utils import normalize_labels, normalize_version_tag logger = logging.getLogger(__name__) @@ -141,7 +142,7 @@ def get_from_tag_name() -> str: Get the from-tag name from the action inputs. """ raw = get_action_input(FROM_TAG_NAME, default="") - return normalize_version_tag(raw) # type: ignore[arg-type] + return normalize_version_tag(raw) @staticmethod def is_from_tag_name_defined() -> bool: @@ -166,11 +167,64 @@ def get_chapters() -> list[dict[str, str]]: logger.error("Error: 'chapters' input is not a valid YAML list.") return [] except yaml.YAMLError as exc: - logger.error("Error parsing 'chapters' input: {%s}", exc) + logger.error("Error parsing 'chapters' input: %s", exc) return [] return chapters + @staticmethod + def get_super_chapters() -> list[dict[str, Any]]: + """ + Get list of validated super-chapter definitions from the action inputs. + + Each returned entry is guaranteed to have: + - 'title': str + - 'labels': list[str] (non-empty, normalized) + + Invalid entries (non-dict, missing title, missing/empty labels) are skipped + with a warning log. + """ + # Get the 'super-chapters' input from environment variables + super_chapters_input: str = get_action_input(SUPER_CHAPTERS, default="") + + # Parse the received string back to YAML array input. + try: + raw_list = yaml.safe_load(super_chapters_input) + if raw_list is None: + return [] + if not isinstance(raw_list, list): + logger.error("Error: 'super-chapters' input is not a valid YAML list.") + return [] + except yaml.YAMLError as exc: + logger.error("Error parsing 'super-chapters' input: %s", exc) + return [] + + result: list[dict[str, Any]] = [] + for entry in raw_list: + if not isinstance(entry, dict): + logger.warning("Skipping super-chapter definition with invalid type %s: %s", type(entry), entry) + continue + if "title" not in entry: + logger.warning("Skipping super-chapter without title key: %s", entry) + continue + title = entry["title"] + if not isinstance(title, str) or not title.strip(): + logger.warning("Skipping super-chapter with invalid title value: %r", title) + continue + + raw_labels = entry.get("labels", entry.get("label")) + if raw_labels is None: + logger.warning("Super-chapter '%s' has no 'label' or 'labels' key; skipping", title) + continue + normalized = normalize_labels(raw_labels) + if not normalized: + logger.warning("Super-chapter '%s' labels definition empty after normalization; skipping", title) + continue + + result.append({"title": title, "labels": normalized}) + logger.debug("Validated super-chapter '%s' with labels: %s", title, normalized) + return result + @staticmethod def get_hierarchy() -> bool: """ @@ -351,8 +405,7 @@ def get_print_empty_chapters() -> bool: """ Get the print empty chapters parameter value from the action inputs. """ - return get_action_input(PRINT_EMPTY_CHAPTERS, "true").lower() == "true" # type: ignore[union-attr] - # mypy: string is returned as default + return get_action_input(PRINT_EMPTY_CHAPTERS, "true").lower() == "true" @staticmethod def validate_input(input_value, expected_type: type, error_message: str, error_buffer: list) -> bool: @@ -540,6 +593,8 @@ def validate_inputs() -> None: logger.debug("CodeRabbit summary ignore groups: %s", coderabbit_summary_ignore_groups) logger.debug("Hidden service chapters: %s", ActionInputs.get_hidden_service_chapters()) logger.debug("Service chapter order: %s", ActionInputs.get_service_chapter_order()) + super_chapters = ActionInputs.get_super_chapters() + logger.debug("Super chapters: %s", super_chapters) @staticmethod def _detect_row_format_invalid_keywords(row_format: str, row_type: str = "Issue", clean: bool = False) -> str: diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index 0975fad7..9c49af1f 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -20,54 +20,27 @@ """ import logging -from typing import Any, Iterable +from dataclasses import dataclass, field +from typing import Any from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.chapters.base_chapters import BaseChapters from release_notes_generator.model.chapter import Chapter +from release_notes_generator.utils.constants import UNCATEGORIZED_CHAPTER_TITLE from release_notes_generator.model.record.commit_record import CommitRecord from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord from release_notes_generator.model.record.record import Record +from release_notes_generator.utils.utils import normalize_labels logger = logging.getLogger(__name__) -def _normalize_labels(raw: Any) -> list[str]: # helper for multi-label - """Normalize a raw labels definition into an ordered, de-duplicated list. +@dataclass +class SuperChapter: + """A label-based grouping that wraps regular chapters in the rendered output.""" - Parameters: - - str: may contain newlines and/or commas - - list[str]: already a sequence of labels (will still be trimmed & deduped) - - Returns: - Ordered list preserving first occurrence order; excludes empty tokens. - Invalid (non str/list) returns empty list to be handled by caller. - """ - if isinstance(raw, list): - working: Iterable[Any] = raw - elif isinstance(raw, str): - # newline first then comma per spec - parts: list[str] = [] - for line in raw.splitlines(): - for token in line.split(","): - parts.append(token) - working = parts - else: - return [] - - cleaned: list[str] = [] - seen: set[str] = set() - for item in working: - if not isinstance(item, str): # skip non-string items silently - continue - token = item.strip() - if not token: # skip empty after trimming - continue - if token in seen: # de-duplicate preserving first occurrence - continue - seen.add(token) - cleaned.append(token) - return cleaned + title: str + labels: list[str] = field(default_factory=list) class CustomChapters(BaseChapters): @@ -75,6 +48,12 @@ class CustomChapters(BaseChapters): A class used to represent the custom chapters in the release notes. """ + def __init__(self, sort_ascending: bool = True, print_empty_chapters: bool = True): + super().__init__(sort_ascending, print_empty_chapters) + self._super_chapters: list[SuperChapter] = [] + self._record_labels: dict[str, list[str]] = {} + self._records: dict[str, Record] = {} + def _find_catch_open_hierarchy_chapter(self) -> Chapter | None: """Return the first chapter with catch_open_hierarchy enabled, or None.""" for ch in self.chapters.values(): @@ -88,6 +67,7 @@ def _add_record_to_chapter(self, record_id: str, record: Record, chapter: Chapte if not chapter.hidden: record.add_to_chapter_presence(chapter.title) chapter.add_row(record_id, record.to_chapter_row(not chapter.hidden)) + self._records[record_id] = record if record_id not in self.populated_record_numbers_list: self.populated_record_numbers_list.append(record_id) if chapter.hidden and ActionInputs.get_verbose(): @@ -136,6 +116,16 @@ def populate(self, records: dict[str, Record]) -> None: record_labels = getattr(record, "labels", []) + # Track labels for super-chapter grouping at render time. + # HierarchyIssueRecords use aggregated labels (own + descendants) so the + # hierarchy parent matches super chapters via sub-issue labels. + if isinstance(record, HierarchyIssueRecord): + all_labels = record.get_labels() + if all_labels: + self._record_labels[record_id] = list(all_labels) + elif record_labels: + self._record_labels[record_id] = list(record_labels) + # Conditional Custom Chapter gate: intercept open hierarchy parents before label routing. # Note: precedes the record_labels early-exit so label-less HierarchyIssueRecord # is still routed to a no-filter COH chapter. @@ -175,9 +165,21 @@ def to_string(self) -> str: """ Converts the custom chapters to a string, excluding hidden chapters. + When super chapters are configured, records are grouped under super-chapter + headings (``## Title``) with regular chapters nested inside (``### Title``). + A record may appear in multiple super chapters. Chapters with no matching + records under a given super chapter are governed by ``print_empty_chapters``. + Returns: str: The chapters as a string with hidden chapters filtered out. """ + if self._super_chapters: + return self._to_string_with_super_chapters() + + return self._to_string_flat() + + def _to_string_flat(self) -> str: + """Render chapters without super-chapter grouping (original behaviour).""" result = "" for chapter in self._sorted_chapters(): # Skip hidden chapters from output @@ -196,7 +198,150 @@ def to_string(self) -> str: # Note: strip is required to remove leading newline chars when empty chapters are not printed option return result.strip() - def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": # type: ignore[override] + def _collect_super_chapter_ids(self, sc: SuperChapter) -> set[str]: + """Return record IDs whose labels match the given super chapter.""" + matching: set[str] = set() + for rid, labels in self._record_labels.items(): + if any(lbl in sc.labels for lbl in labels): + matching.add(rid) + return matching + + def _render_chapter_for_ids( + self, + chapter: Chapter, + matching_ids: set[str], + super_labels: list[str] | None = None, + exclude_labels: list[str] | None = None, + print_empty_chapters: bool | None = None, + ) -> str: + """Render a single chapter filtered to matching_ids, delegating to Chapter.to_string. + + Parameters: + chapter: The chapter to render. + matching_ids: Record IDs to include. + super_labels: When set, HierarchyIssueRecords are re-rendered with sub-issues + filtered to those whose labels intersect *super_labels*. + exclude_labels: When set, HierarchyIssueRecords are re-rendered with sub-issues + whose labels intersect *exclude_labels* removed (for Uncategorized fallback). + print_empty_chapters: Override for the empty-chapter setting; defaults to + ``self.print_empty_chapters`` when ``None``. + """ + effective_print_empty = self.print_empty_chapters if print_empty_chapters is None else print_empty_chapters + original_rows = chapter.rows + filtered_rows: dict[int | str, str] = {} + for rid, row in original_rows.items(): + if str(rid) not in matching_ids and rid not in matching_ids: + continue + record = self._records.get(str(rid)) + if super_labels and isinstance(record, HierarchyIssueRecord): + if not record.has_matching_labels(super_labels): + continue + filtered_rows[rid] = record.to_chapter_row(add_into_chapters=False, label_filter=super_labels) + elif exclude_labels and isinstance(record, HierarchyIssueRecord): + filtered_rows[rid] = record.to_chapter_row(add_into_chapters=False, exclude_labels=exclude_labels) + else: + filtered_rows[rid] = row + chapter.rows = filtered_rows + try: + return chapter.to_string(sort_ascending=self.sort_ascending, print_empty_chapters=effective_print_empty) + finally: + chapter.rows = original_rows + + def _collect_claimed_ids(self, all_super_labels: set[str]) -> set[str]: + """Return record IDs whose labels intersect any super-chapter label.""" + return {rid for rid, labels in self._record_labels.items() if any(lbl in all_super_labels for lbl in labels)} + + def _render_super_chapter_block(self, sc: SuperChapter) -> str: + """Render all chapters filtered to the given super chapter's matching records.""" + matching_ids = self._collect_super_chapter_ids(sc) + sc_block = "" + for chapter in self._sorted_chapters(): + if chapter.hidden: + continue + ch_str = self._render_chapter_for_ids(chapter, matching_ids, super_labels=sc.labels) + if ch_str: + sc_block += ch_str + "\n\n" + if sc_block.strip(): + return f"## {sc.title}\n{sc_block}" + if self.print_empty_chapters: + return f"## {sc.title}\nNo entries detected.\n\n" + return "" + + def _collect_uncategorized_ids(self, claimed_ids: set[str], all_super_labels_list: list[str]) -> set[str]: + """Return IDs for records not claimed by any super chapter, plus partially-matched hierarchy IDs.""" + unclaimed_ids: set[str] = set() + partial_hierarchy_ids: set[str] = set() + for chapter in self._sorted_chapters(): + for row_id in chapter.rows: + row_id_str = str(row_id) + if row_id_str not in claimed_ids: + unclaimed_ids.add(row_id_str) + else: + record = self._records.get(row_id_str) + if isinstance(record, HierarchyIssueRecord) and record.has_unmatched_descendants( + all_super_labels_list + ): + partial_hierarchy_ids.add(row_id_str) + return unclaimed_ids | partial_hierarchy_ids + + def _render_uncategorized_block(self, all_uncat_ids: set[str], all_super_labels_list: list[str]) -> str: + """Render the Uncategorized section for records not claimed by any super chapter.""" + if not all_uncat_ids: + return "" + uc_block = "" + for chapter in self._sorted_chapters(): + if chapter.hidden: + continue + ch_str = self._render_chapter_for_ids( + chapter, all_uncat_ids, exclude_labels=all_super_labels_list, print_empty_chapters=False + ) + if ch_str: + uc_block += ch_str + "\n\n" + if uc_block.strip(): + return f"## {UNCATEGORIZED_CHAPTER_TITLE}\n{uc_block}" + return "" + + def _to_string_with_super_chapters(self) -> str: + """Render chapters grouped under super-chapter headings.""" + all_super_labels: set[str] = {lbl for sc in self._super_chapters for lbl in sc.labels} + all_super_labels_list = list(all_super_labels) + claimed_ids = self._collect_claimed_ids(all_super_labels) + result = "".join(self._render_super_chapter_block(sc) for sc in self._super_chapters) + result += self._render_uncategorized_block( + self._collect_uncategorized_ids(claimed_ids, all_super_labels_list), + all_super_labels_list, + ) + return result.strip() + + @staticmethod + def _enforce_coh_constraint( + title: str, catch_open_hierarchy: bool, coh_chapter_title: str | None + ) -> tuple[bool, str | None]: + """Enforce the single catch-open-hierarchy chapter constraint. + + Parameters: + title: Chapter title being processed. + catch_open_hierarchy: Whether the current entry requests COH. + coh_chapter_title: Title of the first COH chapter seen so far, or None. + + Returns: + Updated (catch_open_hierarchy, coh_chapter_title) tuple. + """ + if not catch_open_hierarchy: + return False, coh_chapter_title + if coh_chapter_title is not None and coh_chapter_title != title: + logger.warning( + "Chapter '%s' has catch-open-hierarchy: true but '%s' already uses it; ignoring duplicate", + title, + coh_chapter_title, + ) + return False, coh_chapter_title + if coh_chapter_title is None: + return True, title + # else: same title repeating COH — silent merge pass-through + return True, coh_chapter_title + + def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": """ Populate the custom chapters from a list of dict objects. @@ -228,20 +373,9 @@ def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": catch_open_hierarchy = self._parse_bool_flag( title, chapter.get("catch-open-hierarchy", False), "catch-open-hierarchy" ) - - # Enforce single catch-open-hierarchy chapter constraint. - # Same-title repeats are treated as a merge and allowed through. - if catch_open_hierarchy: - if coh_chapter_title is not None and coh_chapter_title != title: - logger.warning( - "Chapter '%s' has catch-open-hierarchy: true but '%s' already uses it; ignoring duplicate", - title, - coh_chapter_title, - ) - catch_open_hierarchy = False - elif coh_chapter_title is None: - coh_chapter_title = title - # else: same title repeating COH — silent merge pass-through + catch_open_hierarchy, coh_chapter_title = self._enforce_coh_constraint( + title, catch_open_hierarchy, coh_chapter_title + ) unknown = set(chapter.keys()) - allowed_keys if unknown: @@ -262,8 +396,23 @@ def from_yaml_array(self, chapters: list[dict[str, Any]]) -> "CustomChapters": parsed_order=parsed_order, ) + # Parse super-chapter definitions from action inputs + self._super_chapters = self._parse_super_chapters(ActionInputs.get_super_chapters()) + return self + @staticmethod + def _parse_super_chapters(validated: list[dict[str, Any]]) -> list[SuperChapter]: + """Construct SuperChapter instances from pre-validated ActionInputs dicts. + + Parameters: + validated: List of fully-validated super-chapter dicts with 'title' and 'labels' keys. + + Returns: + List of SuperChapter instances. + """ + return [SuperChapter(title=e["title"], labels=e["labels"]) for e in validated] + @staticmethod def _parse_bool_flag(title: str, raw: Any, key: str) -> bool: """Parse and validate a boolean flag from a chapter config entry. @@ -370,7 +519,7 @@ def _parse_labels_definition(title: str, chapter: dict[str, Any], catch_open_hie ) return None - normalized = _normalize_labels(raw_labels) + normalized = normalize_labels(raw_labels) if not normalized: logger.warning("Chapter '%s' labels definition empty after normalization; skipping", title) return None diff --git a/release_notes_generator/data/utils/bulk_sub_issue_collector.py b/release_notes_generator/data/utils/bulk_sub_issue_collector.py index e0819114..b73a69f6 100644 --- a/release_notes_generator/data/utils/bulk_sub_issue_collector.py +++ b/release_notes_generator/data/utils/bulk_sub_issue_collector.py @@ -24,8 +24,6 @@ import logging import time from dataclasses import dataclass -from typing import Dict, List, Optional, Set, Tuple - import requests from release_notes_generator.utils.record_utils import parse_issue_id, format_issue_id @@ -66,8 +64,8 @@ class BulkSubIssueCollector: def __init__( self, token: str, - cfg: Optional[CollectorConfig] = None, - session: Optional[requests.Session] = None, + cfg: CollectorConfig | None = None, + session: requests.Session | None = None, ): self._cfg = cfg or CollectorConfig() self._session = session or requests.Session() @@ -89,7 +87,7 @@ def scan_sub_issues_for_parents(self, parents_to_check: list[str]) -> list[str]: if not parents_to_check: return [] - new_parents_to_check: Set[str] = set() + new_parents_to_check: set[str] = set() self.parents_sub_issues = {} by_repo: dict[tuple[str, str], list[int]] = {} @@ -106,8 +104,8 @@ def scan_sub_issues_for_parents(self, parents_to_check: list[str]) -> list[str]: repo_chunk = repo_items[i : i + self._cfg.max_repos_per_request] # Maintain cursors per (org, repo, issue). - cursors: Dict[Tuple[str, str, int], Optional[str]] = {} - remaining_by_repo: Dict[Tuple[str, str], Set[int]] = {k: set(v) for k, v in repo_chunk} + cursors: dict[tuple[str, str, int], str | None] = {} + remaining_by_repo: dict[tuple[str, str], set[int]] = {k: set(v) for k, v in repo_chunk} for (org, repo), nums in remaining_by_repo.items(): for n in nums: cursors[(org, repo, n)] = None @@ -116,14 +114,14 @@ def scan_sub_issues_for_parents(self, parents_to_check: list[str]) -> list[str]: while any(remaining_by_repo.values()): # Build one GraphQL query with up to max_repos_per_request repos, # each with up to max_parents_per_repo parent issues that still have pages. - repo_blocks: List[str] = [] - alias_maps: Dict[str, Tuple[str, str, int]] = {} # alias -> (org, repo, parent_num) + repo_blocks: list[str] = [] + alias_maps: dict[str, tuple[str, str, int]] = {} # alias -> (org, repo, parent_num) for r_idx, ((org, repo), parents_rem) in enumerate(remaining_by_repo.items()): if not parents_rem: continue current_parents = list(parents_rem)[: self._cfg.max_parents_per_repo] - issue_blocks: List[str] = [] + issue_blocks: list[str] = [] for p_idx, parent_num in enumerate(current_parents): alias = f"i{r_idx}_{p_idx}" alias_maps[alias] = (org, repo, parent_num) @@ -210,7 +208,7 @@ def scan_sub_issues_for_parents(self, parents_to_check: list[str]) -> list[str]: # ---------- internals ---------- def _post_graphql(self, payload: dict) -> dict: - last_exc: Optional[Exception] = None + last_exc: Exception | None = None for attempt in range(1, self._cfg.max_retries + 1): try: logger.debug("Posting graphql query") @@ -230,16 +228,23 @@ def _post_graphql(self, payload: dict) -> dict: logger.debug("Posted graphql query") return data except Exception as e: # pylint: disable=broad-exception-caught - logger.exception("GraphQL query failed") last_exc = e if attempt == self._cfg.max_retries: + logger.exception("GraphQL query failed after %d attempts", self._cfg.max_retries) raise + logger.warning( + "GraphQL query failed (attempt %d/%d): %s; retrying in %.1fs", + attempt, + self._cfg.max_retries, + e, + self._cfg.base_backoff * attempt, + ) time.sleep(self._cfg.base_backoff * attempt) if last_exc: raise last_exc raise RuntimeError("GraphQL POST failed without exception.") - def _find_alias_node(self, repo_block: dict, alias: str) -> Optional[dict]: + def _find_alias_node(self, repo_block: dict, alias: str) -> dict | None: """ Given top-level 'data' (mapping of repo aliases -> repo object), return the 'issue' object under whichever repo alias contains our issue alias. diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index 9b429584..1201711e 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -19,7 +19,6 @@ """ import logging -from typing import Optional, Any from github.Issue import Issue from release_notes_generator.action_inputs import ActionInputs @@ -36,7 +35,7 @@ class HierarchyIssueRecord(IssueRecord): Inherits from IssueRecord and provides additional functionality specific to issues. """ - def __init__(self, issue: Issue, issue_labels: Optional[list[str]] = None, skip: bool = False): + def __init__(self, issue: Issue, issue_labels: list[str] | None = None, skip: bool = False): super().__init__(issue, issue_labels, skip) self._level: int = 0 @@ -93,6 +92,7 @@ def progress(self) -> str: @property def developers(self) -> list[str]: + """Unique, sorted list of developers across this issue and all descendants.""" issue = self._issue if not issue: return [] @@ -116,6 +116,7 @@ def developers(self) -> list[str]: return sorted(devs) def pull_requests_count(self) -> int: + """Return the total number of pull requests across this issue and all descendants.""" count = super().pull_requests_count() for sub_issue in self._sub_issues.values(): @@ -160,105 +161,190 @@ def contains_change_increment(self) -> bool: return False def get_labels(self) -> list[str]: + """Return all labels from this issue, its sub-issues, sub-hierarchy-issues, and attached PRs.""" labels: set[str] = set() - labels.update(label.name for label in self._issue.get_labels()) + if self._labels is not None: + labels.update(self._labels) + else: + labels.update(label.name for label in self._issue.get_labels()) for sub_issue in self._sub_issues.values(): labels.update(sub_issue.labels) for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): - labels.update(sub_hierarchy_issue.labels) + labels.update(sub_hierarchy_issue.get_labels()) for pull in self._pull_requests.values(): labels.update(label.name for label in pull.get_labels()) return list(labels) + def has_matching_labels(self, label_filter: list[str]) -> bool: + """Check if this hierarchy issue or any descendant has labels matching the filter. + + Uses the same aggregated label set as get_labels() so that PR labels attached to + this record or its descendants are considered when matching super-chapter filters. + + Parameters: + label_filter: Labels to check against. + + Returns: + True if this record or any descendant has at least one matching label. + """ + return any(lbl in label_filter for lbl in self.get_labels()) + + def has_unmatched_descendants(self, all_super_labels: list[str]) -> bool: + """Check if any descendant does NOT match any label in the combined super-chapter label set. + + Parameters: + all_super_labels: Union of all super-chapter labels. + + Returns: + True if at least one descendant has no label intersecting *all_super_labels*. + """ + for sub_issue in self._sub_issues.values(): + if not any(lbl in all_super_labels for lbl in sub_issue.labels): + return True + for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): + if sub_hierarchy_issue.has_unmatched_descendants(all_super_labels): + return True + # A leaf HierarchyIssueRecord (no children of its own) is unmatched if + # its own labels don't intersect the SC set. Intermediate nodes are + # pure containers — their own organisational labels are irrelevant. + is_leaf = not sub_hierarchy_issue.sub_issues and not sub_hierarchy_issue.sub_hierarchy_issues + if is_leaf and not any(lbl in all_super_labels for lbl in sub_hierarchy_issue.labels): + return True + return False + # methods - override ancestor methods - def to_chapter_row(self, add_into_chapters: bool = True) -> str: - logger.debug("Rendering hierarchy issue row for issue #%s", self.issue.number) - row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.chapter_presence_count() > 1 else "" - format_values: dict[str, Any] = {} - # collect format values + def _collect_format_values(self) -> dict[str, str]: + """Collect template substitution values for the hierarchy issue row format string.""" + format_values: dict[str, str] = {} format_values["number"] = f"#{self.issue.number}" format_values["title"] = self.issue.title format_values["author"] = self.author format_values["assignees"] = ", ".join(self.assignees) format_values["developers"] = ", ".join(self.developers) - if self.issue_type is not None: - format_values["type"] = self.issue_type - else: - format_values["type"] = "" + format_values["type"] = self.issue_type if self.issue_type is not None else "" format_values["progress"] = self.progress - list_pr_links = self.get_pr_links() - if len(list_pr_links) > 0: - format_values["pull-requests"] = ", ".join(list_pr_links) + format_values["pull-requests"] = ", ".join(list_pr_links) if list_pr_links else "" + return format_values + + def _append_release_notes_block(self, row: str) -> str: + """Append an indented Release Notes section to *row* when present.""" + if not self.contains_release_notes(): + return row + sub_indent: str = " " * (self._level + 1) + row = f"{row}\n{sub_indent}- _Release Notes_:" + rls_block = "\n".join(f"{sub_indent}{line}" if line else "" for line in self.get_rls_notes().splitlines()) + return f"{row}\n{rls_block}" + + def _build_open_sub_hierarchy_row( + self, + sub_hierarchy_issue: "HierarchyIssueRecord", + label_filter: list[str] | None, + exclude_labels: list[str] | None, + ) -> str: + """Render an open sub-hierarchy issue row and insert the open-hierarchy icon after the list marker. + + Highlight open children under a closed parent to signal incomplete work. + Insert icon after the list marker ('- ') to preserve Markdown structure, + and skip insertion entirely when the icon is empty. + """ + sub_row = sub_hierarchy_issue.to_chapter_row(label_filter=label_filter, exclude_labels=exclude_labels) + icon = ActionInputs.get_open_hierarchy_sub_issue_icon() + if not icon: + return sub_row + header_line, newline, remaining_lines = sub_row.partition("\n") + header_text = header_line.lstrip() + spaces = header_line[: len(header_line) - len(header_text)] + if header_text.startswith("- "): + marker, content = "- ", header_text[2:] else: - format_values["pull-requests"] = "" - - indent: str = " " * self._level - if self._level > 0: - indent += "- " - - # create first issue row - row = f"{indent}{row_prefix}" + format_row_with_suppression( - ActionInputs.get_row_format_hierarchy_issue(), format_values - ) - - # add extra section with release notes if detected - if self.contains_release_notes(): - sub_indent: str = " " * (self._level + 1) - row = f"{row}\n{sub_indent}- _Release Notes_:" - rls_block = "\n".join(f"{sub_indent}{line}" if line else "" for line in self.get_rls_notes().splitlines()) - row = f"{row}\n{rls_block}" - - # add sub-hierarchy issues - for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): + marker, content = "", header_text + return f"{spaces}{marker}{icon} {content}{newline}{remaining_lines}" + + def _append_sub_hierarchy_rows( + self, + row: str, + label_filter: list[str] | None, + exclude_labels: list[str] | None, + ) -> str: + """Append rendered rows for all qualifying sub-hierarchy issues to *row*.""" + for sub_hierarchy_issue in sorted(self._sub_hierarchy_issues.values(), key=lambda r: r.issue.number): logger.debug("Rendering sub-hierarchy issue row for #%s", sub_hierarchy_issue.issue.number) - if self.is_open: - if not sub_hierarchy_issue.contains_change_increment(): - continue + if label_filter and not sub_hierarchy_issue.has_matching_labels(label_filter): + continue + if ( + exclude_labels + and not sub_hierarchy_issue.has_unmatched_descendants(exclude_labels) + and sub_hierarchy_issue.has_matching_labels(exclude_labels) + ): + continue + if self.is_open and not sub_hierarchy_issue.contains_change_increment(): + continue # Closed parent: render all sub-hierarchy issues regardless of state or change increment logger.debug("Rendering sub-hierarchy issue #%s", sub_hierarchy_issue.issue.number) if self.is_closed and sub_hierarchy_issue.is_open: - sub_row = sub_hierarchy_issue.to_chapter_row() - # Highlight open children under a closed parent to signal incomplete work - icon = ActionInputs.get_open_hierarchy_sub_issue_icon() - header_line, newline, remaining_lines = sub_row.partition("\n") - header_text = header_line.lstrip() - indent = header_line[: len(header_line) - len(header_text)] - sub_row = f"{indent}{icon} {header_text}{newline}{remaining_lines}" + sub_row = self._build_open_sub_hierarchy_row(sub_hierarchy_issue, label_filter, exclude_labels) else: - sub_row = sub_hierarchy_issue.to_chapter_row() + sub_row = sub_hierarchy_issue.to_chapter_row(label_filter=label_filter, exclude_labels=exclude_labels) row = f"{row}\n{sub_row}" + return row - # add sub-issues - if len(self._sub_issues) > 0: - sub_indent = " " * (self._level + 1) - for sub_issue in self._sub_issues.values(): - logger.debug("Rendering sub-issue row for issue #%s", sub_issue.issue.number) - if self.is_open: - if sub_issue.is_open: - continue # only closed issues are reported in release notes - if not sub_issue.contains_change_increment(): - continue # skip sub-issues without change increment - # Closed parent: render all sub-issues regardless of state or change increment - - logger.debug("Rendering sub-issue #%s", sub_issue.issue.number) - open_icon_prefix = "" - if self.is_closed and sub_issue.is_open: - # Highlight open children under a closed parent to signal incomplete work - open_icon_prefix = ActionInputs.get_open_hierarchy_sub_issue_icon() + " " - sub_issue_block = "- " + open_icon_prefix + sub_issue.to_chapter_row() - ind_child_block = "\n".join( - f"{sub_indent}{line}" if line else "" for line in sub_issue_block.splitlines() - ) - row = f"{row}\n{ind_child_block}" - # else: this will be reported in service chapters as violation of hierarchy in this initial version - # No data loss - in service chapter there will be all detail not presented here + def _append_sub_issue_rows( + self, + row: str, + label_filter: list[str] | None, + exclude_labels: list[str] | None, + ) -> str: + """Append rendered rows for all qualifying direct sub-issues to *row*.""" + if not self._sub_issues: + # No sub-issues: violations of hierarchy are reported in service chapters (no data loss) + return row + sub_indent = " " * (self._level + 1) + for sub_issue in sorted(self._sub_issues.values(), key=lambda r: r.issue.number): + logger.debug("Rendering sub-issue row for issue #%s", sub_issue.issue.number) + if label_filter and not any(lbl in label_filter for lbl in sub_issue.labels): + continue + if exclude_labels and any(lbl in exclude_labels for lbl in sub_issue.labels): + continue + if self.is_open: + if sub_issue.is_open: + continue # only closed issues are reported in release notes + if not sub_issue.contains_change_increment(): + continue # skip sub-issues without change increment + # Closed parent: render all sub-issues regardless of state or change increment + logger.debug("Rendering sub-issue #%s", sub_issue.issue.number) + open_icon_prefix = "" + if self.is_closed and sub_issue.is_open: + # Highlight open children under a closed parent to signal incomplete work + icon = ActionInputs.get_open_hierarchy_sub_issue_icon() + open_icon_prefix = (icon + " ") if icon else "" + sub_issue_block = "- " + open_icon_prefix + sub_issue.to_chapter_row() + ind_child_block = "\n".join(f"{sub_indent}{line}" if line else "" for line in sub_issue_block.splitlines()) + row = f"{row}\n{ind_child_block}" + return row + def to_chapter_row( + self, + add_into_chapters: bool = True, + label_filter: list[str] | None = None, + exclude_labels: list[str] | None = None, + ) -> str: + logger.debug("Rendering hierarchy issue row for issue #%s", self.issue.number) + row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.chapter_presence_count() > 1 else "" + indent: str = " " * self._level + if self._level > 0: + indent += "- " + row = f"{indent}{row_prefix}" + format_row_with_suppression( + ActionInputs.get_row_format_hierarchy_issue(), self._collect_format_values() + ) + row = self._append_release_notes_block(row) + row = self._append_sub_hierarchy_rows(row, label_filter, exclude_labels) + row = self._append_sub_issue_rows(row, label_filter, exclude_labels) return row def order_hierarchy_levels(self, level: int = 0) -> None: diff --git a/release_notes_generator/utils/constants.py b/release_notes_generator/utils/constants.py index 38a2c35b..5f11a912 100644 --- a/release_notes_generator/utils/constants.py +++ b/release_notes_generator/utils/constants.py @@ -24,6 +24,7 @@ TAG_NAME = "tag-name" FROM_TAG_NAME = "from-tag-name" CHAPTERS = "chapters" +SUPER_CHAPTERS = "super-chapters" DUPLICITY_SCOPE = "duplicity-scope" DUPLICITY_ICON = "duplicity-icon" OPEN_HIERARCHY_SUB_ISSUE_ICON = "open-hierarchy-sub-issue-icon" @@ -49,6 +50,9 @@ SERVICE_CHAPTER_ORDER = "service-chapter-order" PRINT_EMPTY_CHAPTERS = "print-empty-chapters" +# Super chapter fallback heading +UNCATEGORIZED_CHAPTER_TITLE: str = "Uncategorized" + # Release notes comment constants RELEASE_NOTE_TITLE_DEFAULT: str = "[Rr]elease [Nn]otes:" CODERABBIT_RELEASE_NOTE_TITLE_DEFAULT: str = "Summary by CodeRabbit" diff --git a/release_notes_generator/utils/utils.py b/release_notes_generator/utils/utils.py index 179a7894..c3285346 100644 --- a/release_notes_generator/utils/utils.py +++ b/release_notes_generator/utils/utils.py @@ -21,7 +21,8 @@ import logging import re -from typing import Optional +from collections.abc import Iterable +from typing import Any, Optional from github.GitRelease import GitRelease from github.Repository import Repository @@ -29,6 +30,42 @@ logger = logging.getLogger(__name__) +def normalize_labels(raw: Any) -> list[str]: + """Normalize a raw labels definition into an ordered, de-duplicated list. + + Parameters: + raw: May be a str (newline/comma separated) or list[str]. Any other type returns []. + + Returns: + Ordered list preserving first-occurrence order; excludes empty tokens. + Invalid (non str/list) input returns an empty list. + """ + if isinstance(raw, list): + working: Iterable[Any] = raw + elif isinstance(raw, str): + parts: list[str] = [] + for line in raw.splitlines(): + for token in line.split(","): + parts.append(token) + working = parts + else: + return [] + + cleaned: list[str] = [] + seen: set[str] = set() + for item in working: + if not isinstance(item, str): + continue + token = item.strip() + if not token: + continue + if token in seen: + continue + seen.add(token) + cleaned.append(token) + return cleaned + + def get_change_url( tag_name: str, repository: Optional[Repository] = None, git_release: Optional[GitRelease] = None ) -> str: diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index ea8b2817..8ec9b3d0 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -35,8 +35,11 @@ from release_notes_generator.model.record.pull_request_record import PullRequestRecord from release_notes_generator.chapters.service_chapters import ServiceChapters from release_notes_generator.model.chapter import Chapter +from typing import Any + from release_notes_generator.chapters.custom_chapters import CustomChapters from release_notes_generator.model.record.sub_issue_record import SubIssueRecord +from release_notes_generator.utils.enums import DuplicityScopeEnum from release_notes_generator.utils.github_rate_limiter import GithubRateLimiter from release_notes_generator.utils.record_utils import get_id @@ -98,6 +101,38 @@ def custom_chapters_not_print_empty_chapters(): return chapters +def make_super_chapters_cc( + mocker: MockerFixture, + chapters_yaml: list[dict[str, Any]], + super_chapters_yaml: list[Any], + print_empty: bool = True, + hierarchy: bool = False, +) -> CustomChapters: + mocker.patch( + "release_notes_generator.chapters.custom_chapters.ActionInputs.get_super_chapters", + return_value=super_chapters_yaml, + ) + mocker.patch( + "release_notes_generator.chapters.custom_chapters.ActionInputs.get_hierarchy", + return_value=hierarchy, + ) + mocker.patch( + "release_notes_generator.chapters.custom_chapters.ActionInputs.get_verbose", + return_value=False, + ) + mocker.patch( + "release_notes_generator.chapters.custom_chapters.ActionInputs.get_duplicity_scope", + return_value=DuplicityScopeEnum.BOTH, + ) + mocker.patch( + "release_notes_generator.chapters.custom_chapters.ActionInputs.get_skip_release_notes_labels", + return_value=["skip-release-notes"], + ) + cc = CustomChapters(print_empty_chapters=print_empty) + cc.from_yaml_array(chapters_yaml) + return cc + + # Fixtures for Service Chapters @pytest.fixture def service_chapters(): @@ -1260,9 +1295,13 @@ def make_minimal_pr(mocker: MockerFixture, number: int) -> PullRequest: return pr -def make_closed_sub_issue_record_with_pr(mocker: MockerFixture, number: int) -> SubIssueRecord: +def make_closed_sub_issue_record_with_pr( + mocker: MockerFixture, number: int, issue_labels: list[str] | None = None +) -> SubIssueRecord: """Return a closed SubIssueRecord with one PR (no commits).""" - sub_record = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number)) + sub_record = SubIssueRecord( + make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number), issue_labels=issue_labels + ) sub_record.register_pull_request(make_minimal_pr(mocker, number=number + 1000)) return sub_record @@ -1289,9 +1328,13 @@ def make_open_sub_hierarchy_record_with_pr(mocker: MockerFixture, number: int) - return rec -def make_closed_sub_hierarchy_record_with_pr(mocker: MockerFixture, number: int) -> HierarchyIssueRecord: +def make_closed_sub_hierarchy_record_with_pr( + mocker: MockerFixture, number: int, issue_labels: list[str] | None = None +) -> HierarchyIssueRecord: """Return a closed HierarchyIssueRecord with one PR (no commits).""" - rec = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number)) + rec = HierarchyIssueRecord( + make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number), issue_labels=issue_labels + ) rec.register_pull_request(make_minimal_pr(mocker, number=number + 1000)) return rec diff --git a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py index ba731f98..44e4d337 100644 --- a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py @@ -15,13 +15,17 @@ # import pytest -from release_notes_generator.chapters.custom_chapters import CustomChapters, _normalize_labels +from github.Issue import Issue + +from release_notes_generator.chapters.custom_chapters import CustomChapters +from release_notes_generator.utils.utils import normalize_labels from release_notes_generator.model.record.issue_record import IssueRecord from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord from release_notes_generator.model.record.commit_record import CommitRecord from release_notes_generator.model.record.record import Record from release_notes_generator.utils.enums import DuplicityScopeEnum from release_notes_generator.action_inputs import ActionInputs +from tests.unit.conftest import make_super_chapters_cc @pytest.fixture @@ -301,7 +305,7 @@ def test_from_yaml_array_verbose_debug_branch(monkeypatch, caplog): ) def test_normalize_labels_edge_cases(raw, expected): # Arrange / Act - result = _normalize_labels(raw) + result = normalize_labels(raw) # Assert assert result == expected @@ -445,13 +449,6 @@ def test_populate_hidden_chapter_assigns_records(record_stub): def test_populate_hidden_chapter_no_duplicity_count(record_stub, mocker): - """ - Test that hidden chapters don't increment the duplicity counter. - - When a record is assigned to a hidden chapter, to_chapter_row should be called - with add_into_chapters=False to prevent incrementing the chapter presence count. - This ensures hidden chapters don't contribute to duplicity detection. - """ # Arrange cc = CustomChapters() cc.from_yaml_array([{"title": "Hidden", "labels": "bug", "hidden": True}]) @@ -468,13 +465,6 @@ def test_populate_hidden_chapter_no_duplicity_count(record_stub, mocker): def test_populate_visible_chapter_duplicity_count(record_stub, mocker): - """ - Test that visible (non-hidden) chapters increment the duplicity counter. - - When a record is assigned to a visible chapter, to_chapter_row should be called - with add_into_chapters=True to increment the chapter presence count. - This ensures visible chapters contribute to duplicity detection as expected. - """ # Arrange cc = CustomChapters() cc.from_yaml_array([{"title": "Visible", "labels": "bug", "hidden": False}]) @@ -533,12 +523,6 @@ def test_to_string_hidden_chapter_excluded(): def test_to_string_all_hidden_returns_empty(): - """ - Test that when all chapters are hidden, to_string returns empty string. - - This also verifies that hidden chapters are never shown even when - print_empty_chapters is True, since they are filtered out before rendering. - """ # Arrange cc = CustomChapters() cc.print_empty_chapters = True # Verify hidden chapters ignored even with this True @@ -844,42 +828,23 @@ def test_sorted_chapters_hidden_with_order(): assert sorted_chs[2].title == "Visible2" -# ------- Tests for catch-open-hierarchy (Feature 1) ------- - - @pytest.fixture -def hierarchy_record_stub(): +def hierarchy_record_stub(mocker): """Create a minimal HierarchyIssueRecord-like stub for catch-open-hierarchy tests.""" class HierarchyRecordStub(HierarchyIssueRecord): """Stub that avoids calling real GitHub Issue methods.""" - # noinspection PyMissingConstructor def __init__(self, rid, labels, state, contains_change): - # Bypass real __init__ to avoid GitHub API objects - Record.__init__(self, labels=labels, skip=False) + mock_issue = mocker.MagicMock(spec=Issue) + mock_issue.state = state + mock_issue.number = rid + mock_issue.type = None + mock_issue.user = None + mock_issue.assignees = [] + super().__init__(mock_issue, issue_labels=labels) self._rid = rid - self._state = state self._contains = contains_change - self._level = 0 - self._sub_issues = {} - self._sub_hierarchy_issues = {} - self._issue = None - self._pull_requests = {} - self._commits = {} - self._issue_type = None - - @property - def record_id(self): - return self._rid - - @property - def is_closed(self): - return self._state == "closed" - - @property - def is_open(self): - return self._state == "open" @property def author(self): @@ -893,7 +858,7 @@ def assignees(self): def developers(self): return [] - def to_chapter_row(self, add_into_chapters=True): + def to_chapter_row(self, add_into_chapters=True, label_filter=None, exclude_labels=None): return f"{self._rid} row" def contains_change_increment(self): @@ -902,6 +867,9 @@ def contains_change_increment(self): def get_labels(self): return self.labels + def has_unmatched_descendants(self, all_super_labels): + return False + def get_rls_notes(self, _line_marks=None): return "" @@ -1251,3 +1219,820 @@ def test_coh_chapter_not_populated_via_label_routing(hierarchy_record_stub, monk assert "org/repo#C1" in cc.chapters["New Features 🎉"].rows assert "org/repo#O1" in cc.chapters["Silent Live 🤫"].rows assert "org/repo#O1" not in cc.chapters["New Features 🎉"].rows + + +def test_super_chapters_no_super_chapters_renders_flat(mocker, record_stub): + """When no super chapters are defined, output is flat (### headings only).""" + # Arrange + cc = make_super_chapters_cc(mocker, [{"title": "Features", "label": "feature"}], []) + cc.populate({"org/repo#1": record_stub("org/repo#1", ["feature"])}) + # Act + output = cc.to_string() + # Assert + assert output.startswith("### Features") + for line in output.splitlines(): + assert not line.startswith("## ") + + +def test_super_chapters_records_grouped_under_super_chapter(mocker, record_stub): + """Records matching a super-chapter label appear nested under the super-chapter heading.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [ + {"title": "Enhancements", "label": "enhancement"}, + {"title": "Bugfixes", "label": "bug"}, + ], + [ + {"title": "Atum server", "labels": ["atum-server"]}, + {"title": "Atum agent", "labels": ["atum-agent"]}, + ], + ) + r1 = record_stub("org/repo#1", ["enhancement", "atum-server"]) + r2 = record_stub("org/repo#2", ["enhancement", "atum-agent"]) + r3 = record_stub("org/repo#3", ["bug", "atum-agent"]) + cc.populate({"org/repo#1": r1, "org/repo#2": r2, "org/repo#3": r3}) + # Act + output = cc.to_string() + # Assert + assert "## Atum server" in output + assert "## Atum agent" in output + server_section = output.split("## Atum agent")[0] + assert "### Enhancements" in server_section + assert "org/repo#1 row" in server_section + assert "org/repo#2 row" not in server_section + agent_section = output.split("## Atum agent")[1] + assert "### Enhancements" in agent_section + assert "org/repo#2 row" in agent_section + assert "### Bugfixes" in agent_section + assert "org/repo#3 row" in agent_section + + +def test_super_chapters_record_in_multiple_super_chapters(mocker, record_stub): + """A record with labels matching multiple super chapters appears in each.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [{"title": "Enhancements", "label": "enhancement"}], + [{"title": "Module A", "labels": ["mod-a"]}, {"title": "Module B", "labels": ["mod-b"]}], + ) + r1 = record_stub("org/repo#1", ["enhancement", "mod-a", "mod-b"]) + cc.populate({"org/repo#1": r1}) + # Act + output = cc.to_string() + # Assert + assert output.count("org/repo#1 row") == 2 + + +def test_super_chapters_empty_super_chapter_skipped_when_print_empty_false(mocker, record_stub): + """Super chapter with no matching records is omitted when print_empty_chapters=False.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [{"title": "Features", "label": "feature"}], + [{"title": "Module A", "labels": ["mod-a"]}, {"title": "Module B", "labels": ["mod-b"]}], + print_empty=False, + ) + r1 = record_stub("org/repo#1", ["feature", "mod-a"]) + cc.populate({"org/repo#1": r1}) + # Act + output = cc.to_string() + # Assert + assert "## Module A" in output + assert "## Module B" not in output + + +def test_super_chapters_parse_logs_and_skips_invalid(mocker): + """Only valid super-chapter entries produce headings; validation now runs in ActionInputs.""" + # Arrange - mock returns only the one valid entry (as ActionInputs would after validation) + cc = make_super_chapters_cc( + mocker, + [{"title": "Features", "label": "feature"}], + [{"title": "Valid", "labels": ["ok"]}], + ) + # Act + output = cc.to_string() + # Assert - only the "Valid" super chapter produces a heading + assert "## Valid" in output + assert sum(1 for line in output.splitlines() if line.startswith("## ") and not line.startswith("### ")) == 1 + + +def test_super_chapters_uncategorized_fallback(mocker, record_stub): + """Records without any super-chapter label appear under '## Uncategorized'.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [ + {"title": "Enhancements", "label": "enhancement"}, + {"title": "Bugfixes", "label": "bug"}, + ], + [{"title": "Module A", "labels": ["mod-a"]}], + ) + r1 = record_stub("org/repo#1", ["enhancement", "mod-a"]) + r2 = record_stub("org/repo#2", ["bug"]) # no super-chapter label + cc.populate({"org/repo#1": r1, "org/repo#2": r2}) + # Act + output = cc.to_string() + # Assert + assert "## Module A" in output + assert "org/repo#1 row" in output + assert "## Uncategorized" in output + uncategorized_section = output.split("## Uncategorized")[1] + assert "org/repo#2 row" in uncategorized_section + assert "org/repo#1 row" not in uncategorized_section + + +def test_super_chapters_no_uncategorized_when_all_claimed(mocker, record_stub): + """No '## Uncategorized' section when all records match a super chapter.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [{"title": "Enhancements", "label": "enhancement"}], + [{"title": "Module A", "labels": ["mod-a"]}], + ) + r1 = record_stub("org/repo#1", ["enhancement", "mod-a"]) + cc.populate({"org/repo#1": r1}) + # Act + output = cc.to_string() + # Assert + assert "## Module A" in output + assert "Uncategorized" not in output + + +def test_super_chapters_coh_record_visible_in_fallback(mocker, hierarchy_record_stub): + """COH-routed label-less record appears in Uncategorized when super chapters are active.""" + # Arrange + cc = make_super_chapters_cc( + mocker, + [ + {"title": "Features", "labels": "feature"}, + {"title": "Silent Live", "catch-open-hierarchy": True}, + ], + [{"title": "Module A", "labels": ["mod-a"]}], + hierarchy=True, + ) + record = hierarchy_record_stub("org/repo#H1", [], state="open") + cc.populate({"org/repo#H1": record}) + # Act + output = cc.to_string() + # Assert - COH record appears in Uncategorized since it has no super-chapter label + assert "## Uncategorized" in output + assert "org/repo#H1 row" in output + + +@pytest.fixture +def hierarchy_with_sub_issues_stub(mocker): + """Create a HierarchyIssueRecord-like stub that renders sub-issues filtered by label_filter.""" + + class SubIssueStub: + def __init__(self, rid, labels): + self.rid = rid + self._labels = labels + + @property + def labels(self): + return self._labels + + class HierarchyWithSubsStub(HierarchyIssueRecord): + """Stub with sub-issue-aware to_chapter_row that respects label_filter.""" + + def __init__(self, rid, labels, sub_issue_stubs, state="closed"): + mock_issue = mocker.MagicMock(spec=Issue) + mock_issue.state = state + mock_issue.number = rid + mock_issue.type = None + mock_issue.user = None + mock_issue.assignees = [] + super().__init__(mock_issue, issue_labels=labels) + self._rid = rid + self._sub_stubs: list[SubIssueStub] = sub_issue_stubs + + @property + def author(self): + return "author" + + @property + def assignees(self): + return [] + + @property + def developers(self): + return [] + + def to_chapter_row(self, add_into_chapters=True, label_filter=None, exclude_labels=None): + row = f"{self._rid} row" + for stub in self._sub_stubs: + if label_filter and not any(lbl in label_filter for lbl in stub.labels): + continue + if exclude_labels and any(lbl in exclude_labels for lbl in stub.labels): + continue + row += f"\n {stub.rid} sub-row" + return row + + def contains_change_increment(self): + return True + + def get_labels(self): + all_labels = list(self.labels) + for stub in self._sub_stubs: + all_labels.extend(stub.labels) + return all_labels + + def has_matching_labels(self, label_filter): + return any(lbl in label_filter for lbl in self.get_labels()) + + def has_unmatched_descendants(self, all_super_labels): + if not self._sub_stubs: + return False + for stub in self._sub_stubs: + if not any(lbl in all_super_labels for lbl in stub.labels): + return True + return False + + def get_rls_notes(self, _line_marks=None): + return "" + + def _make(rid, labels, sub_issue_stubs, state="closed"): + return HierarchyWithSubsStub(rid, labels, sub_issue_stubs, state) + + _make.SubIssueStub = SubIssueStub + return _make + + +def test_super_chapters_hierarchy_sub_issues_filtered_by_service(mocker, hierarchy_with_sub_issues_stub): + """Epic with frontend + backend tasks renders only relevant sub-issues per super chapter.""" + # Arrange + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "frontend"]), + SI("org/repo#3", ["enhancement", "backend"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [ + {"title": "Frontend", "labels": ["frontend"]}, + {"title": "Backend", "labels": ["backend"]}, + ], + ) + cc.populate({"org/repo#1": epic}) + # Act + output = cc.to_string() + # Assert structure + assert "## Frontend" in output + assert "## Backend" in output + fe_section = output.split("## Backend")[0] + be_section = output.split("## Backend")[1] + # Frontend section has only sub-issue #2 + assert "org/repo#2 sub-row" in fe_section + assert "org/repo#3 sub-row" not in fe_section + # Backend section has only sub-issue #3 + assert "org/repo#3 sub-row" in be_section + assert "org/repo#2 sub-row" not in be_section + + +def test_super_chapters_hierarchy_no_matching_descendants_hides_epic(mocker, hierarchy_with_sub_issues_stub): + """Epic is hidden from a super chapter if none of its descendants match.""" + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[SI("org/repo#2", ["enhancement", "frontend"])], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [ + {"title": "Frontend", "labels": ["frontend"]}, + {"title": "Backend", "labels": ["backend"]}, + ], + print_empty=False, + ) + cc.populate({"org/repo#1": epic}) + # Act + output = cc.to_string() + # Assert + assert "## Frontend" in output + assert "## Backend" not in output, "Backend should be skipped when no matching descendants" + + +def test_sc_combo_c1_all_descendants_match_one_sc(mocker, hierarchy_with_sub_issues_stub): + """When every descendant matches a single super chapter, epic appears only there, not in Uncategorized.""" + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "scope:security"]), + SI("org/repo#3", ["enhancement", "scope:security"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + assert "## Security" in output + assert "org/repo#1 row" in output + assert "org/repo#2 sub-row" in output + assert "org/repo#3 sub-row" in output + assert "Uncategorized" not in output + + +def test_sc_combo_c2_no_descendants_match_any_sc(mocker, hierarchy_with_sub_issues_stub): + """When no descendant matches any super chapter, epic appears only in Uncategorized.""" + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "frontend"]), + SI("org/repo#3", ["enhancement", "backend"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + assert "## Security" in output # header present (empty or not) + security_section = output.split("## Security")[1].split("\n## ")[0] if "## Security" in output else "" + assert "org/repo#1 row" not in security_section + assert "Uncategorized" in output + uncat_section = output.split("Uncategorized")[1] + assert "org/repo#1 row" in uncat_section + assert "org/repo#2 sub-row" in uncat_section + assert "org/repo#3 sub-row" in uncat_section + + +def test_sc_combo_c3_split_descendants_between_sc_and_uncategorized(mocker, hierarchy_with_sub_issues_stub): + """Epic with security-labeled and non-security-labeled descendants appears in both SC and Uncategorized. + + This is the user-reported scenario: + Epic (labels: epic, scope:security) + Task A (labels: enhancement, scope:security) → Security SC + Task B (labels: enhancement) → Uncategorized + """ + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "scope:security"]), + SI("org/repo#3", ["enhancement"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + # Security SC contains epic with only the security sub-issue + assert "## Security" in output + security_section = output.split("## Security")[1].split("\n## ")[0] + assert "org/repo#1 row" in security_section + assert "org/repo#2 sub-row" in security_section + assert "org/repo#3 sub-row" not in security_section + + # Uncategorized contains the same epic with only the non-security sub-issue + assert "Uncategorized" in output + uncat_section = output.split("Uncategorized")[1] + assert "org/repo#1 row" in uncat_section + assert "org/repo#3 sub-row" in uncat_section + assert "org/repo#2 sub-row" not in uncat_section + + +def test_sc_combo_c3b_three_level_hierarchy_split(mocker, hierarchy_with_sub_issues_stub): + """Three-level scenario: Epic → Feature → Tasks; Feature has mixed SC/non-SC descendants. + + Epic (labels: epic, enhancement) + Feature (labels: security) — aggregates scope:security from Task2 + Task1 (enhancement) → Uncategorized + Task2 (enhancement, scope:security) → Security SC + + Expected: + ## Security → Epic row + Feature sub-row containing Task2 + ## Uncategorized → Epic row + Feature sub-row containing Task1 + """ + SI = hierarchy_with_sub_issues_stub.SubIssueStub + + task1_si = SI("org/repo#3", ["enhancement"]) + task2_si = SI("org/repo#4", ["enhancement", "scope:security"]) + + # Feature stub with two sub-issues + feature = hierarchy_with_sub_issues_stub( + rid="org/repo#2", + labels=["security"], + sub_issue_stubs=[task1_si, task2_si], + ) + + # Epic stub with feature as a sub-hierarchy + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[SI("org/repo#2", ["security", "scope:security", "enhancement"])], + ) + # Override to_chapter_row and helper methods so Feature renders correctly + def epic_to_chapter_row(add_into_chapters=True, label_filter=None, exclude_labels=None): + row = f"org/repo#1 row" + sub_row = feature.to_chapter_row( + add_into_chapters=add_into_chapters, label_filter=label_filter, exclude_labels=exclude_labels + ) + if sub_row: + row += f"\n {sub_row}" + return row + + epic.to_chapter_row = epic_to_chapter_row + + def epic_has_matching_labels(label_filter): + if any(lbl in label_filter for lbl in ["epic", "enhancement"]): + return True + return feature.has_matching_labels(label_filter) + + def epic_has_unmatched_descendants(all_super_labels): + return feature.has_unmatched_descendants(all_super_labels) + + def epic_get_labels(): + return ["epic", "enhancement"] + feature.get_labels() + + epic.has_matching_labels = epic_has_matching_labels + epic.has_unmatched_descendants = epic_has_unmatched_descendants + epic.get_labels = epic_get_labels + + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + # Security SC: shows epic with feature fragment containing only task2 + assert "## Security" in output + sec_section = output.split("## Security")[1].split("\n## ")[0] + assert "org/repo#1 row" in sec_section + assert "org/repo#4 sub-row" in sec_section + assert "org/repo#3 sub-row" not in sec_section + + # Uncategorized: same epic with feature fragment containing only task1 + assert "Uncategorized" in output + uncat_section = output.split("Uncategorized")[1] + assert "org/repo#1 row" in uncat_section + assert "org/repo#3 sub-row" in uncat_section + assert "org/repo#4 sub-row" not in uncat_section + + +def test_sc_combo_c4_descendants_match_multiple_scs(mocker, hierarchy_with_sub_issues_stub): + """Epic with descendants matching different super chapters appears in each, filtered appropriately.""" + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "frontend"]), + SI("org/repo#3", ["enhancement", "backend"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [ + {"title": "Frontend", "labels": ["frontend"]}, + {"title": "Backend", "labels": ["backend"]}, + ], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + # Epic appears in both super chapters + assert "## Frontend" in output + assert "## Backend" in output + fe_section = output.split("## Backend")[0] + be_section = output.split("## Backend")[1] + assert "org/repo#2 sub-row" in fe_section + assert "org/repo#3 sub-row" not in fe_section + assert "org/repo#3 sub-row" in be_section + assert "org/repo#2 sub-row" not in be_section + # Every descendant is covered → no Uncategorized + assert "Uncategorized" not in output + + +def test_sc_combo_c5_multi_sc_plus_uncategorized(mocker, hierarchy_with_sub_issues_stub): + """Epic with descendants for two SCs plus an unmatched descendant appears in all three sections.""" + SI = hierarchy_with_sub_issues_stub.SubIssueStub + epic = hierarchy_with_sub_issues_stub( + rid="org/repo#1", + labels=["epic", "enhancement"], + sub_issue_stubs=[ + SI("org/repo#2", ["enhancement", "frontend"]), + SI("org/repo#3", ["enhancement", "backend"]), + SI("org/repo#4", ["enhancement"]), + ], + ) + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [ + {"title": "Frontend", "labels": ["frontend"]}, + {"title": "Backend", "labels": ["backend"]}, + ], + ) + cc.populate({"org/repo#1": epic}) + output = cc.to_string() + + # Frontend + assert "## Frontend" in output + fe_section = output.split("## Backend")[0] + assert "org/repo#2 sub-row" in fe_section + assert "org/repo#3 sub-row" not in fe_section + assert "org/repo#4 sub-row" not in fe_section + + # Backend + assert "## Backend" in output + # Backend section is between "## Backend" and next "\n## " or end + be_section = output.split("## Backend")[1].split("\n## ")[0] + assert "org/repo#3 sub-row" in be_section + assert "org/repo#2 sub-row" not in be_section + assert "org/repo#4 sub-row" not in be_section + + # Uncategorized + assert "Uncategorized" in output + uncat_section = output.split("Uncategorized")[1] + assert "org/repo#4 sub-row" in uncat_section + assert "org/repo#2 sub-row" not in uncat_section + assert "org/repo#3 sub-row" not in uncat_section + + +def test_sc_combo_c6_plain_record_routing_unchanged(mocker, record_stub): + """Non-hierarchy records follow standard label-based super-chapter routing (no split logic).""" + cc = make_super_chapters_cc( + mocker, + [{"title": "New Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + r1 = record_stub("org/repo#10", ["enhancement", "scope:security"]) + r2 = record_stub("org/repo#11", ["enhancement"]) + cc.populate({"org/repo#10": r1, "org/repo#11": r2}) + output = cc.to_string() + + # r1 goes to Security SC + assert "## Security" in output + security_section = output.split("## Security")[1].split("\n## ")[0] + assert "org/repo#10 row" in security_section + + # r2 goes to Uncategorized + assert "Uncategorized" in output + uncat_section = output.split("Uncategorized")[1] + assert "org/repo#11 row" in uncat_section + + +def test_from_yaml_array_coh_chapter_with_explicit_null_labels(): + """catch-open-hierarchy chapter with 'labels: null' is accepted with an empty label list.""" + cc = CustomChapters() + cc.from_yaml_array([{"title": "Open Issues", "catch-open-hierarchy": True, "labels": None}]) + assert "Open Issues" in cc.chapters + assert cc.chapters["Open Issues"].catch_open_hierarchy is True + assert cc.chapters["Open Issues"].labels == [] + + +def test_from_yaml_array_chapter_order_conflict_warns(caplog): + """Same chapter title defined twice with different orders logs a warning and keeps the first.""" + caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") + cc = CustomChapters() + cc.from_yaml_array( + [ + {"title": "Changes", "labels": "bug", "order": 1}, + {"title": "Changes", "labels": "enhancement", "order": 2}, + ] + ) + assert "Changes" in cc.chapters + assert cc.chapters["Changes"].order == 1 + assert any("conflicting order" in r.message for r in caplog.records) + + +def test_from_yaml_array_repeated_chapter_with_hidden_logs_info(caplog): + """Repeated chapter definition with hidden=True logs an info message about hidden status.""" + caplog.set_level("INFO", logger="release_notes_generator.chapters.custom_chapters") + cc = CustomChapters() + cc.from_yaml_array( + [ + {"title": "Changes", "labels": "bug"}, + {"title": "Changes", "labels": "enhancement", "hidden": True}, + ] + ) + assert "Changes" in cc.chapters + assert any("marked as hidden" in r.message for r in caplog.records) + + +def test_super_chapters_hidden_chapter_skipped_in_sc_and_uncat_loops(mocker, record_stub): + """Hidden chapters are skipped during both SC-block and Uncategorized-block rendering. + + When the only unclaimed record lives in a hidden chapter, visible chapters produce + no matching rows for Uncategorized, so ## Uncategorized must not be emitted. + """ + cc = make_super_chapters_cc( + mocker, + [ + {"title": "SC-Chapter", "labels": "sc-label"}, + {"title": "Hidden-Chapter", "labels": "other-label", "hidden": True}, + ], + [{"title": "Security", "labels": ["sc-label"]}], + print_empty=True, + ) + r_sc = record_stub("org/repo#1", ["sc-label"]) + r_plain = record_stub("org/repo#2", ["other-label"]) # assigned only to hidden chapter + cc.populate({"org/repo#1": r_sc, "org/repo#2": r_plain}) + + output = cc.to_string() + + # SC section renders with visible chapter (hidden chapter is skipped in SC loop) + assert "## Security" in output + assert "org/repo#1 row" in output + # r_plain is unclaimed but lives only in a hidden chapter; the visible chapter has no + # matching uncategorized rows → uc_block is empty → ## Uncategorized must NOT appear + assert "Uncategorized" not in output + assert "org/repo#2 row" not in output + + +def test_super_chapters_empty_sc_block_printed_when_all_chapters_hidden(mocker, record_stub): + """When all chapters are hidden, a super chapter with no visible content renders 'No entries detected.'""" + cc = make_super_chapters_cc( + mocker, + [{"title": "Hidden-Only", "labels": "bug", "hidden": True}], + [{"title": "Security", "labels": ["sc-label"]}], + print_empty=True, + ) + r1 = record_stub("org/repo#1", ["bug", "sc-label"]) + cc.populate({"org/repo#1": r1}) + + output = cc.to_string() + + assert "## Security" in output + assert "No entries detected." in output + + +def test_render_chapter_for_ids_hierarchy_record_no_matching_labels_skipped(mocker, record_stub): + """HierarchyIssueRecord whose has_matching_labels returns False is skipped in SC rendering.""" + cc = make_super_chapters_cc( + mocker, + [{"title": "Changes", "labels": "sc-label"}], + [{"title": "Security", "labels": ["sc-label"]}], + print_empty=False, + ) + mock_hi = mocker.create_autospec(HierarchyIssueRecord, instance=True) + mock_hi.labels = ["sc-label"] + mock_hi.get_labels.return_value = ["sc-label"] + mock_hi.contains_change_increment.return_value = True + mock_hi.skip = False + mock_hi.is_open = True + mock_hi.has_matching_labels.return_value = False + mock_hi.to_chapter_row.return_value = "hi_row" + cc.populate({"org/repo#1": mock_hi}) + + output = cc.to_string() + + # The record must NOT appear in Security section because has_matching_labels returned False + security_section = output.split("## Security")[1].split("## ")[0] if "## Security" in output else "" + assert "hi_row" not in security_section + + +# --- _enforce_coh_constraint via from_yaml_array --- + + +def test_enforce_coh_constraint_no_coh_passes_through(caplog): + """Chapter with no catch-open-hierarchy is registered normally (pass-through path).""" + caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") + cc = CustomChapters() + cc.from_yaml_array([{"title": "Features", "labels": "feature"}]) + assert "Features" in cc.chapters + assert cc.chapters["Features"].catch_open_hierarchy is False + assert not caplog.records + + +def test_enforce_coh_constraint_first_coh_chapter_accepted(): + """First chapter with catch-open-hierarchy is accepted without warning.""" + cc = CustomChapters() + cc.from_yaml_array([{"title": "Open Issues", "catch-open-hierarchy": True}]) + assert "Open Issues" in cc.chapters + assert cc.chapters["Open Issues"].catch_open_hierarchy is True + + +def test_enforce_coh_constraint_duplicate_different_title_rejected_with_warning(caplog): + """Second distinct title with catch-open-hierarchy is demoted and a warning is emitted.""" + caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") + cc = CustomChapters() + cc.from_yaml_array( + [ + {"title": "First", "catch-open-hierarchy": True}, + {"title": "Second", "catch-open-hierarchy": True, "labels": "bug"}, + ] + ) + assert cc.chapters["First"].catch_open_hierarchy is True + assert cc.chapters["Second"].catch_open_hierarchy is False + assert any("ignoring duplicate" in r.message.lower() for r in caplog.records) + + +def test_enforce_coh_constraint_same_title_repeat_merges_silently(caplog): + """Repeating catch-open-hierarchy on the same title merges labels without warning.""" + caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") + cc = CustomChapters() + cc.from_yaml_array( + [ + {"title": "Open Issues", "catch-open-hierarchy": True, "labels": "epic"}, + {"title": "Open Issues", "catch-open-hierarchy": True, "labels": "feature"}, + ] + ) + assert cc.chapters["Open Issues"].catch_open_hierarchy is True + assert cc.chapters["Open Issues"].labels == ["epic", "feature"] + assert not any("ignoring duplicate" in r.message.lower() for r in caplog.records) + + +# --- _collect_uncategorized_ids via to_string with super chapters --- + + +def test_collect_uncategorized_ids_claimed_hierarchy_with_all_matched_not_in_uncat(mocker, record_stub): + """A hierarchy record whose every descendant is SC-matched must not appear in Uncategorized.""" + cc = make_super_chapters_cc( + mocker, + [{"title": "Features", "label": "enhancement"}], + [{"title": "Security", "labels": ["scope:security"]}], + ) + r1 = record_stub("org/repo#1", ["enhancement", "scope:security"]) + cc.populate({"org/repo#1": r1}) + output = cc.to_string() + assert "Uncategorized" not in output, f"Claimed record must not appear in Uncategorized; got:\n{output}" + + +def test_collect_uncategorized_ids_no_rows_produces_no_uncat_section(mocker): + """When all chapters are empty, no Uncategorized section is rendered.""" + cc = make_super_chapters_cc( + mocker, + [{"title": "Features", "label": "feature"}], + [{"title": "Security", "labels": ["scope:security"]}], + print_empty=False, + ) + output = cc.to_string() + assert "Uncategorized" not in output + + +def test_uncategorized_no_empty_sub_chapters_when_print_empty_true(mocker, record_stub): + """print-empty-chapters=True must not produce empty ### sub-chapters inside ## Uncategorized.""" + cc = make_super_chapters_cc( + mocker, + [ + {"title": "Bugs", "label": "bug"}, + {"title": "Features", "label": "feature"}, + ], + [{"title": "Security", "labels": ["scope:security"]}], + print_empty=True, + ) + # Only a 'bug' record with no SC label → goes to Uncategorized under Bugs only + r1 = record_stub("org/repo#1", ["bug"]) + cc.populate({"org/repo#1": r1}) + output = cc.to_string() + + assert "## Uncategorized" in output + uncat_section = output.split("## Uncategorized")[1] + assert "org/repo#1 row" in uncat_section + # Features chapter has no matching records → must NOT appear inside Uncategorized + assert "Features" not in uncat_section, ( + "Empty sub-chapter 'Features' must not appear in Uncategorized regardless of print-empty-chapters setting" + ) + + +def test_uncategorized_not_emitted_when_only_visible_unclaimed_records_comes_from_empty_chapters( + mocker, record_stub +): + """## Uncategorized is not emitted when visible chapters have no matching unclaimed rows. + + Distinct from the hidden-chapter case: here a visible chapter has a record but that + record is SC-claimed, so the filtered rows for Uncategorized are empty for every + visible chapter → no ## Uncategorized should appear. + """ + cc = make_super_chapters_cc( + mocker, + [{"title": "Features", "label": "feature"}], + [{"title": "Security", "labels": ["feature"]}], + print_empty=True, + ) + # r1 matches feature AND the SC label → it is claimed; Uncategorized would be empty + r1 = record_stub("org/repo#1", ["feature"]) + cc.populate({"org/repo#1": r1}) + output = cc.to_string() + + assert "Uncategorized" not in output, ( + "## Uncategorized must not appear when all records are SC-claimed" + ) diff --git a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py index a8fb515f..5f1c7f26 100644 --- a/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py +++ b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py @@ -19,6 +19,7 @@ from release_notes_generator.model.record.hierarchy_issue_record import HierarchyIssueRecord from release_notes_generator.model.record.issue_record import IssueRecord +from release_notes_generator.model.record.sub_issue_record import SubIssueRecord from tests.unit.conftest import ( make_closed_sub_hierarchy_record_no_pr, make_closed_sub_hierarchy_record_with_pr, @@ -87,6 +88,52 @@ def test_to_chapter_row_closed_parent_highlights_open_sub_hierarchy_issue(mocker assert "🟡" not in closed_line, f"Closed sub-hierarchy line must NOT contain icon; got: {closed_line!r}" +def test_to_chapter_row_open_sub_hierarchy_icon_placed_after_list_marker(mocker, patch_hierarchy_action_inputs): + """Icon must appear after the '- ' list marker, not before it. + + Correct: ' - 🟡 _title_ #360' + Wrong: ' 🟡 - _title_ #360' + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + record = HierarchyIssueRecord(parent) + child = make_open_sub_hierarchy_record_with_pr(mocker, number=360) + child.level = 1 + record.sub_hierarchy_issues["360"] = child + + row = record.to_chapter_row() + + open_line = next((l for l in row.splitlines() if "#360" in l), None) + assert open_line is not None + stripped = open_line.lstrip() + assert stripped.startswith("- 🟡 "), ( + f"Expected '- 🟡 ' after leading spaces; got: {open_line!r}" + ) + + +def test_to_chapter_row_empty_icon_leaves_no_stray_space(mocker, patch_hierarchy_action_inputs): + """When open-hierarchy-sub-issue-icon is '', the row indentation is unchanged (no stray space).""" + mocker.patch( + "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_open_hierarchy_sub_issue_icon", + return_value="", + ) + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + record = HierarchyIssueRecord(parent) + child = make_open_sub_hierarchy_record_with_pr(mocker, number=360) + child.level = 1 + record.sub_hierarchy_issues["360"] = child + + row = record.to_chapter_row() + + open_line = next((l for l in row.splitlines() if "#360" in l), None) + assert open_line is not None + assert open_line.startswith(" - "), ( + f"Level-1 row must start with exactly ' - ' (2 spaces); got: {open_line!r}" + ) + assert not open_line.startswith(" "), ( + f"No stray extra space in indentation with empty icon; got: {open_line!r}" + ) + + def test_to_chapter_row_open_parent_only_renders_closed_sub_issues_with_change_increment(mocker, patch_hierarchy_action_inputs): """ Open parent with one closed sub-issue (has PR) and one open sub-issue (no PR) → @@ -452,3 +499,516 @@ def test_contains_change_increment_true_nested_with_closed_leaf(mocker, make_hie root.sub_hierarchy_issues.update({"org/repo#71": child}) assert root.contains_change_increment() is True + + +# --- has_matching_labels --- + + +def test_has_matching_labels_own_label_matches(make_hierarchy_issue): + """Parent's own labels trigger a match.""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + issue.get_labels.return_value = [type("L", (), {"name": "frontend"})()] + record = HierarchyIssueRecord(issue, issue_labels=["frontend"]) + + assert record.has_matching_labels(["frontend"]) is True + + +def test_has_matching_labels_sub_issue_label_matches(mocker, make_hierarchy_issue): + """A sub-issue label satisfies the filter even when the parent has no matching labels.""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue, issue_labels=["epic"]) + + sub_issue = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=200) + sub_record = SubIssueRecord(sub_issue, issue_labels=["frontend"]) + record.sub_issues["200"] = sub_record + + assert record.has_matching_labels(["frontend"]) is True + assert record.has_matching_labels(["backend"]) is False + + +def test_has_matching_labels_recursive_through_sub_hierarchy(mocker, make_hierarchy_issue): + """Label matching recurses through nested HierarchyIssueRecords.""" + root_issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + root = HierarchyIssueRecord(root_issue, issue_labels=["epic"]) + + child_issue = make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_OPEN) + child = HierarchyIssueRecord(child_issue, issue_labels=["sub-epic"]) + + leaf_issue = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + leaf = SubIssueRecord(leaf_issue, issue_labels=["backend"]) + child.sub_issues["300"] = leaf + + root.sub_hierarchy_issues["200"] = child + + assert root.has_matching_labels(["backend"]) is True + assert root.has_matching_labels(["frontend"]) is False + + +def test_has_matching_labels_no_match_returns_false(make_hierarchy_issue): + """Returns False when no labels match anywhere in the tree.""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue, issue_labels=["epic"]) + + assert record.has_matching_labels(["frontend"]) is False + + +def test_has_matching_labels_pr_label_matches(mocker, make_hierarchy_issue): + """A label on an attached PR satisfies the filter (aligns with get_labels behaviour).""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_CLOSED) + record = HierarchyIssueRecord(issue, issue_labels=["epic"]) + + pr = make_minimal_pr(mocker, number=1100) + pr.get_labels.return_value = [type("L", (), {"name": "frontend"})()] + record.register_pull_request(pr) + + assert record.has_matching_labels(["frontend"]) is True + assert record.has_matching_labels(["backend"]) is False + + +# --- to_chapter_row with label_filter --- + + +def test_to_chapter_row_label_filter_includes_matching_sub_issues(mocker, patch_hierarchy_action_inputs): + """Only sub-issues whose labels intersect label_filter appear in the output.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + fe_sub = make_closed_sub_issue_record_with_pr(mocker, number=2, issue_labels=["enhancement", "frontend"]) + be_sub = make_closed_sub_issue_record_with_pr(mocker, number=3, issue_labels=["enhancement", "backend"]) + record.sub_issues["2"] = fe_sub + record.sub_issues["3"] = be_sub + + row = record.to_chapter_row(label_filter=["frontend"]) + + assert "#2" in row, f"Frontend sub-issue #2 should appear; got:\n{row}" + assert "#3" not in row, f"Backend sub-issue #3 should NOT appear; got:\n{row}" + + +def test_to_chapter_row_label_filter_includes_matching_sub_hierarchy(mocker, patch_hierarchy_action_inputs): + """Sub-hierarchy issues are filtered by label_filter recursively.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + fe_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=10, issue_labels=["frontend"]) + be_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=20, issue_labels=["backend"]) + record.sub_hierarchy_issues["10"] = fe_child + record.sub_hierarchy_issues["20"] = be_child + + row = record.to_chapter_row(label_filter=["frontend"]) + + assert "#10" in row, f"Frontend sub-hierarchy #10 should appear; got:\n{row}" + assert "#20" not in row, f"Backend sub-hierarchy #20 should NOT appear; got:\n{row}" + + +def test_to_chapter_row_no_label_filter_renders_all(mocker, patch_hierarchy_action_inputs): + """Without label_filter, all sub-issues render (existing behaviour preserved).""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + fe_sub = make_closed_sub_issue_record_with_pr(mocker, number=2, issue_labels=["frontend"]) + be_sub = make_closed_sub_issue_record_with_pr(mocker, number=3, issue_labels=["backend"]) + record.sub_issues["2"] = fe_sub + record.sub_issues["3"] = be_sub + + row = record.to_chapter_row() + + assert "#2" in row + assert "#3" in row + + +def test_has_unmatched_descendants_true_when_sub_issue_has_no_matching_label(mocker, make_hierarchy_issue): + """Returns True when at least one sub-issue does not match the label set.""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue, issue_labels=["epic"]) + + matched = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, 200), issue_labels=["security"]) + unmatched = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, 201), issue_labels=["other"]) + record.sub_issues["200"] = matched + record.sub_issues["201"] = unmatched + + assert record.has_unmatched_descendants(["security"]) is True + + +def test_has_unmatched_descendants_false_when_all_match(mocker, make_hierarchy_issue): + """Returns False when every descendant matches the label set.""" + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue, issue_labels=["epic"]) + + s1 = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, 200), issue_labels=["security"]) + s2 = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, 201), issue_labels=["security", "bug"]) + record.sub_issues["200"] = s1 + record.sub_issues["201"] = s2 + + assert record.has_unmatched_descendants(["security"]) is False + + +def test_has_unmatched_descendants_recursive(mocker, make_hierarchy_issue): + """Recurses through sub-hierarchy-issues to find unmatched descendants.""" + root = HierarchyIssueRecord(make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN), issue_labels=["epic"]) + child = HierarchyIssueRecord(make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_OPEN), issue_labels=["sub-epic"]) + leaf = SubIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, 300), issue_labels=["other"]) + child.sub_issues["300"] = leaf + root.sub_hierarchy_issues["200"] = child + + assert root.has_unmatched_descendants(["security"]) is True + + +def test_has_unmatched_descendants_empty_tree(make_hierarchy_issue): + """A leaf hierarchy issue with no descendants returns False (nothing to be unmatched).""" + record = HierarchyIssueRecord(make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN)) + + assert record.has_unmatched_descendants(["security"]) is False + + +def test_to_chapter_row_exclude_labels_hides_matching_sub_issues(mocker, patch_hierarchy_action_inputs): + """Sub-issues whose labels intersect exclude_labels are hidden from the output.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + sec_sub = make_closed_sub_issue_record_with_pr(mocker, number=2, issue_labels=["enhancement", "scope:security"]) + plain_sub = make_closed_sub_issue_record_with_pr(mocker, number=3, issue_labels=["enhancement"]) + record.sub_issues["2"] = sec_sub + record.sub_issues["3"] = plain_sub + + row = record.to_chapter_row(exclude_labels=["scope:security"]) + + assert "#3" in row, f"Non-security sub-issue should appear; got:\n{row}" + assert "#2" not in row, f"Security sub-issue should be excluded; got:\n{row}" + + +def test_to_chapter_row_exclude_labels_hides_matching_sub_hierarchy(mocker, patch_hierarchy_action_inputs): + """Sub-hierarchy issue with only SC-matching labels is hidden by exclude_labels. + + A leaf sub-hierarchy node whose own labels all match exclude_labels and which + has no unmatched descendants is excluded from the Uncategorized render. + A sibling leaf whose labels do NOT match is kept. + """ + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + sec_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=10, issue_labels=["scope:security"]) + plain_child = make_closed_sub_hierarchy_record_with_pr(mocker, number=20, issue_labels=["other"]) + record.sub_hierarchy_issues["10"] = sec_child + record.sub_hierarchy_issues["20"] = plain_child + + row = record.to_chapter_row(exclude_labels=["scope:security"]) + + assert "#20" in row, f"Non-security sub-hierarchy should appear; got:\n{row}" + assert "#10" not in row, f"Security sub-hierarchy should be excluded; got:\n{row}" + + +def test_to_chapter_row_exclude_labels_none_renders_all(mocker, patch_hierarchy_action_inputs): + """Without exclude_labels, all sub-issues render (backward compatible).""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1) + record = HierarchyIssueRecord(parent) + + s1 = make_closed_sub_issue_record_with_pr(mocker, number=2, issue_labels=["scope:security"]) + s2 = make_closed_sub_issue_record_with_pr(mocker, number=3, issue_labels=["other"]) + record.sub_issues["2"] = s1 + record.sub_issues["3"] = s2 + + row = record.to_chapter_row() + + assert "#2" in row + assert "#3" in row + + +def test_to_chapter_row_exclude_labels_keeps_sub_hierarchy_with_mixed_descendants(mocker, patch_hierarchy_action_inputs): + """Sub-hierarchy issue with SOME matching descendants is kept (not hidden) by exclude_labels. + + Real-data scenario: + Epic (level 0) + Feature (level 1) — aggregates scope:security from Task2 + Task1 (no scope:security) → must appear in Uncategorized + Task2 (scope:security) → must be excluded from Uncategorized + + Bug: Feature's aggregated labels include scope:security so has_matching_labels returns True, + causing the entire Feature branch to be skipped — leaving only the Epic root line. + Fix: skip Feature only when ALL its descendants are SC-covered (has_unmatched_descendants=False). + """ + epic = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) + + feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=10), issue_labels=["security"]) + + task1 = make_closed_sub_issue_record_with_pr(mocker, number=20, issue_labels=["enhancement"]) + task2 = make_closed_sub_issue_record_with_pr(mocker, number=21, issue_labels=["enhancement", "scope:security"]) + + feature.sub_issues["20"] = task1 + feature.sub_issues["21"] = task2 + epic.sub_hierarchy_issues["10"] = feature + + row = epic.to_chapter_row(exclude_labels=["scope:security"]) + + assert "#10" in row, f"Feature should appear (has non-SC sub-issue Task1); got:\n{row}" + assert "#20" in row, f"Task1 (no SC label) should appear; got:\n{row}" + assert "#21" not in row, f"Task2 (scope:security) should be excluded; got:\n{row}" + + +def test_to_chapter_row_exclude_labels_hides_sub_hierarchy_when_all_descendants_match( + mocker, patch_hierarchy_action_inputs +): + """Sub-hierarchy issue is hidden when ALL its descendants match exclude_labels.""" + epic = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) + + feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=10), issue_labels=["security"]) + + task2 = make_closed_sub_issue_record_with_pr(mocker, number=21, issue_labels=["enhancement", "scope:security"]) + + feature.sub_issues["21"] = task2 + epic.sub_hierarchy_issues["10"] = feature + + row = epic.to_chapter_row(exclude_labels=["scope:security"]) + + assert "#10" not in row, f"Feature should be hidden (all descendants have SC label); got:\n{row}" + assert "#21" not in row, f"Task2 should be excluded; got:\n{row}" + + +def test_get_labels_aggregates_deep_sub_hierarchy_labels(mocker, make_hierarchy_issue): + """get_labels() must recursively aggregate labels from nested HierarchyIssueRecords. + + Bug: previous implementation used sub_hierarchy_issue.labels (own labels only), + so a label that exists only on a grandchild was invisible to the root Epic. + This caused the root Epic to be missing from claimed_ids and therefore never rendered + in the matching super chapter. + """ + root_issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_CLOSED) + root = HierarchyIssueRecord(root_issue, issue_labels=["epic"]) + + feature_issue = make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_CLOSED) + feature = HierarchyIssueRecord(feature_issue, issue_labels=["feature"]) # no "scope:security" + + leaf_issue = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + leaf = SubIssueRecord(leaf_issue, issue_labels=["scope:security"]) + feature.sub_issues["300"] = leaf + + root.sub_hierarchy_issues["200"] = feature + + result = root.get_labels() + + assert "scope:security" in result, ( + "Root Epic must aggregate 'scope:security' from grandchild leaf; got: " + str(result) + ) + + +def test_to_chapter_row_sub_issues_rendered_in_ascending_number_order(mocker, patch_hierarchy_action_inputs): + """Leaf sub-issues must be rendered sorted ascending by issue number regardless of insertion order.""" + parent = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) + + high = make_closed_sub_issue_record_with_pr(mocker, number=4150) + low = make_closed_sub_issue_record_with_pr(mocker, number=4149) + + # Intentionally insert higher number first + parent.sub_issues["4150"] = high + parent.sub_issues["4149"] = low + + row = parent.to_chapter_row() + + pos_low = row.index("#4149") + pos_high = row.index("#4150") + assert pos_low < pos_high, f"#4149 must appear before #4150; got:\n{row}" + + +def test_to_chapter_row_sub_hierarchy_issues_rendered_in_ascending_number_order( + mocker, patch_hierarchy_action_inputs +): + """Sub-hierarchy issues must be rendered sorted ascending by issue number regardless of insertion order.""" + parent = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1)) + + high = make_closed_sub_hierarchy_record_with_pr(mocker, number=200) + high.level = 1 + low = make_closed_sub_hierarchy_record_with_pr(mocker, number=100) + low.level = 1 + + # Intentionally insert higher number first + parent.sub_hierarchy_issues["200"] = high + parent.sub_hierarchy_issues["100"] = low + + row = parent.to_chapter_row() + + pos_low = row.index("#100") + pos_high = row.index("#200") + assert pos_low < pos_high, f"#100 must appear before #200; got:\n{row}" + + +def test_five_level_hierarchy_label_filter_and_exclude_labels(mocker, patch_hierarchy_action_inputs): + """All recursive methods work correctly at 5 levels of nesting. + + Tree: + L1: Epic #1 (labels: epic) + L2: Theme #2 (labels: theme) + L3: Feature #3 (labels: feature) + L4: Story #4 (labels: story) + L5a: Task #10 (labels: scope:security) ← SC match + L5b: Task #11 (labels: enhancement) ← no SC label + + Verified behaviours: + - get_labels() on L1 includes scope:security (aggregated from 4 levels down) + - has_matching_labels(["scope:security"]) on L1 is True + - has_unmatched_descendants(["scope:security"]) on L1 is True (Task #11 is unmatched) + - to_chapter_row(label_filter=["scope:security"]) shows only the SC path down to Task #10 + - to_chapter_row(exclude_labels=["scope:security"]) shows the non-SC path down to Task #11 + and omits Task #10 + """ + # Build bottom-up: Task leaves first + task_sc = make_closed_sub_issue_record_with_pr(mocker, number=10, issue_labels=["scope:security"]) + task_plain = make_closed_sub_issue_record_with_pr(mocker, number=11, issue_labels=["enhancement"]) + + story = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=4), issue_labels=["story"]) + story.level = 3 + story.sub_issues["10"] = task_sc + story.sub_issues["11"] = task_plain + + feature = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=3), issue_labels=["feature"]) + feature.level = 2 + feature.sub_hierarchy_issues["4"] = story + + theme = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=2), issue_labels=["theme"]) + theme.level = 1 + theme.sub_hierarchy_issues["3"] = feature + + epic = HierarchyIssueRecord(make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=1), issue_labels=["epic"]) + epic.sub_hierarchy_issues["2"] = theme + + # --- get_labels aggregates across all 5 levels --- + all_labels = epic.get_labels() + assert "scope:security" in all_labels, f"scope:security must bubble up to L1; got {all_labels}" + assert "enhancement" in all_labels, f"enhancement must bubble up to L1; got {all_labels}" + + # --- has_matching_labels --- + assert epic.has_matching_labels(["scope:security"]) is True + assert epic.has_matching_labels(["unknown"]) is False + + # --- has_unmatched_descendants --- + # Task #11 has no scope:security → True + assert epic.has_unmatched_descendants(["scope:security"]) is True + # Both leaf labels covered → False + assert epic.has_unmatched_descendants(["scope:security", "enhancement"]) is False + + # --- to_chapter_row with label_filter: only SC path visible --- + row_sc = epic.to_chapter_row(label_filter=["scope:security"]) + assert "#1" in row_sc, f"Epic root must appear; got:\n{row_sc}" + assert "#2" in row_sc, f"Theme must appear (contains SC path); got:\n{row_sc}" + assert "#3" in row_sc, f"Feature must appear (contains SC path); got:\n{row_sc}" + assert "#4" in row_sc, f"Story must appear (contains SC path); got:\n{row_sc}" + assert "#10" in row_sc, f"SC task must appear; got:\n{row_sc}" + assert "#11" not in row_sc, f"Plain task must be excluded; got:\n{row_sc}" + + # --- to_chapter_row with exclude_labels: only non-SC path visible --- + row_plain = epic.to_chapter_row(exclude_labels=["scope:security"]) + assert "#1" in row_plain, f"Epic root must appear; got:\n{row_plain}" + assert "#2" in row_plain, f"Theme must appear (has non-SC path); got:\n{row_plain}" + assert "#3" in row_plain, f"Feature must appear (has non-SC path); got:\n{row_plain}" + assert "#4" in row_plain, f"Story must appear (has non-SC path); got:\n{row_plain}" + assert "#11" in row_plain, f"Plain task must appear; got:\n{row_plain}" + assert "#10" not in row_plain, f"SC task must be excluded; got:\n{row_plain}" + + +def test_developers_returns_empty_list_when_issue_is_none(): + """developers returns [] when the underlying issue is None (defensive guard).""" + record = HierarchyIssueRecord(None) # type: ignore[arg-type] + assert record.developers == [] + + +def test_pull_requests_count_cross_repo_sub_issue(mocker, make_hierarchy_issue): + """A cross-repo sub-issue counts as 1 PR regardless of its own pull-request list.""" + parent = HierarchyIssueRecord(make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN)) + sub = SubIssueRecord(make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_CLOSED)) + sub.is_cross_repo = True + parent.sub_issues["org/repo#200"] = sub + + assert parent.pull_requests_count() == 1 + + +def test_pull_requests_count_cross_repo_sub_hierarchy_issue(mocker, make_hierarchy_issue): + """A cross-repo sub-hierarchy-issue counts as 1 PR.""" + parent = HierarchyIssueRecord(make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN)) + child = HierarchyIssueRecord(make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_CLOSED)) + child.is_cross_repo = True + parent.sub_hierarchy_issues["org/repo#200"] = child + + assert parent.pull_requests_count() == 1 + + +def test_get_labels_includes_pr_labels(mocker, make_hierarchy_issue, make_sub_issue): + """get_labels aggregates labels from attached pull requests.""" + issue = make_hierarchy_issue(10, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue) + + pr = mocker.Mock() + mock_label = mocker.Mock() + mock_label.name = "pr-label" + pr.get_labels.return_value = [mock_label] + record.register_pull_request(pr) + + labels = record.get_labels() + assert "pr-label" in labels + + +def test_has_unmatched_descendants_leaf_hierarchy_child_unmatched(mocker, make_hierarchy_issue): + """Leaf sub-hierarchy-issue whose own labels don't match the SC set returns True.""" + parent = HierarchyIssueRecord(make_hierarchy_issue(1, IssueRecord.ISSUE_STATE_OPEN)) + + leaf_issue = make_hierarchy_issue(2, IssueRecord.ISSUE_STATE_OPEN) + other_label = mocker.Mock() + other_label.name = "other-label" + leaf_issue.get_labels.return_value = [other_label] + leaf = HierarchyIssueRecord(leaf_issue) + parent.sub_hierarchy_issues["org/repo#2"] = leaf + + assert parent.has_unmatched_descendants(["sc-label"]) is True + + +def test_has_unmatched_descendants_leaf_hierarchy_child_matched(mocker, make_hierarchy_issue): + """Leaf sub-hierarchy-issue whose labels match the SC set returns False.""" + parent = HierarchyIssueRecord(make_hierarchy_issue(1, IssueRecord.ISSUE_STATE_OPEN)) + + leaf_issue = make_hierarchy_issue(2, IssueRecord.ISSUE_STATE_OPEN) + sc_label = mocker.Mock() + sc_label.name = "sc-label" + leaf_issue.get_labels.return_value = [sc_label] + leaf = HierarchyIssueRecord(leaf_issue) + parent.sub_hierarchy_issues["org/repo#2"] = leaf + + assert parent.has_unmatched_descendants(["sc-label"]) is False + + +# --- release notes block rendering --- + + +def test_to_chapter_row_renders_release_notes_heading_and_content(mocker, patch_hierarchy_action_inputs): + """Issue body with a Release Notes section is rendered as an indented block by to_chapter_row().""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + parent.body = "Description\nRelease Notes:\n- Fixed the critical bug" + record = HierarchyIssueRecord(parent) + + row = record.to_chapter_row() + + assert "_Release Notes_:" in row, f"Release Notes heading must appear; got:\n{row}" + assert "Fixed the critical bug" in row, f"Release notes content must appear; got:\n{row}" + + +def test_to_chapter_row_release_notes_block_indented_relative_to_level(mocker, patch_hierarchy_action_inputs): + """Release Notes heading is indented one level deeper than the issue's own hierarchy level.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=300) + parent.body = "Description\nRelease Notes:\n- Fixed the critical bug" + record = HierarchyIssueRecord(parent) + record.level = 0 + + row = record.to_chapter_row() + + rls_line = next((line for line in row.splitlines() if "_Release Notes_:" in line), None) + assert rls_line is not None, f"Release Notes heading line missing; got:\n{row}" + assert rls_line.startswith(" - "), ( + f"At level 0 heading must start with ' - ' (2-space indent + list marker); got: {rls_line!r}" + ) + + +def test_to_chapter_row_no_release_notes_body_omits_block(mocker, patch_hierarchy_action_inputs): + """Issue body without a Release Notes section produces no Release Notes heading.""" + parent = make_minimal_issue(mocker, IssueRecord.ISSUE_STATE_CLOSED, number=301) + parent.body = "Just a plain description with no special section." + record = HierarchyIssueRecord(parent) + + row = record.to_chapter_row() + + assert "_Release Notes_:" not in row, f"No Release Notes heading expected; got:\n{row}" diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index d7c981dd..5b06979d 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -61,6 +61,7 @@ ("get_duplicity_icon", "Oj", "Duplicity icon must be a non-empty string and have a length of 1."), ("get_row_format_issue", "", "Issue row format must be a non-empty string."), ("get_row_format_pr", "", "PR Row format must be a non-empty string."), + ("get_row_format_hierarchy_issue", "", "Hierarchy Issue row format must be a non-empty string."), ("get_release_notes_title", "", "Release Notes title must be a non-empty string and have non-zero length."), ( "get_coderabbit_release_notes_title", @@ -202,6 +203,128 @@ def test_get_chapters_yaml_error(mocker): assert [] == ActionInputs.get_chapters() +def test_get_super_chapters_success(mocker): + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "Module A", "label": "mod-a"}]', + ) + assert ActionInputs.get_super_chapters() == [{"title": "Module A", "labels": ["mod-a"]}] + + +def test_get_super_chapters_empty_input(mocker): + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="") + assert ActionInputs.get_super_chapters() == [] + + +def test_get_super_chapters_blank_input(mocker): + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value=" ") + assert ActionInputs.get_super_chapters() == [] + + +def test_get_super_chapters_not_a_list(mocker, caplog): + caplog.set_level("ERROR") + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="not_a_list: true") + result = ActionInputs.get_super_chapters() + assert result == [] + assert any("not a valid YAML list" in r.message for r in caplog.records) + + +def test_get_super_chapters_yaml_error(mocker, caplog): + caplog.set_level("ERROR") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "A" "label": "b"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [] + assert any("Error parsing 'super-chapters'" in r.message for r in caplog.records) + + +def test_get_super_chapters_list_labels_preserved(mocker): + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "Module A", "labels": ["mod-a", "mod-b"]}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Module A", "labels": ["mod-a", "mod-b"]}] + + +def test_get_super_chapters_comma_separated_labels_string(mocker): + """Comma-separated string labels are split into individual label tokens.""" + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "Module A", "labels": "atum-agent, atum-agent-spark"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Module A", "labels": ["atum-agent", "atum-agent-spark"]}] + + +def test_get_super_chapters_non_dict_entry_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='["not-a-dict", {"title": "Valid", "label": "ok"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["ok"]}] + assert any("invalid type" in r.message for r in caplog.records) + + +def test_get_super_chapters_missing_title_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"no-title": true}, {"title": "Valid", "label": "v"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["v"]}] + assert any("without title key" in r.message for r in caplog.records) + + +def test_get_super_chapters_non_string_title_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": 42, "label": "l"}, {"title": "Valid", "label": "v"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["v"]}] + assert any("invalid title value" in r.message for r in caplog.records) + + +def test_get_super_chapters_blank_title_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": " ", "label": "l"}, {"title": "Valid", "label": "v"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["v"]}] + assert any("invalid title value" in r.message for r in caplog.records) + + +def test_get_super_chapters_missing_labels_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "No labels"}, {"title": "Valid", "label": "v"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["v"]}] + assert any("has no 'label' or 'labels' key" in r.message for r in caplog.records) + + +def test_get_super_chapters_empty_labels_after_normalization_skipped(mocker, caplog): + caplog.set_level("WARNING") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value='[{"title": "Empty", "labels": []}, {"title": "Valid", "label": "v"}]', + ) + result = ActionInputs.get_super_chapters() + assert result == [{"title": "Valid", "labels": ["v"]}] + assert any("empty after normalization" in r.message for r in caplog.records) + + def test_get_warnings(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="true") assert ActionInputs.get_warnings() is True @@ -476,6 +599,62 @@ def test_get_row_format_link_pr_false(mocker): assert ActionInputs.get_row_format_link_pr() is False +def test_get_github_owner_no_slash_in_repository_id(monkeypatch, mocker): + """Repository ID without '/' sets owner to the whole string.""" + monkeypatch.setattr(ActionInputs, "_owner", "") + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="standalone") + assert ActionInputs.get_github_owner() == "standalone" + + +def test_get_github_owner_with_slash_in_repository_id(monkeypatch, mocker): + """Repository ID with '/' extracts the owner part before the slash.""" + monkeypatch.setattr(ActionInputs, "_owner", "") + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="my-org/my-repo") + assert ActionInputs.get_github_owner() == "my-org" + + +def test_get_github_owner_cache_hit(monkeypatch): + """Cached owner value is returned without querying the environment.""" + monkeypatch.setattr(ActionInputs, "_owner", "primed-org") + assert ActionInputs.get_github_owner() == "primed-org" + + +def test_get_github_repo_name_no_slash_in_repository_id(monkeypatch, mocker): + """Repository ID without '/' sets repo_name to the whole string.""" + monkeypatch.setattr(ActionInputs, "_repo_name", "") + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="standalone") + assert ActionInputs.get_github_repo_name() == "standalone" + + +def test_get_github_repo_name_with_slash_in_repository_id(monkeypatch, mocker): + """Repository ID with '/' extracts the repo name part after the slash.""" + monkeypatch.setattr(ActionInputs, "_repo_name", "") + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="my-org/my-repo") + assert ActionInputs.get_github_repo_name() == "my-repo" + + +def test_get_github_repo_name_cache_hit(monkeypatch): + """Cached repo name is returned without querying the environment.""" + monkeypatch.setattr(ActionInputs, "_repo_name", "primed-repo") + assert ActionInputs.get_github_repo_name() == "primed-repo" + + +def test_get_open_hierarchy_sub_issue_icon_default(): + assert ActionInputs.get_open_hierarchy_sub_issue_icon() == "🟡" + + +def test_get_open_hierarchy_sub_issue_icon_custom(mocker): + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="⚡") + assert ActionInputs.get_open_hierarchy_sub_issue_icon() == "⚡" + + +def test_detect_row_format_invalid_keywords_unknown_row_type(caplog): + """Unknown row_type logs a warning and defaults to Issue keys.""" + caplog.set_level("WARNING", logger="release_notes_generator.action_inputs") + ActionInputs._detect_row_format_invalid_keywords("{number} {bogus}", row_type="Unknown") + assert any("Unknown row_type" in r.message for r in caplog.records) + + # Mirrored test file for release_notes_generator/generator.py # Extracted from previous aggregated test_release_notes_generator.py