Git command execution convenience function

This commit introduces the gitlint.git._git(...) convenience function for
executing git commands.

The main benefit of this is that all error and unicode handling of git commands
is now uniform as it's done in a single place.

In addition, this refactor has the effect of the 'sh' library only being invoked
in a single place in the entire gitlint codebase. This will make it easier to
replace it in the future, for example when we'd consider adding Windows support
(see #20).

This commit also fixes some unit tests related to custom comment chars as
introduced by #34.

4 files changed, 153 insertions, 145 deletions

diff --git a/gitlint/cli.py b/gitlint/cli.py
index a9dc19a..6bf0b27 100644
--- a/gitlint/cli.py
+++ b/gitlint/cli.py
@@ -113,22 +113,27 @@ def build_config(ctx, target, config_path, c, extra_path, ignore, verbose, silen
 def cli(ctx, target, config, c, commits, extra_path, ignore, verbose, silent, debug):
     """ Git lint tool, checks your git commit messages for styling issues """
 
-    if debug:
-        logging.getLogger("gitlint").setLevel(logging.DEBUG)
+    try:
+        if debug:
+            logging.getLogger("gitlint").setLevel(logging.DEBUG)
+
+        log_system_info()
 
-    log_system_info()
+        # Get the lint config from the commandline parameters and
+        # store it in the context (click allows storing an arbitrary object in ctx.obj).
+        config, config_builder = build_config(ctx, target, config, c, extra_path, ignore, verbose, silent, debug)
 
-    # Get the lint config from the commandline parameters and
-    # store it in the context (click allows storing an arbitrary object in ctx.obj).
-    config, config_builder = build_config(ctx, target, config, c, extra_path, ignore, verbose, silent, debug)
+        LOG.debug(u"Configuration\n%s", ustr(config))
 
-    LOG.debug(u"Configuration\n%s", ustr(config))
+        ctx.obj = (config, config_builder, commits)
 
-    ctx.obj = (config, config_builder, commits)
+        # If no subcommand is specified, then just lint
+        if ctx.invoked_subcommand is None:
+            ctx.invoke(lint)
 
-    # If no subcommand is specified, then just lint
-    if ctx.invoked_subcommand is None:
-        ctx.invoke(lint)
+    except GitContextError as e:
+        click.echo(ustr(e))
+        ctx.exit(GIT_CONTEXT_ERROR_CODE)
 
 
 @cli.command("lint")
@@ -136,16 +141,13 @@ def cli(ctx, target, config, c, commits, extra_path, ignore, verbose, silent, de
 def lint(ctx):
     """ Lints a git repository [default command] """
     lint_config = ctx.obj[0]
-    try:
-        if sys.stdin.isatty():
-            # If target has not been set explicitly before, fallback to the current directory
-            gitcontext = GitContext.from_local_repository(lint_config.target, ctx.obj[2])
-        else:
-            stdin_str = ustr(sys.stdin.read())
-            gitcontext = GitContext.from_commit_msg(stdin_str)
-    except GitContextError as e:
-        click.echo(ustr(e))
-        ctx.exit(GIT_CONTEXT_ERROR_CODE)
+
+    if sys.stdin.isatty():
+        # If target has not been set explicitly before, fallback to the current directory
+        gitcontext = GitContext.from_local_repository(lint_config.target, ctx.obj[2])
+    else:
+        stdin_str = ustr(sys.stdin.read())
+        gitcontext = GitContext.from_commit_msg(stdin_str)
 
     number_of_commits = len(gitcontext.commits)
     # Exit if we don't have commits in the specified range. Use a 0 exit code, since a popular use-case is one
diff --git a/gitlint/git.py b/gitlint/git.py
index 62730c6..e5e82f3 100644
--- a/gitlint/git.py
+++ b/gitlint/git.py
@@ -18,28 +18,42 @@ class GitNotInstalledError(GitContextError):
             u"See https://git-scm.com/book/en/v2/Getting-Started-Installing-Git on how to install git.")
 
 
-def git_version():
-    """ Determine the git version installed on this host by calling git --version"""
+def _git(*command_parts, **kwargs):
+    """ Convenience function for running git commands. Automatically deals with exceptions and unicode. """
+    # Special arguments passed to sh: http://amoffat.github.io/sh/special_arguments.html
+    git_kwargs = {'_tty_out': False}
+    git_kwargs.update(kwargs)
     try:
-        version = ustr(sh.git("--version")).replace(u"\n", u"")
+        result = sh.git(*command_parts, **git_kwargs)
+        # If we reach this point and the result has an exit_code that is larger than 0, this means that we didn't
+        # get an exception (which is the default sh behavior for non-zero exit codes) and so the user is expecting
+        # a non-zero exit code -> just return the entire result
+        if hasattr(result, 'exit_code') and result.exit_code > 0:
+            return result
+        return ustr(result)
     except CommandNotFound:
         raise GitNotInstalledError()
     except ErrorReturnCode as e:  # Something went wrong while executing the git command
         error_msg = e.stderr.strip()
-        error_msg = u"An error occurred while executing '{0}': {1}".format(e.full_cmd, error_msg)
+        if '_cwd' in git_kwargs and b"Not a git repository" in error_msg:
+            error_msg = u"{0} is not a git repository.".format(git_kwargs['_cwd'])
+        else:
+            error_msg = u"An error occurred while executing '{0}': {1}".format(e.full_cmd, error_msg)
         raise GitContextError(error_msg)
-    return version
+
+
+def git_version():
+    """ Determine the git version installed on this host by calling git --version"""
+    return _git("--version").replace(u"\n", u"")
 
 
 def git_commentchar():
-    """ Shortcut for retrieving comment char from git config
-    """
-    try:
-        commentchar = ustr(sh.git.config('--get', 'core.commentchar')).replace(u"\n", u"")
-    except sh.ErrorReturnCode_1:  # pylint: disable=no-member
-        # exception means that default commentchar used
-        commentchar = '#'
-    return commentchar
+    """ Shortcut for retrieving comment char from git config """
+    commentchar = _git("config", "--get", "core.commentchar", _ok_code=[1])
+    # git will return an exit code of 1 if it can't find a config value, in this case we fall-back to # as commentchar
+    if commentchar.exit_code == 1:  # pylint: disable=no-member
+        commentchar = "#"
+    return ustr(commentchar).replace(u"\n", u"")
 
 
 class GitCommitMessage(object):
@@ -155,57 +169,40 @@ class GitContext(object):
         """
 
         context = GitContext()
-        try:
-            # Special arguments passed to sh: http://amoffat.github.io/sh/special_arguments.html
-            sh_special_args = {
-                '_tty_out': False,
-                '_cwd': repository_path
-            }
-
-            if refspec is None:
-                # We tried many things here e.g.: defaulting to e.g. HEAD or HEAD^... (incl. dealing with
-                # repos that only have a single commit - HEAD^... doesn't work there), but then we still get into
-                # problems with e.g. merge commits. Easiest solution is just taking the SHA from `git log -1`.
-                sha_list = [sh.git.log("-1", "--pretty=%H", **sh_special_args).replace("\n", "")]
-            else:
-                sha_list = sh.git("rev-list", refspec, **sh_special_args).split()
-
-            for sha in sha_list:
-                # Get info from the local git repository: https://git-scm.com/docs/pretty-formats
-                raw_commit = sh.git.log(sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B",
-                                        **sh_special_args).split("\n")
-
-                (name, email, date, parents), commit_msg = raw_commit[0].split(","), "\n".join(raw_commit[1:])
-
-                commit_parents = parents.split(" ")
-                commit_is_merge_commit = len(commit_parents) > 1
-
-                # changed files in last commit
-                changed_files = sh.git("diff-tree", "--no-commit-id", "--name-only",
-                                       "-r", sha, **sh_special_args).split()
-
-                # "YYYY-MM-DD HH:mm:ss Z" -> ISO 8601-like format
-                # Use arrow for datetime parsing, because apparently python is quirky around ISO-8601 dates:
-                # http://stackoverflow.com/a/30696682/381010
-                commit_date = arrow.get(ustr(date), "YYYY-MM-DD HH:mm:ss Z").datetime
-
-                # Create Git commit object with the retrieved info
-                commit_msg_obj = GitCommitMessage.from_full_message(commit_msg)
-                commit = GitCommit(context, commit_msg_obj, sha=sha, author_name=name,
-                                   author_email=email, date=commit_date, changed_files=changed_files,
-                                   parents=commit_parents, is_merge_commit=commit_is_merge_commit)
-
-                context.commits.append(commit)
-
-        except CommandNotFound:
-            raise GitNotInstalledError()
-        except ErrorReturnCode as e:  # Something went wrong while executing the git command
-            error_msg = e.stderr.strip()
-            if b"Not a git repository" in error_msg:
-                error_msg = u"{0} is not a git repository.".format(repository_path)
-            else:
-                error_msg = u"An error occurred while executing '{0}': {1}".format(e.full_cmd, error_msg)
-            raise GitContextError(error_msg)
+
+        # If no refspec is defined, fallback to the last commit on the current branch
+        if refspec is None:
+            # We tried many things here e.g.: defaulting to e.g. HEAD or HEAD^... (incl. dealing with
+            # repos that only have a single commit - HEAD^... doesn't work there), but then we still get into
+            # problems with e.g. merge commits. Easiest solution is just taking the SHA from `git log -1`.
+            sha_list = [_git("log", "-1", "--pretty=%H", _cwd=repository_path).replace(u"\n", u"")]
+        else:
+            sha_list = _git("rev-list", refspec, _cwd=repository_path).split()
+
+        for sha in sha_list:
+            # Get info from the local git repository: https://git-scm.com/docs/pretty-formats
+            raw_commit = _git("log", sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", _cwd=repository_path).split("\n")
+
+            (name, email, date, parents), commit_msg = raw_commit[0].split(","), "\n".join(raw_commit[1:])
+
+            commit_parents = parents.split(" ")
+            commit_is_merge_commit = len(commit_parents) > 1
+
+            # changed files in last commit
+            changed_files = _git("diff-tree", "--no-commit-id", "--name-only", "-r", sha, _cwd=repository_path).split()
+
+            # "YYYY-MM-DD HH:mm:ss Z" -> ISO 8601-like format
+            # Use arrow for datetime parsing, because apparently python is quirky around ISO-8601 dates:
+            # http://stackoverflow.com/a/30696682/381010
+            commit_date = arrow.get(ustr(date), "YYYY-MM-DD HH:mm:ss Z").datetime
+
+            # Create Git commit object with the retrieved info
+            commit_msg_obj = GitCommitMessage.from_full_message(commit_msg)
+            commit = GitCommit(context, commit_msg_obj, sha=sha, author_name=name,
+                               author_email=email, date=commit_date, changed_files=changed_files,
+                               parents=commit_parents, is_merge_commit=commit_is_merge_commit)
+
+            context.commits.append(commit)
 
         return context
 
diff --git a/gitlint/tests/test_cli.py b/gitlint/tests/test_cli.py
index 07d295b..27738ed 100644
--- a/gitlint/tests/test_cli.py
+++ b/gitlint/tests/test_cli.py
@@ -49,12 +49,10 @@ class CLITests(BaseTestCase):
         """ Test for basic simple linting functionality """
         sys.stdin.isatty.return_value = True
 
-        def git_log_side_effect(*_args, **_kwargs):
-            return (u"test åuthor,test-email@föo.com,2016-12-03 15:28:15 01:00,åbc\n"
-                    u"commït-title\n\ncommït-body")
-
-        sh.git.log.side_effect = git_log_side_effect
-        sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360", u"file1.txt\npåth/to/file2.txt\n"]
+        sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360",
+                              u"test åuthor,test-email@föo.com,2016-12-03 15:28:15 01:00,åbc\n"
+                              u"commït-title\n\ncommït-body",
+                              u"file1.txt\npåth/to/file2.txt\n"]
 
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
             result = self.cli.invoke(cli.cli)
@@ -67,17 +65,18 @@ class CLITests(BaseTestCase):
         """ Test for --commits option """
         sys.stdin.isatty.return_value = True
 
-        sh.git.log.side_effect = [u"test åuthor1,test-email1@föo.com,2016-12-03 15:28:15 01:00,åbc\n"
-                                  u"commït-title1\n\ncommït-body1",
-                                  u"test åuthor2,test-email3@föo.com,2016-12-04 15:28:15 01:00,åbc\n"
-                                  u"commït-title2\n\ncommït-body2",
-                                  u"test åuthor3,test-email3@föo.com,2016-12-05 15:28:15 01:00,åbc\n"
-                                  u"commït-title3\n\ncommït-body3", ]
         sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360\n" +  # git rev-list <SHA>
                               "25053ccec5e28e1bb8f7551fdbb5ab213ada2401\n" +
                               "4da2656b0dadc76c7ee3fd0243a96cb64007f125\n",
+                              # git log --pretty <FORMAT> <SHA>
+                              u"test åuthor1,test-email1@föo.com,2016-12-03 15:28:15 01 :00,åbc\n"
+                              u"commït-title1\n\ncommït-body1",
                               u"file1.txt\npåth/to/file2.txt\n",  # git diff-tree <SHA>
+                              u"test åuthor2,test-email3@föo.com,2016-12-04 15:28:15 01:00,åbc\n"
+                              u"commït-title2\n\ncommït-body2",
                               u"file4.txt\npåth/to/file5.txt\n",
+                              u"test åuthor3,test-email3@föo.com,2016-12-05 15:28:15 01:00,åbc\n"
+                              u"commït-title3\n\ncommït-body3",
                               u"file6.txt\npåth/to/file7.txt\n"]
 
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
@@ -98,17 +97,18 @@ class CLITests(BaseTestCase):
         sys.stdin.isatty.return_value = True
 
         # Note that the second commit title has a trailing period that is being ignored by gitlint-ignore: T3
-        sh.git.log.side_effect = [u"test åuthor1,test-email1@föo.com,2016-12-03 15:28:15 01:00,åbc\n"
-                                  u"commït-title1\n\ncommït-body1",
-                                  u"test åuthor2,test-email3@föo.com,2016-12-04 15:28:15 01:00,åbc\n"
-                                  u"commït-title2.\n\ncommït-body2\ngitlint-ignore: T3\n",
-                                  u"test åuthor3,test-email3@föo.com,2016-12-05 15:28:15 01:00,åbc\n"
-                                  u"commït-title3\n\ncommït-body3", ]
         sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360\n" +  # git rev-list <SHA>
                               "25053ccec5e28e1bb8f7551fdbb5ab213ada2401\n" +
                               "4da2656b0dadc76c7ee3fd0243a96cb64007f125\n",
+                              # git log --pretty <FORMAT> <SHA>
+                              u"test åuthor1,test-email1@föo.com,2016-12-03 15:28:15 01:00,åbc\n"
+                              u"commït-title1\n\ncommït-body1",
                               u"file1.txt\npåth/to/file2.txt\n",  # git diff-tree <SHA>
+                              u"test åuthor2,test-email3@föo.com,2016-12-04 15:28:15 01:00,åbc\n"
+                              u"commït-title2.\n\ncommït-body2\ngitlint-ignore: T3\n",
                               u"file4.txt\npåth/to/file5.txt\n",
+                              u"test åuthor3,test-email3@föo.com,2016-12-05 15:28:15 01:00,åbc\n"
+                              u"commït-title3\n\ncommït-body3",
                               u"file6.txt\npåth/to/file7.txt\n"]
 
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
@@ -176,17 +176,17 @@ class CLITests(BaseTestCase):
 
         sys.stdin.isatty.return_value = True
 
-        sh.git.log.side_effect = [u"test åuthor1,test-email1@föo.com,2016-12-03 15:28:15 01:00,abc\n"
-                                  u"commït-title1\n\ncommït-body1",
-                                  u"test åuthor2,test-email2@föo.com,2016-12-04 15:28:15 01:00,abc\n"
-                                  u"commït-title2.\n\ncommït-body2",
-                                  u"test åuthor3,test-email3@föo.com,2016-12-05 15:28:15 01:00,abc\n"
-                                  u"föo\nbar"]
         sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360\n"  # git rev-list <SHA>
                               "25053ccec5e28e1bb8f7551fdbb5ab213ada2401\n"
                               "4da2656b0dadc76c7ee3fd0243a96cb64007f125\n",
-                              u"file1.txt\npåth/to/file2.txt\n",  # git diff-tree <SHA>
+                              u"test åuthor1,test-email1@föo.com,2016-12-03 15:28:15 01:00,abc\n"
+                              u"commït-title1\n\ncommït-body1",
+                              u"file1.txt\npåth/to/file2.txt\n",
+                              u"test åuthor2,test-email2@föo.com,2016-12-04 15:28:15 01:00,abc\n"
+                              u"commït-title2.\n\ncommït-body2",
                               u"file4.txt\npåth/to/file5.txt\n",
+                              u"test åuthor3,test-email3@föo.com,2016-12-05 15:28:15 01:00,abc\n"
+                              u"föo\nbar",
                               u"file6.txt\npåth/to/file7.txt\n"]
 
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
@@ -332,7 +332,7 @@ class CLITests(BaseTestCase):
     def test_git_error(self, sys, sh):
         """ Tests that the cli handles git errors properly """
         sys.stdin.isatty.return_value = True
-        sh.git.log.side_effect = CommandNotFound("git")
+        sh.git.side_effect = CommandNotFound("git")
         result = self.cli.invoke(cli.cli)
         self.assertEqual(result.exit_code, self.GIT_CONTEXT_ERROR_CODE)
 
diff --git a/gitlint/tests/test_git.py b/gitlint/tests/test_git.py
index ec6a8aa..a7bb754 100644
--- a/gitlint/tests/test_git.py
+++ b/gitlint/tests/test_git.py
@@ -6,7 +6,7 @@ from mock import patch, call, PropertyMock
 from sh import ErrorReturnCode, CommandNotFound
 
 from gitlint.tests.base import BaseTestCase
-from gitlint.git import GitContext, GitCommit, GitCommitMessage, GitContextError, GitNotInstalledError
+from gitlint.git import GitContext, GitCommit, GitCommitMessage, GitContextError, GitNotInstalledError, git_commentchar
 
 
 class GitTests(BaseTestCase):
@@ -19,15 +19,16 @@ class GitTests(BaseTestCase):
     def test_get_latest_commit(self, sh):
         sample_sha = "d8ac47e9f2923c7f22d8668e3a1ed04eb4cdbca9"
 
-        sh.git.side_effect = [u"file1.txt\npåth/to/file2.txt\n"]
-        sh.git.log.side_effect = [sample_sha, u"test åuthor,test-emåil@foo.com,2016-12-03 15:28:15 01:00,åbc\n"
-                                              u"cömmit-title\n\ncömmit-body"]
+        sh.git.side_effect = [sample_sha,
+                              u"test åuthor,test-emåil@foo.com,2016-12-03 15:28:15 01:00,åbc\n"
+                              u"cömmit-title\n\ncömmit-body",
+                              u"file1.txt\npåth/to/file2.txt\n"]
 
         context = GitContext.from_local_repository(u"fåke/path")
         # assert that commit info was read using git command
         expected_calls = [
-            call.log("-1", "--pretty=%H", **self.expected_sh_special_args),
-            call.log(sample_sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", **self.expected_sh_special_args),
+            call("log", "-1", "--pretty=%H", **self.expected_sh_special_args),
+            call("log", sample_sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", **self.expected_sh_special_args),
             call('diff-tree', '--no-commit-id', '--name-only', '-r', sample_sha, **self.expected_sh_special_args)
         ]
         self.assertListEqual(sh.git.mock_calls, expected_calls)
@@ -47,15 +48,16 @@ class GitTests(BaseTestCase):
     def test_from_local_repository_specific_ref(self, sh):
         sample_sha = "myspecialref"
 
-        sh.git.side_effect = [sample_sha, u"file1.txt\npåth/to/file2.txt\n"]
-        sh.git.log.side_effect = [u"test åuthor,test-emåil@foo.com,2016-12-03 15:28:15 01:00,åbc\n"
-                                  u"cömmit-title\n\ncömmit-body"]
+        sh.git.side_effect = [sample_sha,
+                              u"test åuthor,test-emåil@foo.com,2016-12-03 15:28:15 01:00,åbc\n"
+                              u"cömmit-title\n\ncömmit-body",
+                              u"file1.txt\npåth/to/file2.txt\n"]
 
         context = GitContext.from_local_repository(u"fåke/path", sample_sha)
         # assert that commit info was read using git command
         expected_calls = [
             call("rev-list", sample_sha, **self.expected_sh_special_args),
-            call.log(sample_sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", **self.expected_sh_special_args),
+            call("log", sample_sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", **self.expected_sh_special_args),
             call('diff-tree', '--no-commit-id', '--name-only', '-r', sample_sha, **self.expected_sh_special_args)
         ]
         self.assertListEqual(sh.git.mock_calls, expected_calls)
@@ -75,15 +77,16 @@ class GitTests(BaseTestCase):
     def test_get_latest_commit_merge_commit(self, sh):
         sample_sha = "d8ac47e9f2923c7f22d8668e3a1ed04eb4cdbca9"
 
-        sh.git.side_effect = [u"file1.txt\npåth/to/file2.txt\n"]
-        sh.git.log.side_effect = [sample_sha, u"test åuthor,test-emåil@foo.com,2016-12-03 15:28:15 01:00,åbc def\n"
-                                              u"Merge \"foo bår commit\""]
+        sh.git.side_effect = [sample_sha,
+                              u"test åuthor,test-emåil@foo.com,2016-12-03 15:28:15 01:00,åbc def\n"
+                              u"Merge \"foo bår commit\"",
+                              u"file1.txt\npåth/to/file2.txt\n"]
 
         context = GitContext.from_local_repository(u"fåke/path")
         # assert that commit info was read using git command
         expected_calls = [
-            call.log("-1", "--pretty=%H", **self.expected_sh_special_args),
-            call.log(sample_sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", **self.expected_sh_special_args),
+            call("log", "-1", "--pretty=%H", **self.expected_sh_special_args),
+            call("log", sample_sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", **self.expected_sh_special_args),
             call('diff-tree', '--no-commit-id', '--name-only', '-r', sample_sha, **self.expected_sh_special_args)
         ]
 
@@ -102,36 +105,36 @@ class GitTests(BaseTestCase):
 
     @patch('gitlint.git.sh')
     def test_get_latest_commit_command_not_found(self, sh):
-        sh.git.log.side_effect = CommandNotFound("git")
+        sh.git.side_effect = CommandNotFound("git")
         expected_msg = "'git' command not found. You need to install git to use gitlint on a local repository. " + \
                        "See https://git-scm.com/book/en/v2/Getting-Started-Installing-Git on how to install git."
         with self.assertRaisesRegex(GitNotInstalledError, expected_msg):
             GitContext.from_local_repository(u"fåke/path")
 
         # assert that commit message was read using git command
-        sh.git.log.assert_called_once_with("-1", "--pretty=%H", **self.expected_sh_special_args)
+        sh.git.assert_called_once_with("log", "-1", "--pretty=%H", **self.expected_sh_special_args)
 
     @patch('gitlint.git.sh')
     def test_get_latest_commit_git_error(self, sh):
         # Current directory not a git repo
         err = b"fatal: Not a git repository (or any of the parent directories): .git"
-        sh.git.log.side_effect = ErrorReturnCode("git log -1 --pretty=%H", b"", err)
+        sh.git.side_effect = ErrorReturnCode("git log -1 --pretty=%H", b"", err)
 
         with self.assertRaisesRegex(GitContextError, u"fåke/path is not a git repository."):
             GitContext.from_local_repository(u"fåke/path")
 
         # assert that commit message was read using git command
-        sh.git.log.assert_called_once_with("-1", "--pretty=%H", **self.expected_sh_special_args)
+        sh.git.assert_called_once_with("log", "-1", "--pretty=%H", **self.expected_sh_special_args)
         sh.git.reset_mock()
         err = b"fatal: Random git error"
-        sh.git.log.side_effect = ErrorReturnCode("git log -1 --pretty=%H", b"", err)
+        sh.git.side_effect = ErrorReturnCode("git log -1 --pretty=%H", b"", err)
 
         expected_msg = u"An error occurred while executing 'git log -1 --pretty=%H': {0}".format(err)
         with self.assertRaisesRegex(GitContextError, expected_msg):
             GitContext.from_local_repository(u"fåke/path")
 
         # assert that commit message was read using git command
-        sh.git.log.assert_called_once_with("-1", "--pretty=%H", **self.expected_sh_special_args)
+        sh.git.assert_called_once_with("log", "-1", "--pretty=%H", **self.expected_sh_special_args)
 
     def test_from_commit_msg_full(self):
         gitcontext = GitContext.from_commit_msg(self.get_sample("commit_message/sample1"))
@@ -255,16 +258,22 @@ class GitTests(BaseTestCase):
             setattr(commit1, attr, prev_val)
             self.assertEqual(commit1, commit2)
 
-    @patch('gitlint.git.sh')
-    def test_custom_commitchar(self, sh):
-        sh.git.config.return_value = ';'
-        from gitlint.git import GitCommitMessage  # pylint: disable=redefined-outer-name,reimported
-
-        with patch.object(GitCommitMessage, 'COMMENT_CHAR', new_callable=PropertyMock) as commit_char_mock:
-            commit_char_mock.return_value = ';'
-            message = GitCommitMessage.from_full_message(u"Tïtle\n\nBödy 1\n;Cömment\nBody 2")
-
-        self.assertEqual(message.title, u"Tïtle")
-        self.assertEqual(message.body, ["", u"Bödy 1", "Body 2"])
-        self.assertEqual(message.full, u"Tïtle\n\nBödy 1\nBody 2")
-        self.assertEqual(message.original, u"Tïtle\n\nBödy 1\n;Cömment\nBody 2")
+    @patch("gitlint.git._git")
+    def test_git_commentchar(self, git):
+        git.return_value.exit_code = 1
+        self.assertEqual(git_commentchar(), "#")
+
+        git.return_value.exit_code = 0
+        git.return_value.__str__ = lambda _: u"ä"
+        git.return_value.__unicode__ = lambda _: u"ä"
+        self.assertEqual(git_commentchar(), u"ä")
+
+    def test_commit_msg_custom_commentchar(self):
+        with patch.object(GitCommitMessage, 'COMMENT_CHAR', new_callable=PropertyMock) as comment_char_mock:
+            comment_char_mock.return_value = u"ä"
+            message = GitCommitMessage.from_full_message(u"Tïtle\n\nBödy 1\näCömment\nBody 2")
+
+            self.assertEqual(message.title, u"Tïtle")
+            self.assertEqual(message.body, ["", u"Bödy 1", "Body 2"])
+            self.assertEqual(message.full, u"Tïtle\n\nBödy 1\nBody 2")
+            self.assertEqual(message.original, u"Tïtle\n\nBödy 1\näCömment\nBody 2")