Add tests.

app/controllers/projects/repositories_controller.rb

@@ -11,14 +11,10 @@ class Projects::RepositoriesController < Projects::ApplicationController
   end
   def archive
-    unless can?(current_user, :download_code, @project)
-      render_404 and return
-    end
-
     begin
       file_path = ArchiveRepositoryService.new(@project, params[:ref], params[:format]).execute
     rescue
-      return render_404
+      return head :not_found
     end
     if file_path

app/services/archive_repository_service.rb

@@ -6,7 +6,7 @@ class ArchiveRepositoryService
     @project, @ref, @format = project, ref, format.downcase
   end
-  def execute
+  def execute(options = {})
     project.repository.clean_old_archives
     raise "No archive file path" unless file_path
@@ -17,7 +17,7 @@ class ArchiveRepositoryService
       RepositoryArchiveWorker.perform_async(project.id, ref, format)
     end
-    archived = wait_until_archived
+    archived = wait_until_archived(options[:timeout] || 5.0)
     file_path if archived
   end
@@ -45,6 +45,8 @@ class ArchiveRepositoryService
   end
   def wait_until_archived(timeout = 5.0)
+    return archived? if timeout == 0.0
+    
     t1 = Time.now
     begin

app/workers/repository_archive_worker.rb

@@ -13,6 +13,7 @@ class RepositoryArchiveWorker
     repository.clean_old_archives
+    return unless file_path
     return if archived? || archiving?
     repository.archive_repo(ref, storage_path, format)

spec/controllers/projects/repositories_controller_spec.rb

+require "spec_helper"
+
+describe Projects::RepositoriesController do
+  let(:project) { create(:project) }
+  let(:user)    { create(:user) }
+
+  describe "GET archive" do
+    before do
+      sign_in(user)
+      project.team << [user, :developer]
+
+      allow(ArchiveRepositoryService).to receive(:new).and_return(service)
+    end
+
+    let(:service) { ArchiveRepositoryService.new(project, "master", "zip") }
+
+    it "executes ArchiveRepositoryService" do
+      expect(ArchiveRepositoryService).to receive(:new).with(project, "master", "zip")
+      expect(service).to receive(:execute)
+
+      get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip"
+    end
+
+    context "when the service raises an error" do
+
+      before do
+        allow(service).to receive(:execute).and_raise("Archive failed")
+      end
+
+      it "renders Not Found" do
+        get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip"
+
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context "when the service doesn't return a path" do
+
+      before do
+        allow(service).to receive(:execute).and_return(nil)
+      end
+
+      it "reloads the page" do
+        get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip"
+
+        expect(response).to redirect_to(archive_namespace_project_repository_path(project.namespace, project, ref: "master", format: "zip"))
+      end
+    end
+
+    context "when the service returns a path" do
+
+      let(:path) { Rails.root.join("spec/fixtures/dk.png").to_s }
+
+      before do
+        allow(service).to receive(:execute).and_return(path)
+      end
+
+      it "sends the file" do
+        get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip"
+
+        expect(response.body).to eq(File.binread(path))
+      end
+    end
+  end
+end

spec/services/archive_repository_service_spec.rb

+require 'spec_helper'
+
+describe ArchiveRepositoryService do
+  let(:project) { create(:project) }
+  subject { ArchiveRepositoryService.new(project, "master", "zip") }
+
+  describe "#execute" do
+    it "cleans old archives" do
+      expect(project.repository).to receive(:clean_old_archives)
+
+      subject.execute(timeout: 0.0)
+    end
+
+    context "when the repository doesn't have an archive file path" do
+      before do
+        allow(project.repository).to receive(:archive_file_path).and_return(nil)
+      end
+
+      it "raises an error" do
+        expect {
+          subject.execute(timeout: 0.0)
+        }.to raise_error
+      end
+    end
+
+    context "when the repository has an archive file path" do
+      let(:file_path)     { "/archive.zip" }
+      let(:pid_file_path) { "/archive.zip.pid" }
+
+      before do
+        allow(project.repository).to receive(:archive_file_path).and_return(file_path)
+        allow(project.repository).to receive(:archive_pid_file_path).and_return(pid_file_path)
+      end
+
+      context "when the archive file already exists" do
+        before do
+          allow(File).to receive(:exist?).with(file_path).and_return(true)
+        end
+
+        it "returns the file path" do
+          expect(subject.execute(timeout: 0.0)).to eq(file_path)
+        end
+      end
+
+      context "when the archive file doesn't exist yet" do
+        before do
+          allow(File).to receive(:exist?).with(file_path).and_return(false)
+          allow(File).to receive(:exist?).with(pid_file_path).and_return(true)
+        end
+
+        context "when the archive pid file doesn't exist yet" do
+          before do
+            allow(File).to receive(:exist?).with(pid_file_path).and_return(false)
+          end
+
+          it "queues the RepositoryArchiveWorker" do
+            expect(RepositoryArchiveWorker).to receive(:perform_async)
+
+            subject.execute(timeout: 0.0)
+          end
+        end
+
+        context "when the archive pid file already exists" do
+          it "doesn't queue the RepositoryArchiveWorker" do
+            expect(RepositoryArchiveWorker).not_to receive(:perform_async)
+
+            subject.execute(timeout: 0.0)
+          end
+        end
+
+        context "when the archive file exists after a little while" do
+          before do
+            Thread.new do
+              sleep 0.1
+              allow(File).to receive(:exist?).with(file_path).and_return(true)
+            end
+          end
+
+          it "returns the file path" do
+            expect(subject.execute(timeout: 0.2)).to eq(file_path)
+          end
+        end
+
+        context "when the archive file doesn't exist after the timeout" do
+          it "returns nil" do
+            expect(subject.execute(timeout: 0.0)).to eq(nil)
+          end
+        end
+      end
+    end
+  end
+end
+

spec/workers/repository_archive_worker_spec.rb

+require 'spec_helper'
+
+describe RepositoryArchiveWorker do
+  let(:project) { create(:project) }
+  subject { RepositoryArchiveWorker.new }
+
+  before do
+    allow(Project).to receive(:find).and_return(project)
+  end
+
+  describe "#perform" do
+    it "cleans old archives" do
+      expect(project.repository).to receive(:clean_old_archives)
+
+      subject.perform(project.id, "master", "zip")
+    end
+
+    context "when the repository doesn't have an archive file path" do
+      before do
+        allow(project.repository).to receive(:archive_file_path).and_return(nil)
+      end
+
+      it "doesn't archive the repo" do
+        expect(project.repository).not_to receive(:archive_repo)
+
+        subject.perform(project.id, "master", "zip")
+      end
+    end
+
+    context "when the repository has an archive file path" do
+      let(:file_path)     { "/archive.zip" }
+      let(:pid_file_path) { "/archive.zip.pid" }
+
+      before do
+        allow(project.repository).to receive(:archive_file_path).and_return(file_path)
+        allow(project.repository).to receive(:archive_pid_file_path).and_return(pid_file_path)
+      end
+
+      context "when the archive file already exists" do
+        before do
+          allow(File).to receive(:exist?).with(file_path).and_return(true)
+        end
+
+        it "doesn't archive the repo" do
+          expect(project.repository).not_to receive(:archive_repo)
+
+          subject.perform(project.id, "master", "zip")
+        end
+      end
+
+      context "when the archive file doesn't exist yet" do
+        before do
+          allow(File).to receive(:exist?).with(file_path).and_return(false)
+          allow(File).to receive(:exist?).with(pid_file_path).and_return(true)
+        end
+
+        context "when the archive pid file doesn't exist yet" do
+          before do
+            allow(File).to receive(:exist?).with(pid_file_path).and_return(false)
+          end
+
+          it "archives the repo" do
+            expect(project.repository).to receive(:archive_repo)
+
+            subject.perform(project.id, "master", "zip")
+          end
+        end
+
+        context "when the archive pid file already exists" do
+          it "doesn't archive the repo" do
+            expect(project.repository).not_to receive(:archive_repo)
+
+            subject.perform(project.id, "master", "zip")
+          end
+        end
+      end
+    end
+  end
+end
+