MR on fork: Some cleanup, test updates

-The forked merge request test now tests it's componenets again, and seems to work every time (did this by reordering the branch updates so their is more time for update_branches to run) -- this could still technically fail, but after several runs it doesn't seem to.
-Removed todo in merge_request, pushed wrapping of grit down to the satellite
-updated action test to check flock status, made it nolonger pending
-moved all logging on failure to helper method in satellite
GITLAB-592

Change-Id: If0554ca35eedc3d3e8461f7d93d4b3939fa2cd75

2 files changed, 17 insertions, 20 deletions

diff --git a/lib/gitlab/satellite/action.rb b/lib/gitlab/satellite/action.rb
index f22d595d5cd..bb93797c1f4 100644
--- a/lib/gitlab/satellite/action.rb
+++ b/lib/gitlab/satellite/action.rb
@@ -1,7 +1,7 @@
 module Gitlab
   module Satellite
     class Action
-      DEFAULT_OPTIONS = { git_timeout: 30.seconds}
+      DEFAULT_OPTIONS = {git_timeout: 30.seconds}
 
       attr_accessor :options, :project, :user
 
@@ -25,11 +25,9 @@ module Gitlab
           end
         end
       rescue Errno::ENOMEM => ex
-        Gitlab::GitLogger.error(ex.message)
-        return false
+        return handle_exception(ex)
       rescue Grit::Git::GitTimeout => ex
-        Gitlab::GitLogger.error(ex.message)
-        return false
+        return handle_exception(ex)
       ensure
         Gitlab::ShellEnv.reset_env
       end
@@ -48,6 +46,11 @@ module Gitlab
       def default_options(options = {})
         {raise: true, timeout: true}.merge(options)
       end
+
+      def handle_exception(exception)
+        Gitlab::GitLogger.error(exception.message)
+        false
+      end
     end
   end
 end
diff --git a/lib/gitlab/satellite/merge_action.rb b/lib/gitlab/satellite/merge_action.rb
index 22c10515d2a..604b2ca9a19 100644
--- a/lib/gitlab/satellite/merge_action.rb
+++ b/lib/gitlab/satellite/merge_action.rb
@@ -41,8 +41,7 @@ module Gitlab
           end
         end
       rescue Grit::Git::CommandFailed => ex
-        Gitlab::GitLogger.error(ex.message)
-        false
+        handle_exception(ex)
       end
 
 
@@ -61,8 +60,7 @@ module Gitlab
           return diff
         end
       rescue Grit::Git::CommandFailed => ex
-        Gitlab::GitLogger.error(ex.message)
-        false
+        handle_exception(ex)
       end
 
       # Only show what is new in the source branch compared to the target branch, not the other way around.
@@ -81,11 +79,11 @@ module Gitlab
             #this method doesn't take default options
             diffs = merge_repo.diff(common_commit, "#{merge_request.source_branch}")
           end
+          diffs = diffs.map { |diff| Gitlab::Git::Diff.new(diff) }
           return diffs
         end
       rescue Grit::Git::CommandFailed => ex
-        Gitlab::GitLogger.error(ex.message)
-        false
+        handle_exception(ex)
       end
 
       # Get commit as an email patch
@@ -101,8 +99,7 @@ module Gitlab
           return patch
         end
       rescue Grit::Git::CommandFailed => ex
-        Gitlab::GitLogger.error(ex.message)
-        false
+        handle_exception(ex)
       end
 
       # Retrieve an array of commits between the source and the target
@@ -115,12 +112,12 @@ module Gitlab
           else
             commits = merge_repo.commits_between("#{merge_request.target_branch}", "#{merge_request.source_branch}")
           end
+          commits = commits.map { |commit| Gitlab::Git::Commit.new(commit, nil) }
           return commits
 
         end
       rescue Grit::Git::CommandFailed => ex
-        Gitlab::GitLogger.error(ex.message)
-        false
+        handle_exception(ex)
       end
 
       private
@@ -142,8 +139,7 @@ module Gitlab
           repo.git.pull(default_options({no_ff: true}), 'origin', merge_request.source_branch)
         end
       rescue Grit::Git::CommandFailed => ex
-        Gitlab::GitLogger.error(ex.message)
-        false
+        handle_exception(ex)
       end
 
       # Assumes a satellite exists that is a fresh clone of the projects repo, prepares satellite for merges, diffs etc
@@ -159,11 +155,9 @@ module Gitlab
           repo.git.checkout(default_options, "#{merge_request.target_branch}")
         end
       rescue Grit::Git::CommandFailed => ex
-        Gitlab::GitLogger.error(ex.message)
-        false
+        handle_exception(ex)
       end
 
-
     end
   end
 end