Return HTTP 500 if git fails - not that Github cares about this at th…

…is point, but it seems more correct

app/controllers/github_hook_controller.rb

@@ -21,12 +21,13 @@ def index
 
     # Get updates from the Github repository
     command = "cd '#{repository.url}' && git fetch origin && git reset --soft refs/remotes/origin/master"
-    exec(command)
-
-    # Fetch the new changesets into Redmine
-    repository.fetch_changesets
-
-    render(:text => 'OK')
+    if exec(command)
+      # Fetch the new changesets into Redmine
+      repository.fetch_changesets
+      render(:text => 'OK')
+    else
+      render(:text => 'Failed', :status => 500)
+    end
   end
 
   private
@@ -38,12 +39,15 @@ def exec(command)
     output = stdout.readlines.collect(&:strip)
     errors = stderr.readlines.collect(&:strip)
 
-    unless errors.empty?
+    if errors.empty?
+      return true
+    else
       error_message = []
       error_message << "Error occurred running git"
       error_message += errors
       error_message += output
       logger.error error_message
+      return false
     end
   end
 

test/functional/github_hook_controller_test.rb

@@ -147,4 +147,17 @@ def test_exec_should_not_log_errors_if_none_occurred
     @controller.logger.expects(:error).never
     do_post
   end
+
+  def test_should_return_500_if_git_has_errors
+    @controller.expects(:exec).returns(false)
+    do_post
+    assert_response 500
+    assert_equal 'Failed', @response.body
+  end
+
+  def test_should_not_import_changeset_if_git_has_errors
+    @controller.expects(:exec).returns(false)
+    @repository.expects(:fetch_changesets).never
+    do_post
+  end
 end