diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-06-20 14:10:13 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-06-20 14:10:13 +0300 |
commit | 0ea3fcec397b69815975647f5e2aa5fe944a8486 (patch) | |
tree | 7979381b89d26011bcf9bdc989a40fcc2f1ed4ff /spec/services/ci | |
parent | 72123183a20411a36d607d70b12d57c484394c8e (diff) |
Add latest changes from gitlab-org/gitlab@15-1-stable-eev15.1.0-rc42
Diffstat (limited to 'spec/services/ci')
8 files changed, 213 insertions, 50 deletions
diff --git a/spec/services/ci/abort_pipelines_service_spec.rb b/spec/services/ci/abort_pipelines_service_spec.rb index db25faff70f..9f9519d6829 100644 --- a/spec/services/ci/abort_pipelines_service_spec.rb +++ b/spec/services/ci/abort_pipelines_service_spec.rb @@ -94,28 +94,5 @@ RSpec.describe Ci::AbortPipelinesService do end end end - - context 'with user pipelines' do - def abort_user_pipelines - described_class.new.execute(user.pipelines, :user_blocked) - end - - it 'fails all running pipelines and related jobs' do - expect(abort_user_pipelines).to be_success - - expect_correct_cancellations - - expect(other_users_pipeline.status).not_to eq('failed') - end - - it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new { abort_user_pipelines }.count - - pipelines = create_list(:ci_pipeline, 5, :running, project: project, user: user) - create_list(:ci_build, 5, :running, pipeline: pipelines.first) - - expect { abort_user_pipelines }.not_to exceed_query_limit(control_count) - end - end end end diff --git a/spec/services/ci/after_requeue_job_service_spec.rb b/spec/services/ci/after_requeue_job_service_spec.rb index c9bd44f78e2..fb67ee18fb2 100644 --- a/spec/services/ci/after_requeue_job_service_spec.rb +++ b/spec/services/ci/after_requeue_job_service_spec.rb @@ -26,6 +26,11 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do script: exit 0 needs: [a1] + a3: + stage: a + script: exit 0 + needs: [a2] + b1: stage: b script: exit 0 @@ -59,6 +64,7 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do check_jobs_statuses( a1: 'pending', a2: 'created', + a3: 'created', b1: 'pending', b2: 'created', c1: 'created', @@ -69,6 +75,7 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do check_jobs_statuses( a1: 'pending', a2: 'created', + a3: 'created', b1: 'success', b2: 'created', c1: 'created', @@ -79,6 +86,7 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do check_jobs_statuses( a1: 'failed', a2: 'skipped', + a3: 'skipped', b1: 'success', b2: 'skipped', c1: 'skipped', @@ -90,6 +98,7 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do check_jobs_statuses( a1: 'pending', a2: 'skipped', + a3: 'skipped', b1: 'success', b2: 'skipped', c1: 'skipped', @@ -103,12 +112,42 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do check_jobs_statuses( a1: 'pending', a2: 'created', + a3: 'skipped', b1: 'success', b2: 'created', c1: 'created', c2: 'created' ) end + + context 'when executed by a different user than the original owner' do + let(:retryer) { create(:user).tap { |u| project.add_maintainer(u) } } + let(:service) { described_class.new(project, retryer) } + + it 'reassigns jobs with updated statuses to the retryer' do + expect(jobs_name_status_owner_needs).to contain_exactly( + { 'name' => 'a1', 'status' => 'pending', 'user_id' => user.id, 'needs' => [] }, + { 'name' => 'a2', 'status' => 'skipped', 'user_id' => user.id, 'needs' => ['a1'] }, + { 'name' => 'a3', 'status' => 'skipped', 'user_id' => user.id, 'needs' => ['a2'] }, + { 'name' => 'b1', 'status' => 'success', 'user_id' => user.id, 'needs' => [] }, + { 'name' => 'b2', 'status' => 'skipped', 'user_id' => user.id, 'needs' => ['a2'] }, + { 'name' => 'c1', 'status' => 'skipped', 'user_id' => user.id, 'needs' => ['b2'] }, + { 'name' => 'c2', 'status' => 'skipped', 'user_id' => user.id, 'needs' => [] } + ) + + execute_after_requeue_service(a1) + + expect(jobs_name_status_owner_needs).to contain_exactly( + { 'name' => 'a1', 'status' => 'pending', 'user_id' => user.id, 'needs' => [] }, + { 'name' => 'a2', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['a1'] }, + { 'name' => 'a3', 'status' => 'skipped', 'user_id' => user.id, 'needs' => ['a2'] }, + { 'name' => 'b1', 'status' => 'success', 'user_id' => user.id, 'needs' => [] }, + { 'name' => 'b2', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['a2'] }, + { 'name' => 'c1', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['b2'] }, + { 'name' => 'c2', 'status' => 'created', 'user_id' => retryer.id, 'needs' => [] } + ) + end + end end context 'stage-dag mixed pipeline with some same-stage needs' do @@ -212,6 +251,12 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do pipeline.processables.latest end + def jobs_name_status_owner_needs + processables.reload.map do |job| + job.attributes.slice('name', 'status', 'user_id').merge('needs' => job.needs.map(&:name)) + end + end + def execute_after_requeue_service(processable) service.execute(processable) end diff --git a/spec/services/ci/create_pipeline_service/rate_limit_spec.rb b/spec/services/ci/create_pipeline_service/rate_limit_spec.rb index caea165cc6c..0000296230f 100644 --- a/spec/services/ci/create_pipeline_service/rate_limit_spec.rb +++ b/spec/services/ci/create_pipeline_service/rate_limit_spec.rb @@ -10,10 +10,8 @@ RSpec.describe Ci::CreatePipelineService, :freeze_time, :clean_gitlab_redis_rate before do stub_ci_pipeline_yaml_file(gitlab_ci_yaml) - stub_feature_flags(ci_throttle_pipelines_creation_dry_run: false) - - allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits) - .and_return(pipelines_create: { threshold: 1, interval: 1.minute }) + stub_application_setting(pipeline_limit_per_project_user_sha: 1) + stub_feature_flags(ci_enforce_throttle_pipelines_creation_override: false) end context 'when user is under the limit' do diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index c39a76ad2fc..aac059f2104 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -12,10 +12,6 @@ RSpec.describe Ci::CreatePipelineService do before do stub_ci_pipeline_to_return_yaml_file - - # Disable rate limiting for pipeline creation - allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits) - .and_return(pipelines_create: { threshold: 0, interval: 1.minute }) end describe '#execute' do @@ -1541,11 +1537,12 @@ RSpec.describe Ci::CreatePipelineService do expect(pipeline.target_sha).to be_nil end - it 'schedules update for the head pipeline of the merge request', :sidekiq_inline do - expect(UpdateHeadPipelineForMergeRequestWorker) - .to receive(:perform_async).with(merge_request.id) + it 'schedules update for the head pipeline of the merge request' do + allow(MergeRequests::UpdateHeadPipelineWorker).to receive(:perform_async) pipeline + + expect(MergeRequests::UpdateHeadPipelineWorker).to have_received(:perform_async).with('Ci::PipelineCreatedEvent', { 'pipeline_id' => pipeline.id }) end it 'schedules a namespace onboarding create action worker' do diff --git a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb index 1c6963e4a31..4f7663d7996 100644 --- a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb @@ -99,6 +99,16 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s expect { subject }.not_to change { artifact.file.exists? } end end + + context 'when the project in which the arfifact belongs to is undergoing stats refresh' do + before do + create(:project_build_artifacts_size_refresh, :pending, project: artifact.project) + end + + it 'does not destroy job artifact' do + expect { subject }.not_to change { Ci::JobArtifact.count } + end + end end context 'when artifact is locked' do diff --git a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb index 5e77041a632..3a04a3af03e 100644 --- a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb @@ -4,7 +4,14 @@ require 'spec_helper' RSpec.describe Ci::JobArtifacts::DestroyBatchService do let(:artifacts) { Ci::JobArtifact.where(id: [artifact_with_file.id, artifact_without_file.id, trace_artifact.id]) } - let(:service) { described_class.new(artifacts, pick_up_at: Time.current) } + let(:skip_projects_on_refresh) { false } + let(:service) do + described_class.new( + artifacts, + pick_up_at: Time.current, + skip_projects_on_refresh: skip_projects_on_refresh + ) + end let_it_be(:artifact_with_file, refind: true) do create(:ci_job_artifact, :zip) @@ -52,6 +59,128 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do .and not_change { Ci::JobArtifact.exists?(trace_artifact.id) } end + context 'when artifact belongs to a project that is undergoing stats refresh' do + let!(:artifact_under_refresh_1) do + create(:ci_job_artifact, :zip) + end + + let!(:artifact_under_refresh_2) do + create(:ci_job_artifact, :zip) + end + + let!(:artifact_under_refresh_3) do + create(:ci_job_artifact, :zip, project: artifact_under_refresh_2.project) + end + + let(:artifacts) do + Ci::JobArtifact.where(id: [artifact_with_file.id, artifact_under_refresh_1.id, artifact_under_refresh_2.id, + artifact_under_refresh_3.id]) + end + + before do + create(:project_build_artifacts_size_refresh, :created, project: artifact_with_file.project) + create(:project_build_artifacts_size_refresh, :pending, project: artifact_under_refresh_1.project) + create(:project_build_artifacts_size_refresh, :running, project: artifact_under_refresh_2.project) + end + + shared_examples 'avoiding N+1 queries' do + let!(:control_artifact_on_refresh) do + create(:ci_job_artifact, :zip) + end + + let!(:control_artifact_non_refresh) do + create(:ci_job_artifact, :zip) + end + + let!(:other_artifact_on_refresh) do + create(:ci_job_artifact, :zip) + end + + let!(:other_artifact_on_refresh_2) do + create(:ci_job_artifact, :zip) + end + + let!(:other_artifact_non_refresh) do + create(:ci_job_artifact, :zip) + end + + let!(:control_artifacts) do + Ci::JobArtifact.where( + id: [ + control_artifact_on_refresh.id, + control_artifact_non_refresh.id + ] + ) + end + + let!(:artifacts) do + Ci::JobArtifact.where( + id: [ + other_artifact_on_refresh.id, + other_artifact_on_refresh_2.id, + other_artifact_non_refresh.id + ] + ) + end + + let(:control_service) do + described_class.new( + control_artifacts, + pick_up_at: Time.current, + skip_projects_on_refresh: skip_projects_on_refresh + ) + end + + before do + create(:project_build_artifacts_size_refresh, :pending, project: control_artifact_on_refresh.project) + create(:project_build_artifacts_size_refresh, :pending, project: other_artifact_on_refresh.project) + create(:project_build_artifacts_size_refresh, :pending, project: other_artifact_on_refresh_2.project) + end + + it 'does not make multiple queries when fetching multiple project refresh records' do + control = ActiveRecord::QueryRecorder.new { control_service.execute } + + expect { subject }.not_to exceed_query_limit(control) + end + end + + context 'and skip_projects_on_refresh is set to false (default)' do + it 'logs the projects undergoing refresh and continues with the delete', :aggregate_failures do + expect(Gitlab::ProjectStatsRefreshConflictsLogger).to receive(:warn_artifact_deletion_during_stats_refresh).with( + method: 'Ci::JobArtifacts::DestroyBatchService#execute', + project_id: artifact_under_refresh_1.project.id + ).once + + expect(Gitlab::ProjectStatsRefreshConflictsLogger).to receive(:warn_artifact_deletion_during_stats_refresh).with( + method: 'Ci::JobArtifacts::DestroyBatchService#execute', + project_id: artifact_under_refresh_2.project.id + ).once + + expect { subject }.to change { Ci::JobArtifact.count }.by(-4) + end + + it_behaves_like 'avoiding N+1 queries' + end + + context 'and skip_projects_on_refresh is set to true' do + let(:skip_projects_on_refresh) { true } + + it 'logs the projects undergoing refresh and excludes the artifacts from deletion', :aggregate_failures do + expect(Gitlab::ProjectStatsRefreshConflictsLogger).to receive(:warn_skipped_artifact_deletion_during_stats_refresh).with( + method: 'Ci::JobArtifacts::DestroyBatchService#execute', + project_ids: match_array([artifact_under_refresh_1.project.id, artifact_under_refresh_2.project.id]) + ) + + expect { subject }.to change { Ci::JobArtifact.count }.by(-1) + expect(Ci::JobArtifact.where(id: artifact_under_refresh_1.id)).to exist + expect(Ci::JobArtifact.where(id: artifact_under_refresh_2.id)).to exist + expect(Ci::JobArtifact.where(id: artifact_under_refresh_3.id)).to exist + end + + it_behaves_like 'avoiding N+1 queries' + end + end + context 'ProjectStatistics' do it 'resets project statistics' do expect(ProjectStatistics).to receive(:increment_statistic).once diff --git a/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb index 98b01e2b303..403afde5da3 100644 --- a/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb +++ b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb @@ -4,17 +4,14 @@ require 'spec_helper' RSpec.describe ::Ci::PipelineArtifacts::CoverageReportService do describe '#execute' do - subject { described_class.new.execute(pipeline) } + let_it_be(:project) { create(:project, :repository) } - context 'when pipeline has coverage reports' do - let(:project) { create(:project, :repository) } - let(:pipeline) { create(:ci_pipeline, :with_coverage_reports, project: project) } + subject { described_class.new(pipeline).execute } + shared_examples 'creating a pipeline coverage report' do context 'when pipeline is finished' do it 'creates a pipeline artifact' do - subject - - expect(Ci::PipelineArtifact.count).to eq(1) + expect { subject }.to change { Ci::PipelineArtifact.count }.from(0).to(1) end it 'persists the default file name' do @@ -37,21 +34,32 @@ RSpec.describe ::Ci::PipelineArtifacts::CoverageReportService do end context 'when pipeline artifact has already been created' do - it 'do not raise an error and do not persist the same artifact twice' do - expect { 2.times { described_class.new.execute(pipeline) } }.not_to raise_error + it 'does not raise an error and does not persist the same artifact twice' do + expect { 2.times { described_class.new(pipeline).execute } }.not_to raise_error expect(Ci::PipelineArtifact.count).to eq(1) end end end + context 'when pipeline has coverage report' do + let!(:pipeline) { create(:ci_pipeline, :with_coverage_reports, project: project) } + + it_behaves_like 'creating a pipeline coverage report' + end + + context 'when pipeline has coverage report from child pipeline' do + let!(:pipeline) { create(:ci_pipeline, :success, project: project) } + let!(:child_pipeline) { create(:ci_pipeline, :with_coverage_reports, project: project, child_of: pipeline) } + + it_behaves_like 'creating a pipeline coverage report' + end + context 'when pipeline is running and coverage report does not exist' do let(:pipeline) { create(:ci_pipeline, :running) } it 'does not persist data' do - subject - - expect(Ci::PipelineArtifact.count).to eq(0) + expect { subject }.not_to change { Ci::PipelineArtifact.count } end end end diff --git a/spec/services/ci/process_sync_events_service_spec.rb b/spec/services/ci/process_sync_events_service_spec.rb index 6b9717fe57d..241ac4995ff 100644 --- a/spec/services/ci/process_sync_events_service_spec.rb +++ b/spec/services/ci/process_sync_events_service_spec.rb @@ -137,10 +137,9 @@ RSpec.describe Ci::ProcessSyncEventsService do end end - context 'when the FFs sync_traversal_ids, use_traversal_ids and use_traversal_ids_for_ancestors are disabled' do + context 'when the FFs use_traversal_ids and use_traversal_ids_for_ancestors are disabled' do before do - stub_feature_flags(sync_traversal_ids: false, - use_traversal_ids: false, + stub_feature_flags(use_traversal_ids: false, use_traversal_ids_for_ancestors: false) end |