Fix AuthorName/AuthorEmail to respect env variables

When committing, the `GIT_AUTHOR_{NAME,EMAIL}` environment variables
take precedence over Git configuration values. Ensure these are
recognized by the hook.

This required a change to the commit integration spec since
GIT_AUTHOR_NAME/GIT_AUTHOR_EMAIL is set by git before it executes the
hook, so even if you set the local configuration to an empty string it
will fall back to using the global configuration.

Author: sds

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Overcommit Changelog
 
+## master (unreleased)
+
+* Fix `AuthorName`/`AuthorEmail` pre-commit hooks to respect
+  `GIT_AUTHOR_NAME`/`GIT_AUTHOR_EMAIL` environment variables, respectively
+
 ## 0.32.0
 
 ### New Features

lib/overcommit/hook/pre_commit/author_email.rb

@@ -2,14 +2,20 @@ module Overcommit::Hook::PreCommit
   # Checks the format of an author's email address.
   class AuthorEmail < Base
     def run
-      result = execute(%w[git config --get user.email])
-      email = result.stdout.chomp
+      email =
+        if ENV.key?('GIT_AUTHOR_EMAIL')
+          ENV['GIT_AUTHOR_EMAIL']
+        else
+          result = execute(%w[git config --get user.email])
+          result.stdout.chomp
+        end
 
       unless email =~ /#{config['pattern']}/
         return :fail,
                "Author has an invalid email address: '#{email}'\n" \
                'Set your email with ' \
-               '`git config --global user.email your_email@example.com`'
+               '`git config --global user.email your_email@example.com` ' \
+               'or via the GIT_AUTHOR_EMAIL environment variable'
       end
 
       :pass

lib/overcommit/hook/pre_commit/author_name.rb

@@ -2,13 +2,19 @@ module Overcommit::Hook::PreCommit
   # Ensures that a commit author has a name with at least first and last names.
   class AuthorName < Base
     def run
-      result = execute(%w[git config --get user.name])
-      name = result.stdout.chomp
+      name =
+        if ENV.key?('GIT_AUTHOR_NAME')
+          ENV['GIT_AUTHOR_NAME']
+        else
+          result = execute(%w[git config --get user.name])
+          result.stdout.chomp
+        end
 
       unless name.split(' ').count >= 2
         return :fail,
                "Author must have at least first and last name, but was: #{name}.\n" \
-               'Set your name with `git config --global user.name "Your Name"`'
+               'Set your name with `git config --global user.name "Your Name"` ' \
+               'or via the GIT_AUTHOR_NAME environment variable'
       end
 
       :pass

spec/integration/committing_spec.rb

@@ -24,7 +24,7 @@
 
   context 'when a hook fails' do
     before do
-      `git config --local user.name ""`
+      `git config --local user.name "John"`
     end
 
     it 'exits with a non-zero status' do

spec/overcommit/hook/pre_commit/author_email_spec.rb

@@ -6,50 +6,66 @@
   subject { described_class.new(config, context) }
   let(:result) { double('result') }
 
-  before do
-    result.stub(:stdout).and_return(email)
-    subject.stub(:execute).and_return(result)
-  end
-
-  context 'when user has no email' do
-    let(:email) { '' }
+  shared_examples_for 'author email check' do
+    context 'when user has no email' do
+      let(:email) { '' }
 
-    it { should fail_hook }
-  end
+      it { should fail_hook }
+    end
 
-  context 'when user has an invalid email' do
-    let(:email) { 'Invalid Email' }
+    context 'when user has an invalid email' do
+      let(:email) { 'Invalid Email' }
 
-    it { should fail_hook }
-  end
+      it { should fail_hook }
+    end
 
-  context 'when user has a valid email' do
-    let(:email) { 'email@example.com' }
+    context 'when user has a valid email' do
+      let(:email) { 'email@example.com' }
 
-    it { should pass }
-  end
+      it { should pass }
+    end
 
-  context 'when a custom pattern is specified' do
-    let(:config) do
-      super().merge(Overcommit::Configuration.new(
-        'PreCommit' => {
-          'AuthorEmail' => {
-            'pattern' => '^[^@]+@brigade\.com$'
+    context 'when a custom pattern is specified' do
+      let(:config) do
+        super().merge(Overcommit::Configuration.new(
+          'PreCommit' => {
+            'AuthorEmail' => {
+              'pattern' => '^[^@]+@brigade\.com$'
+            }
           }
-        }
-      ))
-    end
+        ))
+      end
 
-    context 'and the email does not match the pattern' do
-      let(:email) { 'email@example.com' }
+      context 'and the email does not match the pattern' do
+        let(:email) { 'email@example.com' }
 
-      it { should fail_hook }
+        it { should fail_hook }
+      end
+
+      context 'and the email matches the pattern' do
+        let(:email) { 'email@brigade.com' }
+
+        it { should pass }
+      end
     end
+  end
 
-    context 'and the email matches the pattern' do
-      let(:email) { 'email@brigade.com' }
+  context 'when email is set via config' do
+    before do
+      result.stub(:stdout).and_return(email)
+      subject.stub(:execute).and_return(result)
+    end
 
-      it { should pass }
+    it_should_behave_like 'author email check'
+  end
+
+  context 'when email is set via environment variable' do
+    around do |example|
+      Overcommit::Utils.with_environment 'GIT_AUTHOR_EMAIL' => email do
+        example.run
+      end
     end
+
+    it_should_behave_like 'author email check'
   end
 end

spec/overcommit/hook/pre_commit/author_name_spec.rb

@@ -6,26 +6,42 @@
   subject { described_class.new(config, context) }
   let(:result) { double('result') }
 
-  before do
-    result.stub(:stdout).and_return(name)
-    subject.stub(:execute).and_return(result)
-  end
+  shared_examples_for 'author name check' do
+    context 'when user has no name' do
+      let(:name) { '' }
+
+      it { should fail_hook }
+    end
+
+    context 'when user has only a first name' do
+      let(:name) { 'John' }
+
+      it { should fail_hook }
+    end
 
-  context 'when user has no name' do
-    let(:name) { '' }
+    context 'when user has first and last name' do
+      let(:name) { 'John Doe' }
 
-    it { should fail_hook }
+      it { should pass }
+    end
   end
 
-  context 'when user has only a first name' do
-    let(:name) { 'John' }
+  context 'when name is set via config' do
+    before do
+      result.stub(:stdout).and_return(name)
+      subject.stub(:execute).and_return(result)
+    end
 
-    it { should fail_hook }
+    it_should_behave_like 'author name check'
   end
 
-  context 'when user has first and last name' do
-    let(:name) { 'John Doe' }
+  context 'when name is set via environment variable' do
+    around do |example|
+      Overcommit::Utils.with_environment 'GIT_AUTHOR_NAME' => name do
+        example.run
+      end
+    end
 
-    it { should pass }
+    it_should_behave_like 'author name check'
   end
 end