diff options
Diffstat (limited to 'spec/services/ci/job_artifacts')
4 files changed, 140 insertions, 44 deletions
diff --git a/spec/services/ci/job_artifacts/create_service_spec.rb b/spec/services/ci/job_artifacts/create_service_spec.rb index 5df590a1b78..711002e28af 100644 --- a/spec/services/ci/job_artifacts/create_service_spec.rb +++ b/spec/services/ci/job_artifacts/create_service_spec.rb @@ -61,6 +61,49 @@ RSpec.describe Ci::JobArtifacts::CreateService do expect(new_artifact.locked).to eq(job.pipeline.locked) end + it 'sets accessibility level by default to public' do + expect { subject }.to change { Ci::JobArtifact.count }.by(1) + + new_artifact = job.job_artifacts.last + expect(new_artifact).to be_public_accessibility + end + + context 'when accessibility level passed as private' do + before do + params.merge!('accessibility' => 'private') + end + + it 'sets accessibility level to private' do + expect { subject }.to change { Ci::JobArtifact.count }.by(1) + + new_artifact = job.job_artifacts.last + expect(new_artifact).to be_private_accessibility + end + end + + context 'when accessibility passed as public' do + before do + params.merge!('accessibility' => 'public') + end + + it 'sets accessibility to public level' do + expect { subject }.to change { Ci::JobArtifact.count }.by(1) + + new_artifact = job.job_artifacts.last + expect(new_artifact).to be_public_accessibility + end + end + + context 'when accessibility passed as invalid value' do + before do + params.merge!('accessibility' => 'invalid_value') + end + + it 'fails with argument error' do + expect { subject }.to raise_error(ArgumentError) + end + end + context 'when metadata file is also uploaded' do let(:metadata_file) do file_to_upload('spec/fixtures/ci_build_artifacts_metadata.gz', sha256: artifacts_sha256) @@ -82,6 +125,39 @@ RSpec.describe Ci::JobArtifacts::CreateService do expect(new_artifact.locked).to eq(job.pipeline.locked) end + it 'sets accessibility by default to public' do + expect { subject }.to change { Ci::JobArtifact.count }.by(2) + + new_artifact = job.job_artifacts.last + expect(new_artifact).to be_public_accessibility + end + + context 'when accessibility level passed as private' do + before do + params.merge!('accessibility' => 'private') + end + + it 'sets accessibility to private level' do + expect { subject }.to change { Ci::JobArtifact.count }.by(2) + + new_artifact = job.job_artifacts.last + expect(new_artifact).to be_private_accessibility + end + end + + context 'when accessibility passed as public' do + before do + params.merge!('accessibility' => 'public') + end + + it 'sets accessibility level to public' do + expect { subject }.to change { Ci::JobArtifact.count }.by(2) + + new_artifact = job.job_artifacts.last + expect(new_artifact).to be_public_accessibility + end + end + it 'sets expiration date according to application settings' do expected_expire_at = 1.day.from_now diff --git a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb index 4f7663d7996..dd10c0df374 100644 --- a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb @@ -87,12 +87,9 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s expect { subject }.to change { Ci::DeletedObject.count }.by(1) end - it 'resets project statistics' do - expect(ProjectStatistics).to receive(:increment_statistic).once - .with(artifact.project, :build_artifacts_size, -artifact.file.size) - .and_call_original - - subject + it 'resets project statistics', :sidekiq_inline do + expect { subject } + .to change { artifact.project.statistics.reload.build_artifacts_size }.by(-artifact.file.size) end it 'does not remove the files' do diff --git a/spec/services/ci/job_artifacts/destroy_associations_service_spec.rb b/spec/services/ci/job_artifacts/destroy_associations_service_spec.rb index b1a4741851b..ca36c923dcf 100644 --- a/spec/services/ci/job_artifacts/destroy_associations_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_associations_service_spec.rb @@ -3,23 +3,23 @@ require 'spec_helper' RSpec.describe Ci::JobArtifacts::DestroyAssociationsService do - let(:artifacts) { Ci::JobArtifact.all } - let(:service) { described_class.new(artifacts) } + let_it_be(:project_1) { create(:project) } + let_it_be(:project_2) { create(:project) } - let_it_be(:artifact, refind: true) do - create(:ci_job_artifact) - end + let_it_be(:artifact_1, refind: true) { create(:ci_job_artifact, :zip, project: project_1) } + let_it_be(:artifact_2, refind: true) { create(:ci_job_artifact, :zip, project: project_2) } + let_it_be(:artifact_3, refind: true) { create(:ci_job_artifact, :zip, project: project_1) } - before do - artifact.file = fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip') - artifact.save! - end + let(:artifacts) { Ci::JobArtifact.where(id: [artifact_1.id, artifact_2.id, artifact_3.id]) } + let(:service) { described_class.new(artifacts) } describe '#destroy_records' do it 'removes artifacts without updating statistics' do - expect(ProjectStatistics).not_to receive(:increment_statistic) + expect_next_instance_of(Ci::JobArtifacts::DestroyBatchService) do |service| + expect(service).to receive(:execute).with(update_stats: false).and_call_original + end - expect { service.destroy_records }.to change { Ci::JobArtifact.count } + expect { service.destroy_records }.to change { Ci::JobArtifact.count }.by(-3) end context 'when there are no artifacts' do @@ -33,12 +33,21 @@ RSpec.describe Ci::JobArtifacts::DestroyAssociationsService do describe '#update_statistics' do before do + stub_const("#{described_class}::BATCH_SIZE", 2) service.destroy_records end it 'updates project statistics' do - expect(ProjectStatistics).to receive(:increment_statistic).once - .with(artifact.project, :build_artifacts_size, -artifact.file.size) + project1_increments = [ + have_attributes(amount: -artifact_1.size, ref: artifact_1.id), + have_attributes(amount: -artifact_3.size, ref: artifact_3.id) + ] + project2_increments = [have_attributes(amount: -artifact_2.size, ref: artifact_2.id)] + + expect(ProjectStatistics).to receive(:bulk_increment_statistic).once + .with(project_1, :build_artifacts_size, match_array(project1_increments)) + expect(ProjectStatistics).to receive(:bulk_increment_statistic).once + .with(project_2, :build_artifacts_size, match_array(project2_increments)) service.update_statistics end diff --git a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb index 79920dcb2c7..cde42783d8c 100644 --- a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do create(:ci_job_artifact, :trace, :expired) end - describe '.execute' do + describe '#execute' do subject(:execute) { service.execute } it 'creates a deleted object for artifact with attached file' do @@ -207,44 +207,58 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do end end - context 'ProjectStatistics' do - it 'resets project statistics' do - expect(ProjectStatistics).to receive(:increment_statistic).once - .with(artifact_with_file.project, :build_artifacts_size, -artifact_with_file.file.size) - .and_call_original - expect(ProjectStatistics).to receive(:increment_statistic).once - .with(artifact_without_file.project, :build_artifacts_size, 0) - .and_call_original + context 'ProjectStatistics', :sidekiq_inline do + let_it_be(:project_1) { create(:project) } + let_it_be(:project_2) { create(:project) } + + let(:artifact_with_file) { create(:ci_job_artifact, :zip, project: project_1) } + let(:artifact_with_file_2) { create(:ci_job_artifact, :zip, project: project_1) } + let(:artifact_without_file) { create(:ci_job_artifact, project: project_2) } + let!(:artifacts) { Ci::JobArtifact.where(id: [artifact_with_file.id, artifact_without_file.id, artifact_with_file_2.id]) } + + it 'updates project statistics by the relevant amount' do + expected_amount = -(artifact_with_file.size + artifact_with_file_2.size) + + expect { execute } + .to change { project_1.statistics.reload.build_artifacts_size }.by(expected_amount) + .and change { project_2.statistics.reload.build_artifacts_size }.by(0) + end + + it 'increments project statistics with artifact size as amount and job artifact id as ref' do + project_1_increments = [ + have_attributes(amount: -artifact_with_file.size, ref: artifact_with_file.id), + have_attributes(amount: -artifact_with_file_2.file.size, ref: artifact_with_file_2.id) + ] + project_2_increments = [have_attributes(amount: 0, ref: artifact_without_file.id)] + + expect(ProjectStatistics).to receive(:bulk_increment_statistic).with(project_1, :build_artifacts_size, match_array(project_1_increments)) + expect(ProjectStatistics).to receive(:bulk_increment_statistic).with(project_2, :build_artifacts_size, match_array(project_2_increments)) execute end context 'with update_stats: false' do - let_it_be(:extra_artifact_with_file) do - create(:ci_job_artifact, :zip, project: artifact_with_file.project) - end - - let(:artifacts) do - Ci::JobArtifact.where(id: [artifact_with_file.id, extra_artifact_with_file.id, - artifact_without_file.id, trace_artifact.id]) - end + subject(:execute) { service.execute(update_stats: false) } it 'does not update project statistics' do - expect(ProjectStatistics).not_to receive(:increment_statistic) - - service.execute(update_stats: false) + expect { execute }.not_to change { [project_1.statistics.reload.build_artifacts_size, project_2.statistics.reload.build_artifacts_size] } end - it 'returns size statistics' do + it 'returns statistic updates per project' do + project_1_updates = [ + have_attributes(amount: -artifact_with_file.size, ref: artifact_with_file.id), + have_attributes(amount: -artifact_with_file_2.file.size, ref: artifact_with_file_2.id) + ] + project_2_updates = [have_attributes(amount: 0, ref: artifact_without_file.id)] + expected_updates = { statistics_updates: { - artifact_with_file.project => -(artifact_with_file.file.size + extra_artifact_with_file.file.size), - artifact_without_file.project => 0 + project_1 => match_array(project_1_updates), + project_2 => project_2_updates } } - expect(service.execute(update_stats: false)).to match( - a_hash_including(expected_updates)) + expect(execute).to match(a_hash_including(expected_updates)) end end end |