Make git merge command detect no-op merges

lib/auto_merger.rb

@@ -58,18 +58,25 @@ def merge_and_push_branch(target_branch, source_branch)
     @git.checkout_branch(target_branch.name)
 
     Rails.logger.debug("Attempt to merge #{source_branch.name} into #{target_branch.name}")
-    conflict = @git.detect_conflicts(target_branch.name, source_branch.name, keep_changes: true)
-    unless conflict.present?
+    success, conflict = @git.merge_branches(
+        target_branch.name,
+        source_branch.name,
+        keep_changes: true,
+        commit_message: "Auto merge branch #{source_branch.name} into #{target_branch.name}")
+    if success
       Rails.logger.info("MERGED: #{source_branch.name} has been merged into #{target_branch.name} without conflicts")
       if @git.push
         Rails.logger.info("PUSHED: #{target_branch.name} to origin")
         Merge.create!(source_branch: source_branch, target_branch: target_branch, successful: true)
       else
-        Rails.logger.info("NO-OP: #{target_branch.name} is already up to date with origin")
+        raise "Failed to push #{target_branch.name} to origin, it was already up to date!"
       end
-    else
+    elsif conflict.present?
       Rails.logger.info("CONFLICT: #{target_branch.name} conflicts with #{source_branch.name}\nConflicting files:\n#{conflict.conflicting_files}")
+      @git.reset
       Merge.create!(source_branch: source_branch, target_branch: target_branch, successful: false)
+    else
+      Rails.logger.info("NO-OP: #{target_branch.name} is already up to date with origin")
     end
   end
 

lib/conflict_detector.rb

@@ -55,16 +55,18 @@ def get_conflicts(target_branch, source_branches)
       next if target_branch.name == source_branch.name
 
       Rails.logger.debug("Attempt to merge #{source_branch.name}")
-      conflict = @git.detect_conflicts(target_branch.name, source_branch.name)
-      unless conflict.present?
+      success, conflict = @git.merge_branches(target_branch.name, source_branch.name, keep_changes: false)
+      if success
         Rails.logger.info("MERGED: #{source_branch.name} can be merged into #{target_branch.name} without conflicts")
-      else
+      elsif conflict.present?
         if should_ignore_conflicts?(conflict.conflicting_files)
           Rails.logger.info("MERGED: #{target_branch.name} conflicts with #{source_branch.name}, but all conflicting files are on the ignore list.")
         else
           Rails.logger.info("CONFLICT: #{target_branch.name} conflicts with #{source_branch.name}\nConflicting files:\n#{conflict.conflicting_files}")
           conflicts << conflict
         end
+      else
+        Rails.logger.info("MERGED: #{source_branch.name} was already up to date with #{target_branch.name}")
       end
     end
 

lib/git/git.rb

@@ -44,34 +44,37 @@ def get_branch_list
       end
     end
 
-    def detect_conflicts(target_branch_name, source_branch_name, keep_changes: false)
-      # attempt the merge and gather conflicts, if found
-      begin
-        # TODO: Assert we are actually on the target branch and have a clean working dir
-        execute("pull --no-commit origin #{source_branch_name}")
-        nil
-      rescue GitError => ex
-        keep_changes = false
-        conflicting_files = Git::get_conflict_list_from_failed_merge_output(ex.error_message)
-        unless conflicting_files.empty?
-          GitConflict.new(
-              @repository_name,
-              target_branch_name,
-              source_branch_name,
-              conflicting_files)
-        else
-          nil
-        end
-      ensure
-        # cleanup our "mess"
-        keep_changes or execute("reset --hard origin/#{target_branch_name}")
+    def merge_branches(target_branch_name, source_branch_name, keep_changes: true, commit_message: nil)
+      commit_message_argument = "-m \"#{commit_message}\"" if commit_message
+
+      raw_output = execute("merge --no-ff --no-edit #{commit_message_argument} origin/#{source_branch_name}")
+
+      if raw_output =~ /.*Already up-to-date.\n/
+        [false, nil]
+      else
+        [true, nil]
       end
+    rescue GitError => ex
+      conflicting_files = Git::get_conflict_list_from_failed_merge_output(ex.error_message)
+      unless conflicting_files.empty?
+        [false,
+         GitConflict.new(
+            @repository_name,
+            target_branch_name,
+            source_branch_name,
+            conflicting_files)]
+      else
+        raise
+      end
+    ensure
+      # cleanup our "mess"
+      keep_changes or reset
     end
 
     def clone_repository(default_branch)
       if Dir.exists?("#{@repository_path}")
         # cleanup any changes that might have been left over if we crashed while running
-        execute('reset --hard origin')
+        reset
         execute('clean -f -d')
 
         # move to the master branch
@@ -98,9 +101,13 @@ def push
     end
 
     def checkout_branch(branch_name)
-      execute("reset --hard origin/#{branch_name}")
+      reset
       execute("checkout #{branch_name}")
-      execute("reset --hard origin/#{branch_name}")
+      reset
+    end
+
+    def reset
+      execute("reset --hard origin")
     end
 
     private

spec/lib/auto_merger_spec.rb

@@ -24,16 +24,23 @@ def expect_merges(target_branch_count: 0, conflict_count: 0, branches_up_to_date
     raise 'Conflict count must be <= target branch count' if conflict_count > target_branch_count
     expect_any_instance_of(Git::Git).to receive(:clone_repository)
     expect_any_instance_of(Git::Git).to receive(:get_branch_list) { create_test_git_branches(target_branch_count) }
-    expect_any_instance_of(Git::Git).to receive(:push).exactly(target_branch_count - conflict_count).times.and_return(!branches_up_to_date)
+    if branches_up_to_date
+      expect_any_instance_of(Git::Git).not_to receive(:push)
+    else
+      expect_any_instance_of(Git::Git).to receive(:push).exactly(target_branch_count - conflict_count).times.and_return(true)
+      if conflict_count > 0
+        expect_any_instance_of(Git::Git).to receive(:reset).exactly(conflict_count).times
+      end
+    end
     expect_any_instance_of(Git::Git).to receive(:checkout_branch).exactly(target_branch_count).times
     conflict_results = []
     (0..(conflict_count - 1)).each do |i|
-      conflict_results << create_test_git_conflict(branch_a_name: 'source', branch_b_name: 'target/branch0')
+      conflict_results << [false, create_test_git_conflict(branch_a_name: 'source', branch_b_name: 'target/branch0')]
     end
     (0..(target_branch_count - 1 - conflict_count)).each do |i|
-      conflict_results << nil
+      conflict_results << [!branches_up_to_date, nil]
     end
-    expect_any_instance_of(Git::Git).to receive(:detect_conflicts).exactly(target_branch_count).times.and_return(*conflict_results)
+    expect_any_instance_of(Git::Git).to receive(:merge_branches).exactly(target_branch_count).times.and_return(*conflict_results)
     auto_merger = AutoMerger.new(@settings)
     # a single notification email should be sent
     expect(MergeMailer).to receive(:maybe_send_merge_email_to_user).and_call_original

spec/lib/conflict_detector_spec.rb

@@ -46,29 +46,20 @@ def create_test_git_branches
     def expect_get_conflicts_equals(
         unfiltered_conflict_list,
         expected_conflict_list,
-        expected_push_count: 0,
         target_branch_name: 'branch_a',
-        source_branch_names: ['branch_b', 'branch_c'])
+        source_branch_names: ['branch_b', 'branch_c'],
+        alreadyUpToDate: false)
       target_branch = create_test_branch(name: target_branch_name)
       source_branches = source_branch_names.collect { |branch_name| create_test_branch(name: branch_name) }
       conflict_detector = ConflictDetector.new(@settings)
       expect_any_instance_of(Git::Git).to receive(:checkout_branch)
       if unfiltered_conflict_list.size > 0
-        allow_any_instance_of(Git::Git).to receive(:detect_conflicts).and_return(*unfiltered_conflict_list)
+        return_values = unfiltered_conflict_list.collect { |conflict| [false, conflict] }
+        allow_any_instance_of(Git::Git).to receive(:merge_branches).and_return(*return_values)
       else
-        allow_any_instance_of(Git::Git).to receive(:detect_conflicts).and_return(nil)
-      end
-      if expected_push_count > 0
-        expect_any_instance_of(Git::Git).to receive(:push).exactly(expected_push_count).times.and_return(true)
-      else
-        expect_any_instance_of(Git::Git).not_to receive(:push)
+        allow_any_instance_of(Git::Git).to receive(:merge_branches).and_return([!alreadyUpToDate, nil])
       end
       expect(conflict_detector.send(:get_conflicts, target_branch, source_branches)).to match_array(expected_conflict_list)
-      if expected_push_count > 0
-        expect(Merge.count).to eq(expected_push_count)
-      else
-        expect(Merge.count).to eq(0)
-      end
     end
 
     it 'should include all conflicts when the "ignore files" list is empty' do