From a85948862ebadb02dbe93d5ce74ba00f32d9a0c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mica=C3=ABl=20Bergeron?= Date: Tue, 6 Mar 2018 16:19:24 -0500 Subject: fix some broken specs --- .../object_storage/background_move_worker.rb | 1 - app/workers/object_storage_upload_worker.rb | 1 - lib/gitlab/workhorse.rb | 5 +++- .../projects/artifacts_controller_spec.rb | 1 + spec/models/ci/build_spec.rb | 28 ++++++---------------- spec/models/ci/job_artifact_spec.rb | 24 ++++--------------- spec/requests/api/jobs_spec.rb | 13 ++++++++++ spec/serializers/pipeline_serializer_spec.rb | 1 + 8 files changed, 31 insertions(+), 43 deletions(-) diff --git a/app/workers/object_storage/background_move_worker.rb b/app/workers/object_storage/background_move_worker.rb index 3d0a2109b1d..9c4d72e0ecf 100644 --- a/app/workers/object_storage/background_move_worker.rb +++ b/app/workers/object_storage/background_move_worker.rb @@ -11,7 +11,6 @@ module ObjectStorage return unless uploader_class < ObjectStorage::Concern return unless uploader_class.object_store_enabled? - return unless uploader_class.licensed? return unless uploader_class.background_upload_enabled? subject = subject_class.find(subject_id) diff --git a/app/workers/object_storage_upload_worker.rb b/app/workers/object_storage_upload_worker.rb index 50d7cc82faa..5c80f34069c 100644 --- a/app/workers/object_storage_upload_worker.rb +++ b/app/workers/object_storage_upload_worker.rb @@ -12,7 +12,6 @@ class ObjectStorageUploadWorker return unless uploader_class < ObjectStorage::Concern return unless uploader_class.object_store_enabled? - return unless uploader_class.licensed? return unless uploader_class.background_upload_enabled? subject = subject_class.find(subject_id) diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index 26a75651731..823df67ea39 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -147,8 +147,11 @@ module Gitlab end def send_artifacts_entry(build, entry) + file = build.artifacts_file + archive = file.file_storage? ? file.path : file.url + params = { - 'Archive' => build.artifacts_file.path, + 'Archive' => archive, 'Entry' => Base64.encode64(entry.to_s) } diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index 4ea6f869aa3..f385de725e8 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -118,6 +118,7 @@ describe Projects::ArtifactsController do shared_examples 'a valid file' do it 'serves the file using workhorse' do + binding.pry subject expect(response).to have_gitlab_http_status(200) diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 917e6eec753..74930a8d59c 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -161,21 +161,17 @@ describe Ci::Build do end end - context 'when legacy artifacts are used' do - context 'artifacts archive does not exist' do - let(:build) { create(:ci_build) } - - context 'is not expired' do - it { is_expected.to be_truthy } - end - end - end - context 'when legacy artifacts are used' do let(:build) { create(:ci_build, :legacy_artifacts) } subject { build.artifacts? } + context 'is expired' do + let(:build) { create(:ci_build, :legacy_artifacts, :expired) } + + it { is_expected.to be_falsy } + end + context 'artifacts archive does not exist' do let(:build) { create(:ci_build) } @@ -186,19 +182,13 @@ describe Ci::Build do let(:build) { create(:ci_build, :legacy_artifacts) } it { is_expected.to be_truthy } - - context 'is expired' do - let(:build) { create(:ci_build, :legacy_artifacts, :expired) } - - it { is_expected.to be_falsy } - end end end end describe '#browsable_artifacts?' do subject { build.browsable_artifacts? } - + context 'artifacts metadata does not exist' do before do build.update_attributes(legacy_artifacts_metadata: nil) @@ -219,10 +209,6 @@ describe Ci::Build do subject { build.downloadable_single_artifacts_file? } - before do - expect_any_instance_of(Ci::Build).to receive(:artifacts_metadata?).and_call_original - end - context 'artifacts are stored locally' do let(:store) { ObjectStoreUploader::LOCAL_STORE } diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index a244c2c1762..730b7691279 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -33,28 +33,14 @@ describe Ci::JobArtifact do context 'when object storage is enabled' do context 'when background upload is enabled' do - context 'when is licensed' do - before do - stub_artifacts_object_storage(background_upload: true) - end - - it 'schedules the model for migration' do - expect(ObjectStorageUploadWorker).to receive(:perform_async).with('JobArtifactUploader', described_class.name, :file, kind_of(Numeric)) - - subject - end + before do + stub_artifacts_object_storage(background_upload: true) end - context 'when is unlicensed' do - before do - stub_artifacts_object_storage(background_upload: true, licensed: false) - end - - it 'does not schedule the migration' do - expect(ObjectStorageUploadWorker).not_to receive(:perform_async) + it 'schedules the model for migration' do + expect(ObjectStorageUploadWorker).to receive(:perform_async).with('JobArtifactUploader', described_class.name, :file, kind_of(Numeric)) - subject - end + subject end end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 983bef6cdd0..ae0c377682e 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -307,6 +307,19 @@ describe API::Jobs do end describe 'GET /projects/:id/jobs/:job_id/artifacts' do + shared_examples 'downloads artifact' do + let(:download_headers) do + { 'Content-Transfer-Encoding' => 'binary', + 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } + end + + it 'returns specific job artifacts' do + expect(response).to have_gitlab_http_status(200) + expect(response.headers).to include(download_headers) + expect(response.body).to match_file(job.artifacts_file.file.file) + end + end + before do stub_artifacts_object_storage get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 14c9f5cb58c..f7f10199765 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -116,6 +116,7 @@ describe PipelineSerializer do shared_examples 'no N+1 queries' do it 'verifies number of queries', :request_store do + binding.pry recorded = ActiveRecord::QueryRecorder.new { subject } expect(recorded.count).to be_within(1).of(40) -- cgit v1.2.1