diff options
Diffstat (limited to 'spec/services')
3 files changed, 65 insertions, 274 deletions
diff --git a/spec/services/ci/delete_objects_service_spec.rb b/spec/services/ci/delete_objects_service_spec.rb index a64c0fa4076..939b72cef3b 100644 --- a/spec/services/ci/delete_objects_service_spec.rb +++ b/spec/services/ci/delete_objects_service_spec.rb @@ -4,203 +4,108 @@ require 'spec_helper' RSpec.describe Ci::DeleteObjectsService, :aggregate_failures, feature_category: :continuous_integration do let(:service) { described_class.new } + let(:artifact) { create(:ci_job_artifact, :archive) } let(:data) { [artifact] } describe '#execute' do - shared_examples_for 'deleting objects' do |store_type| - before do - Ci::DeletedObject.bulk_import(data) - # We disable the check because the specs are wrapped in a transaction - allow(service).to receive(:transaction_open?).and_return(false) - end + before do + Ci::DeletedObject.bulk_import(data) + # We disable the check because the specs are wrapped in a transaction + allow(service).to receive(:transaction_open?).and_return(false) + end - subject(:execute) { service.execute } + subject(:execute) { service.execute } - it 'deletes records' do - expect { execute }.to change { Ci::DeletedObject.count }.by(-1) - end + it 'deletes records' do + expect { execute }.to change { Ci::DeletedObject.count }.by(-1) + end - it 'deletes files' do - if store_type == :object_storage - expect { execute } - .to change { fog_connection.directories.get(bucket).files.any? } - else - expect { execute }.to change { artifact.file.exists? } - end - end + it 'deletes files' do + expect { execute }.to change { artifact.file.exists? } + end - context 'when trying to execute without records' do - let(:data) { [] } + context 'when trying to execute without records' do + let(:data) { [] } - it 'does not change the number of objects' do - expect { execute }.not_to change { Ci::DeletedObject.count } - end + it 'does not change the number of objects' do + expect { execute }.not_to change { Ci::DeletedObject.count } end + end - context 'when trying to remove the same file multiple times' do - let(:objects) { Ci::DeletedObject.all.to_a } - - before do - expect(service).to receive(:load_next_batch).twice.and_return(objects) - end + context 'when trying to remove the same file multiple times' do + let(:objects) { Ci::DeletedObject.all.to_a } - it 'executes successfully' do - 2.times { expect(service.execute).to be_truthy } - end + before do + expect(service).to receive(:load_next_batch).twice.and_return(objects) end - context 'with artifacts both ready and not ready for deletion' do - let(:data) { [] } - - let_it_be(:past_ready) { create(:ci_deleted_object, pick_up_at: 2.days.ago) } - let_it_be(:ready) { create(:ci_deleted_object, pick_up_at: 1.day.ago) } - - it 'skips records with pick_up_at in the future' do - not_ready = create(:ci_deleted_object, pick_up_at: 1.day.from_now) - - expect { execute }.to change { Ci::DeletedObject.count }.from(3).to(1) - expect(not_ready.reload.present?).to be_truthy - end - - it 'limits the number of records removed' do - stub_const("#{described_class}::BATCH_SIZE", 1) - - expect { execute }.to change { Ci::DeletedObject.count }.by(-1) - end - - it 'removes records in order' do - stub_const("#{described_class}::BATCH_SIZE", 1) - - execute - - expect { past_ready.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect(ready.reload.present?).to be_truthy - end + it 'executes successfully' do + 2.times { expect(service.execute).to be_truthy } + end + end - it 'updates pick_up_at timestamp' do - allow(service).to receive(:destroy_everything) + context 'with artifacts both ready and not ready for deletion' do + let(:data) { [] } - execute + let_it_be(:past_ready) { create(:ci_deleted_object, pick_up_at: 2.days.ago) } + let_it_be(:ready) { create(:ci_deleted_object, pick_up_at: 1.day.ago) } - expect(past_ready.reload.pick_up_at).to be_like_time(10.minutes.from_now) - end + it 'skips records with pick_up_at in the future' do + not_ready = create(:ci_deleted_object, pick_up_at: 1.day.from_now) - it 'does not delete objects for which file deletion has failed' do - expect(past_ready) - .to receive(:delete_file_from_storage) - .and_return(false) + expect { execute }.to change { Ci::DeletedObject.count }.from(3).to(1) + expect(not_ready.reload.present?).to be_truthy + end - expect(service) - .to receive(:load_next_batch) - .and_return([past_ready, ready]) + it 'limits the number of records removed' do + stub_const("#{described_class}::BATCH_SIZE", 1) - expect { execute }.to change { Ci::DeletedObject.count }.from(2).to(1) - expect(past_ready.reload.present?).to be_truthy - end + expect { execute }.to change { Ci::DeletedObject.count }.by(-1) end - context 'with an open database transaction' do - it 'raises an exception and does not remove records' do - expect(service).to receive(:transaction_open?).and_return(true) + it 'removes records in order' do + stub_const("#{described_class}::BATCH_SIZE", 1) - expect { execute } - .to raise_error(Ci::DeleteObjectsService::TransactionInProgressError) - .and change { Ci::DeletedObject.count }.by(0) - end + execute + + expect { past_ready.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(ready.reload.present?).to be_truthy end - end - context 'when local storage is used' do - let(:artifact) { create(:ci_job_artifact, :archive) } + it 'updates pick_up_at timestamp' do + allow(service).to receive(:destroy_everything) - it_behaves_like 'deleting objects', :local_storage - end + execute - context 'when object storage is used' do - shared_examples_for 'deleting objects from object storage' do - let!(:fog_file) do - fog_connection.directories.new(key: bucket).files.create( # rubocop:disable Rails/SaveBang - key: fog_file_path, - body: 'content' - ) - end - - let(:artifact) do - create( - :ci_job_artifact, - :remote_store - ).tap do |a| - a.update_column(:file, 'artifacts.zip') - a.reload - end - end - - context 'and object was direct uploaded to its final location' do - let(:upload_path) { 'some/path/to/randomfile' } - - before do - artifact.update_column(:file_final_path, upload_path) - artifact.reload - end - - it_behaves_like 'deleting objects', :object_storage - end - - context 'and object was not direct uploaded to its final location' do - let(:upload_path) do - File.join( - artifact.file.store_dir.to_s, - artifact.file_identifier - ) - end - - it_behaves_like 'deleting objects', :object_storage - end + expect(past_ready.reload.pick_up_at).to be_like_time(10.minutes.from_now) end - context 'and bucket prefix is not configured' do - let(:fog_connection) do - stub_artifacts_object_storage - end + it 'does not delete objects for which file deletion has failed' do + expect(past_ready) + .to receive(:delete_file_from_storage) + .and_return(false) - let(:bucket) { 'artifacts' } - let(:fog_file_path) { upload_path } + expect(service) + .to receive(:load_next_batch) + .and_return([past_ready, ready]) - it_behaves_like 'deleting objects from object storage' + expect { execute }.to change { Ci::DeletedObject.count }.from(2).to(1) + expect(past_ready.reload.present?).to be_truthy end + end - context 'and bucket prefix is configured' do - let(:fog_config) do - Gitlab.config.artifacts.object_store.tap do |config| - config[:remote_directory] = bucket - config[:bucket_prefix] = bucket_prefix - end - end - - let(:fog_connection) do - stub_object_storage_uploader( - config: fog_config, - uploader: JobArtifactUploader - ) - end - - let(:bucket_prefix) { 'my/artifacts' } - let(:bucket) { 'main-bucket' } - let(:fog_file_path) { File.join(bucket_prefix, upload_path) } - - it_behaves_like 'deleting objects from object storage' + context 'with an open database transaction' do + it 'raises an exception and does not remove records' do + expect(service).to receive(:transaction_open?).and_return(true) + + expect { execute } + .to raise_error(Ci::DeleteObjectsService::TransactionInProgressError) + .and change { Ci::DeletedObject.count }.by(0) end end end describe '#remaining_batches_count' do - let(:artifact) do - create( - :ci_job_artifact, - :archive - ) - end - subject { service.remaining_batches_count(max_batch_count: 3) } context 'when there is less than one batch size' do diff --git a/spec/services/metrics/users_starred_dashboards/create_service_spec.rb b/spec/services/metrics/users_starred_dashboards/create_service_spec.rb deleted file mode 100644 index e08bdca8410..00000000000 --- a/spec/services/metrics/users_starred_dashboards/create_service_spec.rb +++ /dev/null @@ -1,73 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Metrics::UsersStarredDashboards::CreateService, feature_category: :metrics do - let_it_be(:user) { create(:user) } - - let(:dashboard_path) { 'config/prometheus/common_metrics.yml' } - let(:service_instance) { described_class.new(user, project, dashboard_path) } - let(:project) { create(:project) } - let(:starred_dashboard_params) do - { - user: user, - project: project, - dashboard_path: dashboard_path - } - end - - shared_examples 'prevented starred dashboard creation' do |message| - it 'returns error response', :aggregate_failures do - expect(Metrics::UsersStarredDashboard).not_to receive(:new) - - response = service_instance.execute - - expect(response.status).to be :error - expect(response.message).to eql message - end - end - - describe '.execute' do - context 'with anonymous user' do - it_behaves_like 'prevented starred dashboard creation', 'You are not authorized to add star to this dashboard' - end - - context 'with reporter user' do - before do - project.add_reporter(user) - end - - context 'incorrect dashboard_path' do - let(:dashboard_path) { 'something_incorrect.yml' } - - it_behaves_like 'prevented starred dashboard creation', 'Dashboard with requested path can not be found' - end - - context 'with valid dashboard path' do - it 'creates starred dashboard and returns success response', :aggregate_failures do - expect_next_instance_of(Metrics::UsersStarredDashboard, starred_dashboard_params) do |starred_dashboard| - expect(starred_dashboard).to receive(:save).and_return true - end - - response = service_instance.execute - - expect(response.status).to be :success - end - - context 'Metrics::UsersStarredDashboard has validation errors' do - it 'returns error response', :aggregate_failures do - expect_next_instance_of(Metrics::UsersStarredDashboard, starred_dashboard_params) do |starred_dashboard| - expect(starred_dashboard).to receive(:save).and_return(false) - expect(starred_dashboard).to receive(:errors).and_return(double(messages: { base: ['Model validation error'] })) - end - - response = service_instance.execute - - expect(response.status).to be :error - expect(response.message).to eql(base: ['Model validation error']) - end - end - end - end - end -end diff --git a/spec/services/metrics/users_starred_dashboards/delete_service_spec.rb b/spec/services/metrics/users_starred_dashboards/delete_service_spec.rb deleted file mode 100644 index 8c4bcecc239..00000000000 --- a/spec/services/metrics/users_starred_dashboards/delete_service_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Metrics::UsersStarredDashboards::DeleteService, feature_category: :metrics do - subject(:service_instance) { described_class.new(user, project, dashboard_path) } - - let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } - - describe '#execute' do - let_it_be(:user_starred_dashboard_1) { create(:metrics_users_starred_dashboard, user: user, project: project, dashboard_path: 'dashboard_1') } - let_it_be(:user_starred_dashboard_2) { create(:metrics_users_starred_dashboard, user: user, project: project) } - let_it_be(:other_user_starred_dashboard) { create(:metrics_users_starred_dashboard, project: project) } - let_it_be(:other_project_starred_dashboard) { create(:metrics_users_starred_dashboard, user: user) } - - context 'without dashboard_path' do - let(:dashboard_path) { nil } - - it 'does not scope user starred dashboards by dashboard path' do - result = service_instance.execute - - expect(result.success?).to be_truthy - expect(result.payload[:deleted_rows]).to be(2) - expect(Metrics::UsersStarredDashboard.all).to contain_exactly(other_user_starred_dashboard, other_project_starred_dashboard) - end - end - - context 'with dashboard_path' do - let(:dashboard_path) { 'dashboard_1' } - - it 'does scope user starred dashboards by dashboard path' do - result = service_instance.execute - - expect(result.success?).to be_truthy - expect(result.payload[:deleted_rows]).to be(1) - expect(Metrics::UsersStarredDashboard.all).to contain_exactly(user_starred_dashboard_2, other_user_starred_dashboard, other_project_starred_dashboard) - end - end - end -end |