diff options
Diffstat (limited to 'spec/models')
80 files changed, 2303 insertions, 779 deletions
diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index f0bae3f29c0..51435cc4342 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -358,7 +358,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do it 'calls .destroy_sessions' do expect(ActiveSession).to( receive(:destroy_sessions) - .with(anything, user, [active_session.public_id, rack_session.public_id, rack_session.private_id])) + .with(anything, user, [encrypted_active_session_id, rack_session.public_id, rack_session.private_id])) subject end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 4755d700d72..9a4dd2c799b 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -114,6 +114,21 @@ RSpec.describe ApplicationSetting do it { is_expected.to allow_value(nil).for(:repository_storages_weighted_default) } it { is_expected.not_to allow_value({ default: 100, shouldntexist: 50 }).for(:repository_storages_weighted) } + it { is_expected.to allow_value(400).for(:notes_create_limit) } + it { is_expected.not_to allow_value('two').for(:notes_create_limit) } + it { is_expected.not_to allow_value(nil).for(:notes_create_limit) } + it { is_expected.not_to allow_value(5.5).for(:notes_create_limit) } + it { is_expected.not_to allow_value(-2).for(:notes_create_limit) } + + def many_usernames(num = 100) + Array.new(num) { |i| "username#{i}" } + end + + it { is_expected.to allow_value(many_usernames(100)).for(:notes_create_limit_allowlist) } + it { is_expected.not_to allow_value(many_usernames(101)).for(:notes_create_limit_allowlist) } + it { is_expected.not_to allow_value(nil).for(:notes_create_limit_allowlist) } + it { is_expected.to allow_value([]).for(:notes_create_limit_allowlist) } + context 'help_page_documentation_base_url validations' do it { is_expected.to allow_value(nil).for(:help_page_documentation_base_url) } it { is_expected.to allow_value('https://docs.gitlab.com').for(:help_page_documentation_base_url) } @@ -635,28 +650,28 @@ RSpec.describe ApplicationSetting do end end - describe '#asset_proxy_whitelist' do + describe '#asset_proxy_allowlist' do context 'when given an Array' do it 'sets the domains and adds current running host' do - setting.asset_proxy_whitelist = ['example.com', 'assets.example.com'] - expect(setting.asset_proxy_whitelist).to eq(['example.com', 'assets.example.com', 'localhost']) + setting.asset_proxy_allowlist = ['example.com', 'assets.example.com'] + expect(setting.asset_proxy_allowlist).to eq(['example.com', 'assets.example.com', 'localhost']) end end context 'when given a String' do it 'sets multiple domains with spaces' do - setting.asset_proxy_whitelist = 'example.com *.example.com' - expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost']) + setting.asset_proxy_allowlist = 'example.com *.example.com' + expect(setting.asset_proxy_allowlist).to eq(['example.com', '*.example.com', 'localhost']) end it 'sets multiple domains with newlines and a space' do - setting.asset_proxy_whitelist = "example.com\n *.example.com" - expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost']) + setting.asset_proxy_allowlist = "example.com\n *.example.com" + expect(setting.asset_proxy_allowlist).to eq(['example.com', '*.example.com', 'localhost']) end it 'sets multiple domains with commas' do - setting.asset_proxy_whitelist = "example.com, *.example.com" - expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost']) + setting.asset_proxy_allowlist = "example.com, *.example.com" + expect(setting.asset_proxy_allowlist).to eq(['example.com', '*.example.com', 'localhost']) end end end @@ -949,6 +964,50 @@ RSpec.describe ApplicationSetting do end end + describe 'kroki_format_supported?' do + it 'returns true when Excalidraw is enabled' do + subject.kroki_formats_excalidraw = true + expect(subject.kroki_format_supported?('excalidraw')).to eq(true) + end + + it 'returns true when BlockDiag is enabled' do + subject.kroki_formats_blockdiag = true + # format "blockdiag" aggregates multiple diagram types: actdiag, blockdiag, nwdiag... + expect(subject.kroki_format_supported?('actdiag')).to eq(true) + expect(subject.kroki_format_supported?('blockdiag')).to eq(true) + end + + it 'returns false when BlockDiag is disabled' do + subject.kroki_formats_blockdiag = false + # format "blockdiag" aggregates multiple diagram types: actdiag, blockdiag, nwdiag... + expect(subject.kroki_format_supported?('actdiag')).to eq(false) + expect(subject.kroki_format_supported?('blockdiag')).to eq(false) + end + + it 'returns false when the diagram type is optional and not enabled' do + expect(subject.kroki_format_supported?('bpmn')).to eq(false) + end + + it 'returns true when the diagram type is enabled by default' do + expect(subject.kroki_format_supported?('vegalite')).to eq(true) + expect(subject.kroki_format_supported?('nomnoml')).to eq(true) + expect(subject.kroki_format_supported?('unknown-diagram-type')).to eq(false) + end + + it 'returns false when the diagram type is unknown' do + expect(subject.kroki_format_supported?('unknown-diagram-type')).to eq(false) + end + end + + describe 'kroki_formats' do + it 'returns the value for kroki_formats' do + subject.kroki_formats = { blockdiag: true, bpmn: false, excalidraw: true } + expect(subject.kroki_formats_blockdiag).to eq(true) + expect(subject.kroki_formats_bpmn).to eq(false) + expect(subject.kroki_formats_excalidraw).to eq(true) + end + end + it 'does not allow to set weight for non existing storage' do setting.repository_storages_weighted = { invalid_storage: 100 } diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index 0732b671729..e5ab96ca514 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -81,6 +81,37 @@ RSpec.describe BulkImports::Entity, type: :model do expect(entity.errors).to include(:parent) end end + + context 'validate destination namespace of a group_entity' do + it 'is invalid if destination namespace is the source namespace' do + group_a = create(:group, path: 'group_a') + + entity = build( + :bulk_import_entity, + :group_entity, + source_full_path: group_a.full_path, + destination_namespace: group_a.full_path + ) + + expect(entity).not_to be_valid + expect(entity.errors).to include(:destination_namespace) + end + + it 'is invalid if destination namespace is a descendant of the source' do + group_a = create(:group, path: 'group_a') + group_b = create(:group, parent: group_a, path: 'group_b') + + entity = build( + :bulk_import_entity, + :group_entity, + source_full_path: group_a.full_path, + destination_namespace: group_b.full_path + ) + + expect(entity).not_to be_valid + expect(entity.errors).to include(:destination_namespace) + end + end end describe "#update_tracker_for" do diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 4f09f6f1da4..b50e4204e0a 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -80,6 +80,14 @@ RSpec.describe Ci::Bridge do end end + it "schedules downstream pipeline creation when the status is waiting for resource" do + bridge.status = :waiting_for_resource + + expect(bridge).to receive(:schedule_downstream_pipeline!) + + bridge.enqueue_waiting_for_resource! + end + it 'raises error when the status is failed' do bridge.status = :failed diff --git a/spec/models/ci/build_dependencies_spec.rb b/spec/models/ci/build_dependencies_spec.rb index c5f56dbe5bc..e343ec0e698 100644 --- a/spec/models/ci/build_dependencies_spec.rb +++ b/spec/models/ci/build_dependencies_spec.rb @@ -18,6 +18,10 @@ RSpec.describe Ci::BuildDependencies do let!(:rubocop_test) { create(:ci_build, pipeline: pipeline, name: 'rubocop', stage_idx: 1, stage: 'test') } let!(:staging) { create(:ci_build, pipeline: pipeline, name: 'staging', stage_idx: 2, stage: 'deploy') } + before do + stub_feature_flags(ci_validate_build_dependencies_override: false) + end + describe '#local' do subject { described_class.new(job).local } @@ -360,4 +364,28 @@ RSpec.describe Ci::BuildDependencies do expect(subject).to contain_exactly(1, 2, 3, 4) end end + + describe '#valid?' do + subject { described_class.new(job).valid? } + + let(:job) { rspec_test } + + it { is_expected.to eq(true) } + + context 'when a local dependency is invalid' do + before do + build.update_column(:erased_at, Time.current) + end + + it { is_expected.to eq(false) } + + context 'when ci_validate_build_dependencies_override feature flag is enabled' do + before do + stub_feature_flags(ci_validate_build_dependencies_override: job.project) + end + + it { is_expected.to eq(true) } + end + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c2029b9240b..4ad7ce70a44 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1185,60 +1185,6 @@ RSpec.describe Ci::Build do end end - describe 'state transition with resource group' do - let(:resource_group) { create(:ci_resource_group, project: project) } - - context 'when build status is created' do - let(:build) { create(:ci_build, :created, project: project, resource_group: resource_group) } - - it 'is waiting for resource when build is enqueued' do - expect(Ci::ResourceGroups::AssignResourceFromResourceGroupWorker).to receive(:perform_async).with(resource_group.id) - - expect { build.enqueue! }.to change { build.status }.from('created').to('waiting_for_resource') - - expect(build.waiting_for_resource_at).not_to be_nil - end - - context 'when build is waiting for resource' do - before do - build.update_column(:status, 'waiting_for_resource') - end - - it 'is enqueued when build requests resource' do - expect { build.enqueue_waiting_for_resource! }.to change { build.status }.from('waiting_for_resource').to('pending') - end - - it 'releases a resource when build finished' do - expect(build.resource_group).to receive(:release_resource_from).with(build).and_call_original - expect(Ci::ResourceGroups::AssignResourceFromResourceGroupWorker).to receive(:perform_async).with(build.resource_group_id) - - build.enqueue_waiting_for_resource! - build.success! - end - - context 'when build has prerequisites' do - before do - allow(build).to receive(:any_unmet_prerequisites?) { true } - end - - it 'is preparing when build is enqueued' do - expect { build.enqueue_waiting_for_resource! }.to change { build.status }.from('waiting_for_resource').to('preparing') - end - end - - context 'when there are no available resources' do - before do - resource_group.assign_resource_to(create(:ci_build)) - end - - it 'stays as waiting for resource when build requests resource' do - expect { build.enqueue_waiting_for_resource }.not_to change { build.status } - end - end - end - end - end - describe '#on_stop' do subject { build.on_stop } @@ -1914,7 +1860,7 @@ RSpec.describe Ci::Build do subject { build.artifacts_file_for_type(file_type) } it 'queries artifacts for type' do - expect(build).to receive_message_chain(:job_artifacts, :find_by).with(file_type: Ci::JobArtifact.file_types[file_type]) + expect(build).to receive_message_chain(:job_artifacts, :find_by).with(file_type: [Ci::JobArtifact.file_types[file_type]]) subject end @@ -3605,7 +3551,7 @@ RSpec.describe Ci::Build do context 'when validates for dependencies is enabled' do before do - stub_feature_flags(ci_disable_validates_dependencies: false) + stub_feature_flags(ci_validate_build_dependencies_override: false) end let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0) } @@ -3633,7 +3579,7 @@ RSpec.describe Ci::Build do let(:options) { { dependencies: ['test'] } } before do - stub_feature_flags(ci_disable_validates_dependencies: true) + stub_feature_flags(ci_validate_build_dependencies_override: true) end it_behaves_like 'validation is not active' @@ -4096,18 +4042,6 @@ RSpec.describe Ci::Build do expect(coverage_report.files.keys).to match_array(['src/main/java/com/example/javademo/User.java']) end - - context 'and smart_cobertura_parser feature flag is disabled' do - before do - stub_feature_flags(smart_cobertura_parser: false) - end - - it 'parses blobs and add the results to the coverage report with unmodified paths' do - expect { subject }.not_to raise_error - - expect(coverage_report.files.keys).to match_array(['com/example/javademo/User.java']) - end - end end context 'when there is a corrupted Cobertura coverage report' do @@ -4922,14 +4856,6 @@ RSpec.describe Ci::Build do it_behaves_like 'drops the build without changing allow_failure' end - - context 'when ci_allow_failure_with_exit_codes is disabled' do - before do - stub_feature_flags(ci_allow_failure_with_exit_codes: false) - end - - it_behaves_like 'drops the build without changing allow_failure' - end end end diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 75ed5939724..3d728b9335e 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do it_behaves_like 'having unique enum values' before do - stub_feature_flags(ci_enable_live_trace: true) + stub_feature_flags(ci_enable_live_trace: true, gitlab_ci_trace_read_consistency: true) stub_artifacts_object_storage end diff --git a/spec/models/ci/build_trace_chunks/fog_spec.rb b/spec/models/ci/build_trace_chunks/fog_spec.rb index bc96e2584cf..d9e9533fb26 100644 --- a/spec/models/ci/build_trace_chunks/fog_spec.rb +++ b/spec/models/ci/build_trace_chunks/fog_spec.rb @@ -98,27 +98,6 @@ RSpec.describe Ci::BuildTraceChunks::Fog do expect(data_store.data(model)).to eq new_data end - - context 'when ci_live_trace_use_fog_attributes flag is disabled' do - before do - stub_feature_flags(ci_live_trace_use_fog_attributes: false) - end - - it 'does not pass along Fog attributes' do - expect_next_instance_of(Fog::AWS::Storage::Files) do |files| - expect(files).to receive(:create).with( - key: anything, - body: new_data - ).and_call_original - end - - expect(data_store.data(model)).to be_nil - - data_store.set_data(model, new_data) - - expect(data_store.data(model)).to eq new_data - end - end end end end diff --git a/spec/models/ci/daily_build_group_report_result_spec.rb b/spec/models/ci/daily_build_group_report_result_spec.rb index f16396d62c9..f6e6a6a5e02 100644 --- a/spec/models/ci/daily_build_group_report_result_spec.rb +++ b/spec/models/ci/daily_build_group_report_result_spec.rb @@ -8,6 +8,7 @@ RSpec.describe Ci::DailyBuildGroupReportResult do describe 'associations' do it { is_expected.to belong_to(:last_pipeline) } it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:group) } end describe 'validations' do @@ -83,8 +84,9 @@ RSpec.describe Ci::DailyBuildGroupReportResult do end describe 'scopes' do - let_it_be(:project) { create(:project) } - let(:recent_build_group_report_result) { create(:ci_daily_build_group_report_result, project: project) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let(:recent_build_group_report_result) { create(:ci_daily_build_group_report_result, project: project, group: group) } let(:old_build_group_report_result) do create(:ci_daily_build_group_report_result, date: 1.week.ago, project: project) end @@ -97,6 +99,43 @@ RSpec.describe Ci::DailyBuildGroupReportResult do end end + describe '.by_group' do + subject { described_class.by_group(group) } + + it 'returns records by group' do + expect(subject).to contain_exactly(recent_build_group_report_result) + end + end + + describe '.by_ref_path' do + subject(:coverages) { described_class.by_ref_path(recent_build_group_report_result.ref_path) } + + it 'returns coverages by ref_path' do + expect(coverages).to contain_exactly(recent_build_group_report_result, old_build_group_report_result) + end + end + + describe '.ordered_by_date_and_group_name' do + subject(:coverages) { described_class.ordered_by_date_and_group_name } + + it 'returns coverages ordered by data and group name' do + expect(subject).to contain_exactly(recent_build_group_report_result, old_build_group_report_result) + end + end + + describe '.by_dates' do + subject(:coverages) { described_class.by_dates(start_date, end_date) } + + context 'when daily coverages exist during those dates' do + let(:start_date) { 1.day.ago.to_date.to_s } + let(:end_date) { Date.current.to_s } + + it 'returns coverages' do + expect(coverages).to contain_exactly(recent_build_group_report_result) + end + end + end + describe '.with_coverage' do subject { described_class.with_coverage } diff --git a/spec/models/ci/pipeline_artifact_spec.rb b/spec/models/ci/pipeline_artifact_spec.rb index 8cbace845a9..3fe09f05cab 100644 --- a/spec/models/ci/pipeline_artifact_spec.rb +++ b/spec/models/ci/pipeline_artifact_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::PipelineArtifact, type: :model do - let(:coverage_report) { create(:ci_pipeline_artifact) } + let(:coverage_report) { create(:ci_pipeline_artifact, :with_coverage_report) } describe 'associations' do it { is_expected.to belong_to(:pipeline) } @@ -15,7 +15,7 @@ RSpec.describe Ci::PipelineArtifact, type: :model do it_behaves_like 'UpdateProjectStatistics' do let_it_be(:pipeline, reload: true) { create(:ci_pipeline) } - subject { build(:ci_pipeline_artifact, pipeline: pipeline) } + subject { build(:ci_pipeline_artifact, :with_code_coverage_with_multiple_files, pipeline: pipeline) } end describe 'validations' do @@ -51,7 +51,7 @@ RSpec.describe Ci::PipelineArtifact, type: :model do end describe 'file is being stored' do - subject { create(:ci_pipeline_artifact) } + subject { create(:ci_pipeline_artifact, :with_coverage_report) } context 'when existing object has local store' do it_behaves_like 'mounted file in local store' @@ -68,7 +68,7 @@ RSpec.describe Ci::PipelineArtifact, type: :model do end context 'when file contains multi-byte characters' do - let(:coverage_report_multibyte) { create(:ci_pipeline_artifact, :with_multibyte_characters) } + let(:coverage_report_multibyte) { create(:ci_pipeline_artifact, :with_coverage_multibyte_characters) } it 'sets the size in bytesize' do expect(coverage_report_multibyte.size).to eq(14) @@ -76,48 +76,118 @@ RSpec.describe Ci::PipelineArtifact, type: :model do end end - describe '.has_code_coverage?' do - subject { Ci::PipelineArtifact.has_code_coverage? } + describe '.report_exists?' do + subject(:pipeline_artifact) { Ci::PipelineArtifact.report_exists?(file_type) } - context 'when pipeline artifact has a code coverage' do - let!(:pipeline_artifact) { create(:ci_pipeline_artifact) } + context 'when file_type is code_coverage' do + let(:file_type) { :code_coverage } + + context 'when pipeline artifact has a coverage report' do + let!(:pipeline_artifact) { create(:ci_pipeline_artifact, :with_coverage_report) } + + it 'returns true' do + expect(pipeline_artifact).to be_truthy + end + end - it 'returns true' do - expect(subject).to be_truthy + context 'when pipeline artifact does not have a coverage report' do + it 'returns false' do + expect(pipeline_artifact).to be_falsey + end end end - context 'when pipeline artifact does not have a code coverage' do + context 'when file_type is code_quality_mr_diff' do + let(:file_type) { :code_quality_mr_diff } + + context 'when pipeline artifact has a codequality mr diff report' do + let!(:pipeline_artifact) { create(:ci_pipeline_artifact, :with_codequality_mr_diff_report) } + + it 'returns true' do + expect(pipeline_artifact).to be_truthy + end + end + + context 'when pipeline artifact does not have a codequality mr diff report' do + it 'returns false' do + expect(pipeline_artifact).to be_falsey + end + end + end + + context 'when file_type is nil' do + let(:file_type) { nil } + it 'returns false' do - expect(subject).to be_falsey + expect(pipeline_artifact).to be_falsey end end end - describe '.find_with_code_coverage' do - subject { Ci::PipelineArtifact.find_with_code_coverage } + describe '.find_by_file_type' do + subject(:pipeline_artifact) { Ci::PipelineArtifact.find_by_file_type(file_type) } - context 'when pipeline artifact has a coverage report' do - let!(:coverage_report) { create(:ci_pipeline_artifact) } + context 'when file_type is code_coverage' do + let(:file_type) { :code_coverage } + + context 'when pipeline artifact has a coverage report' do + let!(:coverage_report) { create(:ci_pipeline_artifact, :with_coverage_report) } - it 'returns a pipeline artifact with a code coverage' do - expect(subject.file_type).to eq('code_coverage') + it 'returns a pipeline artifact with a coverage report' do + expect(pipeline_artifact.file_type).to eq('code_coverage') + end + end + + context 'when pipeline artifact does not have a coverage report' do + it 'returns nil' do + expect(pipeline_artifact).to be_nil + end + end + end + + context 'when file_type is code_quality_mr_diff' do + let(:file_type) { :code_quality_mr_diff } + + context 'when pipeline artifact has a quality report' do + let!(:coverage_report) { create(:ci_pipeline_artifact, :with_codequality_mr_diff_report) } + + it 'returns a pipeline artifact with a quality report' do + expect(pipeline_artifact.file_type).to eq('code_quality_mr_diff') + end + end + + context 'when pipeline artifact does not have a quality report' do + it 'returns nil' do + expect(pipeline_artifact).to be_nil + end end end - context 'when pipeline artifact does not have a coverage report' do + context 'when file_type is nil' do + let(:file_type) { nil } + it 'returns nil' do - expect(subject).to be_nil + expect(pipeline_artifact).to be_nil end end end describe '#present' do - subject { coverage_report.present } + subject(:presenter) { report.present } context 'when file_type is code_coverage' do + let(:report) { coverage_report } + it 'uses code coverage presenter' do - expect(subject.present).to be_kind_of(Ci::PipelineArtifacts::CodeCoveragePresenter) + expect(presenter).to be_kind_of(Ci::PipelineArtifacts::CodeCoveragePresenter) + end + end + + context 'when file_type is code_quality_mr_diff' do + let(:report) { create(:ci_pipeline_artifact, :with_codequality_mr_diff_report) } + + it 'uses code codequality mr diff presenter' do + expect(presenter).to be_kind_of(Ci::PipelineArtifacts::CodeQualityMrDiffPresenter) end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 140527e4414..94943fb3644 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1263,26 +1263,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do pipeline.send(event) end - - context 'the feature is disabled' do - it 'does not trigger a worker' do - stub_feature_flags(jira_sync_builds: false) - - expect(worker).not_to receive(:perform_async) - - pipeline.send(event) - end - end - - context 'the feature is enabled for this project' do - it 'does trigger a worker' do - stub_feature_flags(jira_sync_builds: pipeline.project) - - expect(worker).to receive(:perform_async) - - pipeline.send(event) - end - end end end end @@ -2018,13 +1998,34 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do is_expected.to be_falsey end end + + context 'bridge which is allowed to fail fails' do + before do + create :ci_bridge, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop' + end + + it 'returns true' do + is_expected.to be_truthy + end + end + + context 'bridge which is allowed to fail is successful' do + before do + create :ci_bridge, :allowed_to_fail, :success, pipeline: pipeline, name: 'rubocop' + end + + it 'returns false' do + is_expected.to be_falsey + end + end end describe '#number_of_warnings' do it 'returns the number of warnings' do create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop') + create(:ci_bridge, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop') - expect(pipeline.number_of_warnings).to eq(1) + expect(pipeline.number_of_warnings).to eq(2) end it 'supports eager loading of the number of warnings' do @@ -2322,7 +2323,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'on waiting for resource' do before do - allow(build).to receive(:requires_resource?) { true } + allow(build).to receive(:with_resource_group?) { true } allow(Ci::ResourceGroups::AssignResourceFromResourceGroupWorker).to receive(:perform_async) build.enqueue @@ -3389,7 +3390,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#batch_lookup_report_artifact_for_file_type' do context 'with code quality report artifact' do - let(:pipeline) { create(:ci_pipeline, :with_codequality_report, project: project) } + let(:pipeline) { create(:ci_pipeline, :with_codequality_reports, project: project) } it "returns the code quality artifact" do expect(pipeline.batch_lookup_report_artifact_for_file_type(:codequality)).to eq(pipeline.job_artifacts.sample) @@ -3511,6 +3512,66 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '#has_codequality_mr_diff_report?' do + subject { pipeline.has_codequality_mr_diff_report? } + + context 'when pipeline has a codequality mr diff report' do + let(:pipeline) { create(:ci_pipeline, :with_codequality_mr_diff_report, :running, project: project) } + + it { expect(subject).to be_truthy } + end + + context 'when pipeline does not have a codequality mr diff report' do + let(:pipeline) { create(:ci_pipeline, :success, project: project) } + + it { expect(subject).to be_falsey } + end + end + + describe '#can_generate_codequality_reports?' do + subject { pipeline.can_generate_codequality_reports? } + + context 'when pipeline has builds with codequality reports' do + before do + create(:ci_build, :codequality_reports, pipeline: pipeline, project: project) + end + + context 'when pipeline status is running' do + let(:pipeline) { create(:ci_pipeline, :running, project: project) } + + it { expect(subject).to be_falsey } + end + + context 'when pipeline status is success' do + let(:pipeline) { create(:ci_pipeline, :success, project: project) } + + it 'can generate a codequality report' do + expect(subject).to be_truthy + end + + context 'when feature is disabled' do + before do + stub_feature_flags(codequality_mr_diff: false) + end + + it 'can not generate a codequality report' do + expect(subject).to be_falsey + end + end + end + end + + context 'when pipeline does not have builds with codequality reports' do + before do + create(:ci_build, :artifacts, pipeline: pipeline, project: project) + end + + let(:pipeline) { create(:ci_pipeline, :success, project: project) } + + it { expect(subject).to be_falsey } + end + end + describe '#test_report_summary' do subject { pipeline.test_report_summary } diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 35764e2bbbe..6290f4aef16 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -122,4 +122,58 @@ RSpec.describe Ci::Processable do it { is_expected.to be_empty } end end + + describe 'state transition with resource group' do + let(:resource_group) { create(:ci_resource_group, project: project) } + + context 'when build status is created' do + let(:build) { create(:ci_build, :created, project: project, resource_group: resource_group) } + + it 'is waiting for resource when build is enqueued' do + expect(Ci::ResourceGroups::AssignResourceFromResourceGroupWorker).to receive(:perform_async).with(resource_group.id) + + expect { build.enqueue! }.to change { build.status }.from('created').to('waiting_for_resource') + + expect(build.waiting_for_resource_at).not_to be_nil + end + + context 'when build is waiting for resource' do + before do + build.update_column(:status, 'waiting_for_resource') + end + + it 'is enqueued when build requests resource' do + expect { build.enqueue_waiting_for_resource! }.to change { build.status }.from('waiting_for_resource').to('pending') + end + + it 'releases a resource when build finished' do + expect(build.resource_group).to receive(:release_resource_from).with(build).and_call_original + expect(Ci::ResourceGroups::AssignResourceFromResourceGroupWorker).to receive(:perform_async).with(build.resource_group_id) + + build.enqueue_waiting_for_resource! + build.success! + end + + context 'when build has prerequisites' do + before do + allow(build).to receive(:any_unmet_prerequisites?) { true } + end + + it 'is preparing when build is enqueued' do + expect { build.enqueue_waiting_for_resource! }.to change { build.status }.from('waiting_for_resource').to('preparing') + end + end + + context 'when there are no available resources' do + before do + resource_group.assign_resource_to(create(:ci_build)) + end + + it 'stays as waiting for resource when build requests resource' do + expect { build.enqueue_waiting_for_resource }.not_to change { build.status } + end + end + end + end + end end diff --git a/spec/models/ci/resource_group_spec.rb b/spec/models/ci/resource_group_spec.rb index 9f72d1a82e5..50a786419f2 100644 --- a/spec/models/ci/resource_group_spec.rb +++ b/spec/models/ci/resource_group_spec.rb @@ -32,12 +32,12 @@ RSpec.describe Ci::ResourceGroup do let(:build) { create(:ci_build) } let(:resource_group) { create(:ci_resource_group) } - it 'retains resource for the build' do - expect(resource_group.resources.first.build).to be_nil + it 'retains resource for the processable' do + expect(resource_group.resources.first.processable).to be_nil is_expected.to eq(true) - expect(resource_group.resources.first.build).to eq(build) + expect(resource_group.resources.first.processable).to eq(build) end context 'when there are no free resources' do @@ -51,7 +51,7 @@ RSpec.describe Ci::ResourceGroup do end context 'when the build has already retained a resource' do - let!(:another_resource) { create(:ci_resource, resource_group: resource_group, build: build) } + let!(:another_resource) { create(:ci_resource, resource_group: resource_group, processable: build) } it 'fails to retain resource' do expect { subject }.to raise_error(ActiveRecord::RecordNotUnique) @@ -71,11 +71,11 @@ RSpec.describe Ci::ResourceGroup do end it 'releases resource from the build' do - expect(resource_group.resources.first.build).to eq(build) + expect(resource_group.resources.first.processable).to eq(build) is_expected.to eq(true) - expect(resource_group.resources.first.build).to be_nil + expect(resource_group.resources.first.processable).to be_nil end end diff --git a/spec/models/ci/resource_spec.rb b/spec/models/ci/resource_spec.rb index 90f26ef2b31..5574f6f82b2 100644 --- a/spec/models/ci/resource_spec.rb +++ b/spec/models/ci/resource_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Ci::Resource do subject { described_class.retained_by(build) } let(:build) { create(:ci_build) } - let!(:resource) { create(:ci_resource, build: build) } + let!(:resource) { create(:ci_resource, processable: build) } it 'returns retained resources' do is_expected.to eq([resource]) diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index 3d873a1b9c1..0afc491dc73 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -288,6 +288,7 @@ RSpec.describe Ci::Stage, :models do context 'when stage has warnings' do before do create(:ci_build, :failed, :allowed_to_fail, stage_id: stage.id) + create(:ci_bridge, :failed, :allowed_to_fail, stage_id: stage.id) end describe '#has_warnings?' do @@ -310,7 +311,7 @@ RSpec.describe Ci::Stage, :models do expect(synced_queries.count).to eq 1 expect(stage.number_of_warnings.inspect).to include 'BatchLoader' - expect(stage.number_of_warnings).to eq 1 + expect(stage.number_of_warnings).to eq 2 end end end diff --git a/spec/models/clusters/agent_spec.rb b/spec/models/clusters/agent_spec.rb index 49f41570717..a85a72eba0b 100644 --- a/spec/models/clusters/agent_spec.rb +++ b/spec/models/clusters/agent_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Clusters::Agent do subject { create(:cluster_agent) } + it { is_expected.to belong_to(:created_by_user).class_name('User').optional } it { is_expected.to belong_to(:project).class_name('::Project') } it { is_expected.to have_many(:agent_tokens).class_name('Clusters::AgentToken') } diff --git a/spec/models/clusters/agent_token_spec.rb b/spec/models/clusters/agent_token_spec.rb index 9110fdeda52..5cb84ee131a 100644 --- a/spec/models/clusters/agent_token_spec.rb +++ b/spec/models/clusters/agent_token_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Clusters::AgentToken do it { is_expected.to belong_to(:agent).class_name('Clusters::Agent') } + it { is_expected.to belong_to(:created_by_user).class_name('User').optional } describe '#token' do it 'is generated on save' do diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index fb0613187c5..07e64889b93 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -17,6 +17,8 @@ RSpec.describe Clusters::Platforms::Kubernetes do it { is_expected.to delegate_method(:enabled?).to(:cluster) } it { is_expected.to delegate_method(:provided_by_user?).to(:cluster) } + it { is_expected.to nullify_if_blank(:namespace) } + it_behaves_like 'having unique enum values' describe 'before_validation' do @@ -29,14 +31,6 @@ RSpec.describe Clusters::Platforms::Kubernetes do expect(kubernetes.namespace).to eq('abc') end end - - context 'when namespace is blank' do - let(:namespace) { '' } - - it 'nullifies the namespace' do - expect(kubernetes.namespace).to be_nil - end - end end describe 'validation' do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index acbabee9383..a5f02b61132 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -400,6 +400,19 @@ eos allow(commit).to receive(:safe_message).and_return(message + "\n" + message) expect(commit.full_title).to eq(message) end + + it 'truncates html representation if more than 1KiB' do + # Commit title is over 2KiB on a single line + huge_commit_title = ('panic ' * 350) + 'trailing text' + + allow(commit).to receive(:safe_message).and_return(huge_commit_title) + + commit.refresh_markdown_cache + full_title_html = commit.full_title_html + + expect(full_title_html.bytesize).to be < 2.kilobytes + expect(full_title_html).not_to include('trailing text') + end end describe 'description' do diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 532f68c2f18..01da379e001 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -510,6 +510,10 @@ RSpec.describe CommitStatus do end describe '#group_name' do + before do + stub_feature_flags(simplified_commit_status_group_name: false) + end + using RSpec::Parameterized::TableSyntax let(:commit_status) do @@ -557,6 +561,58 @@ RSpec.describe CommitStatus do is_expected.to eq(group_name) end end + + context 'with simplified_commit_status_group_name' do + before do + stub_feature_flags(simplified_commit_status_group_name: true) + end + + where(:name, :group_name) do + 'rspec1' | 'rspec1' + 'rspec1 0 1' | 'rspec1' + 'rspec1 0/2' | 'rspec1' + 'rspec:windows' | 'rspec:windows' + 'rspec:windows 0' | 'rspec:windows 0' + 'rspec:windows 0 2/2' | 'rspec:windows 0' + 'rspec:windows 0 test' | 'rspec:windows 0 test' + 'rspec:windows 0 test 2/2' | 'rspec:windows 0 test' + 'rspec:windows 0 1 2/2' | 'rspec:windows' + 'rspec:windows 0 1 [aws] 2/2' | 'rspec:windows' + 'rspec:windows 0 1 name [aws] 2/2' | 'rspec:windows 0 1 name' + 'rspec:windows 0 1 name' | 'rspec:windows 0 1 name' + 'rspec:windows 0 1 name 1/2' | 'rspec:windows 0 1 name' + 'rspec:windows 0/1' | 'rspec:windows' + 'rspec:windows 0/1 name' | 'rspec:windows 0/1 name' + 'rspec:windows 0/1 name 1/2' | 'rspec:windows 0/1 name' + 'rspec:windows 0:1' | 'rspec:windows' + 'rspec:windows 0:1 name' | 'rspec:windows 0:1 name' + 'rspec:windows 10000 20000' | 'rspec:windows' + 'rspec:windows 0 : / 1' | 'rspec:windows' + 'rspec:windows 0 : / 1 name' | 'rspec:windows 0 : / 1 name' + '0 1 name ruby' | '0 1 name ruby' + '0 :/ 1 name ruby' | '0 :/ 1 name ruby' + 'rspec: [aws]' | 'rspec' + 'rspec: [aws] 0/1' | 'rspec' + 'rspec: [aws, max memory]' | 'rspec' + 'rspec:linux: [aws, max memory, data]' | 'rspec:linux' + 'rspec: [inception: [something, other thing], value]' | 'rspec' + 'rspec:windows 0/1: [name, other]' | 'rspec:windows' + 'rspec:windows: [name, other] 0/1' | 'rspec:windows' + 'rspec:windows: [name, 0/1] 0/1' | 'rspec:windows' + 'rspec:windows: [0/1, name]' | 'rspec:windows' + 'rspec:windows: [, ]' | 'rspec:windows' + 'rspec:windows: [name]' | 'rspec:windows' + 'rspec:windows: [name,other]' | 'rspec:windows' + end + + with_them do + it "#{params[:name]} puts in #{params[:group_name]}" do + commit_status.name = name + + is_expected.to eq(group_name) + end + end + end end describe '#detailed_status' do @@ -725,22 +781,6 @@ RSpec.describe CommitStatus do let(:commit_status) { create(:commit_status) } it { is_expected.to eq(true) } - - context 'when build requires a resource' do - before do - allow(commit_status).to receive(:requires_resource?) { true } - end - - it { is_expected.to eq(false) } - end - - context 'when build has a prerequisite' do - before do - allow(commit_status).to receive(:any_unmet_prerequisites?) { true } - end - - it { is_expected.to eq(false) } - end end describe '#enqueue' do @@ -748,7 +788,6 @@ RSpec.describe CommitStatus do before do allow(Time).to receive(:now).and_return(current_time) - expect(commit_status.any_unmet_prerequisites?).to eq false end shared_examples 'commit status enqueued' do diff --git a/spec/models/concerns/atomic_internal_id_spec.rb b/spec/models/concerns/atomic_internal_id_spec.rb index 5ee3c012dc9..35b0f107676 100644 --- a/spec/models/concerns/atomic_internal_id_spec.rb +++ b/spec/models/concerns/atomic_internal_id_spec.rb @@ -87,6 +87,158 @@ RSpec.describe AtomicInternalId do end end + describe '#clear_scope_iid!' do + context 'when no ensure_if condition is given' do + it 'clears automatically set IIDs' do + expect(milestone).to receive(:clear_project_iid!).and_call_original + + expect_iid_to_be_set_and_rollback(milestone) + + expect(milestone.iid).to be_nil + end + + it 'does not clear manually set IIDS' do + milestone.iid = external_iid + + expect(milestone).to receive(:clear_project_iid!).and_call_original + + expect_iid_to_be_set_and_rollback(milestone) + + expect(milestone.iid).to eq(external_iid) + end + end + + context 'when an ensure_if condition is given' do + let(:test_class) do + Class.new(ApplicationRecord) do + include AtomicInternalId + include Importable + + self.table_name = :milestones + + belongs_to :project + + has_internal_id :iid, scope: :project, track_if: -> { !importing }, ensure_if: -> { !importing } + + def self.name + 'TestClass' + end + end + end + + let(:instance) { test_class.new(milestone.attributes) } + + context 'when the ensure_if condition evaluates to true' do + it 'clears automatically set IIDs' do + expect(instance).to receive(:clear_project_iid!).and_call_original + + expect_iid_to_be_set_and_rollback(instance) + + expect(instance.iid).to be_nil + end + + it 'does not clear manually set IIDs' do + instance.iid = external_iid + + expect(instance).to receive(:clear_project_iid!).and_call_original + + expect_iid_to_be_set_and_rollback(instance) + + expect(instance.iid).to eq(external_iid) + end + end + + context 'when the ensure_if condition evaluates to false' do + before do + instance.importing = true + end + + it 'does not clear IIDs' do + instance.iid = external_iid + + expect(instance).not_to receive(:clear_project_iid!) + + expect_iid_to_be_set_and_rollback(instance) + + expect(instance.iid).to eq(external_iid) + end + end + end + + def expect_iid_to_be_set_and_rollback(instance) + ActiveRecord::Base.transaction(requires_new: true) do + instance.save! + + expect(instance.iid).not_to be_nil + + raise ActiveRecord::Rollback + end + end + end + + describe '#validate_scope_iid_exists!' do + let(:test_class) do + Class.new(ApplicationRecord) do + include AtomicInternalId + include Importable + + self.table_name = :milestones + + belongs_to :project + + def self.name + 'TestClass' + end + end + end + + let(:instance) { test_class.new(milestone.attributes) } + + before do + test_class.has_internal_id :iid, scope: :project, presence: presence, ensure_if: -> { !importing } + + instance.importing = true + end + + context 'when the presence flag is set' do + let(:presence) { true } + + it 'raises an error for blank iids on create' do + expect do + instance.save! + end.to raise_error(described_class::MissingValueError, 'iid was unexpectedly blank!') + end + + it 'raises an error for blank iids on update' do + instance.iid = 100 + instance.save! + + instance.iid = nil + + expect do + instance.save! + end.to raise_error(described_class::MissingValueError, 'iid was unexpectedly blank!') + end + end + + context 'when the presence flag is not set' do + let(:presence) { false } + + it 'does not raise an error for blank iids on create' do + expect { instance.save! }.not_to raise_error + end + + it 'does not raise an error for blank iids on update' do + instance.iid = 100 + instance.save! + + instance.iid = nil + + expect { instance.save! }.not_to raise_error + end + end + end + describe '.with_project_iid_supply' do let(:iid) { 100 } diff --git a/spec/models/concerns/bulk_insert_safe_spec.rb b/spec/models/concerns/bulk_insert_safe_spec.rb index 82b0c00b396..e40b0cf11ff 100644 --- a/spec/models/concerns/bulk_insert_safe_spec.rb +++ b/spec/models/concerns/bulk_insert_safe_spec.rb @@ -44,7 +44,6 @@ RSpec.describe BulkInsertSafe do insecure_mode: false default_value_for :enum_value, 'case_1' - default_value_for :secret_value, 'my-secret' default_value_for :sha_value, '2fd4e1c67a2d28fced849ee1bb76e7391b93eb12' default_value_for :jsonb_value, { "key" => "value" } @@ -53,11 +52,11 @@ RSpec.describe BulkInsertSafe do end def self.valid_list(count) - Array.new(count) { |n| new(name: "item-#{n}") } + Array.new(count) { |n| new(name: "item-#{n}", secret_value: 'my-secret') } end def self.invalid_list(count) - Array.new(count) { new } + Array.new(count) { new(secret_value: 'my-secret') } end end end @@ -102,7 +101,7 @@ RSpec.describe BulkInsertSafe do context 'primary keys' do it 'raises error if primary keys are set prior to insertion' do - item = bulk_insert_item_class.new(name: 'valid', id: 10) + item = bulk_insert_item_class.new(name: 'valid', id: 10, secret_value: 'my-secret') expect { bulk_insert_item_class.bulk_insert!([item]) } .to raise_error(bulk_insert_item_class::PrimaryKeySetError) diff --git a/spec/models/concerns/featurable_spec.rb b/spec/models/concerns/featurable_spec.rb index 99acc563950..b550d22f686 100644 --- a/spec/models/concerns/featurable_spec.rb +++ b/spec/models/concerns/featurable_spec.rb @@ -134,22 +134,6 @@ RSpec.describe Featurable do expect(project.feature_available?(:issues, user)).to eq(true) end end - - context 'when feature is disabled by a feature flag' do - it 'returns false' do - stub_feature_flags(issues: false) - - expect(project.feature_available?(:issues, user)).to eq(false) - end - end - - context 'when feature is enabled by a feature flag' do - it 'returns true' do - stub_feature_flags(issues: true) - - expect(project.feature_available?(:issues, user)).to eq(true) - end - end end describe '#*_enabled?' do diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index ff5b270cf33..3545c8e9686 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Issuable do include ProjectForksHelper + using RSpec::Parameterized::TableSyntax let(:issuable_class) { Issue } let(:issue) { create(:issue, title: 'An issue', description: 'A description') } @@ -45,13 +46,17 @@ RSpec.describe Issuable do end it { is_expected.to validate_presence_of(:project) } - it { is_expected.to validate_presence_of(:iid) } it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_length_of(:title).is_at_most(described_class::TITLE_LENGTH_MAX) } it { is_expected.to validate_length_of(:description).is_at_most(described_class::DESCRIPTION_LENGTH_MAX).on(:create) } - it_behaves_like 'validates description length with custom validation' + it_behaves_like 'validates description length with custom validation' do + before do + allow(InternalId).to receive(:generate_next).and_call_original + end + end + it_behaves_like 'truncates the description to its allowed maximum length on import' end end @@ -820,8 +825,6 @@ RSpec.describe Issuable do end describe '#supports_time_tracking?' do - using RSpec::Parameterized::TableSyntax - where(:issuable_type, :supports_time_tracking) do :issue | true :incident | true @@ -838,8 +841,6 @@ RSpec.describe Issuable do end describe '#supports_severity?' do - using RSpec::Parameterized::TableSyntax - where(:issuable_type, :supports_severity) do :issue | false :incident | true @@ -856,8 +857,6 @@ RSpec.describe Issuable do end describe '#incident?' do - using RSpec::Parameterized::TableSyntax - where(:issuable_type, :incident) do :issue | false :incident | true @@ -874,8 +873,6 @@ RSpec.describe Issuable do end describe '#supports_issue_type?' do - using RSpec::Parameterized::TableSyntax - where(:issuable_type, :supports_issue_type) do :issue | true :merge_request | false @@ -894,8 +891,6 @@ RSpec.describe Issuable do subject { issuable.severity } context 'when issuable is not an incident' do - using RSpec::Parameterized::TableSyntax - where(:issuable_type, :severity) do :issue | 'unknown' :merge_request | 'unknown' diff --git a/spec/models/concerns/nullify_if_blank_spec.rb b/spec/models/concerns/nullify_if_blank_spec.rb new file mode 100644 index 00000000000..2d1bdba39dd --- /dev/null +++ b/spec/models/concerns/nullify_if_blank_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe NullifyIfBlank do + let_it_be(:model) do + Class.new(ApplicationRecord) do + include NullifyIfBlank + + nullify_if_blank :name + + self.table_name = 'users' + end + end + + context 'attribute exists' do + let(:instance) { model.new(name: name) } + + subject { instance.name } + + before do + instance.validate + end + + context 'attribute is blank' do + let(:name) { '' } + + it { is_expected.to be_nil } + end + + context 'attribute is nil' do + let(:name) { nil } + + it { is_expected.to be_nil} + end + + context 'attribute is not blank' do + let(:name) { 'name' } + + it { is_expected.to eq('name') } + end + end + + context 'attribute does not exist' do + before do + model.table_name = 'issues' + end + + it { expect { model.new.valid? }.to raise_error(ActiveModel::UnknownAttributeError) } + end +end diff --git a/spec/models/concerns/protected_ref_spec.rb b/spec/models/concerns/protected_ref_spec.rb new file mode 100644 index 00000000000..0a020736269 --- /dev/null +++ b/spec/models/concerns/protected_ref_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ProtectedRef do + using RSpec::Parameterized::TableSyntax + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user, maintainer_projects: [project]) } + + where(:klass, :factory, :action) do + ProtectedBranch | :protected_branch | :push + ProtectedTag | :protected_tag | :create + end + + with_them do + describe '#protected_ref_accessible_to?' do + subject do + klass.protected_ref_accessible_to?('release', user, project: project, action: action) + end + + it 'user cannot do action if rules do not exist' do + is_expected.to be_falsy + end + + context 'the ref is protected' do + let!(:default_rule) { create(factory, :"developers_can_#{action}", project: project, name: 'release') } + + context 'all rules permit action' do + let!(:maintainers_can) { create(factory, :"maintainers_can_#{action}", project: project, name: 'release*') } + + it 'user can do action' do + is_expected.to be_truthy + end + end + + context 'one of the rules forbids action' do + let!(:no_one_can) { create(factory, :"no_one_can_#{action}", project: project, name: 'release*') } + + it 'user cannot do action' do + is_expected.to be_falsy + end + end + end + end + + describe '#developers_can?' do + subject do + klass.developers_can?(action, 'release') + end + + it 'developers cannot do action if rules do not exist' do + is_expected.to be_falsy + end + + context 'the ref is protected' do + let!(:default_rule) { create(factory, :"developers_can_#{action}", project: project, name: 'release') } + + context 'all rules permit developers to do action' do + let!(:developers_can) { create(factory, :"developers_can_#{action}", project: project, name: 'release*') } + + it 'developers can do action' do + is_expected.to be_truthy + end + end + + context 'one of the rules forbids developers to do action' do + let!(:maintainers_can) { create(factory, :"maintainers_can_#{action}", project: project, name: 'release*') } + + it 'developers cannot do action' do + is_expected.to be_falsy + end + end + end + end + end +end diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index d4fcb2e99eb..3c5f3b2d2ad 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -120,6 +120,61 @@ RSpec.describe Spammable do end end + describe '#render_recaptcha?' do + before do + allow(Gitlab::Recaptcha).to receive(:enabled?) { recaptcha_enabled } + end + + context 'when recaptcha is not enabled' do + let(:recaptcha_enabled) { false } + + it 'returns false' do + expect(issue.render_recaptcha?).to eq(false) + end + end + + context 'when recaptcha is enabled' do + let(:recaptcha_enabled) { true } + + context 'when there are two or more errors' do + before do + issue.errors.add(:base, 'a spam error') + issue.errors.add(:base, 'some other error') + end + + it 'returns false' do + expect(issue.render_recaptcha?).to eq(false) + end + end + + context 'when there are less than two errors' do + before do + issue.errors.add(:base, 'a spam error') + end + + context 'when spammable does not need recaptcha' do + before do + issue.needs_recaptcha = false + end + + it 'returns false' do + expect(issue.render_recaptcha?).to eq(false) + end + end + + context 'when spammable needs recaptcha' do + before do + issue.needs_recaptcha! + end + + it 'returns false' do + expect(issue.render_recaptcha?).to eq(true) + end + end + end + end + end + describe '#clear_spam_flags!' do it 'clears spam and recaptcha flags' do issue.spam = true diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index d8b77e1cd0d..2df76684d71 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -54,7 +54,7 @@ RSpec.describe ApplicationSetting, 'TokenAuthenticatable' do it 'persists new token as an encrypted string' do expect(subject).to eq settings.reload.runners_registration_token expect(settings.read_attribute('runners_registration_token_encrypted')) - .to eq Gitlab::CryptoHelper.aes256_gcm_encrypt(subject) + .to eq Gitlab::CryptoHelper.aes256_gcm_encrypt(subject, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC) expect(settings).to be_persisted end @@ -243,7 +243,7 @@ RSpec.describe Ci::Build, 'TokenAuthenticatable' do it 'persists new token as an encrypted string' do build.ensure_token! - encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(build.token) + encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(build.token, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC) expect(build.read_attribute('token_encrypted')).to eq encrypted end diff --git a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb index f6b8cf7def4..1e1cd97e410 100644 --- a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb +++ b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb @@ -68,6 +68,10 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do context 'when using optional strategy' do let(:options) { { encrypted: :optional } } + before do + stub_feature_flags(dynamic_nonce_creation: false) + end + it 'returns decrypted token when an encrypted token is present' do allow(instance).to receive(:read_attribute) .with('some_field_encrypted') @@ -124,7 +128,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do it 'writes encrypted token and removes plaintext token and returns it' do expect(instance).to receive(:[]=) - .with('some_field_encrypted', encrypted) + .with('some_field_encrypted', any_args) expect(instance).to receive(:[]=) .with('some_field', nil) @@ -137,7 +141,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do it 'writes encrypted token and writes plaintext token' do expect(instance).to receive(:[]=) - .with('some_field_encrypted', encrypted) + .with('some_field_encrypted', any_args) expect(instance).to receive(:[]=) .with('some_field', 'my-value') diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 5bc61db6d21..68d12f51d4b 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -254,26 +254,6 @@ RSpec.describe Deployment do deployment.send(event) end - - context 'the feature is disabled' do - it 'does not trigger a worker' do - stub_feature_flags(jira_sync_deployments: false) - - expect(worker).not_to receive(:perform_async) - - deployment.send(event) - end - end - - context 'the feature is enabled for this project' do - it 'does trigger a worker' do - stub_feature_flags(jira_sync_deployments: deployment.project) - - expect(worker).to receive(:perform_async) - - deployment.send(event) - end - end end end end @@ -416,6 +396,26 @@ RSpec.describe Deployment do end end + describe '.finished_before' do + let!(:deployment1) { create(:deployment, finished_at: 1.day.ago) } + let!(:deployment2) { create(:deployment, finished_at: Time.current) } + + it 'filters deployments by finished_at' do + expect(described_class.finished_before(1.hour.ago)) + .to eq([deployment1]) + end + end + + describe '.finished_after' do + let!(:deployment1) { create(:deployment, finished_at: 1.day.ago) } + let!(:deployment2) { create(:deployment, finished_at: Time.current) } + + it 'filters deployments by finished_at' do + expect(described_class.finished_after(1.hour.ago)) + .to eq([deployment2]) + end + end + describe 'with_deployable' do subject { described_class.with_deployable } @@ -428,22 +428,6 @@ RSpec.describe Deployment do end end - describe 'finished_between' do - subject { described_class.finished_between(start_time, end_time) } - - let_it_be(:start_time) { DateTime.new(2017) } - let_it_be(:end_time) { DateTime.new(2019) } - let_it_be(:deployment_2016) { create(:deployment, finished_at: DateTime.new(2016)) } - let_it_be(:deployment_2017) { create(:deployment, finished_at: DateTime.new(2017)) } - let_it_be(:deployment_2018) { create(:deployment, finished_at: DateTime.new(2018)) } - let_it_be(:deployment_2019) { create(:deployment, finished_at: DateTime.new(2019)) } - let_it_be(:deployment_2020) { create(:deployment, finished_at: DateTime.new(2020)) } - - it 'retrieves deployments that finished between the specified times' do - is_expected.to contain_exactly(deployment_2017, deployment_2018) - end - end - describe 'visible' do subject { described_class.visible } diff --git a/spec/models/design_management/design_spec.rb b/spec/models/design_management/design_spec.rb index d3ce2f2d48f..674d2fc420d 100644 --- a/spec/models/design_management/design_spec.rb +++ b/spec/models/design_management/design_spec.rb @@ -629,25 +629,4 @@ RSpec.describe DesignManagement::Design do end end end - - describe '#immediately_before' do - let_it_be(:design) { create(:design, issue: issue, relative_position: 100) } - let_it_be(:next_design) { create(:design, issue: issue, relative_position: 200) } - - it 'is true when there is no element positioned between this item and the next' do - expect(design.immediately_before?(next_design)).to be true - end - - it 'is false when there is an element positioned between this item and the next' do - create(:design, issue: issue, relative_position: 150) - - expect(design.immediately_before?(next_design)).to be false - end - - it 'is false when the next design is to the left of this design' do - further_left = create(:design, issue: issue, relative_position: 50) - - expect(design.immediately_before?(further_left)).to be false - end - end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 47492715c11..47148c4febc 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -744,13 +744,19 @@ RSpec.describe Event do describe '#wiki_page and #wiki_page?' do context 'for a wiki page event' do - let(:wiki_page) do - create(:wiki_page, project: project) - end + let(:wiki_page) { create(:wiki_page, project: project) } subject(:event) { create(:wiki_page_event, project: project, wiki_page: wiki_page) } it { is_expected.to have_attributes(wiki_page?: be_truthy, wiki_page: wiki_page) } + + context 'title is empty' do + before do + expect(event.target).to receive(:canonical_slug).and_return('') + end + + it { is_expected.to have_attributes(wiki_page?: be_truthy, wiki_page: nil) } + end end context 'for any other event' do @@ -907,6 +913,58 @@ RSpec.describe Event do end end + context 'with snippet note' do + let_it_be(:user) { create(:user) } + let_it_be(:note_on_project_snippet) { create(:note_on_project_snippet, author: user) } + let_it_be(:note_on_personal_snippet) { create(:note_on_personal_snippet, author: user) } + let_it_be(:other_note) { create(:note_on_issue, author: user)} + let_it_be(:personal_snippet_event) { create(:event, :commented, project: nil, target: note_on_personal_snippet, author: user) } + let_it_be(:project_snippet_event) { create(:event, :commented, project: note_on_project_snippet.project, target: note_on_project_snippet, author: user) } + let_it_be(:other_event) { create(:event, :commented, project: other_note.project, target: other_note, author: user) } + + describe '#snippet_note?' do + it 'returns true for a project snippet event' do + expect(project_snippet_event.snippet_note?).to be true + end + + it 'returns true for a personal snippet event' do + expect(personal_snippet_event.snippet_note?).to be true + end + + it 'returns false for a other kinds of event' do + expect(other_event.snippet_note?).to be false + end + end + + describe '#personal_snippet_note?' do + it 'returns false for a project snippet event' do + expect(project_snippet_event.personal_snippet_note?).to be false + end + + it 'returns true for a personal snippet event' do + expect(personal_snippet_event.personal_snippet_note?).to be true + end + + it 'returns false for a other kinds of event' do + expect(other_event.personal_snippet_note?).to be false + end + end + + describe '#project_snippet_note?' do + it 'returns true for a project snippet event' do + expect(project_snippet_event.project_snippet_note?).to be true + end + + it 'returns false for a personal snippet event' do + expect(personal_snippet_event.project_snippet_note?).to be false + end + + it 'returns false for a other kinds of event' do + expect(other_event.project_snippet_note?).to be false + end + end + end + describe '#action_name' do it 'handles all valid design events' do created, updated, destroyed, archived = %i[created updated destroyed archived].map do |trait| diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb index 171bfd116d3..22bbf2df8fd 100644 --- a/spec/models/experiment_spec.rb +++ b/spec/models/experiment_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Experiment do + include AfterNextHelpers + subject { build(:experiment) } describe 'associations' do @@ -67,6 +69,33 @@ RSpec.describe Experiment do end end + describe '.add_group' do + let_it_be(:experiment_name) { :experiment_key } + let_it_be(:variant) { :control } + let_it_be(:group) { build(:group) } + + subject(:add_group) { described_class.add_group(experiment_name, variant: variant, group: group) } + + context 'when an experiment with the provided name does not exist' do + it 'creates a new experiment record' do + allow_next(described_class, name: :experiment_key) + .to receive(:record_group_and_variant!).with(group, variant) + + expect { add_group }.to change(described_class, :count).by(1) + end + end + + context 'when an experiment with the provided name already exists' do + before do + create(:experiment, name: experiment_name) + end + + it 'does not create a new experiment record' do + expect { add_group }.not_to change(described_class, :count) + end + end + end + describe '.record_conversion_event' do let_it_be(:user) { build(:user) } @@ -136,6 +165,34 @@ RSpec.describe Experiment do end end + describe '#record_group_and_variant!' do + let_it_be(:group) { create(:group) } + let_it_be(:variant) { :control } + let_it_be(:experiment) { create(:experiment) } + + subject(:record_group_and_variant!) { experiment.record_group_and_variant!(group, variant) } + + context 'when no existing experiment_subject record exists for the given group' do + it 'creates an experiment_subject record' do + expect_next(ExperimentSubject).to receive(:update!).with(variant: variant).and_call_original + + expect { record_group_and_variant! }.to change(ExperimentSubject, :count).by(1) + end + end + + context 'when an existing experiment_subject exists for the given group' do + context 'but it belonged to a different variant' do + let!(:experiment_subject) do + create(:experiment_subject, experiment: experiment, group: group, user: nil, variant: :experimental) + end + + it 'updates the variant value' do + expect { record_group_and_variant! }.to change { experiment_subject.reload.variant }.to('control') + end + end + end + end + describe '#record_user_and_group' do let_it_be(:experiment) { create(:experiment) } let_it_be(:user) { create(:user) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 0acf2b96b74..e79b54b4674 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -32,6 +32,7 @@ RSpec.describe Group do it { is_expected.to have_many(:dependency_proxy_blobs) } it { is_expected.to have_many(:dependency_proxy_manifests) } it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::GroupDistribution').dependent(:destroy) } + it { is_expected.to have_many(:daily_build_group_report_results).class_name('Ci::DailyBuildGroupReportResult') } describe '#members & #requesters' do let(:requester) { create(:user) } @@ -410,7 +411,7 @@ RSpec.describe Group do it "is false if avatar is html page" do group.update_attribute(:avatar, 'uploads/avatar.html') - expect(group.avatar_type).to eq(["file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico"]) + expect(group.avatar_type).to eq(["file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico, webp"]) end end @@ -1492,6 +1493,28 @@ RSpec.describe Group do end end + describe '.preset_root_ancestor_for' do + let_it_be(:rootgroup, reload: true) { create(:group) } + let_it_be(:subgroup, reload: true) { create(:group, parent: rootgroup) } + let_it_be(:subgroup2, reload: true) { create(:group, parent: subgroup) } + + it 'does noting for single group' do + expect(subgroup).not_to receive(:self_and_ancestors) + + described_class.preset_root_ancestor_for([subgroup]) + end + + it 'sets the same root_ancestor for multiple groups' do + expect(subgroup).not_to receive(:self_and_ancestors) + expect(subgroup2).not_to receive(:self_and_ancestors) + + described_class.preset_root_ancestor_for([rootgroup, subgroup, subgroup2]) + + expect(subgroup.root_ancestor).to eq(rootgroup) + expect(subgroup2.root_ancestor).to eq(rootgroup) + end + end + def subject_and_reload(*models) subject models.map(&:reload) @@ -1756,19 +1779,6 @@ RSpec.describe Group do describe 'with Debian Distributions' do subject { create(:group) } - let!(:distributions) { create_list(:debian_group_distribution, 2, :with_file, container: subject) } - - it 'removes distribution files on removal' do - distribution_file_paths = distributions.map do |distribution| - distribution.file.path - end - - expect { subject.destroy } - .to change { - distribution_file_paths.select do |path| - File.exist? path - end.length - }.from(distribution_file_paths.length).to(0) - end + it_behaves_like 'model with Debian distributions' end end diff --git a/spec/models/issue_link_spec.rb b/spec/models/issue_link_spec.rb index ef41108ebea..433b51b8a70 100644 --- a/spec/models/issue_link_spec.rb +++ b/spec/models/issue_link_spec.rb @@ -9,7 +9,7 @@ RSpec.describe IssueLink do end describe 'link_type' do - it { is_expected.to define_enum_for(:link_type).with_values(relates_to: 0, blocks: 1, is_blocked_by: 2) } + it { is_expected.to define_enum_for(:link_type).with_values(relates_to: 0, blocks: 1) } it 'provides the "related" as default link_type' do expect(create(:issue_link).link_type).to eq 'relates_to' diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 81f045b4db1..969d897e551 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -1244,9 +1244,7 @@ RSpec.describe Issue do end describe '#allows_reviewers?' do - it 'returns false as issues do not support reviewers feature' do - stub_feature_flags(merge_request_reviewers: true) - + it 'returns false as we do not support reviewers on issues yet' do issue = build_stubbed(:issue) expect(issue.allows_reviewers?).to be(false) diff --git a/spec/models/license_template_spec.rb b/spec/models/license_template_spec.rb index 515f728f515..fe06d1a357c 100644 --- a/spec/models/license_template_spec.rb +++ b/spec/models/license_template_spec.rb @@ -57,6 +57,6 @@ RSpec.describe LicenseTemplate do end def build_template(content) - described_class.new(key: 'foo', name: 'foo', category: :Other, content: content) + described_class.new(key: 'foo', name: 'foo', project: nil, category: :Other, content: content) end end diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb index 760eaf1ac7f..13ff239a306 100644 --- a/spec/models/merge_request/metrics_spec.rb +++ b/spec/models/merge_request/metrics_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe MergeRequest::Metrics do describe 'associations' do it { is_expected.to belong_to(:merge_request) } + it { is_expected.to belong_to(:target_project).class_name('Project') } it { is_expected.to belong_to(:latest_closed_by).class_name('User') } it { is_expected.to belong_to(:merged_by).class_name('User') } end @@ -36,5 +37,15 @@ RSpec.describe MergeRequest::Metrics do is_expected.not_to include([metrics_2]) end end + + describe '.by_target_project' do + let(:target_project) { metrics_1.target_project } + + subject { described_class.by_target_project(target_project) } + + it 'finds metrics record with the associated target project' do + is_expected.to eq([metrics_1]) + end + end end end diff --git a/spec/models/merge_request_diff_commit_spec.rb b/spec/models/merge_request_diff_commit_spec.rb index 2edf44ecdc4..a24628b0f9d 100644 --- a/spec/models/merge_request_diff_commit_spec.rb +++ b/spec/models/merge_request_diff_commit_spec.rb @@ -48,7 +48,8 @@ RSpec.describe MergeRequestDiffCommit do "committer_email": "dmitriy.zaporozhets@gmail.com", "merge_request_diff_id": merge_request_diff_id, "relative_order": 0, - "sha": Gitlab::Database::ShaAttribute.serialize("5937ac0a7beb003549fc5fd26fc247adbce4a52e") + "sha": Gitlab::Database::ShaAttribute.serialize("5937ac0a7beb003549fc5fd26fc247adbce4a52e"), + "trailers": {}.to_json }, { "message": "Change some files\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n", @@ -60,7 +61,8 @@ RSpec.describe MergeRequestDiffCommit do "committer_email": "dmitriy.zaporozhets@gmail.com", "merge_request_diff_id": merge_request_diff_id, "relative_order": 1, - "sha": Gitlab::Database::ShaAttribute.serialize("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") + "sha": Gitlab::Database::ShaAttribute.serialize("570e7b2abdd848b95f2f578043fc23bd6f6fd24d"), + "trailers": {}.to_json } ] end @@ -92,7 +94,8 @@ RSpec.describe MergeRequestDiffCommit do "committer_email": "alejorro70@gmail.com", "merge_request_diff_id": merge_request_diff_id, "relative_order": 0, - "sha": Gitlab::Database::ShaAttribute.serialize("ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69") + "sha": Gitlab::Database::ShaAttribute.serialize("ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69"), + "trailers": {}.to_json }] end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index a5493d1650b..5b11a7bf079 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -16,6 +16,8 @@ RSpec.describe MergeRequestDiff do describe 'validations' do subject { diff_with_commits } + it { is_expected.not_to validate_uniqueness_of(:diff_type).scoped_to(:merge_request_id) } + it 'checks sha format of base_commit_sha, head_commit_sha and start_commit_sha' do subject.base_commit_sha = subject.head_commit_sha = subject.start_commit_sha = 'foobar' @@ -23,6 +25,24 @@ RSpec.describe MergeRequestDiff do expect(subject.errors.count).to eq 3 expect(subject.errors).to all(include('is not a valid SHA')) end + + it 'does not validate uniqueness by default' do + expect(build(:merge_request_diff, merge_request: subject.merge_request)).to be_valid + end + + context 'when merge request diff is a merge_head type' do + it 'is valid' do + expect(build(:merge_request_diff, :merge_head, merge_request: subject.merge_request)).to be_valid + end + + context 'when merge_head diff exists' do + let(:existing_merge_head_diff) { create(:merge_request_diff, :merge_head) } + + it 'validates uniqueness' do + expect(build(:merge_request_diff, :merge_head, merge_request: existing_merge_head_diff.merge_request)).not_to be_valid + end + end + end end describe 'create new record' do @@ -35,12 +55,32 @@ RSpec.describe MergeRequestDiff do it { expect(subject.head_commit_sha).to eq('b83d6e391c22777fca1ed3012fce84f633d7fed0') } it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') } it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') } + + context 'when diff_type is merge_head' do + let_it_be(:merge_request) { create(:merge_request) } + + let_it_be(:merge_head) do + MergeRequests::MergeToRefService + .new(merge_request.project, merge_request.author) + .execute(merge_request) + + merge_request.create_merge_head_diff + end + + it { expect(merge_head).to be_valid } + it { expect(merge_head).to be_persisted } + it { expect(merge_head.commits.count).to eq(30) } + it { expect(merge_head.diffs.count).to eq(20) } + it { expect(merge_head.head_commit_sha).to eq(merge_request.merge_ref_head.diff_refs.head_sha) } + it { expect(merge_head.base_commit_sha).to eq(merge_request.merge_ref_head.diff_refs.base_sha) } + it { expect(merge_head.start_commit_sha).to eq(merge_request.target_branch_sha) } + end end describe '.by_commit_sha' do subject(:by_commit_sha) { described_class.by_commit_sha(sha) } - let!(:merge_request) { create(:merge_request, :with_diffs) } + let!(:merge_request) { create(:merge_request) } context 'with sha contained in' do let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } @@ -63,6 +103,7 @@ RSpec.describe MergeRequestDiff do let_it_be(:merge_request) { create(:merge_request) } let_it_be(:outdated) { merge_request.merge_request_diff } let_it_be(:latest) { merge_request.create_merge_request_diff } + let_it_be(:merge_head) { merge_request.create_merge_head_diff } let_it_be(:closed_mr) { create(:merge_request, :closed_last_month) } let(:closed) { closed_mr.merge_request_diff } @@ -103,14 +144,14 @@ RSpec.describe MergeRequestDiff do stub_external_diffs_setting(enabled: true) end - it { is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id, closed_recently.id, merged_recently.id) } + it { is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id, closed_recently.id, merged_recently.id, merge_head.id) } it 'ignores diffs with 0 files' do MergeRequestDiffFile.where(merge_request_diff_id: [closed_recently.id, merged_recently.id]).delete_all closed_recently.update!(files_count: 0) merged_recently.update!(files_count: 0) - is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id) + is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id, merge_head.id) end end @@ -317,7 +358,7 @@ RSpec.describe MergeRequestDiff do end describe '#latest?' do - let!(:mr) { create(:merge_request, :with_diffs) } + let!(:mr) { create(:merge_request) } let!(:first_diff) { mr.merge_request_diff } let!(:last_diff) { mr.create_merge_request_diff } @@ -326,7 +367,7 @@ RSpec.describe MergeRequestDiff do end shared_examples_for 'merge request diffs' do - let(:merge_request) { create(:merge_request, :with_diffs) } + let(:merge_request) { create(:merge_request) } let!(:diff) { merge_request.merge_request_diff.reload } context 'when it was not cleaned by the system' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 1cf197322f5..ebe2cd2ac03 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -271,8 +271,6 @@ RSpec.describe MergeRequest, factory_default: :keep do stub_feature_flags(stricter_mr_branch_name: false) end - using RSpec::Parameterized::TableSyntax - where(:branch_name, :valid) do 'foo' | true 'foo:bar' | false @@ -367,7 +365,7 @@ RSpec.describe MergeRequest, factory_default: :keep do describe '.by_commit_sha' do subject(:by_commit_sha) { described_class.by_commit_sha(sha) } - let!(:merge_request) { create(:merge_request, :with_diffs) } + let!(:merge_request) { create(:merge_request) } context 'with sha contained in latest merge request diff' do let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } @@ -433,7 +431,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when commit is a part of the merge request' do - let!(:merge_request) { create(:merge_request, :with_diffs) } + let!(:merge_request) { create(:merge_request) } let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } it { is_expected.to eq([merge_request]) } @@ -451,6 +449,17 @@ RSpec.describe MergeRequest, factory_default: :keep do it { is_expected.to be_empty } end + + context 'when commit is part of the merge request and a squash commit at the same time' do + let!(:merge_request) { create(:merge_request) } + let(:sha) { merge_request.commits.first.id } + + before do + merge_request.update!(squash_commit_sha: sha) + end + + it { is_expected.to eq([merge_request]) } + end end describe '.by_cherry_pick_sha' do @@ -480,6 +489,7 @@ RSpec.describe MergeRequest, factory_default: :keep do create(:merge_request, params).tap do |mr| diffs.times { mr.merge_request_diffs.create } + mr.create_merge_head_diff end end @@ -705,6 +715,10 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when external issue tracker is enabled' do + let(:project) { create(:project, :repository) } + + subject { create(:merge_request, source_project: project) } + before do subject.project.has_external_issue_tracker = true subject.project.save! @@ -754,9 +768,8 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'when both internal and external issue trackers are enabled' do before do - subject.project.has_external_issue_tracker = true - subject.project.save! create(:jira_service, project: subject.project) + subject.project.reload end it 'does not cache issues from external trackers' do @@ -779,6 +792,10 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when only external issue tracker enabled' do + let(:project) { create(:project, :repository) } + + subject { create(:merge_request, source_project: project) } + before do subject.project.has_external_issue_tracker = true subject.project.issues_enabled = false @@ -808,7 +825,7 @@ RSpec.describe MergeRequest, factory_default: :keep do let(:last_branch_commit) { subject.source_project.repository.commit(Gitlab::Git::BRANCH_REF_PREFIX + subject.source_branch) } context 'with diffs' do - subject { create(:merge_request, :with_diffs) } + subject { create(:merge_request) } it 'returns the sha of the source branch last commit' do expect(subject.source_branch_sha).to eq(last_branch_commit.sha) @@ -875,7 +892,7 @@ RSpec.describe MergeRequest, factory_default: :keep do let(:options) { { paths: ['a/b', 'b/a', 'c/*'] } } context 'when there are MR diffs' do - let(:merge_request) { create(:merge_request, :with_diffs) } + let(:merge_request) { create(:merge_request) } it 'delegates to the MR diffs' do expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(options) @@ -924,7 +941,7 @@ RSpec.describe MergeRequest, factory_default: :keep do describe '#note_positions_for_paths' do let(:user) { create(:user) } - let(:merge_request) { create(:merge_request, :with_diffs) } + let(:merge_request) { create(:merge_request) } let(:project) { merge_request.project } let!(:diff_note) do create(:diff_note_on_merge_request, project: project, noteable: merge_request) @@ -1263,7 +1280,6 @@ RSpec.describe MergeRequest, factory_default: :keep do describe '#issues_mentioned_but_not_closing' do let(:closing_issue) { create :issue, project: subject.project } let(:mentioned_issue) { create :issue, project: subject.project } - let(:commit) { double('commit', safe_message: "Fixes #{closing_issue.to_reference}") } it 'detects issues mentioned in description but not closed' do @@ -1279,13 +1295,12 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when the project has an external issue tracker' do - subject { create(:merge_request, source_project: create(:project, :repository)) } - before do subject.project.add_developer(subject.author) commit = double(:commit, safe_message: 'Fixes TEST-3') create(:jira_service, project: subject.project) + subject.project.reload allow(subject).to receive(:commits).and_return([commit]) allow(subject).to receive(:description).and_return('Is related to TEST-2 and TEST-3') @@ -1645,7 +1660,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end it_behaves_like 'an editable mentionable' do - subject { create(:merge_request, :simple) } + subject { create(:merge_request, :simple, source_project: create(:project, :repository)) } let(:backref_text) { "merge request #{subject.to_reference}" } let(:set_mentionable_text) { ->(txt) { subject.description = txt } } @@ -1971,6 +1986,30 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '#has_codequality_mr_diff_report?' do + subject { merge_request.has_codequality_mr_diff_report? } + + context 'when head pipeline has codequality mr diff report' do + let(:merge_request) { create(:merge_request, :with_codequality_mr_diff_reports) } + + it { is_expected.to be_truthy } + + context 'when feature flag is disabled' do + before do + stub_feature_flags(codequality_mr_diff: false) + end + + it { is_expected.to be_falsey } + end + end + + context 'when head pipeline does not have codeqquality mr diff report' do + let(:merge_request) { create(:merge_request) } + + it { is_expected.to be_falsey } + end + end + describe '#has_codequality_reports?' do subject { merge_request.has_codequality_reports? } @@ -1983,7 +2022,7 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'when feature flag is disabled' do before do - stub_feature_flags(codequality_mr_diff: false) + stub_feature_flags(codequality_backend_comparison: false) end it { is_expected.to be_falsey } @@ -2015,6 +2054,50 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '#has_sast_reports?' do + subject { merge_request.has_sast_reports? } + + let(:project) { create(:project, :repository) } + + before do + stub_licensed_features(sast: true) + end + + context 'when head pipeline has sast reports' do + let(:merge_request) { create(:merge_request, :with_sast_reports, source_project: project) } + + it { is_expected.to be_truthy } + end + + context 'when head pipeline does not have sast reports' do + let(:merge_request) { create(:merge_request, source_project: project) } + + it { is_expected.to be_falsey } + end + end + + describe '#has_secret_detection_reports?' do + subject { merge_request.has_secret_detection_reports? } + + let(:project) { create(:project, :repository) } + + before do + stub_licensed_features(secret_detection: true) + end + + context 'when head pipeline has secret detection reports' do + let(:merge_request) { create(:merge_request, :with_secret_detection_reports, source_project: project) } + + it { is_expected.to be_truthy } + end + + context 'when head pipeline does not have secrets detection reports' do + let(:merge_request) { create(:merge_request, source_project: project) } + + it { is_expected.to be_falsey } + end + end + describe '#calculate_reactive_cache' do let(:merge_request) { create(:merge_request) } @@ -2144,6 +2227,54 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '#find_codequality_mr_diff_reports' do + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, :with_codequality_mr_diff_reports, source_project: project) } + let(:pipeline) { merge_request.head_pipeline } + + subject(:mr_diff_report) { merge_request.find_codequality_mr_diff_reports } + + context 'when head pipeline has coverage reports' do + context 'when reactive cache worker is parsing results asynchronously' do + it 'returns status' do + expect(mr_diff_report[:status]).to eq(:parsing) + end + end + + context 'when reactive cache worker is inline' do + before do + synchronous_reactive_cache(merge_request) + end + + it 'returns status and data' do + expect(mr_diff_report[:status]).to eq(:parsed) + end + + context 'when an error occurrs' do + before do + merge_request.update!(head_pipeline: nil) + end + + it 'returns an error message' do + expect(mr_diff_report[:status]).to eq(:error) + end + end + + context 'when cached results is not latest' do + before do + allow_next_instance_of(Ci::GenerateCodequalityMrDiffReportService) do |service| + allow(service).to receive(:latest?).and_return(false) + end + end + + it 'raises and InvalidateReactiveCache error' do + expect { mr_diff_report }.to raise_error(ReactiveCaching::InvalidateReactiveCache) + end + end + end + end + end + describe '#compare_test_reports' do subject { merge_request.compare_test_reports } @@ -2765,8 +2896,6 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'with skip_ci_check option' do - using RSpec::Parameterized::TableSyntax - before do allow(subject).to receive_messages(check_mergeability: nil, can_be_merged?: true, @@ -2790,8 +2919,6 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'with skip_discussions_check option' do - using RSpec::Parameterized::TableSyntax - before do allow(subject).to receive_messages(mergeable_ci_state?: true, check_mergeability: nil, @@ -3345,6 +3472,10 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when resolve_outdated_diff_discussions is set' do + let(:project) { create(:project, :repository) } + + subject { create(:merge_request, source_project: project) } + before do discussion @@ -3365,7 +3496,7 @@ RSpec.describe MergeRequest, factory_default: :keep do describe '#branch_merge_base_commit' do let(:project) { create(:project, :repository) } - subject { create(:merge_request, :with_diffs, source_project: project) } + subject { create(:merge_request, source_project: project) } context 'source and target branch exist' do it { expect(subject.branch_merge_base_commit.sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') } @@ -3388,7 +3519,7 @@ RSpec.describe MergeRequest, factory_default: :keep do context "with diffs" do let(:project) { create(:project, :repository) } - subject { create(:merge_request, :with_diffs, source_project: project) } + subject { create(:merge_request, source_project: project) } let(:expected_diff_refs) do Gitlab::Diff::DiffRefs.new( @@ -3792,7 +3923,7 @@ RSpec.describe MergeRequest, factory_default: :keep do describe '#fetch_ref!' do let(:project) { create(:project, :repository) } - subject { create(:merge_request, :with_diffs, source_project: project) } + subject { create(:merge_request, source_project: project) } it 'fetches the ref correctly' do expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error @@ -4367,37 +4498,41 @@ RSpec.describe MergeRequest, factory_default: :keep do end describe '#diffable_merge_ref?' do + let(:merge_request) { create(:merge_request) } + context 'merge request can be merged' do - context 'merge_to_ref is not calculated' do + context 'merge_head diff is not created' do it 'returns true' do - expect(subject.diffable_merge_ref?).to eq(false) + expect(merge_request.diffable_merge_ref?).to eq(false) end end - context 'merge_to_ref is calculated' do + context 'merge_head diff is created' do before do - MergeRequests::MergeToRefService.new(subject.project, subject.author).execute(subject) + create(:merge_request_diff, :merge_head, merge_request: merge_request) end it 'returns true' do - expect(subject.diffable_merge_ref?).to eq(true) + expect(merge_request.diffable_merge_ref?).to eq(true) end context 'merge request is merged' do - subject { build_stubbed(:merge_request, :merged, project: project) } + before do + merge_request.mark_as_merged! + end it 'returns false' do - expect(subject.diffable_merge_ref?).to eq(false) + expect(merge_request.diffable_merge_ref?).to eq(false) end end context 'merge request cannot be merged' do before do - subject.mark_as_unchecked! + merge_request.mark_as_unchecked! end it 'returns false' do - expect(subject.diffable_merge_ref?).to eq(true) + expect(merge_request.diffable_merge_ref?).to eq(true) end context 'display_merge_conflicts_in_diff is disabled' do @@ -4406,7 +4541,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end it 'returns false' do - expect(subject.diffable_merge_ref?).to eq(false) + expect(merge_request.diffable_merge_ref?).to eq(false) end end end @@ -4476,17 +4611,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end describe '#allows_reviewers?' do - it 'returns false without merge_request_reviewers feature' do - stub_feature_flags(merge_request_reviewers: false) - - merge_request = build_stubbed(:merge_request) - - expect(merge_request.allows_reviewers?).to be(false) - end - - it 'returns true with merge_request_reviewers feature' do - stub_feature_flags(merge_request_reviewers: true) - + it 'returns true' do merge_request = build_stubbed(:merge_request) expect(merge_request.allows_reviewers?).to be(true) @@ -4506,4 +4631,34 @@ RSpec.describe MergeRequest, factory_default: :keep do .from(nil).to(ref) end end + + describe '#enabled_reports' do + let(:project) { create(:project, :repository) } + + where(:report_type, :with_reports, :feature) do + :sast | :with_sast_reports | :sast + :secret_detection | :with_secret_detection_reports | :secret_detection + end + + with_them do + subject { merge_request.enabled_reports[report_type] } + + before do + stub_feature_flags(drop_license_management_artifact: false) + stub_licensed_features({ feature => true }) + end + + context "when head pipeline has reports" do + let(:merge_request) { create(:merge_request, with_reports, source_project: project) } + + it { is_expected.to be_truthy } + end + + context "when head pipeline does not have reports" do + let(:merge_request) { create(:merge_request, source_project: project) } + + it { is_expected.to be_falsy } + end + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index a3c0a43115e..647e279bf83 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -113,6 +113,7 @@ RSpec.describe Namespace do describe 'inclusions' do it { is_expected.to include_module(Gitlab::VisibilityLevel) } + it { is_expected.to include_module(Namespaces::Traversal::Recursive) } end describe '#visibility_level_field' do @@ -770,80 +771,7 @@ RSpec.describe Namespace do end end - describe '#self_and_hierarchy' do - let!(:group) { create(:group, path: 'git_lab') } - let!(:nested_group) { create(:group, parent: group) } - let!(:deep_nested_group) { create(:group, parent: nested_group) } - let!(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } - let!(:another_group) { create(:group, path: 'gitllab') } - let!(:another_group_nested) { create(:group, path: 'foo', parent: another_group) } - - it 'returns the correct tree' do - expect(group.self_and_hierarchy).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group) - expect(nested_group.self_and_hierarchy).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group) - expect(very_deep_nested_group.self_and_hierarchy).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group) - end - end - - describe '#ancestors' do - let(:group) { create(:group) } - let(:nested_group) { create(:group, parent: group) } - let(:deep_nested_group) { create(:group, parent: nested_group) } - let(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } - - it 'returns the correct ancestors' do - expect(very_deep_nested_group.ancestors).to include(group, nested_group, deep_nested_group) - expect(deep_nested_group.ancestors).to include(group, nested_group) - expect(nested_group.ancestors).to include(group) - expect(group.ancestors).to eq([]) - end - end - - describe '#self_and_ancestors' do - let(:group) { create(:group) } - let(:nested_group) { create(:group, parent: group) } - let(:deep_nested_group) { create(:group, parent: nested_group) } - let(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } - - it 'returns the correct ancestors' do - expect(very_deep_nested_group.self_and_ancestors).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group) - expect(deep_nested_group.self_and_ancestors).to contain_exactly(group, nested_group, deep_nested_group) - expect(nested_group.self_and_ancestors).to contain_exactly(group, nested_group) - expect(group.self_and_ancestors).to contain_exactly(group) - end - end - - describe '#descendants' do - let!(:group) { create(:group, path: 'git_lab') } - let!(:nested_group) { create(:group, parent: group) } - let!(:deep_nested_group) { create(:group, parent: nested_group) } - let!(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } - let!(:another_group) { create(:group, path: 'gitllab') } - let!(:another_group_nested) { create(:group, path: 'foo', parent: another_group) } - - it 'returns the correct descendants' do - expect(very_deep_nested_group.descendants.to_a).to eq([]) - expect(deep_nested_group.descendants.to_a).to include(very_deep_nested_group) - expect(nested_group.descendants.to_a).to include(deep_nested_group, very_deep_nested_group) - expect(group.descendants.to_a).to include(nested_group, deep_nested_group, very_deep_nested_group) - end - end - - describe '#self_and_descendants' do - let!(:group) { create(:group, path: 'git_lab') } - let!(:nested_group) { create(:group, parent: group) } - let!(:deep_nested_group) { create(:group, parent: nested_group) } - let!(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } - let!(:another_group) { create(:group, path: 'gitllab') } - let!(:another_group_nested) { create(:group, path: 'foo', parent: another_group) } - - it 'returns the correct descendants' do - expect(very_deep_nested_group.self_and_descendants).to contain_exactly(very_deep_nested_group) - expect(deep_nested_group.self_and_descendants).to contain_exactly(deep_nested_group, very_deep_nested_group) - expect(nested_group.self_and_descendants).to contain_exactly(nested_group, deep_nested_group, very_deep_nested_group) - expect(group.self_and_descendants).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group) - end - end + it_behaves_like 'recursive namespace traversal' describe '#users_with_descendants' do let(:user_a) { create(:user) } diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 6e87ca6dcf7..364b80e8601 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -837,6 +837,16 @@ RSpec.describe Note do end end + describe '#for_project_snippet?' do + it 'returns true for a project snippet note' do + expect(build(:note_on_project_snippet).for_project_snippet?).to be true + end + + it 'returns false for a personal snippet note' do + expect(build(:note_on_personal_snippet).for_project_snippet?).to be false + end + end + describe '#for_personal_snippet?' do it 'returns false for a project snippet note' do expect(build(:note_on_project_snippet).for_personal_snippet?).to be_falsy @@ -890,35 +900,31 @@ RSpec.describe Note do describe '#cache_markdown_field' do let(:html) { '<p>some html</p>'} + before do + allow(Banzai::Renderer).to receive(:cacheless_render_field).and_call_original + end + context 'note for a project snippet' do let(:snippet) { create(:project_snippet) } - let(:note) { build(:note_on_project_snippet, project: snippet.project, noteable: snippet) } + let(:note) { create(:note_on_project_snippet, project: snippet.project, noteable: snippet) } - before do + it 'skips project check' do expect(Banzai::Renderer).to receive(:cacheless_render_field) - .with(note, :note, { skip_project_check: false }).and_return(html) - - note.save - end + .with(note, :note, { skip_project_check: false }) - it 'creates a note' do - expect(note.note_html).to eq(html) + note.update!(note: html) end end context 'note for a personal snippet' do let(:snippet) { create(:personal_snippet) } - let(:note) { build(:note_on_personal_snippet, noteable: snippet) } + let(:note) { create(:note_on_personal_snippet, noteable: snippet) } - before do + it 'does not skip project check' do expect(Banzai::Renderer).to receive(:cacheless_render_field) - .with(note, :note, { skip_project_check: true }).and_return(html) - - note.save - end + .with(note, :note, { skip_project_check: true }) - it 'creates a note' do - expect(note.note_html).to eq(html) + note.update!(note: html) end end end diff --git a/spec/models/onboarding_progress_spec.rb b/spec/models/onboarding_progress_spec.rb index bd951846bb8..0aa19345a25 100644 --- a/spec/models/onboarding_progress_spec.rb +++ b/spec/models/onboarding_progress_spec.rb @@ -29,6 +29,67 @@ RSpec.describe OnboardingProgress do end end + describe 'scopes' do + describe '.incomplete_actions' do + subject { described_class.incomplete_actions(actions) } + + let!(:no_actions_completed) { create(:onboarding_progress) } + let!(:one_action_completed_one_action_incompleted) { create(:onboarding_progress, "#{action}_at" => Time.current) } + + context 'when given one action' do + let(:actions) { action } + + it { is_expected.to eq [no_actions_completed] } + end + + context 'when given an array of actions' do + let(:actions) { [action, :git_write] } + + it { is_expected.to eq [no_actions_completed] } + end + end + + describe '.completed_actions' do + subject { described_class.completed_actions(actions) } + + let!(:one_action_completed_one_action_incompleted) { create(:onboarding_progress, "#{action}_at" => Time.current) } + let!(:both_actions_completed) { create(:onboarding_progress, "#{action}_at" => Time.current, git_write_at: Time.current) } + + context 'when given one action' do + let(:actions) { action } + + it { is_expected.to eq [one_action_completed_one_action_incompleted, both_actions_completed] } + end + + context 'when given an array of actions' do + let(:actions) { [action, :git_write] } + + it { is_expected.to eq [both_actions_completed] } + end + end + + describe '.completed_actions_with_latest_in_range' do + subject { described_class.completed_actions_with_latest_in_range(actions, 1.day.ago.beginning_of_day..1.day.ago.end_of_day) } + + let!(:one_action_completed_in_range_one_action_incompleted) { create(:onboarding_progress, "#{action}_at" => 1.day.ago.middle_of_day) } + let!(:git_write_action_completed_in_range) { create(:onboarding_progress, git_write_at: 1.day.ago.middle_of_day) } + let!(:both_actions_completed_latest_action_out_of_range) { create(:onboarding_progress, "#{action}_at" => 1.day.ago.middle_of_day, git_write_at: Time.current) } + let!(:both_actions_completed_latest_action_in_range) { create(:onboarding_progress, "#{action}_at" => 1.day.ago.middle_of_day, git_write_at: 2.days.ago.middle_of_day) } + + context 'when given one action' do + let(:actions) { :git_write } + + it { is_expected.to eq [git_write_action_completed_in_range] } + end + + context 'when given an array of actions' do + let(:actions) { [action, :git_write] } + + it { is_expected.to eq [both_actions_completed_latest_action_in_range] } + end + end + end + describe '.onboard' do subject(:onboard) { described_class.onboard(namespace) } @@ -53,6 +114,22 @@ RSpec.describe OnboardingProgress do end end + describe '.onboarding?' do + subject(:onboarding?) { described_class.onboarding?(namespace) } + + context 'when onboarded' do + before do + described_class.onboard(namespace) + end + + it { is_expected.to eq true } + end + + context 'when not onboarding' do + it { is_expected.to eq false } + end + end + describe '.register' do subject(:register_action) { described_class.register(namespace, action) } @@ -104,4 +181,10 @@ RSpec.describe OnboardingProgress do end end end + + describe '.column_name' do + subject { described_class.column_name(action) } + + it { is_expected.to eq(:subscription_created_at) } + end end diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb index 93dd7d4f0bb..d5b3c7a8582 100644 --- a/spec/models/operations/feature_flag_spec.rb +++ b/spec/models/operations/feature_flag_spec.rb @@ -16,6 +16,35 @@ RSpec.describe Operations::FeatureFlag do it { is_expected.to have_many(:scopes) } end + describe '.reference_pattern' do + subject { described_class.reference_pattern } + + it { is_expected.to match('[feature_flag:123]') } + it { is_expected.to match('[feature_flag:gitlab-org/gitlab/123]') } + end + + describe '.link_reference_pattern' do + subject { described_class.link_reference_pattern } + + it { is_expected.to match("#{Gitlab.config.gitlab.url}/gitlab-org/gitlab/-/feature_flags/123/edit") } + it { is_expected.not_to match("#{Gitlab.config.gitlab.url}/gitlab-org/gitlab/issues/123/edit") } + it { is_expected.not_to match("gitlab-org/gitlab/-/feature_flags/123/edit") } + end + + describe '#to_reference' do + let(:namespace) { build(:namespace, path: 'sample-namespace') } + let(:project) { build(:project, name: 'sample-project', namespace: namespace) } + let(:feature_flag) { build(:operations_feature_flag, iid: 1, project: project) } + + it 'returns feature flag id' do + expect(feature_flag.to_reference).to eq '[feature_flag:1]' + end + + it 'returns complete path to the feature flag with full: true' do + expect(feature_flag.to_reference(full: true)).to eq '[feature_flag:sample-namespace/sample-project/1]' + end + end + describe 'validations' do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:name) } diff --git a/spec/models/packages/composer/cache_file_spec.rb b/spec/models/packages/composer/cache_file_spec.rb new file mode 100644 index 00000000000..a03b89ca2f5 --- /dev/null +++ b/spec/models/packages/composer/cache_file_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Packages::Composer::CacheFile, type: :model do + describe 'relationships' do + it { is_expected.to belong_to(:group) } + it { is_expected.to belong_to(:namespace) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:namespace) } + end + + describe 'scopes' do + let_it_be(:group1) { create(:group) } + let_it_be(:group2) { create(:group) } + let_it_be(:cache_file1) { create(:composer_cache_file, file_sha256: '123456', group: group1) } + let_it_be(:cache_file2) { create(:composer_cache_file, delete_at: 2.days.from_now, file_sha256: '456778', group: group2) } + + describe '.with_namespace' do + subject { described_class.with_namespace(group1) } + + it { is_expected.to eq [cache_file1] } + end + + describe '.with_sha' do + subject { described_class.with_sha('123456') } + + it { is_expected.to eq [cache_file1] } + end + end +end diff --git a/spec/models/packages/composer/metadatum_spec.rb b/spec/models/packages/composer/metadatum_spec.rb index ae53532696b..1c888f1563c 100644 --- a/spec/models/packages/composer/metadatum_spec.rb +++ b/spec/models/packages/composer/metadatum_spec.rb @@ -11,4 +11,20 @@ RSpec.describe Packages::Composer::Metadatum, type: :model do it { is_expected.to validate_presence_of(:target_sha) } it { is_expected.to validate_presence_of(:composer_json) } end + + describe 'scopes' do + let_it_be(:package_name) { 'sample-project' } + let_it_be(:json) { { 'name' => package_name } } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :custom_repo, files: { 'composer.json' => json.to_json }, group: group) } + let_it_be(:package1) { create(:composer_package, :with_metadatum, project: project, name: package_name, version: '1.0.0', json: json) } + let_it_be(:package2) { create(:composer_package, :with_metadatum, project: project, name: 'other-name', version: '1.0.0', json: json) } + let_it_be(:package3) { create(:pypi_package, name: package_name, project: project) } + + describe '.for_package' do + subject { described_class.for_package(package_name, project.id) } + + it { is_expected.to eq [package1.composer_metadatum] } + end + end end diff --git a/spec/models/packages/debian/group_component_file_spec.rb b/spec/models/packages/debian/group_component_file_spec.rb new file mode 100644 index 00000000000..bf33ca138c3 --- /dev/null +++ b/spec/models/packages/debian/group_component_file_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::GroupComponentFile do + it_behaves_like 'Debian Component File', :group, false +end diff --git a/spec/models/packages/debian/group_component_spec.rb b/spec/models/packages/debian/group_component_spec.rb new file mode 100644 index 00000000000..f288ebbe5df --- /dev/null +++ b/spec/models/packages/debian/group_component_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::GroupComponent do + it_behaves_like 'Debian Distribution Component', :debian_group_component, :group, false +end diff --git a/spec/models/packages/debian/project_component_file_spec.rb b/spec/models/packages/debian/project_component_file_spec.rb new file mode 100644 index 00000000000..5dfc47c14c0 --- /dev/null +++ b/spec/models/packages/debian/project_component_file_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::ProjectComponentFile do + it_behaves_like 'Debian Component File', :project, true +end diff --git a/spec/models/packages/debian/project_component_spec.rb b/spec/models/packages/debian/project_component_spec.rb new file mode 100644 index 00000000000..4b041068b8d --- /dev/null +++ b/spec/models/packages/debian/project_component_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::ProjectComponent do + it_behaves_like 'Debian Distribution Component', :debian_project_component, :project, true +end diff --git a/spec/models/packages/debian/publication_spec.rb b/spec/models/packages/debian/publication_spec.rb new file mode 100644 index 00000000000..0ed056f499b --- /dev/null +++ b/spec/models/packages/debian/publication_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Packages::Debian::Publication, type: :model do + let_it_be_with_reload(:publication) { create(:debian_publication) } + + subject { publication } + + describe 'relationships' do + it { is_expected.to belong_to(:package).inverse_of(:debian_publication).class_name('Packages::Package') } + it { is_expected.to belong_to(:distribution).inverse_of(:publications).class_name('Packages::Debian::ProjectDistribution').with_foreign_key(:distribution_id) } + end + + describe 'validations' do + describe '#package' do + it { is_expected.to validate_presence_of(:package) } + end + + describe '#valid_debian_package_type' do + context 'with package type not being Debian' do + before do + publication.package.package_type = 'generic' + end + + it 'will not allow package type not being Debian' do + expect(publication).not_to be_valid + expect(publication.errors.to_a).to eq(['Package type must be Debian']) + end + end + + context 'with package not being a Debian package' do + before do + publication.package.version = nil + end + + it 'will not allow package not being a distribution' do + expect(publication).not_to be_valid + expect(publication.errors.to_a).to eq(['Package must be a Debian package']) + end + end + end + + describe '#distribution' do + it { is_expected.to validate_presence_of(:distribution) } + end + end +end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 6645db33503..6c55d37b95f 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -4,6 +4,8 @@ require 'spec_helper' RSpec.describe Packages::Package, type: :model do include SortingHelper + it_behaves_like 'having unique enum values' + describe 'relationships' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:creator) } @@ -14,7 +16,10 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to have_many(:pipelines).through(:build_infos) } it { is_expected.to have_one(:conan_metadatum).inverse_of(:package) } it { is_expected.to have_one(:maven_metadatum).inverse_of(:package) } + it { is_expected.to have_one(:debian_publication).inverse_of(:package).class_name('Packages::Debian::Publication') } + it { is_expected.to have_one(:debian_distribution).through(:debian_publication).source(:distribution).inverse_of(:packages).class_name('Packages::Debian::ProjectDistribution') } it { is_expected.to have_one(:nuget_metadatum).inverse_of(:package) } + it { is_expected.to have_one(:rubygems_metadatum).inverse_of(:package) } end describe '.with_composer_target' do @@ -374,7 +379,28 @@ RSpec.describe Packages::Package, type: :model do end end - Packages::Package.package_types.keys.without('conan').each do |pt| + describe "#unique_debian_package_name" do + let!(:package) { create(:debian_package) } + + it "will allow a Debian package with same project, name and version, but different distribution" do + new_package = build(:debian_package, project: package.project, name: package.name, version: package.version) + expect(new_package).to be_valid + end + + it "will not allow a Debian package with same project, name, version and distribution" do + new_package = build(:debian_package, project: package.project, name: package.name, version: package.version) + new_package.debian_publication.distribution = package.debian_publication.distribution + expect(new_package).not_to be_valid + expect(new_package.errors.to_a).to include('Debian package already exists in Distribution') + end + + it "will allow a Debian package with same project, name, version, but no distribution" do + new_package = build(:debian_package, project: package.project, name: package.name, version: package.version, published_in: nil) + expect(new_package).to be_valid + end + end + + Packages::Package.package_types.keys.without('conan', 'debian').each do |pt| context "project id, name, version and package type uniqueness for package type #{pt}" do let(:package) { create("#{pt}_package") } @@ -581,6 +607,28 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to match_array([pypi_package]) } end + + describe '.displayable' do + let_it_be(:hidden_package) { create(:maven_package, :hidden) } + let_it_be(:processing_package) { create(:maven_package, :processing) } + + subject { described_class.displayable } + + it 'does not include hidden packages', :aggregate_failures do + is_expected.not_to include(hidden_package) + is_expected.not_to include(processing_package) + end + end + + describe '.with_status' do + let_it_be(:hidden_package) { create(:maven_package, :hidden) } + + subject { described_class.with_status(:hidden) } + + it 'returns packages with specified status' do + is_expected.to match_array([hidden_package]) + end + end end describe '.select_distinct_name' do diff --git a/spec/models/packages/rubygems/metadatum_spec.rb b/spec/models/packages/rubygems/metadatum_spec.rb new file mode 100644 index 00000000000..e99a07c7731 --- /dev/null +++ b/spec/models/packages/rubygems/metadatum_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Packages::Rubygems::Metadatum, type: :model do + describe 'relationships' do + it { is_expected.to belong_to(:package) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:package) } + + describe '#rubygems_package_type' do + it 'will not allow a package with a different package_type' do + package = build('conan_package') + rubygems_metadatum = build('rubygems_metadatum', package: package) + + expect(rubygems_metadatum).not_to be_valid + expect(rubygems_metadatum.errors.to_a).to include('Package type must be RubyGems') + end + end + end +end diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 30712af6b32..0a2b04f1a7c 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -56,6 +56,15 @@ RSpec.describe Pages::LookupPath do include_examples 'uses disk storage' + it 'return nil when legacy storage is disabled and there is no deployment' do + stub_feature_flags(pages_serve_from_legacy_storage: false) + expect(Gitlab::ErrorTracking).to receive(:track_exception) + .with(described_class::LegacyStorageDisabledError) + .and_call_original + + expect(source).to eq(nil) + end + context 'when there is pages deployment' do let(:deployment) { create(:pages_deployment, project: project) } @@ -115,6 +124,35 @@ RSpec.describe Pages::LookupPath do include_examples 'uses disk storage' end + + context 'when deployment were created during migration' do + before do + allow(deployment).to receive(:migrated?).and_return(true) + end + + it 'uses deployment from object storage' do + freeze_time do + expect(source).to( + eq({ + type: 'zip', + path: deployment.file.url(expire_at: 1.day.from_now), + global_id: "gid://gitlab/PagesDeployment/#{deployment.id}", + sha256: deployment.file_sha256, + file_size: deployment.size, + file_count: deployment.file_count + }) + ) + end + end + + context 'when pages_serve_from_migrated_zip feature flag is disabled' do + before do + stub_feature_flags(pages_serve_from_migrated_zip: false) + end + + include_examples 'uses disk storage' + end + end end end diff --git a/spec/models/pages/virtual_domain_spec.rb b/spec/models/pages/virtual_domain_spec.rb index 38f5f4d2538..29c14cbeb3e 100644 --- a/spec/models/pages/virtual_domain_spec.rb +++ b/spec/models/pages/virtual_domain_spec.rb @@ -26,31 +26,34 @@ RSpec.describe Pages::VirtualDomain do describe '#lookup_paths' do let(:project_a) { instance_double(Project) } - let(:project_z) { instance_double(Project) } - let(:pages_lookup_path_a) { instance_double(Pages::LookupPath, prefix: 'aaa') } - let(:pages_lookup_path_z) { instance_double(Pages::LookupPath, prefix: 'zzz') } + let(:project_b) { instance_double(Project) } + let(:project_c) { instance_double(Project) } + let(:pages_lookup_path_a) { instance_double(Pages::LookupPath, prefix: 'aaa', source: { type: 'zip', path: 'https://example.com' }) } + let(:pages_lookup_path_b) { instance_double(Pages::LookupPath, prefix: 'bbb', source: { type: 'zip', path: 'https://example.com' }) } + let(:pages_lookup_path_without_source) { instance_double(Pages::LookupPath, prefix: 'ccc', source: nil) } context 'when there is pages domain provided' do let(:domain) { instance_double(PagesDomain) } - subject(:virtual_domain) { described_class.new([project_a, project_z], domain: domain) } + subject(:virtual_domain) { described_class.new([project_a, project_b, project_c], domain: domain) } it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do expect(project_a).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_a) - expect(project_z).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_z) + expect(project_b).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_b) + expect(project_c).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_without_source) - expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_z, pages_lookup_path_a]) + expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_b, pages_lookup_path_a]) end end context 'when there is trim_prefix provided' do - subject(:virtual_domain) { described_class.new([project_a, project_z], trim_prefix: 'group/') } + subject(:virtual_domain) { described_class.new([project_a, project_b], trim_prefix: 'group/') } it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do expect(project_a).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_a) - expect(project_z).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_z) + expect(project_b).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_b) - expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_z, pages_lookup_path_a]) + expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_b, pages_lookup_path_a]) end end end diff --git a/spec/models/pages_deployment_spec.rb b/spec/models/pages_deployment_spec.rb index e83cbc15004..029eb8e513a 100644 --- a/spec/models/pages_deployment_spec.rb +++ b/spec/models/pages_deployment_spec.rb @@ -26,6 +26,46 @@ RSpec.describe PagesDeployment do end end + describe '.migrated_from_legacy_storage' do + it 'only returns migrated deployments' do + project = create(:project) + migrated_deployment = create_migrated_deployment(project) + # create one other deployment + create(:pages_deployment, project: project) + + expect(described_class.migrated_from_legacy_storage).to eq([migrated_deployment]) + end + end + + describe '#migrated?' do + it 'returns false for normal deployment' do + deployment = create(:pages_deployment) + + expect(deployment.migrated?).to eq(false) + end + + it 'returns true for migrated deployment' do + project = create(:project) + deployment = create_migrated_deployment(project) + + expect(deployment.migrated?).to eq(true) + end + end + + def create_migrated_deployment(project) + public_path = File.join(project.pages_path, "public") + FileUtils.mkdir_p(public_path) + File.open(File.join(project.pages_path, "public/index.html"), "w") do |f| + f.write("Hello!") + end + + expect(::Pages::MigrateLegacyStorageToDeploymentService.new(project).execute[:status]).to eq(:success) + + project.reload.pages_metadatum.pages_deployment + ensure + FileUtils.rm_rf(public_path) + end + describe 'default for file_store' do let(:project) { create(:project) } let(:deployment) do diff --git a/spec/models/project_ci_cd_setting_spec.rb b/spec/models/project_ci_cd_setting_spec.rb index 698465e854a..406485d8cc8 100644 --- a/spec/models/project_ci_cd_setting_spec.rb +++ b/spec/models/project_ci_cd_setting_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe ProjectCiCdSetting do + using RSpec::Parameterized::TableSyntax + describe 'validations' do it 'validates default_git_depth is between 0 and 1000 or nil' do expect(subject).to validate_numericality_of(:default_git_depth) @@ -36,4 +38,39 @@ RSpec.describe ProjectCiCdSetting do expect(project.reload.ci_cd_settings.default_git_depth).to eq(0) end end + + describe '#keep_latest_artifacts_available?' do + let(:attrs) { { keep_latest_artifact: project_enabled } } + let(:project_settings) { described_class.new(attrs) } + + subject { project_settings.keep_latest_artifacts_available? } + + context 'without application setting record' do + where(:project_enabled, :result_keep_latest_artifact) do + false | false + true | true + end + + with_them do + it { expect(subject).to eq(result_keep_latest_artifact) } + end + end + + context 'with application setting record' do + where(:instance_enabled, :project_enabled, :result_keep_latest_artifact) do + false | false | false + false | true | false + true | false | false + true | true | true + end + + before do + Gitlab::CurrentSettings.current_application_settings.update!(keep_latest_artifact: instance_enabled) + end + + with_them do + it { expect(subject).to eq(result_keep_latest_artifact) } + end + end + end end diff --git a/spec/models/project_services/alerts_service_spec.rb b/spec/models/project_services/alerts_service_spec.rb deleted file mode 100644 index 75b91c29914..00000000000 --- a/spec/models/project_services/alerts_service_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -# AlertsService is stripped down to only required methods -# to avoid errors loading integration-related pages if -# records are present. -RSpec.describe AlertsService do - let_it_be(:project) { create(:project) } - subject(:service) { described_class.new(project: project) } - - it { is_expected.to be_valid } - - describe '#to_param' do - subject { service.to_param } - - it { is_expected.to eq('alerts') } - end - - describe '#supported_events' do - subject { service.supported_events } - - it { is_expected.to be_empty } - end - - describe '#save' do - it 'prevents records from being created or updated' do - expect(Gitlab::ProjectServiceLogger).to receive(:error).with( - hash_including(message: 'Prevented attempt to save or update deprecated AlertsService') - ) - - expect(service.save).to be_falsey - - expect(service.errors.full_messages).to include( - 'Alerts endpoint is deprecated and should not be created or modified. Use HTTP Integrations instead.' - ) - end - end -end diff --git a/spec/models/project_services/chat_notification_service_spec.rb b/spec/models/project_services/chat_notification_service_spec.rb index 77a1377c138..476d99364b6 100644 --- a/spec/models/project_services/chat_notification_service_spec.rb +++ b/spec/models/project_services/chat_notification_service_spec.rb @@ -75,6 +75,39 @@ RSpec.describe ChatNotificationService do end end + context 'when the data object has a label' do + let(:label) { create(:label, project: project, name: 'Bug')} + let(:issue) { create(:labeled_issue, project: project, labels: [label]) } + let(:note) { create(:note, noteable: issue, project: project)} + let(:data) { Gitlab::DataBuilder::Note.build(note, user) } + + it 'notifies the chat service' do + expect(chat_service).to receive(:notify).with(any_args) + + chat_service.execute(data) + end + + context 'and the chat_service has a label filter that does not matches the label' do + subject(:chat_service) { described_class.new(labels_to_be_notified: '~some random label') } + + it 'does not notify the chat service' do + expect(chat_service).not_to receive(:notify) + + chat_service.execute(data) + end + end + + context 'and the chat_service has a label filter that matches the label' do + subject(:chat_service) { described_class.new(labels_to_be_notified: '~Backend, ~Bug') } + + it 'notifies the chat service' do + expect(chat_service).to receive(:notify).with(any_args) + + chat_service.execute(data) + end + end + end + context 'with "channel" property' do before do allow(chat_service).to receive(:channel).and_return(channel) diff --git a/spec/models/project_services/confluence_service_spec.rb b/spec/models/project_services/confluence_service_spec.rb index 5d153b17070..6c7ba2c9f32 100644 --- a/spec/models/project_services/confluence_service_spec.rb +++ b/spec/models/project_services/confluence_service_spec.rb @@ -43,13 +43,13 @@ RSpec.describe ConfluenceService do end end - describe '#detailed_description' do + describe '#help' do it 'can correctly return a link to the project wiki when active' do project = create(:project) subject.project = project subject.active = true - expect(subject.detailed_description).to include(Gitlab::Routing.url_helpers.project_wikis_url(project)) + expect(subject.help).to include(Gitlab::Routing.url_helpers.project_wikis_url(project)) end context 'when the project wiki is not enabled' do @@ -60,7 +60,7 @@ RSpec.describe ConfluenceService do [true, false].each do |active| subject.active = active - expect(subject.detailed_description).to be_nil + expect(subject.help).to be_nil end end end diff --git a/spec/models/project_services/datadog_service_spec.rb b/spec/models/project_services/datadog_service_spec.rb index 1d9f49e4824..d15ea1f351b 100644 --- a/spec/models/project_services/datadog_service_spec.rb +++ b/spec/models/project_services/datadog_service_spec.rb @@ -11,7 +11,7 @@ RSpec.describe DatadogService, :model do let(:active) { true } let(:dd_site) { 'datadoghq.com' } let(:default_url) { 'https://webhooks-http-intake.logs.datadoghq.com/v1/input/' } - let(:api_url) { nil } + let(:api_url) { '' } let(:api_key) { SecureRandom.hex(32) } let(:dd_env) { 'ci' } let(:dd_service) { 'awesome-gitlab' } @@ -22,13 +22,11 @@ RSpec.describe DatadogService, :model do described_class.new( active: active, project: project, - properties: { - datadog_site: dd_site, - api_url: api_url, - api_key: api_key, - datadog_env: dd_env, - datadog_service: dd_service - } + datadog_site: dd_site, + api_url: api_url, + api_key: api_key, + datadog_env: dd_env, + datadog_service: dd_service ) end @@ -58,7 +56,7 @@ RSpec.describe DatadogService, :model do context 'when selecting site' do let(:dd_site) { 'datadoghq.com' } - let(:api_url) { nil } + let(:api_url) { '' } it { is_expected.to validate_presence_of(:datadog_site) } it { is_expected.not_to validate_presence_of(:api_url) } @@ -66,7 +64,7 @@ RSpec.describe DatadogService, :model do end context 'with custom api_url' do - let(:dd_site) { nil } + let(:dd_site) { '' } let(:api_url) { 'https://webhooks-http-intake.logs.datad0g.com/v1/input/' } it { is_expected.not_to validate_presence_of(:datadog_site) } @@ -76,13 +74,21 @@ RSpec.describe DatadogService, :model do end context 'when missing site and api_url' do - let(:dd_site) { nil } - let(:api_url) { nil } + let(:dd_site) { '' } + let(:api_url) { '' } it { is_expected.not_to be_valid } it { is_expected.to validate_presence_of(:datadog_site) } it { is_expected.to validate_presence_of(:api_url) } end + + context 'when providing both site and api_url' do + let(:dd_site) { 'datadoghq.com' } + let(:api_url) { default_url } + + it { is_expected.not_to allow_value('datadog hq.com').for(:datadog_site) } + it { is_expected.not_to allow_value('example.com').for(:api_url) } + end end context 'when service is not active' do @@ -113,8 +119,8 @@ RSpec.describe DatadogService, :model do end context 'without optional params' do - let(:dd_service) { nil } - let(:dd_env) { nil } + let(:dd_service) { '' } + let(:dd_env) { '' } it { is_expected.to eq(default_url + api_key) } end @@ -126,7 +132,7 @@ RSpec.describe DatadogService, :model do it { is_expected.to eq("https://app.#{dd_site}/account/settings#api") } context 'with unset datadog_site' do - let(:dd_site) { nil } + let(:dd_site) { '' } it { is_expected.to eq("https://docs.datadoghq.com/account_management/api-app-keys/") } end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index cd0873bddd2..78bd0e91208 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -6,6 +6,8 @@ RSpec.describe JiraService do include AssetsHelpers let_it_be(:project) { create(:project, :repository) } + + let(:current_user) { build_stubbed(:user) } let(:url) { 'http://jira.example.com' } let(:api_url) { 'http://api-jira.example.com' } let(:username) { 'jira-username' } @@ -456,6 +458,16 @@ RSpec.describe JiraService do expect(WebMock).to have_requested(:get, issue_url) end + + context 'with options' do + let(:issue_url) { "#{url}/rest/api/2/issue/#{issue_key}?expand=renderedFields" } + + it 'calls the Jira API with the options to get the issue' do + jira_service.find_issue(issue_key, rendered_fields: true) + + expect(WebMock).to have_requested(:get, issue_url) + end + end end describe '#close_issue' do @@ -498,25 +510,38 @@ RSpec.describe JiraService do WebMock.stub_request(:post, @remote_link_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)) end + let(:external_issue) { ExternalIssue.new('JIRA-123', project) } + + def close_issue + @jira_service.close_issue(resource, external_issue, current_user) + end + it 'calls Jira API' do - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue expect(WebMock).to have_requested(:post, @comment_url).with( body: /Issue solved with/ ).once end + it 'tracks usage' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with('i_ecosystem_jira_service_close_issue', values: current_user.id) + + close_issue + end + it 'does not fail if remote_link.all on issue returns nil' do allow(JIRA::Resource::Remotelink).to receive(:all).and_return(nil) - expect { @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) } - .not_to raise_error + expect { close_issue }.not_to raise_error end # Check https://developer.atlassian.com/jiradev/jira-platform/guides/other/guide-jira-remote-issue-links/fields-in-remote-issue-links # for more information it 'creates Remote Link reference in Jira for comment' do - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue favicon_path = "http://localhost/assets/#{find_asset('favicon.png').digest_path}" @@ -540,7 +565,7 @@ RSpec.describe JiraService do context 'when "comment_on_event_enabled" is set to false' do it 'creates Remote Link reference but does not create comment' do allow(@jira_service).to receive_messages(comment_on_event_enabled: false) - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue expect(WebMock).not_to have_requested(:post, @comment_url) expect(WebMock).to have_requested(:post, @remote_link_url) @@ -562,7 +587,7 @@ RSpec.describe JiraService do expect(remote_link).to receive(:save!) - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue expect(WebMock).not_to have_requested(:post, @comment_url) end @@ -571,7 +596,7 @@ RSpec.describe JiraService do it 'does not send comment or remote links to issues already closed' do allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(true) - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue expect(WebMock).not_to have_requested(:post, @comment_url) expect(WebMock).not_to have_requested(:post, @remote_link_url) @@ -580,7 +605,7 @@ RSpec.describe JiraService do it 'does not send comment or remote links to issues with unknown resolution' do allow_any_instance_of(JIRA::Resource::Issue).to receive(:respond_to?).with(:resolution).and_return(false) - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue expect(WebMock).not_to have_requested(:post, @comment_url) expect(WebMock).not_to have_requested(:post, @remote_link_url) @@ -589,7 +614,7 @@ RSpec.describe JiraService do it 'references the GitLab commit' do stub_config_setting(base_url: custom_base_url) - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue expect(WebMock).to have_requested(:post, @comment_url).with( body: %r{#{custom_base_url}/#{project.full_path}/-/commit/#{commit_id}} @@ -604,7 +629,7 @@ RSpec.describe JiraService do { script_name: '/gitlab' } end - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue expect(WebMock).to have_requested(:post, @comment_url).with( body: %r{#{Gitlab.config.gitlab.url}/#{project.full_path}/-/commit/#{commit_id}} @@ -615,7 +640,7 @@ RSpec.describe JiraService do allow(@jira_service).to receive(:log_error) WebMock.stub_request(:post, @transitions_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)).and_raise("Bad Request") - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue expect(@jira_service).to have_received(:log_error).with( "Issue transition failed", @@ -628,7 +653,7 @@ RSpec.describe JiraService do end it 'calls the api with jira_issue_transition_id' do - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue expect(WebMock).to have_requested(:post, @transitions_url).with( body: /999/ @@ -639,7 +664,7 @@ RSpec.describe JiraService do it 'calls the api with transition ids separated by comma' do allow(@jira_service).to receive_messages(jira_issue_transition_id: '1,2,3') - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue 1.upto(3) do |transition_id| expect(WebMock).to have_requested(:post, @transitions_url).with( @@ -651,7 +676,7 @@ RSpec.describe JiraService do it 'calls the api with transition ids separated by semicolon' do allow(@jira_service).to receive_messages(jira_issue_transition_id: '1;2;3') - @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + close_issue 1.upto(3) do |transition_id| expect(WebMock).to have_requested(:post, @transitions_url).with( @@ -702,6 +727,14 @@ RSpec.describe JiraService do body: /mentioned this issue in/ ).once end + + it 'tracks usage' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with('i_ecosystem_jira_service_cross_reference', values: user.id) + + subject + end end context 'when resource is a commit' do diff --git a/spec/models/project_services/jira_tracker_data_spec.rb b/spec/models/project_services/jira_tracker_data_spec.rb index f2e2fa65e93..46194efcb3d 100644 --- a/spec/models/project_services/jira_tracker_data_spec.rb +++ b/spec/models/project_services/jira_tracker_data_spec.rb @@ -3,13 +3,28 @@ require 'spec_helper' RSpec.describe JiraTrackerData do - let(:service) { build(:jira_service) } - - describe 'Associations' do + describe 'associations' do it { is_expected.to belong_to(:service) } end describe 'deployment_type' do it { is_expected.to define_enum_for(:deployment_type).with_values([:unknown, :server, :cloud]).with_prefix(:deployment) } end + + describe 'proxy settings' do + it { is_expected.to validate_length_of(:proxy_address).is_at_most(2048) } + it { is_expected.to validate_length_of(:proxy_port).is_at_most(5) } + it { is_expected.to validate_length_of(:proxy_username).is_at_most(255) } + it { is_expected.to validate_length_of(:proxy_password).is_at_most(255) } + end + + describe 'encrypted attributes' do + subject { described_class.encrypted_attributes.keys } + + it { + is_expected.to contain_exactly( + :api_url, :password, :proxy_address, :proxy_password, :proxy_port, :proxy_username, :url, :username + ) + } + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a2b51684d4d..fd7975bf65d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -21,6 +21,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:services) } it { is_expected.to have_many(:events) } it { is_expected.to have_many(:merge_requests) } + it { is_expected.to have_many(:merge_request_metrics).class_name('MergeRequest::Metrics') } it { is_expected.to have_many(:issues) } it { is_expected.to have_many(:milestones) } it { is_expected.to have_many(:iterations) } @@ -558,6 +559,25 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#default_pipeline_lock' do + let(:project) { build_stubbed(:project) } + + subject { project.default_pipeline_lock } + + where(:keep_latest_artifact_enabled, :result_pipeline_locked) do + false | :unlocked + true | :artifacts_locked + end + + before do + allow(project).to receive(:keep_latest_artifacts_available?).and_return(keep_latest_artifact_enabled) + end + + with_them do + it { expect(subject).to eq(result_pipeline_locked) } + end + end + describe '#autoclose_referenced_issues' do context 'when DB entry is nil' do let(:project) { build(:project, autoclose_referenced_issues: nil) } @@ -1002,103 +1022,125 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#external_issue_tracker' do - let(:project) { create(:project) } - let(:ext_project) { create(:redmine_project) } + describe '#has_wiki?' do + let(:no_wiki_project) { create(:project, :wiki_disabled, has_external_wiki: false) } + let(:wiki_enabled_project) { create(:project) } + let(:external_wiki_project) { create(:project, has_external_wiki: true) } - context 'on existing projects with no value for has_external_issue_tracker' do - before do - project.update_column(:has_external_issue_tracker, nil) - ext_project.update_column(:has_external_issue_tracker, nil) + it 'returns true if project is wiki enabled or has external wiki' do + expect(wiki_enabled_project).to have_wiki + expect(external_wiki_project).to have_wiki + expect(no_wiki_project).not_to have_wiki + end + end + + describe '#default_owner' do + let_it_be(:owner) { create(:user) } + let_it_be(:namespace) { create(:namespace, owner: owner) } + + context 'the project does not have a group' do + let(:project) { build(:project, namespace: namespace) } + + it 'is the namespace owner' do + expect(project.default_owner).to eq(owner) end + end - it 'updates the has_external_issue_tracker boolean' do - expect do - project.external_issue_tracker - end.to change { project.reload.has_external_issue_tracker }.to(false) + context 'the project is in a group' do + let(:group) { build(:group) } + let(:project) { build(:project, group: group, namespace: namespace) } - expect do - ext_project.external_issue_tracker - end.to change { ext_project.reload.has_external_issue_tracker }.to(true) + it 'is the group owner' do + allow(group).to receive(:default_owner).and_return(Object.new) + + expect(project.default_owner).to eq(group.default_owner) end end + end + + describe '#external_issue_tracker' do + it 'sets Project#has_external_issue_tracker when it is nil' do + project_with_no_tracker = create(:project, has_external_issue_tracker: nil) + project_with_tracker = create(:redmine_project, has_external_issue_tracker: nil) + + expect do + project_with_no_tracker.external_issue_tracker + end.to change { project_with_no_tracker.reload.has_external_issue_tracker }.from(nil).to(false) + + expect do + project_with_tracker.external_issue_tracker + end.to change { project_with_tracker.reload.has_external_issue_tracker }.from(nil).to(true) + end it 'returns nil and does not query services when there is no external issue tracker' do - expect(project).not_to receive(:services) + project = create(:project) + expect(project).not_to receive(:services) expect(project.external_issue_tracker).to eq(nil) end it 'retrieves external_issue_tracker querying services and cache it when there is external issue tracker' do - ext_project.reload # Factory returns a project with changed attributes - expect(ext_project).to receive(:services).once.and_call_original + project = create(:redmine_project) - 2.times { expect(ext_project.external_issue_tracker).to be_a_kind_of(RedmineService) } + expect(project).to receive(:services).once.and_call_original + 2.times { expect(project.external_issue_tracker).to be_a_kind_of(RedmineService) } end end - describe '#cache_has_external_issue_tracker' do - let_it_be(:project) { create(:project, has_external_issue_tracker: nil) } - - it 'stores true if there is any external_issue_tracker' do - services = double(:service, external_issue_trackers: [RedmineService.new]) - expect(project).to receive(:services).and_return(services) + describe '#has_external_issue_tracker' do + let_it_be(:project) { create(:project) } - expect do - project.cache_has_external_issue_tracker - end.to change { project.has_external_issue_tracker}.to(true) + def subject + project.reload.has_external_issue_tracker end - it 'stores false if there is no external_issue_tracker' do - services = double(:service, external_issue_trackers: []) - expect(project).to receive(:services).and_return(services) + it 'is false when external issue tracker service is not active' do + create(:service, project: project, category: 'issue_tracker', active: false) - expect do - project.cache_has_external_issue_tracker - end.to change { project.has_external_issue_tracker}.to(false) + is_expected.to eq(false) end - it 'does not cache data when in a read-only GitLab instance' do - allow(Gitlab::Database).to receive(:read_only?) { true } + it 'is false when other service is active' do + create(:service, project: project, category: 'not_issue_tracker', active: true) - expect do - project.cache_has_external_issue_tracker - end.not_to change { project.has_external_issue_tracker } + is_expected.to eq(false) end - end - describe '#has_wiki?' do - let(:no_wiki_project) { create(:project, :wiki_disabled, has_external_wiki: false) } - let(:wiki_enabled_project) { create(:project) } - let(:external_wiki_project) { create(:project, has_external_wiki: true) } - - it 'returns true if project is wiki enabled or has external wiki' do - expect(wiki_enabled_project).to have_wiki - expect(external_wiki_project).to have_wiki - expect(no_wiki_project).not_to have_wiki - end - end + context 'when there is an active external issue tracker service' do + let!(:service) do + create(:service, project: project, type: 'JiraService', category: 'issue_tracker', active: true) + end - describe '#default_owner' do - let_it_be(:owner) { create(:user) } - let_it_be(:namespace) { create(:namespace, owner: owner) } + specify { is_expected.to eq(true) } - context 'the project does not have a group' do - let(:project) { build(:project, namespace: namespace) } + it 'becomes false when external issue tracker service is destroyed' do + expect do + Service.find(service.id).delete + end.to change { subject }.to(false) + end - it 'is the namespace owner' do - expect(project.default_owner).to eq(owner) + it 'becomes false when external issue tracker service becomes inactive' do + expect do + service.update_column(:active, false) + end.to change { subject }.to(false) end - end - context 'the project is in a group' do - let(:group) { build(:group) } - let(:project) { build(:project, group: group, namespace: namespace) } + context 'when there are two active external issue tracker services' do + let_it_be(:second_service) do + create(:service, project: project, type: 'CustomIssueTracker', category: 'issue_tracker', active: true) + end - it 'is the group owner' do - allow(group).to receive(:default_owner).and_return(Object.new) + it 'does not become false when external issue tracker service is destroyed' do + expect do + Service.find(service.id).delete + end.not_to change { subject } + end - expect(project.default_owner).to eq(group.default_owner) + it 'does not become false when external issue tracker service becomes inactive' do + expect do + service.update_column(:active, false) + end.not_to change { subject } + end end end end @@ -1234,7 +1276,7 @@ RSpec.describe Project, factory_default: :keep do it 'is false if avatar is html page' do project.update_attribute(:avatar, 'uploads/avatar.html') - expect(project.avatar_type).to eq(['file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico']) + expect(project.avatar_type).to eq(['file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico, webp']) end end @@ -1529,10 +1571,7 @@ RSpec.describe Project, factory_default: :keep do let(:project) { build(:project) } it 'picks storage from ApplicationSetting' do - expect_next_instance_of(ApplicationSetting) do |instance| - expect(instance).to receive(:pick_repository_storage).and_return('picked') - end - expect(described_class).to receive(:pick_repository_storage).and_call_original + expect(Repository).to receive(:pick_storage_shard).and_return('picked') expect(project.repository_storage).to eq('picked') end @@ -2227,8 +2266,6 @@ RSpec.describe Project, factory_default: :keep do end describe '#ci_config_path=' do - using RSpec::Parameterized::TableSyntax - let(:project) { build_stubbed(:project) } where(:default_ci_config_path, :project_ci_config_path, :expected_ci_config_path) do @@ -2980,6 +3017,7 @@ RSpec.describe Project, factory_default: :keep do it_behaves_like 'can housekeep repository' do let(:resource) { build_stubbed(:project) } let(:resource_key) { 'projects' } + let(:expected_worker_class) { Projects::GitGarbageCollectWorker } end describe '#deployment_variables' do @@ -3926,7 +3964,6 @@ RSpec.describe Project, factory_default: :keep do describe '.filter_by_feature_visibility' do include_context 'ProjectPolicyTable context' include ProjectHelpers - using RSpec::Parameterized::TableSyntax let_it_be(:group) { create(:group) } let!(:project) { create(:project, project_level, namespace: group ) } @@ -4099,7 +4136,7 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#remove_pages' do + describe '#legacy_remove_pages' do let(:project) { create(:project).tap { |project| project.mark_pages_as_deployed } } let(:pages_metadatum) { project.pages_metadatum } let(:namespace) { project.namespace } @@ -4118,34 +4155,22 @@ RSpec.describe Project, factory_default: :keep do expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return(true) expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, namespace.full_path, anything) - expect { project.remove_pages }.to change { pages_metadatum.reload.deployed }.from(true).to(false) + expect { project.legacy_remove_pages }.to change { pages_metadatum.reload.deployed }.from(true).to(false) end - it 'is run when the project is destroyed' do - expect(project).to receive(:remove_pages).and_call_original - - expect { project.destroy }.not_to raise_error - end + it 'does nothing if updates on legacy storage are disabled' do + stub_feature_flags(pages_update_legacy_storage: false) - context 'when there is an old pages deployment' do - let!(:old_deployment_from_another_project) { create(:pages_deployment) } - let!(:old_deployment) { create(:pages_deployment, project: project) } + expect(Gitlab::PagesTransfer).not_to receive(:new) + expect(PagesWorker).not_to receive(:perform_in) - it 'schedules a destruction of pages deployments' do - expect(DestroyPagesDeploymentsWorker).to( - receive(:perform_async).with(project.id) - ) - - project.remove_pages - end + project.legacy_remove_pages + end - it 'removes pages deployments', :sidekiq_inline do - expect do - project.remove_pages - end.to change { PagesDeployment.count }.by(-1) + it 'is run when the project is destroyed' do + expect(project).to receive(:legacy_remove_pages).and_call_original - expect(PagesDeployment.find_by_id(old_deployment.id)).to be_nil - end + expect { project.destroy }.not_to raise_error end end @@ -4176,8 +4201,6 @@ RSpec.describe Project, factory_default: :keep do end describe '#git_transfer_in_progress?' do - using RSpec::Parameterized::TableSyntax - let(:project) { build(:project) } subject { project.git_transfer_in_progress? } @@ -5068,10 +5091,8 @@ RSpec.describe Project, factory_default: :keep do it 'executes services with the specified scope' do data = 'any data' - expect(SlackService).to receive(:allocate).and_wrap_original do |method| - method.call.tap do |instance| - expect(instance).to receive(:async_execute).with(data).once - end + expect_next_found_instance_of(SlackService) do |instance| + expect(instance).to receive(:async_execute).with(data).once end service.project.execute_services(data, :push_hooks) @@ -5801,8 +5822,6 @@ RSpec.describe Project, factory_default: :keep do end describe 'validation #changing_shared_runners_enabled_is_allowed' do - using RSpec::Parameterized::TableSyntax - where(:shared_runners_setting, :project_shared_runners_enabled, :valid_record) do 'enabled' | true | true 'enabled' | false | true @@ -6025,8 +6044,6 @@ RSpec.describe Project, factory_default: :keep do end describe '#closest_setting' do - using RSpec::Parameterized::TableSyntax - shared_examples_for 'fetching closest setting' do let!(:namespace) { create(:namespace) } let!(:project) { create(:project, namespace: namespace) } @@ -6378,20 +6395,7 @@ RSpec.describe Project, factory_default: :keep do describe 'with Debian Distributions' do subject { create(:project) } - let!(:distributions) { create_list(:debian_project_distribution, 2, :with_file, container: subject) } - - it 'removes distribution files on removal' do - distribution_file_paths = distributions.map do |distribution| - distribution.file.path - end - - expect { subject.destroy } - .to change { - distribution_file_paths.select do |path| - File.exist? path - end.length - }.from(distribution_file_paths.length).to(0) - end + it_behaves_like 'model with Debian distributions' end describe '#environments_for_scope' do diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 8001d009901..c04fc70deca 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -47,5 +47,6 @@ RSpec.describe ProjectWiki do let_it_be(:resource) { create(:project_wiki) } let(:resource_key) { 'project_wikis' } + let(:expected_worker_class) { Wikis::GitGarbageCollectWorker } end end diff --git a/spec/models/prometheus_metric_spec.rb b/spec/models/prometheus_metric_spec.rb index 9588167bbcc..a20f4edcf4a 100644 --- a/spec/models/prometheus_metric_spec.rb +++ b/spec/models/prometheus_metric_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe PrometheusMetric do + using RSpec::Parameterized::TableSyntax + subject { build(:prometheus_metric) } it_behaves_like 'having unique enum values' @@ -14,8 +16,6 @@ RSpec.describe PrometheusMetric do it { is_expected.to validate_uniqueness_of(:identifier).scoped_to(:project_id).allow_nil } describe 'common metrics' do - using RSpec::Parameterized::TableSyntax - where(:common, :with_project, :result) do false | true | true false | false | false @@ -34,8 +34,6 @@ RSpec.describe PrometheusMetric do end describe '#query_series' do - using RSpec::Parameterized::TableSyntax - where(:legend, :type) do 'Some other legend' | NilClass 'Status Code' | Array @@ -72,8 +70,6 @@ RSpec.describe PrometheusMetric do end describe '#priority' do - using RSpec::Parameterized::TableSyntax - where(:group, :priority) do :nginx_ingress_vts | 10 :nginx_ingress | 10 @@ -97,8 +93,6 @@ RSpec.describe PrometheusMetric do end describe '#required_metrics' do - using RSpec::Parameterized::TableSyntax - where(:group, :required_metrics) do :nginx_ingress_vts | %w(nginx_upstream_responses_total nginx_upstream_response_msecs_avg) :nginx_ingress | %w(nginx_ingress_controller_requests nginx_ingress_controller_ingress_upstream_latency_seconds_sum) diff --git a/spec/models/protected_branch/push_access_level_spec.rb b/spec/models/protected_branch/push_access_level_spec.rb index 051cb78a6b6..17a589f0485 100644 --- a/spec/models/protected_branch/push_access_level_spec.rb +++ b/spec/models/protected_branch/push_access_level_spec.rb @@ -54,16 +54,6 @@ RSpec.describe ProtectedBranch::PushAccessLevel do specify do expect(push_access_level.check_access(user)).to be_truthy end - - context 'when the deploy_keys_on_protected_branches FF is false' do - before do - stub_feature_flags(deploy_keys_on_protected_branches: false) - end - - it 'is false' do - expect(push_access_level.check_access(user)).to be_falsey - end - end end context 'when the deploy key is not among the active keys of this project' do diff --git a/spec/models/readme_blob_spec.rb b/spec/models/readme_blob_spec.rb deleted file mode 100644 index 95622d55254..00000000000 --- a/spec/models/readme_blob_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ReadmeBlob do - include FakeBlobHelpers - - describe 'policy' do - let(:project) { build(:project, :repository) } - - subject { described_class.new(fake_blob(path: 'README.md'), project.repository) } - - it 'works with policy' do - expect(Ability.allowed?(project.creator, :read_blob, subject)).to be_truthy - end - end -end diff --git a/spec/models/release_highlight_spec.rb b/spec/models/release_highlight_spec.rb index 749b9b8e1ab..60087278671 100644 --- a/spec/models/release_highlight_spec.rb +++ b/spec/models/release_highlight_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ReleaseHighlight do +RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache do let(:fixture_dir_glob) { Dir.glob(File.join('spec', 'fixtures', 'whats_new', '*.yml')).grep(/\d*\_(\d*\_\d*)\.yml$/) } before do diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index b436c2e1088..209ac471210 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Release do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :public, :repository) } - let_it_be(:release) { create(:release, project: project, author: user) } + let(:release) { create(:release, project: project, author: user) } it { expect(release).to be_valid } @@ -89,6 +89,61 @@ RSpec.describe Release do end end + describe '#update' do + subject { release.update(params) } + + context 'when links do not exist' do + context 'when params are specified for creation' do + let(:params) do + { links_attributes: [{ name: 'test', url: 'https://www.google.com/' }] } + end + + it 'creates a link successfuly' do + is_expected.to eq(true) + + expect(release.links.count).to eq(1) + expect(release.links.first.name).to eq('test') + expect(release.links.first.url).to eq('https://www.google.com/') + end + end + end + + context 'when a link exists' do + let!(:link1) { create(:release_link, release: release, name: 'test1', url: 'https://www.google1.com/') } + let!(:link2) { create(:release_link, release: release, name: 'test2', url: 'https://www.google2.com/') } + + before do + release.reload + end + + context 'when params are specified for update' do + let(:params) do + { links_attributes: [{ id: link1.id, name: 'new' }] } + end + + it 'updates the link successfully' do + is_expected.to eq(true) + + expect(release.links.count).to eq(2) + expect(release.links.first.name).to eq('new') + end + end + + context 'when params are specified for deletion' do + let(:params) do + { links_attributes: [{ id: link1.id, _destroy: true }] } + end + + it 'removes the link successfuly' do + is_expected.to eq(true) + + expect(release.links.count).to eq(1) + expect(release.links.first.name).to eq(link2.name) + end + end + end + end + describe '#sources' do subject { release.sources } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index dd54a701282..3a4de7ba279 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -483,12 +483,6 @@ RSpec.describe Repository do it { is_expected.to be_an_instance_of(::Blob) } end - context 'readme blob on HEAD' do - subject { repository.blob_at(repository.head_commit.sha, 'README.md') } - - it { is_expected.to be_an_instance_of(::ReadmeBlob) } - end - context 'readme blob not on HEAD' do subject { repository.blob_at(repository.find_branch('feature').target, 'README.md') } @@ -1142,11 +1136,11 @@ RSpec.describe Repository do expect(repository.license_key).to be_nil end - it 'returns nil when the content is not recognizable' do + it 'returns other when the content is not recognizable' do repository.create_file(user, 'LICENSE', 'Gitlab B.V.', message: 'Add LICENSE', branch_name: 'master') - expect(repository.license_key).to be_nil + expect(repository.license_key).to eq('other') end it 'returns nil when the commit SHA does not exist' do @@ -1186,11 +1180,12 @@ RSpec.describe Repository do expect(repository.license).to be_nil end - it 'returns nil when the content is not recognizable' do + it 'returns other when the content is not recognizable' do + license = Licensee::License.new('other') repository.create_file(user, 'LICENSE', 'Gitlab B.V.', message: 'Add LICENSE', branch_name: 'master') - expect(repository.license).to be_nil + expect(repository.license).to eq(license) end it 'returns the license' do @@ -1938,7 +1933,6 @@ RSpec.describe Repository do expect(repository).to receive(:expire_method_caches).with([ :size, :commit_count, - :rendered_readme, :readme_path, :contribution_guide, :changelog, @@ -1955,8 +1949,8 @@ RSpec.describe Repository do :root_ref, :merged_branch_names, :has_visible_content?, - :issue_template_names, - :merge_request_template_names, + :issue_template_names_by_category, + :merge_request_template_names_by_category, :user_defined_metrics_dashboard_paths, :xcode_project?, :has_ambiguous_refs? @@ -2314,14 +2308,6 @@ RSpec.describe Repository do expect(repository.readme).to be_nil end end - - context 'when a README exists' do - let(:project) { create(:project, :repository) } - - it 'returns the README' do - expect(repository.readme).to be_an_instance_of(ReadmeBlob) - end - end end end @@ -2527,9 +2513,8 @@ RSpec.describe Repository do describe '#refresh_method_caches' do it 'refreshes the caches of the given types' do expect(repository).to receive(:expire_method_caches) - .with(%i(rendered_readme readme_path license_blob license_key license)) + .with(%i(readme_path license_blob license_key license)) - expect(repository).to receive(:rendered_readme) expect(repository).to receive(:readme_path) expect(repository).to receive(:license_blob) expect(repository).to receive(:license_key) @@ -3049,4 +3034,51 @@ RSpec.describe Repository do end end end + + describe '.pick_storage_shard', :request_store do + before do + storages = { + 'default' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories'), + 'picked' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories') + } + + allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + Gitlab::CurrentSettings.current_application_settings + + update_storages({ 'picked' => 0, 'default' => 100 }) + end + + context 'when expire is false' do + it 'does not expire existing repository storage value' do + previous_storage = described_class.pick_storage_shard + expect(previous_storage).to eq('default') + expect(Gitlab::CurrentSettings).not_to receive(:expire_current_application_settings) + + update_storages({ 'picked' => 100, 'default' => 0 }) + + new_storage = described_class.pick_storage_shard(expire: false) + expect(new_storage).to eq(previous_storage) + end + end + + context 'when expire is true' do + it 'expires existing repository storage value' do + previous_storage = described_class.pick_storage_shard + expect(previous_storage).to eq('default') + expect(Gitlab::CurrentSettings).to receive(:expire_current_application_settings).and_call_original + + update_storages({ 'picked' => 100, 'default' => 0 }) + + new_storage = described_class.pick_storage_shard(expire: true) + expect(new_storage).to eq('picked') + end + end + + def update_storages(storage_hash) + settings = ApplicationSetting.last + settings.repository_storages_weighted = storage_hash + settings.save! + end + end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 04b3920cd6c..9ffefd4bbf7 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -39,35 +39,29 @@ RSpec.describe Service do end end - context 'with an existing service template' do - before do + context 'with existing services' do + before_all do create(:service, :template) + create(:service, :instance) + create(:service, project: project) + create(:service, group: group, project: nil) end - it 'validates only one service template per type' do + it 'allows only one service template per type' do expect(build(:service, :template)).to be_invalid end - end - context 'with an existing instance service' do - before do - create(:service, :instance) - end - - it 'validates only one service instance per type' do + it 'allows only one instance service per type' do expect(build(:service, :instance)).to be_invalid end - end - it 'validates uniqueness of type and project_id on create' do - expect(create(:service, project: project, type: 'Service')).to be_valid - expect(build(:service, project: project, type: 'Service').valid?(:create)).to eq(false) - expect(build(:service, project: project, type: 'Service').valid?(:update)).to eq(true) - end + it 'allows only one project service per type' do + expect(build(:service, project: project)).to be_invalid + end - it 'validates uniqueness of type and group_id' do - expect(create(:service, group_id: group.id, project_id: nil, type: 'Service')).to be_valid - expect(build(:service, group_id: group.id, project_id: nil, type: 'Service')).to be_invalid + it 'allows only one group service per type' do + expect(build(:service, group: group, project: nil)).to be_invalid + end end end @@ -753,38 +747,6 @@ RSpec.describe Service do end end - describe "callbacks" do - let!(:service) do - RedmineService.new( - project: project, - active: true, - properties: { - project_url: 'http://redmine/projects/project_name_in_redmine', - issues_url: "http://redmine/#{project.id}/project_name_in_redmine/:id", - new_issue_url: 'http://redmine/projects/project_name_in_redmine/issues/new' - } - ) - end - - describe "on create" do - it "updates the has_external_issue_tracker boolean" do - expect do - service.save! - end.to change { service.project.has_external_issue_tracker }.from(false).to(true) - end - end - - describe "on update" do - it "updates the has_external_issue_tracker boolean" do - service.save! - - expect do - service.update(active: false) - end.to change { service.project.has_external_issue_tracker }.from(true).to(false) - end - end - end - describe '#api_field_names' do let(:fake_service) do Class.new(Service) do @@ -864,20 +826,6 @@ RSpec.describe Service do end end - describe '#external_issue_tracker?' do - where(:category, :active, :result) do - :issue_tracker | true | true - :issue_tracker | false | false - :common | true | false - end - - with_them do - it 'returns the right result' do - expect(build(:service, category: category, active: active).external_issue_tracker?).to eq(result) - end - end - end - describe '#external_wiki?' do where(:type, :active, :result) do 'ExternalWikiService' | true | true diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 68d183d5d55..623767d19e0 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -630,14 +630,10 @@ RSpec.describe Snippet do subject { snippet.repository_storage } before do - expect_next_instance_of(ApplicationSetting) do |instance| - expect(instance).to receive(:pick_repository_storage).and_return('picked') - end + expect(Repository).to receive(:pick_storage_shard).and_return('picked') end it 'returns repository storage from ApplicationSetting' do - expect(described_class).to receive(:pick_repository_storage).and_call_original - expect(subject).to eq 'picked' end diff --git a/spec/models/terraform/state_spec.rb b/spec/models/terraform/state_spec.rb index ed311314086..1319e2adb03 100644 --- a/spec/models/terraform/state_spec.rb +++ b/spec/models/terraform/state_spec.rb @@ -8,8 +8,11 @@ RSpec.describe Terraform::State do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:locked_by_user).class_name('User') } + it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:project_id) } + it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) } + describe 'scopes' do describe '.ordered_by_name' do let_it_be(:project) { create(:project) } @@ -25,6 +28,15 @@ RSpec.describe Terraform::State do it { expect(subject.map(&:name)).to eq(names.sort) } end + + describe '.with_name' do + let_it_be(:matching_name) { create(:terraform_state, name: 'matching-name') } + let_it_be(:other_name) { create(:terraform_state, name: 'other-name') } + + subject { described_class.with_name(matching_name.name) } + + it { is_expected.to contain_exactly(matching_name) } + end end describe '#destroy' do diff --git a/spec/models/terraform/state_version_spec.rb b/spec/models/terraform/state_version_spec.rb index 97ac77d5e7b..ac2e8d167b3 100644 --- a/spec/models/terraform/state_version_spec.rb +++ b/spec/models/terraform/state_version_spec.rb @@ -24,6 +24,24 @@ RSpec.describe Terraform::StateVersion do it { expect(subject.map(&:version)).to eq(versions.sort.reverse) } end + + describe '.with_files_stored_locally' do + subject { described_class.with_files_stored_locally } + + it 'includes states with local storage' do + create_list(:terraform_state_version, 5) + + expect(subject).to have_attributes(count: 5) + end + + it 'excludes states without local storage' do + stub_terraform_state_object_storage + + create_list(:terraform_state_version, 5) + + expect(subject).to have_attributes(count: 0) + end + end end context 'file storage' do diff --git a/spec/models/token_with_iv_spec.rb b/spec/models/token_with_iv_spec.rb new file mode 100644 index 00000000000..8dbccc19217 --- /dev/null +++ b/spec/models/token_with_iv_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe TokenWithIv do + describe 'validations' do + it { is_expected.to validate_presence_of :hashed_token } + it { is_expected.to validate_presence_of :iv } + it { is_expected.to validate_presence_of :hashed_plaintext_token } + end + + describe '.find_by_hashed_token' do + it 'only includes matching record' do + matching_record = create(:token_with_iv, hashed_token: ::Digest::SHA256.digest('hashed-token')) + create(:token_with_iv) + + expect(described_class.find_by_hashed_token('hashed-token')).to eq(matching_record) + end + end + + describe '.find_by_plaintext_token' do + it 'only includes matching record' do + matching_record = create(:token_with_iv, hashed_plaintext_token: ::Digest::SHA256.digest('hashed-token')) + create(:token_with_iv) + + expect(described_class.find_by_plaintext_token('hashed-token')).to eq(matching_record) + end + end +end diff --git a/spec/models/u2f_registration_spec.rb b/spec/models/u2f_registration_spec.rb new file mode 100644 index 00000000000..1f2e4d1e447 --- /dev/null +++ b/spec/models/u2f_registration_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe U2fRegistration do + let_it_be(:user) { create(:user) } + let(:u2f_registration) do + device = U2F::FakeU2F.new(FFaker::BaconIpsum.characters(5)) + create(:u2f_registration, name: 'u2f_device', + user: user, + certificate: Base64.strict_encode64(device.cert_raw), + key_handle: U2F.urlsafe_encode64(device.key_handle_raw), + public_key: Base64.strict_encode64(device.origin_public_key_raw)) + end + + describe 'callbacks' do + describe '#create_webauthn_registration' do + it 'creates webauthn registration' do + u2f_registration.save! + + webauthn_registration = WebauthnRegistration.where(u2f_registration_id: u2f_registration.id) + expect(webauthn_registration).to exist + end + + it 'logs error' do + allow(Gitlab::Auth::U2fWebauthnConverter).to receive(:new).and_raise('boom!') + expect(Gitlab::AppJsonLogger).to( + receive(:error).with(a_hash_including(event: 'u2f_migration', + error: 'RuntimeError', + message: 'U2F to WebAuthn conversion failed')) + ) + + u2f_registration.save! + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0935d3576a4..860c015e166 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2477,7 +2477,7 @@ RSpec.describe User do it 'is false if avatar is html page' do user.update_attribute(:avatar, 'uploads/avatar.html') - expect(user.avatar_type).to eq(['file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico']) + expect(user.avatar_type).to eq(['file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico, webp']) end end @@ -2831,6 +2831,79 @@ RSpec.describe User do end end + describe '#following?' do + it 'check if following another user' do + user = create :user + followee1 = create :user + + expect(user.follow(followee1)).to be_truthy + + expect(user.following?(followee1)).to be_truthy + + expect(user.unfollow(followee1)).to be_truthy + + expect(user.following?(followee1)).to be_falsey + end + end + + describe '#follow' do + it 'follow another user' do + user = create :user + followee1 = create :user + followee2 = create :user + + expect(user.followees).to be_empty + + expect(user.follow(followee1)).to be_truthy + expect(user.follow(followee1)).to be_falsey + + expect(user.followees).to contain_exactly(followee1) + + expect(user.follow(followee2)).to be_truthy + expect(user.follow(followee2)).to be_falsey + + expect(user.followees).to contain_exactly(followee1, followee2) + end + + it 'follow itself is not possible' do + user = create :user + + expect(user.followees).to be_empty + + expect(user.follow(user)).to be_falsey + + expect(user.followees).to be_empty + end + end + + describe '#unfollow' do + it 'unfollow another user' do + user = create :user + followee1 = create :user + followee2 = create :user + + expect(user.followees).to be_empty + + expect(user.follow(followee1)).to be_truthy + expect(user.follow(followee1)).to be_falsey + + expect(user.follow(followee2)).to be_truthy + expect(user.follow(followee2)).to be_falsey + + expect(user.followees).to contain_exactly(followee1, followee2) + + expect(user.unfollow(followee1)).to be_truthy + expect(user.unfollow(followee1)).to be_falsey + + expect(user.followees).to contain_exactly(followee2) + + expect(user.unfollow(followee2)).to be_truthy + expect(user.unfollow(followee2)).to be_falsey + + expect(user.followees).to be_empty + end + end + describe '.find_by_private_commit_email' do context 'with email' do let_it_be(:user) { create(:user) } diff --git a/spec/models/user_status_spec.rb b/spec/models/user_status_spec.rb index 2c0664bd165..51dd91149cc 100644 --- a/spec/models/user_status_spec.rb +++ b/spec/models/user_status_spec.rb @@ -17,4 +17,34 @@ RSpec.describe UserStatus do expect { status.user.destroy }.to change { described_class.count }.from(1).to(0) end + + describe '#clear_status_after=' do + it 'sets clear_status_at' do + status = build(:user_status) + + freeze_time do + status.clear_status_after = '8_hours' + + expect(status.clear_status_at).to be_like_time(8.hours.from_now) + end + end + + it 'unsets clear_status_at' do + status = build(:user_status, clear_status_at: 8.hours.from_now) + + status.clear_status_after = nil + + expect(status.clear_status_at).to be_nil + end + + context 'when unknown clear status is given' do + it 'unsets clear_status_at' do + status = build(:user_status, clear_status_at: 8.hours.from_now) + + status.clear_status_after = 'unknown' + + expect(status.clear_status_at).to be_nil + end + end + end end |