Limit directory-changing to a single method

Author: koppen

app/controllers/github_hook_controller.rb

@@ -31,15 +31,27 @@ def system(command)
     Kernel.system(command)
   end
 
-  # Executes shell command. Returns true if the shell command exits with a success status code
-  def exec(command)
+  # Executes shell command. Returns true if the shell command exits with a
+  # success status code.
+  #
+  # If directory is given the current directory will be changed to that
+  # directory before executing command.
+  def exec(command, directory)
     logger.debug { "GithubHook: Executing command: '#{command}'" }
 
     # Get a path to a temp file
     logfile = Tempfile.new('github_hook_exec')
     logfile.close
 
-    success = system("#{command} > #{logfile.path} 2>&1")
+    full_command = "#{command} > #{logfile.path} 2>&1"
+    success = if directory.present?
+      Dir.chdir(directory) do
+        system(full_command)
+      end
+    else
+      system(full_command)
+    end
+
     output_from_command = File.readlines(logfile.path)
     if success
       logger.debug { "GithubHook: Command output: #{output_from_command.inspect}"}
@@ -58,12 +70,10 @@ def git_command(command)
 
   # Fetches updates from the remote repository
   def update_repository(repository)
-    Dir.chdir(repository.url) do
-      command = git_command('fetch origin')
-      if exec(command)
-        command = git_command("fetch origin \"+refs/heads/*:refs/heads/*\"")
-        exec(command)
-      end
+    command = git_command('fetch origin')
+    if exec(command, repository.url)
+      command = git_command("fetch origin \"+refs/heads/*:refs/heads/*\"")
+      exec(command, repository.url)
     end
   end
 

test/functional/github_hook_controller_test.rb

@@ -92,21 +92,21 @@ def test_should_use_the_repository_name_as_project_identifier
 
   def test_should_fetch_changes_from_origin
     Project.expects(:find_by_identifier).with('github').returns(project)
-    @controller.expects(:exec).with("git --git-dir=\"#{repository.url}\" fetch origin")
+    @controller.expects(:exec).with("git fetch origin", repository.url)
     do_post
   end
 
   def test_should_reset_repository_when_fetch_origin_succeeds
     Project.expects(:find_by_identifier).with('github').returns(project)
-    @controller.expects(:exec).with("git --git-dir=\"#{repository.url}\" fetch origin").returns(true)
-    @controller.expects(:exec).with("git --git-dir=\"#{repository.url}\" fetch origin \"+refs/heads/*:refs/heads/*\"")
+    @controller.expects(:exec).with("git fetch origin", repository.url).returns(true)
+    @controller.expects(:exec).with("git fetch origin \"+refs/heads/*:refs/heads/*\"", repository.url)
     do_post
   end
 
   def test_should_not_reset_repository_when_fetch_origin_fails
     Project.expects(:find_by_identifier).with('github').returns(project)
-    @controller.expects(:exec).with("git --git-dir=\"#{repository.url}\" fetch origin").returns(false)
-    @controller.expects(:exec).with("git --git-dir=\"#{repository.url}\" reset --soft refs\/remotes\/origin\/master").never
+    @controller.expects(:exec).with("git fetch origin", repository.url).returns(false)
+    @controller.expects(:exec).with("git reset --soft refs\/remotes\/origin\/master", repository.url).never
     do_post
   end