diff options
Diffstat (limited to 'spec/services/ci')
18 files changed, 667 insertions, 51 deletions
diff --git a/spec/services/ci/append_build_trace_service_spec.rb b/spec/services/ci/append_build_trace_service_spec.rb new file mode 100644 index 00000000000..a0a7f594881 --- /dev/null +++ b/spec/services/ci/append_build_trace_service_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::AppendBuildTraceService do + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be(:build) { create(:ci_build, :running, pipeline: pipeline) } + + before do + stub_feature_flags(ci_enable_live_trace: true) + end + + context 'build trace append is successful' do + it 'returns a correct stream size and status code' do + stream_size = 192.kilobytes + body_data = 'x' * stream_size + content_range = "0-#{stream_size}" + + result = described_class + .new(build, content_range: content_range) + .execute(body_data) + + expect(result.status).to eq 202 + expect(result.stream_size).to eq stream_size + expect(build.trace_chunks.count).to eq 2 + end + end + + context 'when could not correctly append to a trace' do + it 'responds with content range violation and data stored' do + allow(build).to receive_message_chain(:trace, :append) { 16 } + + result = described_class + .new(build, content_range: '0-128') + .execute('x' * 128) + + expect(result.status).to eq 416 + expect(result.stream_size).to eq 16 + end + + it 'logs exception if build has live trace' do + build.trace.append('abcd', 0) + + expect(::Gitlab::ErrorTracking) + .to receive(:log_exception) + .with(anything, hash_including(chunk_index: 0, chunk_store: 'redis')) + + result = described_class + .new(build, content_range: '0-128') + .execute('x' * 128) + + expect(result.status).to eq 416 + expect(result.stream_size).to eq 4 + end + end +end diff --git a/spec/services/ci/build_report_result_service_spec.rb b/spec/services/ci/build_report_result_service_spec.rb index 134b662a72a..7c2702af086 100644 --- a/spec/services/ci/build_report_result_service_spec.rb +++ b/spec/services/ci/build_report_result_service_spec.rb @@ -26,18 +26,6 @@ RSpec.describe Ci::BuildReportResultService do 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 it 'raises an error and do not persist the same data twice' do expect { 2.times { described_class.new.execute(build) } }.to raise_error(ActiveRecord::RecordNotUnique) diff --git a/spec/services/ci/compare_test_reports_service_spec.rb b/spec/services/ci/compare_test_reports_service_spec.rb index 7d31db73b6a..377c801b008 100644 --- a/spec/services/ci/compare_test_reports_service_spec.rb +++ b/spec/services/ci/compare_test_reports_service_spec.rb @@ -7,15 +7,15 @@ RSpec.describe Ci::CompareTestReportsService do let(:project) { create(:project, :repository) } describe '#execute' do - subject { service.execute(base_pipeline, head_pipeline) } + subject(:comparison) { service.execute(base_pipeline, head_pipeline) } context 'when head pipeline has test reports' do let!(:base_pipeline) { nil } let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports, project: project) } it 'returns status and data' do - expect(subject[:status]).to eq(:parsed) - expect(subject[:data]).to match_schema('entities/test_reports_comparer') + expect(comparison[:status]).to eq(:parsed) + expect(comparison[:data]).to match_schema('entities/test_reports_comparer') end end @@ -24,8 +24,8 @@ RSpec.describe Ci::CompareTestReportsService do let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports, project: project) } it 'returns status and data' do - expect(subject[:status]).to eq(:parsed) - expect(subject[:data]).to match_schema('entities/test_reports_comparer') + expect(comparison[:status]).to eq(:parsed) + expect(comparison[:data]).to match_schema('entities/test_reports_comparer') end end @@ -39,9 +39,44 @@ RSpec.describe Ci::CompareTestReportsService do end it 'returns a parsed TestReports success status and failure on the individual suite' do - expect(subject[:status]).to eq(:parsed) - expect(subject.dig(:data, 'status')).to eq('success') - expect(subject.dig(:data, 'suites', 0, 'status') ).to eq('error') + expect(comparison[:status]).to eq(:parsed) + expect(comparison.dig(:data, 'status')).to eq('success') + expect(comparison.dig(:data, 'suites', 0, 'status') ).to eq('error') + end + end + + context 'test failure history' do + let!(:base_pipeline) { nil } + let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports_with_three_failures, project: project) } + + let(:new_failures) do + comparison.dig(:data, 'suites', 0, 'new_failures') + end + + let(:recent_failures_per_test_case) do + new_failures.map { |f| f['recent_failures'] } + end + + # Create test case failure records based on the head pipeline build + before do + stub_const("Gitlab::Ci::Reports::TestSuiteComparer::DEFAULT_MAX_TESTS", 2) + stub_const("Gitlab::Ci::Reports::TestSuiteComparer::DEFAULT_MIN_TESTS", 1) + + build = head_pipeline.builds.last + build.update_column(:finished_at, 1.day.ago) # Just to be sure we are included in the report window + + # The JUnit fixture for the given build has 3 failures. + # This service will create 1 test case failure record for each. + Ci::TestCasesService.new.execute(build) + end + + it 'loads recent failures on limited test cases to avoid building up a huge DB query', :aggregate_failures do + expect(comparison[:data]).to match_schema('entities/test_reports_comparer') + expect(recent_failures_per_test_case).to eq([ + { 'count' => 1, 'base_branch' => 'master' }, + { 'count' => 1, 'base_branch' => 'master' } + ]) + expect(new_failures.count).to eq(2) 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 0cc380439a7..03cea4074bf 100644 --- a/spec/services/ci/create_downstream_pipeline_service_spec.rb +++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb @@ -581,5 +581,40 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do ) end end + + context 'when downstream pipeline has workflow rule' do + before do + stub_ci_pipeline_yaml_file(config) + end + + let(:config) do + <<-EOY + workflow: + rules: + - if: $my_var + + regular-job: + script: 'echo Hello, World!' + EOY + end + + context 'when passing the required variable' do + before do + bridge.yaml_variables = [{ key: 'my_var', value: 'var', public: true }] + end + + it 'creates the pipeline' do + expect { service.execute(bridge) }.to change(downstream_project.ci_pipelines, :count).by(1) + + expect(bridge.reload).to be_success + end + end + + context 'when not passing the required variable' do + it 'does not create the pipeline' do + expect { service.execute(bridge) }.not_to change(downstream_project.ci_pipelines, :count) + end + end + end end end diff --git a/spec/services/ci/create_pipeline_service/cache_spec.rb b/spec/services/ci/create_pipeline_service/cache_spec.rb index 1438c2e4aa0..5f74c2f1cef 100644 --- a/spec/services/ci/create_pipeline_service/cache_spec.rb +++ b/spec/services/ci/create_pipeline_service/cache_spec.rb @@ -4,13 +4,13 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do context 'cache' do - let(:user) { create(:admin) } + let(:project) { create(:project, :custom_repo, files: files) } + let(:user) { project.owner } let(:ref) { 'refs/heads/master' } let(:source) { :push } let(:service) { described_class.new(project, user, { ref: ref }) } let(:pipeline) { service.execute(source) } let(:job) { pipeline.builds.find_by(name: 'job') } - let(:project) { create(:project, :custom_repo, files: files) } before do stub_ci_pipeline_yaml_file(config) diff --git a/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb b/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb index b5b3832ac00..9ccf289df7c 100644 --- a/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb +++ b/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb @@ -4,8 +4,8 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do describe 'creation errors and warnings' do - let_it_be(:user) { create(:admin) } - let_it_be(:project) { create(:project, :repository, creator: user) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { project.owner } let(:ref) { 'refs/heads/master' } let(:source) { :push } diff --git a/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb b/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb index 122870e0f3a..6320a16d646 100644 --- a/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb +++ b/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { create(:admin) } + let_it_be(:user) { project.owner } let(:ref) { 'refs/heads/master' } let(:service) { described_class.new(project, user, { ref: ref }) } diff --git a/spec/services/ci/create_pipeline_service/dry_run_spec.rb b/spec/services/ci/create_pipeline_service/dry_run_spec.rb index 93378df80f0..60c56ed0f67 100644 --- a/spec/services/ci/create_pipeline_service/dry_run_spec.rb +++ b/spec/services/ci/create_pipeline_service/dry_run_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { create(:admin) } + let_it_be(:user) { project.owner } let(:ref) { 'refs/heads/master' } let(:service) { described_class.new(project, user, { ref: ref }) } diff --git a/spec/services/ci/create_pipeline_service/needs_spec.rb b/spec/services/ci/create_pipeline_service/needs_spec.rb index 915dc46d664..512091035a2 100644 --- a/spec/services/ci/create_pipeline_service/needs_spec.rb +++ b/spec/services/ci/create_pipeline_service/needs_spec.rb @@ -4,8 +4,8 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do context 'needs' do - let_it_be(:user) { create(:admin) } - let_it_be(:project) { create(:project, :repository, creator: user) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { project.owner } let(:ref) { 'refs/heads/master' } let(:source) { :push } @@ -14,6 +14,7 @@ RSpec.describe Ci::CreatePipelineService do before do stub_ci_pipeline_yaml_file(config) + project.add_developer(user) end context 'with a valid config' do diff --git a/spec/services/ci/create_pipeline_service/parameter_content_spec.rb b/spec/services/ci/create_pipeline_service/parameter_content_spec.rb index f656ad52ac8..90b8baa23a7 100644 --- a/spec/services/ci/create_pipeline_service/parameter_content_spec.rb +++ b/spec/services/ci/create_pipeline_service/parameter_content_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { create(:admin) } + let_it_be(:user) { project.owner } let(:service) { described_class.new(project, user, { ref: 'refs/heads/master' }) } let(:content) do <<~EOY 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 fb6cdf55be3..8df9b0c3e60 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 @@ -279,6 +279,40 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do end end end + + context 'when specifying multiple files' do + let(:config) do + <<~YAML + test: + script: rspec + deploy: + variables: + CROSS: downstream + stage: deploy + trigger: + include: + - project: my-namespace/my-project + file: + - 'path/to/child1.yml' + - 'path/to/child2.yml' + YAML + end + + it_behaves_like 'successful creation' do + let(:expected_bridge_options) do + { + 'trigger' => { + 'include' => [ + { + 'file' => ["path/to/child1.yml", "path/to/child2.yml"], + 'project' => 'my-namespace/my-project' + } + ] + } + } + end + end + end end end diff --git a/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb b/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb index 00a2dd74968..c84d9a53973 100644 --- a/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb +++ b/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do describe '.pre/.post stages' do - let_it_be(:user) { create(:admin) } - let_it_be(:project) { create(:project, :repository, creator: user) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { project.owner } let(:source) { :push } let(:service) { described_class.new(project, user, { ref: ref }) } diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index 1a1fa6e8f5d..a0ff2fff0ef 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do - let(:user) { create(:admin) } + let(:project) { create(:project, :repository) } + let(:user) { project.owner } let(:ref) { 'refs/heads/master' } let(:source) { :push } - let(:project) { create(:project, :repository) } let(:service) { described_class.new(project, user, { ref: ref }) } let(:pipeline) { service.execute(source) } let(:build_names) { pipeline.builds.pluck(:name) } diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index c28c3449485..f9015752644 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Ci::CreatePipelineService do include ProjectForksHelper let_it_be(:project, reload: true) { create(:project, :repository) } - let(:user) { create(:admin) } + let_it_be(:user, reload: true) { project.owner } let(:ref_name) { 'refs/heads/master' } before do @@ -41,7 +41,9 @@ RSpec.describe Ci::CreatePipelineService do save_on_errors: save_on_errors, trigger_request: trigger_request, merge_request: merge_request, - external_pull_request: external_pull_request) + external_pull_request: external_pull_request) do |pipeline| + yield(pipeline) if block_given? + end end # rubocop:enable Metrics/ParameterLists @@ -153,6 +155,11 @@ RSpec.describe Ci::CreatePipelineService do context 'when merge request target project is different from source project' do let!(:project) { fork_project(target_project, nil, repository: true) } let!(:target_project) { create(:project, :repository) } + let!(:user) { create(:user) } + + before do + project.add_developer(user) + end it 'updates head pipeline for merge request', :sidekiq_might_not_need_inline do merge_request = create(:merge_request, source_branch: 'feature', @@ -1440,6 +1447,11 @@ RSpec.describe Ci::CreatePipelineService do let(:ref_name) { 'refs/heads/feature' } let!(:project) { fork_project(target_project, nil, repository: true) } let!(:target_project) { create(:project, :repository) } + let!(:user) { create(:user) } + + before do + project.add_developer(user) + end it 'creates a legacy detached merge request pipeline in the forked project', :sidekiq_might_not_need_inline do expect(pipeline).to be_persisted @@ -1858,6 +1870,12 @@ RSpec.describe Ci::CreatePipelineService do - changes: - README.md allow_failure: true + + README: + script: "I use variables for changes!" + rules: + - changes: + - $CI_JOB_NAME* EOY end @@ -1867,10 +1885,10 @@ RSpec.describe Ci::CreatePipelineService do .to receive(:modified_paths).and_return(%w[README.md]) end - it 'creates two jobs' do + it 'creates five jobs' do expect(pipeline).to be_persisted expect(build_names) - .to contain_exactly('regular-job', 'rules-job', 'delayed-job', 'negligible-job') + .to contain_exactly('regular-job', 'rules-job', 'delayed-job', 'negligible-job', 'README') end it 'sets when: for all jobs' do @@ -2274,6 +2292,207 @@ RSpec.describe Ci::CreatePipelineService do end end end + + context 'with workflow rules with persisted variables' do + let(:config) do + <<-EOY + workflow: + rules: + - if: $CI_COMMIT_REF_NAME == "master" + + regular-job: + script: 'echo Hello, World!' + EOY + end + + context 'with matches' do + it 'creates a pipeline' do + expect(pipeline).to be_persisted + expect(build_names).to contain_exactly('regular-job') + end + end + + context 'with no matches' do + let(:ref_name) { 'refs/heads/feature' } + + it 'does not create a pipeline' do + expect(pipeline).not_to be_persisted + end + end + end + + context 'with workflow rules with pipeline variables' do + let(:pipeline) do + execute_service(variables_attributes: variables_attributes) + end + + let(:config) do + <<-EOY + workflow: + rules: + - if: $SOME_VARIABLE + + regular-job: + script: 'echo Hello, World!' + EOY + end + + context 'with matches' do + let(:variables_attributes) do + [{ key: 'SOME_VARIABLE', secret_value: 'SOME_VAR' }] + end + + it 'creates a pipeline' do + expect(pipeline).to be_persisted + expect(build_names).to contain_exactly('regular-job') + end + end + + context 'with no matches' do + let(:variables_attributes) { {} } + + it 'does not create a pipeline' do + expect(pipeline).not_to be_persisted + end + end + end + + context 'with workflow rules with trigger variables' do + let(:pipeline) do + execute_service do |pipeline| + pipeline.variables.build(variables) + end + end + + let(:config) do + <<-EOY + workflow: + rules: + - if: $SOME_VARIABLE + + regular-job: + script: 'echo Hello, World!' + EOY + end + + context 'with matches' do + let(:variables) do + [{ key: 'SOME_VARIABLE', secret_value: 'SOME_VAR' }] + end + + it 'creates a pipeline' do + expect(pipeline).to be_persisted + expect(build_names).to contain_exactly('regular-job') + end + + context 'when FF ci_seed_block_run_before_workflow_rules is disabled' do + before do + stub_feature_flags(ci_seed_block_run_before_workflow_rules: false) + end + + it 'does not a pipeline' do + expect(pipeline).not_to be_persisted + end + end + + context 'when a job requires the same variable' do + let(:config) do + <<-EOY + workflow: + rules: + - if: $SOME_VARIABLE + + build: + stage: build + script: 'echo build' + rules: + - if: $SOME_VARIABLE + + test1: + stage: test + script: 'echo test1' + needs: [build] + + test2: + stage: test + script: 'echo test2' + EOY + end + + it 'creates a pipeline' do + expect(pipeline).to be_persisted + expect(build_names).to contain_exactly('build', 'test1', 'test2') + end + + context 'when FF ci_seed_block_run_before_workflow_rules is disabled' do + before do + stub_feature_flags(ci_seed_block_run_before_workflow_rules: false) + end + + it 'does not a pipeline' do + expect(pipeline).not_to be_persisted + end + end + end + end + + context 'with no matches' do + let(:variables) { {} } + + it 'does not create a pipeline' do + expect(pipeline).not_to be_persisted + end + + context 'when FF ci_seed_block_run_before_workflow_rules is disabled' do + before do + stub_feature_flags(ci_seed_block_run_before_workflow_rules: false) + end + + it 'does not create a pipeline' do + expect(pipeline).not_to be_persisted + end + end + + context 'when a job requires the same variable' do + let(:config) do + <<-EOY + workflow: + rules: + - if: $SOME_VARIABLE + + build: + stage: build + script: 'echo build' + rules: + - if: $SOME_VARIABLE + + test1: + stage: test + script: 'echo test1' + needs: [build] + + test2: + stage: test + script: 'echo test2' + EOY + end + + it 'does not create a pipeline' do + expect(pipeline).not_to be_persisted + end + + context 'when FF ci_seed_block_run_before_workflow_rules is disabled' do + before do + stub_feature_flags(ci_seed_block_run_before_workflow_rules: false) + end + + it 'does not create a pipeline' do + expect(pipeline).not_to be_persisted + end + end + end + end + end end end diff --git a/spec/services/ci/daily_build_group_report_result_service_spec.rb b/spec/services/ci/daily_build_group_report_result_service_spec.rb index f196afb05e8..e54f10cc4f4 100644 --- a/spec/services/ci/daily_build_group_report_result_service_spec.rb +++ b/spec/services/ci/daily_build_group_report_result_service_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Ci::DailyBuildGroupReportResultService, '#execute' do let!(:rspec_job) { create(:ci_build, pipeline: pipeline, name: '3/3 rspec', coverage: 80) } let!(:karma_job) { create(:ci_build, pipeline: pipeline, name: '2/2 karma', coverage: 90) } let!(:extra_job) { create(:ci_build, pipeline: pipeline, name: 'extra', coverage: nil) } + let(:coverages) { Ci::DailyBuildGroupReportResult.all } it 'creates daily code coverage record for each job in the pipeline that has coverage value' do described_class.new.execute(pipeline) @@ -158,4 +159,30 @@ RSpec.describe Ci::DailyBuildGroupReportResultService, '#execute' do expect { described_class.new.execute(new_pipeline) }.not_to raise_error end end + + context 'when pipeline ref_path is the project default branch' do + let(:default_branch) { 'master' } + + before do + allow(pipeline.project).to receive(:default_branch).and_return(default_branch) + end + + it 'sets default branch to true' do + described_class.new.execute(pipeline) + + coverages.each do |coverage| + expect(coverage.default_branch).to be_truthy + end + end + end + + context 'when pipeline ref_path is not the project default branch' do + it 'sets default branch to false' do + described_class.new.execute(pipeline) + + coverages.each do |coverage| + expect(coverage.default_branch).to be_falsey + end + 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 3d5329811ad..c8d426ee657 100644 --- a/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb +++ b/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb @@ -5,26 +5,85 @@ require 'spec_helper' RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared_state do include ExclusiveLeaseHelpers + let(:service) { described_class.new } + describe '.execute' do subject { service.execute } - let(:service) { described_class.new } - - let_it_be(:artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) } + let_it_be(:artifact, reload: true) do + create(:ci_job_artifact, expire_at: 1.day.ago) + end before(:all) do artifact.job.pipeline.unlocked! end context 'when artifact is expired' do - context 'when artifact is not locked' do + context 'with preloaded relationships' do before do - artifact.job.pipeline.unlocked! + job = create(:ci_build, pipeline: artifact.job.pipeline) + create(:ci_job_artifact, :archive, :expired, job: job) + + stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_LIMIT', 1) end - it 'destroys job artifact' do + it 'performs the smallest number of queries for job_artifacts' do + log = ActiveRecord::QueryRecorder.new { subject } + + # SELECT expired ci_job_artifacts + # PRELOAD projects, routes, project_statistics + # BEGIN + # INSERT into ci_deleted_objects + # DELETE loaded ci_job_artifacts + # DELETE security_findings -- for EE + # COMMIT + expect(log.count).to be_within(1).of(8) + end + end + + context 'when artifact is not locked' do + it 'deletes job artifact record' do expect { subject }.to change { Ci::JobArtifact.count }.by(-1) end + + context 'when the artifact does not a file attached to it' do + it 'does not create deleted objects' do + expect(artifact.exists?).to be_falsy # sanity check + + expect { subject }.not_to change { Ci::DeletedObject.count } + end + end + + context 'when the artifact has a file attached to it' do + before do + artifact.file = fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip') + artifact.save! + end + + it 'creates a deleted object' do + expect { subject }.to change { Ci::DeletedObject.count }.by(1) + end + + it 'resets project statistics' do + expect(ProjectStatistics).to receive(:increment_statistic).once + .with(artifact.project, :build_artifacts_size, -artifact.file.size) + .and_call_original + + subject + end + + it 'does not remove the files' do + expect { subject }.not_to change { artifact.file.exists? } + end + + it 'reports metrics for destroyed artifacts' do + counter = service.send(:destroyed_artifacts_counter) + + expect(counter).to receive(:increment).with({}, 1).and_call_original + + subject + end + end end context 'when artifact is locked' do @@ -61,14 +120,34 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared context 'when failed to destroy artifact' do before do stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_LIMIT', 10) + end - allow_any_instance_of(Ci::JobArtifact) - .to receive(:destroy!) - .and_raise(ActiveRecord::RecordNotDestroyed) + context 'when the import fails' do + before do + expect(Ci::DeletedObject) + .to receive(:bulk_import) + .once + .and_raise(ActiveRecord::RecordNotDestroyed) + end + + it 'raises an exception and stop destroying' do + expect { subject }.to raise_error(ActiveRecord::RecordNotDestroyed) + .and not_change { Ci::JobArtifact.count }.from(1) + end end - it 'raises an exception and stop destroying' do - expect { subject }.to raise_error(ActiveRecord::RecordNotDestroyed) + context 'when the delete fails' do + before do + expect(Ci::JobArtifact) + .to receive(:id_in) + .once + .and_raise(ActiveRecord::RecordNotDestroyed) + end + + it 'raises an exception rolls back the insert' do + expect { subject }.to raise_error(ActiveRecord::RecordNotDestroyed) + .and not_change { Ci::DeletedObject.count }.from(0) + end end end @@ -85,7 +164,7 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared context 'when timeout happens' do before do stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_TIMEOUT', 1.second) - allow_any_instance_of(described_class).to receive(:destroy_batch) { true } + allow_any_instance_of(described_class).to receive(:destroy_artifacts_batch) { true } end it 'returns false and does not continue destroying' do @@ -176,4 +255,16 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared end end end + + describe '.destroy_job_artifacts_batch' do + it 'returns a falsy value without artifacts' do + expect(service.send(:destroy_job_artifacts_batch)).to be_falsy + end + end + + describe '.destroy_pipeline_artifacts_batch' do + it 'returns a falsy value without artifacts' do + expect(service.send(:destroy_pipeline_artifacts_batch)).to be_falsy + end + end end diff --git a/spec/services/ci/list_config_variables_service_spec.rb b/spec/services/ci/list_config_variables_service_spec.rb index 5cc0481768b..95c98c2b5ef 100644 --- a/spec/services/ci/list_config_variables_service_spec.rb +++ b/spec/services/ci/list_config_variables_service_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' RSpec.describe Ci::ListConfigVariablesService do - let_it_be(:project) { create(:project, :repository) } - let(:service) { described_class.new(project) } + let(:project) { create(:project, :repository) } + let(:user) { project.creator } + let(:service) { described_class.new(project, user) } let(:result) { YAML.dump(ci_config) } subject { service.execute(sha) } @@ -38,6 +39,40 @@ RSpec.describe Ci::ListConfigVariablesService do end end + context 'when config has includes' do + let(:sha) { 'master' } + let(:ci_config) do + { + include: [{ local: 'other_file.yml' }], + variables: { + KEY1: { value: 'val 1', description: 'description 1' } + }, + test: { + stage: 'test', + script: 'echo' + } + } + end + + before do + allow_next_instance_of(Repository) do |repository| + allow(repository).to receive(:blob_data_at).with(sha, 'other_file.yml') do + <<~HEREDOC + variables: + KEY2: + value: 'val 2' + description: 'description 2' + HEREDOC + end + end + 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: 'description 2' }) + end + end + context 'when sending an invalid sha' do let(:sha) { 'invalid-sha' } let(:ci_config) { nil } diff --git a/spec/services/ci/test_cases_service_spec.rb b/spec/services/ci/test_cases_service_spec.rb new file mode 100644 index 00000000000..b61d308640f --- /dev/null +++ b/spec/services/ci/test_cases_service_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::TestCasesService, :aggregate_failures do + describe '#execute' do + subject(:execute_service) { described_class.new.execute(build) } + + context 'when build has test reports' do + let(:build) { create(:ci_build, :success, :test_reports) } # The test report has 2 test case failures + + it 'creates test case failures records' do + execute_service + + expect(Ci::TestCase.count).to eq(2) + expect(Ci::TestCaseFailure.count).to eq(2) + end + + context 'when feature flag for test failure history is disabled' do + before do + stub_feature_flags(test_failure_history: false) + end + + it 'does not persist data' do + execute_service + + expect(Ci::TestCase.count).to eq(0) + expect(Ci::TestCaseFailure.count).to eq(0) + end + end + + context 'when build is not for the default branch' do + before do + build.update_column(:ref, 'new-feature') + end + + it 'does not persist data' do + execute_service + + expect(Ci::TestCase.count).to eq(0) + expect(Ci::TestCaseFailure.count).to eq(0) + end + end + + context 'when test failure data have already been persisted with the same exact attributes' do + before do + execute_service + end + + it 'does not fail but does not persist new data' do + expect { described_class.new.execute(build) }.not_to raise_error + + expect(Ci::TestCase.count).to eq(2) + expect(Ci::TestCaseFailure.count).to eq(2) + 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 + let(:build) { create(:ci_build, :success, :test_reports_with_duplicate_failed_test_names) } # The test report has 2 test case failures but with the same test case keys + + it 'does not fail but does not persist duplicate data' do + expect { described_class.new.execute(build) }.not_to raise_error + + expect(Ci::TestCase.count).to eq(1) + expect(Ci::TestCaseFailure.count).to eq(1) + end + end + + context 'when number of failed test cases exceed the limit' do + before do + stub_const("#{described_class.name}::MAX_TRACKABLE_FAILURES", 1) + end + + it 'does not persist data' do + execute_service + + expect(Ci::TestCase.count).to eq(0) + expect(Ci::TestCaseFailure.count).to eq(0) + end + end + end + + context 'when build has no test reports' do + let(:build) { create(:ci_build, :running) } + + it 'does not persist data' do + execute_service + + expect(Ci::TestCase.count).to eq(0) + expect(Ci::TestCaseFailure.count).to eq(0) + end + end + end +end |