From 0ad79760c8262aa841dc5ba6a2fe511b9c87afaa Mon Sep 17 00:00:00 2001
From: Taufek Johar <taufek@gmail.com>
Date: Sun, 14 Jan 2018 01:08:21 +0800
Subject: [PATCH] Update Pre-Push Context Modified Files

Previously, `Overcommit::HookContext::PrePush::modified_files` is set as `[]` which
does not give us any value in `applicable_files` as in `PreCommit` context.

This change updates the method to return list of added and updated files:

1. compared between current branch and tracking branch if tracking branch is available.

2. compared between current branch with parent branch if tracking branch is not available.

This gives us above values in `applicable_files` in `PrePush` context.

One usage example is we can create a hook to scan prohibited keywords such as `eval`, `console.log`, etc. before pushing to remote. Previously we can only do it in `PreCommit` hook but now we could also do it in `PrePush`.
---
 lib/overcommit/hook_context/pre_push.rb       |  6 ++
 spec/overcommit/hook_context/pre_push_spec.rb | 84 +++++++++++++++++++
 spec/support/git_spec_helpers.rb              |  8 ++
 3 files changed, 98 insertions(+)

diff --git a/lib/overcommit/hook_context/pre_push.rb b/lib/overcommit/hook_context/pre_push.rb
index b47e2987..560232c2 100644
--- a/lib/overcommit/hook_context/pre_push.rb
+++ b/lib/overcommit/hook_context/pre_push.rb
@@ -17,6 +17,12 @@ def pushed_refs
       end
     end
 
+    def modified_files
+      @modified_files ||= Overcommit::GitRepo.modified_files(
+        refs: "#{pushed_refs[0].remote_sha1}..#{pushed_refs[0].local_sha1}"
+      )
+    end
+
     PushedRef = Struct.new(:local_ref, :local_sha1, :remote_ref, :remote_sha1) do
       def forced?
         !(created? || deleted? || overwritten_commits.empty?)
diff --git a/spec/overcommit/hook_context/pre_push_spec.rb b/spec/overcommit/hook_context/pre_push_spec.rb
index 0db8a6b8..f9a54e7f 100644
--- a/spec/overcommit/hook_context/pre_push_spec.rb
+++ b/spec/overcommit/hook_context/pre_push_spec.rb
@@ -44,6 +44,90 @@
     end
   end
 
+  describe '#modified_files' do
+    subject { context.modified_files }
+
+    let(:remote_repo) do
+      repo do
+        touch 'update-me'
+        echo 'update', 'update-me'
+        touch 'delete-me'
+        echo 'delete', 'delete-me'
+        `git add . 2>&1 > #{File::NULL}`
+        `git commit -m "Initial commit" 2>&1 > #{File::NULL}`
+      end
+    end
+
+    context 'when current branch has tracking branch' do
+      let(:local_ref) { 'refs/heads/project-branch' }
+      let(:local_sha1) { get_sha1(local_ref) }
+      let(:remote_ref) { 'refs/remotes/origin/master' }
+      let(:remote_sha1) { get_sha1(remote_ref) }
+      let(:input) do
+        double('input', read: "#{local_ref} #{local_sha1} #{remote_ref} #{remote_sha1}\n")
+      end
+
+      it 'has modified files based on tracking branch' do
+        repo do
+          `git remote add origin file://#{remote_repo}`
+          `git fetch origin 2>&1 > #{File::NULL} && git reset --hard origin/master`
+
+          `git checkout -b project-branch 2>&1 > #{File::NULL}`
+          `git push -u origin project-branch 2>&1 > #{File::NULL}`
+
+          touch 'added-1'
+          echo 'add', 'added-1'
+          echo 'append', 'update-me'
+          FileUtils.rm 'delete-me'
+          `git add . 2>&1 > #{File::NULL}`
+          `git commit -m "Update Branch 1" 2>&1 > #{File::NULL}`
+
+          touch 'added-2'
+          echo 'add', 'added-2'
+          `git add . 2>&1 > #{File::NULL}`
+          `git commit -m "Update Branch 2" 2>&1 > #{File::NULL}`
+
+          should include(*%w[added-1 update-me added-2].map { |file| File.expand_path(file) })
+          should_not include(*%w[delete-me].map { |file| File.expand_path(file) })
+        end
+      end
+    end
+
+    context 'when current branch has no tracking branch' do
+      let(:local_ref) { 'refs/heads/project-branch' }
+      let(:local_sha1) { get_sha1(local_ref) }
+      let(:remote_ref) { 'refs/heads/master' }
+      let(:remote_sha1) { get_sha1(remote_ref) }
+      let(:input) do
+        double('input', read: "#{local_ref} #{local_sha1} #{remote_ref} #{remote_sha1}\n")
+      end
+
+      it 'has modified files based on parent branch' do
+        repo do
+          `git remote add origin file://#{remote_repo}`
+          `git fetch origin 2>&1 > #{File::NULL} && git reset --hard origin/master`
+
+          `git checkout -b project-branch 2>&1 > #{File::NULL}`
+
+          touch 'added-1'
+          echo 'add', 'added-1'
+          echo 'append', 'update-me'
+          FileUtils.rm 'delete-me'
+          `git add . 2>&1 > #{File::NULL}`
+          `git commit -m "Update Branch 1" 2>&1 > #{File::NULL}`
+
+          touch 'added-2'
+          echo 'add', 'added-2'
+          `git add . 2>&1 > #{File::NULL}`
+          `git commit -m "Update Branch 2" 2>&1 > #{File::NULL}`
+
+          should include(*%w[added-1 update-me added-2].map { |file| File.expand_path(file) })
+          should_not include(*%w[delete-me].map { |file| File.expand_path(file) })
+        end
+      end
+    end
+  end
+
   describe Overcommit::HookContext::PrePush::PushedRef do
     let(:local_ref) { 'refs/heads/master' }
     let(:remote_ref) { 'refs/heads/master' }
diff --git a/spec/support/git_spec_helpers.rb b/spec/support/git_spec_helpers.rb
index 44838bc6..93cc99b1 100644
--- a/spec/support/git_spec_helpers.rb
+++ b/spec/support/git_spec_helpers.rb
@@ -23,6 +23,14 @@ def repo(options = {})
     end
   end
 
+  # Retrieve sha1 based on git ref
+  #
+  # @param ref [String] git ref
+  # @return [String] ref's sha1
+  def get_sha1(ref)
+    `git rev-parse #{ref}`.chomp
+  end
+
   # Creates a directory (with an optional specific name) in a temporary
   # directory which will automatically be destroyed.
   #