From a1187eb97aa3aecb33748f3a1f79978ccb7a875f Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Sat, 17 May 2025 16:57:30 +0800 Subject: [PATCH 1/4] test(changelog): cover more smart tag range test cases --- tests/test_changelog.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/test_changelog.py b/tests/test_changelog.py index 067adb4a9..84f9e7ff8 100644 --- a/tests/test_changelog.py +++ b/tests/test_changelog.py @@ -1534,7 +1534,27 @@ def test_get_smart_tag_range_returns_an_extra_for_a_range(tags): def test_get_smart_tag_range_returns_an_extra_for_a_single_tag(tags): start = tags[0] # len here is 1, but we expect one more tag as designed res = changelog.get_smart_tag_range(tags, start.name) - assert 2 == len(res) + assert res[0].name == tags[0].name + assert res[1].name == tags[1].name + + +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) + + +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 res[0].name == tags[1].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) @dataclass From 17ae1dd0678a3a9af6f227acf42a7f49ada6250d Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Sat, 17 May 2025 17:36:09 +0800 Subject: [PATCH 2/4] refactor(changelog): code cleanup --- commitizen/changelog.py | 76 +++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 37 deletions(-) diff --git a/commitizen/changelog.py b/commitizen/changelog.py index 704efe607..61d6e1035 100644 --- a/commitizen/changelog.py +++ b/commitizen/changelog.py @@ -88,24 +88,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 +167,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 +224,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 +251,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 +268,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 @@ -337,17 +339,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 From fcea12e40732c4953ac9e618f09a1ed57d5ac567 Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Sat, 17 May 2025 17:53:58 +0800 Subject: [PATCH 3/4] refactor(changelog): get_smart_tag_range and more test cases --- commitizen/changelog.py | 48 +++++++++++++++++++++++++---------------- tests/test_changelog.py | 16 ++++++++++++++ 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/commitizen/changelog.py b/commitizen/changelog.py index 61d6e1035..734ce9bd0 100644 --- a/commitizen/changelog.py +++ b/commitizen/changelog.py @@ -283,27 +283,37 @@ def incremental_build( def get_smart_tag_range( tags: list[GitTag], newest: str, oldest: str | None = None ) -> list[GitTag]: - """Smart because it finds the N+1 tag. + """Get a range of tags including the next tag after the oldest tag. - This is because we need to find until the next tag + Args: + tags: List of git tags + newest: Name of the newest tag to include + oldest: Name of the oldest tag to include. If None, same as newest. + + Returns: + List of tags from newest to oldest, plus one tag after oldest if it exists. + For nonexistent end tag, returns all tags. + For nonexistent start tag, returns tags starting from second tag. + For nonexistent start and end tags, returns empty list. """ - 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 + oldest = oldest or newest + + names = [tag.name for tag in tags] + has_newest = newest in names + has_oldest = oldest in names + if not has_newest and not has_oldest: + return [] + + if not has_newest: + return tags[1:] + + if not has_oldest: + return tags + + newest_idx = next(i for i, tag in enumerate(tags) if tag.name == newest) + oldest_idx = next(i for i, tag in enumerate(tags) if tag.name == oldest) + + return tags[newest_idx : oldest_idx + 2] def get_oldest_and_newest_rev( diff --git a/tests/test_changelog.py b/tests/test_changelog.py index 84f9e7ff8..4d9f767b8 100644 --- a/tests/test_changelog.py +++ b/tests/test_changelog.py @@ -1531,6 +1531,22 @@ def test_get_smart_tag_range_returns_an_extra_for_a_range(tags): assert 4 == len(res) +def test_get_smart_tag_range_returns_all_tags_for_a_range(tags): + start = tags[0] + + end = tags[-1] + res = changelog.get_smart_tag_range(tags, start.name, end.name) + assert len(tags) == len(res) + + end = tags[-2] + res = changelog.get_smart_tag_range(tags, start.name, end.name) + assert len(tags) == len(res) + + end = tags[-3] + res = changelog.get_smart_tag_range(tags, start.name, end.name) + assert len(tags) - 1 == len(res) + + def test_get_smart_tag_range_returns_an_extra_for_a_single_tag(tags): start = tags[0] # len here is 1, but we expect one more tag as designed res = changelog.get_smart_tag_range(tags, start.name) From 12166d6fb69c497d246de06758079f4d51877083 Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Sat, 17 May 2025 18:37:04 +0800 Subject: [PATCH 4/4] refactor(changelog): use set to check if a GitTag is used --- commitizen/changelog.py | 6 +++--- commitizen/git.py | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/commitizen/changelog.py b/commitizen/changelog.py index 734ce9bd0..cb5f111fc 100644 --- a/commitizen/changelog.py +++ b/commitizen/changelog.py @@ -102,7 +102,7 @@ def generate_tree_from_commits( current_tag_date = current_tag.date changes: defaultdict[str | None, list] = defaultdict(list) - used_tags = [current_tag] + used_tags: set[GitTag | None] = set([current_tag]) for commit in commits: commit_tag = get_commit_tag(commit, tags) @@ -111,7 +111,7 @@ def generate_tree_from_commits( and commit_tag not in used_tags and rules.include_in_changelog(commit_tag) ): - used_tags.append(commit_tag) + used_tags.add(commit_tag) release = { "version": current_tag_name, "date": current_tag_date, @@ -298,7 +298,7 @@ def get_smart_tag_range( """ oldest = oldest or newest - names = [tag.name for tag in tags] + names = set(tag.name for tag in tags) has_newest = newest in names has_oldest = oldest in names if not has_newest and not has_oldest: diff --git a/commitizen/git.py b/commitizen/git.py index 19ca46b6c..04047e01a 100644 --- a/commitizen/git.py +++ b/commitizen/git.py @@ -41,6 +41,9 @@ def __eq__(self, other) -> bool: return False return self.rev == other.rev # type: ignore + def __hash__(self): + return hash(self.rev) + class GitCommit(GitObject): def __init__(