From aeb681af3fb21820b49683eb37842057badb5b02 Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Sat, 17 May 2025 16:57:30 +0800 Subject: [PATCH 1/3] 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 ed90ed08e4..ff8a512840 100644 --- a/tests/test_changelog.py +++ b/tests/test_changelog.py @@ -1532,7 +1532,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 3d2c316ab16338abcc43b97807835d879a0b82c7 Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Sat, 17 May 2025 17:36:09 +0800 Subject: [PATCH 2/3] refactor(changelog): code cleanup --- commitizen/changelog.py | 146 +++++++++++++++++++++++----------------- commitizen/git.py | 3 + tests/test_changelog.py | 16 +++++ 3 files changed, 103 insertions(+), 62 deletions(-) diff --git a/commitizen/changelog.py b/commitizen/changelog.py index ba6fbbc6b3..74e9d3356a 100644 --- a/commitizen/changelog.py +++ b/commitizen/changelog.py @@ -74,6 +74,23 @@ def get_commit_tag(commit: GitCommit, tags: list[GitTag]) -> GitTag | None: return next((tag for tag in tags if tag.rev == commit.rev), None) +def _get_release_info( + current_tag_name: str, + current_tag_date: str, + changes: dict[str | None, list], + changelog_release_hook: ChangelogReleaseHook | None, + commit_tag: GitTag | None, +) -> dict[str, Any]: + release = { + "version": current_tag_name, + "date": current_tag_date, + "changes": changes, + } + if changelog_release_hook: + return changelog_release_hook(release, commit_tag) + return release + + def generate_tree_from_commits( commits: list[GitCommit], tags: list[GitTag], @@ -88,24 +105,24 @@ 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() + used_tags: set[GitTag] = set() + current_tag_name = unreleased_version or "Unreleased" + current_tag_date = ( + date.today().isoformat() if unreleased_version is not None else "" + ) + # 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() - 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] + current_tag = get_commit_tag(commits[0], tags) if commits else None + if current_tag is not None: + used_tags.add(current_tag) + if current_tag.name: + current_tag_name = current_tag.name + current_tag_date = current_tag.date + + changes: defaultdict[str | None, list] = defaultdict(list) + commit_tag: GitTag | None = None for commit in commits: commit_tag = get_commit_tag(commit, tags) @@ -114,21 +131,21 @@ def generate_tree_from_commits( and commit_tag not in used_tags and rules.include_in_changelog(commit_tag) ): - used_tags.append(commit_tag) - release = { - "version": current_tag_name, - "date": current_tag_date, - "changes": changes, - } - if changelog_release_hook: - release = changelog_release_hook(release, commit_tag) - yield release + used_tags.add(commit_tag) + + yield _get_release_info( + current_tag_name, + current_tag_date, + changes, + changelog_release_hook, + commit_tag, + ) + current_tag_name = commit_tag.name current_tag_date = commit_tag.date changes = defaultdict(list) - matches = pat.match(commit.message) - if not matches: + if not pat.match(commit.message): continue # Process subject from commit message @@ -153,14 +170,13 @@ def generate_tree_from_commits( change_type_map, ) - release = { - "version": current_tag_name, - "date": current_tag_date, - "changes": changes, - } - if changelog_release_hook: - release = changelog_release_hook(release, commit_tag) - yield release + yield _get_release_info( + current_tag_name, + current_tag_date, + changes, + changelog_release_hook, + commit_tag, + ) def process_commit_message( @@ -178,13 +194,15 @@ def process_commit_message( **parsed.groupdict(), } - 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) + if not (processed := hook(message, commit) if hook else message): + return + + 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 generate_ordered_changelog_tree( @@ -228,8 +246,7 @@ def render_changelog( **kwargs: Any, ) -> 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( @@ -256,7 +273,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 @@ -271,13 +290,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 @@ -327,8 +348,7 @@ def get_oldest_and_newest_rev( if not (newest_tag := rules.find_tag_for(tags, newest)): raise NoCommitsFoundError("Could not find a valid revision range.") - oldest_tag = None - oldest_tag_name = None + oldest_tag_name: str | None = None if oldest: if not (oldest_tag := rules.find_tag_for(tags, oldest)): raise NoCommitsFoundError("Could not find a valid revision range.") @@ -340,17 +360,19 @@ 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 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, 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 + oldest_rev: str | None = ( + None + if ( + tags_range[-1].name == tags[-1].name + and tags_range[-1].name == oldest_tag_name + or tags_range[-1].name == newest_rev + ) + else tags_range[-1].name + ) return oldest_rev, newest_rev diff --git a/commitizen/git.py b/commitizen/git.py index fb59750eaf..d98709d053 100644 --- a/commitizen/git.py +++ b/commitizen/git.py @@ -49,6 +49,9 @@ class GitObject: def __eq__(self, other: object) -> bool: return hasattr(other, "rev") and self.rev == other.rev + def __hash__(self): + return hash(self.rev) + class GitCommit(GitObject): def __init__( diff --git a/tests/test_changelog.py b/tests/test_changelog.py index ff8a512840..862a838eac 100644 --- a/tests/test_changelog.py +++ b/tests/test_changelog.py @@ -1529,6 +1529,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 cdd15fdb766d37ff9ae3ae2a7a7454d5230e4bbb Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Sat, 17 May 2025 17:36:09 +0800 Subject: [PATCH 3/3] refactor(changelog): code cleanup --- commitizen/changelog.py | 48 +++++++++++++++++++++++++---------------- commitizen/git.py | 2 +- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/commitizen/changelog.py b/commitizen/changelog.py index 74e9d3356a..b40488a063 100644 --- a/commitizen/changelog.py +++ b/commitizen/changelog.py @@ -305,27 +305,37 @@ def incremental_build( def get_smart_tag_range( tags: Sequence[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 = 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: + 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/commitizen/git.py b/commitizen/git.py index d98709d053..097091826e 100644 --- a/commitizen/git.py +++ b/commitizen/git.py @@ -49,7 +49,7 @@ class GitObject: def __eq__(self, other: object) -> bool: return hasattr(other, "rev") and self.rev == other.rev - def __hash__(self): + def __hash__(self) -> int: return hash(self.rev)