From 91117452e1c106352171aea56ef336dbafd322a6 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Wed, 28 Feb 2018 18:14:41 +0900
Subject: [PATCH] Raise an error if conditions are not fulfilled in archive
 method

---
 lib/gitlab/ci/trace.rb           |  3 ++-
 spec/lib/gitlab/ci/trace_spec.rb | 17 ++++++++++++++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/lib/gitlab/ci/trace.rb b/lib/gitlab/ci/trace.rb
index 34765020287..98481f3580a 100644
--- a/lib/gitlab/ci/trace.rb
+++ b/lib/gitlab/ci/trace.rb
@@ -94,7 +94,8 @@ module Gitlab
       end
 
       def archive!
-        return if trace_artifact
+        raise 'Already archived' if trace_artifact
+        raise 'Job is not finished yet' unless job.complete?
 
         if current_path
           File.open(current_path) do |stream|
diff --git a/spec/lib/gitlab/ci/trace_spec.rb b/spec/lib/gitlab/ci/trace_spec.rb
index e52f5fc39d4..ba7153ed04d 100644
--- a/spec/lib/gitlab/ci/trace_spec.rb
+++ b/spec/lib/gitlab/ci/trace_spec.rb
@@ -455,7 +455,7 @@ describe Gitlab::Ci::Trace do
 
     context 'when job does not have trace artifact' do
       context 'when trace file stored in default path' do
-        let!(:build) { create(:ci_build, :trace_live) }
+        let!(:build) { create(:ci_build, :success, :trace_live) }
         let!(:src_path) { trace.read { |s| return s.path } }
         let!(:src_checksum) { Digest::SHA256.file(src_path).digest }
 
@@ -481,7 +481,8 @@ describe Gitlab::Ci::Trace do
         end
       end
 
-      context 'when trace stored in database' do
+      context 'when trace is stored in database' do
+        let(:build) { create(:ci_build, :success) }
         let(:trace_content) { IO.read(expand_fixture_path('trace/sample_trace')) }
         let!(:src_checksum) { Digest::SHA256.digest(trace_content) }
 
@@ -519,9 +520,19 @@ describe Gitlab::Ci::Trace do
 
       it 'does not archive' do
         expect_any_instance_of(Gitlab::Ci::Trace).not_to receive(:archive_stream!)
-        expect { subject }.not_to change { Ci::JobArtifact.count }
+        expect { subject }.to raise_error('Already archived')
         expect(build.job_artifacts_trace.file.exists?).to be_truthy
       end
     end
+
+    context 'when job is not finished yet' do
+      let!(:build) { create(:ci_build, :running, :trace_live) }
+
+      it 'does not archive' do
+        expect_any_instance_of(Gitlab::Ci::Trace).not_to receive(:archive_stream!)
+        expect { subject }.to raise_error('Job is not finished yet')
+        expect(build.trace.exist?).to be_truthy
+      end
+    end
   end
 end