From 2601a6f18d47fd33bc0ef389af4091ffd016cca1 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 30 Mar 2026 15:50:07 +0200 Subject: [PATCH 1/6] feat: add {progress} format token to hierarchy issue rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a `progress` property to `HierarchyIssueRecord` that returns 'X/Y done' counting direct closed children vs total direct children (sub-issues and sub-hierarchy-issues, no recursion). Leaf nodes return '' so the token collapses cleanly in bare whitespace context via `format_row_with_suppression`. Register `progress` in `SUPPORTED_ROW_FORMAT_KEYS_HIERARCHY_ISSUE` so the format-string validator accepts it without stripping. - release_notes_generator/utils/constants.py: add "progress" to supported keys - release_notes_generator/model/record/hierarchy_issue_record.py: add progress property; pass format_values["progress"] in to_chapter_row() - tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py: new file — 9 tests covering leaf, partial, full, all-open, mixed children, row rendering, whitespace suppression, delimiter adjacency, per-level independence - docs/features/issue_hierarchy_support.md: document {progress} token and update Example Result section --- docs/features/issue_hierarchy_support.md | 24 +- .../model/record/hierarchy_issue_record.py | 18 ++ release_notes_generator/utils/constants.py | 2 +- .../model/test_hierarchy_issue_record.py | 232 ++++++++++++++++++ 4 files changed, 272 insertions(+), 4 deletions(-) create mode 100644 tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py diff --git a/docs/features/issue_hierarchy_support.md b/docs/features/issue_hierarchy_support.md index 6acccadd..5e969623 100644 --- a/docs/features/issue_hierarchy_support.md +++ b/docs/features/issue_hierarchy_support.md @@ -26,15 +26,33 @@ Represent issue → sub-issue relationships directly in release notes, aggregati ## Example Result ```markdown ### New Features 🎉 -- Epic: _Make Login Service releasable under new Maven central repository_ #140 +- Epic: _Make Login Service releasable under new Maven central repository_ #140 1/2 done - Updated `sbt.version` to `1.11.5` for release. - Updated Developers - Updated `sbt-ci-release` to `1.11.2` - Updated `scala213 = "2.13.13"` - - Feature: _Add user MFA enrollment flow_ #123 developed by @alice in #124 + - Feature: _Add user MFA enrollment flow_ #123 developed by @alice in #124 1/1 done - Add user MFA enrollment flow + - Feature: _Add OAuth2 login_ #125 developed by @bob in #126 0/1 done +``` +(1st four indented bullets under Epic line represent the extracted release notes from the parent hierarchy issue's body. `{progress}` counts direct children only — each level independently reports its own sub-issue completion.) + +## `{progress}` Format Token + +The `{progress}` token is available in `row-format-hierarchy-issue`. It renders the sub-issue completion count for each hierarchy node as `"X/Y done"`, where X and Y count **direct children only** — no recursive aggregation. + +- Counts both `SubIssueRecord` and nested `HierarchyIssueRecord` direct children. +- Each hierarchy level computes its own count independently when `to_chapter_row()` recurses. +- Leaf nodes (zero direct sub-issues) return an empty string; the token is suppressed, producing no extra whitespace. + +### Example + +Format: `row-format-hierarchy-issue: "{type}: _{title}_ {number} {progress}"` + +```markdown +- Epic: _Make Login Service releasable_ #140 2/3 done + - Feature: _Add user MFA enrollment flow_ #123 1/1 done ``` -(1st four indented bullets under Epic line represent the extracted release notes from the parent hierarchy issue's body.) ## Related Features - [Custom Row Formats](./custom_row_formats.md) – controls hierarchy line rendering. diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index fcf4c571..9c891379 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -74,6 +74,23 @@ def sub_hierarchy_issues(self): """ return self._sub_hierarchy_issues + @property + def progress(self) -> str: + """ + The sub-issue completion count for this hierarchy node as 'X/Y done'. + + Returns: + '' when this node has no direct sub-issues; otherwise 'X/Y done' + counting direct children only (sub-issues + sub-hierarchy-issues, no recursion). + Note: adjacent delimiter characters are not stripped when empty. + """ + total = len(self._sub_issues) + len(self._sub_hierarchy_issues) + if total == 0: + return "" + closed = sum(1 for s in self._sub_issues.values() if s.is_closed) + closed += sum(1 for s in self._sub_hierarchy_issues.values() if s.is_closed) + return f"{closed}/{total} done" + @property def developers(self) -> list[str]: issue = self._issue @@ -146,6 +163,7 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: format_values["type"] = self.issue_type else: format_values["type"] = "" + format_values["progress"] = self.progress list_pr_links = self.get_pr_links() if len(list_pr_links) > 0: diff --git a/release_notes_generator/utils/constants.py b/release_notes_generator/utils/constants.py index 39c449ea..89693c24 100644 --- a/release_notes_generator/utils/constants.py +++ b/release_notes_generator/utils/constants.py @@ -38,7 +38,7 @@ ROW_FORMAT_ISSUE = "row-format-issue" ROW_FORMAT_PR = "row-format-pr" ROW_FORMAT_LINK_PR = "row-format-link-pr" -SUPPORTED_ROW_FORMAT_KEYS_HIERARCHY_ISSUE = ["type", "number", "title", "author", "assignees", "developers"] +SUPPORTED_ROW_FORMAT_KEYS_HIERARCHY_ISSUE = ["type", "number", "title", "author", "assignees", "developers", "progress"] SUPPORTED_ROW_FORMAT_KEYS_ISSUE = ["type", "number", "title", "author", "assignees", "developers", "pull-requests"] SUPPORTED_ROW_FORMAT_KEYS_PULL_REQUEST = ["number", "title", "author", "assignees", "developers"] 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 new file mode 100644 index 00000000..4dffeb8f --- /dev/null +++ b/tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py @@ -0,0 +1,232 @@ +# +# Copyright 2023 ABSA Group Limited +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +Unit tests for HierarchyIssueRecord. +""" + +from datetime import datetime + +from github.Issue import Issue + +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 + + + +def _make_hierarchy_issue(mocker, number: int, state: str) -> Issue: + issue = mocker.Mock(spec=Issue) + issue.number = number + issue.title = f"HI{number}" + issue.state = state + issue.state_reason = None + issue.body = "" + issue.type = None + issue.created_at = datetime.now() + issue.closed_at = datetime.now() if state == IssueRecord.ISSUE_STATE_CLOSED else None + issue.repository.full_name = "org/repo" + issue.user = None + issue.assignees = [] + issue.get_labels.return_value = [] + issue.get_sub_issues.return_value = [] + return issue + + +def _make_sub_issue(mocker, number: int, state: str) -> SubIssueRecord: + issue = mocker.Mock(spec=Issue) + issue.number = number + issue.title = f"SI{number}" + issue.state = state + issue.state_reason = None + issue.body = "" + issue.type = None + issue.created_at = datetime.now() + issue.closed_at = datetime.now() if state == IssueRecord.ISSUE_STATE_CLOSED else None + issue.repository.full_name = "org/repo" + issue.user = None + issue.assignees = [] + issue.get_labels.return_value = [] + issue.get_sub_issues.return_value = [] + return SubIssueRecord(issue) + + + +def test_progress_leaf_node_returns_empty_string(mocker): + """HierarchyIssueRecord with no direct sub-issues returns '' for progress.""" + issue = _make_hierarchy_issue(mocker, 100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue) + + assert record.progress == "" + + +def test_progress_leaf_node_suppressed_in_row(mocker): + """{progress} token in format template produces no extra whitespace when leaf.""" + mocker.patch( + "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_row_format_hierarchy_issue", + return_value="_{title}_ {number} {progress}", + ) + + issue = _make_hierarchy_issue(mocker, 100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue) + row = record.to_chapter_row(add_into_chapters=False) + + assert "progress" not in row.lower() + assert " " not in row # no double spaces from suppressed token + assert row.endswith("_HI100_ #100"), f"Unexpected row: {row!r}" + + +def test_progress_rendered_in_row(mocker): + """Non-empty progress value appears correctly in the rendered row.""" + mocker.patch( + "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_row_format_hierarchy_issue", + return_value="_{title}_ {number} {progress}", + ) + + issue = _make_hierarchy_issue(mocker, 200, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue) + record._sub_issues = { + "org/repo#401": _make_sub_issue(mocker, 401, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#402": _make_sub_issue(mocker, 402, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#403": _make_sub_issue(mocker, 403, IssueRecord.ISSUE_STATE_OPEN), + } + row = record.to_chapter_row(add_into_chapters=False) + + assert "2/3 done" in row, f"Expected '2/3 done' in row, got: {row!r}" + + +def test_progress_empty_leaves_adjacent_delimiters(mocker): + """Empty {progress} does not strip surrounding delimiter characters.""" + mocker.patch( + "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_row_format_hierarchy_issue", + return_value="_{title}_ {number} ({progress})", + ) + + issue = _make_hierarchy_issue(mocker, 100, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue) # leaf — no sub-issues + row = record.to_chapter_row(add_into_chapters=False) + + assert "()" in row, f"Expected '()' in row when progress is empty, got: {row!r}" + + + +def test_progress_partial_completion(mocker): + """3 direct sub-issues (2 closed, 1 open) → '2/3 done'.""" + issue = _make_hierarchy_issue(mocker, 200, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue) + + record._sub_issues = { + "org/repo#401": _make_sub_issue(mocker, 401, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#402": _make_sub_issue(mocker, 402, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#403": _make_sub_issue(mocker, 403, IssueRecord.ISSUE_STATE_OPEN), + } + + assert record.progress == "2/3 done" + + +def test_progress_all_closed(mocker): + """All direct sub-issues closed → 'Y/Y done'.""" + issue = _make_hierarchy_issue(mocker, 201, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue) + + record._sub_issues = { + "org/repo#501": _make_sub_issue(mocker, 501, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#502": _make_sub_issue(mocker, 502, IssueRecord.ISSUE_STATE_CLOSED), + } + + assert record.progress == "2/2 done" + + +def test_progress_all_open(mocker): + """All direct sub-issues open → '0/N done'.""" + issue = _make_hierarchy_issue(mocker, 202, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(issue) + + record._sub_issues = { + "org/repo#601": _make_sub_issue(mocker, 601, IssueRecord.ISSUE_STATE_OPEN), + "org/repo#602": _make_sub_issue(mocker, 602, IssueRecord.ISSUE_STATE_OPEN), + } + + assert record.progress == "0/2 done" + + +def test_progress_mixed_sub_issues_and_sub_hierarchy_issues(mocker): + """Direct children include both SubIssueRecord and sub HierarchyIssueRecord.""" + parent_issue = _make_hierarchy_issue(mocker, 300, IssueRecord.ISSUE_STATE_OPEN) + record = HierarchyIssueRecord(parent_issue) + + # 1 closed sub-issue + record._sub_issues = { + "org/repo#701": _make_sub_issue(mocker, 701, IssueRecord.ISSUE_STATE_CLOSED), + } + + # 1 open sub-hierarchy-issue, 1 closed sub-hierarchy-issue + child_open_issue = _make_hierarchy_issue(mocker, 801, IssueRecord.ISSUE_STATE_OPEN) + child_closed_issue = _make_hierarchy_issue(mocker, 802, IssueRecord.ISSUE_STATE_CLOSED) + record._sub_hierarchy_issues = { + "org/repo#801": HierarchyIssueRecord(child_open_issue), + "org/repo#802": HierarchyIssueRecord(child_closed_issue), + } + + # total=3 (1 sub_issue + 2 sub_hierarchy_issues), closed=2 (sub_issue + child_closed) + assert record.progress == "2/3 done" + + + +def test_progress_per_level_independence(mocker): + """ + In a 3-layer tree each node counts its own direct children only. + + Tree: + root (open) + child_a (closed) → has 1 closed grandchild + child_b (closed) → has 3 grandchildren (2 closed) + child_c (open) → has 0 grandchildren + + root.progress → 2/3 done (child_a closed, child_b closed, child_c open) + child_a.progress → 1/1 done (its own grandchild) + child_b.progress → 2/3 done (its own grandchildren) + child_c.progress → "" (leaf) + """ + root_issue = _make_hierarchy_issue(mocker, 900, IssueRecord.ISSUE_STATE_OPEN) + root = HierarchyIssueRecord(root_issue) + + child_a_issue = _make_hierarchy_issue(mocker, 901, IssueRecord.ISSUE_STATE_CLOSED) + child_a = HierarchyIssueRecord(child_a_issue) + child_a._sub_issues = { + "org/repo#911": _make_sub_issue(mocker, 911, IssueRecord.ISSUE_STATE_CLOSED), + } + + child_b_issue = _make_hierarchy_issue(mocker, 902, IssueRecord.ISSUE_STATE_CLOSED) + child_b = HierarchyIssueRecord(child_b_issue) + child_b._sub_issues = { + "org/repo#921": _make_sub_issue(mocker, 921, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#922": _make_sub_issue(mocker, 922, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#923": _make_sub_issue(mocker, 923, IssueRecord.ISSUE_STATE_OPEN), + } + + child_c_issue = _make_hierarchy_issue(mocker, 903, IssueRecord.ISSUE_STATE_OPEN) + child_c = HierarchyIssueRecord(child_c_issue) + + root._sub_hierarchy_issues = { + "org/repo#901": child_a, + "org/repo#902": child_b, + "org/repo#903": child_c, + } + + assert root.progress == "2/3 done", f"root: {root.progress!r}" + assert child_a.progress == "1/1 done", f"child_a: {child_a.progress!r}" + assert child_b.progress == "2/3 done", f"child_b: {child_b.progress!r}" + assert child_c.progress == "", f"child_c: {child_c.progress!r}" From caba6aac716437e9f414fa7909708b9e3dc6433b Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 30 Mar 2026 17:12:46 +0200 Subject: [PATCH 2/6] fix: remove progress indicators from example output in issue hierarchy support documentation --- docs/features/issue_hierarchy_support.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/docs/features/issue_hierarchy_support.md b/docs/features/issue_hierarchy_support.md index 5e969623..75475bcd 100644 --- a/docs/features/issue_hierarchy_support.md +++ b/docs/features/issue_hierarchy_support.md @@ -26,16 +26,15 @@ Represent issue → sub-issue relationships directly in release notes, aggregati ## Example Result ```markdown ### New Features 🎉 -- Epic: _Make Login Service releasable under new Maven central repository_ #140 1/2 done +- Epic: _Make Login Service releasable under new Maven central repository_ #140 - Updated `sbt.version` to `1.11.5` for release. - Updated Developers - Updated `sbt-ci-release` to `1.11.2` - Updated `scala213 = "2.13.13"` - - Feature: _Add user MFA enrollment flow_ #123 developed by @alice in #124 1/1 done + - Feature: _Add user MFA enrollment flow_ #123 developed by @alice in #124 - Add user MFA enrollment flow - - Feature: _Add OAuth2 login_ #125 developed by @bob in #126 0/1 done ``` -(1st four indented bullets under Epic line represent the extracted release notes from the parent hierarchy issue's body. `{progress}` counts direct children only — each level independently reports its own sub-issue completion.) +(1st four indented bullets under Epic line represent the extracted release notes from the parent hierarchy issue's body.) ## `{progress}` Format Token @@ -47,11 +46,17 @@ The `{progress}` token is available in `row-format-hierarchy-issue`. It renders ### Example -Format: `row-format-hierarchy-issue: "{type}: _{title}_ {number} {progress}"` +Configuration required to enable `{progress}` in hierarchy rows: +```yaml +row-format-hierarchy-issue: "{type}: _{title}_ {number} {progress}" +``` +Resulting output (hierarchy issues only — sub-issue rows use `row-format-issue` and do not carry `{progress}`): ```markdown -- Epic: _Make Login Service releasable_ #140 2/3 done +- Epic: _Make Login Service releasable_ #140 1/2 done - Feature: _Add user MFA enrollment flow_ #123 1/1 done + - Add user MFA enrollment flow + - Feature: _Add OAuth2 login_ #125 0/1 done ``` ## Related Features From d97b1422a15321b46e11d6359d3cd721f03d2b1b Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 30 Mar 2026 21:27:56 +0200 Subject: [PATCH 3/6] Fixed review comment. --- .../model/test_hierarchy_issue_record.py | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) 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 4dffeb8f..96d53e29 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 @@ -97,11 +97,11 @@ def test_progress_rendered_in_row(mocker): issue = _make_hierarchy_issue(mocker, 200, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) - record._sub_issues = { + record.sub_issues.update({ "org/repo#401": _make_sub_issue(mocker, 401, IssueRecord.ISSUE_STATE_CLOSED), "org/repo#402": _make_sub_issue(mocker, 402, IssueRecord.ISSUE_STATE_CLOSED), "org/repo#403": _make_sub_issue(mocker, 403, IssueRecord.ISSUE_STATE_OPEN), - } + }) row = record.to_chapter_row(add_into_chapters=False) assert "2/3 done" in row, f"Expected '2/3 done' in row, got: {row!r}" @@ -127,11 +127,11 @@ def test_progress_partial_completion(mocker): issue = _make_hierarchy_issue(mocker, 200, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) - record._sub_issues = { + record.sub_issues.update({ "org/repo#401": _make_sub_issue(mocker, 401, IssueRecord.ISSUE_STATE_CLOSED), "org/repo#402": _make_sub_issue(mocker, 402, IssueRecord.ISSUE_STATE_CLOSED), "org/repo#403": _make_sub_issue(mocker, 403, IssueRecord.ISSUE_STATE_OPEN), - } + }) assert record.progress == "2/3 done" @@ -141,10 +141,10 @@ def test_progress_all_closed(mocker): issue = _make_hierarchy_issue(mocker, 201, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) - record._sub_issues = { + record.sub_issues.update({ "org/repo#501": _make_sub_issue(mocker, 501, IssueRecord.ISSUE_STATE_CLOSED), "org/repo#502": _make_sub_issue(mocker, 502, IssueRecord.ISSUE_STATE_CLOSED), - } + }) assert record.progress == "2/2 done" @@ -154,10 +154,10 @@ def test_progress_all_open(mocker): issue = _make_hierarchy_issue(mocker, 202, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) - record._sub_issues = { + record.sub_issues.update({ "org/repo#601": _make_sub_issue(mocker, 601, IssueRecord.ISSUE_STATE_OPEN), "org/repo#602": _make_sub_issue(mocker, 602, IssueRecord.ISSUE_STATE_OPEN), - } + }) assert record.progress == "0/2 done" @@ -168,17 +168,17 @@ def test_progress_mixed_sub_issues_and_sub_hierarchy_issues(mocker): record = HierarchyIssueRecord(parent_issue) # 1 closed sub-issue - record._sub_issues = { + record.sub_issues.update({ "org/repo#701": _make_sub_issue(mocker, 701, IssueRecord.ISSUE_STATE_CLOSED), - } + }) # 1 open sub-hierarchy-issue, 1 closed sub-hierarchy-issue child_open_issue = _make_hierarchy_issue(mocker, 801, IssueRecord.ISSUE_STATE_OPEN) child_closed_issue = _make_hierarchy_issue(mocker, 802, IssueRecord.ISSUE_STATE_CLOSED) - record._sub_hierarchy_issues = { + record.sub_hierarchy_issues.update({ "org/repo#801": HierarchyIssueRecord(child_open_issue), "org/repo#802": HierarchyIssueRecord(child_closed_issue), - } + }) # total=3 (1 sub_issue + 2 sub_hierarchy_issues), closed=2 (sub_issue + child_closed) assert record.progress == "2/3 done" @@ -205,26 +205,26 @@ def test_progress_per_level_independence(mocker): child_a_issue = _make_hierarchy_issue(mocker, 901, IssueRecord.ISSUE_STATE_CLOSED) child_a = HierarchyIssueRecord(child_a_issue) - child_a._sub_issues = { + child_a.sub_issues.update({ "org/repo#911": _make_sub_issue(mocker, 911, IssueRecord.ISSUE_STATE_CLOSED), - } + }) child_b_issue = _make_hierarchy_issue(mocker, 902, IssueRecord.ISSUE_STATE_CLOSED) child_b = HierarchyIssueRecord(child_b_issue) - child_b._sub_issues = { + child_b.sub_issues.update({ "org/repo#921": _make_sub_issue(mocker, 921, IssueRecord.ISSUE_STATE_CLOSED), "org/repo#922": _make_sub_issue(mocker, 922, IssueRecord.ISSUE_STATE_CLOSED), "org/repo#923": _make_sub_issue(mocker, 923, IssueRecord.ISSUE_STATE_OPEN), - } + }) child_c_issue = _make_hierarchy_issue(mocker, 903, IssueRecord.ISSUE_STATE_OPEN) child_c = HierarchyIssueRecord(child_c_issue) - root._sub_hierarchy_issues = { + root.sub_hierarchy_issues.update({ "org/repo#901": child_a, "org/repo#902": child_b, "org/repo#903": child_c, - } + }) assert root.progress == "2/3 done", f"root: {root.progress!r}" assert child_a.progress == "1/1 done", f"child_a: {child_a.progress!r}" From 44d697a7acc4217afdf660ca1978f5d744337b55 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Wed, 1 Apr 2026 09:56:15 +0200 Subject: [PATCH 4/6] - Improved copilot instructions. - Fixed issue introduce by new code. --- .github/copilot-instructions.md | 2 + .../model/record/hierarchy_issue_record.py | 27 +++ .../release_notes_generator/model/conftest.py | 72 ++++++ .../model/test_hierarchy_issue_record.py | 218 +++++++++++------- 4 files changed, 235 insertions(+), 84 deletions(-) create mode 100644 tests/unit/release_notes_generator/model/conftest.py diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 65ed6844..1a07b1d3 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -81,6 +81,8 @@ Testing - Must test return values, exceptions, log messages, and exit codes. - Prefer unit tests under tests/unit/. - Must mock GitHub API interactions and environment variables in unit tests. +- 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. Tooling - Must format with Black (pyproject.toml). diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index 9c891379..de09da6a 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -132,6 +132,33 @@ def pull_requests_count(self) -> int: return count + def contains_change_increment(self) -> bool: + """ + Returns True only when this hierarchy sub-tree has at least one closed descendant with a change. + + A closed descendant with a PR (or a cross-repo placeholder) is the only evidence of finished + work that belongs in release notes. Open sub-issues whose PRs have not yet been merged must + not cause the parent to appear in the output. + """ + if self.is_cross_repo: + return True + + # Direct PRs attached to this hierarchy issue itself (IssueRecord level, no sub-tree) + if super().pull_requests_count() > 0: + return True + + # Only closed leaf sub-issues contribute + for sub_issue in self._sub_issues.values(): + if not sub_issue.is_open and sub_issue.contains_change_increment(): + return True + + # Sub-hierarchy-issues are checked recursively; the same rule applies at every level + for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): + if sub_hierarchy_issue.contains_change_increment(): + return True + + return False + def get_labels(self) -> list[str]: labels: set[str] = set() labels.update(label.name for label in self._issue.get_labels()) diff --git a/tests/unit/release_notes_generator/model/conftest.py b/tests/unit/release_notes_generator/model/conftest.py new file mode 100644 index 00000000..1604d38d --- /dev/null +++ b/tests/unit/release_notes_generator/model/conftest.py @@ -0,0 +1,72 @@ +# +# Copyright 2023 ABSA Group Limited +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +Shared fixtures for model unit tests. +""" + +from datetime import datetime + +import pytest +from github.Issue import Issue + +from release_notes_generator.model.record.issue_record import IssueRecord +from release_notes_generator.model.record.sub_issue_record import SubIssueRecord + + +@pytest.fixture +def make_hierarchy_issue(mocker): + """Factory fixture that creates a mocked hierarchy Issue.""" + def _factory(number: int, state: str) -> Issue: + issue = mocker.Mock(spec=Issue) + issue.number = number + issue.title = f"HI{number}" + issue.state = state + issue.state_reason = None + issue.body = "" + issue.type = None + issue.created_at = datetime.now() + issue.closed_at = datetime.now() if state == IssueRecord.ISSUE_STATE_CLOSED else None + issue.repository.full_name = "org/repo" + issue.user = None + issue.assignees = [] + issue.get_labels.return_value = [] + issue.get_sub_issues.return_value = [] + return issue + + return _factory + + +@pytest.fixture +def make_sub_issue(mocker): + """Factory fixture that creates a mocked SubIssueRecord.""" + def _factory(number: int, state: str) -> SubIssueRecord: + issue = mocker.Mock(spec=Issue) + issue.number = number + issue.title = f"SI{number}" + issue.state = state + issue.state_reason = None + issue.body = "" + issue.type = None + issue.created_at = datetime.now() + issue.closed_at = datetime.now() if state == IssueRecord.ISSUE_STATE_CLOSED else None + issue.repository.full_name = "org/repo" + issue.user = None + issue.assignees = [] + issue.get_labels.return_value = [] + issue.get_sub_issues.return_value = [] + return SubIssueRecord(issue) + + return _factory 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 96d53e29..b4f07499 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 @@ -17,69 +17,27 @@ Unit tests for HierarchyIssueRecord. """ -from datetime import datetime - -from github.Issue import Issue - 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 - - - -def _make_hierarchy_issue(mocker, number: int, state: str) -> Issue: - issue = mocker.Mock(spec=Issue) - issue.number = number - issue.title = f"HI{number}" - issue.state = state - issue.state_reason = None - issue.body = "" - issue.type = None - issue.created_at = datetime.now() - issue.closed_at = datetime.now() if state == IssueRecord.ISSUE_STATE_CLOSED else None - issue.repository.full_name = "org/repo" - issue.user = None - issue.assignees = [] - issue.get_labels.return_value = [] - issue.get_sub_issues.return_value = [] - return issue - - -def _make_sub_issue(mocker, number: int, state: str) -> SubIssueRecord: - issue = mocker.Mock(spec=Issue) - issue.number = number - issue.title = f"SI{number}" - issue.state = state - issue.state_reason = None - issue.body = "" - issue.type = None - issue.created_at = datetime.now() - issue.closed_at = datetime.now() if state == IssueRecord.ISSUE_STATE_CLOSED else None - issue.repository.full_name = "org/repo" - issue.user = None - issue.assignees = [] - issue.get_labels.return_value = [] - issue.get_sub_issues.return_value = [] - return SubIssueRecord(issue) - - - -def test_progress_leaf_node_returns_empty_string(mocker): + + + +def test_progress_leaf_node_returns_empty_string(make_hierarchy_issue): """HierarchyIssueRecord with no direct sub-issues returns '' for progress.""" - issue = _make_hierarchy_issue(mocker, 100, IssueRecord.ISSUE_STATE_OPEN) + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) assert record.progress == "" -def test_progress_leaf_node_suppressed_in_row(mocker): - """{progress} token in format template produces no extra whitespace when leaf.""" +def test_progress_leaf_node_suppressed_in_row(mocker, make_hierarchy_issue): + """"_{title}_ {number} {progress}" token in format template produces no extra whitespace when leaf.""" mocker.patch( "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_row_format_hierarchy_issue", return_value="_{title}_ {number} {progress}", ) - issue = _make_hierarchy_issue(mocker, 100, IssueRecord.ISSUE_STATE_OPEN) + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) row = record.to_chapter_row(add_into_chapters=False) @@ -88,33 +46,33 @@ def test_progress_leaf_node_suppressed_in_row(mocker): assert row.endswith("_HI100_ #100"), f"Unexpected row: {row!r}" -def test_progress_rendered_in_row(mocker): +def test_progress_rendered_in_row(mocker, make_hierarchy_issue, make_sub_issue): """Non-empty progress value appears correctly in the rendered row.""" mocker.patch( "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_row_format_hierarchy_issue", return_value="_{title}_ {number} {progress}", ) - issue = _make_hierarchy_issue(mocker, 200, IssueRecord.ISSUE_STATE_OPEN) + issue = make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) record.sub_issues.update({ - "org/repo#401": _make_sub_issue(mocker, 401, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#402": _make_sub_issue(mocker, 402, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#403": _make_sub_issue(mocker, 403, IssueRecord.ISSUE_STATE_OPEN), + "org/repo#401": make_sub_issue(401, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#402": make_sub_issue(402, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#403": make_sub_issue(403, IssueRecord.ISSUE_STATE_OPEN), }) row = record.to_chapter_row(add_into_chapters=False) assert "2/3 done" in row, f"Expected '2/3 done' in row, got: {row!r}" -def test_progress_empty_leaves_adjacent_delimiters(mocker): +def test_progress_empty_leaves_adjacent_delimiters(mocker, make_hierarchy_issue): """Empty {progress} does not strip surrounding delimiter characters.""" mocker.patch( "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_row_format_hierarchy_issue", return_value="_{title}_ {number} ({progress})", ) - issue = _make_hierarchy_issue(mocker, 100, IssueRecord.ISSUE_STATE_OPEN) + issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) # leaf — no sub-issues row = record.to_chapter_row(add_into_chapters=False) @@ -122,59 +80,59 @@ def test_progress_empty_leaves_adjacent_delimiters(mocker): -def test_progress_partial_completion(mocker): +def test_progress_partial_completion(make_hierarchy_issue, make_sub_issue): """3 direct sub-issues (2 closed, 1 open) → '2/3 done'.""" - issue = _make_hierarchy_issue(mocker, 200, IssueRecord.ISSUE_STATE_OPEN) + issue = make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) record.sub_issues.update({ - "org/repo#401": _make_sub_issue(mocker, 401, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#402": _make_sub_issue(mocker, 402, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#403": _make_sub_issue(mocker, 403, IssueRecord.ISSUE_STATE_OPEN), + "org/repo#401": make_sub_issue(401, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#402": make_sub_issue(402, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#403": make_sub_issue(403, IssueRecord.ISSUE_STATE_OPEN), }) assert record.progress == "2/3 done" -def test_progress_all_closed(mocker): +def test_progress_all_closed(make_hierarchy_issue, make_sub_issue): """All direct sub-issues closed → 'Y/Y done'.""" - issue = _make_hierarchy_issue(mocker, 201, IssueRecord.ISSUE_STATE_OPEN) + issue = make_hierarchy_issue(201, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) record.sub_issues.update({ - "org/repo#501": _make_sub_issue(mocker, 501, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#502": _make_sub_issue(mocker, 502, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#501": make_sub_issue(501, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#502": make_sub_issue(502, IssueRecord.ISSUE_STATE_CLOSED), }) assert record.progress == "2/2 done" -def test_progress_all_open(mocker): +def test_progress_all_open(make_hierarchy_issue, make_sub_issue): """All direct sub-issues open → '0/N done'.""" - issue = _make_hierarchy_issue(mocker, 202, IssueRecord.ISSUE_STATE_OPEN) + issue = make_hierarchy_issue(202, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) record.sub_issues.update({ - "org/repo#601": _make_sub_issue(mocker, 601, IssueRecord.ISSUE_STATE_OPEN), - "org/repo#602": _make_sub_issue(mocker, 602, IssueRecord.ISSUE_STATE_OPEN), + "org/repo#601": make_sub_issue(601, IssueRecord.ISSUE_STATE_OPEN), + "org/repo#602": make_sub_issue(602, IssueRecord.ISSUE_STATE_OPEN), }) assert record.progress == "0/2 done" -def test_progress_mixed_sub_issues_and_sub_hierarchy_issues(mocker): +def test_progress_mixed_sub_issues_and_sub_hierarchy_issues(make_hierarchy_issue, make_sub_issue): """Direct children include both SubIssueRecord and sub HierarchyIssueRecord.""" - parent_issue = _make_hierarchy_issue(mocker, 300, IssueRecord.ISSUE_STATE_OPEN) + parent_issue = make_hierarchy_issue(300, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(parent_issue) # 1 closed sub-issue record.sub_issues.update({ - "org/repo#701": _make_sub_issue(mocker, 701, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#701": make_sub_issue(701, IssueRecord.ISSUE_STATE_CLOSED), }) # 1 open sub-hierarchy-issue, 1 closed sub-hierarchy-issue - child_open_issue = _make_hierarchy_issue(mocker, 801, IssueRecord.ISSUE_STATE_OPEN) - child_closed_issue = _make_hierarchy_issue(mocker, 802, IssueRecord.ISSUE_STATE_CLOSED) + child_open_issue = make_hierarchy_issue(801, IssueRecord.ISSUE_STATE_OPEN) + child_closed_issue = make_hierarchy_issue(802, IssueRecord.ISSUE_STATE_CLOSED) record.sub_hierarchy_issues.update({ "org/repo#801": HierarchyIssueRecord(child_open_issue), "org/repo#802": HierarchyIssueRecord(child_closed_issue), @@ -185,7 +143,7 @@ def test_progress_mixed_sub_issues_and_sub_hierarchy_issues(mocker): -def test_progress_per_level_independence(mocker): +def test_progress_per_level_independence(make_hierarchy_issue, make_sub_issue): """ In a 3-layer tree each node counts its own direct children only. @@ -200,24 +158,24 @@ def test_progress_per_level_independence(mocker): child_b.progress → 2/3 done (its own grandchildren) child_c.progress → "" (leaf) """ - root_issue = _make_hierarchy_issue(mocker, 900, IssueRecord.ISSUE_STATE_OPEN) + root_issue = make_hierarchy_issue(900, IssueRecord.ISSUE_STATE_OPEN) root = HierarchyIssueRecord(root_issue) - child_a_issue = _make_hierarchy_issue(mocker, 901, IssueRecord.ISSUE_STATE_CLOSED) + child_a_issue = make_hierarchy_issue(901, IssueRecord.ISSUE_STATE_CLOSED) child_a = HierarchyIssueRecord(child_a_issue) child_a.sub_issues.update({ - "org/repo#911": _make_sub_issue(mocker, 911, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#911": make_sub_issue(911, IssueRecord.ISSUE_STATE_CLOSED), }) - child_b_issue = _make_hierarchy_issue(mocker, 902, IssueRecord.ISSUE_STATE_CLOSED) + child_b_issue = make_hierarchy_issue(902, IssueRecord.ISSUE_STATE_CLOSED) child_b = HierarchyIssueRecord(child_b_issue) child_b.sub_issues.update({ - "org/repo#921": _make_sub_issue(mocker, 921, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#922": _make_sub_issue(mocker, 922, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#923": _make_sub_issue(mocker, 923, IssueRecord.ISSUE_STATE_OPEN), + "org/repo#921": make_sub_issue(921, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#922": make_sub_issue(922, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#923": make_sub_issue(923, IssueRecord.ISSUE_STATE_OPEN), }) - child_c_issue = _make_hierarchy_issue(mocker, 903, IssueRecord.ISSUE_STATE_OPEN) + child_c_issue = make_hierarchy_issue(903, IssueRecord.ISSUE_STATE_OPEN) child_c = HierarchyIssueRecord(child_c_issue) root.sub_hierarchy_issues.update({ @@ -230,3 +188,95 @@ def test_progress_per_level_independence(mocker): assert child_a.progress == "1/1 done", f"child_a: {child_a.progress!r}" assert child_b.progress == "2/3 done", f"child_b: {child_b.progress!r}" assert child_c.progress == "", f"child_c: {child_c.progress!r}" + + +def _make_mock_pull(mocker, number: int): + """Create a minimal mock PullRequest with the given number.""" + from github.PullRequest import PullRequest as GHPullRequest + pull = mocker.Mock(spec=GHPullRequest) + pull.number = number + pull.get_labels.return_value = [] + return pull + + +def test_contains_change_increment_false_when_all_sub_issues_open(mocker, make_hierarchy_issue, make_sub_issue): + """Bug regression: open hierarchy with only open sub-issues (with PRs) must not appear in release notes.""" + parent = HierarchyIssueRecord(make_hierarchy_issue(10, IssueRecord.ISSUE_STATE_OPEN)) + + sub1 = make_sub_issue(11, IssueRecord.ISSUE_STATE_OPEN) + sub1.register_pull_request(_make_mock_pull(mocker, 111)) + sub2 = make_sub_issue(12, IssueRecord.ISSUE_STATE_OPEN) + sub2.register_pull_request(_make_mock_pull(mocker, 112)) + parent.sub_issues.update({"org/repo#11": sub1, "org/repo#12": sub2}) + + assert parent.contains_change_increment() is False + + +def test_contains_change_increment_true_when_one_closed_sub_issue_has_pr(mocker, make_hierarchy_issue, make_sub_issue): + """A single closed sub-issue with a PR is enough to mark the parent as having a change increment.""" + parent = HierarchyIssueRecord(make_hierarchy_issue(20, IssueRecord.ISSUE_STATE_OPEN)) + + open_sub = make_sub_issue(21, IssueRecord.ISSUE_STATE_OPEN) + open_sub.register_pull_request(_make_mock_pull(mocker, 211)) + closed_sub = make_sub_issue(22, IssueRecord.ISSUE_STATE_CLOSED) + closed_sub.register_pull_request(_make_mock_pull(mocker, 212)) + parent.sub_issues.update({"org/repo#21": open_sub, "org/repo#22": closed_sub}) + + assert parent.contains_change_increment() is True + + +def test_contains_change_increment_false_leaf_no_prs(make_hierarchy_issue): + """A leaf hierarchy issue with no direct PRs and no children returns False.""" + record = HierarchyIssueRecord(make_hierarchy_issue(30, IssueRecord.ISSUE_STATE_OPEN)) + + assert record.contains_change_increment() is False + + +def test_contains_change_increment_true_leaf_with_direct_pr(mocker, make_hierarchy_issue): + """A hierarchy issue with a direct PR on itself (not from sub-issues) returns True.""" + record = HierarchyIssueRecord(make_hierarchy_issue(40, IssueRecord.ISSUE_STATE_OPEN)) + record.register_pull_request(_make_mock_pull(mocker, 401)) + + assert record.contains_change_increment() is True + + +def test_contains_change_increment_true_cross_repo(make_hierarchy_issue): + """Cross-repo records always return True regardless of sub-issue state.""" + record = HierarchyIssueRecord(make_hierarchy_issue(50, IssueRecord.ISSUE_STATE_OPEN)) + record.is_cross_repo = True + + assert record.contains_change_increment() is True + + +def test_contains_change_increment_false_nested_open_only(mocker, make_hierarchy_issue, make_sub_issue): + """ + Regression: open root → open child hierarchy → open leaf sub-issues with PRs. + + The entire tree is open; no closed work exists → root must return False. + """ + root = HierarchyIssueRecord(make_hierarchy_issue(60, IssueRecord.ISSUE_STATE_OPEN)) + child = HierarchyIssueRecord(make_hierarchy_issue(61, IssueRecord.ISSUE_STATE_OPEN)) + + open_leaf = make_sub_issue(62, IssueRecord.ISSUE_STATE_OPEN) + open_leaf.register_pull_request(_make_mock_pull(mocker, 621)) + child.sub_issues.update({"org/repo#62": open_leaf}) + root.sub_hierarchy_issues.update({"org/repo#61": child}) + + assert root.contains_change_increment() is False + + +def test_contains_change_increment_true_nested_with_closed_leaf(mocker, make_hierarchy_issue, make_sub_issue): + """ + Open root → open child hierarchy → one closed leaf sub-issue with a PR. + + A closed descendant exists → root must return True. + """ + root = HierarchyIssueRecord(make_hierarchy_issue(70, IssueRecord.ISSUE_STATE_OPEN)) + child = HierarchyIssueRecord(make_hierarchy_issue(71, IssueRecord.ISSUE_STATE_OPEN)) + + closed_leaf = make_sub_issue(72, IssueRecord.ISSUE_STATE_CLOSED) + closed_leaf.register_pull_request(_make_mock_pull(mocker, 721)) + child.sub_issues.update({"org/repo#72": closed_leaf}) + root.sub_hierarchy_issues.update({"org/repo#71": child}) + + assert root.contains_change_increment() is True From e4391c0d44d9a5a40a31c62ed5c06e08c6d16e9d Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Wed, 1 Apr 2026 10:39:06 +0200 Subject: [PATCH 5/6] feat: annotate pytest fixture parameters with MockerFixture and return types --- .github/copilot-instructions.md | 1 + tests/unit/release_notes_generator/model/conftest.py | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 1a07b1d3..34d4b526 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -83,6 +83,7 @@ Testing - Must mock GitHub API interactions and environment variables in unit tests. - 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. Tooling - Must format with Black (pyproject.toml). diff --git a/tests/unit/release_notes_generator/model/conftest.py b/tests/unit/release_notes_generator/model/conftest.py index 1604d38d..c8bf2e99 100644 --- a/tests/unit/release_notes_generator/model/conftest.py +++ b/tests/unit/release_notes_generator/model/conftest.py @@ -17,9 +17,11 @@ Shared fixtures for model unit tests. """ +from collections.abc import Callable from datetime import datetime import pytest +from pytest_mock import MockerFixture from github.Issue import Issue from release_notes_generator.model.record.issue_record import IssueRecord @@ -27,7 +29,7 @@ @pytest.fixture -def make_hierarchy_issue(mocker): +def make_hierarchy_issue(mocker: MockerFixture) -> Callable[[int, str], Issue]: """Factory fixture that creates a mocked hierarchy Issue.""" def _factory(number: int, state: str) -> Issue: issue = mocker.Mock(spec=Issue) @@ -50,7 +52,7 @@ def _factory(number: int, state: str) -> Issue: @pytest.fixture -def make_sub_issue(mocker): +def make_sub_issue(mocker: MockerFixture) -> Callable[[int, str], SubIssueRecord]: """Factory fixture that creates a mocked SubIssueRecord.""" def _factory(number: int, state: str) -> SubIssueRecord: issue = mocker.Mock(spec=Issue) From 5b29cc76580e0b28c38096c857a54b3430e12c9c Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Wed, 1 Apr 2026 11:43:59 +0200 Subject: [PATCH 6/6] Addressed review comments. --- .../model/record/hierarchy_issue_record.py | 6 +- tests/integration/integration_test.py | 5 +- .../test_release_notes_snapshot.py | 13 +- tests/unit/conftest.py | 133 +++--- .../builder/test_release_notes_builder.py | 99 +++-- .../chapters/test_chapter.py | 1 - .../chapters/test_custom_chapters.py | 364 ++++++++++------ .../chapters/test_service_chapters.py | 1 - .../data/test_filter.py | 11 +- .../data/test_miner.py | 85 +++- .../utils/test_bulk_sub_issue_collector.py | 2 + .../release_notes_generator/model/conftest.py | 2 + .../model/test_commit_record.py | 6 + .../model/test_hierarchy_issue_record.py | 100 +++-- .../model/test_issue_record.py | 17 +- .../model/test_issue_record_row_formatting.py | 12 +- .../model/test_pull_request_record.py | 17 +- .../model/test_record.py | 12 +- .../factory/test_default_record_factory.py | 410 +++++++++++------- .../test_action_inputs.py | 119 ++++- .../release_notes_generator/test_generator.py | 43 +- .../utils/test_gh_action.py | 1 - .../utils/test_pull_request_utils.py | 59 +-- .../utils/test_utils.py | 1 - 24 files changed, 976 insertions(+), 543 deletions(-) diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index de09da6a..05a5aa4c 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -147,12 +147,12 @@ def contains_change_increment(self) -> bool: if super().pull_requests_count() > 0: return True - # Only closed leaf sub-issues contribute + # Only closed leaf sub-issues contribute; recurse to check their own PRs/cross-repo flag for sub_issue in self._sub_issues.values(): - if not sub_issue.is_open and sub_issue.contains_change_increment(): + if sub_issue.is_closed and sub_issue.contains_change_increment(): return True - # Sub-hierarchy-issues are checked recursively; the same rule applies at every level + # Recurse into sub-hierarchy-issues; the same closed-descendant rule applies at every level for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): if sub_hierarchy_issue.contains_change_increment(): return True diff --git a/tests/integration/integration_test.py b/tests/integration/integration_test.py index 75f4c287..673d370b 100644 --- a/tests/integration/integration_test.py +++ b/tests/integration/integration_test.py @@ -24,9 +24,7 @@ urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) -pytestmark = pytest.mark.skipif( - not os.getenv("GITHUB_TOKEN"), reason="GITHUB_TOKEN not set for integration test" -) +pytestmark = pytest.mark.skipif(not os.getenv("GITHUB_TOKEN"), reason="GITHUB_TOKEN not set for integration test") def test_bulk_sub_issue_collector_smoke(): @@ -41,4 +39,3 @@ def test_bulk_sub_issue_collector_smoke(): iterations += 1 # Collector internal state should be dict-like even if empty assert hasattr(collector, "parents_sub_issues") - diff --git a/tests/integration/test_release_notes_snapshot.py b/tests/integration/test_release_notes_snapshot.py index d833fb0b..3caa228e 100644 --- a/tests/integration/test_release_notes_snapshot.py +++ b/tests/integration/test_release_notes_snapshot.py @@ -59,12 +59,7 @@ def test_legacy_single_label_snapshot(): custom.populate(records) output = custom.to_string() - expected = ( - "### Bugfixes 🛠️\n" - "- org/repo#1 row\n\n" - "### Enhancements 🎉\n" - "- org/repo#2 row" - ) + expected = "### Bugfixes 🛠️\n" "- org/repo#1 row\n\n" "### Enhancements 🎉\n" "- org/repo#2 row" assert output == expected, f"Legacy snapshot changed.\nExpected:\n{expected}\nActual:\n{output}" @@ -79,10 +74,12 @@ def test_multi_label_integration_snapshot(): # T019 custom.from_yaml_array(chapters_yaml) records = { - "org/repo#10": build_mock_record("org/repo#10", ["bug", "enhancement"]), # appears once in Changes, once in Mixed + "org/repo#10": build_mock_record( + "org/repo#10", ["bug", "enhancement"] + ), # appears once in Changes, once in Mixed "org/repo#11": build_mock_record("org/repo#11", ["platform"]), # appears in Platform only "org/repo#12": build_mock_record("org/repo#12", ["infra", "platform"]), # Platform only once - "org/repo#13": build_mock_record("org/repo#13", ["feature"]) # Mixed only + "org/repo#13": build_mock_record("org/repo#13", ["feature"]), # Mixed only } custom.populate(records) out = custom.to_string() diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 2f7fdd6e..c924e4dd 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -39,21 +39,25 @@ from release_notes_generator.utils.github_rate_limiter import GithubRateLimiter from release_notes_generator.utils.record_utils import get_id - # Test classes + class MockLabel: def __init__(self, name): self.name = name + class FakeRepo: - def __init__(self, full_name): self.full_name = full_name + def __init__(self, full_name): + self.full_name = full_name + def mock_safe_call_decorator(_rate_limiter): def wrapper(fn): if fn.__name__ == "get_issues_for_pr": return mock_get_issues_for_pr return fn + return wrapper @@ -301,7 +305,7 @@ def mock_open_sub_issue(mocker, mock_user): issue.get_labels.return_value = [] issue.get_sub_issues.return_value = [] - return (issue) + return issue @pytest.fixture @@ -353,6 +357,7 @@ def mock_open_hierarchy_issue(mocker, mock_user): return issue + @pytest.fixture def mock_open_hierarchy_issue_epic(mocker, mock_user): issue_type = mocker.Mock(spec=IssueType) @@ -737,7 +742,7 @@ def mined_data_simple(mock_repo, mock_issue_closed): data = MinedData(mock_repo) # single issue record (closed) - solo_closed_issue = deepcopy(mock_issue_closed) # 121 + solo_closed_issue = deepcopy(mock_issue_closed) # 121 solo_closed_issue.body += "\nRelease Notes:\n- Solo issue release note" solo_closed_issue.get_labels.return_value = [] @@ -750,8 +755,14 @@ def mined_data_simple(mock_repo, mock_issue_closed): @pytest.fixture def mined_data_isolated_record_types_no_labels_no_type_defined( - mock_repo, mock_issue_closed, mock_pull_closed, mock_pull_merged, mock_commit, - mock_open_hierarchy_issue, mock_open_sub_issue, mock_closed_sub_issue + mock_repo, + mock_issue_closed, + mock_pull_closed, + mock_pull_merged, + mock_commit, + mock_open_hierarchy_issue, + mock_open_sub_issue, + mock_closed_sub_issue, ): # - single issue record (closed) # - single hierarchy issue record - two sub-issues without PRs @@ -763,7 +774,7 @@ def mined_data_isolated_record_types_no_labels_no_type_defined( data = MinedData(mock_repo) # single issue record (closed) - solo_closed_issue = deepcopy(mock_issue_closed) # 121 + solo_closed_issue = deepcopy(mock_issue_closed) # 121 solo_closed_issue.body += "\nRelease Notes:\n- Solo issue release note" solo_closed_issue.get_labels.return_value = [] data.parents_sub_issues[get_id(solo_closed_issue, mock_repo)] = [] @@ -832,7 +843,9 @@ def mined_data_isolated_record_types_no_labels_no_type_defined( hi_one_sub_hierarchy_two_sub_issues_with_prs_with_commit = deepcopy(mock_open_hierarchy_issue) hi_one_sub_hierarchy_two_sub_issues_with_prs_with_commit.number = 304 hi_one_sub_hierarchy_two_sub_issues_with_prs_with_commit.title = "HI304 open" - hi_one_sub_hierarchy_two_sub_issues_with_prs_with_commit.body = "I304 open\nRelease Notes:\n- Hierarchy level release note" + hi_one_sub_hierarchy_two_sub_issues_with_prs_with_commit.body = ( + "I304 open\nRelease Notes:\n- Hierarchy level release note" + ) sub_hierarchy_issue = deepcopy(mock_open_hierarchy_issue) sub_hierarchy_issue.number = 350 sub_hierarchy_issue.title = "HI350 open" @@ -860,9 +873,9 @@ def mined_data_isolated_record_types_no_labels_no_type_defined( data.parents_sub_issues[get_id(hi_one_sub_hierarchy_two_sub_issues_with_prs_with_commit, mock_repo)] = [shi] # single pull request record (closed, merged) - mock_pr_closed_1 = deepcopy(mock_pull_closed) # 123 + mock_pr_closed_1 = deepcopy(mock_pull_closed) # 123 mock_pr_closed_1.get_labels.return_value = [] - mock_pr_merged_1 = deepcopy(mock_pull_merged) # 124 + mock_pr_merged_1 = deepcopy(mock_pull_merged) # 124 mock_pr_merged_1.get_labels.return_value = [] # single direct commit record @@ -870,26 +883,38 @@ def mined_data_isolated_record_types_no_labels_no_type_defined( mock_commit_3.sha = "merge_commit_sha_direct" mock_commit_3.commit.message = "Direct commit example" - data.issues = {solo_closed_issue: mock_repo, - hi_two_sub_issues_no_prs: mock_repo, - hi_two_sub_issues_with_prs: mock_repo, - hi_two_sub_issues_with_prs_with_commit: mock_repo, - hi_one_sub_hierarchy_two_sub_issues_with_prs_with_commit: mock_repo, # index 4 - sub_issue_1: mock_repo, sub_issue_2: mock_repo, # index 5,6 - sub_issue_3: mock_repo, sub_issue_4: mock_repo, # index 7,8 - sub_issue_5: mock_repo, sub_issue_6: mock_repo, # index 9,10 - sub_issue_7: mock_repo, sub_issue_8: mock_repo, # index 11,12 - sub_hierarchy_issue: mock_repo} # index 13 - data.pull_requests = {mock_pr_closed_1: mock_repo, mock_pr_merged_1: mock_repo, - mock_pr_closed_2: mock_repo, mock_pr_closed_3: mock_repo, - mock_pr_closed_4: mock_repo} + data.issues = { + solo_closed_issue: mock_repo, + hi_two_sub_issues_no_prs: mock_repo, + hi_two_sub_issues_with_prs: mock_repo, + hi_two_sub_issues_with_prs_with_commit: mock_repo, + hi_one_sub_hierarchy_two_sub_issues_with_prs_with_commit: mock_repo, # index 4 + sub_issue_1: mock_repo, + sub_issue_2: mock_repo, # index 5,6 + sub_issue_3: mock_repo, + sub_issue_4: mock_repo, # index 7,8 + sub_issue_5: mock_repo, + sub_issue_6: mock_repo, # index 9,10 + sub_issue_7: mock_repo, + sub_issue_8: mock_repo, # index 11,12 + sub_hierarchy_issue: mock_repo, + } # index 13 + data.pull_requests = { + mock_pr_closed_1: mock_repo, + mock_pr_merged_1: mock_repo, + mock_pr_closed_2: mock_repo, + mock_pr_closed_3: mock_repo, + mock_pr_closed_4: mock_repo, + } data.commits = {mock_commit_1: mock_repo, mock_commit_2: mock_repo, mock_commit_3: mock_repo} return data @pytest.fixture -def mined_data_isolated_record_types_with_labels_no_type_defined(mocker, mined_data_isolated_record_types_no_labels_no_type_defined): +def mined_data_isolated_record_types_with_labels_no_type_defined( + mocker, mined_data_isolated_record_types_no_labels_no_type_defined +): data = mined_data_isolated_record_types_no_labels_no_type_defined l_enh = mocker.Mock(spec=MockLabel) @@ -906,12 +931,12 @@ def mined_data_isolated_record_types_with_labels_no_type_defined(mocker, mined_d iks = list(data.issues.keys()) iks[0].get_labels.return_value = [l_enh] - iks[1].get_labels.return_value = [l_epic] # 301 - iks[2].get_labels.return_value = [l_epic] # 302 - iks[3].get_labels.return_value = [l_epic] # 303 - iks[4].get_labels.return_value = [l_epic] # 304 + iks[1].get_labels.return_value = [l_epic] # 301 + iks[2].get_labels.return_value = [l_epic] # 302 + iks[3].get_labels.return_value = [l_epic] # 303 + iks[4].get_labels.return_value = [l_epic] # 304 - iks[13].get_labels.return_value = [l_feature] # 350 + iks[13].get_labels.return_value = [l_feature] # 350 iks[5].get_labels.return_value = [l_api] iks[6].get_labels.return_value = [l_api] @@ -930,7 +955,9 @@ def mined_data_isolated_record_types_with_labels_no_type_defined(mocker, mined_d @pytest.fixture -def mined_data_isolated_record_types_no_labels_with_type_defined(mocker, mined_data_isolated_record_types_no_labels_no_type_defined): +def mined_data_isolated_record_types_no_labels_with_type_defined( + mocker, mined_data_isolated_record_types_no_labels_no_type_defined +): data = mined_data_isolated_record_types_no_labels_no_type_defined t_epic = mocker.Mock(spec=IssueType) @@ -953,16 +980,16 @@ def mined_data_isolated_record_types_no_labels_with_type_defined(mocker, mined_d iks[0].type = t_feature iks[0].get_labels.return_value = [l_feature] - iks[1].type = t_epic # 301 + iks[1].type = t_epic # 301 iks[1].get_labels.return_value = [l_epic] - iks[2].type = t_epic # 302 + iks[2].type = t_epic # 302 iks[2].get_labels.return_value = [l_epic] - iks[3].type = t_epic # 303 + iks[3].type = t_epic # 303 iks[3].get_labels.return_value = [l_epic] - iks[4].type = t_epic # 304 + iks[4].type = t_epic # 304 iks[4].get_labels.return_value = [l_epic] - iks[13].type = t_feature # 350 + iks[13].type = t_feature # 350 iks[13].get_labels.return_value = [l_feature] iks[5].type = t_task @@ -986,7 +1013,9 @@ def mined_data_isolated_record_types_no_labels_with_type_defined(mocker, mined_d @pytest.fixture -def mined_data_isolated_record_types_with_labels_with_type_defined(mocker, mined_data_isolated_record_types_with_labels_no_type_defined): +def mined_data_isolated_record_types_with_labels_with_type_defined( + mocker, mined_data_isolated_record_types_with_labels_no_type_defined +): data = mined_data_isolated_record_types_with_labels_no_type_defined t_epic = mocker.Mock(spec=IssueType) @@ -1001,12 +1030,12 @@ def mined_data_isolated_record_types_with_labels_with_type_defined(mocker, mined iks = list(data.issues.keys()) iks[0].type = t_bug - iks[1].type = t_epic # 301 - iks[2].type = t_epic # 302 - iks[3].type = t_epic # 303 - iks[4].type = t_epic # 304 + iks[1].type = t_epic # 301 + iks[2].type = t_epic # 302 + iks[3].type = t_epic # 303 + iks[4].type = t_epic # 304 - iks[13].type = t_feature # 350 + iks[13].type = t_feature # 350 iks[5].type = t_task iks[6].type = t_task @@ -1025,18 +1054,22 @@ def mined_data_isolated_record_types_with_labels_with_type_defined(mocker, mined def record_with_issue_open_no_pull(request): return IssueRecord(issue=request.getfixturevalue("mock_issue_open")) + @pytest.fixture def record_with_issue_closed_no_pull(request): return IssueRecord(issue=request.getfixturevalue("mock_issue_closed")) + @pytest.fixture def record_with_pr_only(request): return PullRequestRecord(pull=request.getfixturevalue("mock_pull_merged_with_rls_notes_101")) + @pytest.fixture def record_with_direct_commit(request): return CommitRecord(commit=request.getfixturevalue("mock_commit")) + @pytest.fixture def record_with_issue_closed_one_pull(request): rec = IssueRecord(issue=request.getfixturevalue("mock_issue_closed")) @@ -1060,37 +1093,39 @@ def record_with_issue_closed_one_pull_merged_skip(request): @pytest.fixture def record_with_issue_closed_two_pulls(request): - rec = IssueRecord(issue=request.getfixturevalue("mock_issue_closed_i1_bug")) # TODO - renamed from mock_issue_closed_i2_bug + rec = IssueRecord( + issue=request.getfixturevalue("mock_issue_closed_i1_bug") + ) # TODO - renamed from mock_issue_closed_i2_bug rec.register_pull_request(request.getfixturevalue("mock_pull_closed_with_rls_notes_101")) rec.register_pull_request(request.getfixturevalue("mock_pull_closed_with_rls_notes_102")) return rec + @pytest.fixture def record_with_hierarchy_issues(request): rec_epic_issue = HierarchyIssueRecord( - issue=request.getfixturevalue("mock_open_hierarchy_issue_epic"), - issue_labels=["epic"] + issue=request.getfixturevalue("mock_open_hierarchy_issue_epic"), issue_labels=["epic"] ) # nr:200 - rec_feature_issue = HierarchyIssueRecord(request.getfixturevalue("mock_open_hierarchy_issue_feature")) # nr:201 + rec_feature_issue = HierarchyIssueRecord(request.getfixturevalue("mock_open_hierarchy_issue_feature")) # nr:201 rec_feature_issue.level = 1 rec_epic_issue.sub_hierarchy_issues[rec_feature_issue.issue.number] = rec_feature_issue - rec_task_issue = SubIssueRecord(request.getfixturevalue("mock_closed_issue_type_task")) # nr:202 + rec_task_issue = SubIssueRecord(request.getfixturevalue("mock_closed_issue_type_task")) # nr:202 rec_feature_issue.sub_issues[rec_task_issue.issue.number] = rec_task_issue # add sub_issue - rec_sub_issue_no_type = SubIssueRecord(request.getfixturevalue("mock_closed_issue_type_none")) # nr:204 + rec_sub_issue_no_type = SubIssueRecord(request.getfixturevalue("mock_closed_issue_type_none")) # nr:204 rec_feature_issue.sub_issues[rec_sub_issue_no_type.issue.number] = rec_sub_issue_no_type # add pr to sub_issue sub_issue_merged_pr = request.getfixturevalue("mock_pull_merged_with_rls_notes_102") # nr:205 - sub_issue_merged_pr.number = 205 # simulate PR closing sub-issue nr:204 + sub_issue_merged_pr.number = 205 # simulate PR closing sub-issue nr:204 sub_issue_merged_pr.body = "Closes #204\n\nRelease Notes:\n- Sub issue 204 closed by merged PR" sub_issue_merged_pr.title = "Sub issue 204 closed by merged PR" rec_sub_issue_no_type.register_pull_request(sub_issue_merged_pr) - rec_bug_issue = SubIssueRecord(request.getfixturevalue("mock_closed_issue_type_bug")) # nr:203 + rec_bug_issue = SubIssueRecord(request.getfixturevalue("mock_closed_issue_type_bug")) # nr:203 rec_feature_issue.sub_issues[rec_bug_issue.issue.number] = rec_bug_issue # not description keyword used - registration simulate API way (relation) diff --git a/tests/unit/release_notes_generator/builder/test_release_notes_builder.py b/tests/unit/release_notes_generator/builder/test_release_notes_builder.py index a56551ce..26a0cb95 100644 --- a/tests/unit/release_notes_generator/builder/test_release_notes_builder.py +++ b/tests/unit/release_notes_generator/builder/test_release_notes_builder.py @@ -77,11 +77,11 @@ DEFAULT_CHANGELOG_URL = "http://example.com/changelog" default_chapters = [ - {"title": "Breaking Changes 💥", "label": "breaking-change"}, - {"title": "New Features 🎉", "label": "feature"}, - {"title": "New Features 🎉", "label": "enhancement"}, - {"title": "Bugfixes 🛠", "label": "bug"}, - ] + {"title": "Breaking Changes 💥", "label": "breaking-change"}, + {"title": "New Features 🎉", "label": "feature"}, + {"title": "New Features 🎉", "label": "enhancement"}, + {"title": "Bugfixes 🛠", "label": "bug"}, +] RELEASE_NOTES_NO_DATA = """### Breaking Changes 💥 No entries detected. @@ -713,7 +713,9 @@ http://example.com/changelog """ -RELEASE_NOTES_DATA_SERVICE_CHAPTERS_CLOSED_PR_NO_ISSUE_SKIP_USER_LABELS = RELEASE_NOTES_NO_DATA_NO_WARNING_NO_EMPTY_CHAPTERS +RELEASE_NOTES_DATA_SERVICE_CHAPTERS_CLOSED_PR_NO_ISSUE_SKIP_USER_LABELS = ( + RELEASE_NOTES_NO_DATA_NO_WARNING_NO_EMPTY_CHAPTERS +) RELEASE_NOTES_DATA_SERVICE_CHAPTERS_OPEN_ISSUE_AND_MERGED_PR_NO_USER_LABELS = """### Merged PRs Linked to 'Not Closed' Issue ⚠️ - #122 _I1 open_ developed by @pr_author_101, @pr_author_102, @user in #101, #102 @@ -800,7 +802,9 @@ http://example.com/changelog """ -RELEASE_NOTES_DATA_CLOSED_ISSUE_WITH_MERGED_PRS_WITH_USER_LABELS_WITH_SKIP_LABEL = RELEASE_NOTES_NO_DATA_NO_WARNING_NO_EMPTY_CHAPTERS +RELEASE_NOTES_DATA_CLOSED_ISSUE_WITH_MERGED_PRS_WITH_USER_LABELS_WITH_SKIP_LABEL = ( + RELEASE_NOTES_NO_DATA_NO_WARNING_NO_EMPTY_CHAPTERS +) RELEASE_NOTES_DATA_CLOSED_ISSUE_WITH_MERGED_PRS_WITH_USER_LABELS = """### Chapter 1 🛠 - #122 _I1+bug_ developed by @pr_author_124, @user in #124 @@ -1174,7 +1178,9 @@ def test_build_open_issue_with_merged_pr_service_chapter_linked_to_not_closed_is @pytest.mark.parametrize("hierarchy_value", [True, False]) -def test_build_open_issue(custom_chapters_not_print_empty_chapters, record_with_issue_open_no_pull, mocker, hierarchy_value): +def test_build_open_issue( + custom_chapters_not_print_empty_chapters, record_with_issue_open_no_pull, mocker, hierarchy_value +): expected_release_notes = RELEASE_NOTES_NO_DATA_NO_WARNING_NO_EMPTY_CHAPTERS rec = record_with_issue_open_no_pull mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_print_empty_chapters", return_value=False) @@ -1192,7 +1198,9 @@ def test_build_open_issue(custom_chapters_not_print_empty_chapters, record_with_ @pytest.mark.parametrize("hierarchy_value", [True, False]) -def test_build_closed_issue(custom_chapters_not_print_empty_chapters, record_with_issue_closed_no_pull, mocker, hierarchy_value): +def test_build_closed_issue( + custom_chapters_not_print_empty_chapters, record_with_issue_closed_no_pull, mocker, hierarchy_value +): expected_release_notes = RELEASE_NOTES_DATA_SERVICE_CHAPTERS_CLOSED_ISSUE_NO_PR_NO_USER_LABELS rec = record_with_issue_closed_no_pull mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_print_empty_chapters", return_value=False) @@ -1210,7 +1218,9 @@ def test_build_closed_issue(custom_chapters_not_print_empty_chapters, record_wit @pytest.mark.parametrize("hierarchy_value", [True, False]) -def test_build_reopened_issue(custom_chapters_not_print_empty_chapters, record_with_issue_open_no_pull, mocker, hierarchy_value): +def test_build_reopened_issue( + custom_chapters_not_print_empty_chapters, record_with_issue_open_no_pull, mocker, hierarchy_value +): expected_release_notes = RELEASE_NOTES_NO_DATA_NO_WARNING_NO_EMPTY_CHAPTERS rec = record_with_issue_open_no_pull rec.issue.state_reason = "reopened" @@ -1375,6 +1385,7 @@ def test_build_closed_pr_without_issue_non_draft( # TODO - research situation when PR is not merged and is in draft state + @pytest.mark.parametrize("hierarchy_value", [True, False]) def test_merged_pr_without_issue_with_more_user_labels_duplicity_reduction_on( custom_chapters_not_print_empty_chapters, pull_request_record_merged, mocker, hierarchy_value @@ -1478,7 +1489,7 @@ def test_merged_pr_with_closed_issue_mention_with_user_labels_with_skip_label_on @pytest.mark.parametrize("hierarchy_value", [True, False]) def test_build_closed_pr_service_chapter_without_issue_with_skip_label_on_pr( - custom_chapters_not_print_empty_chapters, pull_request_record_closed_with_skip_label, mocker, hierarchy_value + custom_chapters_not_print_empty_chapters, pull_request_record_closed_with_skip_label, mocker, hierarchy_value ): expected_release_notes = RELEASE_NOTES_DATA_SERVICE_CHAPTERS_CLOSED_PR_NO_ISSUE_SKIP_USER_LABELS rec = pull_request_record_closed_with_skip_label @@ -1497,16 +1508,23 @@ def test_build_closed_pr_service_chapter_without_issue_with_skip_label_on_pr( def test_build_hierarchy_rls_notes_no_labels_no_type( - mocker, mock_repo, - custom_chapters_not_print_empty_chapters, - mined_data_isolated_record_types_no_labels_no_type_defined + mocker, + mock_repo, + custom_chapters_not_print_empty_chapters, + mined_data_isolated_record_types_no_labels_no_type_defined, ): expected_release_notes = RELEASE_NOTES_DATA_HIERARCHY_NO_LABELS_NO_TYPE - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator, + ) mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_print_empty_chapters", return_value=True) mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_hierarchy", return_value=True) - mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_row_format_hierarchy_issue", return_value="{type}: _{title}_ {number}") + mocker.patch( + "release_notes_generator.builder.builder.ActionInputs.get_row_format_hierarchy_issue", + return_value="{type}: _{title}_ {number}", + ) mock_github_client = mocker.Mock(spec=Github) @@ -1530,15 +1548,23 @@ def test_build_hierarchy_rls_notes_no_labels_no_type( def test_build_hierarchy_rls_notes_with_labels_no_type( - mocker, mock_repo, - custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_with_labels_no_type_defined + mocker, + mock_repo, + custom_chapters_not_print_empty_chapters, + mined_data_isolated_record_types_with_labels_no_type_defined, ): expected_release_notes = RELEASE_NOTES_DATA_HIERARCHY_WITH_LABELS_NO_TYPE - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator, + ) mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_print_empty_chapters", return_value=True) mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_hierarchy", return_value=True) - mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_row_format_hierarchy_issue", return_value="{type}: _{title}_ {number}") + mocker.patch( + "release_notes_generator.builder.builder.ActionInputs.get_row_format_hierarchy_issue", + return_value="{type}: _{title}_ {number}", + ) mock_github_client = mocker.Mock(spec=Github) @@ -1562,15 +1588,23 @@ def test_build_hierarchy_rls_notes_with_labels_no_type( def test_build_hierarchy_rls_notes_no_labels_with_type( - mocker, mock_repo, - custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_no_labels_with_type_defined + mocker, + mock_repo, + custom_chapters_not_print_empty_chapters, + mined_data_isolated_record_types_no_labels_with_type_defined, ): expected_release_notes = RELEASE_NOTES_DATA_HIERARCHY_NO_LABELS_WITH_TYPE - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator, + ) mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_print_empty_chapters", return_value=True) mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_hierarchy", return_value=True) - mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_row_format_hierarchy_issue", return_value="{type}: _{title}_ {number}") + mocker.patch( + "release_notes_generator.builder.builder.ActionInputs.get_row_format_hierarchy_issue", + return_value="{type}: _{title}_ {number}", + ) mock_github_client = mocker.Mock(spec=Github) @@ -1592,16 +1626,25 @@ def test_build_hierarchy_rls_notes_no_labels_with_type( assert expected_release_notes == actual_release_notes + def test_build_hierarchy_rls_notes_with_labels_with_type( - mocker, mock_repo, - custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_with_labels_with_type_defined + mocker, + mock_repo, + custom_chapters_not_print_empty_chapters, + mined_data_isolated_record_types_with_labels_with_type_defined, ): expected_release_notes = RELEASE_NOTES_DATA_HIERARCHY_WITH_LABELS_WITH_TYPE - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator, + ) mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_print_empty_chapters", return_value=True) mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_hierarchy", return_value=True) - mocker.patch("release_notes_generator.builder.builder.ActionInputs.get_row_format_hierarchy_issue", return_value="{type}: _{title}_ {number}") + mocker.patch( + "release_notes_generator.builder.builder.ActionInputs.get_row_format_hierarchy_issue", + return_value="{type}: _{title}_ {number}", + ) mock_github_client = mocker.Mock(spec=Github) diff --git a/tests/unit/release_notes_generator/chapters/test_chapter.py b/tests/unit/release_notes_generator/chapters/test_chapter.py index 6d7b5f70..90c38b75 100644 --- a/tests/unit/release_notes_generator/chapters/test_chapter.py +++ b/tests/unit/release_notes_generator/chapters/test_chapter.py @@ -16,7 +16,6 @@ from release_notes_generator.model.chapter import Chapter - # __init__ 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 5dabbe5e..ba731f98 100644 --- a/tests/unit/release_notes_generator/chapters/test_custom_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_custom_chapters.py @@ -158,12 +158,33 @@ def test_custom_chapters_from_yaml_array(): "chapter_def, expected, warning_substring", [ pytest.param({"title": "Multi", "labels": "bug"}, ["bug"], None, id="single-string-labels"), - pytest.param({"title": "Multi", "labels": "bug, enhancement"}, ["bug", "enhancement"], None, id="comma-separated"), - pytest.param({"title": "Multi", "labels": "bug\nenhancement"}, ["bug", "enhancement"], None, id="newline-separated"), - pytest.param({"title": "Multi", "labels": ["bug", "enhancement"]}, ["bug", "enhancement"], None, id="yaml-list"), - pytest.param({"title": "Mixed", "labels": " bug, enhancement,bug\nfeature , enhancement"}, ["bug", "enhancement", "feature"], None, id="mixed-separators-dedup-trim"), - pytest.param({"title": "Precedence", "label": "legacy", "labels": "new1, new2"}, ["new1", "new2"], "precedence", id="labels-key-precedence"), - pytest.param({"title": "UnicodeSpace", "labels": "\u2003bug\u00A0,\u2009enhancement"}, ["bug", "enhancement"], None, id="unicode-whitespace-trim"), + pytest.param( + {"title": "Multi", "labels": "bug, enhancement"}, ["bug", "enhancement"], None, id="comma-separated" + ), + pytest.param( + {"title": "Multi", "labels": "bug\nenhancement"}, ["bug", "enhancement"], None, id="newline-separated" + ), + pytest.param( + {"title": "Multi", "labels": ["bug", "enhancement"]}, ["bug", "enhancement"], None, id="yaml-list" + ), + pytest.param( + {"title": "Mixed", "labels": " bug, enhancement,bug\nfeature , enhancement"}, + ["bug", "enhancement", "feature"], + None, + id="mixed-separators-dedup-trim", + ), + pytest.param( + {"title": "Precedence", "label": "legacy", "labels": "new1, new2"}, + ["new1", "new2"], + "precedence", + id="labels-key-precedence", + ), + pytest.param( + {"title": "UnicodeSpace", "labels": "\u2003bug\u00a0,\u2009enhancement"}, + ["bug", "enhancement"], + None, + id="unicode-whitespace-trim", + ), ], ) def test_from_yaml_array_normalization_variants(chapter_def, expected, warning_substring, caplog): @@ -198,10 +219,12 @@ def test_duplicate_suppression_with_multi_label_record(record_stub): def test_overlapping_chapters_record_in_both(record_stub): # Arrange cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Bugs", "labels": "bug"}, - {"title": "Features", "labels": "feature, bug"}, - ]) + cc.from_yaml_array( + [ + {"title": "Bugs", "labels": "bug"}, + {"title": "Features", "labels": "feature, bug"}, + ] + ) record = record_stub("org/repo#99", ["bug"]) # qualifies for both records: dict[str, Record] = {"org/repo#99": record} # Act @@ -424,7 +447,7 @@ 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. @@ -447,7 +470,7 @@ 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. @@ -470,11 +493,13 @@ def test_populate_visible_chapter_duplicity_count(record_stub, mocker): def test_populate_mixed_visible_hidden_duplicity(record_stub): # Arrange cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Visible1", "labels": "bug", "hidden": False}, - {"title": "Hidden1", "labels": "bug", "hidden": True}, - {"title": "Visible2", "labels": "bug", "hidden": False}, - ]) + cc.from_yaml_array( + [ + {"title": "Visible1", "labels": "bug", "hidden": False}, + {"title": "Hidden1", "labels": "bug", "hidden": True}, + {"title": "Visible2", "labels": "bug", "hidden": False}, + ] + ) record = record_stub("org/repo#1", ["bug"]) records = {"org/repo#1": record} # Act @@ -490,10 +515,12 @@ def test_populate_mixed_visible_hidden_duplicity(record_stub): def test_to_string_hidden_chapter_excluded(): # Arrange cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Visible", "labels": "bug"}, - {"title": "Hidden", "labels": "feature", "hidden": True}, - ]) + cc.from_yaml_array( + [ + {"title": "Visible", "labels": "bug"}, + {"title": "Hidden", "labels": "feature", "hidden": True}, + ] + ) cc.chapters["Visible"].add_row(1, "Bug fix") cc.chapters["Hidden"].add_row(2, "Hidden feature") # Act @@ -508,17 +535,19 @@ 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 - cc.from_yaml_array([ - {"title": "Hidden1", "labels": "bug", "hidden": True}, - {"title": "Hidden2", "labels": "feature", "hidden": True}, - ]) + cc.from_yaml_array( + [ + {"title": "Hidden1", "labels": "bug", "hidden": True}, + {"title": "Hidden2", "labels": "feature", "hidden": True}, + ] + ) cc.chapters["Hidden1"].add_row(1, "Bug fix") cc.chapters["Hidden2"].add_row(2, "Feature") # Act @@ -575,10 +604,12 @@ def test_backward_compatibility_no_hidden_field(): # Arrange - test that chapters without hidden field work as before cc = CustomChapters() # Act - cc.from_yaml_array([ - {"title": "Breaking Changes 💥", "label": "breaking-change"}, - {"title": "New Features 🎉", "labels": ["enhancement", "feature"]}, - ]) + cc.from_yaml_array( + [ + {"title": "Breaking Changes 💥", "label": "breaking-change"}, + {"title": "New Features 🎉", "labels": ["enhancement", "feature"]}, + ] + ) # Assert assert "Breaking Changes 💥" in cc.chapters assert "New Features 🎉" in cc.chapters @@ -590,11 +621,13 @@ def test_from_yaml_array_order_parsed(): # Arrange cc = CustomChapters() # Act - cc.from_yaml_array([ - {"title": "Bugfixes 🛠", "labels": "bug", "order": 20}, - {"title": "Breaking Changes 💥", "label": "breaking-change", "order": 10}, - {"title": "Features 🎉", "labels": "feature"}, - ]) + cc.from_yaml_array( + [ + {"title": "Bugfixes 🛠", "labels": "bug", "order": 20}, + {"title": "Breaking Changes 💥", "label": "breaking-change", "order": 10}, + {"title": "Features 🎉", "labels": "feature"}, + ] + ) # Assert assert cc.chapters["Bugfixes 🛠"].order == 20 assert cc.chapters["Breaking Changes 💥"].order == 10 @@ -651,10 +684,12 @@ def test_from_yaml_array_repeated_title_same_order(): # Arrange cc = CustomChapters() # Act - cc.from_yaml_array([ - {"title": "Bugfixes 🛠", "label": "bug", "order": 20}, - {"title": "Bugfixes 🛠", "label": "error", "order": 20}, - ]) + cc.from_yaml_array( + [ + {"title": "Bugfixes 🛠", "label": "bug", "order": 20}, + {"title": "Bugfixes 🛠", "label": "error", "order": 20}, + ] + ) # Assert assert cc.chapters["Bugfixes 🛠"].labels == ["bug", "error"] assert cc.chapters["Bugfixes 🛠"].order == 20 @@ -665,10 +700,12 @@ def test_from_yaml_array_repeated_title_conflicting_order(caplog): caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") cc = CustomChapters() # Act - cc.from_yaml_array([ - {"title": "Bugfixes 🛠", "label": "bug", "order": 20}, - {"title": "Bugfixes 🛠", "label": "error", "order": 10}, - ]) + cc.from_yaml_array( + [ + {"title": "Bugfixes 🛠", "label": "bug", "order": 20}, + {"title": "Bugfixes 🛠", "label": "error", "order": 10}, + ] + ) # Assert - keeps first explicit value assert cc.chapters["Bugfixes 🛠"].order == 20 assert cc.chapters["Bugfixes 🛠"].labels == ["bug", "error"] @@ -679,10 +716,12 @@ def test_from_yaml_array_repeated_title_order_then_no_order(): # Arrange cc = CustomChapters() # Act - cc.from_yaml_array([ - {"title": "Ch", "label": "bug", "order": 10}, - {"title": "Ch", "label": "error"}, - ]) + cc.from_yaml_array( + [ + {"title": "Ch", "label": "bug", "order": 10}, + {"title": "Ch", "label": "error"}, + ] + ) # Assert - first explicit order kept assert cc.chapters["Ch"].order == 10 @@ -691,10 +730,12 @@ def test_from_yaml_array_repeated_title_no_order_then_order(): # Arrange cc = CustomChapters() # Act - cc.from_yaml_array([ - {"title": "Ch", "label": "bug"}, - {"title": "Ch", "label": "error", "order": 15}, - ]) + cc.from_yaml_array( + [ + {"title": "Ch", "label": "bug"}, + {"title": "Ch", "label": "error", "order": 15}, + ] + ) # Assert - second provides order, adopted assert cc.chapters["Ch"].order == 15 @@ -702,11 +743,13 @@ def test_from_yaml_array_repeated_title_no_order_then_order(): def test_to_string_order_sorting(): # Arrange cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Bugfixes 🛠", "labels": "bug", "order": 20}, - {"title": "Breaking Changes 💥", "label": "breaking-change", "order": 10}, - {"title": "Features 🎉", "labels": "feature"}, - ]) + cc.from_yaml_array( + [ + {"title": "Bugfixes 🛠", "labels": "bug", "order": 20}, + {"title": "Breaking Changes 💥", "label": "breaking-change", "order": 10}, + {"title": "Features 🎉", "labels": "feature"}, + ] + ) cc.chapters["Bugfixes 🛠"].add_row(1, "Fix 1") cc.chapters["Breaking Changes 💥"].add_row(2, "Break 1") cc.chapters["Features 🎉"].add_row(3, "Feat 1") @@ -722,11 +765,13 @@ def test_to_string_order_sorting(): def test_to_string_order_tie_preserves_first_seen(): # Arrange cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Alpha", "labels": "a", "order": 10}, - {"title": "Beta", "labels": "b", "order": 10}, - {"title": "Gamma", "labels": "c", "order": 10}, - ]) + cc.from_yaml_array( + [ + {"title": "Alpha", "labels": "a", "order": 10}, + {"title": "Beta", "labels": "b", "order": 10}, + {"title": "Gamma", "labels": "c", "order": 10}, + ] + ) cc.chapters["Alpha"].add_row(1, "A row") cc.chapters["Beta"].add_row(2, "B row") cc.chapters["Gamma"].add_row(3, "C row") @@ -742,11 +787,13 @@ def test_to_string_order_tie_preserves_first_seen(): def test_to_string_no_order_preserves_first_seen(): # Arrange cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Bugfixes 🛠", "labels": "bug"}, - {"title": "Features 🎉", "labels": "feature"}, - {"title": "Breaking Changes 💥", "label": "breaking-change"}, - ]) + cc.from_yaml_array( + [ + {"title": "Bugfixes 🛠", "labels": "bug"}, + {"title": "Features 🎉", "labels": "feature"}, + {"title": "Breaking Changes 💥", "label": "breaking-change"}, + ] + ) cc.chapters["Bugfixes 🛠"].add_row(1, "Fix 1") cc.chapters["Features 🎉"].add_row(2, "Feat 1") cc.chapters["Breaking Changes 💥"].add_row(3, "Break 1") @@ -762,12 +809,14 @@ def test_to_string_no_order_preserves_first_seen(): def test_to_string_mixed_ordered_and_unordered(): # Arrange cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Unordered1", "labels": "a"}, - {"title": "Ordered30", "labels": "b", "order": 30}, - {"title": "Unordered2", "labels": "c"}, - {"title": "Ordered10", "labels": "d", "order": 10}, - ]) + cc.from_yaml_array( + [ + {"title": "Unordered1", "labels": "a"}, + {"title": "Ordered30", "labels": "b", "order": 30}, + {"title": "Unordered2", "labels": "c"}, + {"title": "Ordered10", "labels": "d", "order": 10}, + ] + ) for title in cc.chapters: cc.chapters[title].add_row(1, "row") # Act @@ -780,11 +829,13 @@ def test_to_string_mixed_ordered_and_unordered(): def test_sorted_chapters_hidden_with_order(): # Arrange - hidden chapters with order are still sorted (but filtered in to_string) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Visible", "labels": "a", "order": 20}, - {"title": "Hidden", "labels": "b", "order": 10, "hidden": True}, - {"title": "Visible2", "labels": "c"}, - ]) + cc.from_yaml_array( + [ + {"title": "Visible", "labels": "a", "order": 20}, + {"title": "Hidden", "labels": "b", "order": 10, "hidden": True}, + {"title": "Visible2", "labels": "c"}, + ] + ) # Act sorted_chs = cc._sorted_chapters() # Assert @@ -866,10 +917,12 @@ def test_catch_open_hierarchy_no_label_filter(hierarchy_record_stub, monkeypatch monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "New Features 🎉", "labels": "feature"}, - {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, - ]) + cc.from_yaml_array( + [ + {"title": "New Features 🎉", "labels": "feature"}, + {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, + ] + ) record = hierarchy_record_stub("org/repo#H1", ["feature"], state="open") records = {"org/repo#H1": record} # Act @@ -884,10 +937,12 @@ def test_catch_open_hierarchy_with_label_filter_match(hierarchy_record_stub, mon monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "New Features 🎉", "labels": "feature"}, - {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "labels": "feature"}, - ]) + cc.from_yaml_array( + [ + {"title": "New Features 🎉", "labels": "feature"}, + {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "labels": "feature"}, + ] + ) record = hierarchy_record_stub("org/repo#H2", ["feature"], state="open") records = {"org/repo#H2": record} cc.populate(records) @@ -900,10 +955,12 @@ def test_catch_open_hierarchy_with_label_filter_no_match(hierarchy_record_stub, monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "New Features 🎉", "labels": "feature"}, - {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "labels": "epic"}, - ]) + cc.from_yaml_array( + [ + {"title": "New Features 🎉", "labels": "feature"}, + {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "labels": "epic"}, + ] + ) record = hierarchy_record_stub("org/repo#H3", ["feature"], state="open") records = {"org/repo#H3": record} cc.populate(records) @@ -916,10 +973,12 @@ def test_catch_open_hierarchy_closed_parent_not_intercepted(hierarchy_record_stu monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "New Features 🎉", "labels": "feature"}, - {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, - ]) + cc.from_yaml_array( + [ + {"title": "New Features 🎉", "labels": "feature"}, + {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, + ] + ) record = hierarchy_record_stub("org/repo#H4", ["feature"], state="closed") records = {"org/repo#H4": record} cc.populate(records) @@ -933,10 +992,12 @@ def test_catch_open_hierarchy_disabled_hierarchy_noop(hierarchy_record_stub, mon monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) caplog.set_level("WARNING") cc = CustomChapters() - cc.from_yaml_array([ - {"title": "New Features 🎉", "labels": "feature"}, - {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, - ]) + cc.from_yaml_array( + [ + {"title": "New Features 🎉", "labels": "feature"}, + {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, + ] + ) record = hierarchy_record_stub("org/repo#H5", ["feature"], state="open") records = {"org/repo#H5": record} cc.populate(records) @@ -950,10 +1011,12 @@ def test_catch_open_hierarchy_duplicate_warning(caplog): """AC-6: Two catch-open-hierarchy chapters → only first used, warning logged.""" caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, - {"title": "Also Silent 🤫", "catch-open-hierarchy": True, "labels": "bug"}, - ]) + cc.from_yaml_array( + [ + {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, + {"title": "Also Silent 🤫", "catch-open-hierarchy": True, "labels": "bug"}, + ] + ) # First chapter has it assert cc.chapters["Silent Live 🤫"].catch_open_hierarchy is True # Second chapter has it disabled due to duplicate @@ -964,9 +1027,11 @@ def test_catch_open_hierarchy_duplicate_warning(caplog): def test_catch_open_hierarchy_no_labels_chapter_created(): """catch-open-hierarchy chapter without labels is created with empty label list.""" cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, - ]) + cc.from_yaml_array( + [ + {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, + ] + ) assert "Silent Live 🤫" in cc.chapters assert cc.chapters["Silent Live 🤫"].labels == [] assert cc.chapters["Silent Live 🤫"].catch_open_hierarchy is True @@ -977,10 +1042,12 @@ def test_catch_open_hierarchy_with_hidden(hierarchy_record_stub, monkeypatch): monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "New Features 🎉", "labels": "feature"}, - {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "hidden": True}, - ]) + cc.from_yaml_array( + [ + {"title": "New Features 🎉", "labels": "feature"}, + {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "hidden": True}, + ] + ) record = hierarchy_record_stub("org/repo#H6", ["feature"], state="open") records = {"org/repo#H6": record} cc.populate(records) @@ -997,10 +1064,12 @@ def test_catch_open_hierarchy_non_hierarchy_record_not_intercepted(record_stub, monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "New Features 🎉", "labels": "feature"}, - {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, - ]) + cc.from_yaml_array( + [ + {"title": "New Features 🎉", "labels": "feature"}, + {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, + ] + ) record = record_stub("org/repo#R1", ["feature"]) records = {"org/repo#R1": record} cc.populate(records) @@ -1035,7 +1104,9 @@ def test_from_yaml_array_catch_open_hierarchy_validation(coh_value, expected, sh if should_warn: assert any("catch-open-hierarchy" in r.message for r in caplog.records) else: - assert not any("catch-open-hierarchy" in r.message.lower() and "invalid" in r.message.lower() for r in caplog.records) + assert not any( + "catch-open-hierarchy" in r.message.lower() and "invalid" in r.message.lower() for r in caplog.records + ) def test_catch_open_hierarchy_merge_path_adopts_flag(hierarchy_record_stub, monkeypatch): @@ -1044,10 +1115,12 @@ def test_catch_open_hierarchy_merge_path_adopts_flag(hierarchy_record_stub, monk monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Silent Live 🤫", "labels": "bug"}, # first: no COH - {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, # second: adds COH - ]) + cc.from_yaml_array( + [ + {"title": "Silent Live 🤫", "labels": "bug"}, # first: no COH + {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, # second: adds COH + ] + ) assert cc.chapters["Silent Live 🤫"].catch_open_hierarchy is True record = hierarchy_record_stub("org/repo#M1", ["bug"], state="open") @@ -1061,10 +1134,12 @@ def test_catch_open_hierarchy_no_labels_record_captured(hierarchy_record_stub, m monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "New Features 🎉", "labels": "feature"}, - {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, # no label filter - ]) + cc.from_yaml_array( + [ + {"title": "New Features 🎉", "labels": "feature"}, + {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, # no label filter + ] + ) record = hierarchy_record_stub("org/repo#N1", [], state="open") # deliberately no labels cc.populate({"org/repo#N1": record}) assert "org/repo#N1" in cc.chapters["Silent Live 🤫"].rows @@ -1076,9 +1151,11 @@ def test_catch_open_hierarchy_visible_increments_chapter_presence(hierarchy_reco monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, - ]) + cc.from_yaml_array( + [ + {"title": "Silent Live 🤫", "catch-open-hierarchy": True}, + ] + ) record = hierarchy_record_stub("org/repo#P1", ["feature"], state="open") cc.populate({"org/repo#P1": record}) assert record.chapter_presence_count() == 1 @@ -1089,9 +1166,11 @@ def test_catch_open_hierarchy_hidden_does_not_increment_chapter_presence(hierarc monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "hidden": True}, - ]) + cc.from_yaml_array( + [ + {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "hidden": True}, + ] + ) record = hierarchy_record_stub("org/repo#P2", ["feature"], state="open") cc.populate({"org/repo#P2": record}) assert "org/repo#P2" in cc.chapters["Silent Live 🤫"].rows @@ -1105,10 +1184,12 @@ def test_catch_open_hierarchy_skipped_chapter_does_not_consume_coh_slot(caplog, monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Broken COH", "catch-open-hierarchy": True, "labels": " "}, # empty → skipped - {"title": "Real Silent Live", "catch-open-hierarchy": True}, # should succeed - ]) + cc.from_yaml_array( + [ + {"title": "Broken COH", "catch-open-hierarchy": True, "labels": " "}, # empty → skipped + {"title": "Real Silent Live", "catch-open-hierarchy": True}, # should succeed + ] + ) # Skipped chapter not created assert "Broken COH" not in cc.chapters # Valid second COH chapter created normally @@ -1125,10 +1206,12 @@ def test_catch_open_hierarchy_same_title_repeat_no_warning(caplog): must produce one chapter with the flag set and all labels merged.""" caplog.set_level("WARNING", logger="release_notes_generator.chapters.custom_chapters") cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "labels": "feature"}, - {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "labels": "epic"}, - ]) + cc.from_yaml_array( + [ + {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "labels": "feature"}, + {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "labels": "epic"}, + ] + ) assert "Silent Live 🤫" in cc.chapters assert cc.chapters["Silent Live 🤫"].catch_open_hierarchy is True assert cc.chapters["Silent Live 🤫"].labels == ["feature", "epic"] @@ -1150,10 +1233,12 @@ def test_coh_chapter_not_populated_via_label_routing(hierarchy_record_stub, monk monkeypatch.setattr(ActionInputs, "get_hierarchy", staticmethod(lambda: True)) monkeypatch.setattr(ActionInputs, "get_verbose", staticmethod(lambda: False)) cc = CustomChapters() - cc.from_yaml_array([ - {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "labels": "feature"}, - {"title": "New Features 🎉", "labels": "feature"}, - ]) + cc.from_yaml_array( + [ + {"title": "Silent Live 🤫", "catch-open-hierarchy": True, "labels": "feature"}, + {"title": "New Features 🎉", "labels": "feature"}, + ] + ) # Closed hierarchy parent with matching label → must go to normal chapter, not COH closed_parent = hierarchy_record_stub("org/repo#C1", ["feature"], state="closed") @@ -1166,4 +1251,3 @@ 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 - diff --git a/tests/unit/release_notes_generator/chapters/test_service_chapters.py b/tests/unit/release_notes_generator/chapters/test_service_chapters.py index d1173571..f2a353e5 100644 --- a/tests/unit/release_notes_generator/chapters/test_service_chapters.py +++ b/tests/unit/release_notes_generator/chapters/test_service_chapters.py @@ -28,7 +28,6 @@ ) from release_notes_generator.utils.enums import DuplicityScopeEnum - # __init__ diff --git a/tests/unit/release_notes_generator/data/test_filter.py b/tests/unit/release_notes_generator/data/test_filter.py index 3f1cd502..75cb5c85 100644 --- a/tests/unit/release_notes_generator/data/test_filter.py +++ b/tests/unit/release_notes_generator/data/test_filter.py @@ -1,4 +1,3 @@ - # Copyright 2023 ABSA Group Limited # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -84,7 +83,9 @@ def test_filter_with_release(mocker): assert next(iter(filtered_data.issues.keys())).closed_at == datetime(2023, 1, 2) assert next(iter(filtered_data.pull_requests.keys())).merged_at == datetime(2023, 2, 3) assert next(iter(filtered_data.commits.keys())).commit.author.date == datetime(2024, 1, 4) - assert ('Starting issue, prs and commit reduction by the latest release since time.',) == mock_log_info.call_args_list[0][0] - assert ('Count of issues reduced from %d to %d', 2, 1) == mock_log_debug.call_args_list[1][0] - assert ('Count of pulls reduced from %d to %d', 2, 1) == mock_log_debug.call_args_list[2][0] - assert ('Count of commits reduced from %d to %d', 2, 1) == mock_log_debug.call_args_list[3][0] + assert ( + "Starting issue, prs and commit reduction by the latest release since time.", + ) == mock_log_info.call_args_list[0][0] + assert ("Count of issues reduced from %d to %d", 2, 1) == mock_log_debug.call_args_list[1][0] + assert ("Count of pulls reduced from %d to %d", 2, 1) == mock_log_debug.call_args_list[2][0] + assert ("Count of commits reduced from %d to %d", 2, 1) == mock_log_debug.call_args_list[3][0] diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index 572b405a..3e8a666a 100644 --- a/tests/unit/release_notes_generator/data/test_miner.py +++ b/tests/unit/release_notes_generator/data/test_miner.py @@ -38,6 +38,7 @@ class ChildBulkSubIssueCollector(BulkSubIssueCollector): Inherit from the real class so isinstance checks still pass. We AVOID calling super().__init__ to skip any network work. """ + def __init__(self, patch_parent_sub_issues: Optional[dict[str, list[str]]] = None): super().__init__("FAKE") self.patch_parent_sub_issues: Optional[dict[str, list[str]]] = patch_parent_sub_issues @@ -51,9 +52,11 @@ def scan_sub_issues_for_parents(self, parent_ids: list[str]) -> list[str]: return [] + def _identity(fn): return fn + def fake_fetch_repository(full_name: str): # you can branch based on input if full_name == "org_1/another_repo": @@ -63,7 +66,8 @@ def fake_fetch_repository(full_name: str): elif full_name == "org_3/another_repo": return FakeRepo("org_3/another_repo") else: - return None # simulate “not found” + return None # simulate “not found” + def decorator_mock(func): """ @@ -71,6 +75,7 @@ def decorator_mock(func): """ return func + class MinedDataMock(MinedData): """ Mock class for MinedData to simulate the behavior of the real MinedData class. @@ -93,6 +98,7 @@ def __init__(self, mocker, rls_mock: Optional[GitRelease], mock_repo: Repository } self.since = datetime.now() + def test_get_latest_release_from_tag_name_not_defined_2_releases_type_error(mocker, mock_repo, mock_git_releases): mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=False) mock_log_info = mocker.patch("release_notes_generator.data.miner.logger.info") @@ -115,10 +121,15 @@ def test_get_latest_release_from_tag_name_not_defined_2_releases_type_error(mock latest_release = release_notes_miner.get_latest_release(data.home_repository) assert latest_release is None - assert ('Getting latest release by semantic ordering (could not be the last one by time).',) == mock_log_info.call_args_list[0][0] - assert ('Latest release not found for %s. 1st release for repository!', 'org/repo') == mock_log_info.call_args_list[1][0] - assert ('Skipping invalid type of version tag: %s | Error: %s', 'v1.0.0', '') == mock_log_error.call_args_list[0][0] - assert ('Skipping invalid type of version tag: %s | Error: %s', 'v2.0.0', '') == mock_log_error.call_args_list[2][0] + assert ( + "Getting latest release by semantic ordering (could not be the last one by time).", + ) == mock_log_info.call_args_list[0][0] + assert ("Latest release not found for %s. 1st release for repository!", "org/repo") == mock_log_info.call_args_list[ + 1 + ][0] + assert ("Skipping invalid type of version tag: %s | Error: %s", "v1.0.0", "") == mock_log_error.call_args_list[0][0] + assert ("Skipping invalid type of version tag: %s | Error: %s", "v2.0.0", "") == mock_log_error.call_args_list[2][0] + def test_get_latest_release_from_tag_name_not_defined_2_releases_value_error(mocker, mock_repo, mock_git_releases): mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=False) @@ -144,10 +155,15 @@ def test_get_latest_release_from_tag_name_not_defined_2_releases_value_error(moc latest_release = release_notes_miner.get_latest_release(data.home_repository) assert latest_release is None - assert ('Getting latest release by semantic ordering (could not be the last one by time).',) == mock_log_info.call_args_list[0][0] - assert ('Latest release not found for %s. 1st release for repository!', 'org/repo') == mock_log_info.call_args_list[1][0] - assert ('Skipping invalid value of version tag: %s', 'v1.0.0') == mock_log_error.call_args_list[0][0] - assert ('Skipping invalid value of version tag: %s', 'v2.0.0') == mock_log_error.call_args_list[1][0] + assert ( + "Getting latest release by semantic ordering (could not be the last one by time).", + ) == mock_log_info.call_args_list[0][0] + assert ("Latest release not found for %s. 1st release for repository!", "org/repo") == mock_log_info.call_args_list[ + 1 + ][0] + assert ("Skipping invalid value of version tag: %s", "v1.0.0") == mock_log_error.call_args_list[0][0] + assert ("Skipping invalid value of version tag: %s", "v2.0.0") == mock_log_error.call_args_list[1][0] + def test_get_latest_release_from_tag_name_not_defined_2_releases(mocker, mock_repo, mock_git_releases): mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=False) @@ -169,7 +185,10 @@ def test_get_latest_release_from_tag_name_not_defined_2_releases(mocker, mock_re latest_release = release_notes_miner.get_latest_release(data.home_repository) assert latest_release is not None - assert ('Getting latest release by semantic ordering (could not be the last one by time).',) == mock_log_info.call_args_list[0][0] + assert ( + "Getting latest release by semantic ordering (could not be the last one by time).", + ) == mock_log_info.call_args_list[0][0] + def test_get_latest_release_from_tag_name_not_defined_no_release(mocker, mock_repo): mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=False) @@ -192,8 +211,13 @@ def test_get_latest_release_from_tag_name_not_defined_no_release(mocker, mock_re assert latest_release is None assert 2 == len(mock_log_info.call_args_list) - assert ('Getting latest release by semantic ordering (could not be the last one by time).',) == mock_log_info.call_args_list[0][0] - assert ('Latest release not found for %s. 1st release for repository!', 'org/repo') == mock_log_info.call_args_list[1][0] + assert ( + "Getting latest release by semantic ordering (could not be the last one by time).", + ) == mock_log_info.call_args_list[0][0] + assert ("Latest release not found for %s. 1st release for repository!", "org/repo") == mock_log_info.call_args_list[ + 1 + ][0] + def test_get_latest_release_from_tag_name_defined_release_exists(mocker, mock_repo): mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=True) @@ -219,7 +243,8 @@ def test_get_latest_release_from_tag_name_defined_release_exists(mocker, mock_re assert rls_mock == latest_release mock_exit.assert_not_called() assert 1 == len(mock_log_info.call_args_list) - assert ('Getting latest release by from-tag name %s', "") == mock_log_info.call_args_list[0][0] + assert ("Getting latest release by from-tag name %s", "") == mock_log_info.call_args_list[0][0] + # get_latest_release tests def test_get_latest_release_from_tag_name_defined_no_release(mocker, mock_repo): @@ -247,8 +272,8 @@ def test_get_latest_release_from_tag_name_defined_no_release(mocker, mock_repo): mock_exit.assert_called_once_with(1) assert 1 == len(mock_log_info.call_args_list) assert 1 == len(mock_log_error.call_args_list) - assert ('Getting latest release by from-tag name %s', "") == mock_log_info.call_args_list[0][0] - assert ('Latest release not found for received from-tag %s. Ending!', '') == mock_log_error.call_args_list[0][0] + assert ("Getting latest release by from-tag name %s", "") == mock_log_info.call_args_list[0][0] + assert ("Latest release not found for received from-tag %s. Ending!", "") == mock_log_error.call_args_list[0][0] def test_mine_data_repo_none_raises(mocker): @@ -262,10 +287,7 @@ def test_mine_data_repo_none_raises(mocker): miner._safe_call = _identity # ActionInputs.get_github_repository is consulted inside mine_data - mocker.patch( - "release_notes_generator.data.miner.ActionInputs.get_github_repository", - return_value="owner/repo" - ) + mocker.patch("release_notes_generator.data.miner.ActionInputs.get_github_repository", return_value="owner/repo") with pytest.raises(ValueError, match="Repository not found"): miner.mine_data() @@ -303,6 +325,7 @@ def test_mine_data_commits_without_since(mocker, mock_repo): # mine_missing_sub_issues + def test_scan_sub_issues_for_parents(mocker, mock_repo, mined_data_simple): gh = mocker.Mock() gh.get_repo.return_value = mock_repo @@ -337,7 +360,9 @@ def test_fetch_all_repositories_in_cache(mocker, mock_repo, mined_data_simple): patch_parents_sub_issues: dict[str, list[str]] = {} patch_parents_sub_issues["org_1/another_repo#122"] = ["org_2/another_repo#122", "org_3/another_repo#122", "o/r#1"] - mocker.patch.object(miner, "_make_bulk_sub_issue_collector", return_value=ChildBulkSubIssueCollector(patch_parents_sub_issues)) + mocker.patch.object( + miner, "_make_bulk_sub_issue_collector", return_value=ChildBulkSubIssueCollector(patch_parents_sub_issues) + ) mocker.patch.object(miner, "_fetch_missing_issues", return_value={}) mocker.patch.object(miner, "_fetch_prs_for_fetched_cross_issues", return_value={}) @@ -382,7 +407,16 @@ def fake_get_issue(num): miner._safe_call = lambda f: f patch_parents_sub_issues: dict[str, list[str]] = {} - patch_parents_sub_issues["org/repo#1"] = ["org/repo#2", "org/repo#3", "org*repo#4", "org/repoX#5", "org/repo#6", "org/repo#7", "org/repo#8", "org/repo#9"] + patch_parents_sub_issues["org/repo#1"] = [ + "org/repo#2", + "org/repo#3", + "org*repo#4", + "org/repoX#5", + "org/repo#6", + "org/repo#7", + "org/repo#8", + "org/repo#9", + ] for i in range(2, 10): if i == 4: patch_parents_sub_issues[f"org*repo#{i}"] = [] @@ -391,7 +425,9 @@ def fake_get_issue(num): else: patch_parents_sub_issues[f"org/repo#{i}"] = [] - mocker.patch.object(miner, "_make_bulk_sub_issue_collector", return_value=ChildBulkSubIssueCollector(patch_parents_sub_issues)) + mocker.patch.object( + miner, "_make_bulk_sub_issue_collector", return_value=ChildBulkSubIssueCollector(patch_parents_sub_issues) + ) mocker.patch.object(miner, "_fetch_all_repositories_in_cache", return_value={}) mocker.patch.object(miner, "_fetch_prs_for_fetched_cross_issues", return_value={}) @@ -422,7 +458,9 @@ def fake_get_issue(num): patch_parents_sub_issues: dict[str, list[str]] = {} - mocker.patch.object(miner, "_make_bulk_sub_issue_collector", return_value=ChildBulkSubIssueCollector(patch_parents_sub_issues)) + mocker.patch.object( + miner, "_make_bulk_sub_issue_collector", return_value=ChildBulkSubIssueCollector(patch_parents_sub_issues) + ) mocker.patch.object(miner, "_fetch_all_repositories_in_cache", return_value={}) mocker.patch.object(miner, "_fetch_prs_for_fetched_cross_issues", return_value={}) @@ -482,4 +520,3 @@ def test_fetch_prs_for_fetched_cross_issues(mocker, mock_repo): assert key_ok in result and result[key_ok] == [pr_obj] assert key_err in result and result[key_err] == [] warn_mock.assert_called_once() - diff --git a/tests/unit/release_notes_generator/data/utils/test_bulk_sub_issue_collector.py b/tests/unit/release_notes_generator/data/utils/test_bulk_sub_issue_collector.py index 642e10d2..d39c341b 100644 --- a/tests/unit/release_notes_generator/data/utils/test_bulk_sub_issue_collector.py +++ b/tests/unit/release_notes_generator/data/utils/test_bulk_sub_issue_collector.py @@ -22,6 +22,7 @@ CollectorConfig, ) + class DummyResponse: def __init__(self, data, status=200): self._data = data @@ -228,6 +229,7 @@ def test_graphql_errors_raise_runtime_error(): with pytest.raises(RuntimeError): col.scan_sub_issues_for_parents(["org/repo#1"]) + def test_issue_node_missing_marks_parent_complete(): # Response has repo alias but NO issue alias 'i0_0' resp = { diff --git a/tests/unit/release_notes_generator/model/conftest.py b/tests/unit/release_notes_generator/model/conftest.py index c8bf2e99..082c0cd6 100644 --- a/tests/unit/release_notes_generator/model/conftest.py +++ b/tests/unit/release_notes_generator/model/conftest.py @@ -31,6 +31,7 @@ @pytest.fixture def make_hierarchy_issue(mocker: MockerFixture) -> Callable[[int, str], Issue]: """Factory fixture that creates a mocked hierarchy Issue.""" + def _factory(number: int, state: str) -> Issue: issue = mocker.Mock(spec=Issue) issue.number = number @@ -54,6 +55,7 @@ def _factory(number: int, state: str) -> Issue: @pytest.fixture def make_sub_issue(mocker: MockerFixture) -> Callable[[int, str], SubIssueRecord]: """Factory fixture that creates a mocked SubIssueRecord.""" + def _factory(number: int, state: str) -> SubIssueRecord: issue = mocker.Mock(spec=Issue) issue.number = number diff --git a/tests/unit/release_notes_generator/model/test_commit_record.py b/tests/unit/release_notes_generator/model/test_commit_record.py index e2253d96..54ff21a3 100644 --- a/tests/unit/release_notes_generator/model/test_commit_record.py +++ b/tests/unit/release_notes_generator/model/test_commit_record.py @@ -24,15 +24,18 @@ def test_basic_properties(mock_commit): assert rec.is_open is False assert rec.skip is False + def test_authors_with_author(mock_commit): rec = CommitRecord(mock_commit) assert rec.author == "author" + def test_authors_no_author(mock_commit): mock_commit.author = None rec = CommitRecord(mock_commit) assert rec.author == "" + def test_to_chapter_row_single_occurrence(monkeypatch, mock_commit): monkeypatch.setattr( "release_notes_generator.model.record.commit_record.ActionInputs.get_duplicity_icon", @@ -44,6 +47,7 @@ def test_to_chapter_row_single_occurrence(monkeypatch, mock_commit): assert row.startswith("Commit: merge_c... - Fixed bug") assert "🔁" not in row + def test_to_chapter_row_duplicate_with_icon(monkeypatch, mock_commit): monkeypatch.setattr( "release_notes_generator.model.record.commit_record.ActionInputs.get_duplicity_icon", @@ -60,6 +64,7 @@ def test_to_chapter_row_duplicate_with_icon(monkeypatch, mock_commit): assert second.startswith("[D] ") assert rec.chapter_presence_count() == 2 + def test_to_chapter_row_with_release_notes_injected(monkeypatch, mock_commit): # Force contains_release_notes to True and provide fake release notes monkeypatch.setattr( @@ -72,6 +77,7 @@ def test_to_chapter_row_with_release_notes_injected(monkeypatch, mock_commit): row = rec.to_chapter_row() assert "\nExtra release notes." in row + def test_get_rls_notes_empty(mock_commit): rec = CommitRecord(mock_commit) assert rec.get_rls_notes() == "" 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 b4f07499..d9f6ba6e 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 @@ -21,7 +21,6 @@ from release_notes_generator.model.record.issue_record import IssueRecord - def test_progress_leaf_node_returns_empty_string(make_hierarchy_issue): """HierarchyIssueRecord with no direct sub-issues returns '' for progress.""" issue = make_hierarchy_issue(100, IssueRecord.ISSUE_STATE_OPEN) @@ -31,7 +30,7 @@ def test_progress_leaf_node_returns_empty_string(make_hierarchy_issue): def test_progress_leaf_node_suppressed_in_row(mocker, make_hierarchy_issue): - """"_{title}_ {number} {progress}" token in format template produces no extra whitespace when leaf.""" + """ "_{title}_ {number} {progress}" token in format template produces no extra whitespace when leaf.""" mocker.patch( "release_notes_generator.model.record.hierarchy_issue_record.ActionInputs.get_row_format_hierarchy_issue", return_value="_{title}_ {number} {progress}", @@ -55,11 +54,13 @@ def test_progress_rendered_in_row(mocker, make_hierarchy_issue, make_sub_issue): issue = make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) - record.sub_issues.update({ - "org/repo#401": make_sub_issue(401, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#402": make_sub_issue(402, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#403": make_sub_issue(403, IssueRecord.ISSUE_STATE_OPEN), - }) + record.sub_issues.update( + { + "org/repo#401": make_sub_issue(401, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#402": make_sub_issue(402, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#403": make_sub_issue(403, IssueRecord.ISSUE_STATE_OPEN), + } + ) row = record.to_chapter_row(add_into_chapters=False) assert "2/3 done" in row, f"Expected '2/3 done' in row, got: {row!r}" @@ -79,17 +80,18 @@ def test_progress_empty_leaves_adjacent_delimiters(mocker, make_hierarchy_issue) assert "()" in row, f"Expected '()' in row when progress is empty, got: {row!r}" - def test_progress_partial_completion(make_hierarchy_issue, make_sub_issue): """3 direct sub-issues (2 closed, 1 open) → '2/3 done'.""" issue = make_hierarchy_issue(200, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) - record.sub_issues.update({ - "org/repo#401": make_sub_issue(401, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#402": make_sub_issue(402, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#403": make_sub_issue(403, IssueRecord.ISSUE_STATE_OPEN), - }) + record.sub_issues.update( + { + "org/repo#401": make_sub_issue(401, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#402": make_sub_issue(402, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#403": make_sub_issue(403, IssueRecord.ISSUE_STATE_OPEN), + } + ) assert record.progress == "2/3 done" @@ -99,10 +101,12 @@ def test_progress_all_closed(make_hierarchy_issue, make_sub_issue): issue = make_hierarchy_issue(201, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) - record.sub_issues.update({ - "org/repo#501": make_sub_issue(501, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#502": make_sub_issue(502, IssueRecord.ISSUE_STATE_CLOSED), - }) + record.sub_issues.update( + { + "org/repo#501": make_sub_issue(501, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#502": make_sub_issue(502, IssueRecord.ISSUE_STATE_CLOSED), + } + ) assert record.progress == "2/2 done" @@ -112,10 +116,12 @@ def test_progress_all_open(make_hierarchy_issue, make_sub_issue): issue = make_hierarchy_issue(202, IssueRecord.ISSUE_STATE_OPEN) record = HierarchyIssueRecord(issue) - record.sub_issues.update({ - "org/repo#601": make_sub_issue(601, IssueRecord.ISSUE_STATE_OPEN), - "org/repo#602": make_sub_issue(602, IssueRecord.ISSUE_STATE_OPEN), - }) + record.sub_issues.update( + { + "org/repo#601": make_sub_issue(601, IssueRecord.ISSUE_STATE_OPEN), + "org/repo#602": make_sub_issue(602, IssueRecord.ISSUE_STATE_OPEN), + } + ) assert record.progress == "0/2 done" @@ -126,23 +132,26 @@ def test_progress_mixed_sub_issues_and_sub_hierarchy_issues(make_hierarchy_issue record = HierarchyIssueRecord(parent_issue) # 1 closed sub-issue - record.sub_issues.update({ - "org/repo#701": make_sub_issue(701, IssueRecord.ISSUE_STATE_CLOSED), - }) + record.sub_issues.update( + { + "org/repo#701": make_sub_issue(701, IssueRecord.ISSUE_STATE_CLOSED), + } + ) # 1 open sub-hierarchy-issue, 1 closed sub-hierarchy-issue child_open_issue = make_hierarchy_issue(801, IssueRecord.ISSUE_STATE_OPEN) child_closed_issue = make_hierarchy_issue(802, IssueRecord.ISSUE_STATE_CLOSED) - record.sub_hierarchy_issues.update({ - "org/repo#801": HierarchyIssueRecord(child_open_issue), - "org/repo#802": HierarchyIssueRecord(child_closed_issue), - }) + record.sub_hierarchy_issues.update( + { + "org/repo#801": HierarchyIssueRecord(child_open_issue), + "org/repo#802": HierarchyIssueRecord(child_closed_issue), + } + ) # total=3 (1 sub_issue + 2 sub_hierarchy_issues), closed=2 (sub_issue + child_closed) assert record.progress == "2/3 done" - def test_progress_per_level_independence(make_hierarchy_issue, make_sub_issue): """ In a 3-layer tree each node counts its own direct children only. @@ -163,26 +172,32 @@ def test_progress_per_level_independence(make_hierarchy_issue, make_sub_issue): child_a_issue = make_hierarchy_issue(901, IssueRecord.ISSUE_STATE_CLOSED) child_a = HierarchyIssueRecord(child_a_issue) - child_a.sub_issues.update({ - "org/repo#911": make_sub_issue(911, IssueRecord.ISSUE_STATE_CLOSED), - }) + child_a.sub_issues.update( + { + "org/repo#911": make_sub_issue(911, IssueRecord.ISSUE_STATE_CLOSED), + } + ) child_b_issue = make_hierarchy_issue(902, IssueRecord.ISSUE_STATE_CLOSED) child_b = HierarchyIssueRecord(child_b_issue) - child_b.sub_issues.update({ - "org/repo#921": make_sub_issue(921, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#922": make_sub_issue(922, IssueRecord.ISSUE_STATE_CLOSED), - "org/repo#923": make_sub_issue(923, IssueRecord.ISSUE_STATE_OPEN), - }) + child_b.sub_issues.update( + { + "org/repo#921": make_sub_issue(921, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#922": make_sub_issue(922, IssueRecord.ISSUE_STATE_CLOSED), + "org/repo#923": make_sub_issue(923, IssueRecord.ISSUE_STATE_OPEN), + } + ) child_c_issue = make_hierarchy_issue(903, IssueRecord.ISSUE_STATE_OPEN) child_c = HierarchyIssueRecord(child_c_issue) - root.sub_hierarchy_issues.update({ - "org/repo#901": child_a, - "org/repo#902": child_b, - "org/repo#903": child_c, - }) + root.sub_hierarchy_issues.update( + { + "org/repo#901": child_a, + "org/repo#902": child_b, + "org/repo#903": child_c, + } + ) assert root.progress == "2/3 done", f"root: {root.progress!r}" assert child_a.progress == "1/1 done", f"child_a: {child_a.progress!r}" @@ -193,6 +208,7 @@ def test_progress_per_level_independence(make_hierarchy_issue, make_sub_issue): def _make_mock_pull(mocker, number: int): """Create a minimal mock PullRequest with the given number.""" from github.PullRequest import PullRequest as GHPullRequest + pull = mocker.Mock(spec=GHPullRequest) pull.number = number pull.get_labels.return_value = [] diff --git a/tests/unit/release_notes_generator/model/test_issue_record.py b/tests/unit/release_notes_generator/model/test_issue_record.py index 0b6b1841..3638c031 100644 --- a/tests/unit/release_notes_generator/model/test_issue_record.py +++ b/tests/unit/release_notes_generator/model/test_issue_record.py @@ -79,14 +79,20 @@ def test_issue_record_init_with_type_value(mocker): def test_code_rabbit_empty_body(issue_record, mocker): - mocker.patch("release_notes_generator.model.record.issue_record.ActionInputs.get_coderabbit_summary_ignore_groups", return_value=[]) + mocker.patch( + "release_notes_generator.model.record.issue_record.ActionInputs.get_coderabbit_summary_ignore_groups", + return_value=[], + ) pr = make_pr(mocker, None) out = get_rls_notes_code_rabbit(pr.body, LINE_MARKS, CR_REGEX) assert out == "" def test_code_rabbit_extract_basic(issue_record, mocker): - mocker.patch("release_notes_generator.model.record.issue_record.ActionInputs.get_coderabbit_summary_ignore_groups", return_value=[]) + mocker.patch( + "release_notes_generator.model.record.issue_record.ActionInputs.get_coderabbit_summary_ignore_groups", + return_value=[], + ) body = """Intro line Summary by CodeRabbit - Added feature A @@ -101,7 +107,7 @@ def test_code_rabbit_extract_basic(issue_record, mocker): def test_code_rabbit_ignored_group_then_kept_group(issue_record, mocker): mocker.patch( "release_notes_generator.model.record.issue_record.ActionInputs.get_coderabbit_summary_ignore_groups", - return_value=["Chore"] + return_value=["Chore"], ) body = """Header Summary by CodeRabbit @@ -118,7 +124,10 @@ def test_code_rabbit_ignored_group_then_kept_group(issue_record, mocker): def test_code_rabbit_non_bullet_terminates(issue_record, mocker): - mocker.patch("release_notes_generator.model.record.issue_record.ActionInputs.get_coderabbit_summary_ignore_groups", return_value=[]) + mocker.patch( + "release_notes_generator.model.record.issue_record.ActionInputs.get_coderabbit_summary_ignore_groups", + return_value=[], + ) body = """Preamble Summary by CodeRabbit - First line diff --git a/tests/unit/release_notes_generator/model/test_issue_record_row_formatting.py b/tests/unit/release_notes_generator/model/test_issue_record_row_formatting.py index 930daff5..411ee933 100644 --- a/tests/unit/release_notes_generator/model/test_issue_record_row_formatting.py +++ b/tests/unit/release_notes_generator/model/test_issue_record_row_formatting.py @@ -51,12 +51,8 @@ def test_missing_type_no_assignees_no_prs(mocker, mock_issue_closed): # Should NOT contain any of these fragments when fields are empty assert "N/A" not in row, f"Row should not contain 'N/A', got: {row}" assert "assigned to" not in row, f"Row should not contain 'assigned to', got: {row}" - assert ( - "developed by" not in row - ), f"Row should not contain 'developed by', got: {row}" - assert ( - " in " not in row.lower() or " in #" in row.lower() - ), f"Row should not contain dangling 'in', got: {row}" + assert "developed by" not in row, f"Row should not contain 'developed by', got: {row}" + assert " in " not in row.lower() or " in #" in row.lower(), f"Row should not contain dangling 'in', got: {row}" # Should contain these assert "#231" in row @@ -133,9 +129,7 @@ def test_has_type_missing_assignees_has_prs(mocker, mock_issue_closed, mock_pull assert "Task:" in row or "Task" in row, f"Row should contain type, got: {row}" assert "#200" in row assert "Add feature" in row - assert ( - "assigned to" not in row - ), f"Row should not contain 'assigned to', got: {row}" + assert "assigned to" not in row, f"Row should not contain 'assigned to', got: {row}" assert "developed by" in row # Should show because there are developers assert "#201" in row diff --git a/tests/unit/release_notes_generator/model/test_pull_request_record.py b/tests/unit/release_notes_generator/model/test_pull_request_record.py index df4a4d3c..2ed1fbd3 100644 --- a/tests/unit/release_notes_generator/model/test_pull_request_record.py +++ b/tests/unit/release_notes_generator/model/test_pull_request_record.py @@ -20,6 +20,7 @@ # get_rls_notes + def test_get_rls_notes_coderabbit_ignores_groups(mocker, mock_repo): pr = mocker.Mock(spec=PullRequest) pr.state = PullRequestRecord.PR_STATE_CLOSED @@ -38,16 +39,26 @@ def test_get_rls_notes_coderabbit_ignores_groups(mocker, mock_repo): rec = PullRequestRecord(pr, mock_repo) - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_release_notes_title", return_value=r"^Release Notes:$") + mocker.patch( + "release_notes_generator.action_inputs.ActionInputs.get_release_notes_title", return_value=r"^Release Notes:$" + ) mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_coderabbit_support_active", return_value=True) - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_coderabbit_release_notes_title", return_value="Summary by CodeRabbit") - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_coderabbit_summary_ignore_groups", return_value=["Improvements"]) + mocker.patch( + "release_notes_generator.action_inputs.ActionInputs.get_coderabbit_release_notes_title", + return_value="Summary by CodeRabbit", + ) + mocker.patch( + "release_notes_generator.action_inputs.ActionInputs.get_coderabbit_summary_ignore_groups", + return_value=["Improvements"], + ) notes = rec.get_rls_notes() assert notes == " - Crash fix\n + Minor patch" + # get_commit + def test_get_commit_found_and_missing(pull_request_record_merged): found = pull_request_record_merged.get_commit("merge_commit_sha") missing = pull_request_record_merged.get_commit("nonexistent") diff --git a/tests/unit/release_notes_generator/model/test_record.py b/tests/unit/release_notes_generator/model/test_record.py index 22bf44af..8e0021a9 100644 --- a/tests/unit/release_notes_generator/model/test_record.py +++ b/tests/unit/release_notes_generator/model/test_record.py @@ -19,7 +19,9 @@ class DummyRecord(Record): - def __init__(self, skip=False, labels=None, authors=None, closed=True, record_id=1, rls_notes: Optional[str]="notes"): + def __init__( + self, skip=False, labels=None, authors=None, closed=True, record_id=1, rls_notes: Optional[str] = "notes" + ): super().__init__(labels, skip) self._labels = labels or ["bug", "feature"] self._authors = authors or ["alice", "bob"] @@ -67,18 +69,21 @@ def get_labels(self) -> list[str]: def contains_change_increment(self) -> bool: return True + def test_is_present_in_chapters(): rec = DummyRecord() assert not rec.is_present_in_chapters rec.add_to_chapter_presence("chapter1") assert rec.is_present_in_chapters + def test_skip_property(): rec = DummyRecord(skip=True) assert rec.skip is True rec2 = DummyRecord(skip=False) assert rec2.skip is False + def test_present_in_chapters_count(): rec = DummyRecord() assert rec.chapter_presence_count() == 0 @@ -86,6 +91,7 @@ def test_present_in_chapters_count(): rec.add_to_chapter_presence("chapter2") assert rec.chapter_presence_count() == 2 + def test_present_in_chapters_unique(): """Test that adding the same chapter multiple times doesn't increase the count.""" rec = DummyRecord() @@ -96,23 +102,27 @@ def test_present_in_chapters_unique(): rec.add_to_chapter_presence("chapter2") assert rec.chapter_presence_count() == 2 + def test_contains_min_one_label(): rec = DummyRecord(labels=["bug", "feature"]) assert rec.contains_min_one_label(["bug"]) assert not rec.contains_min_one_label(["enhancement"]) + def test_contain_all_labels(): rec = DummyRecord(labels=["bug", "feature"]) assert rec.contain_all_labels(["bug", "feature"]) assert not rec.contain_all_labels(["bug", "other"]) assert rec.contain_all_labels(["bug"]) + def test_contains_release_notes_true(): rec = DummyRecord(rls_notes="Some notes") assert rec.contains_release_notes() is True assert rec.contains_release_notes(re_check=True) is True assert rec.contains_release_notes() is True + def test_contains_release_notes_false(): rec = DummyRecord(rls_notes=None) assert rec.contains_release_notes() is False diff --git a/tests/unit/release_notes_generator/record/factory/test_default_record_factory.py b/tests/unit/release_notes_generator/record/factory/test_default_record_factory.py index d38d4610..7da947fa 100644 --- a/tests/unit/release_notes_generator/record/factory/test_default_record_factory.py +++ b/tests/unit/release_notes_generator/record/factory/test_default_record_factory.py @@ -31,7 +31,6 @@ from release_notes_generator.record.factory.default_record_factory import DefaultRecordFactory from tests.unit.conftest import mock_safe_call_decorator - # generate - non hierarchy issue records @@ -166,15 +165,20 @@ def setup_issues_pulls_commits(mocker, mock_repo): return mock_git_issue1, mock_git_issue2, mock_git_pr1, mock_git_pr2, mock_git_commit1, mock_git_commit2 + def mock_get_issues_for_pr(pull_number: int) -> list[str]: if pull_number == 101: - return ['org/repo#1'] + return ["org/repo#1"] elif pull_number == 102: - return ['org/repo#2'] + return ["org/repo#2"] return [] + def test_generate_with_issues_and_pulls_and_commits(mocker, mock_repo): - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator, + ) mock_github_client = mocker.Mock(spec=Github) issue1, _issue2, pr1, _pr2, commit1, commit2 = setup_issues_pulls_commits(mocker, mock_repo) @@ -196,23 +200,30 @@ def test_generate_with_issues_and_pulls_and_commits(mocker, mock_repo): # Check if records for issues and PRs were created assert len(records) == 3 - assert 'org/repo#1' in records - assert not records['org/repo#1'].skip + assert "org/repo#1" in records + assert not records["org/repo#1"].skip # Verify the record creation - assert records['org/repo#1'].__class__ is IssueRecord - rec_i1 = cast(IssueRecord, records['org/repo#1']) + assert records["org/repo#1"].__class__ is IssueRecord + rec_i1 = cast(IssueRecord, records["org/repo#1"]) # Verify that PRs are registered assert 1 == rec_i1.pull_requests_count() assert pr1 == rec_i1.get_pull_request(101) # Verify that commits are registered - assert commit1 == rec_i1.get_commit(101, 'abc123') + assert commit1 == rec_i1.get_commit(101, "abc123") + def test_generate_with_issues_and_pulls_and_commits_with_skip_labels(mocker, mock_repo): - mocker.patch("release_notes_generator.record.factory.default_record_factory.ActionInputs.get_skip_release_notes_labels", return_value=["skip-release-notes"]) - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.ActionInputs.get_skip_release_notes_labels", + return_value=["skip-release-notes"], + ) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator, + ) mock_github_client = mocker.Mock(spec=Github) issue1, issue2, pr1, pr2, commit1, commit2 = setup_issues_pulls_commits(mocker, mock_repo) @@ -236,20 +247,22 @@ def test_generate_with_issues_and_pulls_and_commits_with_skip_labels(mocker, moc # Check if records for issues and PRs were created assert len(records) == 3 - assert 'org/repo#1' in records - assert 'org/repo#2' in records - assert 'ghi789' in records + assert "org/repo#1" in records + assert "org/repo#2" in records + assert "ghi789" in records # Verify the record creation - assert isinstance(records['org/repo#1'], IssueRecord) - assert isinstance(records['org/repo#2'], IssueRecord) - assert isinstance(records['ghi789'], CommitRecord) + assert isinstance(records["org/repo#1"], IssueRecord) + assert isinstance(records["org/repo#2"], IssueRecord) + assert isinstance(records["ghi789"], CommitRecord) - assert records['org/repo#1'].skip # skip label applied to issue as the record was created from issue - assert not records['org/repo#2'].skip # skip label is present only on inner PR but record create from issues (leading) + assert records["org/repo#1"].skip # skip label applied to issue as the record was created from issue + assert not records[ + "org/repo#2" + ].skip # skip label is present only on inner PR but record create from issues (leading) - rec_i1 = cast(IssueRecord, records['org/repo#1']) - rec_i2 = cast(IssueRecord, records['org/repo#2']) + rec_i1 = cast(IssueRecord, records["org/repo#1"]) + rec_i2 = cast(IssueRecord, records["org/repo#2"]) # Verify that PRs are registered assert 1 == rec_i1.pull_requests_count() @@ -259,9 +272,9 @@ def test_generate_with_issues_and_pulls_and_commits_with_skip_labels(mocker, moc assert pr2 == rec_i2.get_pull_request(102) # Verify that commits are registered - assert commit1 == rec_i1.get_commit(101, 'abc123') - assert commit2 == rec_i2.get_commit(102, 'def456') - assert commit3 == cast(CommitRecord, records['ghi789']).commit + assert commit1 == rec_i1.get_commit(101, "abc123") + assert commit2 == rec_i2.get_commit(102, "def456") + assert commit3 == cast(CommitRecord, records["ghi789"]).commit def test_generate_with_no_commits(mocker, mock_repo): @@ -279,17 +292,19 @@ def test_generate_with_no_commits(mocker, mock_repo): mock_repo.get_issue.return_value = issue2 data.commits = {} # No commits - mocker.patch("release_notes_generator.record.factory.default_record_factory.get_issues_for_pr", return_value={'org/repo#2'}) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.get_issues_for_pr", return_value={"org/repo#2"} + ) records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) assert 2 == len(records) # Verify the record creation - assert isinstance(records['org/repo#1'], IssueRecord) - assert isinstance(records['org/repo#2'], IssueRecord) + assert isinstance(records["org/repo#1"], IssueRecord) + assert isinstance(records["org/repo#2"], IssueRecord) - rec_issue1 = cast(IssueRecord,records['org/repo#1']) - rec_issue2 = cast(IssueRecord,records['org/repo#2']) + rec_issue1 = cast(IssueRecord, records["org/repo#1"]) + rec_issue2 = cast(IssueRecord, records["org/repo#2"]) # Verify that PRs are registered assert 1 == rec_issue1.pull_requests_count() @@ -297,6 +312,7 @@ def test_generate_with_no_commits(mocker, mock_repo): assert pr1 == rec_issue2.get_pull_request(101) + def test_generate_with_no_commits_with_wrong_issue_number_in_pull_body_mention(mocker, mock_repo): mock_github_client = mocker.Mock(spec=Github) data = MinedData(mock_repo) @@ -313,17 +329,19 @@ def test_generate_with_no_commits_with_wrong_issue_number_in_pull_body_mention(m mock_repo.get_issue.return_value = issue2 data.commits = {} # No commits - mocker.patch("release_notes_generator.record.factory.default_record_factory.get_issues_for_pr", return_value={'org/repo#2'}) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.get_issues_for_pr", return_value={"org/repo#2"} + ) records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) assert 2 == len(records) # Verify the record creation - assert isinstance(records['org/repo#1'], IssueRecord) - assert isinstance(records['org/repo#2'], IssueRecord) + assert isinstance(records["org/repo#1"], IssueRecord) + assert isinstance(records["org/repo#2"], IssueRecord) - rec_issue1 = cast(IssueRecord, records['org/repo#1']) - rec_issue2 = cast(IssueRecord, records['org/repo#2']) + rec_issue1 = cast(IssueRecord, records["org/repo#1"]) + rec_issue2 = cast(IssueRecord, records["org/repo#2"]) # Verify that PRs are registered assert 0 == rec_issue1.pull_requests_count() @@ -331,19 +349,25 @@ def test_generate_with_no_commits_with_wrong_issue_number_in_pull_body_mention(m assert pr1 == rec_issue2.get_pull_request(101) + def mock_safe_call_decorator_no_issues(_rate_limiter): def wrapper(fn): if getattr(fn, "__name__", None) == "get_issues_for_pr": return mock_get_issues_for_pr_no_issues return fn + return wrapper + def mock_get_issues_for_pr_no_issues(pull_number: int, requester: Requester) -> list[str]: return [] def test_generate_with_no_issues(mocker, mock_repo, request): - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator_no_issues) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator_no_issues, + ) mock_github_client = mocker.Mock(spec=Github) data = MinedData(mock_repo) pr1, pr2, commit1, commit2 = setup_no_issues_pulls_commits(mocker) @@ -356,11 +380,11 @@ def test_generate_with_no_issues(mocker, mock_repo, request): # Verify the record creation assert 2 == len(records) - assert isinstance(records['org/repo#101'], PullRequestRecord) - assert isinstance(records['org/repo#102'], PullRequestRecord) + assert isinstance(records["org/repo#101"], PullRequestRecord) + assert isinstance(records["org/repo#102"], PullRequestRecord) - rec_pr1 = cast(PullRequestRecord, records['org/repo#101']) - rec_pr2 = cast(PullRequestRecord, records['org/repo#102']) + rec_pr1 = cast(PullRequestRecord, records["org/repo#101"]) + rec_pr2 = cast(PullRequestRecord, records["org/repo#102"]) # Verify that PRs are registered assert pr1 == rec_pr1.pull_request @@ -369,12 +393,19 @@ def test_generate_with_no_issues(mocker, mock_repo, request): # Verify that commits are registered assert 1 == rec_pr1.commits_count() assert 1 == rec_pr2.commits_count() - assert commit1 == rec_pr1.get_commit('abc123') - assert commit2 == rec_pr2.get_commit('def456') + assert commit1 == rec_pr1.get_commit("abc123") + assert commit2 == rec_pr2.get_commit("def456") + def test_generate_with_no_issues_skip_labels(mocker, mock_repo, request): - mocker.patch("release_notes_generator.record.factory.default_record_factory.ActionInputs.get_skip_release_notes_labels", return_value=["skip-release-notes", "another-skip-label"]) - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator_no_issues) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.ActionInputs.get_skip_release_notes_labels", + return_value=["skip-release-notes", "another-skip-label"], + ) + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator_no_issues, + ) mock_github_client = mocker.Mock(spec=Github) data = MinedData(mock_repo) pr1, pr2, commit1, commit2 = setup_no_issues_pulls_commits(mocker) @@ -397,21 +428,21 @@ def test_generate_with_no_issues_skip_labels(mocker, mock_repo, request): # Verify the record creation assert 2 == len(records) - assert isinstance(records['org/repo#101'], PullRequestRecord) - assert isinstance(records['org/repo#102'], PullRequestRecord) + assert isinstance(records["org/repo#101"], PullRequestRecord) + assert isinstance(records["org/repo#102"], PullRequestRecord) - assert records['org/repo#101'].skip - assert records['org/repo#102'].skip + assert records["org/repo#101"].skip + assert records["org/repo#102"].skip - rec_pr1 = cast(PullRequestRecord, records['org/repo#101']) - rec_pr2 = cast(PullRequestRecord, records['org/repo#102']) + rec_pr1 = cast(PullRequestRecord, records["org/repo#101"]) + rec_pr2 = cast(PullRequestRecord, records["org/repo#102"]) # Verify that PRs are registered assert 1 == rec_pr1.commits_count() assert 1 == rec_pr2.commits_count() - assert commit1 == rec_pr1.get_commit('abc123') - assert commit2 == rec_pr2.get_commit('def456') + assert commit1 == rec_pr1.get_commit("abc123") + assert commit2 == rec_pr2.get_commit("def456") def test_generate_with_no_pulls(mocker, mock_repo): @@ -425,12 +456,12 @@ def test_generate_with_no_pulls(mocker, mock_repo): # Verify the record creation assert 2 == len(records) - assert isinstance(records['org/repo#1'], IssueRecord) - assert isinstance(records['org/repo#2'], IssueRecord) + assert isinstance(records["org/repo#1"], IssueRecord) + assert isinstance(records["org/repo#2"], IssueRecord) # Verify that PRs are registered - assert 0 == cast(IssueRecord, records['org/repo#1']).pull_requests_count() - assert 0 == cast(IssueRecord, records['org/repo#2']).pull_requests_count() + assert 0 == cast(IssueRecord, records["org/repo#1"]).pull_requests_count() + assert 0 == cast(IssueRecord, records["org/repo#2"]).pull_requests_count() def mock_get_issues_for_pr_with_wrong_issue_number(pull_number: int) -> list[int]: @@ -453,6 +484,7 @@ def test_generate_no_input_data(mocker, mock_repo): assert 0 == len(result.values()) + # - single issue record (closed) # - single hierarchy issue record - two sub-issues without PRs # - single hierarchy issue record - two sub-issues with PRs - no commits @@ -460,9 +492,13 @@ def test_generate_no_input_data(mocker, mock_repo): # - single hierarchy issue record - one sub hierarchy issues - two sub-issues with PRs - with commits # - single pull request record (closed, merged) # - single direct commit record -def test_generate_isolated_record_types_no_labels_no_type_defined(mocker, mock_repo, - mined_data_isolated_record_types_no_labels_no_type_defined): - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) +def test_generate_isolated_record_types_no_labels_no_type_defined( + mocker, mock_repo, mined_data_isolated_record_types_no_labels_no_type_defined +): + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator, + ) mock_github_client = mocker.Mock(spec=Github) mock_rate_limit = mocker.Mock() @@ -475,52 +511,70 @@ def test_generate_isolated_record_types_no_labels_no_type_defined(mocker, mock_r result = factory.generate(mined_data_isolated_record_types_no_labels_no_type_defined) assert 8 == len(result) - assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "merge_commit_sha_direct"}.issubset(result.keys()) - - assert isinstance(result['org/repo#121'], IssueRecord) - assert isinstance(result['org/repo#301'], HierarchyIssueRecord) - assert isinstance(result['org/repo#302'], HierarchyIssueRecord) - assert isinstance(result['org/repo#303'], HierarchyIssueRecord) - assert isinstance(result['org/repo#304'], HierarchyIssueRecord) - assert isinstance(result['org/repo#123'], PullRequestRecord) - assert isinstance(result['org/repo#124'], PullRequestRecord) + assert { + "org/repo#121", + "org/repo#301", + "org/repo#302", + "org/repo#303", + "org/repo#304", + "org/repo#123", + "org/repo#124", + "merge_commit_sha_direct", + }.issubset(result.keys()) + + assert isinstance(result["org/repo#121"], IssueRecord) + assert isinstance(result["org/repo#301"], HierarchyIssueRecord) + assert isinstance(result["org/repo#302"], HierarchyIssueRecord) + assert isinstance(result["org/repo#303"], HierarchyIssueRecord) + assert isinstance(result["org/repo#304"], HierarchyIssueRecord) + assert isinstance(result["org/repo#123"], PullRequestRecord) + assert isinstance(result["org/repo#124"], PullRequestRecord) assert isinstance(result["merge_commit_sha_direct"], CommitRecord) - rec_i = cast(IssueRecord, result['org/repo#121']) + rec_i = cast(IssueRecord, result["org/repo#121"]) assert 0 == rec_i.pull_requests_count() - rec_hi_1 = cast(HierarchyIssueRecord, result['org/repo#301']) + rec_hi_1 = cast(HierarchyIssueRecord, result["org/repo#301"]) assert 0 == rec_hi_1.pull_requests_count() assert 0 == len(rec_hi_1.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_1.sub_issues.values()) - assert 0 == rec_hi_1.sub_issues['org/repo#450'].pull_requests_count() + assert 0 == rec_hi_1.sub_issues["org/repo#450"].pull_requests_count() assert 0 == rec_hi_1.level - rec_hi_2 = cast(HierarchyIssueRecord, result['org/repo#302']) + rec_hi_2 = cast(HierarchyIssueRecord, result["org/repo#302"]) assert 1 == rec_hi_2.pull_requests_count() assert 0 == len(rec_hi_2.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_2.sub_issues.values()) - assert 1 == rec_hi_2.sub_issues['org/repo#451'].pull_requests_count() + assert 1 == rec_hi_2.sub_issues["org/repo#451"].pull_requests_count() assert 0 == rec_hi_2.level - rec_hi_3 = cast(HierarchyIssueRecord, result['org/repo#303']) + rec_hi_3 = cast(HierarchyIssueRecord, result["org/repo#303"]) assert 1 == rec_hi_3.pull_requests_count() assert 0 == len(rec_hi_3.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_3.sub_issues.values()) - assert 1 == rec_hi_3.sub_issues['org/repo#452'].pull_requests_count() - assert "Fixed bug in PR 151" == rec_hi_3.sub_issues['org/repo#452'].get_commit(151, "merge_commit_sha_151").commit.message + assert 1 == rec_hi_3.sub_issues["org/repo#452"].pull_requests_count() + assert ( + "Fixed bug in PR 151" + == rec_hi_3.sub_issues["org/repo#452"].get_commit(151, "merge_commit_sha_151").commit.message + ) assert 0 == rec_hi_3.level - rec_hi_4 = cast(HierarchyIssueRecord, result['org/repo#304']) + rec_hi_4 = cast(HierarchyIssueRecord, result["org/repo#304"]) assert 1 == rec_hi_4.pull_requests_count() assert 1 == len(rec_hi_4.sub_hierarchy_issues.values()) assert 0 == len(rec_hi_4.sub_issues.values()) assert 1 == rec_hi_4.pull_requests_count() - assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues['org/repo#350'].sub_issues['org/repo#453'].get_commit(152, "merge_commit_sha_152").commit.message + assert ( + "Fixed bug in PR 152" + == rec_hi_4.sub_hierarchy_issues["org/repo#350"] + .sub_issues["org/repo#453"] + .get_commit(152, "merge_commit_sha_152") + .commit.message + ) assert 0 == rec_hi_4.level - rec_hi_5 = cast(HierarchyIssueRecord, result['org/repo#304']) - assert 1 == rec_hi_5.sub_hierarchy_issues['org/repo#350'].level + rec_hi_5 = cast(HierarchyIssueRecord, result["org/repo#304"]) + assert 1 == rec_hi_5.sub_hierarchy_issues["org/repo#350"].level # - single issue record (closed) @@ -530,9 +584,13 @@ def test_generate_isolated_record_types_no_labels_no_type_defined(mocker, mock_r # - single hierarchy issue record - one sub hierarchy issues - two sub-issues with PRs - with commits # - single pull request record (closed, merged) # - single direct commit record -def test_generate_isolated_record_types_with_labels_no_type_defined(mocker, mock_repo, - mined_data_isolated_record_types_with_labels_no_type_defined): - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) +def test_generate_isolated_record_types_with_labels_no_type_defined( + mocker, mock_repo, mined_data_isolated_record_types_with_labels_no_type_defined +): + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator, + ) mock_github_client = mocker.Mock(spec=Github) mock_rate_limit = mocker.Mock() @@ -545,45 +603,63 @@ def test_generate_isolated_record_types_with_labels_no_type_defined(mocker, mock result = factory.generate(mined_data_isolated_record_types_with_labels_no_type_defined) assert 8 == len(result) - assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "merge_commit_sha_direct"}.issubset(result.keys()) - - assert isinstance(result['org/repo#121'], IssueRecord) - assert isinstance(result['org/repo#301'], HierarchyIssueRecord) - assert isinstance(result['org/repo#302'], HierarchyIssueRecord) - assert isinstance(result['org/repo#303'], HierarchyIssueRecord) - assert isinstance(result['org/repo#304'], HierarchyIssueRecord) - assert isinstance(result['org/repo#123'], PullRequestRecord) - assert isinstance(result['org/repo#124'], PullRequestRecord) + assert { + "org/repo#121", + "org/repo#301", + "org/repo#302", + "org/repo#303", + "org/repo#304", + "org/repo#123", + "org/repo#124", + "merge_commit_sha_direct", + }.issubset(result.keys()) + + assert isinstance(result["org/repo#121"], IssueRecord) + assert isinstance(result["org/repo#301"], HierarchyIssueRecord) + assert isinstance(result["org/repo#302"], HierarchyIssueRecord) + assert isinstance(result["org/repo#303"], HierarchyIssueRecord) + assert isinstance(result["org/repo#304"], HierarchyIssueRecord) + assert isinstance(result["org/repo#123"], PullRequestRecord) + assert isinstance(result["org/repo#124"], PullRequestRecord) assert isinstance(result["merge_commit_sha_direct"], CommitRecord) - rec_i = cast(IssueRecord, result['org/repo#121']) + rec_i = cast(IssueRecord, result["org/repo#121"]) assert 0 == rec_i.pull_requests_count() - rec_hi_1 = cast(HierarchyIssueRecord, result['org/repo#301']) + rec_hi_1 = cast(HierarchyIssueRecord, result["org/repo#301"]) assert 0 == rec_hi_1.pull_requests_count() assert 0 == len(rec_hi_1.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_1.sub_issues.values()) - assert 0 == rec_hi_1.sub_issues['org/repo#450'].pull_requests_count() + assert 0 == rec_hi_1.sub_issues["org/repo#450"].pull_requests_count() - rec_hi_2 = cast(HierarchyIssueRecord, result['org/repo#302']) + rec_hi_2 = cast(HierarchyIssueRecord, result["org/repo#302"]) assert 1 == rec_hi_2.pull_requests_count() assert 0 == len(rec_hi_2.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_2.sub_issues.values()) - assert 1 == rec_hi_2.sub_issues['org/repo#451'].pull_requests_count() + assert 1 == rec_hi_2.sub_issues["org/repo#451"].pull_requests_count() - rec_hi_3 = cast(HierarchyIssueRecord, result['org/repo#303']) + rec_hi_3 = cast(HierarchyIssueRecord, result["org/repo#303"]) assert 1 == rec_hi_3.pull_requests_count() assert 0 == len(rec_hi_3.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_3.sub_issues.values()) - assert 1 == rec_hi_3.sub_issues['org/repo#452'].pull_requests_count() - assert "Fixed bug in PR 151" == rec_hi_3.sub_issues['org/repo#452'].get_commit(151, "merge_commit_sha_151").commit.message + assert 1 == rec_hi_3.sub_issues["org/repo#452"].pull_requests_count() + assert ( + "Fixed bug in PR 151" + == rec_hi_3.sub_issues["org/repo#452"].get_commit(151, "merge_commit_sha_151").commit.message + ) - rec_hi_4 = cast(HierarchyIssueRecord, result['org/repo#304']) + rec_hi_4 = cast(HierarchyIssueRecord, result["org/repo#304"]) assert 1 == rec_hi_4.pull_requests_count() assert 1 == len(rec_hi_4.sub_hierarchy_issues.values()) assert 0 == len(rec_hi_4.sub_issues.values()) assert 1 == rec_hi_4.pull_requests_count() - assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues['org/repo#350'].sub_issues['org/repo#453'].get_commit(152, "merge_commit_sha_152").commit.message + assert ( + "Fixed bug in PR 152" + == rec_hi_4.sub_hierarchy_issues["org/repo#350"] + .sub_issues["org/repo#453"] + .get_commit(152, "merge_commit_sha_152") + .commit.message + ) # - single issue record (closed) @@ -593,9 +669,13 @@ def test_generate_isolated_record_types_with_labels_no_type_defined(mocker, mock # - single hierarchy issue record - one sub hierarchy issues - two sub-issues with PRs - with commits # - single pull request record (closed, merged) # - single direct commit record -def test_generate_isolated_record_types_no_labels_with_type_defined(mocker, mock_repo, - mined_data_isolated_record_types_no_labels_with_type_defined): - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) +def test_generate_isolated_record_types_no_labels_with_type_defined( + mocker, mock_repo, mined_data_isolated_record_types_no_labels_with_type_defined +): + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator, + ) mock_github_client = mocker.Mock(spec=Github) mock_rate_limit = mocker.Mock() @@ -608,45 +688,63 @@ def test_generate_isolated_record_types_no_labels_with_type_defined(mocker, mock result = factory.generate(mined_data_isolated_record_types_no_labels_with_type_defined) assert 8 == len(result) - assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "merge_commit_sha_direct"}.issubset(result.keys()) - - assert isinstance(result['org/repo#121'], IssueRecord) - assert isinstance(result['org/repo#301'], HierarchyIssueRecord) - assert isinstance(result['org/repo#302'], HierarchyIssueRecord) - assert isinstance(result['org/repo#303'], HierarchyIssueRecord) - assert isinstance(result['org/repo#304'], HierarchyIssueRecord) - assert isinstance(result['org/repo#123'], PullRequestRecord) - assert isinstance(result['org/repo#124'], PullRequestRecord) + assert { + "org/repo#121", + "org/repo#301", + "org/repo#302", + "org/repo#303", + "org/repo#304", + "org/repo#123", + "org/repo#124", + "merge_commit_sha_direct", + }.issubset(result.keys()) + + assert isinstance(result["org/repo#121"], IssueRecord) + assert isinstance(result["org/repo#301"], HierarchyIssueRecord) + assert isinstance(result["org/repo#302"], HierarchyIssueRecord) + assert isinstance(result["org/repo#303"], HierarchyIssueRecord) + assert isinstance(result["org/repo#304"], HierarchyIssueRecord) + assert isinstance(result["org/repo#123"], PullRequestRecord) + assert isinstance(result["org/repo#124"], PullRequestRecord) assert isinstance(result["merge_commit_sha_direct"], CommitRecord) - rec_i = cast(IssueRecord, result['org/repo#121']) + rec_i = cast(IssueRecord, result["org/repo#121"]) assert 0 == rec_i.pull_requests_count() - rec_hi_1 = cast(HierarchyIssueRecord, result['org/repo#301']) + rec_hi_1 = cast(HierarchyIssueRecord, result["org/repo#301"]) assert 0 == rec_hi_1.pull_requests_count() assert 0 == len(rec_hi_1.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_1.sub_issues.values()) - assert 0 == rec_hi_1.sub_issues['org/repo#450'].pull_requests_count() + assert 0 == rec_hi_1.sub_issues["org/repo#450"].pull_requests_count() - rec_hi_2 = cast(HierarchyIssueRecord, result['org/repo#302']) + rec_hi_2 = cast(HierarchyIssueRecord, result["org/repo#302"]) assert 1 == rec_hi_2.pull_requests_count() assert 0 == len(rec_hi_2.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_2.sub_issues.values()) - assert 1 == rec_hi_2.sub_issues['org/repo#451'].pull_requests_count() + assert 1 == rec_hi_2.sub_issues["org/repo#451"].pull_requests_count() - rec_hi_3 = cast(HierarchyIssueRecord, result['org/repo#303']) + rec_hi_3 = cast(HierarchyIssueRecord, result["org/repo#303"]) assert 1 == rec_hi_3.pull_requests_count() assert 0 == len(rec_hi_3.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_3.sub_issues.values()) - assert 1 == rec_hi_3.sub_issues['org/repo#452'].pull_requests_count() - assert "Fixed bug in PR 151" == rec_hi_3.sub_issues['org/repo#452'].get_commit(151, "merge_commit_sha_151").commit.message + assert 1 == rec_hi_3.sub_issues["org/repo#452"].pull_requests_count() + assert ( + "Fixed bug in PR 151" + == rec_hi_3.sub_issues["org/repo#452"].get_commit(151, "merge_commit_sha_151").commit.message + ) - rec_hi_4 = cast(HierarchyIssueRecord, result['org/repo#304']) + rec_hi_4 = cast(HierarchyIssueRecord, result["org/repo#304"]) assert 1 == rec_hi_4.pull_requests_count() assert 1 == len(rec_hi_4.sub_hierarchy_issues.values()) assert 0 == len(rec_hi_4.sub_issues.values()) assert 1 == rec_hi_4.pull_requests_count() - assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues['org/repo#350'].sub_issues['org/repo#453'].get_commit(152, "merge_commit_sha_152").commit.message + assert ( + "Fixed bug in PR 152" + == rec_hi_4.sub_hierarchy_issues["org/repo#350"] + .sub_issues["org/repo#453"] + .get_commit(152, "merge_commit_sha_152") + .commit.message + ) # - single issue record (closed) @@ -656,9 +754,13 @@ def test_generate_isolated_record_types_no_labels_with_type_defined(mocker, mock # - single hierarchy issue record - one sub hierarchy issues - two sub-issues with PRs - with commits # - single pull request record (closed, merged) # - single direct commit record -def test_generate_isolated_record_types_with_labels_with_type_defined(mocker, mock_repo, - mined_data_isolated_record_types_with_labels_with_type_defined): - mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) +def test_generate_isolated_record_types_with_labels_with_type_defined( + mocker, mock_repo, mined_data_isolated_record_types_with_labels_with_type_defined +): + mocker.patch( + "release_notes_generator.record.factory.default_record_factory.safe_call_decorator", + side_effect=mock_safe_call_decorator, + ) mock_github_client = mocker.Mock(spec=Github) mock_rate_limit = mocker.Mock() @@ -671,42 +773,60 @@ def test_generate_isolated_record_types_with_labels_with_type_defined(mocker, mo result = factory.generate(mined_data_isolated_record_types_with_labels_with_type_defined) assert 8 == len(result) - assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "merge_commit_sha_direct"}.issubset(result.keys()) - - assert isinstance(result['org/repo#121'], IssueRecord) - assert isinstance(result['org/repo#301'], HierarchyIssueRecord) - assert isinstance(result['org/repo#302'], HierarchyIssueRecord) - assert isinstance(result['org/repo#303'], HierarchyIssueRecord) - assert isinstance(result['org/repo#304'], HierarchyIssueRecord) - assert isinstance(result['org/repo#123'], PullRequestRecord) - assert isinstance(result['org/repo#124'], PullRequestRecord) + assert { + "org/repo#121", + "org/repo#301", + "org/repo#302", + "org/repo#303", + "org/repo#304", + "org/repo#123", + "org/repo#124", + "merge_commit_sha_direct", + }.issubset(result.keys()) + + assert isinstance(result["org/repo#121"], IssueRecord) + assert isinstance(result["org/repo#301"], HierarchyIssueRecord) + assert isinstance(result["org/repo#302"], HierarchyIssueRecord) + assert isinstance(result["org/repo#303"], HierarchyIssueRecord) + assert isinstance(result["org/repo#304"], HierarchyIssueRecord) + assert isinstance(result["org/repo#123"], PullRequestRecord) + assert isinstance(result["org/repo#124"], PullRequestRecord) assert isinstance(result["merge_commit_sha_direct"], CommitRecord) - rec_i = cast(IssueRecord, result['org/repo#121']) + rec_i = cast(IssueRecord, result["org/repo#121"]) assert 0 == rec_i.pull_requests_count() - rec_hi_1 = cast(HierarchyIssueRecord, result['org/repo#301']) + rec_hi_1 = cast(HierarchyIssueRecord, result["org/repo#301"]) assert 0 == rec_hi_1.pull_requests_count() assert 0 == len(rec_hi_1.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_1.sub_issues.values()) - assert 0 == rec_hi_1.sub_issues['org/repo#450'].pull_requests_count() + assert 0 == rec_hi_1.sub_issues["org/repo#450"].pull_requests_count() - rec_hi_2 = cast(HierarchyIssueRecord, result['org/repo#302']) + rec_hi_2 = cast(HierarchyIssueRecord, result["org/repo#302"]) assert 1 == rec_hi_2.pull_requests_count() assert 0 == len(rec_hi_2.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_2.sub_issues.values()) - assert 1 == rec_hi_2.sub_issues['org/repo#451'].pull_requests_count() + assert 1 == rec_hi_2.sub_issues["org/repo#451"].pull_requests_count() - rec_hi_3 = cast(HierarchyIssueRecord, result['org/repo#303']) + rec_hi_3 = cast(HierarchyIssueRecord, result["org/repo#303"]) assert 1 == rec_hi_3.pull_requests_count() assert 0 == len(rec_hi_3.sub_hierarchy_issues.values()) assert 2 == len(rec_hi_3.sub_issues.values()) - assert 1 == rec_hi_3.sub_issues['org/repo#452'].pull_requests_count() - assert "Fixed bug in PR 151" == rec_hi_3.sub_issues['org/repo#452'].get_commit(151, "merge_commit_sha_151").commit.message + assert 1 == rec_hi_3.sub_issues["org/repo#452"].pull_requests_count() + assert ( + "Fixed bug in PR 151" + == rec_hi_3.sub_issues["org/repo#452"].get_commit(151, "merge_commit_sha_151").commit.message + ) - rec_hi_4 = cast(HierarchyIssueRecord, result['org/repo#304']) + rec_hi_4 = cast(HierarchyIssueRecord, result["org/repo#304"]) assert 1 == rec_hi_4.pull_requests_count() assert 1 == len(rec_hi_4.sub_hierarchy_issues.values()) assert 0 == len(rec_hi_4.sub_issues.values()) assert 1 == rec_hi_4.pull_requests_count() - assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues['org/repo#350'].sub_issues['org/repo#453'].get_commit(152, "merge_commit_sha_152").commit.message + assert ( + "Fixed bug in PR 152" + == rec_hi_4.sub_hierarchy_issues["org/repo#350"] + .sub_issues["org/repo#453"] + .get_commit(152, "merge_commit_sha_152") + .commit.message + ) diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index 42bc864a..d7c981dd 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -62,12 +62,21 @@ ("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_release_notes_title", "", "Release Notes title must be a non-empty string and have non-zero length."), - ("get_coderabbit_release_notes_title", "", "CodeRabbit Release Notes title must be a non-empty string and have non-zero length."), - ("get_coderabbit_summary_ignore_groups", [""], "CodeRabbit summary ignore groups must be a non-empty string and have non-zero length."), + ( + "get_coderabbit_release_notes_title", + "", + "CodeRabbit Release Notes title must be a non-empty string and have non-zero length.", + ), + ( + "get_coderabbit_summary_ignore_groups", + [""], + "CodeRabbit summary ignore groups must be a non-empty string and have non-zero length.", + ), ("get_row_format_link_pr", "not_bool", "'row-format-link-pr' value must be a boolean."), ("get_hierarchy", "not_bool", "Hierarchy must be a boolean."), ] + def apply_mocks(case, mocker): patchers = [] for key, value in case.items(): @@ -76,10 +85,12 @@ def apply_mocks(case, mocker): patchers.append(patcher) return patchers + def stop_mocks(patchers): for patcher in patchers: patcher.stop() + def test_validate_inputs_success(mocker): patchers = apply_mocks(success_case, mocker) try: @@ -88,6 +99,7 @@ def test_validate_inputs_success(mocker): finally: stop_mocks(patchers) + @pytest.mark.parametrize("method, value, expected_error", failure_cases) def test_validate_inputs_failure(method, value, expected_error, mocker): case = success_case.copy() @@ -102,130 +114,171 @@ def test_validate_inputs_failure(method, value, expected_error, mocker): finally: stop_mocks(patchers) + def test_get_github_repository(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="owner/repo") assert "owner/repo" == ActionInputs.get_github_repository() + def test_get_github_token(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="fake-token") assert ActionInputs.get_github_token() == "fake-token" + def test_get_tag_name_version_full(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="v1.0.0") assert ActionInputs.get_tag_name() == "v1.0.0" + def test_get_tag_name_version_shorted_with_v(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="v1.2") assert ActionInputs.get_tag_name() == "v1.2.0" + def test_get_tag_name_version_shorted_no_v(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="1.2") assert ActionInputs.get_tag_name() == "v1.2.0" + def test_get_tag_name_empty(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="") assert ActionInputs.get_tag_name() == "" + def test_get_tag_name_invalid_format(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="v1.2.beta") with pytest.raises(ValueError) as excinfo: ActionInputs.get_tag_name() - assert "Invalid version tag format: 'v1.2.beta'. Expected vMAJOR.MINOR[.PATCH], e.g. 'v0.2' or 'v0.2.0'." in str(excinfo.value) + assert "Invalid version tag format: 'v1.2.beta'. Expected vMAJOR.MINOR[.PATCH], e.g. 'v0.2' or 'v0.2.0'." in str( + excinfo.value + ) + def test_get_tag_from_name_version_full(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="v1.0.0") assert ActionInputs.get_from_tag_name() == "v1.0.0" + def test_get_from_tag_name_version_shorted_with_v(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="v1.2") assert ActionInputs.get_from_tag_name() == "v1.2.0" + def test_get_from_tag_name_version_shorted_no_v(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="1.2") assert ActionInputs.get_from_tag_name() == "v1.2.0" + def test_get_from_tag_name_empty(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="") assert ActionInputs.get_from_tag_name() == "" + def test_get_from_tag_name_invalid_format(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="v1.2.beta") with pytest.raises(ValueError) as excinfo: ActionInputs.get_from_tag_name() - assert "Invalid version tag format: 'v1.2.beta'. Expected vMAJOR.MINOR[.PATCH], e.g. 'v0.2' or 'v0.2.0'." in str(excinfo.value) + assert "Invalid version tag format: 'v1.2.beta'. Expected vMAJOR.MINOR[.PATCH], e.g. 'v0.2' or 'v0.2.0'." in str( + excinfo.value + ) + def test_get_chapters_success(mocker): - mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="[{\"title\": \"Title\", \"label\": \"Label\"}]") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", return_value='[{"title": "Title", "label": "Label"}]' + ) assert ActionInputs.get_chapters() == [{"title": "Title", "label": "Label"}] + def test_get_chapters_exception(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="wrong value") assert [] == ActionInputs.get_chapters() + def test_get_chapters_yaml_error(mocker): - mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="[{\"title\": \"Title\" \"label\": \"Label\"}]") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", return_value='[{"title": "Title" "label": "Label"}]' + ) assert [] == ActionInputs.get_chapters() + def test_get_warnings(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="true") assert ActionInputs.get_warnings() is True + def test_get_published_at(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="false") assert ActionInputs.get_published_at() is False + def test_get_skip_release_notes_label(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="skip-release-notes") assert ActionInputs.get_skip_release_notes_labels() == ["skip-release-notes"] + def test_get_skip_release_notes_label_not_defined(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="") assert ActionInputs.get_skip_release_notes_labels() == ["skip-release-notes"] + def test_get_skip_release_notes_labels(mocker): - mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="skip-release-notes, another-skip-label") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", return_value="skip-release-notes, another-skip-label" + ) assert ActionInputs.get_skip_release_notes_labels() == ["skip-release-notes", "another-skip-label"] + def test_get_skip_release_notes_labels_no_space(mocker): - mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="skip-release-notes,another-skip-label") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", return_value="skip-release-notes,another-skip-label" + ) assert ActionInputs.get_skip_release_notes_labels() == ["skip-release-notes", "another-skip-label"] + def test_get_print_empty_chapters(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="true") assert ActionInputs.get_print_empty_chapters() is True + def test_get_verbose_verbose_by_action_input(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="true") mocker.patch("os.getenv", return_value=0) assert ActionInputs.get_verbose() is True + def test_get_verbose_verbose_by_workflow_debug(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="false") mocker.patch("os.getenv", return_value="1") assert ActionInputs.get_verbose() is True + def test_get_verbose_verbose_by_both(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="true") mocker.patch("os.getenv", return_value=1) assert ActionInputs.get_verbose() is True + def test_get_verbose_not_verbose(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="false") mocker.patch("os.getenv", return_value=0) assert ActionInputs.get_verbose() is False + def test_get_duplicity_scope_wrong_value(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="huh") mock_error = mocker.patch("release_notes_generator.action_inputs.logger.error") assert ActionInputs.get_duplicity_scope() == "BOTH" mock_error.assert_called_with("Error: '%s' is not a valid DuplicityType.", "HUH") + def test_detect_row_format_invalid_keywords_no_invalid_keywords(caplog): caplog.set_level(logging.ERROR) row_format = "{number} _{title}_ in {pull-requests}" ActionInputs._detect_row_format_invalid_keywords(row_format) assert len(caplog.records) == 0 + def test_detect_row_format_invalid_keywords_with_invalid_keywords(caplog): caplog.set_level(logging.ERROR) row_format = "{number} _{title}_ in {pull-requests} {invalid_key} {another_invalid}" @@ -233,11 +286,12 @@ def test_detect_row_format_invalid_keywords_with_invalid_keywords(caplog): assert len(caplog.records) == 2 expected_errors = [ "Invalid `invalid_key` detected in `Issue` row format keyword(s) found: invalid_key, another_invalid. Will be removed from string.", - "Invalid `another_invalid` detected in `Issue` row format keyword(s) found: invalid_key, another_invalid. Will be removed from string." + "Invalid `another_invalid` detected in `Issue` row format keyword(s) found: invalid_key, another_invalid. Will be removed from string.", ] actual_errors = [record.getMessage() for record in caplog.records] assert actual_errors == expected_errors + def test_get_row_format_hierarchy_issue_cleans_invalid_keywords(mocker, caplog): caplog.set_level(logging.ERROR) mocker.patch( @@ -247,41 +301,51 @@ def test_get_row_format_hierarchy_issue_cleans_invalid_keywords(mocker, caplog): fmt = ActionInputs.get_row_format_hierarchy_issue() assert "{invalid}" not in fmt + def test_clean_row_format_invalid_keywords_no_keywords(): expected_row_format = "{number} _{title}_ in {pull-requests}" actual_format = ActionInputs._detect_row_format_invalid_keywords(expected_row_format, clean=True) assert expected_row_format == actual_format + def test_clean_row_format_invalid_keywords_nested_braces(): row_format = "{number} _{title}_ in {pull-requests} {invalid_key} {another_invalid}" expected_format = "{number} _{title}_ in {pull-requests} " actual_format = ActionInputs._detect_row_format_invalid_keywords(row_format, clean=True) assert expected_format == actual_format + def test_release_notes_title_default(): assert ActionInputs.get_release_notes_title() == "[Rr]elease [Nn]otes:" + def test_release_notes_title_custom(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="Custom Title") assert ActionInputs.get_release_notes_title() == "Custom Title" + def test_coderabbit_support_active_default(mocker): assert not ActionInputs.is_coderabbit_support_active() + def test_coderabbit_release_notes_title_default(): assert ActionInputs.get_coderabbit_release_notes_title() == "Summary by CodeRabbit" + def test_coderabbit_release_notes_title_custom(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="Custom Title") assert ActionInputs.get_coderabbit_release_notes_title() == "Custom Title" + def test_coderabbit_summary_ignore_groups_default(): assert ActionInputs.get_coderabbit_summary_ignore_groups() == [] + def test_coderabbit_summary_ignore_groups_custom(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="Group1\nGroup2") assert ActionInputs.get_coderabbit_summary_ignore_groups() == ["Group1", "Group2"] + def test_coderabbit_summary_ignore_groups_int_input(mocker): mock_log_error = mocker.patch("release_notes_generator.action_inputs.logger.error") mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value=1) @@ -289,31 +353,45 @@ def test_coderabbit_summary_ignore_groups_int_input(mocker): mock_log_error.assert_called_once() assert "coderabbit_summary_ignore_groups' is not a valid string" in mock_log_error.call_args[0][0] + def test_coderabbit_summary_ignore_groups_empty_group_input(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value=",") # Note: this is not valid input which is catched by the validation_inputs() method - assert ActionInputs.get_coderabbit_summary_ignore_groups() == ['', ''] + assert ActionInputs.get_coderabbit_summary_ignore_groups() == ["", ""] + def test_get_hidden_service_chapters_default(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="") assert ActionInputs.get_hidden_service_chapters() == [] + def test_get_hidden_service_chapters_single_comma_separated(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="Direct Commits ⚠️") assert ActionInputs.get_hidden_service_chapters() == ["Direct Commits ⚠️"] + def test_get_hidden_service_chapters_multiple_comma_separated(mocker): - mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="Direct Commits ⚠️, Others - No Topic ⚠️") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", return_value="Direct Commits ⚠️, Others - No Topic ⚠️" + ) assert ActionInputs.get_hidden_service_chapters() == ["Direct Commits ⚠️", "Others - No Topic ⚠️"] + def test_get_hidden_service_chapters_newline_separated(mocker): - mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="Direct Commits ⚠️\nOthers - No Topic ⚠️") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", return_value="Direct Commits ⚠️\nOthers - No Topic ⚠️" + ) assert ActionInputs.get_hidden_service_chapters() == ["Direct Commits ⚠️", "Others - No Topic ⚠️"] + def test_get_hidden_service_chapters_with_extra_whitespace(mocker): - mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value=" Direct Commits ⚠️ , Others - No Topic ⚠️ ") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", + return_value=" Direct Commits ⚠️ , Others - No Topic ⚠️ ", + ) assert ActionInputs.get_hidden_service_chapters() == ["Direct Commits ⚠️", "Others - No Topic ⚠️"] + def test_get_hidden_service_chapters_int_input(mocker): mock_log_error = mocker.patch("release_notes_generator.action_inputs.logger.error") mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value=123) @@ -350,7 +428,9 @@ def test_get_service_chapter_order_partial(mocker): def test_get_service_chapter_order_newline_separated(mocker): - mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value=f"{OTHERS_NO_TOPIC}\n{DIRECT_COMMITS}") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", return_value=f"{OTHERS_NO_TOPIC}\n{DIRECT_COMMITS}" + ) result = ActionInputs.get_service_chapter_order() assert result[0] == OTHERS_NO_TOPIC assert result[1] == DIRECT_COMMITS @@ -367,7 +447,9 @@ def test_get_service_chapter_order_invalid_title(mocker): def test_get_service_chapter_order_duplicate_title(mocker): mock_log_error = mocker.patch("release_notes_generator.action_inputs.logger.error") - mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value=f"{OTHERS_NO_TOPIC}, {OTHERS_NO_TOPIC}") + mocker.patch( + "release_notes_generator.action_inputs.get_action_input", return_value=f"{OTHERS_NO_TOPIC}, {OTHERS_NO_TOPIC}" + ) result = ActionInputs.get_service_chapter_order() assert result[0] == OTHERS_NO_TOPIC assert result.count(OTHERS_NO_TOPIC) == 1 @@ -388,10 +470,12 @@ def test_get_row_format_link_pr_true(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="true") assert ActionInputs.get_row_format_link_pr() is True + def test_get_row_format_link_pr_false(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="false") assert ActionInputs.get_row_format_link_pr() is False + # Mirrored test file for release_notes_generator/generator.py # Extracted from previous aggregated test_release_notes_generator.py @@ -402,6 +486,7 @@ def test_get_row_format_link_pr_false(mocker): from release_notes_generator.chapters.custom_chapters import CustomChapters from release_notes_generator.utils.constants import ROW_FORMAT_ISSUE + def test_generate_release_notes_repository_not_found(mocker): github_mock = mocker.Mock(spec=Github) github_mock.get_repo.return_value = None @@ -413,6 +498,7 @@ def test_generate_release_notes_repository_not_found(mocker): release_notes = ReleaseNotesGenerator(github_mock, custom_chapters).generate() assert release_notes is None + def test_generate_release_notes_latest_release_not_found( mocker, mock_repo, @@ -447,6 +533,7 @@ def test_generate_release_notes_latest_release_not_found( assert "- PR: #101 _Fixed bug_" in release_notes assert "- PR: #102 _Fixed bug_" in release_notes + def test_generate_release_notes_latest_release_found_by_created_at( mocker, mock_repo, @@ -490,6 +577,7 @@ def test_generate_release_notes_latest_release_found_by_created_at( assert "- PR: #101 _Fixed bug_" not in release_notes assert "- PR: #102 _Fixed bug_" in release_notes + def test_generate_release_notes_latest_release_found_by_published_at( mocker, mock_repo, @@ -529,4 +617,3 @@ def test_generate_release_notes_latest_release_found_by_published_at( assert "- #122 _I1+bug_" in release_notes assert "- PR: #101 _Fixed bug_" not in release_notes assert "- PR: #102 _Fixed bug_" in release_notes - diff --git a/tests/unit/release_notes_generator/test_generator.py b/tests/unit/release_notes_generator/test_generator.py index 7efbc58a..f5a6ff10 100644 --- a/tests/unit/release_notes_generator/test_generator.py +++ b/tests/unit/release_notes_generator/test_generator.py @@ -23,7 +23,6 @@ from release_notes_generator.chapters.custom_chapters import CustomChapters from release_notes_generator.utils.constants import ROW_FORMAT_ISSUE - # generate_release_notes tests @@ -44,13 +43,13 @@ def test_generate_release_notes_repository_not_found(mocker): def test_generate_release_notes_latest_release_not_found( - mocker, - mock_repo, - mock_issue_closed, - mock_issue_closed_i1_bug, - mock_pull_closed_with_rls_notes_101, - mock_pull_closed_with_rls_notes_102, - mock_commit, + mocker, + mock_repo, + mock_issue_closed, + mock_issue_closed_i1_bug, + mock_pull_closed_with_rls_notes_101, + mock_pull_closed_with_rls_notes_102, + mock_commit, ): github_mock = mocker.Mock(spec=Github) github_mock.get_repo.return_value = mock_repo @@ -85,13 +84,13 @@ def test_generate_release_notes_latest_release_not_found( def test_generate_release_notes_latest_release_found_by_created_at( - mocker, - mock_repo, - mock_git_release, - mock_issue_closed_i1_bug, - mock_pull_closed_with_rls_notes_101, - mock_pull_closed_with_rls_notes_102, - mock_commit, + mocker, + mock_repo, + mock_git_release, + mock_issue_closed_i1_bug, + mock_pull_closed_with_rls_notes_101, + mock_pull_closed_with_rls_notes_102, + mock_commit, ): github_mock = mocker.Mock(spec=Github) github_mock.get_repo.return_value = mock_repo @@ -138,13 +137,13 @@ def test_generate_release_notes_latest_release_found_by_created_at( def test_generate_release_notes_latest_release_found_by_published_at( - mocker, - mock_repo, - mock_git_release, - mock_issue_closed_i1_bug, - mock_pull_closed_with_rls_notes_101, - mock_pull_closed_with_rls_notes_102, - mock_commit, + mocker, + mock_repo, + mock_git_release, + mock_issue_closed_i1_bug, + mock_pull_closed_with_rls_notes_101, + mock_pull_closed_with_rls_notes_102, + mock_commit, ): github_mock = mocker.Mock(spec=Github) github_mock.get_repo.return_value = mock_repo diff --git a/tests/unit/release_notes_generator/utils/test_gh_action.py b/tests/unit/release_notes_generator/utils/test_gh_action.py index 5d66c8c4..3a9bec1e 100644 --- a/tests/unit/release_notes_generator/utils/test_gh_action.py +++ b/tests/unit/release_notes_generator/utils/test_gh_action.py @@ -16,7 +16,6 @@ from release_notes_generator.utils.gh_action import get_action_input, set_action_output, set_action_failed - # get_input diff --git a/tests/unit/release_notes_generator/utils/test_pull_request_utils.py b/tests/unit/release_notes_generator/utils/test_pull_request_utils.py index 374dc89a..92940340 100644 --- a/tests/unit/release_notes_generator/utils/test_pull_request_utils.py +++ b/tests/unit/release_notes_generator/utils/test_pull_request_utils.py @@ -36,10 +36,12 @@ def _patch_action_inputs(monkeypatch, owner="OWN", repo="REPO", token="TOK"): monkeypatch.setattr(pru.ActionInputs, "get_github_repo_name", lambda: repo) monkeypatch.setattr(pru.ActionInputs, "get_github_token", lambda: token) + def _patch_issues_template(monkeypatch, template="Q {number} {owner} {name} {first}"): monkeypatch.setattr(pru, "ISSUES_FOR_PRS", template) monkeypatch.setattr(pru, "LINKED_ISSUES_MAX", 10) + # extract_issue_numbers_from_body @@ -54,21 +56,21 @@ def test_extract_issue_numbers_from_body_single_issue(mocker, mock_repo): mock_pr = mocker.Mock(spec=PullRequest) mock_pr.body = "This PR closes #123." issue_ids = extract_issue_numbers_from_body(mock_pr, mock_repo) - assert issue_ids == {'org/repo#123'} + assert issue_ids == {"org/repo#123"} def test_extract_issue_numbers_from_body_multiple_issues(mocker, mock_repo): mock_pr = mocker.Mock(spec=PullRequest) mock_pr.body = "This PR fixes #123 and resolves #456." issue_ids = extract_issue_numbers_from_body(mock_pr, mock_repo) - assert issue_ids == {'org/repo#123', 'org/repo#456'} + assert issue_ids == {"org/repo#123", "org/repo#456"} def test_extract_issue_numbers_from_body_mixed_case_keywords(mocker, mock_repo): mock_pr = mocker.Mock(spec=PullRequest) mock_pr.body = "This PR Fixes #123 and Resolves #456." issue_ids = extract_issue_numbers_from_body(mock_pr, mock_repo) - assert issue_ids == {'org/repo#123', 'org/repo#456'} + assert issue_ids == {"org/repo#123", "org/repo#456"} def test_extract_issue_numbers_from_body_no_body(mocker, mock_repo): @@ -87,7 +89,7 @@ def test_extract_issue_numbers_from_body_complex_text_with_wrong_syntax(mocker, - resolves the bug in #789 """ issue_ids = extract_issue_numbers_from_body(mock_pr, mock_repo) - assert issue_ids == {'org/repo#123'} + assert issue_ids == {"org/repo#123"} # get_issues_for_pr @@ -102,6 +104,7 @@ def __init__(self): self.called = False self.last_query = None self.last_headers = None + def graphql_query(self, query, headers): self.called = True self.last_query = query @@ -110,47 +113,38 @@ def graphql_query(self, query, headers): return headers, { "data": { "repository": { - "pullRequest": { - "closingIssuesReferences": { - "nodes": [{"number": 11}, {"number": 22}] - } - } + "pullRequest": {"closingIssuesReferences": {"nodes": [{"number": 11}, {"number": 22}]}} } } } requester = MockRequester() result = pru.get_issues_for_pr(123, requester) - assert result == {'OWN/REPO#11', 'OWN/REPO#22'} + assert result == {"OWN/REPO#11", "OWN/REPO#22"} assert requester.called assert requester.last_query == "Q 123 OWN REPO 10" assert requester.last_headers["Authorization"] == "Bearer TOK" assert requester.last_headers["Content-Type"] == "application/json" + def test_get_issues_for_pr_empty_nodes(monkeypatch): _patch_action_inputs(monkeypatch) _patch_issues_template(monkeypatch) class MockRequester: def graphql_query(self, query, headers): - return headers, { - "data": { - "repository": { - "pullRequest": { - "closingIssuesReferences": {"nodes": []} - } - } - } - } + return headers, {"data": {"repository": {"pullRequest": {"closingIssuesReferences": {"nodes": []}}}}} requester = MockRequester() assert pru.get_issues_for_pr(5, requester) == set() + def test_get_issues_for_pr_http_error(monkeypatch): _patch_action_inputs(monkeypatch) _patch_issues_template(monkeypatch) from github import GithubException + class MockGithubException(GithubException): def __init__(self, status, data): super().__init__(status, data) @@ -165,6 +159,7 @@ def graphql_query(self, query, headers): pru.get_issues_for_pr(77, requester) assert "GitHub HTTP error 500" in str(excinfo.value) + def test_get_issues_for_pr_malformed_response(monkeypatch): _patch_action_inputs(monkeypatch) _patch_issues_template(monkeypatch) @@ -177,53 +172,45 @@ def graphql_query(self, query, headers): with pytest.raises(KeyError): pru.get_issues_for_pr(42, requester) + def test_get_issues_for_pr_caching(monkeypatch): _patch_action_inputs(monkeypatch) _patch_issues_template(monkeypatch) calls = {"count": 0} + class MockRequester: def graphql_query(self, query, headers): calls["count"] += 1 return headers, { - "data": { - "repository": { - "pullRequest": { - "closingIssuesReferences": {"nodes": [{"number": 9}]} - } - } - } + "data": {"repository": {"pullRequest": {"closingIssuesReferences": {"nodes": [{"number": 9}]}}}} } requester = MockRequester() first = pru.get_issues_for_pr(900, requester) second = pru.get_issues_for_pr(900, requester) # should use cache - assert first == {'OWN/REPO#9'} and second == {'OWN/REPO#9'} + assert first == {"OWN/REPO#9"} and second == {"OWN/REPO#9"} assert calls["count"] == 1 # only one network call + def test_get_issues_for_pr_different_numbers_not_cached(monkeypatch): _patch_action_inputs(monkeypatch) _patch_issues_template(monkeypatch) calls = {"nums": []} + class MockRequester: def graphql_query(self, query, headers): # Extract pull number back from formatted query tail pull_num = int(query.split()[1]) calls["nums"].append(pull_num) return headers, { - "data": { - "repository": { - "pullRequest": { - "closingIssuesReferences": {"nodes": [{"number": pull_num}]} - } - } - } + "data": {"repository": {"pullRequest": {"closingIssuesReferences": {"nodes": [{"number": pull_num}]}}}} } requester = MockRequester() r1 = pru.get_issues_for_pr(1, requester) r2 = pru.get_issues_for_pr(2, requester) - assert r1 == {'OWN/REPO#1'} - assert r2 == {'OWN/REPO#2'} + assert r1 == {"OWN/REPO#1"} + assert r2 == {"OWN/REPO#2"} assert calls["nums"] == [1, 2] diff --git a/tests/unit/release_notes_generator/utils/test_utils.py b/tests/unit/release_notes_generator/utils/test_utils.py index a02fd3d4..5f49f49b 100644 --- a/tests/unit/release_notes_generator/utils/test_utils.py +++ b/tests/unit/release_notes_generator/utils/test_utils.py @@ -16,7 +16,6 @@ from release_notes_generator.utils.utils import get_change_url - # get_change_url