Add specs for commit-msg hooks

Change-Id: I469b1c6fc51ade36e41a4d38f4844f9b3df05584
Reviewed-on: http://gerrit.causes.com/35564
Reviewed-by: Shane da Silva <shane@causes.com>
Tested-by: Shane da Silva <shane@causes.com>

Author: sds

lib/overcommit/hook/commit_msg/gerrit_change_id.rb

@@ -6,11 +6,13 @@ module Overcommit::Hook::CommitMsg
   # you need to do it in a commit-msg hook. This is because the user could still
   # edit the message after a prepare-commit-msg hook was run.
   class GerritChangeId < Base
-    SCRIPT_LOCATION = Overcommit::Utils.script_path('gerrit-change-id')
-
     def run
-      command "#{SCRIPT_LOCATION} #{commit_message_file}"
-      :good
+      result = command("#{SCRIPT_LOCATION} #{commit_message_file}")
+      return (result.success? ? :good : :bad), result.stdout
     end
+
+  private
+
+    SCRIPT_LOCATION = Overcommit::Utils.script_path('gerrit-change-id')
   end
 end

lib/overcommit/hook/commit_msg/russian_novel.rb

@@ -4,7 +4,7 @@ class RussianNovel < Base
     RUSSIAN_NOVEL_LENGTH = 30
 
     def run
-      if commit_message.length > RUSSIAN_NOVEL_LENGTH
+      if commit_message.length >= RUSSIAN_NOVEL_LENGTH
         return :warn, 'You seem to have authored a Russian novel; congratulations!'
       end
 

lib/overcommit/hook_context/commit_msg.rb

@@ -1,24 +1,20 @@
 module Overcommit::HookContext
   class CommitMsg < Base
-    # User commit message stripped of comments and diff (from verbose output)
+    # User commit message stripped of comments and diff (from verbose output).
     def commit_message
-      @commit_message ||= raw_commit_message.
+      raw_commit_message.
         reject     { |line| line =~ /^#/ }.
         take_while { |line| !line.start_with?('diff --git') }
     end
 
-  private
-
-    def raw_commit_message
-      @raw_commit_message ||= ::IO.readlines(commit_message_file)
+    def commit_message_file
+      @args[0]
     end
 
-    def commit_message_file
-      unless @args[0] && ::File.exist?(@args[0])
-        fail 'Not running in the context of a commit message'
-      end
+  private
 
-      @args[0]
+    def raw_commit_message
+      ::IO.readlines(commit_message_file)
     end
   end
 end

spec/overcommit/hook/commit_msg/gerrit_change_id_spec.rb

@@ -0,0 +1,38 @@
+require 'spec_helper'
+
+describe Overcommit::Hook::CommitMsg::GerritChangeId do
+  let(:config)  { Overcommit::ConfigurationLoader.default_configuration }
+  let(:context) { double('context') }
+  let(:subject) { described_class.new(config, context) }
+  let(:commit_msg_file) { Tempfile.new('commit-msg') }
+
+  before do
+    commit_msg_file.write(commit_msg)
+    commit_msg_file.close
+    subject.stub(:commit_message_file).and_return(commit_msg_file.path)
+  end
+
+  context 'when the commit message contains no Change-Id' do
+    let(:commit_msg) { 'Add code to repo' }
+
+    it { should pass }
+
+    it 'adds a Change-Id to the commit message' do
+      subject.run
+      File.open(commit_msg_file.path, 'r').read.should =~ /Change-Id/
+    end
+  end
+
+  context 'when the commit message already contains a Change-Id' do
+    let(:commit_msg) { "Add code to repo\n\nChange-Id: I9f2b5528fa20ac91a55bbe9371e76a12dd1cce11" }
+
+    it { should pass }
+
+    it 'does nothing' do
+      before = File.open(commit_msg_file.path, 'r').read
+      subject.run
+      after = File.open(commit_msg_file.path, 'r').read
+      before.should == after
+    end
+  end
+end

spec/overcommit/hook/commit_msg/hard_tabs_spec.rb

@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe Overcommit::Hook::CommitMsg::HardTabs do
+  let(:config)  { Overcommit::ConfigurationLoader.default_configuration }
+  let(:context) { double('context') }
+  let(:subject) { described_class.new(config, context) }
+
+  before do
+    subject.stub(:commit_message).and_return(commit_msg)
+  end
+
+  context 'when message contains hard tabs' do
+    let(:commit_msg) { ["This is a hard-tab\tcommit message"] }
+
+    it { should warn }
+  end
+
+  context 'when message does not contain hard tabs' do
+    let(:commit_msg) { ['No hard tabs to be found'] }
+
+    it { should pass }
+  end
+end

spec/overcommit/hook/commit_msg/russian_novel_spec.rb

@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe Overcommit::Hook::CommitMsg::RussianNovel do
+  let(:config)  { Overcommit::ConfigurationLoader.default_configuration }
+  let(:context) { double('context') }
+  let(:subject) { described_class.new(config, context) }
+
+  before do
+    subject.stub(:commit_message).and_return(commit_msg)
+  end
+
+  context 'when message contains fewer than 30 lines' do
+    let(:commit_msg) { ['A single line'] * 10 }
+
+    it { should pass }
+  end
+
+  context 'when message contains at least 30 lines' do
+    let(:commit_msg) { ['A single line'] * 30 }
+
+    it { should warn }
+  end
+end

spec/overcommit/hook/commit_msg/single_line_subject_spec.rb

@@ -0,0 +1,32 @@
+require 'spec_helper'
+
+describe Overcommit::Hook::CommitMsg::SingleLineSubject do
+  let(:config)  { Overcommit::ConfigurationLoader.default_configuration }
+  let(:context) { double('context') }
+  let(:subject) { described_class.new(config, context) }
+
+  before do
+    subject.stub(:commit_message).and_return(commit_msg.split("\n"))
+  end
+
+  context 'when subject is separated from body by a blank line' do
+    let(:commit_msg) { <<-MSG }
+      Initial commit
+
+      Mostly cats so far.
+    MSG
+
+    it { should pass }
+  end
+
+  context 'when subject is not kept to one line' do
+    let(:commit_msg) { <<-MSG }
+      Initial commit where I forget about commit message
+      standards and decide to hard-wrap my subject
+
+      Still mostly cats so far.
+    MSG
+
+    it { should warn }
+  end
+end

spec/overcommit/hook/commit_msg/text_width_spec.rb

@@ -0,0 +1,45 @@
+require 'spec_helper'
+
+describe Overcommit::Hook::CommitMsg::TextWidth do
+  let(:config)  { Overcommit::ConfigurationLoader.default_configuration }
+  let(:context) { double('context') }
+  let(:subject) { described_class.new(config, context) }
+
+  before do
+    subject.stub(:commit_message).and_return(commit_msg.split("\n"))
+  end
+
+  context 'when subject is longer than 60 characters' do
+    let(:commit_msg) { 'A' * 61 }
+
+    it { should warn /subject/ }
+  end
+
+  context 'when subject is 60 characters or fewer' do
+    let(:commit_msg) { 'A' * 60 }
+
+    it { should pass }
+  end
+
+  context 'when a line in the message is longer than 72 characters' do
+    let(:commit_msg) { <<-MSG }
+      Some summary
+
+      This line is longer than 72 characters which is clearly be seen by count.
+    MSG
+
+    it { should warn /72 char/ }
+  end
+
+  context 'when all lines in the message are fewer than 72 characters' do
+    let(:commit_msg) { <<-MSG }
+      Some summary
+
+      A reasonable line.
+
+      Another reasonable line.
+    MSG
+
+    it { should pass }
+  end
+end

spec/overcommit/hook/commit_msg/trailing_period_spec.rb

@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe Overcommit::Hook::CommitMsg::TrailingPeriod do
+  let(:config)  { Overcommit::ConfigurationLoader.default_configuration }
+  let(:context) { double('context') }
+  let(:subject) { described_class.new(config, context) }
+
+  before do
+    subject.stub(:commit_message).and_return(commit_msg.split("\n"))
+  end
+
+  context 'when subject contains a trailing period' do
+    let(:commit_msg) { 'This subject has a period.' }
+
+    it { should warn }
+  end
+
+  context 'when subject does not contain a trailing period' do
+    let(:commit_msg) { 'This subject has no period' }
+
+    it { should pass }
+  end
+end

spec/spec_helper.rb

@@ -1,4 +1,5 @@
 require 'overcommit'
+require 'tempfile'
 
 hook_types = Dir[File.join(Overcommit::OVERCOMMIT_HOME, 'lib/overcommit/hook/*')].
   select { |f| File.directory?(f) }.