refactor(changelog): some cleanup and adjust test cases

Author: bearomorphism

commitizen/changelog.py

@@ -32,6 +32,7 @@
 from collections.abc import Iterable
 from dataclasses import dataclass
 from datetime import date
+from itertools import islice
 from typing import TYPE_CHECKING
 
 from jinja2 import (
@@ -88,24 +89,21 @@ def generate_tree_from_commits(
     pat = re.compile(changelog_pattern)
     map_pat = re.compile(commit_parser, re.MULTILINE)
     body_map_pat = re.compile(commit_parser, re.MULTILINE | re.DOTALL)
-    current_tag: GitTag | None = None
     rules = rules or TagRules()
 
     # Check if the latest commit is not tagged
-    if commits:
-        latest_commit = commits[0]
-        current_tag = get_commit_tag(latest_commit, tags)
-
-    current_tag_name: str = unreleased_version or "Unreleased"
-    current_tag_date: str = ""
-    if unreleased_version is not None:
-        current_tag_date = date.today().isoformat()
+    current_tag = get_commit_tag(commits[0], tags) if commits else None
+
+    current_tag_name = unreleased_version or "Unreleased"
+    current_tag_date = (
+        date.today().isoformat() if unreleased_version is not None else ""
+    )
     if current_tag is not None and current_tag.name:
         current_tag_name = current_tag.name
         current_tag_date = current_tag.date
 
-    changes: dict = defaultdict(list)
-    used_tags: list = [current_tag]
+    changes: defaultdict[str | None, list] = defaultdict(list)
+    used_tags = [current_tag]
     for commit in commits:
         commit_tag = get_commit_tag(commit, tags)
 
@@ -170,21 +168,23 @@ def process_commit_message(
     changes: dict[str | None, list],
     change_type_map: dict[str, str] | None = None,
 ):
-    message: dict = {
+    message = {
         "sha1": commit.rev,
         "parents": commit.parents,
         "author": commit.author,
         "author_email": commit.author_email,
         **parsed.groupdict(),
     }
+    processed = hook(message, commit) if hook else message
+    if not processed:
+        return
 
-    if processed := hook(message, commit) if hook else message:
-        messages = [processed] if isinstance(processed, dict) else processed
-        for msg in messages:
-            change_type = msg.pop("change_type", None)
-            if change_type_map:
-                change_type = change_type_map.get(change_type, change_type)
-            changes[change_type].append(msg)
+    processed_messages = [processed] if isinstance(processed, dict) else processed
+    for msg in processed_messages:
+        change_type = msg.pop("change_type", None)
+        if change_type_map:
+            change_type = change_type_map.get(change_type, change_type)
+        changes[change_type].append(msg)
 
 
 def order_changelog_tree(tree: Iterable, change_type_order: list[str]) -> Iterable:
@@ -225,8 +225,7 @@ def render_changelog(
     **kwargs,
 ) -> str:
     jinja_template = get_changelog_template(loader, template)
-    changelog: str = jinja_template.render(tree=tree, **kwargs)
-    return changelog
+    return jinja_template.render(tree=tree, **kwargs)
 
 
 def incremental_build(
@@ -253,7 +252,9 @@ def incremental_build(
     for index, line in enumerate(lines):
         if index == unreleased_start:
             skip = True
-        elif index == unreleased_end:
+            continue
+
+        if index == unreleased_end:
             skip = False
             if (
                 latest_version_position is None
@@ -268,13 +269,15 @@ def incremental_build(
 
         if index == latest_version_position:
             output_lines.extend([new_content, "\n"])
-
         output_lines.append(line)
-    if not isinstance(latest_version_position, int):
-        if output_lines and output_lines[-1].strip():
-            # Ensure at least one blank line between existing and new content.
-            output_lines.append("\n")
-        output_lines.append(new_content)
+
+    if isinstance(latest_version_position, int):
+        return output_lines
+
+    if output_lines and output_lines[-1].strip():
+        # Ensure at least one blank line between existing and new content.
+        output_lines.append("\n")
+    output_lines.append(new_content)
     return output_lines
 
 
@@ -285,23 +288,26 @@ def get_smart_tag_range(
 
     This is because we need to find until the next tag
     """
-    accumulator = []
-    keep = False
     if not oldest:
         oldest = newest
-    for index, tag in enumerate(tags):
-        if tag.name == newest:
-            keep = True
-        if keep:
-            accumulator.append(tag)
-        if tag.name == oldest:
-            keep = False
-            try:
-                accumulator.append(tags[index + 1])
-            except IndexError:
-                pass
-            break
-    return accumulator
+
+    # Find indices of newest and oldest tags
+    try:
+        newest_idx = next((i for i, tag in enumerate(tags) if tag.name == newest))
+        oldest_idx = newest_idx + (
+            next(
+                i
+                for i, tag in enumerate(islice(tags, newest_idx, None))
+                if tag.name == oldest
+            )
+        )
+    except StopIteration:
+        # Should not reach here
+        raise NoCommitsFoundError(f"Could not find the tags {newest=} and {oldest=}.")
+
+    # Get the range of tags including the next tag after oldest if it exists
+    end_idx = min(oldest_idx + 2, len(tags))
+    return tags[newest_idx:end_idx]
 
 
 def get_oldest_and_newest_rev(
@@ -337,17 +343,17 @@ def get_oldest_and_newest_rev(
     if not tags_range:
         raise NoCommitsFoundError("Could not find a valid revision range.")
 
-    oldest_rev: str | None = tags_range[-1].name
+    oldest_rev = tags_range[-1].name
     newest_rev = newest_tag.name
 
-    # check if it's the first tag created
-    # and it's also being requested as part of the range
-    if oldest_rev == tags[-1].name and oldest_rev == oldest_tag_name:
-        return None, newest_rev
-
-    # when they are the same, and it's also the
-    # first tag created
-    if oldest_rev == newest_rev:
+    # Return None for oldest_rev if:
+    # 1. The oldest tag is the last tag in the list and matches the requested oldest tag, or
+    # 2. The oldest and newest tags are the same
+    if (
+        oldest_rev == tags[-1].name
+        and oldest_rev == oldest_tag_name
+        or oldest_rev == newest_rev
+    ):
         return None, newest_rev
 
     return oldest_rev, newest_rev

tests/test_changelog.py

@@ -1539,21 +1539,21 @@ def test_get_smart_tag_range_returns_an_extra_for_a_single_tag(tags):
 
 def test_get_smart_tag_range_returns_an_empty_list_for_nonexistent_end_tag(tags):
     start = tags[0]
-    res = changelog.get_smart_tag_range(tags, start.name, "nonexistent")
-    assert len(tags) == len(res)
+    with pytest.raises(changelog.NoCommitsFoundError):
+        changelog.get_smart_tag_range(tags, start.name, "nonexistent")
 
 
 def test_get_smart_tag_range_returns_an_empty_list_for_nonexistent_start_tag(tags):
     end = tags[0]
-    res = changelog.get_smart_tag_range(tags, "nonexistent", end.name)
-    assert 1 == len(res)
+    with pytest.raises(changelog.NoCommitsFoundError):
+        changelog.get_smart_tag_range(tags, "nonexistent", end.name)
 
 
 def test_get_smart_tag_range_returns_an_empty_list_for_nonexistent_start_and_end_tags(
     tags,
 ):
-    res = changelog.get_smart_tag_range(tags, "nonexistent", "nonexistent")
-    assert 0 == len(res)
+    with pytest.raises(changelog.NoCommitsFoundError):
+        changelog.get_smart_tag_range(tags, "nonexistent", "nonexistent")
 
 
 @dataclass