diff options
Diffstat (limited to 'spec/workers')
-rw-r--r-- | spec/workers/archive_trace_worker_spec.rb | 2 | ||||
-rw-r--r-- | spec/workers/background_migration_worker_spec.rb | 20 | ||||
-rw-r--r-- | spec/workers/ci/archive_traces_cron_worker_spec.rb | 23 | ||||
-rw-r--r-- | spec/workers/cluster_configure_worker_spec.rb | 71 | ||||
-rw-r--r-- | spec/workers/cluster_project_configure_worker_spec.rb | 14 | ||||
-rw-r--r-- | spec/workers/namespaces/root_statistics_worker_spec.rb | 11 | ||||
-rw-r--r-- | spec/workers/namespaces/schedule_aggregation_worker_spec.rb | 10 | ||||
-rw-r--r-- | spec/workers/pipeline_process_worker_spec.rb | 11 | ||||
-rw-r--r-- | spec/workers/post_receive_spec.rb | 152 | ||||
-rw-r--r-- | spec/workers/process_commit_worker_spec.rb | 40 | ||||
-rw-r--r-- | spec/workers/project_cache_worker_spec.rb | 10 | ||||
-rw-r--r-- | spec/workers/repository_update_remote_mirror_worker_spec.rb | 111 | ||||
-rw-r--r-- | spec/workers/stuck_ci_jobs_worker_spec.rb | 4 |
13 files changed, 219 insertions, 260 deletions
diff --git a/spec/workers/archive_trace_worker_spec.rb b/spec/workers/archive_trace_worker_spec.rb index 368ed3f3db1..44f7be15201 100644 --- a/spec/workers/archive_trace_worker_spec.rb +++ b/spec/workers/archive_trace_worker_spec.rb @@ -11,7 +11,7 @@ describe ArchiveTraceWorker do it 'executes service' do expect_any_instance_of(Ci::ArchiveTraceService) - .to receive(:execute).with(job) + .to receive(:execute).with(job, anything) subject end diff --git a/spec/workers/background_migration_worker_spec.rb b/spec/workers/background_migration_worker_spec.rb index 746c858609f..e5be8ce0423 100644 --- a/spec/workers/background_migration_worker_spec.rb +++ b/spec/workers/background_migration_worker_spec.rb @@ -55,21 +55,13 @@ describe BackgroundMigrationWorker, :sidekiq, :clean_gitlab_redis_shared_state d end describe '#healthy_database?' do - context 'using MySQL', :mysql do - it 'returns true' do - expect(worker.healthy_database?).to eq(true) - end - end - - context 'using PostgreSQL', :postgresql do - context 'when replication lag is too great' do - it 'returns false' do - allow(Postgresql::ReplicationSlot) - .to receive(:lag_too_great?) - .and_return(true) + context 'when replication lag is too great' do + it 'returns false' do + allow(Postgresql::ReplicationSlot) + .to receive(:lag_too_great?) + .and_return(true) - expect(worker.healthy_database?).to eq(false) - end + expect(worker.healthy_database?).to eq(false) end context 'when replication lag is small enough' do diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb index eca6cf5235f..01232e2a58b 100644 --- a/spec/workers/ci/archive_traces_cron_worker_spec.rb +++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' describe Ci::ArchiveTracesCronWorker do subject { described_class.new.perform } + let(:finished_at) { 1.day.ago } + before do stub_feature_flags(ci_enable_live_trace: true) end @@ -28,20 +30,26 @@ describe Ci::ArchiveTracesCronWorker do end context 'when a job succeeded' do - let!(:build) { create(:ci_build, :success, :trace_live) } + let!(:build) { create(:ci_build, :success, :trace_live, finished_at: finished_at) } it_behaves_like 'archives trace' it 'executes service' do expect_any_instance_of(Ci::ArchiveTraceService) - .to receive(:execute).with(build) + .to receive(:execute).with(build, anything) subject end + context 'when the job finished recently' do + let(:finished_at) { 1.hour.ago } + + it_behaves_like 'does not archive trace' + end + context 'when a trace had already been archived' do let!(:build) { create(:ci_build, :success, :trace_live, :trace_artifact) } - let!(:build2) { create(:ci_build, :success, :trace_live) } + let!(:build2) { create(:ci_build, :success, :trace_live, finished_at: finished_at) } it 'continues to archive live traces' do subject @@ -52,7 +60,7 @@ describe Ci::ArchiveTracesCronWorker do end context 'when an unexpected exception happened during archiving' do - let!(:build) { create(:ci_build, :success, :trace_live) } + let!(:build) { create(:ci_build, :success, :trace_live, finished_at: finished_at) } before do allow(Gitlab::Sentry).to receive(:track_exception) @@ -60,7 +68,10 @@ describe Ci::ArchiveTracesCronWorker do end it 'puts a log' do - expect(Rails.logger).to receive(:error).with("Failed to archive trace. id: #{build.id} message: Unexpected error") + expect(Sidekiq.logger).to receive(:warn).with( + class: described_class.name, + message: "Failed to archive trace. message: Unexpected error.", + job_id: build.id) subject end @@ -68,7 +79,7 @@ describe Ci::ArchiveTracesCronWorker do end context 'when a job was cancelled' do - let!(:build) { create(:ci_build, :canceled, :trace_live) } + let!(:build) { create(:ci_build, :canceled, :trace_live, finished_at: finished_at) } it_behaves_like 'archives trace' end diff --git a/spec/workers/cluster_configure_worker_spec.rb b/spec/workers/cluster_configure_worker_spec.rb deleted file mode 100644 index 975088f3ee6..00000000000 --- a/spec/workers/cluster_configure_worker_spec.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe ClusterConfigureWorker, '#perform' do - let(:worker) { described_class.new } - - shared_examples 'configured cluster' do - it 'creates a namespace' do - expect(Clusters::RefreshService).to receive(:create_or_update_namespaces_for_cluster).with(cluster).once - - worker.perform(cluster.id) - end - end - - shared_examples 'unconfigured cluster' do - it 'does not create a namespace' do - expect(Clusters::RefreshService).not_to receive(:create_or_update_namespaces_for_cluster) - - worker.perform(cluster.id) - end - end - - context 'group cluster' do - let(:cluster) { create(:cluster, :group, :provided_by_gcp) } - let(:group) { cluster.group } - - context 'when group has a project' do - let!(:project) { create(:project, group: group) } - - it_behaves_like 'unconfigured cluster' - end - - context 'when group has project in a sub-group' do - let!(:subgroup) { create(:group, parent: group) } - let!(:project) { create(:project, group: subgroup) } - - it_behaves_like 'unconfigured cluster' - end - end - - context 'when provider type is gcp' do - let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } - - it_behaves_like 'configured cluster' - end - - context 'when provider type is user' do - let!(:cluster) { create(:cluster, :project, :provided_by_user) } - - it_behaves_like 'configured cluster' - end - - context 'when cluster is not managed' do - let(:cluster) { create(:cluster, :not_managed) } - - it 'does not configure the cluster' do - expect(Clusters::RefreshService).not_to receive(:create_or_update_namespaces_for_cluster) - - described_class.new.perform(cluster.id) - end - end - - context 'when cluster does not exist' do - it 'does not provision a cluster' do - expect_any_instance_of(Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService).not_to receive(:execute) - - described_class.new.perform(123) - end - end -end diff --git a/spec/workers/cluster_project_configure_worker_spec.rb b/spec/workers/cluster_project_configure_worker_spec.rb deleted file mode 100644 index 2ac9d0f61b4..00000000000 --- a/spec/workers/cluster_project_configure_worker_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe ClusterProjectConfigureWorker, '#perform' do - let(:worker) { described_class.new } - let(:cluster) { create(:cluster, :project) } - - it 'configures the cluster' do - expect(Clusters::RefreshService).to receive(:create_or_update_namespaces_for_project) - - described_class.new.perform(cluster.projects.first.id) - end -end diff --git a/spec/workers/namespaces/root_statistics_worker_spec.rb b/spec/workers/namespaces/root_statistics_worker_spec.rb index 8dd74b96d49..6bbdfe03ceb 100644 --- a/spec/workers/namespaces/root_statistics_worker_spec.rb +++ b/spec/workers/namespaces/root_statistics_worker_spec.rb @@ -74,15 +74,4 @@ describe Namespaces::RootStatisticsWorker, '#perform' do worker.perform(group.id) end end - - context 'when update_statistics_namespace is off' do - it 'does not create a new one' do - stub_feature_flags(update_statistics_namespace: false, namespace: group) - - expect_any_instance_of(Namespaces::StatisticsRefresherService) - .not_to receive(:execute) - - worker.perform(group.id) - end - end end diff --git a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb index d4a49a3f53a..be722f451e0 100644 --- a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb +++ b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb @@ -31,16 +31,6 @@ describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_redis_ expect(group.aggregation_schedule).to be_present end end - - context 'when update_statistics_namespace is off' do - it 'does not create a new one' do - stub_feature_flags(update_statistics_namespace: false, namespace: group) - - expect do - worker.perform(group.id) - end.not_to change(Namespace::AggregationSchedule, :count) - end - end end context 'when group is not the root ancestor' do diff --git a/spec/workers/pipeline_process_worker_spec.rb b/spec/workers/pipeline_process_worker_spec.rb index d33cf72e51e..ac677e3b555 100644 --- a/spec/workers/pipeline_process_worker_spec.rb +++ b/spec/workers/pipeline_process_worker_spec.rb @@ -12,6 +12,17 @@ describe PipelineProcessWorker do described_class.new.perform(pipeline.id) end + + context 'when build_ids are passed' do + let(:build) { create(:ci_build, pipeline: pipeline, name: 'my-build') } + + it 'processes pipeline with a list of builds' do + expect_any_instance_of(Ci::Pipeline).to receive(:process!) + .with([build.id]) + + described_class.new.perform(pipeline.id, [build.id]) + end + end end context 'when pipeline does not exist' do diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 39f1beb4efa..c8a0c22b0e8 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -14,6 +14,10 @@ describe PostReceive do create(:project, :repository, auto_cancel_pending_pipelines: 'disabled') end + def perform(changes: base64_changes) + described_class.new.perform(gl_repository, key_id, changes) + end + context "as a sidekiq worker" do it "responds to #perform" do expect(described_class.new).to respond_to(:perform) @@ -28,18 +32,41 @@ describe PostReceive do it "returns false and logs an error" do expect(Gitlab::GitLogger).to receive(:error).with("POST-RECEIVE: #{error_message}") - expect(described_class.new.perform(gl_repository, key_id, base64_changes)).to be(false) + expect(perform).to be(false) end end 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_any_instance_of(Gitlab::GitPostReceive).to receive(:identify).and_return(empty_project.owner) + allow(Gitlab::GlRepository).to receive(:parse).and_return([empty_project, Gitlab::GlRepository::PROJECT]) + end + + it 'expire the status cache' do + expect(empty_project.repository).to receive(:expire_status_cache) + + perform + end + + it 'schedules a cache update for commit count and size' do + expect(ProjectCacheWorker).to receive(:perform_async) + .with(empty_project.id, [], [:repository_size, :commit_count], true) + + perform + end + end + context 'empty changes' do it "does not call any PushService but runs after project hooks" do expect(Git::BranchPushService).not_to receive(:new) expect(Git::TagPushService).not_to receive(:new) expect_next_instance_of(SystemHooksService) { |service| expect(service).to receive(:execute_hooks) } - described_class.new.perform(gl_repository, key_id, "") + perform(changes: "") end end @@ -50,40 +77,118 @@ describe PostReceive do expect(Git::BranchPushService).not_to receive(:new) expect(Git::TagPushService).not_to receive(:new) - expect(described_class.new.perform(gl_repository, key_id, base64_changes)).to be false + expect(perform).to be false end end context 'with changes' do before do allow_any_instance_of(Gitlab::GitPostReceive).to receive(:identify).and_return(project.owner) + allow(Gitlab::GlRepository).to receive(:parse).and_return([project, Gitlab::GlRepository::PROJECT]) end context "branches" do - let(:changes) { "123456 789012 refs/heads/tést" } + let(:changes) do + <<~EOF + 123456 789012 refs/heads/tést1 + 123456 789012 refs/heads/tést2 + EOF + end - it "calls Git::BranchPushService" do - expect_next_instance_of(Git::BranchPushService) do |service| + it 'expires the branches cache' do + expect(project.repository).to receive(:expire_branches_cache).once + + perform + end + + it 'expires the status cache' do + expect(project).to receive(:empty_repo?).and_return(true) + expect(project.repository).to receive(:expire_status_cache) + + perform + end + + it 'calls Git::BranchPushService' do + expect_any_instance_of(Git::BranchPushService) do |service| expect(service).to receive(:execute).and_return(true) end expect(Git::TagPushService).not_to receive(:new) - described_class.new.perform(gl_repository, key_id, base64_changes) + perform + end + + it 'schedules a cache update for repository size only' do + expect(ProjectCacheWorker).to receive(:perform_async) + .with(project.id, [], [:repository_size], true) + + perform + end + + 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} + EOF + end + + it 'schedules a cache update for commit count and size' do + expect(ProjectCacheWorker).to receive(:perform_async) + .with(project.id, [], [:repository_size, :commit_count], true) + + perform + end end end context "tags" do - let(:changes) { "123456 789012 refs/tags/tag" } + let(:changes) do + <<~EOF + 654321 210987 refs/tags/tag1 + 654322 210986 refs/tags/tag2 + 654323 210985 refs/tags/tag3 + 654324 210984 refs/tags/tag4 + 654325 210983 refs/tags/tag5 + EOF + end + + before do + expect(Gitlab::GlRepository).to receive(:parse).and_return([project, Gitlab::GlRepository::PROJECT]) + end + + it 'does not expire branches cache' do + expect(project.repository).not_to receive(:expire_branches_cache) + + perform + end + + it "only invalidates tags once" do + expect(project.repository).to receive(:repository_event).exactly(5).times.with(:push_tag).and_call_original + expect(project.repository).to receive(:expire_caches_for_tags).once.and_call_original + expect(project.repository).to receive(:expire_tags_cache).once.and_call_original + + perform + end it "calls Git::TagPushService" do - expect(Git::BranchPushService).not_to receive(:execute) + expect(Git::BranchPushService).not_to receive(:new) - expect_next_instance_of(Git::TagPushService) do |service| + expect_any_instance_of(Git::TagPushService) do |service| expect(service).to receive(:execute).and_return(true) end - described_class.new.perform(gl_repository, key_id, base64_changes) + expect(Git::BranchPushService).not_to receive(:new) + + perform + end + + it 'schedules a single ProjectCacheWorker update' do + expect(ProjectCacheWorker).to receive(:perform_async) + .with(project.id, [], [:repository_size], true) + + perform end end @@ -94,7 +199,7 @@ describe PostReceive do expect(Git::BranchPushService).not_to receive(:new) expect(Git::TagPushService).not_to receive(:new) - described_class.new.perform(gl_repository, key_id, base64_changes) + perform end end @@ -111,7 +216,7 @@ describe PostReceive do let(:changes_count) { changes.lines.count } - subject { described_class.new.perform(gl_repository, key_id, base64_changes) } + subject { perform } context "with valid .gitlab-ci.yml" do before do @@ -180,7 +285,13 @@ describe PostReceive do it 'calls SystemHooksService' do expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(fake_hook_data, :repository_update_hooks).and_return(true) - described_class.new.perform(gl_repository, key_id, base64_changes) + perform + end + + it 'increments the usage data counter of pushes event' do + counter = Gitlab::UsageDataCounters::SourceCodeCounter + + expect { perform }.to change { counter.read(:pushes) }.by(1) end end end @@ -197,7 +308,7 @@ describe PostReceive do # a second to ensure we see changes. Timecop.freeze(1.second.from_now) do expect do - described_class.new.perform(gl_repository, key_id, base64_changes) + perform project.reload end.to change(project, :last_activity_at) .and change(project, :last_repository_updated_at) @@ -208,7 +319,8 @@ describe PostReceive do context "webhook" do it "fetches the correct project" do expect(Project).to receive(:find_by).with(id: project.id.to_s) - described_class.new.perform(gl_repository, key_id, base64_changes) + + perform end it "does not run if the author is not in the project" do @@ -218,16 +330,18 @@ describe PostReceive do expect(project).not_to receive(:execute_hooks) - expect(described_class.new.perform(gl_repository, key_id, base64_changes)).to be_falsey + expect(perform).to be_falsey end it "asks the project to trigger all hooks" do + create(:project_hook, push_events: true, tag_push_events: true, project: project) + create(:custom_issue_tracker_service, push_events: true, merge_requests_events: false, project: project) allow(Project).to receive(:find_by).and_return(project) expect(project).to receive(:execute_hooks).twice expect(project).to receive(:execute_services).twice - described_class.new.perform(gl_repository, key_id, base64_changes) + perform end it "enqueues a UpdateMergeRequestsWorker job" do @@ -235,7 +349,7 @@ describe PostReceive do expect(UpdateMergeRequestsWorker).to receive(:perform_async).with(project.id, project.owner.id, any_args) - described_class.new.perform(gl_repository, key_id, base64_changes) + perform end end end diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb index 47bac63511e..eb1d3c364ac 100644 --- a/spec/workers/process_commit_worker_spec.rb +++ b/spec/workers/process_commit_worker_spec.rb @@ -3,8 +3,6 @@ require 'spec_helper' describe ProcessCommitWorker do - include ProjectForksHelper - let(:worker) { described_class.new } let(:user) { create(:user) } let(:project) { create(:project, :public, :repository) } @@ -35,44 +33,6 @@ describe ProcessCommitWorker do worker.perform(project.id, user.id, commit.to_hash) end - - context 'when the project is forked' do - context 'when commit already exists in the upstream project' do - it 'does not process the commit message' do - forked = fork_project(project, user, repository: true) - - expect(worker).not_to receive(:process_commit_message) - - worker.perform(forked.id, user.id, forked.commit.to_hash) - end - end - - context 'when the commit does not exist in the upstream project' do - it 'processes the commit message' do - empty_project = create(:project, :public) - forked = fork_project(empty_project, user, repository: true) - - TestEnv.copy_repo(forked, - bare_repo: TestEnv.factory_repo_path_bare, - refs: TestEnv::BRANCH_SHA) - - expect(worker).to receive(:process_commit_message) - - worker.perform(forked.id, user.id, forked.commit.to_hash) - end - end - - context 'when the upstream project no longer exists' do - it 'processes the commit message' do - forked = fork_project(project, user, repository: true) - project.destroy! - - expect(worker).to receive(:process_commit_message) - - worker.perform(forked.id, user.id, forked.commit.to_hash) - end - end - end end describe '#process_commit_message' do diff --git a/spec/workers/project_cache_worker_spec.rb b/spec/workers/project_cache_worker_spec.rb index edc55920b8e..7f3c4881b89 100644 --- a/spec/workers/project_cache_worker_spec.rb +++ b/spec/workers/project_cache_worker_spec.rb @@ -49,6 +49,16 @@ describe ProjectCacheWorker do worker.perform(project.id, %w(readme)) end + context 'with statistics disabled' do + let(:statistics) { [] } + + it 'does not update the project statistics' do + expect(worker).not_to receive(:update_statistics) + + worker.perform(project.id, [], [], false) + end + end + context 'with statistics' do let(:statistics) { %w(repository_size) } diff --git a/spec/workers/repository_update_remote_mirror_worker_spec.rb b/spec/workers/repository_update_remote_mirror_worker_spec.rb index 4de51ecb3e9..66d517332ba 100644 --- a/spec/workers/repository_update_remote_mirror_worker_spec.rb +++ b/spec/workers/repository_update_remote_mirror_worker_spec.rb @@ -2,99 +2,70 @@ require 'rails_helper' -describe RepositoryUpdateRemoteMirrorWorker do +describe RepositoryUpdateRemoteMirrorWorker, :clean_gitlab_redis_shared_state do subject { described_class.new } - let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } + let(:remote_mirror) { create(:remote_mirror) } let(:scheduled_time) { Time.now - 5.minutes } around do |example| Timecop.freeze(Time.now) { example.run } end - describe '#perform' do - context 'with status none' do - before do - remote_mirror.update(update_status: 'none') - end - - it 'sets status as finished when update remote mirror service executes successfully' do - expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success) - - expect { subject.perform(remote_mirror.id, Time.now) }.to change { remote_mirror.reload.update_status }.to('finished') - end - - it 'resets the notification flag upon success' do - expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success) - remote_mirror.update_column(:error_notification_sent, true) - - expect { subject.perform(remote_mirror.id, Time.now) }.to change { remote_mirror.reload.error_notification_sent }.to(false) - end - - it 'sets status as failed when update remote mirror service executes with errors' do - error_message = 'fail!' - - expect_next_instance_of(Projects::UpdateRemoteMirrorService) do |service| - expect(service).to receive(:execute).with(remote_mirror).and_return(status: :error, message: error_message) - end + def expect_mirror_service_to_return(mirror, result, tries = 0) + expect_next_instance_of(Projects::UpdateRemoteMirrorService) do |service| + expect(service).to receive(:execute).with(mirror, tries).and_return(result) + end + end - # Mock the finder so that it returns an object we can set expectations on - expect_next_instance_of(RemoteMirrorFinder) do |finder| - expect(finder).to receive(:execute).and_return(remote_mirror) - end - expect(remote_mirror).to receive(:mark_as_failed).with(error_message) + describe '#perform' do + it 'calls out to the service to perform the update' do + expect_mirror_service_to_return(remote_mirror, status: :success) - expect do - subject.perform(remote_mirror.id, Time.now) - end.to raise_error(RepositoryUpdateRemoteMirrorWorker::UpdateError, error_message) - end + subject.perform(remote_mirror.id, scheduled_time) + end - it 'does nothing if last_update_started_at is higher than the time the job was scheduled in' do - remote_mirror.update(last_update_started_at: Time.now) + it 'does not do anything if the mirror was already updated' do + remote_mirror.update(last_update_started_at: Time.now, update_status: :finished) - expect_any_instance_of(RemoteMirror).to receive(:updated_since?).with(scheduled_time).and_return(true) - expect_any_instance_of(Projects::UpdateRemoteMirrorService).not_to receive(:execute).with(remote_mirror) + expect(Projects::UpdateRemoteMirrorService).not_to receive(:new) - expect(subject.perform(remote_mirror.id, scheduled_time)).to be_nil - end + subject.perform(remote_mirror.id, scheduled_time) end - context 'with unexpected error' do - it 'marks mirror as failed' do - allow_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_raise(RuntimeError) + it 'schedules a retry when the mirror is marked for retrying' do + remote_mirror = create(:remote_mirror, update_status: :to_retry) + expect_mirror_service_to_return(remote_mirror, status: :error, message: 'Retry!') - expect do - subject.perform(remote_mirror.id, Time.now) - end.to raise_error(RepositoryUpdateRemoteMirrorWorker::UpdateError) - expect(remote_mirror.reload.update_status).to eq('failed') - end - end + expect(described_class) + .to receive(:perform_in) + .with(remote_mirror.backoff_delay, remote_mirror.id, scheduled_time, 1) - context 'with another worker already running' do - before do - remote_mirror.update(update_status: 'started') - end - - it 'raises RemoteMirrorUpdateAlreadyInProgressError' do - expect do - subject.perform(remote_mirror.id, Time.now) - end.to raise_error(RepositoryUpdateRemoteMirrorWorker::UpdateAlreadyInProgressError) - end + subject.perform(remote_mirror.id, scheduled_time) end - context 'with status failed' do - before do - remote_mirror.update(update_status: 'failed') + it 'clears the lease if there was an unexpected exception' do + expect_next_instance_of(Projects::UpdateRemoteMirrorService) do |service| + expect(service).to receive(:execute).with(remote_mirror, 1).and_raise('Unexpected!') end + expect { subject.perform(remote_mirror.id, Time.now, 1) }.to raise_error('Unexpected!') - it 'sets status as finished if last_update_started_at is higher than the time the job was scheduled in' do - remote_mirror.update(last_update_started_at: Time.now) + lease = Gitlab::ExclusiveLease.new("#{described_class.name}:#{remote_mirror.id}", timeout: 1.second) - expect_any_instance_of(RemoteMirror).to receive(:updated_since?).with(scheduled_time).and_return(false) - expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success) + expect(lease.try_obtain).not_to be_nil + end - expect { subject.perform(remote_mirror.id, scheduled_time) }.to change { remote_mirror.reload.update_status }.to('finished') - end + it 'retries 3 times for the worker to finish before rescheduling' do + expect(subject).to receive(:in_lock) + .with("#{described_class.name}:#{remote_mirror.id}", + retries: 3, + ttl: remote_mirror.max_runtime, + sleep_sec: described_class::LOCK_WAIT_TIME) + .and_raise(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError) + expect(described_class).to receive(:perform_in) + .with(remote_mirror.backoff_delay, remote_mirror.id, scheduled_time, 0) + + subject.perform(remote_mirror.id, scheduled_time) end end end diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index 72de62f1188..c3d577e2dae 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -7,8 +7,6 @@ describe StuckCiJobsWorker do let!(:runner) { create :ci_runner } let!(:job) { create :ci_build, runner: runner } - let(:trace_lease_key) { "trace:write:lock:#{job.id}" } - let(:trace_lease_uuid) { SecureRandom.uuid } let(:worker_lease_key) { StuckCiJobsWorker::EXCLUSIVE_LEASE_KEY } let(:worker_lease_uuid) { SecureRandom.uuid } @@ -16,7 +14,6 @@ describe StuckCiJobsWorker do before do stub_exclusive_lease(worker_lease_key, worker_lease_uuid) - stub_exclusive_lease(trace_lease_key, trace_lease_uuid) job.update!(status: status, updated_at: updated_at) end @@ -195,7 +192,6 @@ describe StuckCiJobsWorker do end it 'cancels exclusive leases after worker perform' do - expect_to_cancel_exclusive_lease(trace_lease_key, trace_lease_uuid) expect_to_cancel_exclusive_lease(worker_lease_key, worker_lease_uuid) worker.perform |