From 1cc5858b1f985a1cad794b45997a030241a404a8 Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Fri, 16 May 2025 02:18:37 +0800 Subject: [PATCH 1/5] refactor(git): code cleanup and better test coverage --- commitizen/git.py | 133 +++++++++++++++------------------------------- tests/test_git.py | 32 ++++++----- 2 files changed, 61 insertions(+), 104 deletions(-) diff --git a/commitizen/git.py b/commitizen/git.py index 19ca46b6c..d0cc901a6 100644 --- a/commitizen/git.py +++ b/commitizen/git.py @@ -1,34 +1,14 @@ from __future__ import annotations import os -from enum import Enum -from os import linesep from pathlib import Path from tempfile import NamedTemporaryFile from commitizen import cmd, out from commitizen.exceptions import GitCommandError -UNIX_EOL = "\n" -WINDOWS_EOL = "\r\n" - - -class EOLTypes(Enum): - """The EOL type from `git config core.eol`.""" - - LF = "lf" - CRLF = "crlf" - NATIVE = "native" - - def get_eol_for_open(self) -> str: - """Get the EOL character for `open()`.""" - map = { - EOLTypes.CRLF: WINDOWS_EOL, - EOLTypes.LF: UNIX_EOL, - EOLTypes.NATIVE: linesep, - } - - return map[self] +_UNIX_EOL = "\n" +_WINDOWS_EOL = "\r\n" class GitObject: @@ -37,9 +17,7 @@ class GitObject: date: str def __eq__(self, other) -> bool: - if not hasattr(other, "rev"): - return False - return self.rev == other.rev # type: ignore + return hasattr(other, "rev") and self.rev == other.rev class GitCommit(GitObject): @@ -63,6 +41,20 @@ def __init__( def message(self): return f"{self.title}\n\n{self.body}".strip() + @classmethod + def from_rev_and_commit(cls, rev_and_commit: str) -> GitCommit: + rev, parents, title, author, author_email, *body_list = rev_and_commit.split( + "\n" + ) + return cls( + rev=rev.strip(), + title=title.strip(), + body="\n".join(body_list).strip(), + author=author, + author_email=author_email, + parents=[p for p in parents.strip().split(" ") if p], + ) + def __repr__(self): return f"{self.title} ({self.rev})" @@ -101,13 +93,11 @@ def tag( # according to https://git-scm.com/book/en/v2/Git-Basics-Tagging, # we're not able to create lightweight tag with message. # by adding message, we make it a annotated tags - c = cmd.run(f'git tag {_opt} "{tag if _opt == "" or msg is None else msg}"') - return c + return cmd.run(f'git tag {_opt} "{tag if _opt == "" or msg is None else msg}"') def add(*args: str) -> cmd.Command: - c = cmd.run(f"git add {' '.join(args)}") - return c + return cmd.run(f"git add {' '.join(args)}") def commit( @@ -140,24 +130,10 @@ def get_commits( ) -> list[GitCommit]: """Get the commits between start and end.""" git_log_entries = _get_log_as_str_list(start, end, args) - git_commits = [] - for rev_and_commit in git_log_entries: - if not rev_and_commit: - continue - rev, parents, title, author, author_email, *body_list = rev_and_commit.split( - "\n" - ) - if rev_and_commit: - git_commit = GitCommit( - rev=rev.strip(), - title=title.strip(), - body="\n".join(body_list).strip(), - author=author, - author_email=author_email, - parents=[p for p in parents.strip().split(" ") if p], - ) - git_commits.append(git_commit) - return git_commits + return [ + GitCommit.from_rev_and_commit(rev_and_commit) + for rev_and_commit in filter(None, git_log_entries) + ] def get_filenames_in_commit(git_reference: str = ""): @@ -170,8 +146,7 @@ def get_filenames_in_commit(git_reference: str = ""): c = cmd.run(f"git show --name-only --pretty=format: {git_reference}") if c.return_code == 0: return c.out.strip().split("\n") - else: - raise GitCommandError(c.err) + raise GitCommandError(c.err) def get_tags( @@ -197,16 +172,11 @@ def get_tags( if c.err: out.warn(f"Attempting to proceed after: {c.err}") - if not c.out: - return [] - - git_tags = [ + return [ GitTag.from_line(line=line, inner_delimiter=inner_delimiter) for line in c.out.split("\n")[:-1] ] - return git_tags - def tag_exist(tag: str) -> bool: c = cmd.run(f"git tag --list {tag}") @@ -231,18 +201,18 @@ def get_tag_message(tag: str) -> str | None: return c.out.strip() -def get_tag_names() -> list[str | None]: +def get_tag_names() -> list[str]: c = cmd.run("git tag --list") if c.err: return [] - return [tag.strip() for tag in c.out.split("\n") if tag.strip()] + return list(filter(None, (tag.strip() for tag in c.out.split("\n")))) def find_git_project_root() -> Path | None: c = cmd.run("git rev-parse --show-toplevel") - if not c.err: - return Path(c.out.strip()) - return None + if c.err: + return None + return Path(c.out.strip()) def is_staging_clean() -> bool: @@ -253,32 +223,19 @@ def is_staging_clean() -> bool: def is_git_project() -> bool: c = cmd.run("git rev-parse --is-inside-work-tree") - if c.out.strip() == "true": - return True - return False + return c.out.strip() == "true" -def get_eol_style() -> EOLTypes: +def get_eol_for_open() -> str: + # See: https://git-scm.com/docs/git-config#Documentation/git-config.txt-coreeol c = cmd.run("git config core.eol") eol = c.out.strip().lower() - # We enumerate the EOL types of the response of - # `git config core.eol`, and map it to our enumration EOLTypes. - # - # It is just like the variant of the "match" syntax. - map = { - "lf": EOLTypes.LF, - "crlf": EOLTypes.CRLF, - "native": EOLTypes.NATIVE, - } - - # If the response of `git config core.eol` is in the map: - if eol in map: - return map[eol] - else: - # The default value is "native". - # https://git-scm.com/docs/git-config#Documentation/git-config.txt-coreeol - return map["native"] + if eol == "lf": + return _UNIX_EOL + if eol == "crlf": + return _WINDOWS_EOL + return os.linesep def get_core_editor() -> str | None: @@ -288,22 +245,18 @@ def get_core_editor() -> str | None: return None -def smart_open(*args, **kargs): +def smart_open(*args, **kwargs): """Open a file with the EOL style determined from Git.""" - return open(*args, newline=get_eol_style().get_eol_for_open(), **kargs) + return open(*args, newline=get_eol_for_open(), **kwargs) def _get_log_as_str_list(start: str | None, end: str, args: str) -> list[str]: """Get string representation of each log entry""" delimiter = "----------commit-delimiter----------" log_format: str = "%H%n%P%n%s%n%an%n%ae%n%b" - git_log_cmd = ( - f"git -c log.showSignature=False log --pretty={log_format}{delimiter} {args}" - ) - if start: - command = f"{git_log_cmd} {start}..{end}" - else: - command = f"{git_log_cmd} {end}" + command_range = f"{start}..{end}" if start else end + command = f"git -c log.showSignature=False log --pretty={log_format}{delimiter} {args} {command_range}" + c = cmd.run(command) if c.return_code != 0: raise GitCommandError(c.err) diff --git a/tests/test_git.py b/tests/test_git.py index 8b2fc2b86..4e4b61ffe 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -79,8 +79,7 @@ def test_get_reachable_tags_with_commits( monkeypatch.setenv("LANGUAGE", f"{locale}.UTF-8") monkeypatch.setenv("LC_ALL", f"{locale}.UTF-8") with tmp_commitizen_project.as_cwd(): - tags = git.get_tags(reachable_only=True) - assert tags == [] + assert git.get_tags(reachable_only=True) == [] def test_get_tag_names(mocker: MockFixture): @@ -271,7 +270,7 @@ def test_get_commits_with_signature(): def test_get_tag_names_has_correct_arrow_annotation(): arrow_annotation = inspect.getfullargspec(git.get_tag_names).annotations["return"] - assert arrow_annotation == "list[str | None]" + assert arrow_annotation == "list[str]" def test_get_latest_tag_name(tmp_commitizen_project): @@ -317,24 +316,18 @@ def test_is_staging_clean_when_updating_file(tmp_commitizen_project): assert git.is_staging_clean() is False -def test_git_eol_style(tmp_commitizen_project): +def test_get_eol_for_open(tmp_commitizen_project): with tmp_commitizen_project.as_cwd(): - assert git.get_eol_style() == git.EOLTypes.NATIVE + assert git.get_eol_for_open() == os.linesep cmd.run("git config core.eol lf") - assert git.get_eol_style() == git.EOLTypes.LF + assert git.get_eol_for_open() == "\n" cmd.run("git config core.eol crlf") - assert git.get_eol_style() == git.EOLTypes.CRLF + assert git.get_eol_for_open() == "\r\n" cmd.run("git config core.eol native") - assert git.get_eol_style() == git.EOLTypes.NATIVE - - -def test_eoltypes_get_eol_for_open(): - assert git.EOLTypes.get_eol_for_open(git.EOLTypes.NATIVE) == os.linesep - assert git.EOLTypes.get_eol_for_open(git.EOLTypes.LF) == "\n" - assert git.EOLTypes.get_eol_for_open(git.EOLTypes.CRLF) == "\r\n" + assert git.get_eol_for_open() == os.linesep def test_get_core_editor(mocker): @@ -401,3 +394,14 @@ def test_commit_with_spaces_in_path(mocker, file_path, expected_cmd): mock_run.assert_called_once_with(expected_cmd) mock_unlink.assert_called_once_with(file_path) + + +def test_get_filenames_in_commit_error(mocker: MockFixture): + """Test that GitCommandError is raised when git command fails.""" + mocker.patch( + "commitizen.cmd.run", + return_value=FakeCommand(out="", err="fatal: bad object HEAD", return_code=1), + ) + with pytest.raises(exceptions.GitCommandError) as excinfo: + git.get_filenames_in_commit() + assert str(excinfo.value) == "fatal: bad object HEAD" From 61c254f65b58a02602f751d2b67abdc2162fc133 Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Sat, 17 May 2025 16:04:08 +0800 Subject: [PATCH 2/5] test(git): add test for from_rev_and_commit --- tests/test_git.py | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/test_git.py b/tests/test_git.py index 4e4b61ffe..cb4ea9337 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -405,3 +405,47 @@ def test_get_filenames_in_commit_error(mocker: MockFixture): with pytest.raises(exceptions.GitCommandError) as excinfo: git.get_filenames_in_commit() assert str(excinfo.value) == "fatal: bad object HEAD" + + +def test_git_commit_from_rev_and_commit(): + # Test data with all fields populated + rev_and_commit = ( + "abc123\n" # rev + "def456 ghi789\n" # parents + "feat: add new feature\n" # title + "John Doe\n" # author + "john@example.com\n" # author_email + "This is a detailed description\n" # body + "of the new feature\n" + "with multiple lines" + ) + + commit = git.GitCommit.from_rev_and_commit(rev_and_commit) + + assert commit.rev == "abc123" + assert commit.title == "feat: add new feature" + assert ( + commit.body + == "This is a detailed description\nof the new feature\nwith multiple lines" + ) + assert commit.author == "John Doe" + assert commit.author_email == "john@example.com" + assert commit.parents == ["def456", "ghi789"] + + # Test with minimal data + minimal_commit = ( + "abc123\n" # rev + "\n" # no parents + "feat: minimal commit\n" # title + "John Doe\n" # author + "john@example.com\n" # author_email + ) + + commit = git.GitCommit.from_rev_and_commit(minimal_commit) + + assert commit.rev == "abc123" + assert commit.title == "feat: minimal commit" + assert commit.body == "" + assert commit.author == "John Doe" + assert commit.author_email == "john@example.com" + assert commit.parents == [] From 40abbbdb93ea6457e1a75cf9ec7e321c0905806d Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Sat, 17 May 2025 16:04:38 +0800 Subject: [PATCH 3/5] docs(git): from_rev_and_commit docstring --- commitizen/git.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/commitizen/git.py b/commitizen/git.py index d0cc901a6..d80545bc4 100644 --- a/commitizen/git.py +++ b/commitizen/git.py @@ -43,6 +43,48 @@ def message(self): @classmethod def from_rev_and_commit(cls, rev_and_commit: str) -> GitCommit: + """Create a GitCommit instance from a formatted commit string. + + This method parses a multi-line string containing commit information in the following format: + ``` + + + + <author> + <author_email> + <body_line_1> + <body_line_2> + ... + ``` + + Args: + rev_and_commit (str): A string containing commit information with fields separated by newlines. + - rev: The commit hash/revision + - parents: Space-separated list of parent commit hashes + - title: The commit title/message + - author: The commit author's name + - author_email: The commit author's email + - body: Optional multi-line commit body + + Returns: + GitCommit: A new GitCommit instance with the parsed information. + + Example: + >>> commit_str = '''abc123 + ... def456 ghi789 + ... feat: add new feature + ... John Doe + ... john@example.com + ... This is a detailed description + ... of the new feature''' + >>> commit = GitCommit.from_rev_and_commit(commit_str) + >>> commit.rev + 'abc123' + >>> commit.title + 'feat: add new feature' + >>> commit.parents + ['def456', 'ghi789'] + """ rev, parents, title, author, author_email, *body_list = rev_and_commit.split( "\n" ) From 20f48926feeec5a0e087a6b7d2102d715c3d5f74 Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung <bear890707@gmail.com> Date: Sun, 18 May 2025 19:11:52 +0800 Subject: [PATCH 4/5] refactor(EOLType): add eol enum back and reorganize methods --- commitizen/git.py | 48 ++++++++++++++++++++++++++++++++--------------- tests/test_git.py | 8 ++++---- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/commitizen/git.py b/commitizen/git.py index d80545bc4..ef4e0452e 100644 --- a/commitizen/git.py +++ b/commitizen/git.py @@ -1,14 +1,44 @@ from __future__ import annotations import os +from enum import Enum +from functools import lru_cache from pathlib import Path from tempfile import NamedTemporaryFile from commitizen import cmd, out from commitizen.exceptions import GitCommandError -_UNIX_EOL = "\n" -_WINDOWS_EOL = "\r\n" + +class EOLType(Enum): + """The EOL type from `git config core.eol`.""" + + LF = "lf" + CRLF = "crlf" + NATIVE = "native" + + @classmethod + def for_open(cls) -> str: + c = cmd.run("git config core.eol") + eol = c.out.strip().upper() + return cls._char_for_open()[cls._safe_cast(eol)] + + @classmethod + def _safe_cast(cls, eol: str) -> EOLType: + try: + return cls[eol] + except KeyError: + return cls.NATIVE + + @classmethod + @lru_cache + def _char_for_open(cls) -> dict[EOLType, str]: + """Get the EOL character for `open()`.""" + return { + cls.LF: "\n", + cls.CRLF: "\r\n", + cls.NATIVE: os.linesep, + } class GitObject: @@ -268,18 +298,6 @@ def is_git_project() -> bool: return c.out.strip() == "true" -def get_eol_for_open() -> str: - # See: https://git-scm.com/docs/git-config#Documentation/git-config.txt-coreeol - c = cmd.run("git config core.eol") - eol = c.out.strip().lower() - - if eol == "lf": - return _UNIX_EOL - if eol == "crlf": - return _WINDOWS_EOL - return os.linesep - - def get_core_editor() -> str | None: c = cmd.run("git var GIT_EDITOR") if c.out: @@ -289,7 +307,7 @@ def get_core_editor() -> str | None: def smart_open(*args, **kwargs): """Open a file with the EOL style determined from Git.""" - return open(*args, newline=get_eol_for_open(), **kwargs) + return open(*args, newline=EOLType.for_open(), **kwargs) def _get_log_as_str_list(start: str | None, end: str, args: str) -> list[str]: diff --git a/tests/test_git.py b/tests/test_git.py index cb4ea9337..de3130412 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -318,16 +318,16 @@ def test_is_staging_clean_when_updating_file(tmp_commitizen_project): def test_get_eol_for_open(tmp_commitizen_project): with tmp_commitizen_project.as_cwd(): - assert git.get_eol_for_open() == os.linesep + assert git.EOLType.for_open() == os.linesep cmd.run("git config core.eol lf") - assert git.get_eol_for_open() == "\n" + assert git.EOLType.for_open() == "\n" cmd.run("git config core.eol crlf") - assert git.get_eol_for_open() == "\r\n" + assert git.EOLType.for_open() == "\r\n" cmd.run("git config core.eol native") - assert git.get_eol_for_open() == os.linesep + assert git.EOLType.for_open() == os.linesep def test_get_core_editor(mocker): From 3962b808a3e7a621aa414a90e45657819a646d46 Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung <bear890707@gmail.com> Date: Sun, 18 May 2025 19:26:22 +0800 Subject: [PATCH 5/5] refactor(git): refactor get_tag_names --- commitizen/git.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commitizen/git.py b/commitizen/git.py index ef4e0452e..fa59e34d4 100644 --- a/commitizen/git.py +++ b/commitizen/git.py @@ -277,7 +277,7 @@ def get_tag_names() -> list[str]: c = cmd.run("git tag --list") if c.err: return [] - return list(filter(None, (tag.strip() for tag in c.out.split("\n")))) + return [tag for raw in c.out.split("\n") if (tag := raw.strip())] def find_git_project_root() -> Path | None: