Add specs for PreCommit and CommitMsg contexts

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

Author: sds

TODO

@@ -23,6 +23,7 @@ Display when no applicable hooks ran (e.g. instead of "All pre-commit checks pas
 Add pre-commit check ensuring Gemfile.lock matches Gemfile (unless in .gitignore)
 Add check for https://github.com/mdevils/node-jscs
 Add post-checkout check for asking to update git submodules
+FIX let(:subject)
 
 
 Allow features of hooks to be customized (stealth, required, etc.)

lib/overcommit/hook/base.rb

@@ -5,7 +5,7 @@ module Overcommit::Hook
   class Base
     extend Forwardable
 
-    def_delegators :@context, :staged_files
+    def_delegators :@context, :modified_files
 
     def initialize(config, context)
       @config = config.hook_config(self)
@@ -62,7 +62,7 @@ def command(cmd)
     # Gets a list of staged files that apply to this hook based on its
     # configured `include` and `exclude` lists.
     def applicable_files
-      @applicable_files ||= staged_files.select { |file| applicable_file?(file) }
+      @applicable_files ||= modified_files.select { |file| applicable_file?(file) }
     end
 
   private

lib/overcommit/hook/commit_msg/base.rb

@@ -5,6 +5,6 @@ module Overcommit::Hook::CommitMsg
   class Base < Overcommit::Hook::Base
     extend Forwardable
 
-    def_delegators :@context, :commit_message
+    def_delegators :@context, :commit_message, :commit_message_lines
   end
 end

lib/overcommit/hook/commit_msg/hard_tabs.rb

@@ -3,7 +3,7 @@ module Overcommit::Hook::CommitMsg
   class HardTabs < Base
     def run
       # Catches hard tabs entered by the user (not auto-generated)
-      if commit_message.join.index(/\t/)
+      if commit_message.index(/\t/)
         return :warn, "Don't use hard tabs in commit messages"
       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_lines.length >= RUSSIAN_NOVEL_LENGTH
         return :warn, 'You seem to have authored a Russian novel; congratulations!'
       end
 

lib/overcommit/hook/commit_msg/single_line_subject.rb

@@ -2,7 +2,7 @@ module Overcommit::Hook::CommitMsg
   # Ensures commit message subject lines are followed by a blank line.
   class SingleLineSubject < Base
     def run
-      unless commit_message[1].to_s.strip.empty?
+      unless commit_message_lines[1].to_s.strip.empty?
         return :warn, 'Subject should be one line and followed by a blank line'
       end
 

lib/overcommit/hook/commit_msg/text_width.rb

@@ -3,11 +3,11 @@ module Overcommit::Hook::CommitMsg
   # under the preferred limits.
   class TextWidth < Base
     def run
-      if commit_message.first.size > 60
+      if commit_message_lines.first.size > 60
         return :warn, 'Please keep the subject < ~60 characters'
       end
 
-      commit_message.each do |line|
+      commit_message_lines.each do |line|
         chomped = line.chomp
         if chomped.size > 72
           return :warn, "> 72 characters, please hard wrap: '#{chomped}'"

lib/overcommit/hook/commit_msg/trailing_period.rb

@@ -2,7 +2,7 @@ module Overcommit::Hook::CommitMsg
   # Ensures commit message subject lines do not have a trailing period
   class TrailingPeriod < Base
     def run
-      if commit_message[0].rstrip.end_with?('.')
+      if commit_message_lines.first.rstrip.end_with?('.')
         return :warn, 'Please omit trailing period from commit message subject'
       end
 

lib/overcommit/hook_context/base.rb

@@ -19,13 +19,20 @@ def hook_class_name
       @hook_class_name ||= self.class.name.split('::').last
     end
 
-    # Get a list of added, copied, or modified files that have been staged.
-    # Renames and deletions are ignored, since there should be nothing to check.
-    def staged_files
-      @staged_files ||=
-        `git diff --cached --name-only --diff-filter=ACM --ignore-submodules=all`.
-          split("\n").
-          map { |relative_file| File.expand_path(relative_file) }
+    # Returns a list of files that have been modified.
+    #
+    # By default, this returns an empty list. Subclasses should implement if
+    # there is a concept of files changing for the type of hook being run.
+    def modified_files
+      []
+    end
+
+    # Returns a set of lines that have been modified for a file.
+    #
+    # By default, this returns an empty set. Subclasses should implement if
+    # there is a concept of files changing for the type of hook being run.
+    def modified_lines(file)
+      Set.new
     end
   end
 end

lib/overcommit/hook_context/commit_msg.rb

@@ -1,8 +1,13 @@
 module Overcommit::HookContext
+  # Contains helpers related to contextual information used by commit-msg hooks.
   class CommitMsg < Base
     # User commit message stripped of comments and diff (from verbose output).
     def commit_message
-      raw_commit_message.
+      commit_message_lines.join
+    end
+
+    def commit_message_lines
+      raw_commit_message_lines.
         reject     { |line| line =~ /^#/ }.
         take_while { |line| !line.start_with?('diff --git') }
     end
@@ -13,7 +18,7 @@ def commit_message_file
 
   private
 
-    def raw_commit_message
+    def raw_commit_message_lines
       ::IO.readlines(commit_message_file)
     end
   end

lib/overcommit/hook_context/pre_commit.rb

@@ -4,11 +4,20 @@ module Overcommit::HookContext
   # This includes staged files, which lines of those files have been modified,
   # etc.
   class PreCommit < Base
+    # Get a list of added, copied, or modified files that have been staged.
+    # Renames and deletions are ignored, since there should be nothing to check.
+    def modified_files
+      @modified_files ||=
+        `git diff --cached --name-only --diff-filter=ACM --ignore-submodules=all`.
+          split("\n").
+          map { |relative_file| File.expand_path(relative_file) }
+    end
+
     # Returns the set of line numbers corresponding to the lines that were
     # changed in a specified file.
-    def modified_lines(staged_file)
+    def modified_lines(file)
       @modified_lines ||= {}
-      @modified_lines[staged_file] ||= extract_modified_lines(staged_file)
+      @modified_lines[file] ||= extract_modified_lines(file)
     end
 
   private
@@ -27,7 +36,7 @@ def extract_modified_lines(staged_file)
         scan(DIFF_HUNK_REGEX) do |start_line, lines_added|
 
         lines_added = (lines_added || 1).to_i # When blank, one line was added
-        cur_line    = start_line.to_i
+        cur_line = start_line.to_i
 
         lines_added.times do
           lines.add cur_line

spec/overcommit/hook/commit_msg/hard_tabs_spec.rb

@@ -10,13 +10,13 @@
   end
 
   context 'when message contains hard tabs' do
-    let(:commit_msg) { ["This is a hard-tab\tcommit message"] }
+    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'] }
+    let(:commit_msg) { 'No hard tabs to be found' }
 
     it { should pass }
   end

spec/overcommit/hook/commit_msg/russian_novel_spec.rb

@@ -6,7 +6,7 @@
   let(:subject) { described_class.new(config, context) }
 
   before do
-    subject.stub(:commit_message).and_return(commit_msg)
+    subject.stub(:commit_message_lines).and_return(commit_msg)
   end
 
   context 'when message contains fewer than 30 lines' do

spec/overcommit/hook/commit_msg/single_line_subject_spec.rb

@@ -6,7 +6,7 @@
   let(:subject) { described_class.new(config, context) }
 
   before do
-    subject.stub(:commit_message).and_return(commit_msg.split("\n"))
+    subject.stub(:commit_message_lines).and_return(commit_msg.split("\n"))
   end
 
   context 'when subject is separated from body by a blank line' do

spec/overcommit/hook/commit_msg/text_width_spec.rb

@@ -6,7 +6,7 @@
   let(:subject) { described_class.new(config, context) }
 
   before do
-    subject.stub(:commit_message).and_return(commit_msg.split("\n"))
+    subject.stub(:commit_message_lines).and_return(commit_msg.split("\n"))
   end
 
   context 'when subject is longer than 60 characters' do

spec/overcommit/hook/commit_msg/trailing_period_spec.rb

@@ -6,7 +6,7 @@
   let(:subject) { described_class.new(config, context) }
 
   before do
-    subject.stub(:commit_message).and_return(commit_msg.split("\n"))
+    subject.stub(:commit_message_lines).and_return(commit_msg.split("\n"))
   end
 
   context 'when subject contains a trailing period' do

spec/overcommit/hook/pre_commit/author_name_spec.rb

@@ -4,7 +4,7 @@
   let(:config)  { Overcommit::ConfigurationLoader.default_configuration }
   let(:context) { double('context') }
   let(:subject) { described_class.new(config, context) }
-  let(:result)  { mock('result') }
+  let(:result)  { double('result') }
 
   before do
     result = mock('result)')

spec/overcommit/hook_context/base_spec.rb

@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe Overcommit::HookContext::Base do
+  let(:config) { double('context') }
+  let(:args) { [] }
+  let(:input) { '' }
+  let(:context) { described_class.new(config, args, input) }
+
+  describe '#hook_class_name' do
+    subject { context.hook_class_name }
+
+    it 'returns the short class name of the context' do
+      subject.should == 'Base'
+    end
+  end
+end

spec/overcommit/hook_context/commit_msg_spec.rb

@@ -0,0 +1,43 @@
+require 'spec_helper'
+require 'overcommit/hook_context/commit_msg'
+
+describe Overcommit::HookContext::CommitMsg do
+  let(:config) { double('context') }
+  let(:args) { [commit_message_file] }
+  let(:input) { '' }
+  let(:context) { described_class.new(config, args, input) }
+  let(:commit_msg) do
+    [
+    '# Please enter the commit message for your changes.',
+    'Some commit message',
+    '# On branch master',
+    'diff --git a/file b/file',
+    'index 4ae1030..342a117 100644',
+    '--- a/file',
+    '+++ b/file',
+    ]
+  end
+
+  let(:commit_message_file) do
+    Tempfile.new('commit-message').tap do |file|
+      file.write(commit_msg.join("\n"))
+      file.fsync
+    end.path
+  end
+
+  describe '#commit_message' do
+    subject { context.commit_message }
+
+    it 'strips comments and trailing diff' do
+      subject.should == "Some commit message\n"
+    end
+  end
+
+  describe '#commit_message_lines' do
+    subject { context.commit_message_lines }
+
+    it 'strips comments and trailing diff' do
+      subject.should == ["Some commit message\n"]
+    end
+  end
+end

spec/overcommit/hook_context/pre_commit_spec.rb

@@ -0,0 +1,77 @@
+require 'spec_helper'
+require 'overcommit/hook_context/pre_commit'
+
+describe Overcommit::HookContext::PreCommit do
+  let(:config) { double('context') }
+  let(:args) { [] }
+  let(:input) { '' }
+  let(:context) { described_class.new(config, args, input) }
+
+  describe '#modified_files' do
+    subject { context.modified_files }
+
+    it 'does not include submodules' do
+      submodule = repo do
+        `touch foo`
+        `git add foo`
+        `git commit -m "Initial commit"`
+      end
+
+      repo do
+        `git submodule add #{submodule} test-sub`
+        expect(subject).to_not include File.expand_path('test-sub')
+      end
+    end
+
+    context 'when no files were staged' do
+      around do |example|
+        repo do
+          example.run
+        end
+      end
+
+      it { should be_empty }
+    end
+
+    context 'when files were added' do
+      around do |example|
+        repo do
+          FileUtils.touch('some-file')
+          `git add some-file`
+          example.run
+        end
+      end
+
+      it { should == [File.expand_path('some-file')] }
+    end
+
+    context 'when files were modified' do
+      around do |example|
+        repo do
+          FileUtils.touch('some-file')
+          `git add some-file`
+          `git commit -m 'Initial commit'`
+          `echo Hello > some-file`
+          `git add some-file`
+          example.run
+        end
+      end
+
+      it { should == [File.expand_path('some-file')] }
+    end
+
+    context 'when files were deleted' do
+      around do |example|
+        repo do
+          FileUtils.touch('some-file')
+          `git add some-file`
+          `git commit -m 'Initial commit'`
+          `git rm some-file`
+          example.run
+        end
+      end
+
+      it { should be_empty }
+    end
+  end
+end