Repository: commitizen-tools/commitizen

Issue:
`cz changelog` is not reporting git errors (`git.py` missing `cmd.return_code` checks)
### Description

`cz commit` worked just fine, but `cz changelog` resulted in the error:
> "No commits found"

I later came to find the calls to git were silently failing in `git.get_tags` due to an unsupported version of git.
It's not very obvious the issue is with git.
Might we want some kind of warnings/exceptions [here](https://github.com/commitizen-tools/commitizen/blob/afa0d93544405bb0a871b5072e0a814720d933be/commitizen/git.py#L123)"?


### Steps to reproduce

1. unsupported git (e.g. 1.8.3.1) (normally we use a newer version)
2. `cz changelog` (`cz commit` worked)

### Current behavior

Error handling could be improved.

### Desired behavior

Maybe a warning or `raise` would help users out when `cmd.run( ...git...)` fails?

### Screenshots

_No response_

### Environment

CentOS7

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/commitizen-tools/commitizen/pull/540?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#540](https://codecov.io/gh/commitizen-tools/commitizen/pull/540?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (3551756) into [master](https://codecov.io/gh/commitizen-tools/commitizen/commit/53c66f642d630dacb44fbeefd73e5d33f59d095f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (53c66f6) will **decrease** coverage by `0.30%`.
> The diff coverage is `93.33%`.

```diff
@@            Coverage Diff             @@
##           master     #540      +/-   ##
==========================================
- Coverage   98.01%   97.70%   -0.31%     
==========================================
  Files          39       39              
  Lines        1563     1570       +7     
==========================================
+ Hits         1532     1534       +2     
- Misses         31       36       +5     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `97.70% <93.33%> (-0.31%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/commitizen-tools/commitizen/pull/540?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [commitizen/git.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/540/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9naXQucHk=) | `99.13% <88.88%> (-0.87%)` | :arrow_down: |
| [commitizen/\_\_version\_\_.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/540/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9fX3ZlcnNpb25fXy5weQ==) | `100.00% <100.00%> (ø)` | |
| [commitizen/changelog.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/540/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9jaGFuZ2Vsb2cucHk=) | `96.17% <100.00%> (-1.10%)` | :arrow_down: |
| [commitizen/exceptions.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/540/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9leGNlcHRpb25zLnB5) | `100.00% <100.00%> (ø)` | |
| [commitizen/cli.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/540/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9jbGkucHk=) | `92.42% <0.00%> (-1.52%)` | :arrow_down: |
| [commitizen/commands/changelog.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/540/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9jb21tYW5kcy9jaGFuZ2Vsb2cucHk=) | `95.45% <0.00%> (-1.14%)` | :arrow_down: |
| [commitizen/cz/base.py](https://codecov.io/gh/commitizen-tools/commitizen/pull/540/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tbWl0aXplbi9jei9iYXNlLnB5) | `100.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/commitizen-tools/commitizen/pull/540?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/commitizen-tools/commitizen/pull/540?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Last update [362ec36...3551756](https://codecov.io/gh/commitizen-tools/commitizen/pull/540?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).


Patch:
diff --git a/commitizen/changelog.py b/commitizen/changelog.py
index 1ccdb4e2..ead80ab7 100644
--- a/commitizen/changelog.py
+++ b/commitizen/changelog.py
@@ -35,7 +35,7 @@ from jinja2 import Environment, PackageLoader
 
 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/commitizen/commands/changelog.py b/commitizen/commands/changelog.py
index d8d8768c..0520b952 100644
--- a/commitizen/commands/changelog.py
+++ b/commitizen/commands/changelog.py
@@ -16,10 +16,6 @@ from commitizen.exceptions import (
 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/commitizen/exceptions.py b/commitizen/exceptions.py
index 16869b5a..17a91778 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 935686c3..babd4198 100644
--- a/commitizen/git.py
+++ b/commitizen/git.py
@@ -5,7 +5,8 @@ from pathlib import Path
 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"
 WINDOWS_EOL = "\r\n"
@@ -104,25 +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 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")
@@ -147,8 +135,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:
+        out.warn(f"Attempting to proceed after: {c.err}")
 
-    if c.err or not c.out:
+    if not c.out:
         return []
 
     git_tags = [
@@ -228,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/docs/exit_codes.md b/docs/exit_codes.md
index b3996460..54be954f 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                                                              |


Test patch:
diff --git a/tests/commands/test_bump_command.py b/tests/commands/test_bump_command.py
index 927a7190..2c9ca367 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()
 
diff --git a/tests/commands/test_changelog_command.py b/tests/commands/test_changelog_command.py
index ed6aa619..2d5d67db 100644
--- a/tests/commands/test_changelog_command.py
+++ b/tests/commands/test_changelog_command.py
@@ -14,17 +14,6 @@ from commitizen.exceptions import (
 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(NoCommitsFoundError) as excinfo:
-        cli.main()
-
-    assert "No commits found" in str(excinfo)
-
-
 @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")
@@ -348,6 +337,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
 ):
@@ -630,6 +628,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')
 
@@ -654,12 +653,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')
 
@@ -681,7 +681,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")
@@ -913,3 +913,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()
diff --git a/tests/commands/test_check_command.py b/tests/commands/test_check_command.py
index 6fbbf322..6f1b22ab 100644
--- a/tests/commands/test_check_command.py
+++ b/tests/commands/test_check_command.py
@@ -10,6 +10,7 @@ from commitizen.exceptions import (
     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()
diff --git a/tests/test_git.py b/tests/test_git.py
index 42fd2e08..ca51c8f2 100644
--- a/tests/test_git.py
+++ b/tests/test_git.py
@@ -5,7 +5,7 @@ from typing import List, Optional
 
 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")