Add essential tests

app/uploaders/job_artifact_uploader.rb

@@ -16,7 +16,13 @@ class JobArtifactUploader < GitlabUploader
   def open
     raise 'Only File System is supported' unless file_storage?
 
-    File.open(path, "rb")
+    File.open(path, "rb") if path
+  end
+
+  def filename
+    return 'trace.log' if model.trace?
+
+    super
   end
 
   private

spec/factories/ci/job_artifacts.rb

@@ -26,5 +26,14 @@ FactoryBot.define do
           Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'), 'application/x-gzip')
       end
     end
+
+    trait :trace do
+      file_type :trace
+
+      after(:build) do |artifact, evaluator|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/trace/sample_trace'), 'text/plain')
+      end
+    end
   end
 end

spec/fixtures/trace/sample_trace

@@ -0,0 +1,18 @@
+Running with gitlab-runner 10.0.2 (a9a76a50)
+  on ShinyaMaedas-MacBook-Pro.local (e1e5600d)
+Using Docker executor with image ruby:2.1 ...
+Using docker image sha256:35c04f14f9926d1c8c68927cb43f69435fda36ecbaa3ca6f92218205363a2b99 for predefined container...
+Pulling docker image ruby:2.1 ...
+Using docker image ruby:2.1 ID=sha256:223d1eaa9523fa64e78f5a92b701c9c11cbc507f0ff62246dbbacdae395ffea3 for build container...
+Running on runner-e1e5600d-project-12-concurrent-0 via ShinyaMaedas-MacBook-Pro.local...
+Fetching changes...
+Removing hoge.txt
+HEAD is now at bc8d55a Update .gitlab-ci.yml
+Checking out bc8d55ab as master...
+Skipping Git submodules setup
+$ echo "hoge" >> hoge.txt
+Uploading artifacts...
+hoge.txt: found 1 matching files                   
+Uploading artifacts to coordinator... ok            id=1045 responseStatus=201 Created token=c1uexKnX
+Job succeeded
+

spec/lib/gitlab/ci/trace_spec.rb

@@ -238,11 +238,98 @@ describe Gitlab::Ci::Trace do
     end
   end
 
+  describe '#read' do
+    shared_examples 'read successfully with IO' do
+      it 'yields with source' do
+        trace.read do |stream|
+          expect(stream).to be_a(Gitlab::Ci::Trace::Stream)
+          expect(stream.stream).to be_a(IO)
+        end
+      end
+    end
+
+    shared_examples 'read successfully with StringIO' do
+      it 'yields with source' do
+        trace.read do |stream|
+          expect(stream).to be_a(Gitlab::Ci::Trace::Stream)
+          expect(stream.stream).to be_a(StringIO)
+        end
+      end
+    end
+
+    shared_examples 'failed to read' do
+      it 'yields without source' do
+        trace.read do |stream|
+          expect(stream).to be_a(Gitlab::Ci::Trace::Stream)
+          expect(stream.stream).to be_nil
+        end
+      end
+    end
+
+    context 'when trace artifact exists' do
+      before do
+        create(:ci_job_artifact, :trace, job: build)
+      end
+
+      it_behaves_like 'read successfully with IO'
+    end
+
+    context 'when current_path (with project_id) exists' do
+      before do
+        expect(trace).to receive(:default_path) { expand_fixture_path('trace/sample_trace') }
+      end
+
+      it_behaves_like 'read successfully with IO'
+    end
+
+    context 'when current_path (with project_ci_id) exists' do
+      before do
+        expect(trace).to receive(:deprecated_path) { expand_fixture_path('trace/sample_trace') }
+      end
+
+      it_behaves_like 'read successfully with IO'
+    end
+
+    context 'when db trace exists' do
+      before do
+        build.send(:write_attribute, :trace, "data")
+      end
+
+      it_behaves_like 'read successfully with StringIO'
+    end
+
+    context 'when no sources exist' do
+      it_behaves_like 'failed to read'
+    end
+  end
+
   describe 'trace handling' do
+    subject { trace.exist? }
+
     context 'trace does not exist' do
       it { expect(trace.exist?).to be(false) }
     end
 
+    context 'when trace artifact exists' do
+      before do
+        create(:ci_job_artifact, :trace, job: build)
+      end
+
+      it { is_expected.to be_truthy }
+
+      context 'when the trace artifact has been erased' do
+        before do
+          trace.erase!
+        end
+
+        it { is_expected.to be_falsy }
+
+        it 'removes associations' do
+          expect(Ci::JobArtifact.exists?(job_id: build.id, file_type: :trace)).to be_falsy
+        end
+      end
+    end
+
     context 'new trace path is used' do
       before do
         trace.send(:ensure_directory)

spec/models/ci/job_artifact_spec.rb

@@ -12,6 +12,9 @@ describe Ci::JobArtifact do
   it { is_expected.to respond_to(:created_at) }
   it { is_expected.to respond_to(:updated_at) }
 
+  it { is_expected.to delegate_method(:open).to(:file) }
+  it { is_expected.to delegate_method(:exists?).to(:file) }
+
   describe '#set_size' do
     it 'sets the size' do
       expect(artifact.size).to eq(106365)

spec/services/ci/create_trace_artifact_service_spec.rb

@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe Ci::CreateTraceArtifactService do
+  describe '#execute' do
+    subject { described_class.new(nil, nil).execute(job) }
+
+    let(:job) { create(:ci_build) }
+
+    context 'when the job does not have trace artifact' do
+      context 'when the job has a trace file' do
+        before do
+          allow_any_instance_of(Gitlab::Ci::Trace)
+            .to receive(:default_path) { expand_fixture_path('trace/sample_trace') }
+
+          allow_any_instance_of(JobArtifactUploader).to receive(:move_to_cache) { false }
+          allow_any_instance_of(JobArtifactUploader).to receive(:move_to_store) { false }
+        end
+
+        it 'creates trace artifact' do
+          expect { subject }.to change { Ci::JobArtifact.count }.by(1)
+
+          expect(job.job_artifacts_trace.read_attribute(:file)).to eq('trace.log')
+        end
+
+        context 'when the job has already had trace artifact' do
+          before do
+            create(:ci_job_artifact, :trace, job: job)
+          end
+
+          it 'does not create trace artifact' do
+            expect { subject }.not_to change { Ci::JobArtifact.count }
+          end
+        end
+      end
+
+      context 'when the job does not have a trace file' do
+        it 'does not create trace artifact' do
+          expect { subject }.not_to change { Ci::JobArtifact.count }
+        end
+      end
+    end
+  end
+end

spec/uploaders/job_artifact_uploader_spec.rb

@@ -11,6 +11,49 @@ describe JobArtifactUploader do
                   cache_dir: %r[artifacts/tmp/cache],
                   work_dir: %r[artifacts/tmp/work]
 
+  describe '#open' do
+    subject { uploader.open }
+
+    context 'when trace is stored in File storage' do
+      context 'when file exists' do
+        let(:file) do
+          fixture_file_upload(
+            Rails.root.join('spec/fixtures/trace/sample_trace'), 'text/plain')
+        end
+
+        before do
+          uploader.store!(file)
+        end
+
+        it 'returns io stream' do
+          is_expected.to be_a(IO)
+        end
+      end
+
+      context 'when file does not exist' do
+        it 'returns nil' do
+          is_expected.to be_nil
+        end
+      end
+    end
+  end
+
+  describe '#filename' do
+    subject { uploader.filename }
+
+    context 'when artifact file_type is archive' do
+      let(:job_artifact) { create(:ci_job_artifact, :archive) }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'when artifact file_type is trace' do
+      let(:job_artifact) { create(:ci_job_artifact, :trace) }
+
+      it { is_expected.to eq('trace.log') }
+    end
+  end
+
   context 'file is stored in valid local_path' do
     let(:file) do
       fixture_file_upload(

spec/workers/build_finished_worker_spec.rb

@@ -13,6 +13,8 @@ describe BuildFinishedWorker do
 
         expect(BuildTraceSectionsWorker)
           .to receive(:perform_async)
+        expect(CreateTraceArtifactWorker)
+          .to receive(:perform_async)
         expect_any_instance_of(BuildCoverageWorker)
           .to receive(:perform)
         expect_any_instance_of(BuildHooksWorker)

spec/workers/create_trace_artifact_worker_spec.rb

@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe CreateTraceArtifactWorker do
+  describe '#perform' do
+    subject { described_class.new.perform(job) }
+
+    context 'when job is found' do
+      let(:job) { create(:ci_build) }
+
+      it 'executes service' do
+        expect_any_instance_of(Ci::CreateTraceArtifactService)
+          .to receive(:execute)
+
+        subject
+      end
+    end
+
+    context 'when job is not found' do
+      let(:job) { nil }
+
+      it 'does not execute service' do
+        expect_any_instance_of(Ci::CreateTraceArtifactService)
+          .not_to receive(:execute)
+
+        subject
+      end
+    end
+  end
+end