From d36c0c2ed0095c75cbb3a5349a0adf58a88df621 Mon Sep 17 00:00:00 2001 From: bhelgs <33836927+bhelgs@users.noreply.github.com> Date: Mon, 1 Aug 2022 11:33:53 -0400 Subject: [PATCH 1/8] fix(git): improves git error checking in get_commits older version of git did not have the --author-date-order argument so it is best to report that kind of error. --- commitizen/exceptions.py | 5 +++++ commitizen/git.py | 3 +++ docs/exit_codes.md | 1 + 3 files changed, 9 insertions(+) diff --git a/commitizen/exceptions.py b/commitizen/exceptions.py index 16869b5a28..17a91778df 100644 --- a/commitizen/exceptions.py +++ b/commitizen/exceptions.py @@ -27,6 +27,7 @@ class ExitCode(enum.IntEnum): NOT_ALLOWED = 20 NO_INCREMENT = 21 UNRECOGNIZED_CHARACTERSET_ENCODING = 22 + GIT_COMMAND_ERROR = 23 class CommitizenException(Exception): @@ -153,3 +154,7 @@ class NotAllowed(CommitizenException): class CharacterSetDecodeError(CommitizenException): exit_code = ExitCode.UNRECOGNIZED_CHARACTERSET_ENCODING + + +class GitCommandError(CommitizenException): + exit_code = ExitCode.GIT_COMMAND_ERROR diff --git a/commitizen/git.py b/commitizen/git.py index 935686c317..6ad2440a89 100644 --- a/commitizen/git.py +++ b/commitizen/git.py @@ -6,6 +6,7 @@ from typing import List, Optional from commitizen import cmd +from commitizen.exceptions import GitCommandError UNIX_EOL = "\n" WINDOWS_EOL = "\r\n" @@ -118,6 +119,8 @@ def get_commits( else: command = f"{git_log_cmd} {end}" c = cmd.run(command) + if c.return_code != 0: + raise GitCommandError(c.err) if not c.out: return [] diff --git a/docs/exit_codes.md b/docs/exit_codes.md index b39964603a..54be954ff4 100644 --- a/docs/exit_codes.md +++ b/docs/exit_codes.md @@ -30,3 +30,4 @@ These exit codes can be found in `commitizen/exceptions.py::ExitCode`. | NotAllowed | 20 | `--incremental` cannot be combined with a `rev_range` | | NoneIncrementExit | 21 | The commits found are not eligible to be bumped | | CharacterSetDecodeError | 22 | The character encoding of the command output could not be determined | +| GitCommandError | 23 | Unexpected failure while calling a git command | From 9b243a3d3bcbc9e3a9391c27ac753d284cf9d00a Mon Sep 17 00:00:00 2001 From: bhelgs <33836927+bhelgs@users.noreply.github.com> Date: Mon, 1 Aug 2022 12:21:30 -0400 Subject: [PATCH 2/8] test(bump): fixes logic issue made evident by the latest fix(git) commit git was failing "fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree". There is no HEAD for get_commits until at least one commit is made. --- tests/commands/test_bump_command.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/commands/test_bump_command.py b/tests/commands/test_bump_command.py index 927a719010..2c9ca36774 100644 --- a/tests/commands/test_bump_command.py +++ b/tests/commands/test_bump_command.py @@ -273,9 +273,19 @@ def test_bump_when_version_is_not_specify(mocker): @pytest.mark.usefixtures("tmp_commitizen_project") def test_bump_when_no_new_commit(mocker): + """bump without any commits since the last bump.""" + # We need this first commit otherwise the revision is invalid. + create_file_and_commit("feat: initial") + testargs = ["cz", "bump", "--yes"] mocker.patch.object(sys, "argv", testargs) + # First bump. + # The next bump should fail since + # there is not a commit between the two bumps. + cli.main() + + # bump without a new commit. with pytest.raises(NoCommitsFoundError) as excinfo: cli.main() From 5318325e9e42ca4c1d298d69494545724664ddbe Mon Sep 17 00:00:00 2001 From: bhelgs <33836927+bhelgs@users.noreply.github.com> Date: Mon, 1 Aug 2022 12:34:40 -0400 Subject: [PATCH 3/8] test(changelog): fixes logic issue made evident by latest fix(git) commit Exception is raised by git error: "fatal: your current branch 'master' does not have any commits yet". This response is more informative than the original "No commits found" exception and the "fail fast" logic is a bit easier to follow. --- tests/commands/test_changelog_command.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/commands/test_changelog_command.py b/tests/commands/test_changelog_command.py index ed6aa6194e..900915ec88 100644 --- a/tests/commands/test_changelog_command.py +++ b/tests/commands/test_changelog_command.py @@ -6,6 +6,7 @@ from commitizen.commands.changelog import Changelog from commitizen.exceptions import ( DryRunExit, + GitCommandError, NoCommitsFoundError, NoRevisionError, NotAGitProjectError, @@ -19,10 +20,12 @@ def test_changelog_on_empty_project(mocker): testargs = ["cz", "changelog", "--dry-run"] mocker.patch.object(sys, "argv", testargs) - with pytest.raises(NoCommitsFoundError) as excinfo: + with pytest.raises(GitCommandError): cli.main() - assert "No commits found" in str(excinfo) + # git will error out with something like: + # "your current branch 'XYZ' does not have any commits yet" + # is it wise to assert the exception contains a message like this? @pytest.mark.usefixtures("tmp_commitizen_project") From b4dbd84ca004ad970630e65712ed97bc23bf3622 Mon Sep 17 00:00:00 2001 From: bhelgs <33836927+bhelgs@users.noreply.github.com> Date: Mon, 1 Aug 2022 12:55:01 -0400 Subject: [PATCH 4/8] refactor(changelog): fixes logic issue made evident by latest fix(git) commit get_commits was calling git "git log None..None" in the event that a tag range is invalid. Raise exception sooner rather than later. --- commitizen/changelog.py | 4 ++-- tests/commands/test_changelog_command.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/commitizen/changelog.py b/commitizen/changelog.py index 1ccdb4e297..ead80ab775 100644 --- a/commitizen/changelog.py +++ b/commitizen/changelog.py @@ -35,7 +35,7 @@ from commitizen import defaults from commitizen.bump import normalize_tag -from commitizen.exceptions import InvalidConfigurationError +from commitizen.exceptions import InvalidConfigurationError, NoCommitsFoundError from commitizen.git import GitCommit, GitTag @@ -309,7 +309,7 @@ def get_oldest_and_newest_rev( tags_range = get_smart_tag_range(tags, newest=newest_tag, oldest=oldest_tag) if not tags_range: - return None, None + raise NoCommitsFoundError("Could not find a valid revision range.") oldest_rev: Optional[str] = tags_range[-1].name newest_rev = newest_tag diff --git a/tests/commands/test_changelog_command.py b/tests/commands/test_changelog_command.py index 900915ec88..3889af335d 100644 --- a/tests/commands/test_changelog_command.py +++ b/tests/commands/test_changelog_command.py @@ -633,6 +633,7 @@ def test_changelog_from_rev_latest_version_from_arg( def test_changelog_from_rev_single_version_not_found( mocker, config_path, changelog_path ): + """Provides an invalid revision ID to changelog command""" with open(config_path, "a") as f: f.write('tag_format = "$version"\n') @@ -657,12 +658,13 @@ def test_changelog_from_rev_single_version_not_found( with pytest.raises(NoCommitsFoundError) as excinfo: cli.main() - assert "No commits found" in str(excinfo) + assert "Could not find a valid revision" in str(excinfo) @pytest.mark.usefixtures("tmp_commitizen_project") @pytest.mark.freeze_time("2022-02-13") def test_changelog_from_rev_range_version_not_found(mocker, config_path): + """Provides an invalid end revision ID to changelog command""" with open(config_path, "a") as f: f.write('tag_format = "$version"\n') @@ -684,7 +686,7 @@ def test_changelog_from_rev_range_version_not_found(mocker, config_path): with pytest.raises(NoCommitsFoundError) as excinfo: cli.main() - assert "No commits found" in str(excinfo) + assert "Could not find a valid revision" in str(excinfo) @pytest.mark.usefixtures("tmp_commitizen_project") From 91ed06516f9494f3b12b343503336ef96bfb6bd6 Mon Sep 17 00:00:00 2001 From: bhelgs <33836927+bhelgs@users.noreply.github.com> Date: Mon, 1 Aug 2022 19:38:15 -0400 Subject: [PATCH 5/8] test(check): fixes logic issue made evident by the latest fix(git) commit git was failing with "fatal: ambiguous argument 'master..master': unknown revision or path not in the working tree". git `master` branch doesn't exist unless there is at least one "initial" commit or when only the PR branch has been cloned (e.g. CI). --- tests/commands/test_check_command.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/commands/test_check_command.py b/tests/commands/test_check_command.py index 6fbbf322cf..6f1b22abc3 100644 --- a/tests/commands/test_check_command.py +++ b/tests/commands/test_check_command.py @@ -10,6 +10,7 @@ InvalidCommitMessageError, NoCommitsFoundError, ) +from tests.utils import create_file_and_commit COMMIT_LOG = [ "refactor: A code change that neither fixes a bug nor adds a feature", @@ -217,7 +218,12 @@ def test_check_command_with_invalid_argument(config): ) +@pytest.mark.usefixtures("tmp_commitizen_project") def test_check_command_with_empty_range(config, mocker): + + # must initialize git with a commit + create_file_and_commit("feat: initial") + check_cmd = commands.Check(config=config, arguments={"rev_range": "master..master"}) with pytest.raises(NoCommitsFoundError) as excinfo: check_cmd() From 991e8297bc2a58b38063a781a211172769e24741 Mon Sep 17 00:00:00 2001 From: bhelgs <33836927+bhelgs@users.noreply.github.com> Date: Mon, 1 Aug 2022 15:14:01 -0400 Subject: [PATCH 6/8] fix(git): Improves error checking in get_tags `git tag` is has return code of zero whether or not tags exist so we should always expect the code to be zero. --- commitizen/git.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/commitizen/git.py b/commitizen/git.py index 6ad2440a89..39c3a78ff8 100644 --- a/commitizen/git.py +++ b/commitizen/git.py @@ -5,7 +5,7 @@ from tempfile import NamedTemporaryFile from typing import List, Optional -from commitizen import cmd +from commitizen import cmd, out from commitizen.exceptions import GitCommandError UNIX_EOL = "\n" @@ -150,8 +150,13 @@ def get_tags(dateformat: str = "%Y-%m-%d") -> List[GitTag]: f'%(object)"' ) c = cmd.run(f"git tag --format={formatter} --sort=-creatordate") + if c.return_code != 0: + raise GitCommandError(c.err) - if c.err or not c.out: + if c.err: + out.warn(f"Attempting to proceed after: {c.err}") + + if not c.out: return [] git_tags = [ From d5b67b0c7ae9a14b79442adfab32c106b1473970 Mon Sep 17 00:00:00 2001 From: bhelgs <33836927+bhelgs@users.noreply.github.com> Date: Mon, 1 Aug 2022 22:19:14 -0400 Subject: [PATCH 7/8] test(changelog): code coverage improvements --- commitizen/commands/changelog.py | 4 ---- tests/commands/test_changelog_command.py | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/commitizen/commands/changelog.py b/commitizen/commands/changelog.py index d8d8768c81..0520b95257 100644 --- a/commitizen/commands/changelog.py +++ b/commitizen/commands/changelog.py @@ -16,10 +16,6 @@ from commitizen.git import GitTag, smart_open -def similar(a, b): - return SequenceMatcher(None, a, b).ratio() - - class Changelog: """Generate a changelog based on the commit history.""" diff --git a/tests/commands/test_changelog_command.py b/tests/commands/test_changelog_command.py index 3889af335d..e6b6a4b38b 100644 --- a/tests/commands/test_changelog_command.py +++ b/tests/commands/test_changelog_command.py @@ -351,6 +351,15 @@ def test_changelog_without_revision(mocker, tmp_commitizen_project): cli.main() +def test_changelog_incremental_with_revision(mocker): + """combining incremental with a revision doesn't make sense""" + testargs = ["cz", "changelog", "--incremental", "0.2.0"] + mocker.patch.object(sys, "argv", testargs) + + with pytest.raises(NotAllowed): + cli.main() + + def test_changelog_with_different_tag_name_and_changelog_content( mocker, tmp_commitizen_project ): @@ -918,3 +927,15 @@ def test_changelog_with_customized_change_type_order( out = f.read() file_regression.check(out, extension=".md") + + +@pytest.mark.usefixtures("tmp_commitizen_project") +def test_empty_commit_list(mocker): + create_file_and_commit("feat: a new world") + + # test changelog properly handles when no commits are found for the revision + mocker.patch("commitizen.git.get_commits", return_value=[]) + testargs = ["cz", "changelog"] + mocker.patch.object(sys, "argv", testargs) + with pytest.raises(NoCommitsFoundError): + cli.main() From 7562808676d5618685a85576f43ca5cf887396c4 Mon Sep 17 00:00:00 2001 From: bhelgs <33836927+bhelgs@users.noreply.github.com> Date: Wed, 10 Aug 2022 21:56:06 -0400 Subject: [PATCH 8/8] refactor(git): test the git log parser behaves properly when the repository has no commits We should expect an exception raised or an empty list (no matter what version of git was used). --- commitizen/git.py | 38 +++++++++++++----------- tests/commands/test_changelog_command.py | 14 --------- tests/test_git.py | 12 +++++++- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/commitizen/git.py b/commitizen/git.py index 39c3a78ff8..babd41989b 100644 --- a/commitizen/git.py +++ b/commitizen/git.py @@ -105,27 +105,12 @@ def get_commits( start: Optional[str] = None, end: str = "HEAD", *, - log_format: str = "%H%n%s%n%an%n%ae%n%b", - delimiter: str = "----------commit-delimiter----------", args: str = "", ) -> List[GitCommit]: """Get the commits between start and end.""" - 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}" - c = cmd.run(command) - if c.return_code != 0: - raise GitCommandError(c.err) - if not c.out: - return [] - + git_log_entries = _get_log_as_str_list(start, end, args) git_commits = [] - for rev_and_commit in c.out.split(f"{delimiter}\n"): + for rev_and_commit in git_log_entries: if not rev_and_commit: continue rev, title, author, author_email, *body_list = rev_and_commit.split("\n") @@ -236,3 +221,22 @@ def get_eol_style() -> EOLTypes: def smart_open(*args, **kargs): """Open a file with the EOL style determined from Git.""" return open(*args, newline=get_eol_style().get_eol_for_open(), **kargs) + + +def _get_log_as_str_list(start: Optional[str], end: str, args: str) -> List[str]: + """Get string representation of each log entry""" + delimiter = "----------commit-delimiter----------" + log_format: str = "%H%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}" + c = cmd.run(command) + if c.return_code != 0: + raise GitCommandError(c.err) + if not c.out: + return [] + return c.out.split(f"{delimiter}\n") diff --git a/tests/commands/test_changelog_command.py b/tests/commands/test_changelog_command.py index e6b6a4b38b..2d5d67db9c 100644 --- a/tests/commands/test_changelog_command.py +++ b/tests/commands/test_changelog_command.py @@ -6,7 +6,6 @@ from commitizen.commands.changelog import Changelog from commitizen.exceptions import ( DryRunExit, - GitCommandError, NoCommitsFoundError, NoRevisionError, NotAGitProjectError, @@ -15,19 +14,6 @@ from tests.utils import create_file_and_commit, wait_for_tag -@pytest.mark.usefixtures("tmp_commitizen_project") -def test_changelog_on_empty_project(mocker): - testargs = ["cz", "changelog", "--dry-run"] - mocker.patch.object(sys, "argv", testargs) - - with pytest.raises(GitCommandError): - cli.main() - - # git will error out with something like: - # "your current branch 'XYZ' does not have any commits yet" - # is it wise to assert the exception contains a message like this? - - @pytest.mark.usefixtures("tmp_commitizen_project") def test_changelog_from_version_zero_point_two(mocker, capsys, file_regression): create_file_and_commit("feat: new file") diff --git a/tests/test_git.py b/tests/test_git.py index 42fd2e08be..ca51c8f2cd 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -5,7 +5,7 @@ import pytest -from commitizen import cmd, git +from commitizen import cmd, exceptions, git from tests.utils import FakeCommand, create_file_and_commit @@ -58,6 +58,16 @@ def test_git_message_with_empty_body(): assert commit.message == commit_title +@pytest.mark.usefixtures("tmp_commitizen_project") +def test_get_log_as_str_list_empty(): + """ensure an exception or empty list in an empty project""" + try: + gitlog = git._get_log_as_str_list(start=None, end="HEAD", args="") + except exceptions.GitCommandError: + return + assert len(gitlog) == 0, "list should be empty if no assert" + + @pytest.mark.usefixtures("tmp_commitizen_project") def test_get_commits(): create_file_and_commit("feat(users): add username")