diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-04-20 23:50:22 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-04-20 23:50:22 +0000 |
commit | 9dc93a4519d9d5d7be48ff274127136236a3adb3 (patch) | |
tree | 70467ae3692a0e35e5ea56bcb803eb512a10bedb /spec/services/ci | |
parent | 4b0f34b6d759d6299322b3a54453e930c6121ff0 (diff) | |
download | gitlab-ce-9dc93a4519d9d5d7be48ff274127136236a3adb3.tar.gz |
Add latest changes from gitlab-org/gitlab@13-11-stable-eev13.11.0-rc43
Diffstat (limited to 'spec/services/ci')
39 files changed, 834 insertions, 247 deletions
diff --git a/spec/services/ci/abort_pipelines_service_spec.rb b/spec/services/ci/abort_pipelines_service_spec.rb new file mode 100644 index 00000000000..e31a45cb123 --- /dev/null +++ b/spec/services/ci/abort_pipelines_service_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::AbortPipelinesService do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, namespace: user.namespace) } + + let_it_be(:cancelable_pipeline, reload: true) { create(:ci_pipeline, :running, project: project, user: user) } + let_it_be(:manual_pipeline, reload: true) { create(:ci_pipeline, status: :manual, project: project, user: user) } # not cancelable + let_it_be(:other_users_pipeline, reload: true) { create(:ci_pipeline, :running, project: project, user: create(:user)) } # not this user's pipeline + let_it_be(:cancelable_build, reload: true) { create(:ci_build, :running, pipeline: cancelable_pipeline) } + let_it_be(:non_cancelable_build, reload: true) { create(:ci_build, :success, pipeline: cancelable_pipeline) } + let_it_be(:cancelable_stage, reload: true) { create(:ci_stage_entity, name: 'stageA', status: :running, pipeline: cancelable_pipeline, project: project) } + let_it_be(:non_cancelable_stage, reload: true) { create(:ci_stage_entity, name: 'stageB', status: :success, pipeline: cancelable_pipeline, project: project) } + + describe '#execute' do + def expect_correct_cancellations + expect(cancelable_pipeline.finished_at).not_to be_nil + expect(cancelable_pipeline.status).to eq('failed') + expect((cancelable_pipeline.stages - [non_cancelable_stage]).map(&:status)).to all(eq('failed')) + expect(cancelable_build.status).to eq('failed') + expect(cancelable_build.finished_at).not_to be_nil + + expect(manual_pipeline.status).not_to eq('failed') + expect(non_cancelable_stage.status).not_to eq('failed') + expect(non_cancelable_build.status).not_to eq('failed') + end + + context 'with project pipelines' do + def abort_project_pipelines + described_class.new.execute(project.all_pipelines, :project_deleted) + end + + it 'fails all running pipelines and related jobs' do + expect(abort_project_pipelines).to be_success + + expect_correct_cancellations + + expect(other_users_pipeline.status).to eq('failed') + expect(other_users_pipeline.failure_reason).to eq('project_deleted') + expect(other_users_pipeline.stages.map(&:status)).to all(eq('failed')) + end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new { abort_project_pipelines }.count + + pipelines = create_list(:ci_pipeline, 5, :running, project: project) + create_list(:ci_build, 5, :running, pipeline: pipelines.first) + + expect { abort_project_pipelines }.not_to exceed_query_limit(control_count) + end + + context 'with live build logs' do + before do + create(:ci_build_trace_chunk, build: cancelable_build) + end + + it 'makes failed builds with stale trace visible' do + expect(Ci::Build.with_stale_live_trace.count).to eq 0 + + travel_to(2.days.ago) do + abort_project_pipelines + end + + expect(Ci::Build.with_stale_live_trace.count).to eq 1 + 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/abort_project_pipelines_service_spec.rb b/spec/services/ci/abort_project_pipelines_service_spec.rb deleted file mode 100644 index 9af909ac2ab..00000000000 --- a/spec/services/ci/abort_project_pipelines_service_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Ci::AbortProjectPipelinesService do - let_it_be(:project) { create(:project) } - let_it_be(:pipeline) { create(:ci_pipeline, :running, project: project) } - let_it_be(:build) { create(:ci_build, :running, pipeline: pipeline) } - - describe '#execute' do - it 'cancels all running pipelines and related jobs' do - result = described_class.new.execute(project) - - expect(result).to be_success - expect(pipeline.reload).to be_canceled - expect(build.reload).to be_canceled - end - - it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new { described_class.new.execute(project) }.count - - pipelines = create_list(:ci_pipeline, 5, :running, project: project) - create_list(:ci_build, 5, :running, pipeline: pipelines.first) - - expect { described_class.new.execute(project) }.not_to exceed_query_limit(control_count) - end - end - - context 'when feature disabled' do - before do - stub_feature_flags(abort_deleted_project_pipelines: false) - end - - it 'does not abort the pipeline' do - result = described_class.new.execute(project) - - expect(result).to be(nil) - expect(pipeline.reload).to be_running - expect(build.reload).to be_running - 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 new file mode 100644 index 00000000000..a2147759dba --- /dev/null +++ b/spec/services/ci/after_requeue_job_service_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::AfterRequeueJobService do + let_it_be(:project) { create(:project) } + let_it_be(:user) { project.owner } + + let(:pipeline) { create(:ci_pipeline, project: project) } + + let!(:build) { create(:ci_build, pipeline: pipeline, stage_idx: 0) } + let!(:test1) { create(:ci_build, :success, pipeline: pipeline, stage_idx: 1) } + let!(:test2) { create(:ci_build, :skipped, pipeline: pipeline, stage_idx: 1) } + + subject(:execute_service) { described_class.new(project, user).execute(build) } + + it 'marks subsequent skipped jobs as processable' do + expect(test1.reload).to be_success + expect(test2.reload).to be_skipped + + execute_service + + expect(test1.reload).to be_success + expect(test2.reload).to be_created + end + + context 'when the pipeline is a downstream pipeline and the bridge is depended' do + let!(:trigger_job) { create(:ci_bridge, :strategy_depend, status: 'success') } + + before do + create(:ci_sources_pipeline, pipeline: pipeline, source_job: trigger_job) + end + + it 'marks source bridge as pending' do + expect { execute_service }.to change { trigger_job.reload.status }.from('success').to('pending') + end + end +end diff --git a/spec/services/ci/archive_trace_service_spec.rb b/spec/services/ci/archive_trace_service_spec.rb index 07ea314debc..a4f498f17c3 100644 --- a/spec/services/ci/archive_trace_service_spec.rb +++ b/spec/services/ci/archive_trace_service_spec.rb @@ -24,6 +24,52 @@ RSpec.describe Ci::ArchiveTraceService, '#execute' do it 'does not create an archived trace' do expect { subject }.not_to change { Ci::JobArtifact.trace.count } end + + context 'when live trace chunks still exist' do + before do + create(:ci_build_trace_chunk, build: job) + end + + context 'when the feature flag `erase_traces_from_already_archived_jobs_when_archiving_again` is enabled' do + before do + stub_feature_flags(erase_traces_from_already_archived_jobs_when_archiving_again: true) + end + + it 'removes the trace chunks' do + expect { subject }.to change { job.trace_chunks.count }.to(0) + end + + context 'when associated data does not exist' do + before do + job.job_artifacts_trace.file.remove! + end + + it 'removes the trace artifact' do + expect { subject }.to change { job.reload.job_artifacts_trace }.to(nil) + end + end + end + + context 'when the feature flag `erase_traces_from_already_archived_jobs_when_archiving_again` is disabled' do + before do + stub_feature_flags(erase_traces_from_already_archived_jobs_when_archiving_again: false) + end + + it 'does not remove the trace chunks' do + expect { subject }.not_to change { job.trace_chunks.count } + end + + context 'when associated data does not exist' do + before do + job.job_artifacts_trace.file.remove! + end + + it 'does not remove the trace artifact' do + expect { subject }.not_to change { job.reload.job_artifacts_trace } + end + end + end + end end context 'when job does not have trace' do diff --git a/spec/services/ci/cancel_user_pipelines_service_spec.rb b/spec/services/ci/cancel_user_pipelines_service_spec.rb deleted file mode 100644 index 8491242dfd5..00000000000 --- a/spec/services/ci/cancel_user_pipelines_service_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Ci::CancelUserPipelinesService do - describe '#execute' do - let(:user) { create(:user) } - - subject { described_class.new.execute(user) } - - context 'when user has running CI pipelines' do - let(:pipeline) { create(:ci_pipeline, :running, user: user) } - let!(:build) { create(:ci_build, :running, pipeline: pipeline) } - - it 'cancels all running pipelines and related jobs', :sidekiq_might_not_need_inline do - subject - - expect(pipeline.reload).to be_canceled - expect(build.reload).to be_canceled - end - end - - context 'when an error ocurrs' do - it 'raises a service level error' do - service = double(execute: ServiceResponse.error(message: 'Error canceling pipeline')) - allow(::Ci::CancelUserPipelinesService).to receive(:new).and_return(service) - - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - end - end - end -end diff --git a/spec/services/ci/create_downstream_pipeline_service_spec.rb b/spec/services/ci/create_downstream_pipeline_service_spec.rb index 860932d4fde..dd10fb017aa 100644 --- a/spec/services/ci/create_downstream_pipeline_service_spec.rb +++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb @@ -112,7 +112,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'updates bridge status when downstream pipeline gets processed' do pipeline = service.execute(bridge) - expect(pipeline.reload).to be_pending + expect(pipeline.reload).to be_created expect(bridge.reload).to be_success end @@ -227,7 +227,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'updates bridge status when downstream pipeline gets processed' do pipeline = service.execute(bridge) - expect(pipeline.reload).to be_pending + expect(pipeline.reload).to be_created expect(bridge.reload).to be_success end diff --git a/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb b/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb index 9cf66dfceb0..d4e9946ac46 100644 --- a/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb +++ b/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb @@ -40,6 +40,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do it 'creates bridge job with resource group' do pipeline = create_pipeline! + Ci::InitialPipelineProcessWorker.new.perform(pipeline.id) test = pipeline.statuses.find_by(name: 'instrumentation_test') expect(pipeline).to be_created_successfully diff --git a/spec/services/ci/create_pipeline_service/needs_spec.rb b/spec/services/ci/create_pipeline_service/needs_spec.rb index a6b0a9662c9..4521067cd52 100644 --- a/spec/services/ci/create_pipeline_service/needs_spec.rb +++ b/spec/services/ci/create_pipeline_service/needs_spec.rb @@ -202,7 +202,7 @@ RSpec.describe Ci::CreatePipelineService do YAML end - it 'creates a pipeline with build_a and test_b pending; deploy_b manual' do + it 'creates a pipeline with build_a and test_b pending; deploy_b manual', :sidekiq_inline do processables = pipeline.processables build_a = processables.find { |processable| processable.name == 'build_a' } 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 a3818937113..5ea75c2253b 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 @@ -91,6 +91,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do it 'creates bridge job with resource group', :aggregate_failures do pipeline = create_pipeline! + Ci::InitialPipelineProcessWorker.new.perform(pipeline.id) test = pipeline.statuses.find_by(name: 'instrumentation_test') expect(pipeline).to be_created_successfully diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index e97e74c1515..33ec6aacc44 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -151,11 +151,29 @@ RSpec.describe Ci::CreatePipelineService do context 'variables:' do let(:config) do <<-EOY - job: + variables: + VAR4: workflow var 4 + VAR5: workflow var 5 + VAR7: workflow var 7 + + workflow: + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + VAR4: overridden workflow var 4 + - if: $CI_COMMIT_REF_NAME =~ /feature/ + variables: + VAR5: overridden workflow var 5 + VAR6: new workflow var 6 + VAR7: overridden workflow var 7 + - when: always + + job1: script: "echo job1" variables: - VAR1: my var 1 - VAR2: my var 2 + VAR1: job var 1 + VAR2: job var 2 + VAR5: job var 5 rules: - if: $CI_COMMIT_REF_NAME =~ /master/ variables: @@ -164,45 +182,117 @@ RSpec.describe Ci::CreatePipelineService do variables: VAR2: overridden var 2 VAR3: new var 3 + VAR7: overridden var 7 + - when: on_success + + job2: + script: "echo job2" + inherit: + variables: [VAR4, VAR6, VAR7] + variables: + VAR4: job var 4 + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + VAR7: overridden var 7 - when: on_success EOY end - let(:job) { pipeline.builds.find_by(name: 'job') } + let(:job1) { pipeline.builds.find_by(name: 'job1') } + let(:job2) { pipeline.builds.find_by(name: 'job2') } + + let(:variable_keys) { %w(VAR1 VAR2 VAR3 VAR4 VAR5 VAR6 VAR7) } + + context 'when no match' do + let(:ref) { 'refs/heads/wip' } + + it 'does not affect vars' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['job var 1', 'job var 2', nil, 'workflow var 4', 'job var 5', nil, 'workflow var 7'] + ) + + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, nil, 'workflow var 7'] + ) + end + end context 'when matching to the first rule' do let(:ref) { 'refs/heads/master' } - it 'overrides VAR1' do - variables = job.scoped_variables.to_hash + it 'overrides variables' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['overridden var 1', 'job var 2', nil, 'overridden workflow var 4', 'job var 5', nil, 'workflow var 7'] + ) - expect(variables['VAR1']).to eq('overridden var 1') - expect(variables['VAR2']).to eq('my var 2') - expect(variables['VAR3']).to be_nil + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, nil, 'overridden var 7'] + ) + end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'does not affect workflow variables but job variables' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['overridden var 1', 'job var 2', nil, 'workflow var 4', 'job var 5', nil, 'workflow var 7'] + ) + + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, nil, 'overridden var 7'] + ) + end end end context 'when matching to the second rule' do let(:ref) { 'refs/heads/feature' } - it 'overrides VAR2 and adds VAR3' do - variables = job.scoped_variables.to_hash + it 'overrides variables' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['job var 1', 'overridden var 2', 'new var 3', 'workflow var 4', 'job var 5', 'new workflow var 6', 'overridden var 7'] + ) - expect(variables['VAR1']).to eq('my var 1') - expect(variables['VAR2']).to eq('overridden var 2') - expect(variables['VAR3']).to eq('new var 3') + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, 'new workflow var 6', 'overridden workflow var 7'] + ) end end - context 'when no match' do - let(:ref) { 'refs/heads/wip' } + context 'using calculated workflow var in job rules' do + let(:config) do + <<-EOY + variables: + VAR1: workflow var 4 + + workflow: + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + VAR1: overridden workflow var 4 + - when: always + + job: + script: "echo job1" + rules: + - if: $VAR1 =~ "overridden workflow var 4" + variables: + VAR1: overridden var 1 + - when: on_success + EOY + end - it 'does not affect vars' do - variables = job.scoped_variables.to_hash + let(:job) { pipeline.builds.find_by(name: 'job') } + + context 'when matching the first workflow condition' do + let(:ref) { 'refs/heads/master' } - expect(variables['VAR1']).to eq('my var 1') - expect(variables['VAR2']).to eq('my var 2') - expect(variables['VAR3']).to be_nil + it 'uses VAR1 of job rules result' do + expect(job.scoped_variables.to_hash['VAR1']).to eq('overridden var 1') + end end end end @@ -230,8 +320,8 @@ RSpec.describe Ci::CreatePipelineService do end context 'matching the first rule in the list' do - it 'saves a pending pipeline' do - expect(pipeline).to be_pending + it 'saves a created pipeline' do + expect(pipeline).to be_created expect(pipeline).to be_persisted end end @@ -239,8 +329,8 @@ RSpec.describe Ci::CreatePipelineService do context 'matching the last rule in the list' do let(:ref) { 'refs/heads/feature' } - it 'saves a pending pipeline' do - expect(pipeline).to be_pending + it 'saves a created pipeline' do + expect(pipeline).to be_created expect(pipeline).to be_persisted end end @@ -280,8 +370,8 @@ RSpec.describe Ci::CreatePipelineService do end context 'matching the first rule in the list' do - it 'saves a pending pipeline' do - expect(pipeline).to be_pending + it 'saves a created pipeline' do + expect(pipeline).to be_created expect(pipeline).to be_persisted end end @@ -305,8 +395,8 @@ RSpec.describe Ci::CreatePipelineService do context 'with partial match' do let(:ref) { 'refs/heads/feature' } - it 'saves a pending pipeline' do - expect(pipeline).to be_pending + it 'saves a created pipeline' do + expect(pipeline).to be_created expect(pipeline).to be_persisted end end @@ -349,8 +439,8 @@ RSpec.describe Ci::CreatePipelineService do context 'where workflow passes and the job passes' do let(:ref) { 'refs/heads/feature' } - it 'saves a pending pipeline' do - expect(pipeline).to be_pending + it 'saves a created pipeline' do + expect(pipeline).to be_created expect(pipeline).to be_persisted end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 9fafc57a770..98c85234fe7 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -63,7 +63,7 @@ RSpec.describe Ci::CreatePipelineService do expect(pipeline).to be_push expect(pipeline).to eq(project.ci_pipelines.last) expect(pipeline).to have_attributes(user: user) - expect(pipeline).to have_attributes(status: 'pending') + expect(pipeline).to have_attributes(status: 'created') expect(pipeline.iid).not_to be_nil expect(pipeline.repository_source?).to be true expect(pipeline.builds.first).to be_kind_of(Ci::Build) @@ -71,19 +71,21 @@ RSpec.describe Ci::CreatePipelineService do end it 'increments the prometheus counter' do - expect(Gitlab::Metrics).to receive(:counter) - .with(:pipelines_created_total, "Counter of pipelines created") - .and_call_original - allow(Gitlab::Metrics).to receive(:counter).and_call_original # allow other counters + counter = spy('pipeline created counter') + + allow(Gitlab::Ci::Pipeline::Metrics) + .to receive(:pipelines_created_counter).and_return(counter) pipeline + + expect(counter).to have_received(:increment) end it 'records pipeline size in a prometheus histogram' do histogram = spy('pipeline size histogram') allow(Gitlab::Ci::Pipeline::Metrics) - .to receive(:new).and_return(histogram) + .to receive(:pipeline_size_histogram).and_return(histogram) execute_service @@ -253,7 +255,7 @@ RSpec.describe Ci::CreatePipelineService do pipeline pipeline_on_previous_commit - expect(pipeline.reload).to have_attributes(status: 'pending', auto_canceled_by_id: nil) + expect(pipeline.reload).to have_attributes(status: 'created', auto_canceled_by_id: nil) end it 'auto cancel pending non-HEAD pipelines', :sidekiq_might_not_need_inline do @@ -263,8 +265,8 @@ RSpec.describe Ci::CreatePipelineService do expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) end - it 'cancels running outdated pipelines', :sidekiq_might_not_need_inline do - pipeline_on_previous_commit.run + it 'cancels running outdated pipelines', :sidekiq_inline do + pipeline_on_previous_commit.reload.run head_pipeline = execute_service expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: head_pipeline.id) @@ -278,13 +280,13 @@ RSpec.describe Ci::CreatePipelineService do end it 'does not cancel pipelines from the other branches' do - pending_pipeline = execute_service( + new_pipeline = execute_service( ref: 'refs/heads/feature', after: previous_commit_sha_from_ref('feature') ) pipeline - expect(pending_pipeline.reload).to have_attributes(status: 'pending', auto_canceled_by_id: nil) + expect(new_pipeline.reload).to have_attributes(status: 'created', auto_canceled_by_id: nil) end context 'when the interruptible attribute is' do @@ -465,12 +467,12 @@ RSpec.describe Ci::CreatePipelineService do project.update!(auto_cancel_pending_pipelines: 'disabled') end - it 'does not auto cancel pending non-HEAD pipelines' do + it 'does not auto cancel created non-HEAD pipelines' do pipeline_on_previous_commit pipeline expect(pipeline_on_previous_commit.reload) - .to have_attributes(status: 'pending', auto_canceled_by_id: nil) + .to have_attributes(status: 'created', auto_canceled_by_id: nil) end end @@ -580,6 +582,13 @@ RSpec.describe Ci::CreatePipelineService do it_behaves_like 'a failed pipeline' + it 'increments the error metric' do + stub_ci_pipeline_yaml_file(ci_yaml) + + counter = Gitlab::Metrics.counter(:gitlab_ci_pipeline_failure_reasons, 'desc') + expect { execute_service }.to change { counter.get(reason: 'config_error') }.by(1) + end + context 'when receive git commit' do before do allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message } @@ -770,7 +779,7 @@ RSpec.describe Ci::CreatePipelineService do stub_ci_pipeline_yaml_file(config) end - it 'does not create a new pipeline' do + it 'does not create a new pipeline', :sidekiq_inline do result = execute_service expect(result).to be_persisted diff --git a/spec/services/ci/create_web_ide_terminal_service_spec.rb b/spec/services/ci/create_web_ide_terminal_service_spec.rb index c1c94e30018..c1acf8fd60c 100644 --- a/spec/services/ci/create_web_ide_terminal_service_spec.rb +++ b/spec/services/ci/create_web_ide_terminal_service_spec.rb @@ -66,6 +66,25 @@ RSpec.describe Ci::CreateWebIdeTerminalService do it_behaves_like 'be successful' end + + context 'for configuration with variables' do + let(:config_content) do + <<-EOS + terminal: + script: rspec + variables: + KEY1: VAL1 + EOS + end + + it_behaves_like 'be successful' + + it 'saves the variables' do + expect(subject[:pipeline].builds[0].variables).to include( + key: 'KEY1', value: 'VAL1', public: true, masked: false + ) + end + end end end diff --git a/spec/services/ci/disable_user_pipeline_schedules_service_spec.rb b/spec/services/ci/disable_user_pipeline_schedules_service_spec.rb new file mode 100644 index 00000000000..4ff8dcf075b --- /dev/null +++ b/spec/services/ci/disable_user_pipeline_schedules_service_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::DisableUserPipelineSchedulesService do + describe '#execute' do + let(:user) { create(:user) } + + subject(:service) { described_class.new.execute(user) } + + context 'when user has active pipeline schedules' do + let(:owned_pipeline_schedule) { create(:ci_pipeline_schedule, active: true, owner: user) } + + it 'disables all active pipeline schedules', :aggregate_failures do + expect { service }.to change { owned_pipeline_schedule.reload.active? } + end + end + end +end diff --git a/spec/services/ci/drop_pipeline_service_spec.rb b/spec/services/ci/drop_pipeline_service_spec.rb new file mode 100644 index 00000000000..4adbb99b9e2 --- /dev/null +++ b/spec/services/ci/drop_pipeline_service_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::DropPipelineService do + let_it_be(:user) { create(:user) } + + let(:failure_reason) { :user_blocked } + + let!(:cancelable_pipeline) { create(:ci_pipeline, :running, user: user) } + let!(:running_build) { create(:ci_build, :running, pipeline: cancelable_pipeline) } + let!(:success_pipeline) { create(:ci_pipeline, :success, user: user) } + let!(:success_build) { create(:ci_build, :success, pipeline: success_pipeline) } + + describe '#execute_async_for_all' do + subject { described_class.new.execute_async_for_all(user.pipelines, failure_reason, user) } + + it 'drops only cancelable pipelines asynchronously', :sidekiq_inline do + subject + + expect(cancelable_pipeline.reload).to be_failed + expect(running_build.reload).to be_failed + + expect(success_pipeline.reload).to be_success + expect(success_build.reload).to be_success + end + end + + describe '#execute' do + subject { described_class.new.execute(cancelable_pipeline.id, failure_reason) } + + def drop_pipeline!(pipeline) + described_class.new.execute(pipeline, failure_reason) + end + + it 'drops each cancelable build in the pipeline', :aggregate_failures do + drop_pipeline!(cancelable_pipeline) + + expect(running_build.reload).to be_failed + expect(running_build.failure_reason).to eq(failure_reason.to_s) + + expect(success_build.reload).to be_success + end + + it 'avoids N+1 queries when reading data' do + control_count = ActiveRecord::QueryRecorder.new do + drop_pipeline!(cancelable_pipeline) + end.count + + writes_per_build = 2 + expected_reads_count = control_count - writes_per_build + + create_list(:ci_build, 5, :running, pipeline: cancelable_pipeline) + + expect do + drop_pipeline!(cancelable_pipeline) + end.not_to exceed_query_limit(expected_reads_count + (5 * writes_per_build)) + end + end +end diff --git a/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb b/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb index e2bdfae27f0..0cbeaa5446b 100644 --- a/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb +++ b/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb @@ -34,7 +34,7 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do expect(subject).to eq(project.ci_pipelines.last) expect(subject.external_pull_request).to eq(pull_request) expect(subject.user).to eq(user) - expect(subject.status).to eq('pending') + expect(subject.status).to eq('created') expect(subject.ref).to eq(pull_request.source_branch) expect(subject.sha).to eq(pull_request.source_sha) expect(subject.source_sha).to eq(pull_request.source_sha) diff --git a/spec/services/ci/create_job_artifacts_service_spec.rb b/spec/services/ci/job_artifacts/create_service_spec.rb index 1efd1d390a2..22aa9e62c6f 100644 --- a/spec/services/ci/create_job_artifacts_service_spec.rb +++ b/spec/services/ci/job_artifacts/create_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreateJobArtifactsService do +RSpec.describe Ci::JobArtifacts::CreateService do let_it_be(:project) { create(:project) } let(:service) { described_class.new(job) } let(:job) { create(:ci_build, project: project) } diff --git a/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb index d315dd35632..04fa55068f2 100644 --- a/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared_state do +RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_shared_state do include ExclusiveLeaseHelpers let(:service) { described_class.new } @@ -24,7 +24,7 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared job = create(:ci_build, pipeline: artifact.job.pipeline) create(:ci_job_artifact, :archive, :expired, job: job) - stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_LIMIT', 1) + stub_const("#{described_class}::LOOP_LIMIT", 1) end it 'performs the smallest number of queries for job_artifacts' do @@ -113,7 +113,7 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared context 'when failed to destroy artifact' do before do - stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_LIMIT', 10) + stub_const("#{described_class}::LOOP_LIMIT", 10) end context 'when the import fails' do @@ -159,8 +159,8 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared let!(:second_artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) } before do - stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_TIMEOUT', 0.seconds) - stub_const('Ci::DestroyExpiredJobArtifactsService::BATCH_SIZE', 1) + stub_const("#{described_class}::LOOP_TIMEOUT", 0.seconds) + stub_const("#{described_class}::BATCH_SIZE", 1) second_artifact.job.pipeline.unlocked! end @@ -176,8 +176,8 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared context 'when loop reached loop limit' do before do - stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_LIMIT', 1) - stub_const('Ci::DestroyExpiredJobArtifactsService::BATCH_SIZE', 1) + stub_const("#{described_class}::LOOP_LIMIT", 1) + stub_const("#{described_class}::BATCH_SIZE", 1) second_artifact.job.pipeline.unlocked! end @@ -209,7 +209,7 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared context 'when there are artifacts more than batch sizes' do before do - stub_const('Ci::DestroyExpiredJobArtifactsService::BATCH_SIZE', 1) + stub_const("#{described_class}::BATCH_SIZE", 1) second_artifact.job.pipeline.unlocked! end 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 74fbbf28ef1..52aaf73d67e 100644 --- a/spec/services/ci/job_artifacts_destroy_batch_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::JobArtifactsDestroyBatchService do +RSpec.describe Ci::JobArtifacts::DestroyBatchService do include ExclusiveLeaseHelpers let(:artifacts) { Ci::JobArtifact.all } diff --git a/spec/services/ci/pipeline_artifacts/destroy_expired_artifacts_service_spec.rb b/spec/services/ci/pipeline_artifacts/destroy_all_expired_service_spec.rb index ac1a590face..3dc4f35df22 100644 --- a/spec/services/ci/pipeline_artifacts/destroy_expired_artifacts_service_spec.rb +++ b/spec/services/ci/pipeline_artifacts/destroy_all_expired_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::PipelineArtifacts::DestroyExpiredArtifactsService do +RSpec.describe Ci::PipelineArtifacts::DestroyAllExpiredService do let(:service) { described_class.new } describe '.execute' do @@ -10,7 +10,7 @@ RSpec.describe Ci::PipelineArtifacts::DestroyExpiredArtifactsService do context 'when timeout happens' do before do - stub_const('Ci::PipelineArtifacts::DestroyExpiredArtifactsService::LOOP_TIMEOUT', 0.1.seconds) + stub_const('Ci::PipelineArtifacts::DestroyAllExpiredService::LOOP_TIMEOUT', 0.1.seconds) allow(service).to receive(:destroy_artifacts_batch) { true } end @@ -27,8 +27,8 @@ RSpec.describe Ci::PipelineArtifacts::DestroyExpiredArtifactsService do context 'when the loop limit is reached' do before do - stub_const('::Ci::PipelineArtifacts::DestroyExpiredArtifactsService::LOOP_LIMIT', 1) - stub_const('::Ci::PipelineArtifacts::DestroyExpiredArtifactsService::BATCH_SIZE', 1) + stub_const('::Ci::PipelineArtifacts::DestroyAllExpiredService::LOOP_LIMIT', 1) + stub_const('::Ci::PipelineArtifacts::DestroyAllExpiredService::BATCH_SIZE', 1) create_list(:ci_pipeline_artifact, 2, expire_at: 1.week.ago) end @@ -44,7 +44,7 @@ RSpec.describe Ci::PipelineArtifacts::DestroyExpiredArtifactsService do context 'when there are artifacts more than batch sizes' do before do - stub_const('Ci::PipelineArtifacts::DestroyExpiredArtifactsService::BATCH_SIZE', 1) + stub_const('Ci::PipelineArtifacts::DestroyAllExpiredService::BATCH_SIZE', 1) create_list(:ci_pipeline_artifact, 2, expire_at: 1.week.ago) end 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 a9f9db8c689..572808cd2db 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 @@ -49,7 +49,13 @@ RSpec.shared_context 'Pipeline Processing Service Tests With Yaml' do 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}!") } + statuses.each do |status| + if event == 'play' + status.play(user) + else + status.public_send("#{event}!") + end + end end end end diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_build_test_two_manual_review_test_staging_production.yml b/spec/services/ci/pipeline_processing/test_cases/dag_build_test_two_manual_review_test_staging_production.yml new file mode 100644 index 00000000000..ed009ee4f25 --- /dev/null +++ b/spec/services/ci/pipeline_processing/test_cases/dag_build_test_two_manual_review_test_staging_production.yml @@ -0,0 +1,171 @@ +config: + stages: [build, test, review, deploy, post_deploy] + + build: + stage: build + script: exit 0 + + test: + stage: test + script: exit 0 + + release_test1: + stage: test + when: manual + script: exit 0 + + release_test2: + stage: test + when: manual + script: exit 0 + + review: + stage: review + script: exit 0 + needs: [test, release_test1, release_test2] + + staging: + stage: deploy + script: exit 0 + needs: [release_test1] + + production: + stage: deploy + script: exit 0 + needs: [release_test2] + + after_deploy: + stage: post_deploy + script: exit 0 + needs: [production] + + handle_failure: + stage: post_deploy + when: on_failure + script: exit 0 + +init: + expect: + pipeline: pending + stages: + build: pending + test: created + review: created + deploy: created + post_deploy: created + jobs: + build: pending + test: created + release_test1: created + release_test2: created + review: created + staging: created + production: created + after_deploy: created + handle_failure: created + +transitions: + - event: success + jobs: [build] + expect: + pipeline: running + stages: + build: success + test: pending + review: skipped + deploy: skipped + post_deploy: pending + jobs: + build: success + test: pending + release_test1: manual + release_test2: manual + review: skipped + staging: skipped + production: skipped + after_deploy: skipped + handle_failure: created + + - event: success + jobs: [test] + expect: + pipeline: success + stages: + build: success + test: success + review: skipped + deploy: skipped + post_deploy: skipped + jobs: + build: success + test: success + release_test1: manual + release_test2: manual + review: skipped + staging: skipped + production: skipped + after_deploy: skipped + handle_failure: skipped + + - event: play + jobs: [release_test1] + expect: + pipeline: running + stages: + build: success + test: running + review: skipped + deploy: pending + post_deploy: pending + jobs: + build: success + test: success + release_test1: pending + release_test2: manual + review: skipped + staging: created + production: skipped + after_deploy: skipped + handle_failure: created + + - event: success + jobs: [release_test1] + expect: + pipeline: running + stages: + build: success + test: success + review: skipped + deploy: pending + post_deploy: pending + jobs: + build: success + test: success + release_test1: success + release_test2: manual + review: skipped + staging: pending + production: skipped + after_deploy: skipped + handle_failure: created + + - event: success + jobs: [staging] + expect: + pipeline: success + stages: + build: success + test: success + review: skipped + deploy: success + post_deploy: skipped + jobs: + build: success + test: success + release_test1: success + release_test2: manual + review: skipped + staging: success + production: skipped + after_deploy: skipped + handle_failure: skipped diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false.yml b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false.yml index 1d61cd24f8c..7987f4568a4 100644 --- a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false.yml +++ b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false.yml @@ -21,7 +21,7 @@ init: deploy: created transitions: - - event: enqueue + - event: play jobs: [test] expect: pipeline: pending diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false_deploy_always.yml b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false_deploy_always.yml index bb8723aa303..ea7f0f06c50 100644 --- a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false_deploy_always.yml +++ b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false_deploy_always.yml @@ -22,7 +22,7 @@ init: deploy: created transitions: - - event: enqueue + - event: play jobs: [test] expect: pipeline: pending diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false_deploy_on_failure.yml b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false_deploy_on_failure.yml index 3099a94befb..5c839ebc0e9 100644 --- a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false_deploy_on_failure.yml +++ b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_false_deploy_on_failure.yml @@ -22,7 +22,7 @@ init: deploy: created transitions: - - event: enqueue + - event: play jobs: [test] expect: pipeline: pending diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true_deploy_on_failure.yml b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true_deploy_on_failure.yml index 81aad4940b6..2d379f2d7c5 100644 --- a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true_deploy_on_failure.yml +++ b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true_deploy_on_failure.yml @@ -22,7 +22,7 @@ init: deploy: skipped transitions: - - event: enqueue + - event: play jobs: [test] expect: pipeline: pending @@ -31,7 +31,7 @@ transitions: deploy: skipped jobs: test: pending - deploy: skipped + deploy: created - event: drop jobs: [test] diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true.yml b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true_fails.yml index b8fcdd1566a..fbe04c7e18e 100644 --- a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true.yml +++ b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true_fails.yml @@ -21,7 +21,7 @@ init: deploy: skipped transitions: - - event: enqueue + - event: play jobs: [test] expect: pipeline: pending @@ -30,7 +30,7 @@ transitions: deploy: skipped jobs: test: pending - deploy: skipped + deploy: created - event: run jobs: [test] @@ -41,15 +41,26 @@ transitions: deploy: skipped jobs: test: running - deploy: skipped + deploy: created - event: drop jobs: [test] expect: + pipeline: running + stages: + test: success + deploy: pending + jobs: + test: failed + deploy: pending + + - event: success + jobs: [deploy] + expect: pipeline: success stages: test: success - deploy: skipped + deploy: success jobs: test: failed - deploy: skipped + deploy: success diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true_succeeds.yml b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true_succeeds.yml new file mode 100644 index 00000000000..68ef057f62d --- /dev/null +++ b/spec/services/ci/pipeline_processing/test_cases/dag_test_manual_allow_failure_true_succeeds.yml @@ -0,0 +1,66 @@ +config: + test: + stage: test + when: manual + allow_failure: true + script: exit 0 + + deploy: + stage: deploy + script: exit 0 + needs: [test] + +init: + expect: + pipeline: skipped + stages: + test: skipped + deploy: skipped + jobs: + test: manual + deploy: skipped + +transitions: + - event: play + jobs: [test] + expect: + pipeline: pending + stages: + test: pending + deploy: skipped + jobs: + test: pending + deploy: created + + - event: run + jobs: [test] + expect: + pipeline: running + stages: + test: running + deploy: skipped + jobs: + test: running + deploy: created + + - event: success + jobs: [test] + expect: + pipeline: running + stages: + test: success + deploy: pending + jobs: + test: success + deploy: pending + + - event: success + jobs: [deploy] + expect: + pipeline: success + stages: + test: success + deploy: success + jobs: + test: success + deploy: success diff --git a/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_false.yml b/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_false.yml index 2ffa35b56d7..759b4d0ae75 100644 --- a/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_false.yml +++ b/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_false.yml @@ -20,7 +20,7 @@ init: deploy: created transitions: - - event: enqueue + - event: play jobs: [test] expect: pipeline: pending diff --git a/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_true.yml b/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_true.yml index 088fab5ca09..93eecae8fcf 100644 --- a/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_true.yml +++ b/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_true.yml @@ -31,7 +31,7 @@ transitions: test: manual deploy: success - - event: enqueue + - event: play jobs: [test] expect: pipeline: running diff --git a/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_true_deploy_on_failure.yml b/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_true_deploy_on_failure.yml index 2b30316aef6..301f9631845 100644 --- a/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_true_deploy_on_failure.yml +++ b/spec/services/ci/pipeline_processing/test_cases/stage_test_manual_allow_failure_true_deploy_on_failure.yml @@ -21,7 +21,7 @@ init: deploy: skipped transitions: - - event: enqueue + - event: play jobs: [test] expect: pipeline: pending @@ -30,7 +30,7 @@ transitions: deploy: skipped jobs: test: pending - deploy: skipped + deploy: created - event: drop jobs: [test] diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb index 89d3da89011..36055779a2e 100644 --- a/spec/services/ci/pipeline_trigger_service_spec.rb +++ b/spec/services/ci/pipeline_trigger_service_spec.rb @@ -55,17 +55,6 @@ RSpec.describe Ci::PipelineTriggerService do expect(var.variable_type).to eq('file') end - context 'when FF ci_trigger_payload_into_pipeline is disabled' do - before do - stub_feature_flags(ci_trigger_payload_into_pipeline: false) - end - - it 'does not store the payload as a variable' do - expect { result }.not_to change { Ci::PipelineVariable.count } - expect(result[:pipeline].variables).to be_empty - end - end - context 'when commit message has [ci skip]' do before do allow_next(Ci::Pipeline).to receive(:git_commit_message) { '[ci skip]' } diff --git a/spec/services/ci/play_bridge_service_spec.rb b/spec/services/ci/play_bridge_service_spec.rb index 0482ad4d76f..d6130325b5a 100644 --- a/spec/services/ci/play_bridge_service_spec.rb +++ b/spec/services/ci/play_bridge_service_spec.rb @@ -35,6 +35,28 @@ RSpec.describe Ci::PlayBridgeService, '#execute' do expect(bridge.reload.user).to eq(user) end + context 'when a subsequent job is skipped' do + let!(:job) { create(:ci_build, :skipped, pipeline: pipeline, stage_idx: bridge.stage_idx + 1) } + + before do + create(:ci_build_need, build: job, name: bridge.name) + end + + it 'marks the subsequent job as processable' do + expect { execute_service }.to change { job.reload.status }.from('skipped').to('created') + end + + context 'when the FF ci_fix_pipeline_status_for_dag_needs_manual is disabled' do + before do + stub_feature_flags(ci_fix_pipeline_status_for_dag_needs_manual: false) + end + + it 'does not change the subsequent job' do + expect { execute_service }.not_to change { job.reload.status }.from('skipped') + end + end + end + context 'when bridge is not playable' do let(:bridge) { create(:ci_bridge, :failed, pipeline: pipeline, downstream: downstream_project) } diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb index 00c6de7681d..78de91675f9 100644 --- a/spec/services/ci/play_build_service_spec.rb +++ b/spec/services/ci/play_build_service_spec.rb @@ -61,6 +61,28 @@ RSpec.describe Ci::PlayBuildService, '#execute' do expect(build.reload.user).to eq user end + context 'when a subsequent job is skipped' do + let!(:job) { create(:ci_build, :skipped, pipeline: pipeline, stage_idx: build.stage_idx + 1) } + + before do + create(:ci_build_need, build: job, name: build.name) + end + + it 'marks the subsequent job as processable' do + expect { service.execute(build) }.to change { job.reload.status }.from('skipped').to('created') + end + + context 'when the FF ci_fix_pipeline_status_for_dag_needs_manual is disabled' do + before do + stub_feature_flags(ci_fix_pipeline_status_for_dag_needs_manual: false) + end + + it 'does not change the subsequent job' do + expect { service.execute(build) }.not_to change { job.reload.status }.from('skipped') + end + end + end + context 'when variables are supplied' do let(:job_variables) do [{ key: 'first', secret_value: 'first' }, diff --git a/spec/services/ci/process_build_service_spec.rb b/spec/services/ci/process_build_service_spec.rb index 42a92504839..b54fc45d36a 100644 --- a/spec/services/ci/process_build_service_spec.rb +++ b/spec/services/ci/process_build_service_spec.rb @@ -145,28 +145,5 @@ RSpec.describe Ci::ProcessBuildService, '#execute' do expect { subject }.to change { build.status }.to(after_status) end end - - context 'when FF skip_dag_manual_and_delayed_jobs is disabled on the project' do - let_it_be(:other_project) { create(:project) } - - before do - stub_feature_flags(skip_dag_manual_and_delayed_jobs: other_project) - end - - where(:build_when, :current_status, :after_status) do - :on_success | 'success' | 'pending' - :on_success | 'skipped' | 'skipped' - :manual | 'success' | 'manual' - :manual | 'skipped' | 'manual' - :delayed | 'success' | 'manual' - :delayed | 'skipped' | 'manual' - end - - with_them do - it 'proceeds the build' do - expect { subject }.to change { build.status }.to(after_status) - end - end - end end end diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index e02536fd07f..254bd19c808 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -10,6 +10,14 @@ RSpec.describe Ci::ProcessPipelineService do create(:ci_empty_pipeline, ref: 'master', project: project) end + let(:pipeline_processing_events_counter) { double(increment: true) } + let(:legacy_update_jobs_counter) { double(increment: true) } + + let(:metrics) do + double(pipeline_processing_events_counter: pipeline_processing_events_counter, + legacy_update_jobs_counter: legacy_update_jobs_counter) + end + subject { described_class.new(pipeline) } before do @@ -17,22 +25,13 @@ RSpec.describe Ci::ProcessPipelineService do stub_not_protect_default_branch project.add_developer(user) + + allow(subject).to receive(:metrics).and_return(metrics) end describe 'processing events counter' do - let(:metrics) { double('pipeline metrics') } - let(:counter) { double('events counter') } - - before do - allow(subject) - .to receive(:metrics).and_return(metrics) - allow(metrics) - .to receive(:pipeline_processing_events_counter) - .and_return(counter) - end - it 'increments processing events counter' do - expect(counter).to receive(:increment) + expect(pipeline_processing_events_counter).to receive(:increment) subject.execute end @@ -64,33 +63,22 @@ RSpec.describe Ci::ProcessPipelineService do expect(all_builds.retried).to contain_exactly(build_retried) end - context 'counter ci_legacy_update_jobs_as_retried_total' do - let(:counter) { double(increment: true) } + it 'increments the counter' do + expect(legacy_update_jobs_counter).to receive(:increment) + subject.execute + end + + context 'when the previous build has already retried column true' do before do - allow(Gitlab::Metrics).to receive(:counter).and_call_original - allow(Gitlab::Metrics).to receive(:counter) - .with(:ci_legacy_update_jobs_as_retried_total, anything) - .and_return(counter) + build_retried.update_columns(retried: true) end - it 'increments the counter' do - expect(counter).to receive(:increment) + it 'does not increment the counter' do + expect(legacy_update_jobs_counter).not_to receive(:increment) subject.execute end - - context 'when the previous build has already retried column true' do - before do - build_retried.update_columns(retried: true) - end - - it 'does not increment the counter' do - expect(counter).not_to receive(:increment) - - subject.execute - end - end end end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 9187dd4f300..02b48e8ba06 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -225,6 +225,28 @@ module Ci end end + context 'when the use_distinct_in_register_job_object_hierarchy feature flag is enabled' do + before do + stub_feature_flags(use_distinct_in_register_job_object_hierarchy: true) + stub_feature_flags(use_distinct_for_all_object_hierarchy: true) + end + + it 'calls DISTINCT' do + expect(described_class.new(group_runner).send(:builds_for_group_runner).to_sql).to include("DISTINCT") + end + end + + context 'when the use_distinct_in_register_job_object_hierarchy feature flag is disabled' do + before do + stub_feature_flags(use_distinct_in_register_job_object_hierarchy: false) + stub_feature_flags(use_distinct_for_all_object_hierarchy: false) + end + + it 'does not call DISTINCT' do + expect(described_class.new(group_runner).send(:builds_for_group_runner).to_sql).not_to include("DISTINCT") + end + end + context 'group runner' do let(:build) { execute(group_runner) } @@ -593,9 +615,22 @@ module Ci create(:ci_build, pipeline: pipeline, tag_list: %w[non-matching]) end - it "observes queue size of only matching jobs" do + it 'observes queue size of only matching jobs' do # pending_job + 2 x matching ones - expect(Gitlab::Ci::Queue::Metrics.queue_size_total).to receive(:observe).with({}, 3) + expect(Gitlab::Ci::Queue::Metrics.queue_size_total).to receive(:observe) + .with({ runner_type: specific_runner.runner_type }, 3) + + expect(execute(specific_runner)).to eq(pending_job) + end + + it 'observes queue processing time by the runner type' do + expect(Gitlab::Ci::Queue::Metrics.queue_iteration_duration_seconds) + .to receive(:observe) + .with({ runner_type: specific_runner.runner_type }, anything) + + expect(Gitlab::Ci::Queue::Metrics.queue_retrieval_duration_seconds) + .to receive(:observe) + .with({ runner_type: specific_runner.runner_type }, anything) expect(execute(specific_runner)).to eq(pending_job) end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index bdf60bb3fdc..7dd3d963e56 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -181,7 +181,7 @@ RSpec.describe Ci::RetryBuildService do end it 'resolves todos for old build that failed' do - expect(MergeRequests::AddTodoWhenBuildFailsService) + expect(::MergeRequests::AddTodoWhenBuildFailsService) .to receive_message_chain(:new, :close) service.execute(build) diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index 3c6a99efbf8..3e2e9f07723 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -272,7 +272,7 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do end it 'closes all todos about failed jobs for pipeline' do - expect(MergeRequests::AddTodoWhenBuildFailsService) + expect(::MergeRequests::AddTodoWhenBuildFailsService) .to receive_message_chain(:new, :close_all) service.execute(pipeline) diff --git a/spec/services/ci/test_failure_history_service_spec.rb b/spec/services/ci/test_failure_history_service_spec.rb index d9c1c8dc3fa..c19df6e217b 100644 --- a/spec/services/ci/test_failure_history_service_spec.rb +++ b/spec/services/ci/test_failure_history_service_spec.rb @@ -11,15 +11,15 @@ RSpec.describe Ci::TestFailureHistoryService, :aggregate_failures do context 'when pipeline has failed builds with test reports' do before do - # The test report has 2 test case failures + # The test report has 2 unit test failures create(:ci_build, :failed, :test_reports, pipeline: pipeline, project: project) end - it 'creates test case failures records' do + it 'creates unit test failures records' do execute_service - expect(Ci::TestCase.count).to eq(2) - expect(Ci::TestCaseFailure.count).to eq(2) + expect(Ci::UnitTest.count).to eq(2) + expect(Ci::UnitTestFailure.count).to eq(2) end context 'when pipeline is not for the default branch' do @@ -30,8 +30,8 @@ RSpec.describe Ci::TestFailureHistoryService, :aggregate_failures do it 'does not persist data' do execute_service - expect(Ci::TestCase.count).to eq(0) - expect(Ci::TestCaseFailure.count).to eq(0) + expect(Ci::UnitTest.count).to eq(0) + expect(Ci::UnitTestFailure.count).to eq(0) end end @@ -43,12 +43,12 @@ RSpec.describe Ci::TestFailureHistoryService, :aggregate_failures do it 'does not fail but does not persist new data' do expect { described_class.new(pipeline).execute }.not_to raise_error - expect(Ci::TestCase.count).to eq(2) - expect(Ci::TestCaseFailure.count).to eq(2) + expect(Ci::UnitTest.count).to eq(2) + expect(Ci::UnitTestFailure.count).to eq(2) end end - context 'when number of failed test cases exceed the limit' do + context 'when number of failed unit tests exceed the limit' do before do stub_const("#{described_class.name}::MAX_TRACKABLE_FAILURES", 1) end @@ -56,16 +56,16 @@ RSpec.describe Ci::TestFailureHistoryService, :aggregate_failures do it 'does not persist data' do execute_service - expect(Ci::TestCase.count).to eq(0) - expect(Ci::TestCaseFailure.count).to eq(0) + expect(Ci::UnitTest.count).to eq(0) + expect(Ci::UnitTestFailure.count).to eq(0) end end - context 'when number of failed test cases across multiple builds exceed the limit' do + context 'when number of failed unit tests across multiple builds exceed the limit' do before do stub_const("#{described_class.name}::MAX_TRACKABLE_FAILURES", 2) - # This other test report has 1 unique test case failure which brings us to 3 total failures across all builds + # This other test report has 1 unique unit test failure which brings us to 3 total failures across all builds # thus exceeding the limit of 2 for MAX_TRACKABLE_FAILURES create(:ci_build, :failed, :test_reports_with_duplicate_failed_test_names, pipeline: pipeline, project: project) end @@ -73,23 +73,23 @@ RSpec.describe Ci::TestFailureHistoryService, :aggregate_failures do it 'does not persist data' do execute_service - expect(Ci::TestCase.count).to eq(0) - expect(Ci::TestCaseFailure.count).to eq(0) + expect(Ci::UnitTest.count).to eq(0) + expect(Ci::UnitTestFailure.count).to eq(0) end end end - context 'when test failure data have duplicates within the same payload (happens when the JUnit report has duplicate test case names but have different failures)' do + context 'when test failure data have duplicates within the same payload (happens when the JUnit report has duplicate unit test names but have different failures)' do before do - # The test report has 2 test case failures but with the same test case keys + # The test report has 2 unit test failures but with the same unit test keys create(:ci_build, :failed, :test_reports_with_duplicate_failed_test_names, pipeline: pipeline, project: project) end it 'does not fail but does not persist duplicate data' do expect { execute_service }.not_to raise_error - expect(Ci::TestCase.count).to eq(1) - expect(Ci::TestCaseFailure.count).to eq(1) + expect(Ci::UnitTest.count).to eq(1) + expect(Ci::UnitTestFailure.count).to eq(1) end end @@ -102,8 +102,8 @@ RSpec.describe Ci::TestFailureHistoryService, :aggregate_failures do it 'does not persist data' do execute_service - expect(Ci::TestCase.count).to eq(0) - expect(Ci::TestCaseFailure.count).to eq(0) + expect(Ci::UnitTest.count).to eq(0) + expect(Ci::UnitTestFailure.count).to eq(0) end end end |