diff options
Diffstat (limited to 'spec/workers')
24 files changed, 316 insertions, 266 deletions
diff --git a/spec/workers/bulk_import_worker_spec.rb b/spec/workers/bulk_import_worker_spec.rb index 61c33f123fa..ec8550bb3bc 100644 --- a/spec/workers/bulk_import_worker_spec.rb +++ b/spec/workers/bulk_import_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImportWorker do +RSpec.describe BulkImportWorker, feature_category: :importers do describe '#perform' do context 'when no bulk import is found' do it 'does nothing' do @@ -56,6 +56,19 @@ RSpec.describe BulkImportWorker do end end + context 'when maximum allowed number of import entities in progress' do + it 'reenqueues itself' do + bulk_import = create(:bulk_import, :started) + create(:bulk_import_entity, :created, bulk_import: bulk_import) + (described_class::DEFAULT_BATCH_SIZE + 1).times { |_| create(:bulk_import_entity, :started, bulk_import: bulk_import) } + + expect(described_class).to receive(:perform_in).with(described_class::PERFORM_DELAY, bulk_import.id) + expect(BulkImports::ExportRequestWorker).not_to receive(:perform_async) + + subject.perform(bulk_import.id) + end + end + context 'when bulk import is created' do it 'marks bulk import as started' do bulk_import = create(:bulk_import, :created) @@ -82,16 +95,20 @@ RSpec.describe BulkImportWorker do context 'when there are created entities to process' do let_it_be(:bulk_import) { create(:bulk_import, :created) } - it 'marks all entities as started, enqueues EntityWorker, ExportRequestWorker and reenqueues' do + before do + stub_const("#{described_class}::DEFAULT_BATCH_SIZE", 1) + end + + it 'marks a batch of entities as started, enqueues EntityWorker, ExportRequestWorker and reenqueues' do create(:bulk_import_entity, :created, bulk_import: bulk_import) create(:bulk_import_entity, :created, bulk_import: bulk_import) expect(described_class).to receive(:perform_in).with(described_class::PERFORM_DELAY, bulk_import.id) - expect(BulkImports::ExportRequestWorker).to receive(:perform_async).twice + expect(BulkImports::ExportRequestWorker).to receive(:perform_async).once subject.perform(bulk_import.id) - expect(bulk_import.entities.map(&:status_name)).to contain_exactly(:started, :started) + expect(bulk_import.entities.map(&:status_name)).to contain_exactly(:created, :started) end context 'when there are project entities to process' do diff --git a/spec/workers/bulk_imports/pipeline_worker_spec.rb b/spec/workers/bulk_imports/pipeline_worker_spec.rb index 03ec6267ca8..e8b0714471d 100644 --- a/spec/workers/bulk_imports/pipeline_worker_spec.rb +++ b/spec/workers/bulk_imports/pipeline_worker_spec.rb @@ -433,11 +433,11 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do allow(status).to receive(:failed?).and_return(false) end - entity.update!(created_at: entity_created_at) + pipeline_tracker.update!(created_at: created_at) end context 'when timeout is not reached' do - let(:entity_created_at) { 1.minute.ago } + let(:created_at) { 1.minute.ago } it 'reenqueues pipeline worker' do expect(described_class) @@ -455,8 +455,8 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do end end - context 'when timeout is reached' do - let(:entity_created_at) { 10.minutes.ago } + context 'when empty export timeout is reached' do + let(:created_at) { 10.minutes.ago } it 'marks as failed and logs the error' do expect_next_instance_of(Gitlab::Import::Logger) do |logger| @@ -485,12 +485,32 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do expect(pipeline_tracker.reload.status_name).to eq(:failed) end end + + context 'when tracker created_at is nil' do + let(:created_at) { nil } + + it 'falls back to entity created_at' do + entity.update!(created_at: 10.minutes.ago) + + expect_next_instance_of(Gitlab::Import::Logger) do |logger| + expect(logger) + .to receive(:error) + .with( + hash_including('exception.message' => 'Empty export status on source instance') + ) + end + + subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + + expect(pipeline_tracker.reload.status_name).to eq(:failed) + end + end end context 'when job reaches timeout' do it 'marks as failed and logs the error' do - old_created_at = entity.created_at - entity.update!(created_at: (BulkImports::Pipeline::NDJSON_EXPORT_TIMEOUT + 1.hour).ago) + old_created_at = pipeline_tracker.created_at + pipeline_tracker.update!(created_at: (BulkImports::Pipeline::NDJSON_EXPORT_TIMEOUT + 1.hour).ago) expect_next_instance_of(Gitlab::Import::Logger) do |logger| expect(logger) diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb index 14abe819587..0c1010960a1 100644 --- a/spec/workers/ci/archive_traces_cron_worker_spec.rb +++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::ArchiveTracesCronWorker do +RSpec.describe Ci::ArchiveTracesCronWorker, feature_category: :continuous_integration do subject { described_class.new.perform } let(:finished_at) { 1.day.ago } @@ -34,14 +34,28 @@ RSpec.describe Ci::ArchiveTracesCronWorker do it_behaves_like 'archives trace' - it 'executes service' do + it 'batch_execute service' do expect_next_instance_of(Ci::ArchiveTraceService) do |instance| - expect(instance).to receive(:execute).with(build, anything) + expect(instance).to receive(:batch_execute).with(worker_name: "Ci::ArchiveTracesCronWorker") end subject end + context "with FF deduplicate_archive_traces_cron_worker false" do + before do + stub_feature_flags(deduplicate_archive_traces_cron_worker: false) + end + + it 'calls execute service' do + expect_next_instance_of(Ci::ArchiveTraceService) do |instance| + expect(instance).to receive(:execute).with(build, worker_name: "Ci::ArchiveTracesCronWorker") + end + + subject + end + end + context 'when the job finished recently' do let(:finished_at) { 1.hour.ago } diff --git a/spec/workers/ci/cancel_redundant_pipelines_worker_spec.rb b/spec/workers/ci/cancel_redundant_pipelines_worker_spec.rb new file mode 100644 index 00000000000..f6639faab10 --- /dev/null +++ b/spec/workers/ci/cancel_redundant_pipelines_worker_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::CancelRedundantPipelinesWorker, feature_category: :continuous_integration do + let_it_be(:project) { create(:project) } + + let(:prev_pipeline) { create(:ci_pipeline, project: project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + + describe '#perform' do + subject(:perform) { described_class.new.perform(pipeline.id) } + + let(:service) { instance_double('Ci::PipelineCreation::CancelRedundantPipelinesService') } + + it 'calls cancel redundant pipeline service' do + expect(Ci::PipelineCreation::CancelRedundantPipelinesService) + .to receive(:new) + .with(pipeline) + .and_return(service) + + expect(service).to receive(:execute) + + perform + end + + context 'if pipeline is deleted' do + subject(:perform) { described_class.new.perform(non_existing_record_id) } + + it 'does not call redundant pipeline service' do + expect(Ci::PipelineCreation::CancelRedundantPipelinesService) + .not_to receive(:new) + + perform + end + end + + describe 'interacting with previous pending pipelines', :sidekiq_inline do + before do + create(:ci_build, :interruptible, :running, pipeline: prev_pipeline) + end + + it_behaves_like 'an idempotent worker', :sidekiq_inline do + let(:job_args) { pipeline } + + it 'cancels the previous pending pipeline' do + perform + + expect(prev_pipeline.builds.pluck(:status)).to contain_exactly('canceled') + end + end + end + end +end diff --git a/spec/workers/ci/external_pull_requests/create_pipeline_worker_spec.rb b/spec/workers/ci/external_pull_requests/create_pipeline_worker_spec.rb index a637ac088ff..1f20729a821 100644 --- a/spec/workers/ci/external_pull_requests/create_pipeline_worker_spec.rb +++ b/spec/workers/ci/external_pull_requests/create_pipeline_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::ExternalPullRequests::CreatePipelineWorker do +RSpec.describe Ci::ExternalPullRequests::CreatePipelineWorker, feature_category: :continuous_integration do let_it_be(:project) { create(:project, :auto_devops, :repository) } let_it_be(:user) { project.first_owner } let_it_be(:external_pull_request) do diff --git a/spec/workers/ci/runners/stale_machines_cleanup_cron_worker_spec.rb b/spec/workers/ci/runners/stale_machines_cleanup_cron_worker_spec.rb new file mode 100644 index 00000000000..d8f620bc024 --- /dev/null +++ b/spec/workers/ci/runners/stale_machines_cleanup_cron_worker_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::Runners::StaleMachinesCleanupCronWorker, feature_category: :runner_fleet do + let(:worker) { described_class.new } + + describe '#perform', :freeze_time do + subject(:perform) { worker.perform } + + let!(:runner_machine1) do + create(:ci_runner_machine, created_at: 7.days.ago, contacted_at: 7.days.ago) + end + + let!(:runner_machine2) { create(:ci_runner_machine) } + let!(:runner_machine3) { create(:ci_runner_machine, created_at: 6.days.ago) } + + it_behaves_like 'an idempotent worker' do + it 'delegates to Ci::Runners::StaleMachinesCleanupService' do + expect_next_instance_of(Ci::Runners::StaleMachinesCleanupService) do |service| + expect(service) + .to receive(:execute).and_call_original + end + + perform + + expect(worker.logging_extras).to eq({ + "extra.ci_runners_stale_machines_cleanup_cron_worker.status" => :success, + "extra.ci_runners_stale_machines_cleanup_cron_worker.deleted_machines" => true + }) + end + + it 'cleans up stale runner machines', :aggregate_failures do + expect(Ci::RunnerMachine.stale.count).to eq 1 + + expect { perform }.to change { Ci::RunnerMachine.count }.from(3).to(2) + + expect(Ci::RunnerMachine.all).to match_array [runner_machine2, runner_machine3] + end + end + end +end diff --git a/spec/workers/concerns/application_worker_spec.rb b/spec/workers/concerns/application_worker_spec.rb index 5fde54b98f0..0abb029f146 100644 --- a/spec/workers/concerns/application_worker_spec.rb +++ b/spec/workers/concerns/application_worker_spec.rb @@ -103,6 +103,15 @@ RSpec.describe ApplicationWorker do expect(instance.logging_extras).to eq({ 'extra.gitlab_foo_bar_dummy_worker.key1' => "value1", 'extra.gitlab_foo_bar_dummy_worker.key2' => "value2" }) end + it 'returns extra data to be logged that was set from #log_hash_metadata_on_done' do + instance.log_hash_metadata_on_done({ key1: 'value0', key2: 'value1' }) + + expect(instance.logging_extras).to match_array({ + 'extra.gitlab_foo_bar_dummy_worker.key1' => 'value0', + 'extra.gitlab_foo_bar_dummy_worker.key2' => 'value1' + }) + end + context 'when nothing is set' do it 'returns {}' do expect(instance.logging_extras).to eq({}) diff --git a/spec/workers/concerns/waitable_worker_spec.rb b/spec/workers/concerns/waitable_worker_spec.rb index 1449c327052..737424ffd8c 100644 --- a/spec/workers/concerns/waitable_worker_spec.rb +++ b/spec/workers/concerns/waitable_worker_spec.rb @@ -22,38 +22,6 @@ RSpec.describe WaitableWorker do subject(:job) { worker.new } - describe '.bulk_perform_and_wait' do - context '1 job' do - it 'runs the jobs asynchronously' do - arguments = [[1]] - - expect(worker).to receive(:bulk_perform_async).with(arguments) - - worker.bulk_perform_and_wait(arguments) - end - end - - context 'between 2 and 3 jobs' do - it 'runs the jobs asynchronously' do - arguments = [[1], [2], [3]] - - expect(worker).to receive(:bulk_perform_async).with(arguments) - - worker.bulk_perform_and_wait(arguments) - end - end - - context '>= 4 jobs' do - it 'runs jobs using sidekiq' do - arguments = 1.upto(5).map { |i| [i] } - - expect(worker).to receive(:bulk_perform_async).with(arguments) - - worker.bulk_perform_and_wait(arguments) - end - end - end - describe '#perform' do shared_examples 'perform' do it 'notifies the JobWaiter when done if the key is provided' do diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index c444e1f383c..c0d46a206ce 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -303,6 +303,7 @@ RSpec.describe 'Every Sidekiq worker' do 'Gitlab::JiraImport::Stage::StartImportWorker' => 5, 'Gitlab::PhabricatorImport::ImportTasksWorker' => 5, 'GitlabPerformanceBarStatsWorker' => 3, + 'GitlabSubscriptions::RefreshSeatsWorker' => 0, 'GitlabShellWorker' => 3, 'GitlabServicePingWorker' => 3, 'GroupDestroyWorker' => 3, @@ -363,6 +364,7 @@ RSpec.describe 'Every Sidekiq worker' do 'Onboarding::PipelineCreatedWorker' => 3, 'Onboarding::ProgressWorker' => 3, 'Onboarding::UserAddedWorker' => 3, + 'Namespaces::FreeUserCap::OverLimitNotificationWorker' => false, 'Namespaces::RefreshRootStatisticsWorker' => 3, 'Namespaces::RootStatisticsWorker' => 3, 'Namespaces::ScheduleAggregationWorker' => 3, diff --git a/spec/workers/google_cloud/fetch_google_ip_list_worker_spec.rb b/spec/workers/google_cloud/fetch_google_ip_list_worker_spec.rb index c0b32515d15..bdafc076465 100644 --- a/spec/workers/google_cloud/fetch_google_ip_list_worker_spec.rb +++ b/spec/workers/google_cloud/fetch_google_ip_list_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe GoogleCloud::FetchGoogleIpListWorker do +RSpec.describe GoogleCloud::FetchGoogleIpListWorker, feature_category: :build_artifacts do describe '#perform' do it 'returns success' do allow_next_instance_of(GoogleCloud::FetchGoogleIpListService) do |service| diff --git a/spec/workers/incident_management/close_incident_worker_spec.rb b/spec/workers/incident_management/close_incident_worker_spec.rb index c96bb4a3d1e..145ee780573 100644 --- a/spec/workers/incident_management/close_incident_worker_spec.rb +++ b/spec/workers/incident_management/close_incident_worker_spec.rb @@ -13,7 +13,7 @@ RSpec.describe IncidentManagement::CloseIncidentWorker do let(:issue_id) { issue.id } it 'calls the close issue service' do - expect_next_instance_of(Issues::CloseService, project: project, current_user: user) do |service| + expect_next_instance_of(Issues::CloseService, container: project, current_user: user) do |service| expect(service).to receive(:execute).with(issue, system_note: false).and_call_original end diff --git a/spec/workers/issues/close_worker_spec.rb b/spec/workers/issues/close_worker_spec.rb index 41611447db1..3902618ae03 100644 --- a/spec/workers/issues/close_worker_spec.rb +++ b/spec/workers/issues/close_worker_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Issues::CloseWorker do external_issue = ExternalIssue.new("foo", project) closer = instance_double(Issues::CloseService, execute: true) - expect(Issues::CloseService).to receive(:new).with(project: project, current_user: user).and_return(closer) + expect(Issues::CloseService).to receive(:new).with(container: project, current_user: user).and_return(closer) expect(closer).to receive(:execute).with(external_issue, commit: commit) worker.perform(project.id, external_issue.id, external_issue.class.to_s, opts) diff --git a/spec/workers/merge_requests/close_issue_worker_spec.rb b/spec/workers/merge_requests/close_issue_worker_spec.rb index 5e6bdc2a43e..72fb3be7470 100644 --- a/spec/workers/merge_requests/close_issue_worker_spec.rb +++ b/spec/workers/merge_requests/close_issue_worker_spec.rb @@ -12,7 +12,7 @@ RSpec.describe MergeRequests::CloseIssueWorker do let!(:merge_request) { create(:merge_request, source_project: project) } it 'calls the close issue service' do - expect_next_instance_of(Issues::CloseService, project: project, current_user: user) do |service| + expect_next_instance_of(Issues::CloseService, container: project, current_user: user) do |service| expect(service).to receive(:execute).with(issue, commit: merge_request) end diff --git a/spec/workers/merge_requests/delete_source_branch_worker_spec.rb b/spec/workers/merge_requests/delete_source_branch_worker_spec.rb index a7e4ffad259..e17ad02e272 100644 --- a/spec/workers/merge_requests/delete_source_branch_worker_spec.rb +++ b/spec/workers/merge_requests/delete_source_branch_worker_spec.rb @@ -13,114 +13,53 @@ RSpec.describe MergeRequests::DeleteSourceBranchWorker do before do allow_next_instance_of(::Projects::DeleteBranchWorker) do |instance| allow(instance).to receive(:perform).with(merge_request.source_project.id, user.id, - merge_request.source_branch) + merge_request.source_branch) end end - context 'when the add_delete_branch_worker feature flag is enabled' do - context 'with a non-existing merge request' do - it 'does nothing' do - expect(::Projects::DeleteBranchWorker).not_to receive(:new) - - worker.perform(non_existing_record_id, sha, user.id) - end - end + context 'with a non-existing merge request' do + it 'does nothing' do + expect(::Projects::DeleteBranchWorker).not_to receive(:new) - context 'with a non-existing user' do - it 'does nothing' do - expect(::Projects::DeleteBranchWorker).not_to receive(:new) - - worker.perform(merge_request.id, sha, non_existing_record_id) - end + worker.perform(non_existing_record_id, sha, user.id) end + end - context 'with existing user and merge request' do - it 'creates a new delete branch worker async' do - expect_next_instance_of(::Projects::DeleteBranchWorker) do |instance| - expect(instance).to receive(:perform).with(merge_request.source_project.id, user.id, - merge_request.source_branch) - end - - worker.perform(merge_request.id, sha, user.id) - end - - context 'source branch sha does not match' do - it 'does nothing' do - expect(::Projects::DeleteBranchWorker).not_to receive(:new) - - worker.perform(merge_request.id, 'new-source-branch-sha', user.id) - end - end - end + context 'with a non-existing user' do + it 'does nothing' do + expect(::Projects::DeleteBranchWorker).not_to receive(:new) - it_behaves_like 'an idempotent worker' do - let(:job_args) { [merge_request.id, sha, user.id] } + worker.perform(merge_request.id, sha, non_existing_record_id) end end - context 'when the add_delete_branch_worker feature flag is disabled' do - before do - stub_feature_flags(add_delete_branch_worker: false) - end - - context 'with a non-existing merge request' do - it 'does nothing' do - expect(::Branches::DeleteService).not_to receive(:new) - expect(::MergeRequests::RetargetChainService).not_to receive(:new) - - worker.perform(non_existing_record_id, sha, user.id) + context 'with existing user and merge request' do + it 'calls delete branch worker' do + expect_next_instance_of(::Projects::DeleteBranchWorker) do |instance| + expect(instance).to receive(:perform).with(merge_request.source_project.id, user.id, + merge_request.source_branch) end + + worker.perform(merge_request.id, sha, user.id) end - context 'with a non-existing user' do + context 'source branch sha does not match' do it 'does nothing' do - expect(::Branches::DeleteService).not_to receive(:new) - expect(::MergeRequests::RetargetChainService).not_to receive(:new) + expect(::Projects::DeleteBranchWorker).not_to receive(:new) - worker.perform(merge_request.id, sha, non_existing_record_id) + worker.perform(merge_request.id, 'new-source-branch-sha', user.id) end end - context 'with existing user and merge request' do - it 'calls service to delete source branch' do - expect_next_instance_of(::Branches::DeleteService) do |instance| - expect(instance).to receive(:execute).with(merge_request.source_branch) - end + context 'when delete worker raises an error' do + it 'still retargets the merge request' do + expect(::Projects::DeleteBranchWorker).to receive(:new).and_raise(StandardError) - worker.perform(merge_request.id, sha, user.id) - end - - it 'calls service to try retarget merge requests' do expect_next_instance_of(::MergeRequests::RetargetChainService) do |instance| expect(instance).to receive(:execute).with(merge_request) end - worker.perform(merge_request.id, sha, user.id) - end - - context 'source branch sha does not match' do - it 'does nothing' do - expect(::Branches::DeleteService).not_to receive(:new) - expect(::MergeRequests::RetargetChainService).not_to receive(:new) - - worker.perform(merge_request.id, 'new-source-branch-sha', user.id) - end - end - - context 'when delete service returns an error' do - let(:service_result) { ServiceResponse.error(message: 'placeholder') } - - it 'still retargets the merge request' do - expect_next_instance_of(::Branches::DeleteService) do |instance| - expect(instance).to receive(:execute).with(merge_request.source_branch).and_return(service_result) - end - - expect_next_instance_of(::MergeRequests::RetargetChainService) do |instance| - expect(instance).to receive(:execute).with(merge_request) - end - - worker.perform(merge_request.id, sha, user.id) - end + expect { worker.perform(merge_request.id, sha, user.id) }.to raise_error(StandardError) end end diff --git a/spec/workers/new_merge_request_worker_spec.rb b/spec/workers/new_merge_request_worker_spec.rb index 358939a963a..a8e1c3f4bf1 100644 --- a/spec/workers/new_merge_request_worker_spec.rb +++ b/spec/workers/new_merge_request_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe NewMergeRequestWorker do +RSpec.describe NewMergeRequestWorker, feature_category: :code_review_workflow do describe '#perform' do let(:worker) { described_class.new } @@ -71,19 +71,64 @@ RSpec.describe NewMergeRequestWorker do it_behaves_like 'a new merge request where the author cannot trigger notifications' end - context 'when everything is ok' do + include_examples 'an idempotent worker' do let(:user) { create(:user) } - - it 'creates a new event record' do - expect { worker.perform(merge_request.id, user.id) }.to change { Event.count }.from(0).to(1) - end - - it 'creates a notification for the mentioned user' do - expect(Notify).to receive(:new_merge_request_email) - .with(mentioned.id, merge_request.id, NotificationReason::MENTIONED) - .and_return(double(deliver_later: true)) - - worker.perform(merge_request.id, user.id) + let(:job_args) { [merge_request.id, user.id] } + + context 'when everything is ok' do + it 'creates a new event record' do + expect { worker.perform(merge_request.id, user.id) }.to change { Event.count }.from(0).to(1) + end + + it 'creates a notification for the mentioned user' do + expect(Notify).to receive(:new_merge_request_email) + .with(mentioned.id, merge_request.id, NotificationReason::MENTIONED) + .and_return(double(deliver_later: true)) + + worker.perform(merge_request.id, user.id) + end + + context 'when add_prepared_state_to_mr feature flag is off' do + before do + stub_feature_flags(add_prepared_state_to_mr: false) + end + + it 'calls the create service' do + expect_next_instance_of(MergeRequests::AfterCreateService, project: merge_request.target_project, current_user: user) do |service| + expect(service).to receive(:execute).with(merge_request) + end + + worker.perform(merge_request.id, user.id) + end + end + + context 'when add_prepared_state_to_mr feature flag is on' do + before do + stub_feature_flags(add_prepared_state_to_mr: true) + end + + context 'when the merge request is prepared' do + before do + merge_request.update!(prepared_at: Time.current) + end + + it 'does not call the create service' do + expect(MergeRequests::AfterCreateService).not_to receive(:new) + + worker.perform(merge_request.id, user.id) + end + end + + context 'when the merge request is not prepared' do + it 'calls the create service' do + expect_next_instance_of(MergeRequests::AfterCreateService, project: merge_request.target_project, current_user: user) do |service| + expect(service).to receive(:execute).with(merge_request) + end + + worker.perform(merge_request.id, user.id) + end + end + end end end end diff --git a/spec/workers/packages/debian/generate_distribution_worker_spec.rb b/spec/workers/packages/debian/generate_distribution_worker_spec.rb index a3e956f14c8..c4e974ec8eb 100644 --- a/spec/workers/packages/debian/generate_distribution_worker_spec.rb +++ b/spec/workers/packages/debian/generate_distribution_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Packages::Debian::GenerateDistributionWorker, type: :worker do +RSpec.describe Packages::Debian::GenerateDistributionWorker, type: :worker, feature_category: :package_registry do describe '#perform' do let(:container_type) { distribution.container_type } let(:distribution_id) { distribution.id } @@ -18,12 +18,6 @@ RSpec.describe Packages::Debian::GenerateDistributionWorker, type: :worker do context "for #{container_type}" do include_context 'with Debian distribution', container_type - context 'with FIPS mode enabled', :fips_mode do - it 'raises an error' do - expect { subject }.to raise_error(::Packages::FIPS::DisabledError) - end - end - context 'with mocked service' do it 'calls GenerateDistributionService' do expect(Gitlab::ErrorTracking).not_to receive(:log_exception) diff --git a/spec/workers/packages/debian/process_changes_worker_spec.rb b/spec/workers/packages/debian/process_changes_worker_spec.rb index fc482245ebe..b96b75e93b9 100644 --- a/spec/workers/packages/debian/process_changes_worker_spec.rb +++ b/spec/workers/packages/debian/process_changes_worker_spec.rb @@ -2,12 +2,14 @@ require 'spec_helper' -RSpec.describe Packages::Debian::ProcessChangesWorker, type: :worker do +RSpec.describe Packages::Debian::ProcessChangesWorker, type: :worker, feature_category: :package_registry do let_it_be(:user) { create(:user) } - let_it_be_with_reload(:distribution) { create(:debian_project_distribution, :with_file, codename: 'unstable') } + let_it_be_with_reload(:distribution) do + create(:debian_project_distribution, :with_file, codename: FFaker::Lorem.word, suite: 'unstable') + end let(:incoming) { create(:debian_incoming, project: distribution.project) } - let(:package_file) { incoming.package_files.last } + let(:package_file) { incoming.package_files.with_file_name('sample_1.2.3~alpha2_amd64.changes').first } let(:worker) { described_class.new } describe '#perform' do @@ -16,12 +18,6 @@ RSpec.describe Packages::Debian::ProcessChangesWorker, type: :worker do subject { worker.perform(package_file_id, user_id) } - context 'with FIPS mode enabled', :fips_mode do - it 'raises an error' do - expect { subject }.to raise_error(::Packages::FIPS::DisabledError) - end - end - context 'with mocked service' do it 'calls ProcessChangesService' do expect(Gitlab::ErrorTracking).not_to receive(:log_exception) diff --git a/spec/workers/packages/debian/process_package_file_worker_spec.rb b/spec/workers/packages/debian/process_package_file_worker_spec.rb index 532bfb096a3..239ee8e1035 100644 --- a/spec/workers/packages/debian/process_package_file_worker_spec.rb +++ b/spec/workers/packages/debian/process_package_file_worker_spec.rb @@ -3,18 +3,19 @@ require 'spec_helper' RSpec.describe Packages::Debian::ProcessPackageFileWorker, type: :worker, feature_category: :package_registry do - let_it_be(:user) { create(:user) } - let_it_be_with_reload(:distribution) { create(:debian_project_distribution, :with_file, codename: 'unstable') } + let_it_be_with_reload(:distribution) { create(:debian_project_distribution, :with_file) } + let_it_be_with_reload(:package) do + create(:debian_package, :processing, project: distribution.project, published_in: nil) + end - let(:incoming) { create(:debian_incoming, project: distribution.project) } let(:distribution_name) { distribution.codename } + let(:debian_file_metadatum) { package_file.debian_file_metadatum } let(:worker) { described_class.new } describe '#perform' do let(:package_file_id) { package_file.id } - let(:user_id) { user.id } - subject { worker.perform(package_file_id, user_id, distribution_name, component_name) } + subject { worker.perform(package_file_id, distribution_name, component_name) } shared_examples 'returns early without error' do it 'returns early without error' do @@ -34,7 +35,7 @@ RSpec.describe Packages::Debian::ProcessPackageFileWorker, type: :worker, featur with_them do context 'with Debian package file' do - let(:package_file) { incoming.package_files.with_file_name(file_name).first } + let(:package_file) { package.package_files.with_file_name(file_name).first } context 'with mocked service' do it 'calls ProcessPackageFileService' do @@ -48,57 +49,44 @@ RSpec.describe Packages::Debian::ProcessPackageFileWorker, type: :worker, featur end end - context 'with non existing user' do - let(:user_id) { non_existing_record_id } - - it_behaves_like 'returns early without error' - end - - context 'with nil user id' do - let(:user_id) { nil } - - it_behaves_like 'returns early without error' - end - context 'when the service raises an error' do - let(:package_file) { incoming.package_files.with_file_name('sample_1.2.3~alpha2.tar.xz').first } + let(:package_file) { package.package_files.with_file_name('sample_1.2.3~alpha2.tar.xz').first } - it 'removes package file', :aggregate_failures do + it 'marks the package as errored', :aggregate_failures do expect(Gitlab::ErrorTracking).to receive(:log_exception).with( instance_of(ArgumentError), package_file_id: package_file_id, - user_id: user_id, distribution_name: distribution_name, component_name: component_name ) expect { subject } .to not_change(Packages::Package, :count) - .and change { Packages::PackageFile.count }.by(-1) - .and change { incoming.package_files.count }.from(7).to(6) - - expect { package_file.reload }.to raise_error(ActiveRecord::RecordNotFound) + .and not_change { Packages::PackageFile.count } + .and not_change { package.package_files.count } + .and change { package.reload.status }.from('processing').to('error') end end it_behaves_like 'an idempotent worker' do - let(:job_args) { [package_file.id, user.id, distribution_name, component_name] } + let(:job_args) { [package_file.id, distribution_name, component_name] } it 'sets the Debian file type as deb', :aggregate_failures do + expect(::Packages::Debian::GenerateDistributionWorker) + .to receive(:perform_async).with(:project, distribution.id) expect(Gitlab::ErrorTracking).not_to receive(:log_exception) # Using subject inside this block will process the job multiple times expect { subject } - .to change { Packages::Package.count }.from(1).to(2) + .to not_change(Packages::Package, :count) .and not_change(Packages::PackageFile, :count) - .and change { incoming.package_files.count }.from(7).to(6) - .and change { - package_file&.debian_file_metadatum&.reload&.file_type - }.from('unknown').to(expected_file_type) - - created_package = Packages::Package.last - expect(created_package.name).to eq 'sample' - expect(created_package.version).to eq '1.2.3~alpha2' - expect(created_package.creator).to eq user + .and change { Packages::Debian::Publication.count }.by(1) + .and not_change(package.package_files, :count) + .and change { package.reload.name }.to('sample') + .and change { package.version }.to('1.2.3~alpha2') + .and change { package.status }.from('processing').to('default') + .and change { package.debian_publication }.from(nil) + .and change { debian_file_metadatum.reload.file_type }.from('unknown').to(expected_file_type) + .and change { debian_file_metadatum.component }.from(nil).to(component_name) end end end @@ -113,15 +101,9 @@ RSpec.describe Packages::Debian::ProcessPackageFileWorker, type: :worker, featur end context 'with a deb' do - let(:package_file) { incoming.package_files.with_file_name('libsample0_1.2.3~alpha2_amd64.deb').first } + let(:package_file) { package.package_files.with_file_name('libsample0_1.2.3~alpha2_amd64.deb').first } let(:component_name) { 'main' } - context 'with FIPS mode enabled', :fips_mode do - it 'raises an error' do - expect { subject }.to raise_error(::Packages::FIPS::DisabledError) - end - end - context 'with non existing package file' do let(:package_file_id) { non_existing_record_id } diff --git a/spec/workers/pipeline_schedule_worker_spec.rb b/spec/workers/pipeline_schedule_worker_spec.rb index db58dc00338..da6a0254a17 100644 --- a/spec/workers/pipeline_schedule_worker_spec.rb +++ b/spec/workers/pipeline_schedule_worker_spec.rb @@ -49,19 +49,7 @@ RSpec.describe PipelineScheduleWorker, :sidekiq_inline, feature_category: :conti end end - shared_examples 'successful scheduling with/without ci_use_run_pipeline_schedule_worker' do - it_behaves_like 'successful scheduling' - - context 'when feature flag ci_use_run_pipeline_schedule_worker is disabled' do - before do - stub_feature_flags(ci_use_run_pipeline_schedule_worker: false) - end - - it_behaves_like 'successful scheduling' - end - end - - it_behaves_like 'successful scheduling with/without ci_use_run_pipeline_schedule_worker' + it_behaves_like 'successful scheduling' context 'when the latest commit contains [ci skip]' do before do @@ -70,7 +58,7 @@ RSpec.describe PipelineScheduleWorker, :sidekiq_inline, feature_category: :conti .and_return('some commit [ci skip]') end - it_behaves_like 'successful scheduling with/without ci_use_run_pipeline_schedule_worker' + it_behaves_like 'successful scheduling' end end diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb index 072c660bc2b..143809e8f2a 100644 --- a/spec/workers/process_commit_worker_spec.rb +++ b/spec/workers/process_commit_worker_spec.rb @@ -137,7 +137,7 @@ RSpec.describe ProcessCommitWorker do context 'when issue has no first_mentioned_in_commit_at set' do it 'updates issue metrics' do - expect(update_metrics_and_reload) + expect { update_metrics_and_reload.call } .to change { issue.metrics.first_mentioned_in_commit_at }.to(commit.committed_date) end end @@ -148,7 +148,7 @@ RSpec.describe ProcessCommitWorker do end it "doesn't update issue metrics" do - expect(update_metrics_and_reload).not_to change { issue.metrics.first_mentioned_in_commit_at } + expect { update_metrics_and_reload.call }.not_to change { issue.metrics.first_mentioned_in_commit_at } end end @@ -158,7 +158,7 @@ RSpec.describe ProcessCommitWorker do end it "doesn't update issue metrics" do - expect(update_metrics_and_reload) + expect { update_metrics_and_reload.call } .to change { issue.metrics.first_mentioned_in_commit_at }.to(commit.committed_date) end end diff --git a/spec/workers/projects/post_creation_worker_spec.rb b/spec/workers/projects/post_creation_worker_spec.rb index 732dc540fb7..b702eed9ea4 100644 --- a/spec/workers/projects/post_creation_worker_spec.rb +++ b/spec/workers/projects/post_creation_worker_spec.rb @@ -93,13 +93,10 @@ RSpec.describe Projects::PostCreationWorker do context 'when project is created', :aggregate_failures do it 'creates tags for the project' do - expect { subject }.to change { IncidentManagement::TimelineEventTag.count }.by(2) + expect { subject }.to change { IncidentManagement::TimelineEventTag.count }.by(6) expect(project.incident_management_timeline_event_tags.pluck_names).to match_array( - [ - ::IncidentManagement::TimelineEventTag::START_TIME_TAG_NAME, - ::IncidentManagement::TimelineEventTag::END_TIME_TAG_NAME - ] + ::IncidentManagement::TimelineEventTag::PREDEFINED_TAGS ) end diff --git a/spec/workers/projects/refresh_build_artifacts_size_statistics_worker_spec.rb b/spec/workers/projects/refresh_build_artifacts_size_statistics_worker_spec.rb index 00c45255316..99627ff1ad2 100644 --- a/spec/workers/projects/refresh_build_artifacts_size_statistics_worker_spec.rb +++ b/spec/workers/projects/refresh_build_artifacts_size_statistics_worker_spec.rb @@ -17,12 +17,14 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsWorker do build( :project_build_artifacts_size_refresh, :running, + id: 99, project_id: 77, last_job_artifact_id: 123 ) end it 'logs refresh information' do + expect(worker).to receive(:log_extra_metadata_on_done).with(:refresh_id, refresh.id) expect(worker).to receive(:log_extra_metadata_on_done).with(:project_id, refresh.project_id) expect(worker).to receive(:log_extra_metadata_on_done).with(:last_job_artifact_id, refresh.last_job_artifact_id) expect(worker).to receive(:log_extra_metadata_on_done).with(:last_batch, refresh.destroyed?) diff --git a/spec/workers/run_pipeline_schedule_worker_spec.rb b/spec/workers/run_pipeline_schedule_worker_spec.rb index 25158de3341..75938d3b793 100644 --- a/spec/workers/run_pipeline_schedule_worker_spec.rb +++ b/spec/workers/run_pipeline_schedule_worker_spec.rb @@ -21,11 +21,6 @@ RSpec.describe RunPipelineScheduleWorker, feature_category: :continuous_integrat end end - it 'accepts an option' do - expect { worker.perform(pipeline_schedule.id, user.id, {}) }.not_to raise_error - expect { worker.perform(pipeline_schedule.id, user.id, {}, {}) }.to raise_error(ArgumentError) - end - context 'when a schedule not found' do it 'does not call the Service' do expect(Ci::CreatePipelineService).not_to receive(:new) @@ -60,6 +55,7 @@ RSpec.describe RunPipelineScheduleWorker, feature_category: :continuous_integrat describe "#run_pipeline_schedule" do let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService, execute: service_response) } let(:service_response) { instance_double(ServiceResponse, payload: pipeline, error?: false) } + let(:pipeline) { instance_double(Ci::Pipeline, persisted?: true) } context 'when pipeline can be created' do before do @@ -69,8 +65,6 @@ RSpec.describe RunPipelineScheduleWorker, feature_category: :continuous_integrat end context "when pipeline is persisted" do - let(:pipeline) { instance_double(Ci::Pipeline, persisted?: true) } - it "returns the service response" do expect(worker.perform(pipeline_schedule.id, user.id)).to eq(service_response) end @@ -81,17 +75,23 @@ RSpec.describe RunPipelineScheduleWorker, feature_category: :continuous_integrat expect(worker.perform(pipeline_schedule.id, user.id)).to eq(service_response) end - it "changes the next_run_at" do - expect { worker.perform(pipeline_schedule.id, user.id) }.to change { pipeline_schedule.reload.next_run_at }.by(1.day) + it "does not change the next_run_at" do + expect { worker.perform(pipeline_schedule.id, user.id) }.not_to change { pipeline_schedule.reload.next_run_at } end - context 'when feature flag ci_use_run_pipeline_schedule_worker is disabled' do - before do - stub_feature_flags(ci_use_run_pipeline_schedule_worker: false) + context 'when scheduling option is given as true' do + it "returns the service response" do + expect(worker.perform(pipeline_schedule.id, user.id, scheduling: true)).to eq(service_response) + end + + it "does not log errors" do + expect(worker).not_to receive(:log_extra_metadata_on_done) + + expect(worker.perform(pipeline_schedule.id, user.id, scheduling: true)).to eq(service_response) end - it 'does not change the next_run_at' do - expect { worker.perform(pipeline_schedule.id, user.id) }.not_to change { pipeline_schedule.reload.next_run_at } + it "changes the next_run_at" do + expect { worker.perform(pipeline_schedule.id, user.id, scheduling: true) }.to change { pipeline_schedule.reload.next_run_at }.by(1.day) end end end @@ -122,31 +122,12 @@ RSpec.describe RunPipelineScheduleWorker, feature_category: :continuous_integrat expect { worker.perform(pipeline_schedule.id, user.id) }.to not_change { pipeline_schedule.reload.next_run_at } end - it 'does not create a pipeline' do - expect(Ci::CreatePipelineService).not_to receive(:new) + it 'creates a pipeline' do + expect(Ci::CreatePipelineService).to receive(:new).with(project, user, ref: pipeline_schedule.ref).and_return(create_pipeline_service) + expect(create_pipeline_service).to receive(:execute).with(:schedule, ignore_skip_ci: true, save_on_errors: false, schedule: pipeline_schedule).and_return(service_response) worker.perform(pipeline_schedule.id, user.id) end - - context 'when feature flag ci_use_run_pipeline_schedule_worker is disabled' do - let(:pipeline) { instance_double(Ci::Pipeline, persisted?: true) } - - before do - stub_feature_flags(ci_use_run_pipeline_schedule_worker: false) - - expect(Ci::CreatePipelineService).to receive(:new).with(project, user, ref: pipeline_schedule.ref).and_return(create_pipeline_service) - - expect(create_pipeline_service).to receive(:execute).with(:schedule, ignore_skip_ci: true, save_on_errors: false, schedule: pipeline_schedule).and_return(service_response) - end - - it 'does not change the next_run_at' do - expect { worker.perform(pipeline_schedule.id, user.id) }.to not_change { pipeline_schedule.reload.next_run_at } - end - - it "returns the service response" do - expect(worker.perform(pipeline_schedule.id, user.id)).to eq(service_response) - end - end end end diff --git a/spec/workers/tasks_to_be_done/create_worker_spec.rb b/spec/workers/tasks_to_be_done/create_worker_spec.rb index e884a71933e..c3c3612f9a7 100644 --- a/spec/workers/tasks_to_be_done/create_worker_spec.rb +++ b/spec/workers/tasks_to_be_done/create_worker_spec.rb @@ -20,7 +20,7 @@ RSpec.describe TasksToBeDone::CreateWorker do expect(service_class) .to receive(:new) - .with(project: member_task.project, current_user: current_user, assignee_ids: assignee_ids) + .with(container: member_task.project, current_user: current_user, assignee_ids: assignee_ids) .and_call_original end |