diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-19 09:08:42 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-19 09:08:42 +0000 |
commit | b76ae638462ab0f673e5915986070518dd3f9ad3 (patch) | |
tree | bdab0533383b52873be0ec0eb4d3c66598ff8b91 /spec/workers | |
parent | 434373eabe7b4be9593d18a585fb763f1e5f1a6f (diff) | |
download | gitlab-ce-b76ae638462ab0f673e5915986070518dd3f9ad3.tar.gz |
Add latest changes from gitlab-org/gitlab@14-2-stable-eev14.2.0-rc42
Diffstat (limited to 'spec/workers')
27 files changed, 481 insertions, 262 deletions
diff --git a/spec/workers/analytics/usage_trends/counter_job_worker_spec.rb b/spec/workers/analytics/usage_trends/counter_job_worker_spec.rb index 9e4c82ee981..dd180229d12 100644 --- a/spec/workers/analytics/usage_trends/counter_job_worker_spec.rb +++ b/spec/workers/analytics/usage_trends/counter_job_worker_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Analytics::UsageTrends::CounterJobWorker do let(:job_args) { [users_measurement_identifier, user_1.id, user_2.id, recorded_at] } before do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false) + allow(::Analytics::UsageTrends::Measurement.connection).to receive(:transaction_open?).and_return(false) end include_examples 'an idempotent worker' do diff --git a/spec/workers/authorized_project_update/project_recalculate_worker_spec.rb b/spec/workers/authorized_project_update/project_recalculate_worker_spec.rb index 403793a15e2..a9a15565580 100644 --- a/spec/workers/authorized_project_update/project_recalculate_worker_spec.rb +++ b/spec/workers/authorized_project_update/project_recalculate_worker_spec.rb @@ -43,7 +43,7 @@ RSpec.describe AuthorizedProjectUpdate::ProjectRecalculateWorker do end context 'exclusive lease' do - let(:lock_key) { "#{described_class.name.underscore}/#{project.root_namespace.id}" } + let(:lock_key) { "#{described_class.name.underscore}/projects/#{project.id}" } let(:timeout) { 10.seconds } context 'when exclusive lease has not been taken' do diff --git a/spec/workers/authorized_project_update/user_refresh_from_replica_worker_spec.rb b/spec/workers/authorized_project_update/user_refresh_from_replica_worker_spec.rb index c27629c3a15..027ce3b7f89 100644 --- a/spec/workers/authorized_project_update/user_refresh_from_replica_worker_spec.rb +++ b/spec/workers/authorized_project_update/user_refresh_from_replica_worker_spec.rb @@ -44,11 +44,7 @@ RSpec.describe AuthorizedProjectUpdate::UserRefreshFromReplicaWorker do end end - context 'with load balancing enabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - end - + context 'with load balancing enabled', :db_load_balancing do it 'reads from the replica database' do expect(Gitlab::Database::LoadBalancing::Session.current).to receive(:use_replicas_for_read_queries).and_call_original diff --git a/spec/workers/build_finished_worker_spec.rb b/spec/workers/build_finished_worker_spec.rb index 6b7162ee886..4e34d2348d6 100644 --- a/spec/workers/build_finished_worker_spec.rb +++ b/spec/workers/build_finished_worker_spec.rb @@ -15,7 +15,6 @@ RSpec.describe BuildFinishedWorker do end it 'calculates coverage and calls hooks', :aggregate_failures do - expect(build).to receive(:parse_trace_sections!).ordered expect(build).to receive(:update_coverage).ordered expect_next_instance_of(Ci::BuildReportResultService) do |build_report_result_service| diff --git a/spec/workers/ci/build_finished_worker_spec.rb b/spec/workers/ci/build_finished_worker_spec.rb index 374ecd8619f..9096b0d2ba9 100644 --- a/spec/workers/ci/build_finished_worker_spec.rb +++ b/spec/workers/ci/build_finished_worker_spec.rb @@ -15,7 +15,6 @@ RSpec.describe Ci::BuildFinishedWorker do end it 'calculates coverage and calls hooks', :aggregate_failures do - expect(build).to receive(:parse_trace_sections!).ordered expect(build).to receive(:update_coverage).ordered expect_next_instance_of(Ci::BuildReportResultService) do |build_report_result_service| diff --git a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb index 4c96daea7b3..c1ac5ffebe8 100644 --- a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb +++ b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb @@ -21,6 +21,12 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do end.new end + let_it_be(:project) { create(:project, :import_started) } + + let(:importer_class) { double(:importer_class, name: 'klass_name') } + let(:importer_instance) { double(:importer_instance) } + let(:client) { double(:client) } + before do stub_const('MockRepresantation', Class.new do include Gitlab::GithubImport::Representation::ToHash @@ -38,12 +44,7 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do end) end - describe '#import', :clean_gitlab_redis_shared_state do - let(:importer_class) { double(:importer_class, name: 'klass_name') } - let(:importer_instance) { double(:importer_instance) } - let(:project) { double(:project, full_path: 'foo/bar', id: 1) } - let(:client) { double(:client) } - + describe '#import', :clean_gitlab_redis_cache do before do expect(worker) .to receive(:importer_class) @@ -60,26 +61,23 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do expect(importer_instance) .to receive(:execute) - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) - .to receive(:info) - .with( - github_id: 1, - message: 'starting importer', - import_source: :github, - project_id: 1, - importer: 'klass_name' - ) - expect(logger) - .to receive(:info) - .with( - github_id: 1, - message: 'importer finished', - import_source: :github, - project_id: 1, - importer: 'klass_name' - ) - end + expect(Gitlab::GithubImport::Logger) + .to receive(:info) + .with( + github_id: 1, + message: 'starting importer', + project_id: project.id, + importer: 'klass_name' + ) + + expect(Gitlab::GithubImport::Logger) + .to receive(:info) + .with( + github_id: 1, + message: 'importer finished', + project_id: project.id, + importer: 'klass_name' + ) worker.import(project, client, { 'number' => 10, 'github_id' => 1 }) @@ -100,74 +98,45 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do .to receive(:execute) .and_raise(exception) - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) - .to receive(:info) - .with( - github_id: 1, - message: 'starting importer', - import_source: :github, - project_id: project.id, - importer: 'klass_name' - ) - expect(logger) - .to receive(:error) - .with( - github_id: 1, - message: 'importer failed', - import_source: :github, - project_id: project.id, - importer: 'klass_name', - 'error.message': 'some error', - 'github.data': { - 'github_id' => 1, - 'number' => 10 - } - ) - end - - expect(Gitlab::ErrorTracking) - .to receive(:track_and_raise_exception) + expect(Gitlab::GithubImport::Logger) + .to receive(:info) .with( - exception, - import_source: :github, github_id: 1, - project_id: 1, + message: 'starting importer', + project_id: project.id, importer: 'klass_name' - ).and_call_original + ) + + expect(Gitlab::Import::ImportFailureService) + .to receive(:track) + .with( + project_id: project.id, + exception: exception, + error_source: 'klass_name' + ) + .and_call_original + + worker.import(project, client, { 'number' => 10, 'github_id' => 1 }) + + expect(project.import_state.reload.status).to eq('started') - expect { worker.import(project, client, { 'number' => 10, 'github_id' => 1 }) } - .to raise_error(exception) + expect(project.import_failures).not_to be_empty + expect(project.import_failures.last.exception_class).to eq('StandardError') + expect(project.import_failures.last.exception_message).to eq('some error') end it 'logs error when representation does not have a github_id' do expect(importer_class).not_to receive(:new) - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) - .to receive(:error) - .with( - github_id: nil, - message: 'importer failed', - import_source: :github, - project_id: project.id, - importer: 'klass_name', - 'error.message': 'key not found: :github_id', - 'github.data': { - 'number' => 10 - } - ) - end - - expect(Gitlab::ErrorTracking) - .to receive(:track_and_raise_exception) + expect(Gitlab::Import::ImportFailureService) + .to receive(:track) .with( - an_instance_of(KeyError), - import_source: :github, - github_id: nil, - project_id: 1, - importer: 'klass_name' - ).and_call_original + project_id: project.id, + exception: a_kind_of(KeyError), + error_source: 'klass_name', + fail_import: true + ) + .and_call_original expect { worker.import(project, client, { 'number' => 10 }) } .to raise_error(KeyError, 'key not found: :github_id') diff --git a/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb b/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb index 651ea77a71c..aeb86f5aa8c 100644 --- a/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb +++ b/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::StageMethods do - let(:project) { create(:project) } + let_it_be(:project) { create(:project, :import_started, import_url: 'https://t0ken@github.com/repo/repo.git') } + let(:worker) do Class.new do def self.name @@ -15,8 +16,6 @@ RSpec.describe Gitlab::GithubImport::StageMethods do end describe '#perform' do - let(:project) { create(:project, import_url: 'https://t0ken@github.com/repo/repo.git') } - it 'returns if no project could be found' do expect(worker).not_to receive(:try_import) @@ -36,71 +35,119 @@ RSpec.describe Gitlab::GithubImport::StageMethods do an_instance_of(Project) ) - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) + expect(Gitlab::GithubImport::Logger) + .to receive(:info) + .with( + message: 'starting stage', + project_id: project.id, + import_stage: 'DummyStage' + ) + + expect(Gitlab::GithubImport::Logger) + .to receive(:info) + .with( + message: 'stage finished', + project_id: project.id, + import_stage: 'DummyStage' + ) + + worker.perform(project.id) + end + + context 'when abort_on_failure is false' do + it 'logs error when import fails' do + exception = StandardError.new('some error') + + allow(worker) + .to receive(:find_project) + .with(project.id) + .and_return(project) + + expect(worker) + .to receive(:try_import) + .and_raise(exception) + + expect(Gitlab::GithubImport::Logger) .to receive(:info) .with( message: 'starting stage', - import_source: :github, project_id: project.id, import_stage: 'DummyStage' ) - expect(logger) - .to receive(:info) + + expect(Gitlab::Import::ImportFailureService) + .to receive(:track) .with( - message: 'stage finished', - import_source: :github, project_id: project.id, - import_stage: 'DummyStage' - ) - end + exception: exception, + error_source: 'DummyStage', + fail_import: false + ).and_call_original - worker.perform(project.id) + expect { worker.perform(project.id) } + .to raise_error(exception) + + expect(project.import_state.reload.status).to eq('started') + + expect(project.import_failures).not_to be_empty + expect(project.import_failures.last.exception_class).to eq('StandardError') + expect(project.import_failures.last.exception_message).to eq('some error') + end end - it 'logs error when import fails' do - exception = StandardError.new('some error') + context 'when abort_on_failure is true' do + let(:worker) do + Class.new do + def self.name + 'DummyStage' + end - allow(worker) - .to receive(:find_project) - .with(project.id) - .and_return(project) + def abort_on_failure + true + end - expect(worker) - .to receive(:try_import) - .and_raise(exception) + include(Gitlab::GithubImport::StageMethods) + end.new + end + + it 'logs, captures and re-raises the exception and also marks the import as failed' do + exception = StandardError.new('some error') + + allow(worker) + .to receive(:find_project) + .with(project.id) + .and_return(project) - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) + expect(worker) + .to receive(:try_import) + .and_raise(exception) + + expect(Gitlab::GithubImport::Logger) .to receive(:info) .with( message: 'starting stage', - import_source: :github, project_id: project.id, import_stage: 'DummyStage' ) - expect(logger) - .to receive(:error) + + expect(Gitlab::Import::ImportFailureService) + .to receive(:track) .with( - message: 'stage failed', - import_source: :github, project_id: project.id, - import_stage: 'DummyStage', - 'error.message': 'some error' - ) - end + exception: exception, + error_source: 'DummyStage', + fail_import: true + ).and_call_original - expect(Gitlab::ErrorTracking) - .to receive(:track_and_raise_exception) - .with( - exception, - import_source: :github, - project_id: project.id, - import_stage: 'DummyStage' - ) - .and_call_original + expect { worker.perform(project.id) }.to raise_error(exception) + + expect(project.import_state.reload.status).to eq('failed') + expect(project.import_state.last_error).to eq('some error') - expect { worker.perform(project.id) }.to raise_error(exception) + expect(project.import_failures).not_to be_empty + expect(project.import_failures.last.exception_class).to eq('StandardError') + expect(project.import_failures.last.exception_message).to eq('some error') + end end end @@ -132,16 +179,14 @@ RSpec.describe Gitlab::GithubImport::StageMethods do end describe '#find_project' do - let(:import_state) { create(:import_state, project: project) } - it 'returns a Project for an existing ID' do - import_state.update_column(:status, 'started') + project.import_state.update_column(:status, 'started') expect(worker.find_project(project.id)).to eq(project) end it 'returns nil for a project that failed importing' do - import_state.update_column(:status, 'failed') + project.import_state.update_column(:status, 'failed') expect(worker.find_project(project.id)).to be_nil end diff --git a/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb b/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb index 506124216af..fdba67638c1 100644 --- a/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb +++ b/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb @@ -25,6 +25,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do expect(ContainerExpirationPolicies::CleanupService) .to receive(:new).with(repository).and_return(double(execute: service_response)) expect_log_extra_metadata(service_response: service_response) + expect_log_info(project_id: project.id, container_repository_id: repository.id) subject end @@ -35,6 +36,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do expect(ContainerExpirationPolicies::CleanupService) .to receive(:new).with(repository).and_return(double(execute: service_response)) expect_log_extra_metadata(service_response: service_response, cleanup_status: :unfinished) + expect_log_info(project_id: project.id, container_repository_id: repository.id) subject end @@ -45,6 +47,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do expect(ContainerExpirationPolicies::CleanupService) .to receive(:new).with(repository).and_return(double(execute: service_response)) expect_log_extra_metadata(service_response: service_response, cleanup_status: :unfinished, truncated: true) + expect_log_info(project_id: project.id, container_repository_id: repository.id) subject end @@ -65,6 +68,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do expect(ContainerExpirationPolicies::CleanupService) .to receive(:new).with(repository).and_return(double(execute: service_response)) expect_log_extra_metadata(service_response: service_response, cleanup_status: :unfinished, truncated: truncated) + expect_log_info(project_id: project.id, container_repository_id: repository.id) subject end @@ -78,6 +82,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do expect(ContainerExpirationPolicies::CleanupService) .to receive(:new).with(repository).and_return(double(execute: service_response)) expect_log_extra_metadata(service_response: service_response, cleanup_status: :error) + expect_log_info(project_id: project.id, container_repository_id: repository.id) subject end @@ -361,6 +366,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do expect(ContainerExpirationPolicies::CleanupService) .to receive(:new).with(repository).and_return(double(execute: service_response)) expect_log_extra_metadata(service_response: service_response) + expect_log_info(project_id: project.id, container_repository_id: repository.id) subject end @@ -396,6 +402,11 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_error_message, service_response.message) end end + + def expect_log_info(structure) + expect(worker.logger) + .to receive(:info).with(worker.structured_payload(structure)) + end end describe '#remaining_work_count' do @@ -446,6 +457,12 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do end it { is_expected.to eq(0) } + + it 'does not log a selected container' do + expect(worker).not_to receive(:log_info) + + subject + end end end diff --git a/spec/workers/container_expiration_policy_worker_spec.rb b/spec/workers/container_expiration_policy_worker_spec.rb index 69ddbe5c0f4..9f370b10f6a 100644 --- a/spec/workers/container_expiration_policy_worker_spec.rb +++ b/spec/workers/container_expiration_policy_worker_spec.rb @@ -156,11 +156,7 @@ RSpec.describe ContainerExpirationPolicyWorker do subject end - context 'with load balancing enabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - end - + context 'with load balancing enabled', :db_load_balancing do it 'reads the counts from the replica' do expect(Gitlab::Database::LoadBalancing::Session.current).to receive(:use_replicas_for_read_queries).and_call_original diff --git a/spec/workers/database/drop_detached_partitions_worker_spec.rb b/spec/workers/database/drop_detached_partitions_worker_spec.rb new file mode 100644 index 00000000000..42c3fa3c188 --- /dev/null +++ b/spec/workers/database/drop_detached_partitions_worker_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Database::DropDetachedPartitionsWorker do + describe '#perform' do + subject { described_class.new.perform } + + let(:dropper) { instance_double('DropDetachedPartitions', perform: nil) } + let(:monitoring) { instance_double('PartitionMonitoring', report_metrics: nil) } + + before do + allow(Gitlab::Database::Partitioning::DetachedPartitionDropper).to receive(:new).and_return(dropper) + allow(Gitlab::Database::Partitioning::PartitionMonitoring).to receive(:new).and_return(monitoring) + end + + it 'delegates to DropPartitionsPendingDrop' do + expect(dropper).to receive(:perform) + + subject + end + + it 'reports partition metrics' do + expect(monitoring).to receive(:report_metrics) + + subject + end + end +end diff --git a/spec/workers/deployments/hooks_worker_spec.rb b/spec/workers/deployments/hooks_worker_spec.rb index f1fe7b0fc5d..5d8edf85dd9 100644 --- a/spec/workers/deployments/hooks_worker_spec.rb +++ b/spec/workers/deployments/hooks_worker_spec.rb @@ -49,5 +49,10 @@ RSpec.describe Deployments::HooksWorker do worker.perform(deployment_id: deployment.id, status_changed_at: status_changed_at) end + + it_behaves_like 'worker with data consistency', + described_class, + feature_flag: :load_balancing_for_deployments_hooks_worker, + data_consistency: :delayed end end diff --git a/spec/workers/environments/auto_delete_cron_worker_spec.rb b/spec/workers/environments/auto_delete_cron_worker_spec.rb new file mode 100644 index 00000000000..b18f3da5d10 --- /dev/null +++ b/spec/workers/environments/auto_delete_cron_worker_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Environments::AutoDeleteCronWorker do + include CreateEnvironmentsHelpers + + let(:worker) { described_class.new } + + describe '#perform' do + subject { worker.perform } + + let_it_be(:project) { create(:project, :repository) } + + let!(:environment) { create(:environment, :auto_deletable, project: project) } + + it 'deletes the environment' do + expect { subject }.to change { Environment.count }.by(-1) + end + + context 'when environment is not stopped' do + let!(:environment) { create(:environment, :available, auto_delete_at: 1.day.ago, project: project) } + + it 'does not delete the environment' do + expect { subject }.not_to change { Environment.count } + end + end + + context 'when auto_delete_at is null' do + let!(:environment) { create(:environment, :stopped, auto_delete_at: nil, project: project) } + + it 'does not delete the environment' do + expect { subject }.not_to change { Environment.count } + end + end + + context 'with multiple deletable environments' do + let!(:other_environment) { create(:environment, :auto_deletable, project: project) } + + it 'deletes all deletable environments' do + expect { subject }.to change { Environment.count }.by(-2) + end + + context 'when loop reached loop limit' do + before do + stub_const("#{described_class}::LOOP_LIMIT", 1) + stub_const("#{described_class}::BATCH_SIZE", 1) + end + + it 'deletes only one deletable environment' do + expect { subject }.to change { Environment.count }.by(-1) + end + end + + context 'when batch size is less than the number of environments' do + before do + stub_const("#{described_class}::BATCH_SIZE", 1) + end + + it 'deletes all deletable environments' do + expect { subject }.to change { Environment.count }.by(-2) + end + end + end + + context 'with multiple deployments' do + it 'deletes the deployment records and refs' do + deployment_1 = create(:deployment, environment: environment, project: project) + deployment_2 = create(:deployment, environment: environment, project: project) + deployment_1.create_ref + deployment_2.create_ref + + expect(project.repository.commit(deployment_1.ref_path)).to be_present + expect(project.repository.commit(deployment_2.ref_path)).to be_present + + expect { subject }.to change { Deployment.count }.by(-2) + + expect(project.repository.commit(deployment_1.ref_path)).not_to be_present + expect(project.repository.commit(deployment_2.ref_path)).not_to be_present + end + end + + context 'when loop reached timeout' do + before do + stub_const("#{described_class}::LOOP_TIMEOUT", 0.seconds) + stub_const("#{described_class}::LOOP_LIMIT", 100_000) + allow_next_instance_of(described_class) do |worker| + allow(worker).to receive(:destroy_in_batch) { true } + end + end + + it 'does not delete the environment' do + expect { subject }.not_to change { Environment.count } + end + end + + context 'with idempotent flag' do + include_examples 'an idempotent worker' do + it 'deletes the environment' do + expect { subject }.to change { Environment.count }.by(-1) + end + end + end + end +end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index c75b9b43ef4..ea1f0153f83 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -8,17 +8,17 @@ RSpec.describe 'Every Sidekiq worker' do end it 'does not use the default queue' do - expect(workers_without_defaults.map(&:queue)).not_to include('default') + expect(workers_without_defaults.map(&:generated_queue_name)).not_to include('default') end it 'uses the cronjob queue when the worker runs as a cronjob' do - expect(Gitlab::SidekiqConfig.cron_workers.map(&:queue)).to all(start_with('cronjob:')) + expect(Gitlab::SidekiqConfig.cron_workers.map(&:generated_queue_name)).to all(start_with('cronjob:')) end it 'has its queue in Gitlab::SidekiqConfig::QUEUE_CONFIG_PATHS', :aggregate_failures do file_worker_queues = Gitlab::SidekiqConfig.worker_queues.to_set - worker_queues = Gitlab::SidekiqConfig.workers.map(&:queue).to_set + worker_queues = Gitlab::SidekiqConfig.workers.map(&:generated_queue_name).to_set worker_queues << ActionMailer::MailDeliveryJob.new.queue_name worker_queues << 'default' @@ -33,7 +33,7 @@ RSpec.describe 'Every Sidekiq worker' do config_queues = Gitlab::SidekiqConfig.config_queues.to_set Gitlab::SidekiqConfig.workers.each do |worker| - queue = worker.queue + queue = worker.generated_queue_name queue_namespace = queue.split(':').first expect(config_queues).to include(queue).or(include(queue_namespace)) @@ -430,7 +430,6 @@ RSpec.describe 'Every Sidekiq worker' do 'StageUpdateWorker' => 3, 'StatusPage::PublishWorker' => 5, 'StoreSecurityReportsWorker' => 3, - 'StoreSecurityScansWorker' => 3, 'SyncSeatLinkRequestWorker' => 20, 'SyncSeatLinkWorker' => 12, 'SystemHookPushWorker' => 3, diff --git a/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb b/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb index 8dea24dc74f..132fe1dc618 100644 --- a/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb @@ -26,21 +26,18 @@ RSpec.describe Gitlab::GithubImport::Stage::FinishImportWorker do .to receive(:increment) .and_call_original - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) - .to receive(:info) - .with( - message: 'GitHub project import finished', - import_stage: 'Gitlab::GithubImport::Stage::FinishImportWorker', - import_source: :github, - object_counts: { - 'fetched' => {}, - 'imported' => {} - }, - project_id: project.id, - duration_s: a_kind_of(Numeric) - ) - end + expect(Gitlab::GithubImport::Logger) + .to receive(:info) + .with( + message: 'GitHub project import finished', + import_stage: 'Gitlab::GithubImport::Stage::FinishImportWorker', + object_counts: { + 'fetched' => {}, + 'imported' => {} + }, + project_id: project.id, + duration_s: a_kind_of(Numeric) + ) worker.report_import_time(project) end diff --git a/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb index bc51a44e057..875fc082975 100644 --- a/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do let(:project) { double(:project, id: 4) } + let(:worker) { described_class.new } describe '#import' do @@ -36,15 +37,19 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do context 'when the import fails' do it 'does not schedule the importing of the base data' do client = double(:client) + exception_class = Gitlab::Git::Repository::NoRepository expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance| - expect(instance).to receive(:execute).and_return(false) + expect(instance).to receive(:execute).and_raise(exception_class) end expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker) .not_to receive(:perform_async) - worker.import(client, project) + expect(worker.abort_on_failure).to eq(true) + + expect { worker.import(client, project) } + .to raise_error(exception_class) end end end diff --git a/spec/workers/gitlab/import/stuck_import_job_spec.rb b/spec/workers/gitlab/import/stuck_import_job_spec.rb new file mode 100644 index 00000000000..3a1463e98a0 --- /dev/null +++ b/spec/workers/gitlab/import/stuck_import_job_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Import::StuckImportJob do + let_it_be(:project) { create(:project, :import_started, import_source: 'foo/bar') } + + let(:worker) do + Class.new do + def self.name + 'MyStuckProjectImportsWorker' + end + + include(Gitlab::Import::StuckImportJob) + + def track_metrics(...) + nil + end + + def enqueued_import_states + ProjectImportState.with_status([:scheduled, :started]) + end + end.new + end + + it 'marks the stuck import project as failed and track the error on import_failures' do + worker.perform + + expect(project.import_state.reload.status).to eq('failed') + expect(project.import_state.last_error).to eq('Import timed out. Import took longer than 86400 seconds') + + expect(project.import_failures).not_to be_empty + expect(project.import_failures.last.exception_class).to eq('Gitlab::Import::StuckImportJob::StuckImportJobError') + expect(project.import_failures.last.exception_message).to eq('Import timed out. Import took longer than 86400 seconds') + end +end diff --git a/spec/workers/jira_connect/forward_event_worker_spec.rb b/spec/workers/jira_connect/forward_event_worker_spec.rb index adfc071779a..7de9952a1da 100644 --- a/spec/workers/jira_connect/forward_event_worker_spec.rb +++ b/spec/workers/jira_connect/forward_event_worker_spec.rb @@ -15,23 +15,23 @@ RSpec.describe JiraConnect::ForwardEventWorker do let(:client_key) { '123' } let(:shared_secret) { '123' } - subject { described_class.new.perform(jira_connect_installation.id, base_path, event_path) } + subject(:perform) { described_class.new.perform(jira_connect_installation.id, base_path, event_path) } - it 'forwards the event including the auth header and deletes the installation' do + it 'forwards the event and deletes the installation' do stub_request(:post, event_url) expect(Atlassian::Jwt).to receive(:create_query_string_hash).with(event_url, 'POST', base_url).and_return('some_qsh') expect(Atlassian::Jwt).to receive(:encode).with({ iss: client_key, qsh: 'some_qsh' }, shared_secret).and_return('auth_token') - expect { subject }.to change(JiraConnectInstallation, :count).by(-1) + expect(JiraConnect::RetryRequestWorker).to receive(:perform_async).with(event_url, 'auth_token') - expect(WebMock).to have_requested(:post, event_url).with(headers: { 'Authorization' => 'JWT auth_token' }) + expect { perform }.to change(JiraConnectInstallation, :count).by(-1) end context 'when installation does not exist' do let(:jira_connect_installation) { instance_double(JiraConnectInstallation, id: -1) } it 'does nothing' do - expect { subject }.not_to change(JiraConnectInstallation, :count) + expect { perform }.not_to change(JiraConnectInstallation, :count) end end @@ -39,17 +39,9 @@ RSpec.describe JiraConnect::ForwardEventWorker do let!(:jira_connect_installation) { create(:jira_connect_installation) } it 'forwards the event including the auth header' do - expect { subject }.to change(JiraConnectInstallation, :count).by(-1) + expect { perform }.to change(JiraConnectInstallation, :count).by(-1) - expect(WebMock).not_to have_requested(:post, '*') - end - end - - context 'when it fails to forward the event' do - it 'still deletes the installation' do - allow(Gitlab::HTTP).to receive(:post).and_raise(StandardError) - - expect { subject }.to raise_error(StandardError).and change(JiraConnectInstallation, :count).by(-1) + expect(JiraConnect::RetryRequestWorker).not_to receive(:perform_async) end end end diff --git a/spec/workers/jira_connect/retry_request_worker_spec.rb b/spec/workers/jira_connect/retry_request_worker_spec.rb new file mode 100644 index 00000000000..7a93e5fe41d --- /dev/null +++ b/spec/workers/jira_connect/retry_request_worker_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::RetryRequestWorker do + describe '#perform' do + let(:jwt) { 'some-jwt' } + let(:event_url) { 'https://example.com/somewhere' } + let(:attempts) { 3 } + + subject(:perform) { described_class.new.perform(event_url, jwt, attempts) } + + it 'sends the request, with the appropriate headers' do + expect(JiraConnect::RetryRequestWorker).not_to receive(:perform_in) + + stub_request(:post, event_url) + + perform + + expect(WebMock).to have_requested(:post, event_url).with(headers: { 'Authorization' => 'JWT some-jwt' }) + end + + context 'when the proxied request fails' do + before do + stub_request(:post, event_url).to_return(status: 500, body: '', headers: {}) + end + + it 'arranges to retry the request' do + expect(JiraConnect::RetryRequestWorker).to receive(:perform_in).with(1.hour, event_url, jwt, attempts - 1) + + perform + end + + context 'when there are no more attempts left' do + let(:attempts) { 0 } + + it 'does not retry' do + expect(JiraConnect::RetryRequestWorker).not_to receive(:perform_in) + + perform + end + end + end + end +end diff --git a/spec/workers/merge_request_mergeability_check_worker_spec.rb b/spec/workers/merge_request_mergeability_check_worker_spec.rb index 0349de5cbb3..32debcf9651 100644 --- a/spec/workers/merge_request_mergeability_check_worker_spec.rb +++ b/spec/workers/merge_request_mergeability_check_worker_spec.rb @@ -10,6 +10,12 @@ RSpec.describe MergeRequestMergeabilityCheckWorker do it 'does not execute MergeabilityCheckService' do expect(MergeRequests::MergeabilityCheckService).not_to receive(:new) + expect(Sidekiq.logger).to receive(:error).once + .with( + merge_request_id: 1, + worker: "MergeRequestMergeabilityCheckWorker", + message: 'Failed to find merge request') + subject.perform(1) end end @@ -24,6 +30,20 @@ RSpec.describe MergeRequestMergeabilityCheckWorker do subject.perform(merge_request.id) end + + it 'structurally logs a failed mergeability check' do + expect_next_instance_of(MergeRequests::MergeabilityCheckService, merge_request) do |service| + expect(service).to receive(:execute).and_return(double(error?: true, message: "solar flares")) + end + + expect(Sidekiq.logger).to receive(:error).once + .with( + merge_request_id: merge_request.id, + worker: "MergeRequestMergeabilityCheckWorker", + message: 'Failed to check mergeability of merge request: solar flares') + + subject.perform(merge_request.id) + end end it_behaves_like 'an idempotent worker' do diff --git a/spec/workers/packages/debian/generate_distribution_worker_spec.rb b/spec/workers/packages/debian/generate_distribution_worker_spec.rb index a8751ccceae..a4627ec5d36 100644 --- a/spec/workers/packages/debian/generate_distribution_worker_spec.rb +++ b/spec/workers/packages/debian/generate_distribution_worker_spec.rb @@ -9,6 +9,9 @@ RSpec.describe Packages::Debian::GenerateDistributionWorker, type: :worker do subject { described_class.new.perform(container_type, distribution_id) } + let(:subject2) { described_class.new.perform(container_type, distribution_id) } + let(:subject3) { described_class.new.perform(container_type, distribution_id) } + include_context 'with published Debian package' [:project, :group].each do |container_type| diff --git a/spec/workers/pipeline_notification_worker_spec.rb b/spec/workers/pipeline_notification_worker_spec.rb index 9a15864173c..583c4bf1c0c 100644 --- a/spec/workers/pipeline_notification_worker_spec.rb +++ b/spec/workers/pipeline_notification_worker_spec.rb @@ -20,5 +20,9 @@ RSpec.describe PipelineNotificationWorker, :mailer do subject.perform(non_existing_record_id) end + + it_behaves_like 'worker with data consistency', + described_class, + data_consistency: :delayed end end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 04a38874905..c111c3164eb 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -5,7 +5,13 @@ require 'spec_helper' RSpec.describe PostReceive do include AfterNextHelpers - let(:changes) { "123456 789012 refs/heads/tést\n654321 210987 refs/tags/tag" } + let(:changes) do + <<~EOF + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/heads/tést + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/tags/tag + EOF + end + let(:wrongly_encoded_changes) { changes.encode("ISO-8859-1").force_encoding("UTF-8") } let(:base64_changes) { Base64.encode64(wrongly_encoded_changes) } let(:gl_repository) { "project-#{project.id}" } @@ -64,7 +70,6 @@ RSpec.describe PostReceive do describe '#process_project_changes' do context 'with an empty project' do let(:empty_project) { create(:project, :empty_repo) } - let(:changes) { "123456 789012 refs/heads/tést1\n" } before do allow_next(Gitlab::GitPostReceive).to receive(:identify).and_return(empty_project.owner) @@ -85,14 +90,6 @@ RSpec.describe PostReceive do perform end - it 'tracks an event for the new_project_readme experiment', :experiment do - expect_next_instance_of(NewProjectReadmeExperiment, :new_project_readme, nil, actor: empty_project.owner) do |e| - expect(e).to receive(:track_initial_writes).with(empty_project) - end - - perform - end - it 'tracks an event for the empty_repo_upload experiment', :experiment do expect_next_instance_of(EmptyRepoUploadExperiment) do |e| expect(e).to receive(:track_initial_write) @@ -154,8 +151,8 @@ RSpec.describe PostReceive do context 'branches' do let(:changes) do <<~EOF - 123456 789012 refs/heads/tést1 - 123456 789012 refs/heads/tést2 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/heads/tést1 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/heads/tést2 EOF end @@ -190,9 +187,9 @@ RSpec.describe PostReceive do context 'with a default branch' do let(:changes) do <<~EOF - 123456 789012 refs/heads/tést1 - 123456 789012 refs/heads/tést2 - 678912 123455 refs/heads/#{project.default_branch} + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/heads/tést1 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/heads/tést2 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/heads/#{project.default_branch} EOF end @@ -208,9 +205,9 @@ RSpec.describe PostReceive do context 'tags' do let(:changes) do <<~EOF - 654321 210987 refs/tags/tag1 - 654322 210986 refs/tags/tag2 - 654323 210985 refs/tags/tag3 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/tags/tag1 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/tags/tag2 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/tags/tag3 EOF end @@ -249,7 +246,7 @@ RSpec.describe PostReceive do end context 'merge-requests' do - let(:changes) { "123456 789012 refs/merge-requests/123" } + let(:changes) { "#{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/merge-requests/123" } it "does not call any of the services" do expect(Git::ProcessRefChangesService).not_to receive(:new) @@ -261,7 +258,6 @@ RSpec.describe PostReceive do end context 'after project changes hooks' do - let(:changes) { '123456 789012 refs/heads/tést' } let(:fake_hook_data) { { event_name: 'repository_update' } } before do @@ -313,12 +309,12 @@ RSpec.describe PostReceive do context 'master' do let(:default_branch) { 'master' } - let(:oldrev) { '012345' } - let(:newrev) { '6789ab' } + let(:oldrev) { SeedRepo::Commit::PARENT_ID } + let(:newrev) { SeedRepo::Commit::ID } let(:changes) do <<~EOF #{oldrev} #{newrev} refs/heads/#{default_branch} - 123456 789012 refs/heads/tést2 + #{oldrev} #{newrev} refs/heads/tést2 EOF end @@ -334,8 +330,8 @@ RSpec.describe PostReceive do context 'branches' do let(:changes) do <<~EOF - 123456 789012 refs/heads/tést1 - 123456 789012 refs/heads/tést2 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/heads/tést1 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/heads/tést2 EOF end @@ -414,8 +410,8 @@ RSpec.describe PostReceive do context 'branches' do let(:changes) do <<~EOF - 123456 789012 refs/heads/tést1 - 123456 789012 refs/heads/tést2 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/heads/tést1 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/heads/tést2 EOF end @@ -442,9 +438,9 @@ RSpec.describe PostReceive do context 'tags' do let(:changes) do <<~EOF - 654321 210987 refs/tags/tag1 - 654322 210986 refs/tags/tag2 - 654323 210985 refs/tags/tag3 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/tags/tag1 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/tags/tag2 + #{SeedRepo::Commit::PARENT_ID} #{SeedRepo::Commit::ID} refs/tags/tag3 EOF end diff --git a/spec/workers/propagate_integration_worker_spec.rb b/spec/workers/propagate_integration_worker_spec.rb index 2461b30a2ed..902e3206d35 100644 --- a/spec/workers/propagate_integration_worker_spec.rb +++ b/spec/workers/propagate_integration_worker_spec.rb @@ -4,9 +4,10 @@ require 'spec_helper' RSpec.describe PropagateIntegrationWorker do describe '#perform' do + let(:project) { create(:project) } let(:integration) do Integrations::Pushover.create!( - template: true, + project: project, active: true, device: 'MyDevice', sound: 'mic', diff --git a/spec/workers/propagate_service_template_worker_spec.rb b/spec/workers/propagate_service_template_worker_spec.rb deleted file mode 100644 index b692ce3d72b..00000000000 --- a/spec/workers/propagate_service_template_worker_spec.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe PropagateServiceTemplateWorker do - include ExclusiveLeaseHelpers - - describe '#perform' do - it 'calls the propagate service with the template' do - template = Integrations::Pushover.create!( - template: true, - active: true, - properties: { - device: 'MyDevice', - sound: 'mic', - priority: 4, - user_key: 'asdf', - api_key: '123456789' - }) - - stub_exclusive_lease("propagate_service_template_worker:#{template.id}", - timeout: PropagateServiceTemplateWorker::LEASE_TIMEOUT) - - expect(Admin::PropagateServiceTemplate) - .to receive(:propagate) - .with(template) - - subject.perform(template.id) - end - end -end diff --git a/spec/workers/repository_remove_remote_worker_spec.rb b/spec/workers/repository_remove_remote_worker_spec.rb index 758f7f75e03..11081ec9b37 100644 --- a/spec/workers/repository_remove_remote_worker_spec.rb +++ b/spec/workers/repository_remove_remote_worker_spec.rb @@ -24,37 +24,25 @@ RSpec.describe RepositoryRemoveRemoteWorker do .and_return(project) end - it 'does not remove remote when cannot obtain lease' do + it 'does nothing when cannot obtain lease' do stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) expect(project.repository) .not_to receive(:remove_remote) - expect(subject) - .to receive(:log_error) - .with("Cannot obtain an exclusive lease for #{lease_key}. There must be another instance already in execution.") + .not_to receive(:log_error) subject.perform(project.id, remote_name) end - it 'removes remote from repository when obtain a lease' do + it 'does nothing when obtain a lease' do stub_exclusive_lease(lease_key, timeout: lease_timeout) - masterrev = project.repository.find_branch('master').dereferenced_target - create_remote_branch(remote_name, 'remote_branch', masterrev) expect(project.repository) - .to receive(:remove_remote) - .with(remote_name) - .and_call_original + .not_to receive(:remove_remote) subject.perform(project.id, remote_name) end end end - - def create_remote_branch(remote_name, branch_name, target) - rugged = rugged_repo(project.repository) - - rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id) - end end diff --git a/spec/workers/users/create_statistics_worker_spec.rb b/spec/workers/users/create_statistics_worker_spec.rb index e3f082313a0..2118cc42f3a 100644 --- a/spec/workers/users/create_statistics_worker_spec.rb +++ b/spec/workers/users/create_statistics_worker_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Users::CreateStatisticsWorker do subject { described_class.new.perform } before do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false) + allow(UsersStatistics.connection).to receive(:transaction_open?).and_return(false) end context 'when successful' do diff --git a/spec/workers/web_hook_worker_spec.rb b/spec/workers/web_hook_worker_spec.rb index a86964aa417..0f40177eb7d 100644 --- a/spec/workers/web_hook_worker_spec.rb +++ b/spec/workers/web_hook_worker_spec.rb @@ -15,6 +15,10 @@ RSpec.describe WebHookWorker do subject.perform(project_hook.id, data, hook_name) end + it 'does not error when the WebHook record cannot be found' do + expect { subject.perform(non_existing_record_id, data, hook_name) }.not_to raise_error + end + it_behaves_like 'worker with data consistency', described_class, data_consistency: :delayed |