Merge pull request #16 from docwhat/exec

Improve gitlab_shell spec and replace `system()` with `exec()`

lib/gitlab_shell.rb

@@ -44,13 +44,17 @@ class GitlabShell
 
   def process_cmd
     repo_full_path = File.join(repos_path, repo_name)
-    system("#{@git_cmd} #{repo_full_path}")
+    exec_cmd "#{@git_cmd} #{repo_full_path}"
   end
 
   def validate_access
     api.allowed?(@git_cmd, @repo_name, @key_id, '_any')
   end
 
+  def exec_cmd args
+    Kernel::exec args
+  end
+
   def api
     GitlabNet.new
   end

spec/gitlab_shell_spec.rb

@@ -2,58 +2,113 @@ require_relative 'spec_helper'
 require_relative '../lib/gitlab_shell'
 
 describe GitlabShell do
-  describe :initialize do
-    before do
-      ssh_cmd 'git-receive-pack'
-      ARGV[0] = 'key-56'
-      @shell = GitlabShell.new
+  subject do
+    ARGV[0] = key_id
+    GitlabShell.new.tap do |shell|
+      shell.stub(exec_cmd: :exec_called)
+      shell.stub(api: api)
+    end
+  end
+  let(:api) do
+    double(GitlabNet).tap do |api|
+      api.stub(discover: 'John Doe')
+      api.stub(allowed?: true)
     end
+  end
+  let(:key_id) { "key-#{rand(100) + 100}" }
+  let(:repository_path) { "/home/git#{rand(100)}/repos" }
+  before { GitlabConfig.any_instance.stub(:repos_path).and_return(repository_path) }
+
+  describe :initialize do
+    before { ssh_cmd 'git-receive-pack' }
 
-    it { @shell.key_id.should == 'key-56' }
-    it { @shell.repos_path.should == "/home/git/repositories" }
+    its(:key_id) { should == key_id }
+    its(:repos_path) { should == repository_path }
   end
 
   describe :parse_cmd do
     context 'w/o namespace' do
       before do
         ssh_cmd 'git-upload-pack gitlab-ci.git'
-        @shell = GitlabShell.new
-        @shell.send :parse_cmd
+        subject.send :parse_cmd
       end
 
-      it { @shell.repo_name.should == 'gitlab-ci.git' }
-      it { @shell.git_cmd.should == 'git-upload-pack' }
+      its(:repo_name) { should == 'gitlab-ci.git' }
+      its(:git_cmd) { should == 'git-upload-pack' }
     end
 
     context 'namespace' do
       before do
         ssh_cmd 'git-upload-pack dmitriy.zaporozhets/gitlab-ci.git'
-        @shell = GitlabShell.new
-        @shell.send :parse_cmd
+        subject.send :parse_cmd
       end
 
-      it { @shell.repo_name.should == 'dmitriy.zaporozhets/gitlab-ci.git' }
-      it { @shell.git_cmd.should == 'git-upload-pack' }
+      its(:repo_name) { should == 'dmitriy.zaporozhets/gitlab-ci.git' }
+      its(:git_cmd) { should == 'git-upload-pack' }
     end
   end
 
   describe :exec do
     context 'git-upload-pack' do
-      before do
-        ssh_cmd 'git-upload-pack gitlab-ci.git'
-        stubbed_shell
+      before { ssh_cmd 'git-upload-pack gitlab-ci.git' }
+      after { subject.exec }
+
+      it "should process the command" do
+        subject.should_receive(:process_cmd).with()
+      end
+
+      it "should execute the command" do
+        subject.should_receive(:exec_cmd).with("git-upload-pack #{File.join(repository_path, 'gitlab-ci.git')}")
       end
 
-      it { @shell.exec.should be_true }
+      it "should set the GL_ID environment variable" do
+        ENV.should_receive("[]=").with("GL_ID", key_id)
+      end
     end
 
     context 'git-receive-pack' do
-      before do
-        ssh_cmd 'git-receive-pack gitlab-ci.git'
-        stubbed_shell
+      before { ssh_cmd 'git-receive-pack gitlab-ci.git' }
+      after { subject.exec }
+
+      it "should process the command" do
+        subject.should_receive(:process_cmd).with()
       end
 
-      it { @shell.exec.should be_true }
+      it "should execute the command" do
+        subject.should_receive(:exec_cmd).with("git-receive-pack #{File.join(repository_path, 'gitlab-ci.git')}")
+      end
+    end
+
+    context 'arbitrary command' do
+      before { ssh_cmd 'arbitrary command' }
+      after { subject.exec }
+
+      it "should not process the command" do
+        subject.should_not_receive(:process_cmd)
+      end
+
+      it "should not execute the command" do
+        subject.should_not_receive(:exec_cmd)
+      end
+    end
+
+    context 'no command' do
+      before { ssh_cmd nil }
+      after { subject.exec }
+
+      it "should call api.discover" do
+        api.should_receive(:discover).with(key_id)
+      end
+    end
+  end
+
+  describe :validate_access do
+    before { ssh_cmd 'git-upload-pack gitlab-ci.git' }
+    after { subject.exec }
+
+    it "should call api.allowed?" do
+      api.should_receive(:allowed?).
+        with('git-upload-pack', 'gitlab-ci.git', key_id, '_any')
     end
   end
 
@@ -61,10 +116,4 @@ describe GitlabShell do
     ENV['SSH_ORIGINAL_COMMAND'] = cmd
   end
 
-  def stubbed_shell
-    ARGV[0] = 'key-56'
-    @shell = GitlabShell.new
-    @shell.stub(validate_access: true)
-    @shell.stub(process_cmd: true)
-  end
 end