From 48aff82709769b098321c738f3444b9bdaa694c6 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 21 Oct 2020 07:08:36 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-5-stable-ee --- .../ci/build_report_result_service_spec.rb | 21 +++- .../ci/create_downstream_pipeline_service_spec.rb | 14 --- .../ci/create_pipeline_service/cache_spec.rb | 15 ++- .../parent_child_pipeline_spec.rb | 61 +++++++++- spec/services/ci/create_pipeline_service_spec.rb | 27 +---- spec/services/ci/delete_objects_service_spec.rb | 133 +++++++++++++++++++++ .../destroy_expired_job_artifacts_service_spec.rb | 13 +- .../ci/expire_pipeline_cache_service_spec.rb | 2 + .../ci/list_config_variables_service_spec.rb | 77 ++++++++++++ .../shared_processing_service.rb | 10 +- .../shared_processing_service_tests_with_yaml.rb | 4 +- .../ci/pipelines/create_artifact_service_spec.rb | 10 -- spec/services/ci/play_bridge_service_spec.rb | 56 +++++++++ spec/services/ci/play_manual_stage_service_spec.rb | 42 +++++-- spec/services/ci/retry_build_service_spec.rb | 37 +++--- .../services/ci/update_build_queue_service_spec.rb | 16 +-- .../services/ci/update_build_state_service_spec.rb | 109 +++++++++++++++-- 17 files changed, 530 insertions(+), 117 deletions(-) create mode 100644 spec/services/ci/delete_objects_service_spec.rb create mode 100644 spec/services/ci/list_config_variables_service_spec.rb create mode 100644 spec/services/ci/play_bridge_service_spec.rb (limited to 'spec/services/ci') diff --git a/spec/services/ci/build_report_result_service_spec.rb b/spec/services/ci/build_report_result_service_spec.rb index 70bcf74ba43..134b662a72a 100644 --- a/spec/services/ci/build_report_result_service_spec.rb +++ b/spec/services/ci/build_report_result_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::BuildReportResultService do - describe "#execute" do + describe '#execute', :clean_gitlab_redis_shared_state do subject(:build_report_result) { described_class.new.execute(build) } context 'when build is finished' do @@ -17,6 +17,25 @@ RSpec.describe Ci::BuildReportResultService do expect(build_report_result.tests_skipped).to eq(0) expect(build_report_result.tests_duration).to eq(0.010284) expect(Ci::BuildReportResult.count).to eq(1) + + unique_test_cases_parsed = Gitlab::UsageDataCounters::HLLRedisCounter.unique_events( + event_names: described_class::EVENT_NAME, + start_date: 2.weeks.ago, + end_date: 2.weeks.from_now + ) + expect(unique_test_cases_parsed).to eq(4) + end + + context 'when feature flag for tracking is disabled' do + before do + stub_feature_flags(track_unique_test_cases_parsed: false) + end + + it 'creates the report but does not track the event' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + expect(build_report_result.tests_name).to eq("test") + expect(Ci::BuildReportResult.count).to eq(1) + end end context 'when data has already been persisted' do diff --git a/spec/services/ci/create_downstream_pipeline_service_spec.rb b/spec/services/ci/create_downstream_pipeline_service_spec.rb index a6ea30e4703..0cc380439a7 100644 --- a/spec/services/ci/create_downstream_pipeline_service_spec.rb +++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb @@ -325,20 +325,6 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do expect(bridge.reload).to be_success end - - context 'when FF ci_child_of_child_pipeline is disabled' do - before do - stub_feature_flags(ci_child_of_child_pipeline: false) - end - - it 'does not create a further child pipeline' do - expect { service.execute(bridge) } - .not_to change { Ci::Pipeline.count } - - expect(bridge.reload).to be_failed - expect(bridge.failure_reason).to eq 'bridge_pipeline_is_child_pipeline' - end - end end context 'when upstream pipeline has a parent pipeline, which has a parent pipeline' do diff --git a/spec/services/ci/create_pipeline_service/cache_spec.rb b/spec/services/ci/create_pipeline_service/cache_spec.rb index 614e46f1b1a..1438c2e4aa0 100644 --- a/spec/services/ci/create_pipeline_service/cache_spec.rb +++ b/spec/services/ci/create_pipeline_service/cache_spec.rb @@ -36,7 +36,8 @@ RSpec.describe Ci::CreatePipelineService do 'key' => 'a-key', 'paths' => ['logs/', 'binaries/'], 'policy' => 'pull-push', - 'untracked' => true + 'untracked' => true, + 'when' => 'on_success' } expect(pipeline).to be_persisted @@ -67,7 +68,8 @@ RSpec.describe Ci::CreatePipelineService do expected = { 'key' => /[a-f0-9]{40}/, 'paths' => ['logs/'], - 'policy' => 'pull-push' + 'policy' => 'pull-push', + 'when' => 'on_success' } expect(pipeline).to be_persisted @@ -82,7 +84,8 @@ RSpec.describe Ci::CreatePipelineService do expected = { 'key' => /default/, 'paths' => ['logs/'], - 'policy' => 'pull-push' + 'policy' => 'pull-push', + 'when' => 'on_success' } expect(pipeline).to be_persisted @@ -114,7 +117,8 @@ RSpec.describe Ci::CreatePipelineService do expected = { 'key' => /\$ENV_VAR-[a-f0-9]{40}/, 'paths' => ['logs/'], - 'policy' => 'pull-push' + 'policy' => 'pull-push', + 'when' => 'on_success' } expect(pipeline).to be_persisted @@ -129,7 +133,8 @@ RSpec.describe Ci::CreatePipelineService do expected = { 'key' => /\$ENV_VAR-default/, 'paths' => ['logs/'], - 'policy' => 'pull-push' + 'policy' => 'pull-push', + 'when' => 'on_success' } expect(pipeline).to be_persisted diff --git a/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb b/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb index 016a5dfd18b..fb6cdf55be3 100644 --- a/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb +++ b/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do end shared_examples 'successful creation' do - it 'creates bridge jobs correctly' do + it 'creates bridge jobs correctly', :aggregate_failures do pipeline = create_pipeline! test = pipeline.statuses.find_by(name: 'test') @@ -221,6 +221,65 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do end end end + + context 'when including configs from a project' do + context 'when specifying all attributes' do + let(:config) do + <<~YAML + test: + script: rspec + deploy: + variables: + CROSS: downstream + stage: deploy + trigger: + include: + - project: my-namespace/my-project + file: 'path/to/child.yml' + ref: 'master' + YAML + end + + it_behaves_like 'successful creation' do + let(:expected_bridge_options) do + { + 'trigger' => { + 'include' => [ + { + 'file' => 'path/to/child.yml', + 'project' => 'my-namespace/my-project', + 'ref' => 'master' + } + ] + } + } + end + end + end + + context 'without specifying file' do + let(:config) do + <<~YAML + test: + script: rspec + deploy: + variables: + CROSS: downstream + stage: deploy + trigger: + include: + - project: my-namespace/my-project + ref: 'master' + YAML + end + + it_behaves_like 'creation failure' do + let(:expected_error) do + /include config must specify the file where to fetch the config from/ + end + end + end + end end def create_pipeline! diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index e0893ed6de3..c28c3449485 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -731,30 +731,11 @@ RSpec.describe Ci::CreatePipelineService do .and_call_original end - context 'when ci_pipeline_rewind_iid is enabled' do - before do - stub_feature_flags(ci_pipeline_rewind_iid: true) - end - - it 'rewinds iid' do - result = execute_service - - expect(result).not_to be_persisted - expect(internal_id.last_value).to eq(0) - end - end - - context 'when ci_pipeline_rewind_iid is disabled' do - before do - stub_feature_flags(ci_pipeline_rewind_iid: false) - end - - it 'does not rewind iid' do - result = execute_service + it 'rewinds iid' do + result = execute_service - expect(result).not_to be_persisted - expect(internal_id.last_value).to eq(1) - end + expect(result).not_to be_persisted + expect(internal_id.last_value).to eq(0) end end end diff --git a/spec/services/ci/delete_objects_service_spec.rb b/spec/services/ci/delete_objects_service_spec.rb new file mode 100644 index 00000000000..448f8979681 --- /dev/null +++ b/spec/services/ci/delete_objects_service_spec.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::DeleteObjectsService, :aggregate_failure do + let(:service) { described_class.new } + let(:artifact) { create(:ci_job_artifact, :archive) } + let(:data) { [artifact] } + + describe '#execute' do + before do + Ci::DeletedObject.bulk_import(data) + # We disable the check because the specs are wrapped in a transaction + allow(service).to receive(:transaction_open?).and_return(false) + end + + subject(:execute) { service.execute } + + it 'deletes records' do + expect { execute }.to change { Ci::DeletedObject.count }.by(-1) + end + + it 'deletes files' do + expect { execute }.to change { artifact.file.exists? } + end + + context 'when trying to execute without records' do + let(:data) { [] } + + it 'does not change the number of objects' do + expect { execute }.not_to change { Ci::DeletedObject.count } + end + end + + context 'when trying to remove the same file multiple times' do + let(:objects) { Ci::DeletedObject.all.to_a } + + before do + expect(service).to receive(:load_next_batch).twice.and_return(objects) + end + + it 'executes successfully' do + 2.times { expect(service.execute).to be_truthy } + end + end + + context 'with artifacts both ready and not ready for deletion' do + let(:data) { [] } + + let_it_be(:past_ready) { create(:ci_deleted_object, pick_up_at: 2.days.ago) } + let_it_be(:ready) { create(:ci_deleted_object, pick_up_at: 1.day.ago) } + + it 'skips records with pick_up_at in the future' do + not_ready = create(:ci_deleted_object, pick_up_at: 1.day.from_now) + + expect { execute }.to change { Ci::DeletedObject.count }.from(3).to(1) + expect(not_ready.reload.present?).to be_truthy + end + + it 'limits the number of records removed' do + stub_const("#{described_class}::BATCH_SIZE", 1) + + expect { execute }.to change { Ci::DeletedObject.count }.by(-1) + end + + it 'removes records in order' do + stub_const("#{described_class}::BATCH_SIZE", 1) + + execute + + expect { past_ready.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(ready.reload.present?).to be_truthy + end + + it 'updates pick_up_at timestamp' do + allow(service).to receive(:destroy_everything) + + execute + + expect(past_ready.reload.pick_up_at).to be_like_time(10.minutes.from_now) + end + + it 'does not delete objects for which file deletion has failed' do + expect(past_ready) + .to receive(:delete_file_from_storage) + .and_return(false) + + expect(service) + .to receive(:load_next_batch) + .and_return([past_ready, ready]) + + expect { execute }.to change { Ci::DeletedObject.count }.from(2).to(1) + expect(past_ready.reload.present?).to be_truthy + end + end + + context 'with an open database transaction' do + it 'raises an exception and does not remove records' do + expect(service).to receive(:transaction_open?).and_return(true) + + expect { execute } + .to raise_error(Ci::DeleteObjectsService::TransactionInProgressError) + .and change { Ci::DeletedObject.count }.by(0) + end + end + end + + describe '#remaining_batches_count' do + subject { service.remaining_batches_count(max_batch_count: 3) } + + context 'when there is less than one batch size' do + before do + Ci::DeletedObject.bulk_import(data) + end + + it { is_expected.to eq(1) } + end + + context 'when there is more than one batch size' do + before do + objects_scope = double + + expect(Ci::DeletedObject) + .to receive(:ready_for_destruction) + .and_return(objects_scope) + + expect(objects_scope).to receive(:size).and_return(110) + end + + it { is_expected.to eq(2) } + end + end +end diff --git a/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb b/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb index 1c96be42a2f..3d5329811ad 100644 --- a/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb +++ b/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb @@ -9,9 +9,10 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared subject { service.execute } let(:service) { described_class.new } - let!(:artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) } - before do + let_it_be(:artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) } + + before(:all) do artifact.job.pipeline.unlocked! end @@ -38,7 +39,9 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared end context 'when artifact is not expired' do - let!(:artifact) { create(:ci_job_artifact, expire_at: 1.day.since) } + before do + artifact.update_column(:expire_at, 1.day.since) + end it 'does not destroy expired job artifacts' do expect { subject }.not_to change { Ci::JobArtifact.count } @@ -46,7 +49,9 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared end context 'when artifact is permanent' do - let!(:artifact) { create(:ci_job_artifact, expire_at: nil) } + before do + artifact.update_column(:expire_at, nil) + end it 'does not destroy expired job artifacts' do expect { subject }.not_to change { Ci::JobArtifact.count } diff --git a/spec/services/ci/expire_pipeline_cache_service_spec.rb b/spec/services/ci/expire_pipeline_cache_service_spec.rb index b5d664947de..8df5d0bc159 100644 --- a/spec/services/ci/expire_pipeline_cache_service_spec.rb +++ b/spec/services/ci/expire_pipeline_cache_service_spec.rb @@ -26,9 +26,11 @@ RSpec.describe Ci::ExpirePipelineCacheService do project = merge_request.target_project merge_request_pipelines_path = "/#{project.full_path}/-/merge_requests/#{merge_request.iid}/pipelines.json" + merge_request_widget_path = "/#{project.full_path}/-/merge_requests/#{merge_request.iid}/cached_widget.json" allow_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch) expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(merge_request_pipelines_path) + expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(merge_request_widget_path) subject.execute(merge_request.all_pipelines.last) end diff --git a/spec/services/ci/list_config_variables_service_spec.rb b/spec/services/ci/list_config_variables_service_spec.rb new file mode 100644 index 00000000000..5cc0481768b --- /dev/null +++ b/spec/services/ci/list_config_variables_service_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::ListConfigVariablesService do + let_it_be(:project) { create(:project, :repository) } + let(:service) { described_class.new(project) } + let(:result) { YAML.dump(ci_config) } + + subject { service.execute(sha) } + + before do + stub_gitlab_ci_yml_for_sha(sha, result) + end + + context 'when sending a valid sha' do + let(:sha) { 'master' } + let(:ci_config) do + { + variables: { + KEY1: { value: 'val 1', description: 'description 1' }, + KEY2: { value: 'val 2', description: '' }, + KEY3: { value: 'val 3' }, + KEY4: 'val 4' + }, + test: { + stage: 'test', + script: 'echo' + } + } + end + + it 'returns variable list' do + expect(subject['KEY1']).to eq({ value: 'val 1', description: 'description 1' }) + expect(subject['KEY2']).to eq({ value: 'val 2', description: '' }) + expect(subject['KEY3']).to eq({ value: 'val 3', description: nil }) + expect(subject['KEY4']).to eq({ value: 'val 4', description: nil }) + end + end + + context 'when sending an invalid sha' do + let(:sha) { 'invalid-sha' } + let(:ci_config) { nil } + + it 'returns empty json' do + expect(subject).to eq({}) + end + end + + context 'when sending an invalid config' do + let(:sha) { 'master' } + let(:ci_config) do + { + variables: { + KEY1: { value: 'val 1', description: 'description 1' } + }, + test: { + stage: 'invalid', + script: 'echo' + } + } + end + + it 'returns empty result' do + expect(subject).to eq({}) + end + end + + private + + def stub_gitlab_ci_yml_for_sha(sha, result) + allow_any_instance_of(Repository) + .to receive(:gitlab_ci_yml_for) + .with(sha, '.gitlab-ci.yml') + .and_return(result) + end +end diff --git a/spec/services/ci/pipeline_processing/shared_processing_service.rb b/spec/services/ci/pipeline_processing/shared_processing_service.rb index 7de22b6a4cc..bbd7422b435 100644 --- a/spec/services/ci/pipeline_processing/shared_processing_service.rb +++ b/spec/services/ci/pipeline_processing/shared_processing_service.rb @@ -259,14 +259,14 @@ RSpec.shared_examples 'Pipeline Processing Service' do expect(builds_names_and_statuses).to eq({ 'build': 'success', 'rollout10%': 'scheduled' }) - Timecop.travel 2.minutes.from_now do + travel_to 2.minutes.from_now do enqueue_scheduled('rollout10%') end succeed_pending expect(builds_names_and_statuses).to eq({ 'build': 'success', 'rollout10%': 'success', 'rollout100%': 'scheduled' }) - Timecop.travel 2.minutes.from_now do + travel_to 2.minutes.from_now do enqueue_scheduled('rollout100%') end succeed_pending @@ -330,7 +330,7 @@ RSpec.shared_examples 'Pipeline Processing Service' do expect(builds_names_and_statuses).to eq({ 'build': 'success', 'rollout10%': 'scheduled' }) - Timecop.travel 2.minutes.from_now do + travel_to 2.minutes.from_now do enqueue_scheduled('rollout10%') end fail_running_or_pending @@ -398,7 +398,7 @@ RSpec.shared_examples 'Pipeline Processing Service' do expect(process_pipeline).to be_truthy expect(builds_names_and_statuses).to eq({ 'delayed1': 'scheduled', 'delayed2': 'scheduled' }) - Timecop.travel 2.minutes.from_now do + travel_to 2.minutes.from_now do enqueue_scheduled('delayed1') end @@ -419,7 +419,7 @@ RSpec.shared_examples 'Pipeline Processing Service' do expect(process_pipeline).to be_truthy expect(builds_names_and_statuses).to eq({ 'delayed': 'scheduled' }) - Timecop.travel 2.minutes.from_now do + travel_to 2.minutes.from_now do enqueue_scheduled('delayed') end fail_running_or_pending diff --git a/spec/services/ci/pipeline_processing/shared_processing_service_tests_with_yaml.rb b/spec/services/ci/pipeline_processing/shared_processing_service_tests_with_yaml.rb index 77645298bc7..2936d6fae4d 100644 --- a/spec/services/ci/pipeline_processing/shared_processing_service_tests_with_yaml.rb +++ b/spec/services/ci/pipeline_processing/shared_processing_service_tests_with_yaml.rb @@ -43,12 +43,12 @@ RSpec.shared_context 'Pipeline Processing Service Tests With Yaml' do { pipeline: pipeline.status, stages: pipeline.stages.pluck(:name, :status).to_h, - jobs: pipeline.statuses.latest.pluck(:name, :status).to_h + jobs: pipeline.latest_statuses.pluck(:name, :status).to_h } end def event_on_jobs(event, job_names) - statuses = pipeline.statuses.latest.by_name(job_names).to_a + statuses = pipeline.latest_statuses.by_name(job_names).to_a expect(statuses.count).to eq(job_names.count) # ensure that we have the same counts statuses.each { |status| status.public_send("#{event}!") } diff --git a/spec/services/ci/pipelines/create_artifact_service_spec.rb b/spec/services/ci/pipelines/create_artifact_service_spec.rb index d5e9cf83a6d..6f177889ed3 100644 --- a/spec/services/ci/pipelines/create_artifact_service_spec.rb +++ b/spec/services/ci/pipelines/create_artifact_service_spec.rb @@ -35,16 +35,6 @@ RSpec.describe ::Ci::Pipelines::CreateArtifactService do end end - context 'when feature is disabled' do - it 'does not create a pipeline artifact' do - stub_feature_flags(coverage_report_view: false) - - subject - - expect(Ci::PipelineArtifact.count).to eq(0) - end - 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(ActiveRecord::RecordNotUnique) diff --git a/spec/services/ci/play_bridge_service_spec.rb b/spec/services/ci/play_bridge_service_spec.rb new file mode 100644 index 00000000000..0482ad4d76f --- /dev/null +++ b/spec/services/ci/play_bridge_service_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PlayBridgeService, '#execute' do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:downstream_project) { create(:project) } + let(:bridge) { create(:ci_bridge, :playable, pipeline: pipeline, downstream: downstream_project) } + let(:instance) { described_class.new(project, user) } + + subject(:execute_service) { instance.execute(bridge) } + + context 'when user can run the bridge' do + before do + allow(instance).to receive(:can?).with(user, :play_job, bridge).and_return(true) + end + + it 'marks the bridge pending' do + execute_service + + expect(bridge.reload).to be_pending + end + + it 'enqueues Ci::CreateCrossProjectPipelineWorker' do + expect(::Ci::CreateCrossProjectPipelineWorker).to receive(:perform_async).with(bridge.id) + + execute_service + end + + it "updates bridge's user" do + execute_service + + expect(bridge.reload.user).to eq(user) + end + + context 'when bridge is not playable' do + let(:bridge) { create(:ci_bridge, :failed, pipeline: pipeline, downstream: downstream_project) } + + it 'raises StateMachines::InvalidTransition' do + expect { execute_service }.to raise_error StateMachines::InvalidTransition + end + end + end + + context 'when user can not run the bridge' do + before do + allow(instance).to receive(:can?).with(user, :play_job, bridge).and_return(false) + end + + it 'allows user with developer role to play a bridge' do + expect { execute_service }.to raise_error Gitlab::Access::AccessDeniedError + end + end +end diff --git a/spec/services/ci/play_manual_stage_service_spec.rb b/spec/services/ci/play_manual_stage_service_spec.rb index e30ec8bfda5..3e2a95ee975 100644 --- a/spec/services/ci/play_manual_stage_service_spec.rb +++ b/spec/services/ci/play_manual_stage_service_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Ci::PlayManualStageService, '#execute' do let(:current_user) { create(:user) } let(:pipeline) { create(:ci_pipeline, user: current_user) } let(:project) { pipeline.project } + let(:downstream_project) { create(:project) } let(:service) { described_class.new(project, current_user, pipeline: pipeline) } let(:stage_status) { 'manual' } @@ -18,40 +19,42 @@ RSpec.describe Ci::PlayManualStageService, '#execute' do before do project.add_maintainer(current_user) + downstream_project.add_maintainer(current_user) create_builds_for_stage(status: stage_status) + create_bridge_for_stage(status: stage_status) end - context 'when pipeline has manual builds' do + context 'when pipeline has manual processables' do before do service.execute(stage) end - it 'starts manual builds from pipeline' do - expect(pipeline.builds.manual.count).to eq(0) + it 'starts manual processables from pipeline' do + expect(pipeline.processables.manual.count).to eq(0) end - it 'updates manual builds' do - pipeline.builds.each do |build| - expect(build.user).to eq(current_user) + it 'updates manual processables' do + pipeline.processables.each do |processable| + expect(processable.user).to eq(current_user) end end end - context 'when pipeline has no manual builds' do + context 'when pipeline has no manual processables' do let(:stage_status) { 'failed' } before do service.execute(stage) end - it 'does not update the builds' do - expect(pipeline.builds.failed.count).to eq(3) + it 'does not update the processables' do + expect(pipeline.processables.failed.count).to eq(4) end end - context 'when user does not have permission on a specific build' do + context 'when user does not have permission on a specific processable' do before do - allow_next_instance_of(Ci::Build) do |instance| + allow_next_instance_of(Ci::Processable) do |instance| allow(instance).to receive(:play).and_raise(Gitlab::Access::AccessDeniedError) end @@ -60,12 +63,14 @@ RSpec.describe Ci::PlayManualStageService, '#execute' do it 'logs the error' do expect(Gitlab::AppLogger).to receive(:error) - .exactly(stage.builds.manual.count) + .exactly(stage.processables.manual.count) service.execute(stage) end end + private + def create_builds_for_stage(options) options.merge!({ when: 'manual', @@ -77,4 +82,17 @@ RSpec.describe Ci::PlayManualStageService, '#execute' do create_list(:ci_build, 3, options) end + + def create_bridge_for_stage(options) + options.merge!({ + when: 'manual', + pipeline: pipeline, + stage: stage.name, + stage_id: stage.id, + user: pipeline.user, + downstream: downstream_project + }) + + create(:ci_bridge, options) + end end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 51741440075..81d56a0e42a 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -3,25 +3,32 @@ require 'spec_helper' RSpec.describe Ci::RetryBuildService do - let_it_be(:user) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:developer) { create(:user) } let_it_be(:project) { create(:project, :repository) } let_it_be(:pipeline) do create(:ci_pipeline, project: project, sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0') end - let(:stage) do + let_it_be(:stage) do create(:ci_stage_entity, project: project, pipeline: pipeline, name: 'test') end - let(:build) { create(:ci_build, pipeline: pipeline, stage_id: stage.id) } + let_it_be_with_refind(:build) { create(:ci_build, pipeline: pipeline, stage_id: stage.id) } + let(:user) { developer } let(:service) do described_class.new(project, user) end + before_all do + project.add_developer(developer) + project.add_reporter(reporter) + end + clone_accessors = described_class.clone_accessors reject_accessors = @@ -39,7 +46,8 @@ RSpec.describe Ci::RetryBuildService do job_variables waiting_for_resource_at job_artifacts_metrics_referee job_artifacts_network_referee job_artifacts_dotenv job_artifacts_cobertura needs job_artifacts_accessibility - job_artifacts_requirements job_artifacts_coverage_fuzzing].freeze + job_artifacts_requirements job_artifacts_coverage_fuzzing + job_artifacts_api_fuzzing].freeze ignore_accessors = %i[type lock_version target_url base_tags trace_sections @@ -53,9 +61,9 @@ RSpec.describe Ci::RetryBuildService do pipeline_id report_results pending_state pages_deployments].freeze shared_examples 'build duplication' do - let(:another_pipeline) { create(:ci_empty_pipeline, project: project) } + let_it_be(:another_pipeline) { create(:ci_empty_pipeline, project: project) } - let(:build) do + let_it_be(:build) do create(:ci_build, :failed, :expired, :erased, :queued, :coverage, :tags, :allowed_to_fail, :on_tag, :triggered, :teardown_environment, :resource_group, description: 'my-job', stage: 'test', stage_id: stage.id, @@ -63,7 +71,7 @@ RSpec.describe Ci::RetryBuildService do scheduled_at: 10.seconds.since) end - before do + before_all do # Test correctly behaviour of deprecated artifact because it can be still in use stub_feature_flags(drop_license_management_artifact: false) @@ -81,8 +89,6 @@ RSpec.describe Ci::RetryBuildService do create(:ci_job_variable, job: build) create(:ci_build_need, build: build) - - build.reload end describe 'clone accessors' do @@ -154,7 +160,7 @@ RSpec.describe Ci::RetryBuildService do describe '#execute' do let(:new_build) do - Timecop.freeze(1.second.from_now) do + travel_to(1.second.from_now) do service.execute(build) end end @@ -162,8 +168,6 @@ RSpec.describe Ci::RetryBuildService do context 'when user has ability to execute build' do before do stub_not_protect_default_branch - - project.add_developer(user) end it_behaves_like 'build duplication' @@ -235,7 +239,6 @@ RSpec.describe Ci::RetryBuildService do context 'when the pipeline is a child pipeline and the bridge is depended' do let!(:parent_pipeline) { create(:ci_pipeline, project: project) } - let!(:pipeline) { create(:ci_pipeline, project: project) } let!(:bridge) { create(:ci_bridge, :strategy_depend, pipeline: parent_pipeline, status: 'success') } let!(:source_pipeline) { create(:ci_sources_pipeline, pipeline: pipeline, source_job: bridge) } @@ -248,6 +251,8 @@ RSpec.describe Ci::RetryBuildService do end context 'when user does not have ability to execute build' do + let(:user) { reporter } + it 'raises an error' do expect { service.execute(build) } .to raise_error Gitlab::Access::AccessDeniedError @@ -257,7 +262,7 @@ RSpec.describe Ci::RetryBuildService do describe '#reprocess' do let(:new_build) do - Timecop.freeze(1.second.from_now) do + travel_to(1.second.from_now) do service.reprocess!(build) end end @@ -265,8 +270,6 @@ RSpec.describe Ci::RetryBuildService do context 'when user has ability to execute build' do before do stub_not_protect_default_branch - - project.add_developer(user) end it_behaves_like 'build duplication' @@ -316,6 +319,8 @@ RSpec.describe Ci::RetryBuildService do end context 'when user does not have ability to execute build' do + let(:user) { reporter } + it 'raises an error' do expect { service.reprocess!(build) } .to raise_error Gitlab::Access::AccessDeniedError diff --git a/spec/services/ci/update_build_queue_service_spec.rb b/spec/services/ci/update_build_queue_service_spec.rb index 0f4c0fa5ecb..ebccfdc5140 100644 --- a/spec/services/ci/update_build_queue_service_spec.rb +++ b/spec/services/ci/update_build_queue_service_spec.rb @@ -45,21 +45,7 @@ RSpec.describe Ci::UpdateBuildQueueService do runner.update!(contacted_at: Ci::Runner.recent_queue_deadline) end - context 'when ci_update_queues_for_online_runners is enabled' do - before do - stub_feature_flags(ci_update_queues_for_online_runners: true) - end - - it_behaves_like 'does not refresh runner' - end - - context 'when ci_update_queues_for_online_runners is disabled' do - before do - stub_feature_flags(ci_update_queues_for_online_runners: false) - end - - it_behaves_like 'refreshes runner' - end + it_behaves_like 'does not refresh runner' end end diff --git a/spec/services/ci/update_build_state_service_spec.rb b/spec/services/ci/update_build_state_service_spec.rb index f5ad732bf7e..2545909bf56 100644 --- a/spec/services/ci/update_build_state_service_spec.rb +++ b/spec/services/ci/update_build_state_service_spec.rb @@ -83,9 +83,26 @@ RSpec.describe Ci::UpdateBuildStateService do { checksum: 'crc32:12345678', state: 'failed', failure_reason: 'script_failure' } end + context 'when build does not have associated trace chunks' do + it 'updates a build status' do + result = subject.execute + + expect(build).to be_failed + expect(result.status).to eq 200 + end + + it 'does not increment invalid trace metric' do + execute_with_stubbed_metrics! + + expect(metrics) + .not_to have_received(:increment_trace_operation) + .with(operation: :invalid) + end + end + context 'when build trace has been migrated' do before do - create(:ci_build_trace_chunk, :database_with_data, build: build) + create(:ci_build_trace_chunk, :persisted, build: build, initial_data: 'abcd') end it 'updates a build state' do @@ -100,6 +117,12 @@ RSpec.describe Ci::UpdateBuildStateService do expect(result.status).to eq 200 end + it 'does not set a backoff value' do + result = subject.execute + + expect(result.backoff).to be_nil + end + it 'increments trace finalized operation metric' do execute_with_stubbed_metrics! @@ -107,6 +130,60 @@ RSpec.describe Ci::UpdateBuildStateService do .to have_received(:increment_trace_operation) .with(operation: :finalized) end + + it 'records migration duration in a histogram' do + freeze_time do + create(:ci_build_pending_state, build: build, created_at: 0.5.seconds.ago) + + execute_with_stubbed_metrics! + end + + expect(metrics) + .to have_received(:observe_migration_duration) + .with(0.5) + end + + context 'when trace checksum is not valid' do + it 'increments invalid trace metric' do + execute_with_stubbed_metrics! + + expect(metrics) + .to have_received(:increment_trace_operation) + .with(operation: :invalid) + end + end + + context 'when trace checksum is valid' do + let(:params) { { checksum: 'crc32:ed82cd11', state: 'success' } } + + it 'does not increment invalid trace metric' do + execute_with_stubbed_metrics! + + expect(metrics) + .not_to have_received(:increment_trace_operation) + .with(operation: :invalid) + end + end + + context 'when failed to acquire a build trace lock' do + it 'accepts a state update request' do + build.trace.lock do + result = subject.execute + + expect(result.status).to eq 202 + end + end + + it 'increment locked trace metric' do + build.trace.lock do + execute_with_stubbed_metrics! + + expect(metrics) + .to have_received(:increment_trace_operation) + .with(operation: :locked) + end + end + end end context 'when build trace has not been migrated yet' do @@ -126,6 +203,12 @@ RSpec.describe Ci::UpdateBuildStateService do expect(result.status).to eq 202 end + it 'sets a request backoff value' do + result = subject.execute + + expect(result.backoff.to_i).to be > 0 + end + it 'schedules live chunks for migration' do expect(Ci::BuildTraceChunkFlushWorker) .to receive(:perform_async) @@ -134,14 +217,6 @@ RSpec.describe Ci::UpdateBuildStateService do subject.execute end - it 'increments trace accepted operation metric' do - execute_with_stubbed_metrics! - - expect(metrics) - .to have_received(:increment_trace_operation) - .with(operation: :accepted) - end - it 'creates a pending state record' do subject.execute @@ -153,6 +228,22 @@ RSpec.describe Ci::UpdateBuildStateService do end end + it 'increments trace accepted operation metric' do + execute_with_stubbed_metrics! + + expect(metrics) + .to have_received(:increment_trace_operation) + .with(operation: :accepted) + end + + it 'does not increment invalid trace metric' do + execute_with_stubbed_metrics! + + expect(metrics) + .not_to have_received(:increment_trace_operation) + .with(operation: :invalid) + end + context 'when build pending state is outdated' do before do build.create_pending_state( -- cgit v1.2.3