Fail nicer when username missing

gitlint/git.py

@@ -29,6 +29,14 @@ def __init__(self):
             u"See https://git-scm.com/book/en/v2/Getting-Started-Installing-Git on how to install git.")
 
 
+class GitExitCodeError(GitContextError):
+    def __init__(self, command, stderr):
+        self.command = command
+        self.stderr = stderr
+        super(GitExitCodeError, self).__init__(
+            u"An error occurred while executing '{0}': {1}".format(command, stderr))
+
+
 def _git(*command_parts, **kwargs):
     """ Convenience function for running git commands. Automatically deals with exceptions and unicode. """
     git_kwargs = {'_tty_out': False}
@@ -49,12 +57,13 @@ def _git(*command_parts, **kwargs):
         error_msg_lower = error_msg.lower()
         if '_cwd' in git_kwargs and b"not a git repository" in error_msg_lower:
             error_msg = u"{0} is not a git repository.".format(git_kwargs['_cwd'])
-        elif (b"does not have any commits yet" in error_msg_lower or
-              b"ambiguous argument 'head': unknown revision" in error_msg_lower):
+            raise GitContextError(error_msg)
+
+        if (b"does not have any commits yet" in error_msg_lower or
+                b"ambiguous argument 'head': unknown revision" in error_msg_lower):
             raise GitContextError(u"Current branch has no commits. Gitlint requires at least one commit to function.")
-        else:
-            error_msg = u"An error occurred while executing '{0}': {1}".format(e.full_cmd, error_msg)
-        raise GitContextError(error_msg)
+
+        raise GitExitCodeError(e.full_cmd, error_msg)
 
 
 def git_version():
@@ -296,12 +305,18 @@ def __init__(self, context, commit_message):  # pylint: disable=super-init-not-c
     @property
     @cache
     def author_name(self):
-        return ustr(_git("config", "--get", "user.name", _cwd=self.context.repository_path)).strip()
+        try:
+            return ustr(_git("config", "--get", "user.name", _cwd=self.context.repository_path)).strip()
+        except GitExitCodeError:
+            raise GitContextError("Missing git configuration: please set user.name")
 
     @property
     @cache
     def author_email(self):
-        return ustr(_git("config", "--get", "user.email", _cwd=self.context.repository_path)).strip()
+        try:
+            return ustr(_git("config", "--get", "user.email", _cwd=self.context.repository_path)).strip()
+        except GitExitCodeError:
+            raise GitContextError("Missing git configuration: please set user.email")
 
     @property
     @cache

gitlint/tests/git/test_git_commit.py

@@ -14,7 +14,9 @@
     from unittest.mock import patch, call  # pylint: disable=no-name-in-module, import-error
 
 from gitlint.tests.base import BaseTestCase
-from gitlint.git import GitContext, GitCommit, LocalGitCommit, StagedLocalGitCommit, GitCommitMessage
+from gitlint.git import GitContext, GitCommit, GitContextError, LocalGitCommit, StagedLocalGitCommit, GitCommitMessage
+from gitlint.shell import ErrorReturnCode
+from gitlint.utils import ustr
 
 
 class GitCommitTests(BaseTestCase):
@@ -479,6 +481,35 @@ def test_staged_commit(self, now, sh):
         self.assertListEqual(last_commit.changed_files, ["file1.txt", u"påth/to/file2.txt"])
         self.assertListEqual(sh.git.mock_calls, expected_calls[0:5])
 
+    @patch('gitlint.git.sh')
+    def test_staged_commit_with_missing_username(self, sh):
+        # StagedLocalGitCommit()
+
+        sh.git.side_effect = [
+            u"#",                               # git config --get core.commentchar
+            ErrorReturnCode('git config --get user.name', b"", b""),
+        ]
+
+        expected_msg = "Missing git configuration: please set user.name"
+        with self.assertRaisesMessage(GitContextError, expected_msg):
+            ctx = GitContext.from_staged_commit(u"Foōbar 123\n\ncömmit-body\n", u"fåke/path")
+            [ustr(commit) for commit in ctx.commits]
+
+    @patch('gitlint.git.sh')
+    def test_staged_commit_with_missing_email(self, sh):
+        # StagedLocalGitCommit()
+
+        sh.git.side_effect = [
+            u"#",                               # git config --get core.commentchar
+            u"test åuthor\n",                   # git config --get user.name
+            ErrorReturnCode('git config --get user.name', b"", b""),
+        ]
+
+        expected_msg = "Missing git configuration: please set user.email"
+        with self.assertRaisesMessage(GitContextError, expected_msg):
+            ctx = GitContext.from_staged_commit(u"Foōbar 123\n\ncömmit-body\n", u"fåke/path")
+            [ustr(commit) for commit in ctx.commits]
+
     def test_gitcommitmessage_equality(self):
         commit_message1 = GitCommitMessage(GitContext(), u"tëst\n\nfoo", u"tëst\n\nfoo", u"tēst", ["", u"föo"])
         attrs = ['original', 'full', 'title', 'body']

qa/base.py

@@ -93,6 +93,13 @@ def create_file(parent_dir):
         io.open(os.path.join(parent_dir, test_filename), 'a', encoding=DEFAULT_ENCODING).close()
         return test_filename
 
+    def create_tmp_git_config(self, contents):
+        """ Creates an environment with the GIT_CONFIG variable set to a file with the given contents. """
+        tmp_config = self.create_tmpfile(contents)
+        env = os.environ.copy()
+        env["GIT_CONFIG"] = tmp_config
+        return env
+
     def create_simple_commit(self, message, out=None, ok_code=None, env=None, git_repo=None, tty_in=False):
         """ Creates a simple commit with an empty test file.
             :param message: Commit message for the commit. """

qa/shell.py

@@ -67,6 +67,8 @@ def _exec(*args, **kwargs):
         popen_kwargs = {'stdout': pipe, 'stderr': pipe, 'shell': kwargs.get('_tty_out', False)}
         if '_cwd' in kwargs:
             popen_kwargs['cwd'] = kwargs['_cwd']
+        if '_env' in kwargs:
+            popen_kwargs['env'] = kwargs['_env']
 
         try:
             p = subprocess.Popen(args, **popen_kwargs)

qa/test_gitlint.py

@@ -157,6 +157,28 @@ def test_msg_filename_no_tty(self):
 
         self.assertEqualStdout(output, self.get_expected("test_gitlint/test_msg_filename_no_tty_1"))
 
+    def test_no_git_name_set(self):
+        """ Ensure we print out a helpful message if user.name is not set """
+        tmp_commit_msg_file = self.create_tmpfile(u"WIP: msg-fïlename NO name test.")
+        # Name is checked before email so this isn't strictly
+        # necessary but seems good for consistency.
+        env = self.create_tmp_git_config(u"[user]\n  email = test-emåil@foo.com\n")
+        output = gitlint("--staged", "--msg-filename", tmp_commit_msg_file,
+                         _ok_code=[self.GIT_CONTEXT_ERROR_CODE],
+                         _env=env)
+        expected = u"Missing git configuration: please set user.name\n"
+        self.assertEqualStdout(output, expected)
+
+    def test_no_git_email_set(self):
+        """ Ensure we print out a helpful message if user.email is not set """
+        tmp_commit_msg_file = self.create_tmpfile(u"WIP: msg-fïlename NO email test.")
+        env = self.create_tmp_git_config(u"[user]\n  name = test åuthor\n")
+        output = gitlint("--staged", "--msg-filename", tmp_commit_msg_file,
+                         _ok_code=[self.GIT_CONTEXT_ERROR_CODE],
+                         _env=env)
+        expected = u"Missing git configuration: please set user.email\n"
+        self.assertEqualStdout(output, expected)
+
     def test_git_errors(self):
         # Repo has no commits: caused by `git log`
         empty_git_repo = self.create_tmp_git_repo()