diff options
author | Shinya Maeda <shinya@gitlab.com> | 2018-02-23 23:31:01 +0900 |
---|---|---|
committer | Shinya Maeda <shinya@gitlab.com> | 2018-02-23 23:31:01 +0900 |
commit | 0fca3bce812f2c37891b62f80b754099d369f92f (patch) | |
tree | e082b004845f1cffb26000d370711f48fc9d2b02 | |
parent | 801e42715c801c8460849a8ae18a3272515f36ab (diff) | |
download | gitlab-ce-0fca3bce812f2c37891b62f80b754099d369f92f.tar.gz |
Copy original file to temp file always to prevent data loss
-rw-r--r-- | app/services/ci/create_trace_artifact_service.rb | 8 | ||||
-rw-r--r-- | spec/services/ci/create_trace_artifact_service_spec.rb | 42 |
2 files changed, 33 insertions, 17 deletions
diff --git a/app/services/ci/create_trace_artifact_service.rb b/app/services/ci/create_trace_artifact_service.rb index bfc5c1c0732..d31d8b63da6 100644 --- a/app/services/ci/create_trace_artifact_service.rb +++ b/app/services/ci/create_trace_artifact_service.rb @@ -8,7 +8,7 @@ module Ci temp_file!(JobArtifactUploader.workhorse_upload_path) do |temp_path| FileUtils.cp(stream.path, temp_path) - create_job_trace!(temp_path) + create_job_trace!(job, temp_path) FileUtils.rm(stream.path) end end @@ -16,17 +16,17 @@ module Ci private - def create_job_trace!(path) + def create_job_trace!(job, path) job.create_job_artifacts_trace!( project: job.project, file_type: :trace, - file: UploadedFile.new(path, 'build.log', 'application/octet-stream') + file: UploadedFile.new(path, 'job.log', 'application/octet-stream') ) end def temp_file!(temp_dir) FileUtils.mkdir_p(temp_dir) - temp_file = Tempfile.new('file', temp_dir) + temp_file = Tempfile.new('legacy-trace-tmp-', temp_dir) temp_file&.close yield(temp_file.path) ensure diff --git a/spec/services/ci/create_trace_artifact_service_spec.rb b/spec/services/ci/create_trace_artifact_service_spec.rb index 847a88920fe..0628e6d112e 100644 --- a/spec/services/ci/create_trace_artifact_service_spec.rb +++ b/spec/services/ci/create_trace_artifact_service_spec.rb @@ -4,40 +4,56 @@ 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') } + let!(:job) { create(:ci_build, :trace_live) } + let!(:legacy_path) { job.trace.read { |stream| return stream.path } } - allow_any_instance_of(JobArtifactUploader).to receive(:move_to_cache) { false } - allow_any_instance_of(JobArtifactUploader).to receive(:move_to_store) { false } - end + it { expect(File.exists?(legacy_path)).to be_truthy } it 'creates trace artifact' do expect { subject }.to change { Ci::JobArtifact.count }.by(1) - expect(job.job_artifacts_trace.read_attribute(:file)).to eq('sample_trace') + expect(File.exists?(legacy_path)).to be_falsy + expect(File.exists?(job.job_artifacts_trace.file.path)).to be_truthy + expect(job.job_artifacts_trace.exists?).to be_truthy + expect(job.job_artifacts_trace.file.filename).to eq('job.log') end - context 'when the job has already had trace artifact' do + context 'when failed to create trace artifact record' do before do - create(:ci_job_artifact, :trace, job: job) + # When ActiveRecord error happens + allow_any_instance_of(Ci::JobArtifact).to receive(:save).and_return(false) + allow_any_instance_of(Ci::JobArtifact).to receive_message_chain(:errors, :full_messages) + .and_return("Error") + + subject rescue nil + + job.reload end - it 'does not create trace artifact' do - expect { subject }.not_to change { Ci::JobArtifact.count } + it 'keeps legacy trace and removes trace artifact' do + expect(File.exists?(legacy_path)).to be_truthy + expect(job.job_artifacts_trace).to be_nil end end end context 'when the job does not have a trace file' do + let!(:job) { create(:ci_build) } + it 'does not create trace artifact' do expect { subject }.not_to change { Ci::JobArtifact.count } end end end + + context 'when the job has already had trace artifact' do + let!(:job) { create(:ci_build, :trace_artifact) } + + it 'does not create trace artifact' do + expect { subject }.not_to change { Ci::JobArtifact.count } + end + end end end |