diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-01-20 12:16:11 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-01-20 12:16:11 +0300 |
commit | edaa33dee2ff2f7ea3fac488d41558eb5f86d68c (patch) | |
tree | 11f143effbfeba52329fb7afbd05e6e2a3790241 /spec/models | |
parent | d8a5691316400a0f7ec4f83832698f1988eb27c1 (diff) |
Add latest changes from gitlab-org/gitlab@14-7-stable-eev14.7.0-rc42
Diffstat (limited to 'spec/models')
73 files changed, 2746 insertions, 1045 deletions
diff --git a/spec/models/alert_management/alert_spec.rb b/spec/models/alert_management/alert_spec.rb index 35398e29062..40bdfd4bc92 100644 --- a/spec/models/alert_management/alert_spec.rb +++ b/spec/models/alert_management/alert_spec.rb @@ -211,12 +211,6 @@ RSpec.describe AlertManagement::Alert do end end - describe '.open' do - subject { described_class.open } - - it { is_expected.to contain_exactly(acknowledged_alert, triggered_alert) } - end - describe '.not_resolved' do subject { described_class.not_resolved } @@ -324,33 +318,6 @@ RSpec.describe AlertManagement::Alert do end end - describe '.open_status?' do - using RSpec::Parameterized::TableSyntax - - where(:status, :is_open_status) do - :triggered | true - :acknowledged | true - :resolved | false - :ignored | false - nil | false - end - - with_them do - it 'returns true when the status is open status' do - expect(described_class.open_status?(status)).to eq(is_open_status) - end - end - end - - describe '#open?' do - it 'returns true when the status is open status' do - expect(triggered_alert.open?).to be true - expect(acknowledged_alert.open?).to be true - expect(resolved_alert.open?).to be false - expect(ignored_alert.open?).to be false - end - end - describe '#to_reference' do it { expect(triggered_alert.to_reference).to eq("^alert##{triggered_alert.iid}") } end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index f0212da3041..9c9a048999c 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -147,22 +147,20 @@ RSpec.describe ApplicationRecord do end end - # rubocop:disable Database/MultipleDatabases it 'increments a counter when a transaction is created in ActiveRecord' do expect(described_class.connection.transaction_open?).to be false expect(::Gitlab::Database::Metrics) .to receive(:subtransactions_increment) - .with('ActiveRecord::Base') + .with('ApplicationRecord') .once - ActiveRecord::Base.transaction do - ActiveRecord::Base.transaction(requires_new: true) do - expect(ActiveRecord::Base.connection.transaction_open?).to be true + ApplicationRecord.transaction do + ApplicationRecord.transaction(requires_new: true) do + expect(ApplicationRecord.connection.transaction_open?).to be true end end end - # rubocop:enable Database/MultipleDatabases end describe '.with_fast_read_statement_timeout' do diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 67314084c4f..0ece212d692 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -77,9 +77,24 @@ RSpec.describe ApplicationSetting do it { is_expected.to validate_numericality_of(:container_registry_cleanup_tags_service_max_list_size).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_expiration_policies_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to validate_numericality_of(:container_registry_import_max_tags_count).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to validate_numericality_of(:container_registry_import_max_retries).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to validate_numericality_of(:container_registry_import_start_max_retries).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to validate_numericality_of(:container_registry_import_max_step_duration).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.not_to allow_value(nil).for(:container_registry_import_max_tags_count) } + it { is_expected.not_to allow_value(nil).for(:container_registry_import_max_retries) } + it { is_expected.not_to allow_value(nil).for(:container_registry_import_start_max_retries) } + it { is_expected.not_to allow_value(nil).for(:container_registry_import_max_step_duration) } + + it { is_expected.to validate_presence_of(:container_registry_import_target_plan) } + it { is_expected.to validate_presence_of(:container_registry_import_created_before) } + it { is_expected.to validate_numericality_of(:dependency_proxy_ttl_group_policy_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.not_to allow_value(nil).for(:dependency_proxy_ttl_group_policy_worker_capacity) } + it { is_expected.to validate_numericality_of(:packages_cleanup_package_file_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.not_to allow_value(nil).for(:packages_cleanup_package_file_worker_capacity) } + it { is_expected.to validate_numericality_of(:snippet_size_limit).only_integer.is_greater_than(0) } it { is_expected.to validate_numericality_of(:wiki_page_max_content_bytes).only_integer.is_greater_than_or_equal_to(1024) } it { is_expected.to validate_presence_of(:max_artifacts_size) } @@ -126,11 +141,13 @@ RSpec.describe ApplicationSetting do it { is_expected.not_to allow_value('default' => 101).for(:repository_storages_weighted).with_message("value for 'default' must be between 0 and 100") } it { is_expected.not_to allow_value('default' => 100, shouldntexist: 50).for(:repository_storages_weighted).with_message("can't include: shouldntexist") } - 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) } + %i[notes_create_limit user_email_lookup_limit].each do |setting| + it { is_expected.to allow_value(400).for(setting) } + it { is_expected.not_to allow_value('two').for(setting) } + it { is_expected.not_to allow_value(nil).for(setting) } + it { is_expected.not_to allow_value(5.5).for(setting) } + it { is_expected.not_to allow_value(-2).for(setting) } + end def many_usernames(num = 100) Array.new(num) { |i| "username#{i}" } @@ -489,7 +506,7 @@ RSpec.describe ApplicationSetting do context 'key restrictions' do it 'supports all key types' do - expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519) + expect(described_class::SUPPORTED_KEY_TYPES).to eq(Gitlab::SSHPublicKey.supported_types) end it 'does not allow all key types to be disabled' do @@ -1242,7 +1259,7 @@ RSpec.describe ApplicationSetting do end end - describe '#static_objects_external_storage_auth_token=' do + describe '#static_objects_external_storage_auth_token=', :aggregate_failures do subject { setting.static_objects_external_storage_auth_token = token } let(:token) { 'Test' } @@ -1266,5 +1283,20 @@ RSpec.describe ApplicationSetting do expect(setting.static_objects_external_storage_auth_token).to be_nil end end + + context 'with plaintext token only' do + let(:token) { '' } + + it 'ignores the plaintext token' do + subject + + ApplicationSetting.update_all(static_objects_external_storage_auth_token: 'Test') + + setting.reload + expect(setting[:static_objects_external_storage_auth_token]).to be_nil + expect(setting[:static_objects_external_storage_auth_token_encrypted]).to be_nil + expect(setting.static_objects_external_storage_auth_token).to be_nil + end + end end end diff --git a/spec/models/bulk_imports/file_transfer/project_config_spec.rb b/spec/models/bulk_imports/file_transfer/project_config_spec.rb index 02151da583e..61caff647d6 100644 --- a/spec/models/bulk_imports/file_transfer/project_config_spec.rb +++ b/spec/models/bulk_imports/file_transfer/project_config_spec.rb @@ -91,4 +91,10 @@ RSpec.describe BulkImports::FileTransfer::ProjectConfig do end end end + + describe '#file_relations' do + it 'returns project file relations' do + expect(subject.file_relations).to contain_exactly('uploads', 'lfs_objects') + end + end end diff --git a/spec/models/ci/build_report_result_spec.rb b/spec/models/ci/build_report_result_spec.rb index e78f602feef..3f53c6c1c0e 100644 --- a/spec/models/ci/build_report_result_spec.rb +++ b/spec/models/ci/build_report_result_spec.rb @@ -5,6 +5,11 @@ require 'spec_helper' RSpec.describe Ci::BuildReportResult do let(:build_report_result) { build(:ci_build_report_result, :with_junit_success) } + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:project) } + let!(:model) { create(:ci_build_report_result, project: parent) } + end + describe 'associations' do it { is_expected.to belong_to(:build) } it { is_expected.to belong_to(:project) } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index b9a12339e61..b8c5af5a911 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -565,6 +565,26 @@ RSpec.describe Ci::Build do expect(build.reload.runtime_metadata).not_to be_present end end + + context 'when a failure reason is provided' do + context 'when a failure reason is a symbol' do + it 'correctly sets a failure reason' do + build.drop!(:script_failure) + + expect(build.failure_reason).to eq 'script_failure' + end + end + + context 'when a failure reason is an object' do + it 'correctly sets a failure reason' do + reason = ::Gitlab::Ci::Build::Status::Reason.new(build, :script_failure) + + build.drop!(reason) + + expect(build.failure_reason).to eq 'script_failure' + end + end + end end describe '#schedulable?' do @@ -2002,6 +2022,16 @@ RSpec.describe Ci::Build do it { is_expected.not_to be_retryable } end + + context 'when build is waiting for deployment approval' do + subject { build_stubbed(:ci_build, :manual, environment: 'production') } + + before do + create(:deployment, :blocked, deployable: subject) + end + + it { is_expected.not_to be_retryable } + end end end @@ -2064,6 +2094,31 @@ RSpec.describe Ci::Build do end describe 'build auto retry feature' do + context 'with deployment job' do + let(:build) do + create(:ci_build, :deploy_to_production, :with_deployment, + user: user, pipeline: pipeline, project: project) + end + + before do + project.add_developer(user) + allow(build).to receive(:auto_retry_allowed?) { true } + end + + it 'creates a deployment when a build is dropped' do + expect { build.drop!(:script_failure) }.to change { Deployment.count }.by(1) + + retried_deployment = Deployment.last + expect(build.deployment.environment).to eq(retried_deployment.environment) + expect(build.deployment.ref).to eq(retried_deployment.ref) + expect(build.deployment.sha).to eq(retried_deployment.sha) + expect(build.deployment.tag).to eq(retried_deployment.tag) + expect(build.deployment.user).to eq(retried_deployment.user) + expect(build.deployment).to be_failed + expect(retried_deployment).to be_created + end + end + describe '#retries_count' do subject { create(:ci_build, name: 'test', pipeline: pipeline) } @@ -2152,6 +2207,28 @@ RSpec.describe Ci::Build do end end + describe '#auto_retry_expected?' do + subject { create(:ci_build, :failed) } + + context 'when build is failed and auto retry is configured' do + before do + allow(subject) + .to receive(:auto_retry_allowed?) + .and_return(true) + end + + it 'expects auto-retry to happen' do + expect(subject.auto_retry_expected?).to be true + end + end + + context 'when build failed by auto retry is not configured' do + it 'does not expect auto-retry to happen' do + expect(subject.auto_retry_expected?).to be false + end + end + end + describe '#artifacts_file_for_type' do let(:build) { create(:ci_build, :artifacts) } let(:file_type) { :archive } @@ -2443,6 +2520,16 @@ RSpec.describe Ci::Build do it { is_expected.not_to be_playable } end + + context 'when build is waiting for deployment approval' do + subject { build_stubbed(:ci_build, :manual, environment: 'production') } + + before do + create(:deployment, :blocked, deployable: subject) + end + + it { is_expected.not_to be_playable } + end end describe 'project settings' do @@ -2653,6 +2740,8 @@ RSpec.describe Ci::Build do { key: 'CI_DEPENDENCY_PROXY_USER', value: 'gitlab-ci-token', public: true, masked: false }, { key: 'CI_DEPENDENCY_PROXY_PASSWORD', value: 'my-token', public: false, masked: true }, { key: 'CI_JOB_JWT', value: 'ci.job.jwt', public: false, masked: true }, + { key: 'CI_JOB_JWT_V1', value: 'ci.job.jwt', public: false, masked: true }, + { key: 'CI_JOB_JWT_V2', value: 'ci.job.jwtv2', public: false, masked: true }, { key: 'CI_JOB_NAME', value: 'test', public: true, masked: false }, { key: 'CI_JOB_STAGE', value: 'test', public: true, masked: false }, { key: 'CI_NODE_TOTAL', value: '1', public: true, masked: false }, @@ -2720,6 +2809,7 @@ RSpec.describe Ci::Build do before do allow(Gitlab::Ci::Jwt).to receive(:for_build).and_return('ci.job.jwt') + allow(Gitlab::Ci::JwtV2).to receive(:for_build).and_return('ci.job.jwtv2') build.set_token('my-token') build.yaml_variables = [] end @@ -2771,6 +2861,8 @@ RSpec.describe Ci::Build do let(:build_yaml_var) { { key: 'yaml', value: 'value', public: true, masked: false } } let(:dependency_proxy_var) { { key: 'dependency_proxy', value: 'value', public: true, masked: false } } let(:job_jwt_var) { { key: 'CI_JOB_JWT', value: 'ci.job.jwt', public: false, masked: true } } + let(:job_jwt_var_v1) { { key: 'CI_JOB_JWT_V1', value: 'ci.job.jwt', public: false, masked: true } } + let(:job_jwt_var_v2) { { key: 'CI_JOB_JWT_V2', value: 'ci.job.jwtv2', public: false, masked: true } } let(:job_dependency_var) { { key: 'job_dependency', value: 'value', public: true, masked: false } } before do @@ -2784,7 +2876,7 @@ RSpec.describe Ci::Build do allow(build).to receive(:dependency_variables) { [job_dependency_var] } allow(build).to receive(:dependency_proxy_variables) { [dependency_proxy_var] } - allow(build.project) + allow(build.pipeline.project) .to receive(:predefined_variables) { [project_pre_var] } project.variables.create!(key: 'secret', value: 'value') @@ -3084,7 +3176,7 @@ RSpec.describe Ci::Build do context 'when the branch is protected' do before do - allow(build.project).to receive(:protected_for?).with(ref).and_return(true) + allow(build.pipeline.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -3092,7 +3184,7 @@ RSpec.describe Ci::Build do context 'when the tag is protected' do before do - allow(build.project).to receive(:protected_for?).with(ref).and_return(true) + allow(build.pipeline.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -3131,7 +3223,7 @@ RSpec.describe Ci::Build do context 'when the branch is protected' do before do - allow(build.project).to receive(:protected_for?).with(ref).and_return(true) + allow(build.pipeline.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -3139,7 +3231,7 @@ RSpec.describe Ci::Build do context 'when the tag is protected' do before do - allow(build.project).to receive(:protected_for?).with(ref).and_return(true) + allow(build.pipeline.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -3526,6 +3618,20 @@ RSpec.describe Ci::Build do build.scoped_variables end + + context 'when variables builder is used' do + it 'returns the same variables' do + build.user = create(:user) + + allow(build.pipeline).to receive(:use_variables_builder_definitions?).and_return(false) + legacy_variables = build.scoped_variables.to_hash + + allow(build.pipeline).to receive(:use_variables_builder_definitions?).and_return(true) + new_variables = build.scoped_variables.to_hash + + expect(new_variables).to eq(legacy_variables) + end + end end describe '#simple_variables_without_dependencies' do @@ -3538,7 +3644,8 @@ RSpec.describe Ci::Build do shared_examples "secret CI variables" do context 'when ref is branch' do - let(:build) { create(:ci_build, ref: 'master', tag: false, project: project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, ref: 'master', tag: false, pipeline: pipeline, project: project) } context 'when ref is protected' do before do @@ -3554,7 +3661,8 @@ RSpec.describe Ci::Build do end context 'when ref is tag' do - let(:build) { create(:ci_build, ref: 'v1.1.0', tag: true, project: project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, ref: 'v1.1.0', tag: true, pipeline: pipeline, project: project) } context 'when ref is protected' do before do @@ -3652,8 +3760,6 @@ RSpec.describe Ci::Build do .and_return(project_variables) end - it { is_expected.to eq(project_variables) } - context 'environment is nil' do let(:environment) { nil } @@ -3661,6 +3767,35 @@ RSpec.describe Ci::Build do end end + describe '#user_variables' do + subject { build.user_variables.to_hash } + + context 'with user' do + let(:expected_variables) do + { + 'GITLAB_USER_EMAIL' => user.email, + 'GITLAB_USER_ID' => user.id.to_s, + 'GITLAB_USER_LOGIN' => user.username, + 'GITLAB_USER_NAME' => user.name + } + end + + before do + build.user = user + end + + it { is_expected.to eq(expected_variables) } + end + + context 'without user' do + before do + expect(build).to receive(:user).and_return(nil) + end + + it { is_expected.to be_empty } + end + end + describe '#any_unmet_prerequisites?' do let(:build) { create(:ci_build, :created) } @@ -3762,6 +3897,18 @@ RSpec.describe Ci::Build do end end + describe 'when the build is waiting for deployment approval' do + let(:build) { create(:ci_build, :manual, environment: 'production') } + + before do + create(:deployment, :blocked, deployable: build) + end + + it 'does not allow the build to be enqueued' do + expect { build.enqueue! }.to raise_error(StateMachines::InvalidTransition) + end + end + describe 'state transition: any => [:pending]' do let(:build) { create(:ci_build, :created) } @@ -5174,25 +5321,32 @@ RSpec.describe Ci::Build do .to change { build.reload.failed? } end - it 'is executed inside a transaction' do - expect(build).to receive(:drop!) - .with(:unknown_failure) - .and_raise(ActiveRecord::Rollback) - - expect(build).to receive(:conditionally_allow_failure!) - .with(1) - .and_call_original - - expect { drop_with_exit_code } - .not_to change { build.reload.allow_failure } - end - context 'when exit_code is nil' do let(:exit_code) {} it_behaves_like 'drops the build without changing allow_failure' end end + + context 'when build is configured to be retried' do + let(:options) { { retry: 3 } } + + context 'when there is an MR attached to the pipeline and a failed job todo for that MR' do + let!(:merge_request) { create(:merge_request, source_project: project, author: user, head_pipeline: pipeline) } + let!(:todo) { create(:todo, :build_failed, user: user, project: project, author: user, target: merge_request) } + + before do + build.update!(user: user) + project.add_developer(user) + end + + it 'resolves the todo for the old failed build' do + expect do + drop_with_exit_code + end.to change { todo.reload.state }.from('pending').to('done') + end + end + end end describe '#exit_codes_defined?' do @@ -5377,7 +5531,8 @@ RSpec.describe Ci::Build do describe '#doom!' do subject { build.doom! } - let_it_be(:build) { create(:ci_build, :queued) } + let(:traits) { [] } + let(:build) { create(:ci_build, *traits, pipeline: pipeline) } it 'updates status and failure_reason', :aggregate_failures do subject @@ -5386,10 +5541,33 @@ RSpec.describe Ci::Build do expect(build.failure_reason).to eq("data_integrity_failure") end - it 'drops associated pending build' do + it 'logs a message' do + expect(Gitlab::AppLogger) + .to receive(:info) + .with(a_hash_including(message: 'Build doomed', class: build.class.name, build_id: build.id)) + .and_call_original + subject + end + + context 'with queued builds' do + let(:traits) { [:queued] } + + it 'drops associated pending build' do + subject - expect(build.reload.queuing_entry).not_to be_present + expect(build.reload.queuing_entry).not_to be_present + end + end + + context 'with running builds' do + let(:traits) { [:picked] } + + it 'drops associated runtime metadata' do + subject + + expect(build.reload.runtime_metadata).not_to be_present + end end end diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index b6e128c317c..31c7c7a44bc 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -49,9 +49,8 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git end context 'FastDestroyAll' do - let(:parent) { create(:project) } - let(:pipeline) { create(:ci_pipeline, project: parent) } - let!(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline, project: parent) } + let(:pipeline) { create(:ci_pipeline) } + let!(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline) } let(:subjects) { build.trace_chunks } describe 'Forbid #destroy and #destroy_all' do @@ -84,7 +83,7 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git expect(external_data_counter).to be > 0 expect(subjects.count).to be > 0 - expect { parent.destroy! }.not_to raise_error + expect { pipeline.destroy! }.not_to raise_error expect(subjects.count).to eq(0) expect(external_data_counter).to eq(0) @@ -830,7 +829,7 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git expect(described_class.count).to eq(3) - subject + expect(subject).to be_truthy expect(described_class.count).to eq(0) @@ -852,7 +851,7 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git context 'when project is destroyed' do let(:subject) do - project.destroy! + Projects::DestroyService.new(project, project.owner).execute end it_behaves_like 'deletes all build_trace_chunk and data in redis' 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 acc87c61036..43ba4c32477 100644 --- a/spec/models/ci/daily_build_group_report_result_spec.rb +++ b/spec/models/ci/daily_build_group_report_result_spec.rb @@ -164,4 +164,16 @@ RSpec.describe Ci::DailyBuildGroupReportResult do end end end + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:model) { create(:ci_daily_build_group_report_result) } + + let!(:parent) { model.group } + end + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:model) { create(:ci_daily_build_group_report_result) } + + let!(:parent) { model.project } + end end diff --git a/spec/models/ci/freeze_period_spec.rb b/spec/models/ci/freeze_period_spec.rb index f7f840c6696..b9bf1657e28 100644 --- a/spec/models/ci/freeze_period_spec.rb +++ b/spec/models/ci/freeze_period_spec.rb @@ -5,6 +5,11 @@ require 'spec_helper' RSpec.describe Ci::FreezePeriod, type: :model do subject { build(:ci_freeze_period) } + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:project) } + let!(:model) { create(:ci_freeze_period, project: parent) } + end + let(:invalid_cron) { '0 0 0 * *' } it { is_expected.to belong_to(:project) } diff --git a/spec/models/ci/group_variable_spec.rb b/spec/models/ci/group_variable_spec.rb index f0eec549da7..4cb3b9eef0c 100644 --- a/spec/models/ci/group_variable_spec.rb +++ b/spec/models/ci/group_variable_spec.rb @@ -42,4 +42,10 @@ RSpec.describe Ci::GroupVariable do end end end + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:model) { create(:ci_group_variable) } + + let!(:parent) { model.group } + end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 38061e0975f..2e8c41b410a 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -143,6 +143,17 @@ RSpec.describe Ci::JobArtifact do end end + describe '.erasable_file_types' do + subject { described_class.erasable_file_types } + + it 'returns a list of erasable file types' do + all_types = described_class.file_types.keys + erasable_types = all_types - described_class::NON_ERASABLE_FILE_TYPES + + expect(subject).to contain_exactly(*erasable_types) + end + end + describe '.erasable' do subject { described_class.erasable } @@ -534,20 +545,8 @@ RSpec.describe Ci::JobArtifact do context 'when the artifact is a trace' do let(:file_type) { :trace } - context 'when ci_store_trace_outside_transaction is enabled' do - it 'returns true' do - expect(artifact.store_after_commit?).to be_truthy - end - end - - context 'when ci_store_trace_outside_transaction is disabled' do - before do - stub_feature_flags(ci_store_trace_outside_transaction: false) - end - - it 'returns false' do - expect(artifact.store_after_commit?).to be_falsey - end + it 'returns true' do + expect(artifact.store_after_commit?).to be_truthy end end diff --git a/spec/models/ci/job_token/project_scope_link_spec.rb b/spec/models/ci/job_token/project_scope_link_spec.rb index dd6a75dfd89..8d7bb44bd16 100644 --- a/spec/models/ci/job_token/project_scope_link_spec.rb +++ b/spec/models/ci/job_token/project_scope_link_spec.rb @@ -9,6 +9,11 @@ RSpec.describe Ci::JobToken::ProjectScopeLink do let_it_be(:project) { create(:project) } + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:user) } + let!(:model) { create(:ci_job_token_project_scope_link, added_by: parent) } + end + describe 'unique index' do let!(:link) { create(:ci_job_token_project_scope_link) } diff --git a/spec/models/ci/namespace_mirror_spec.rb b/spec/models/ci/namespace_mirror_spec.rb index b4c71f51377..a9d916115fc 100644 --- a/spec/models/ci/namespace_mirror_spec.rb +++ b/spec/models/ci/namespace_mirror_spec.rb @@ -8,50 +8,91 @@ RSpec.describe Ci::NamespaceMirror do let!(:group3) { create(:group, parent: group2) } let!(:group4) { create(:group, parent: group3) } - describe '.sync!' do - let!(:event) { namespace.sync_events.create! } + before do + # refreshing ci mirrors according to the parent tree above + Namespaces::SyncEvent.find_each { |event| Ci::NamespaceMirror.sync!(event) } + + # checking initial situation. we need to reload to reflect the changes of event sync + expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id]) + expect(group2.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id]) + expect(group3.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id]) + expect(group4.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id, group4.id]) + end + + context 'scopes' do + describe '.contains_namespace' do + let_it_be(:another_group) { create(:group) } + + subject(:result) { described_class.contains_namespace(group2.id) } + + it 'returns groups having group2.id in traversal_ids' do + expect(result.pluck(:namespace_id)).to contain_exactly(group2.id, group3.id, group4.id) + end + end + + describe '.contains_any_of_namespaces' do + let!(:other_group1) { create(:group) } + let!(:other_group2) { create(:group, parent: other_group1) } + let!(:other_group3) { create(:group, parent: other_group2) } + + subject(:result) { described_class.contains_any_of_namespaces([group2.id, other_group2.id]) } + + it 'returns groups having group2.id in traversal_ids' do + expect(result.pluck(:namespace_id)).to contain_exactly( + group2.id, group3.id, group4.id, other_group2.id, other_group3.id + ) + end + end + + describe '.by_namespace_id' do + subject(:result) { described_class.by_namespace_id(group2.id) } + + it 'returns namesapce mirrors of namespace id' do + expect(result).to contain_exactly(group2.ci_namespace_mirror) + end + end + end - subject(:sync) { described_class.sync!(event.reload) } + describe '.sync!' do + subject(:sync) { described_class.sync!(Namespaces::SyncEvent.last) } - context 'when namespace hierarchy does not exist in the first place' do + context 'when namespace mirror does not exist in the first place' do let(:namespace) { group3 } - it 'creates the hierarchy' do - expect { sync }.to change { described_class.count }.from(0).to(1) + before do + namespace.ci_namespace_mirror.destroy! + namespace.sync_events.create! + end + + it 'creates the mirror' do + expect { sync }.to change { described_class.count }.from(3).to(4) - expect(namespace.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id]) + expect(namespace.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id]) end end - context 'when namespace hierarchy does already exist' do + context 'when namespace mirror does already exist' do let(:namespace) { group3 } before do - described_class.create!(namespace: namespace, traversal_ids: [namespace.id]) + namespace.sync_events.create! end - it 'updates the hierarchy' do + it 'updates the mirror' do expect { sync }.not_to change { described_class.count } - expect(namespace.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id]) + expect(namespace.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id]) end end - # I did not extract this context to a `shared_context` because the behavior will change - # after implementing the TODO in `Ci::NamespaceMirror.sync!` - context 'changing the middle namespace' do + shared_context 'changing the middle namespace' do let(:namespace) { group2 } before do - described_class.create!(namespace_id: group1.id, traversal_ids: [group1.id]) - described_class.create!(namespace_id: group2.id, traversal_ids: [group1.id, group2.id]) - described_class.create!(namespace_id: group3.id, traversal_ids: [group1.id, group2.id, group3.id]) - described_class.create!(namespace_id: group4.id, traversal_ids: [group1.id, group2.id, group3.id, group4.id]) - - group2.update!(parent: nil) + group2.update!(parent: nil) # creates a sync event end - it 'updates hierarchies for the base but wait for events for the children' do + it 'updates traversal_ids for the base and descendants' do expect { sync }.not_to change { described_class.count } expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id]) @@ -61,6 +102,8 @@ RSpec.describe Ci::NamespaceMirror do end end + it_behaves_like 'changing the middle namespace' + context 'when the FFs sync_traversal_ids, use_traversal_ids and use_traversal_ids_for_ancestors are disabled' do before do stub_feature_flags(sync_traversal_ids: false, @@ -68,27 +111,7 @@ RSpec.describe Ci::NamespaceMirror do use_traversal_ids_for_ancestors: false) end - context 'changing the middle namespace' do - let(:namespace) { group2 } - - before do - described_class.create!(namespace_id: group1.id, traversal_ids: [group1.id]) - described_class.create!(namespace_id: group2.id, traversal_ids: [group1.id, group2.id]) - described_class.create!(namespace_id: group3.id, traversal_ids: [group1.id, group2.id, group3.id]) - described_class.create!(namespace_id: group4.id, traversal_ids: [group1.id, group2.id, group3.id, group4.id]) - - group2.update!(parent: nil) - end - - it 'updates hierarchies for the base and descendants' do - expect { sync }.not_to change { described_class.count } - - expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id]) - expect(group2.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id]) - expect(group3.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id, group3.id]) - expect(group4.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id, group3.id, group4.id]) - end - end + it_behaves_like 'changing the middle namespace' end end end diff --git a/spec/models/ci/pending_build_spec.rb b/spec/models/ci/pending_build_spec.rb index abf0fb443bb..5692444339f 100644 --- a/spec/models/ci/pending_build_spec.rb +++ b/spec/models/ci/pending_build_spec.rb @@ -223,4 +223,14 @@ RSpec.describe Ci::PendingBuild do end end end + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:namespace) } + let!(:model) { create(:ci_pending_build, namespace: parent) } + end + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:project) } + let!(:model) { create(:ci_pending_build, project: parent) } + end end diff --git a/spec/models/ci/pipeline_artifact_spec.rb b/spec/models/ci/pipeline_artifact_spec.rb index f65483d2290..801505f0231 100644 --- a/spec/models/ci/pipeline_artifact_spec.rb +++ b/spec/models/ci/pipeline_artifact_spec.rb @@ -215,4 +215,11 @@ RSpec.describe Ci::PipelineArtifact, type: :model do end end end + + context 'loose foreign key on ci_pipeline_artifacts.project_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:project) } + let!(:model) { create(:ci_pipeline_artifact, project: parent) } + end + end end diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index fee74f8f674..0f1cb721e95 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -23,6 +23,11 @@ RSpec.describe Ci::PipelineSchedule do subject { build(:ci_pipeline_schedule, project: project) } end + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:user) } + let!(:model) { create(:ci_pipeline_schedule, owner: parent) } + end + describe 'validations' do it 'does not allow invalid cron patterns' do pipeline_schedule = build(:ci_pipeline_schedule, cron: '0 0 0 * *') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index fd9970699d7..90f56c1e0a4 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -31,6 +31,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it { is_expected.to have_many(:statuses_order_id_desc) } it { is_expected.to have_many(:bridges) } it { is_expected.to have_many(:job_artifacts).through(:builds) } + it { is_expected.to have_many(:build_trace_chunks).through(:builds) } it { is_expected.to have_many(:auto_canceled_pipelines) } it { is_expected.to have_many(:auto_canceled_jobs) } it { is_expected.to have_many(:sourced_pipelines) } @@ -1516,30 +1517,12 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'pipeline caching' do - context 'when expire_job_and_pipeline_cache_synchronously is enabled' do - before do - stub_feature_flags(expire_job_and_pipeline_cache_synchronously: true) - end - - it 'executes Ci::ExpirePipelineCacheService' do - expect_next_instance_of(Ci::ExpirePipelineCacheService) do |service| - expect(service).to receive(:execute).with(pipeline) - end - - pipeline.cancel + it 'executes Ci::ExpirePipelineCacheService' do + expect_next_instance_of(Ci::ExpirePipelineCacheService) do |service| + expect(service).to receive(:execute).with(pipeline) end - end - - context 'when expire_job_and_pipeline_cache_synchronously is disabled' do - before do - stub_feature_flags(expire_job_and_pipeline_cache_synchronously: false) - end - - it 'performs ExpirePipelinesCacheWorker' do - expect(ExpirePipelineCacheWorker).to receive(:perform_async).with(pipeline.id) - pipeline.cancel - end + pipeline.cancel end end @@ -4677,4 +4660,23 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do let!(:model) { create(:ci_pipeline, user: create(:user)) } let!(:parent) { model.user } end + + describe 'tags count' do + let_it_be_with_refind(:pipeline) do + create(:ci_empty_pipeline, project: project) + end + + it { expect(pipeline.tags_count).to eq(0) } + it { expect(pipeline.distinct_tags_count).to eq(0) } + + context 'with builds' do + before do + create(:ci_build, pipeline: pipeline, tag_list: %w[a b]) + create(:ci_build, pipeline: pipeline, tag_list: %w[b c]) + end + + it { expect(pipeline.tags_count).to eq(4) } + it { expect(pipeline.distinct_tags_count).to eq(3) } + end + end end diff --git a/spec/models/ci/project_mirror_spec.rb b/spec/models/ci/project_mirror_spec.rb index 199285b036c..5ef520b4230 100644 --- a/spec/models/ci/project_mirror_spec.rb +++ b/spec/models/ci/project_mirror_spec.rb @@ -8,12 +8,36 @@ RSpec.describe Ci::ProjectMirror do let!(:project) { create(:project, namespace: group2) } + context 'scopes' do + let_it_be(:another_project) { create(:project, namespace: group1) } + + describe '.by_project_id' do + subject(:result) { described_class.by_project_id(project.id) } + + it 'returns project mirrors of project' do + expect(result.pluck(:project_id)).to contain_exactly(project.id) + end + end + + describe '.by_namespace_id' do + subject(:result) { described_class.by_namespace_id(group2.id) } + + it 'returns project mirrors of namespace id' do + expect(result).to contain_exactly(project.ci_project_mirror) + end + end + end + describe '.sync!' do let!(:event) { Projects::SyncEvent.create!(project: project) } - subject(:sync) { described_class.sync!(event.reload) } + subject(:sync) { described_class.sync!(event) } + + context 'when project mirror does not exist in the first place' do + before do + project.ci_project_mirror.destroy! + end - context 'when project hierarchy does not exist in the first place' do it 'creates a ci_projects record' do expect { sync }.to change { described_class.count }.from(0).to(1) @@ -21,11 +45,7 @@ RSpec.describe Ci::ProjectMirror do end end - context 'when project hierarchy does already exist' do - before do - described_class.create!(project_id: project.id, namespace_id: group1.id) - end - + context 'when project mirror does already exist' do it 'updates the related ci_projects record' do expect { sync }.not_to change { described_class.count } diff --git a/spec/models/ci/resource_group_spec.rb b/spec/models/ci/resource_group_spec.rb index aae16157fbf..76e74f3193c 100644 --- a/spec/models/ci/resource_group_spec.rb +++ b/spec/models/ci/resource_group_spec.rb @@ -3,6 +3,11 @@ require 'spec_helper' RSpec.describe Ci::ResourceGroup do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:project) } + let!(:model) { create(:ci_resource_group, project: parent) } + end + describe 'validation' do it 'valids when key includes allowed character' do resource_group = build(:ci_resource_group, key: 'test') diff --git a/spec/models/ci/runner_namespace_spec.rb b/spec/models/ci/runner_namespace_spec.rb index 41d805adb9f..2d1fe11147c 100644 --- a/spec/models/ci/runner_namespace_spec.rb +++ b/spec/models/ci/runner_namespace_spec.rb @@ -6,4 +6,10 @@ RSpec.describe Ci::RunnerNamespace do it_behaves_like 'includes Limitable concern' do subject { build(:ci_runner_namespace, group: create(:group, :nested), runner: create(:ci_runner, :group)) } end + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:model) { create(:ci_runner_namespace) } + + let!(:parent) { model.namespace } + end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 5142f70fa2c..6830a8daa3b 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -48,7 +48,7 @@ RSpec.describe Ci::Runner do let(:runner) { create(:ci_runner, :group, groups: [group]) } it 'disallows assigning group if already assigned to a group' do - runner.runner_namespaces << build(:ci_runner_namespace) + runner.runner_namespaces << create(:ci_runner_namespace) expect(runner).not_to be_valid expect(runner.errors.full_messages).to include('Runner needs to be assigned to exactly one group') @@ -203,28 +203,56 @@ RSpec.describe Ci::Runner do end end - describe '.belonging_to_parent_group_of_project' do - let(:project) { create(:project, group: group) } - let(:group) { create(:group) } - let(:runner) { create(:ci_runner, :group, groups: [group]) } - let!(:unrelated_group) { create(:group) } - let!(:unrelated_project) { create(:project, group: unrelated_group) } - let!(:unrelated_runner) { create(:ci_runner, :group, groups: [unrelated_group]) } + shared_examples '.belonging_to_parent_group_of_project' do + let!(:group1) { create(:group) } + let!(:project1) { create(:project, group: group1) } + let!(:runner1) { create(:ci_runner, :group, groups: [group1]) } + + let!(:group2) { create(:group) } + let!(:project2) { create(:project, group: group2) } + let!(:runner2) { create(:ci_runner, :group, groups: [group2]) } + + let(:project_id) { project1.id } + + subject(:result) { described_class.belonging_to_parent_group_of_project(project_id) } it 'returns the specific group runner' do - expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner) + expect(result).to contain_exactly(runner1) end - context 'with a parent group with a runner' do - let(:runner) { create(:ci_runner, :group, groups: [parent_group]) } - let(:project) { create(:project, group: group) } - let(:group) { create(:group, parent: parent_group) } - let(:parent_group) { create(:group) } + context 'with a parent group with a runner', :sidekiq_inline do + before do + group1.update!(parent: group2) + end - it 'returns the group runner from the parent group' do - expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner) + it 'returns the group runner from the group and the parent group' do + expect(result).to contain_exactly(runner1, runner2) end end + + context 'with multiple project ids' do + let(:project_id) { [project1.id, project2.id] } + + it 'raises ArgumentError' do + expect { result }.to raise_error(ArgumentError) + end + end + end + + context 'when use_traversal_ids* are enabled' do + it_behaves_like '.belonging_to_parent_group_of_project' + end + + context 'when use_traversal_ids* are disabled' do + before do + stub_feature_flags( + use_traversal_ids: false, + use_traversal_ids_for_ancestors: false, + use_traversal_ids_for_ancestor_scopes: false + ) + end + + it_behaves_like '.belonging_to_parent_group_of_project' end describe '.owned_or_instance_wide' do @@ -1358,7 +1386,7 @@ RSpec.describe Ci::Runner do it { is_expected.to eq(contacted_at_stored) } end - describe '.belonging_to_group' do + describe '.legacy_belonging_to_group' do shared_examples 'returns group runners' do it 'returns the specific group runner' do group = create(:group) @@ -1366,7 +1394,7 @@ RSpec.describe Ci::Runner do unrelated_group = create(:group) create(:ci_runner, :group, groups: [unrelated_group]) - expect(described_class.belonging_to_group(group.id)).to contain_exactly(runner) + expect(described_class.legacy_belonging_to_group(group.id)).to contain_exactly(runner) end context 'runner belonging to parent group' do @@ -1376,13 +1404,13 @@ RSpec.describe Ci::Runner do context 'when include_parent option is passed' do it 'returns the group runner from the parent group' do - expect(described_class.belonging_to_group(group.id, include_ancestors: true)).to contain_exactly(parent_runner) + expect(described_class.legacy_belonging_to_group(group.id, include_ancestors: true)).to contain_exactly(parent_runner) end end context 'when include_parent option is not passed' do it 'does not return the group runner from the parent group' do - expect(described_class.belonging_to_group(group.id)).to be_empty + expect(described_class.legacy_belonging_to_group(group.id)).to be_empty end end end @@ -1398,4 +1426,48 @@ RSpec.describe Ci::Runner do it_behaves_like 'returns group runners' end end + + describe '.belonging_to_group' do + it 'returns the specific group runner' do + group = create(:group) + runner = create(:ci_runner, :group, groups: [group]) + unrelated_group = create(:group) + create(:ci_runner, :group, groups: [unrelated_group]) + + expect(described_class.belonging_to_group(group.id)).to contain_exactly(runner) + end + end + + describe '.belonging_to_group_and_ancestors' do + let_it_be(:parent_group) { create(:group) } + let_it_be(:parent_runner) { create(:ci_runner, :group, groups: [parent_group]) } + let_it_be(:group) { create(:group, parent: parent_group) } + + it 'returns the group runner from the parent group' do + expect(described_class.belonging_to_group_and_ancestors(group.id)).to contain_exactly(parent_runner) + end + end + + describe '.belonging_to_group_or_project_descendants' do + it 'returns the specific group runners' do + group1 = create(:group) + group2 = create(:group, parent: group1) + group3 = create(:group) + + project1 = create(:project, namespace: group1) + project2 = create(:project, namespace: group2) + project3 = create(:project, namespace: group3) + + runner1 = create(:ci_runner, :group, groups: [group1]) + runner2 = create(:ci_runner, :group, groups: [group2]) + _runner3 = create(:ci_runner, :group, groups: [group3]) + runner4 = create(:ci_runner, :project, projects: [project1]) + runner5 = create(:ci_runner, :project, projects: [project2]) + _runner6 = create(:ci_runner, :project, projects: [project3]) + + expect(described_class.belonging_to_group_or_project_descendants(group1.id)).to contain_exactly( + runner1, runner2, runner4, runner5 + ) + end + end end diff --git a/spec/models/ci/running_build_spec.rb b/spec/models/ci/running_build_spec.rb index 629861e35b8..d2f74494308 100644 --- a/spec/models/ci/running_build_spec.rb +++ b/spec/models/ci/running_build_spec.rb @@ -49,4 +49,9 @@ RSpec.describe Ci::RunningBuild do end end end + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:project) } + let!(:model) { create(:ci_running_build, project: parent) } + end end diff --git a/spec/models/ci/secure_file_spec.rb b/spec/models/ci/secure_file_spec.rb new file mode 100644 index 00000000000..ae57b63e7a4 --- /dev/null +++ b/spec/models/ci/secure_file_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::SecureFile do + let(:sample_file) { fixture_file('ci_secure_files/upload-keystore.jks') } + + subject { create(:ci_secure_file) } + + before do + stub_ci_secure_file_object_storage + end + + it { is_expected.to be_a FileStoreMounter } + + it { is_expected.to belong_to(:project).required } + + it_behaves_like 'having unique enum values' + + describe 'validations' do + it { is_expected.to validate_presence_of(:checksum) } + it { is_expected.to validate_presence_of(:file_store) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_presence_of(:permissions) } + it { is_expected.to validate_presence_of(:project_id) } + end + + describe '#permissions' do + it 'defaults to read_only file permssions' do + expect(subject.permissions).to eq('read_only') + end + end + + describe '#checksum' do + it 'computes SHA256 checksum on the file before encrypted' do + subject.file = CarrierWaveStringFile.new(sample_file) + subject.save! + expect(subject.checksum).to eq(Digest::SHA256.hexdigest(sample_file)) + end + end + + describe '#checksum_algorithm' do + it 'returns the configured checksum_algorithm' do + expect(subject.checksum_algorithm).to eq('sha256') + end + end + + describe '#file' do + it 'returns the saved file' do + subject.file = CarrierWaveStringFile.new(sample_file) + subject.save! + expect(Base64.encode64(subject.file.read)).to eq(Base64.encode64(sample_file)) + end + end +end diff --git a/spec/models/ci/unit_test_spec.rb b/spec/models/ci/unit_test_spec.rb index 2207a362be3..556cf93c266 100644 --- a/spec/models/ci/unit_test_spec.rb +++ b/spec/models/ci/unit_test_spec.rb @@ -3,6 +3,11 @@ require 'spec_helper' RSpec.describe Ci::UnitTest do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:project) } + let!(:model) { create(:ci_unit_test, project: parent) } + end + describe 'relationships' do it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:unit_test_failures) } diff --git a/spec/models/clusters/agent_spec.rb b/spec/models/clusters/agent_spec.rb index 3b521086c14..f279e779de5 100644 --- a/spec/models/clusters/agent_spec.rb +++ b/spec/models/clusters/agent_spec.rb @@ -76,12 +76,12 @@ RSpec.describe Clusters::Agent do end end - describe '#active?' do + describe '#connected?' do let_it_be(:agent) { create(:cluster_agent) } let!(:token) { create(:cluster_agent_token, agent: agent, last_used_at: last_used_at) } - subject { agent.active? } + subject { agent.connected? } context 'agent has never connected' do let(:last_used_at) { nil } @@ -99,6 +99,14 @@ RSpec.describe Clusters::Agent do let(:last_used_at) { 2.minutes.ago } it { is_expected.to be_truthy } + + context 'agent token has been revoked' do + before do + token.revoked! + end + + it { is_expected.to be_falsey } + end end context 'agent has multiple tokens' do @@ -108,4 +116,19 @@ RSpec.describe Clusters::Agent do it { is_expected.to be_truthy } end end + + describe '#activity_event_deletion_cutoff' do + let_it_be(:agent) { create(:cluster_agent) } + let_it_be(:event1) { create(:agent_activity_event, agent: agent, recorded_at: 1.hour.ago) } + let_it_be(:event2) { create(:agent_activity_event, agent: agent, recorded_at: 2.hours.ago) } + let_it_be(:event3) { create(:agent_activity_event, agent: agent, recorded_at: 3.hours.ago) } + + subject { agent.activity_event_deletion_cutoff } + + before do + stub_const("#{described_class}::ACTIVITY_EVENT_LIMIT", 2) + end + + it { is_expected.to be_like_time(event2.recorded_at) } + end end diff --git a/spec/models/clusters/agent_token_spec.rb b/spec/models/clusters/agent_token_spec.rb index ad9f948224f..efa2a3eb09b 100644 --- a/spec/models/clusters/agent_token_spec.rb +++ b/spec/models/clusters/agent_token_spec.rb @@ -9,17 +9,29 @@ RSpec.describe Clusters::AgentToken do it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_presence_of(:name) } + it_behaves_like 'having unique enum values' + describe 'scopes' do describe '.order_last_used_at_desc' do - let_it_be(:token_1) { create(:cluster_agent_token, last_used_at: 7.days.ago) } - let_it_be(:token_2) { create(:cluster_agent_token, last_used_at: nil) } - let_it_be(:token_3) { create(:cluster_agent_token, last_used_at: 2.days.ago) } + let_it_be(:agent) { create(:cluster_agent) } + let_it_be(:token_1) { create(:cluster_agent_token, agent: agent, last_used_at: 7.days.ago) } + let_it_be(:token_2) { create(:cluster_agent_token, agent: agent, last_used_at: nil) } + let_it_be(:token_3) { create(:cluster_agent_token, agent: agent, last_used_at: 2.days.ago) } it 'sorts by last_used_at descending, with null values at last' do expect(described_class.order_last_used_at_desc) .to eq([token_3, token_1, token_2]) end end + + describe '.with_status' do + let!(:active_token) { create(:cluster_agent_token) } + let!(:revoked_token) { create(:cluster_agent_token, :revoked) } + + subject { described_class.with_status(:active) } + + it { is_expected.to contain_exactly(active_token) } + end end describe '#token' do @@ -37,83 +49,4 @@ RSpec.describe Clusters::AgentToken do expect(agent_token.token.length).to be >= 50 end end - - describe '#track_usage', :clean_gitlab_redis_cache do - let_it_be(:agent) { create(:cluster_agent) } - - let(:agent_token) { create(:cluster_agent_token, agent: agent) } - - subject { agent_token.track_usage } - - context 'when last_used_at was updated recently' do - before do - agent_token.update!(last_used_at: 10.minutes.ago) - end - - it 'updates cache but not database' do - expect { subject }.not_to change { agent_token.reload.read_attribute(:last_used_at) } - - expect_redis_update - end - end - - context 'when last_used_at was not updated recently' do - it 'updates cache and database' do - does_db_update - expect_redis_update - end - - context 'with invalid token' do - before do - agent_token.description = SecureRandom.hex(2000) - end - - it 'still updates caches and database' do - expect(agent_token).to be_invalid - - does_db_update - expect_redis_update - end - end - - context 'agent is inactive' do - before do - allow(agent).to receive(:active?).and_return(false) - end - - it 'creates an activity event' do - expect { subject }.to change { agent.activity_events.count } - - event = agent.activity_events.last - expect(event).to have_attributes( - kind: 'agent_connected', - level: 'info', - recorded_at: agent_token.reload.read_attribute(:last_used_at), - agent_token: agent_token - ) - end - end - - context 'agent is active' do - before do - allow(agent).to receive(:active?).and_return(true) - end - - it 'does not create an activity event' do - expect { subject }.not_to change { agent.activity_events.count } - end - end - end - - def expect_redis_update - Gitlab::Redis::Cache.with do |redis| - redis_key = "cache:#{described_class.name}:#{agent_token.id}:attributes" - expect(redis.get(redis_key)).to be_present - end - end - - def does_db_update - expect { subject }.to change { agent_token.reload.read_attribute(:last_used_at) } - end - end end diff --git a/spec/models/clusters/agents/activity_event_spec.rb b/spec/models/clusters/agents/activity_event_spec.rb index 18b9c82fa6a..2e3833898fd 100644 --- a/spec/models/clusters/agents/activity_event_spec.rb +++ b/spec/models/clusters/agents/activity_event_spec.rb @@ -16,11 +16,10 @@ RSpec.describe Clusters::Agents::ActivityEvent do let_it_be(:agent) { create(:cluster_agent) } describe '.in_timeline_order' do - let(:recorded_at) { 1.hour.ago } - - let!(:event1) { create(:agent_activity_event, agent: agent, recorded_at: recorded_at) } - let!(:event2) { create(:agent_activity_event, agent: agent, recorded_at: Time.current) } - let!(:event3) { create(:agent_activity_event, agent: agent, recorded_at: recorded_at) } + let_it_be(:recorded_at) { 1.hour.ago } + let_it_be(:event1) { create(:agent_activity_event, agent: agent, recorded_at: recorded_at) } + let_it_be(:event2) { create(:agent_activity_event, agent: agent, recorded_at: Time.current) } + let_it_be(:event3) { create(:agent_activity_event, agent: agent, recorded_at: recorded_at) } subject { described_class.in_timeline_order } @@ -28,5 +27,19 @@ RSpec.describe Clusters::Agents::ActivityEvent do is_expected.to eq([event2, event3, event1]) end end + + describe '.recorded_before' do + let_it_be(:event1) { create(:agent_activity_event, agent: agent, recorded_at: 1.hour.ago) } + let_it_be(:event2) { create(:agent_activity_event, agent: agent, recorded_at: 2.hours.ago) } + let_it_be(:event3) { create(:agent_activity_event, agent: agent, recorded_at: 3.hours.ago) } + + let(:cutoff) { event2.recorded_at } + + subject { described_class.recorded_before(cutoff) } + + it 'returns only events recorded before the cutoff' do + is_expected.to contain_exactly(event3) + end + end end end diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 434d7ad4a90..8f02161843b 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -101,19 +101,6 @@ RSpec.describe Clusters::Applications::Runner do end end - describe '#prepare_uninstall' do - it 'pauses associated runner' do - active_runner = create(:ci_runner, contacted_at: 1.second.ago) - - expect(active_runner.active).to be_truthy - - application_runner = create(:clusters_applications_runner, :scheduled, runner: active_runner) - application_runner.prepare_uninstall - - expect(active_runner.active).to be_falsey - end - end - describe '#make_uninstalling!' do subject { create(:clusters_applications_runner, :scheduled, runner: ci_runner) } diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 665a2a936af..d5e74d36b58 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -46,28 +46,10 @@ RSpec.describe CommitStatus do describe 'status state machine' do let!(:commit_status) { create(:commit_status, :running, project: project) } - context 'when expire_job_and_pipeline_cache_synchronously is enabled' do - before do - stub_feature_flags(expire_job_and_pipeline_cache_synchronously: true) - end - - it 'invalidates the cache after a transition' do - expect(commit_status).to receive(:expire_etag_cache!) + it 'invalidates the cache after a transition' do + expect(commit_status).to receive(:expire_etag_cache!) - commit_status.success! - end - end - - context 'when expire_job_and_pipeline_cache_synchronously is disabled' do - before do - stub_feature_flags(expire_job_and_pipeline_cache_synchronously: false) - end - - it 'invalidates the cache after a transition' do - expect(ExpireJobCacheWorker).to receive(:perform_async).with(commit_status.id) - - commit_status.success! - end + commit_status.success! end describe 'transitioning to running' do @@ -773,6 +755,26 @@ RSpec.describe CommitStatus do expect { commit_status.drop! }.to change { commit_status.status }.from('manual').to('failed') end end + + context 'when a failure reason is provided' do + context 'when a failure reason is a symbol' do + it 'correctly sets a failure reason' do + commit_status.drop!(:script_failure) + + expect(commit_status).to be_script_failure + end + end + + context 'when a failure reason is an object' do + it 'correctly sets a failure reason' do + reason = ::Gitlab::Ci::Build::Status::Reason.new(commit_status, :script_failure) + + commit_status.drop!(reason) + + expect(commit_status).to be_script_failure + end + end + end end describe 'ensure stage assignment' do @@ -961,18 +963,17 @@ RSpec.describe CommitStatus do describe '.bulk_insert_tags!' do let(:statuses) { double('statuses') } - let(:tag_list_by_build) { double('tag list') } let(:inserter) { double('inserter') } it 'delegates to bulk insert class' do expect(Gitlab::Ci::Tags::BulkInsert) .to receive(:new) - .with(statuses, tag_list_by_build) + .with(statuses) .and_return(inserter) expect(inserter).to receive(:insert!) - described_class.bulk_insert_tags!(statuses, tag_list_by_build) + described_class.bulk_insert_tags!(statuses) end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 2a3f639a8ac..e9c3d1dc646 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -922,6 +922,22 @@ RSpec.describe Issuable do end end + describe '#supports_escalation?' do + where(:issuable_type, :supports_escalation) do + :issue | false + :incident | true + :merge_request | false + end + + with_them do + let(:issuable) { build_stubbed(issuable_type) } + + subject { issuable.supports_escalation? } + + it { is_expected.to eq(supports_escalation) } + end + end + describe '#incident?' do where(:issuable_type, :incident) do :issue | false diff --git a/spec/models/concerns/participable_spec.rb b/spec/models/concerns/participable_spec.rb index 50cf7377b99..99a3a0fb79a 100644 --- a/spec/models/concerns/participable_spec.rb +++ b/spec/models/concerns/participable_spec.rb @@ -138,7 +138,7 @@ RSpec.describe Participable do allow(instance).to receive_message_chain(:model_name, :element) { 'class' } expect(instance).to receive(:foo).and_return(user2) expect(instance).to receive(:bar).and_return(user3) - expect(instance).to receive(:project).thrice.and_return(project) + expect(instance).to receive(:project).twice.and_return(project) participants = instance.visible_participants(user1) @@ -159,31 +159,10 @@ RSpec.describe Participable do allow(instance).to receive_message_chain(:model_name, :element) { 'class' } allow(instance).to receive(:bar).and_return(user2) - expect(instance).to receive(:project).thrice.and_return(project) + expect(instance).to receive(:project).twice.and_return(project) expect(instance.visible_participants(user1)).to be_empty end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(verify_participants_access: false) - end - - it 'returns unavailable participants' do - model.participant(:bar) - - instance = model.new - user1 = build(:user) - user2 = build(:user) - project = build(:project, :public) - - allow(instance).to receive_message_chain(:model_name, :element) { 'class' } - allow(instance).to receive(:bar).and_return(user2) - expect(instance).to receive(:project).thrice.and_return(project) - - expect(instance.visible_participants(user1)).to match_array([user2]) - end - end end end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 2330147b376..cf66ba83e87 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -141,6 +141,11 @@ RSpec.describe Group, 'Routable', :with_clean_rails_cache do end end + it 'creates route with namespace referencing group' do + expect(group.route).not_to be_nil + expect(group.route.namespace).to eq(group) + end + describe '.where_full_path_in' do context 'without any paths' do it 'returns an empty relation' do @@ -208,30 +213,20 @@ RSpec.describe Project, 'Routable', :with_clean_rails_cache do it_behaves_like 'routable resource with parent' do let_it_be(:record) { project } end + + it 'creates route with namespace referencing project namespace' do + expect(project.route).not_to be_nil + expect(project.route.namespace).to eq(project.project_namespace) + end end RSpec.describe Namespaces::ProjectNamespace, 'Routable', :with_clean_rails_cache do let_it_be(:group) { create(:group) } - let_it_be(:project_namespace) do - # For now we create only project namespace w/o project, otherwise same path - # would be used for project and project namespace. - # This can be removed when route is created automatically for project namespaces. - # https://gitlab.com/gitlab-org/gitlab/-/issues/346448 - create(:project_namespace, project: nil, parent: group, - visibility_level: Gitlab::VisibilityLevel::PUBLIC, - path: 'foo', name: 'foo').tap do |project_namespace| - Route.create!(source: project_namespace, path: project_namespace.full_path, - name: project_namespace.full_name) - end - end - - # we have couple of places where we use generic Namespace, in that case - # we don't want to include ProjectNamespace routes yet - it 'ignores project namespace when searching for generic namespace' do - redirect_route = create(:redirect_route, source: project_namespace) - expect(Namespace.find_by_full_path(project_namespace.full_path)).to be_nil - expect(Namespace.find_by_full_path(redirect_route.path, follow_redirects: true)).to be_nil + it 'skips route creation for the resource' do + expect do + described_class.create!(project: nil, parent: group, visibility_level: Gitlab::VisibilityLevel::PUBLIC, path: 'foo', name: 'foo') + end.not_to change { Route.count } end end diff --git a/spec/models/concerns/triggerable_hooks_spec.rb b/spec/models/concerns/triggerable_hooks_spec.rb index 10a6c1aa821..90c88c888ff 100644 --- a/spec/models/concerns/triggerable_hooks_spec.rb +++ b/spec/models/concerns/triggerable_hooks_spec.rb @@ -46,7 +46,7 @@ RSpec.describe TriggerableHooks do describe '.select_active' do it 'returns hooks that match the active filter' do TestableHook.create!(url: 'http://example1.com', push_events: true) - TestableHook.create!(url: 'http://example2.com', push_events: true) + TestableHook.create!(url: 'http://example.org', push_events: true) filter1 = double(:filter1) filter2 = double(:filter2) allow(ActiveHookFilter).to receive(:new).twice.and_return(filter1, filter2) diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 51fdbfebd3a..8f7c13d7ae6 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -25,12 +25,20 @@ RSpec.describe ContainerRepository do headers: { 'Content-Type' => 'application/json' }) end + it_behaves_like 'having unique enum values' + describe 'associations' do it 'belongs to the project' do expect(repository).to belong_to(:project) end end + describe 'validations' do + it { is_expected.to validate_presence_of(:migration_retries_count) } + it { is_expected.to validate_numericality_of(:migration_retries_count).is_greater_than_or_equal_to(0) } + it { is_expected.to validate_presence_of(:migration_state) } + end + describe '#tag' do it 'has a test tag' do expect(repository.tag('test')).not_to be_nil diff --git a/spec/models/customer_relations/contact_spec.rb b/spec/models/customer_relations/contact_spec.rb index 7e26d324ac2..1225f9d089b 100644 --- a/spec/models/customer_relations/contact_spec.rb +++ b/spec/models/customer_relations/contact_spec.rb @@ -26,6 +26,38 @@ RSpec.describe CustomerRelations::Contact, type: :model do it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email end + describe '#unique_email_for_group_hierarchy' do + let_it_be(:parent) { create(:group) } + let_it_be(:group) { create(:group, parent: parent) } + let_it_be(:subgroup) { create(:group, parent: group) } + + let_it_be(:existing_contact) { create(:contact, group: group) } + + context 'with unique email for group hierarchy' do + subject { build(:contact, group: group) } + + it { is_expected.to be_valid } + end + + context 'with duplicate email in group' do + subject { build(:contact, email: existing_contact.email, group: group) } + + it { is_expected.to be_invalid } + end + + context 'with duplicate email in parent group' do + subject { build(:contact, email: existing_contact.email, group: subgroup) } + + it { is_expected.to be_invalid } + end + + context 'with duplicate email in subgroup' do + subject { build(:contact, email: existing_contact.email, group: parent) } + + it { is_expected.to be_invalid } + end + end + describe '#before_validation' do it 'strips leading and trailing whitespace' do contact = described_class.new(first_name: ' First ', last_name: ' Last ', phone: ' 123456 ') @@ -43,20 +75,27 @@ RSpec.describe CustomerRelations::Contact, type: :model do let_it_be(:other_contacts) { create_list(:contact, 2) } it 'returns ids of contacts from group' do - contact_ids = described_class.find_ids_by_emails(group.id, group_contacts.pluck(:email)) + contact_ids = described_class.find_ids_by_emails(group, group_contacts.pluck(:email)) + + expect(contact_ids).to match_array(group_contacts.pluck(:id)) + end + + it 'returns ids of contacts from parent group' do + subgroup = create(:group, parent: group) + contact_ids = described_class.find_ids_by_emails(subgroup, group_contacts.pluck(:email)) expect(contact_ids).to match_array(group_contacts.pluck(:id)) end it 'does not return ids of contacts from other groups' do - contact_ids = described_class.find_ids_by_emails(group.id, other_contacts.pluck(:email)) + contact_ids = described_class.find_ids_by_emails(group, other_contacts.pluck(:email)) expect(contact_ids).to be_empty end it 'raises ArgumentError when called with too many emails' do too_many_emails = described_class::MAX_PLUCK + 1 - expect { described_class.find_ids_by_emails(group.id, Array(0..too_many_emails)) }.to raise_error(ArgumentError) + expect { described_class.find_ids_by_emails(group, Array(0..too_many_emails)) }.to raise_error(ArgumentError) end end end diff --git a/spec/models/customer_relations/issue_contact_spec.rb b/spec/models/customer_relations/issue_contact_spec.rb index 474455a9884..c6373fddbfb 100644 --- a/spec/models/customer_relations/issue_contact_spec.rb +++ b/spec/models/customer_relations/issue_contact_spec.rb @@ -5,7 +5,8 @@ require 'spec_helper' RSpec.describe CustomerRelations::IssueContact do let_it_be(:issue_contact, reload: true) { create(:issue_customer_relations_contact) } let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:project) { create(:project, group: subgroup) } let_it_be(:issue) { create(:issue, project: project) } subject { issue_contact } @@ -33,17 +34,29 @@ RSpec.describe CustomerRelations::IssueContact do end it 'builds using the same group', :aggregate_failures do - expect(for_issue.contact.group).to eq(group) + expect(for_issue.contact.group).to eq(subgroup) expect(for_contact.issue.project.group).to eq(group) end end describe 'validation' do - let(:built) { build(:issue_customer_relations_contact, issue: create(:issue), contact: create(:contact)) } + it 'fails when the contact group does not belong to the issue group or ancestors' do + built = build(:issue_customer_relations_contact, issue: create(:issue), contact: create(:contact)) - it 'fails when the contact group does not match the issue group' do expect(built).not_to be_valid end + + it 'succeeds when the contact group is the same as the issue group' do + built = build(:issue_customer_relations_contact, issue: create(:issue, project: project), contact: create(:contact, group: subgroup)) + + expect(built).to be_valid + end + + it 'succeeds when the contact group is an ancestor of the issue group' do + built = build(:issue_customer_relations_contact, issue: create(:issue, project: project), contact: create(:contact, group: group)) + + expect(built).to be_valid + end end describe '#self.find_contact_ids_by_emails' do diff --git a/spec/models/dependency_proxy/blob_spec.rb b/spec/models/dependency_proxy/blob_spec.rb index 3c54d3126a8..10d06406ad7 100644 --- a/spec/models/dependency_proxy/blob_spec.rb +++ b/spec/models/dependency_proxy/blob_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe DependencyProxy::Blob, type: :model do it_behaves_like 'ttl_expirable' + it_behaves_like 'destructible', factory: :dependency_proxy_blob describe 'relationships' do it { is_expected.to belong_to(:group) } diff --git a/spec/models/dependency_proxy/manifest_spec.rb b/spec/models/dependency_proxy/manifest_spec.rb index 59415096989..ab7881b1d39 100644 --- a/spec/models/dependency_proxy/manifest_spec.rb +++ b/spec/models/dependency_proxy/manifest_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe DependencyProxy::Manifest, type: :model do it_behaves_like 'ttl_expirable' + it_behaves_like 'destructible', factory: :dependency_proxy_manifest describe 'relationships' do it { is_expected.to belong_to(:group) } diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index 59299a507e4..b76063bfa1a 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Email do end describe 'validations' do - it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email do + it_behaves_like 'an object with email-formatted attributes', :email do subject { build(:email) } end @@ -71,4 +71,84 @@ RSpec.describe Email do end end end + + describe '#confirm' do + let(:expired_confirmation_sent_at) { Date.today - described_class.confirm_within - 7.days } + let(:extant_confirmation_sent_at) { Date.today } + + let(:email) do + create(:email, email: 'test@gitlab.com').tap do |email| + email.update!(confirmation_sent_at: confirmation_sent_at) + end + end + + shared_examples_for 'unconfirmed email' do + it 'returns unconfirmed' do + expect(email.confirmed?).to be_falsey + end + end + + context 'when the confirmation period has expired' do + let(:confirmation_sent_at) { expired_confirmation_sent_at } + + it_behaves_like 'unconfirmed email' + + it 'does not confirm the email' do + email.confirm + + expect(email.confirmed?).to be_falsey + end + end + + context 'when the confirmation period has not expired' do + let(:confirmation_sent_at) { extant_confirmation_sent_at } + + it_behaves_like 'unconfirmed email' + + it 'confirms the email' do + email.confirm + + expect(email.confirmed?).to be_truthy + end + end + end + + describe '#force_confirm' do + let(:expired_confirmation_sent_at) { Date.today - described_class.confirm_within - 7.days } + let(:extant_confirmation_sent_at) { Date.today } + + let(:email) do + create(:email, email: 'test@gitlab.com').tap do |email| + email.update!(confirmation_sent_at: confirmation_sent_at) + end + end + + shared_examples_for 'unconfirmed email' do + it 'returns unconfirmed' do + expect(email.confirmed?).to be_falsey + end + end + + shared_examples_for 'confirms the email on force_confirm' do + it 'confirms an email' do + email.force_confirm + + expect(email.reload.confirmed?).to be_truthy + end + end + + context 'when the confirmation period has expired' do + let(:confirmation_sent_at) { expired_confirmation_sent_at } + + it_behaves_like 'unconfirmed email' + it_behaves_like 'confirms the email on force_confirm' + end + + context 'when the confirmation period has not expired' do + let(:confirmation_sent_at) { extant_confirmation_sent_at } + + it_behaves_like 'unconfirmed email' + it_behaves_like 'confirms the email on force_confirm' + end + end end diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb index ea5d2b27028..de6ce3ba053 100644 --- a/spec/models/experiment_spec.rb +++ b/spec/models/experiment_spec.rb @@ -235,6 +235,54 @@ RSpec.describe Experiment do end end + describe '#record_conversion_event_for_subject' do + let_it_be(:user) { create(:user) } + let_it_be(:experiment) { create(:experiment) } + let_it_be(:context) { { a: 42 } } + + subject(:record_conversion) { experiment.record_conversion_event_for_subject(user, context) } + + context 'when no existing experiment_subject record exists for the given user' do + it 'does not update or create an experiment_subject record' do + expect { record_conversion }.not_to change { ExperimentSubject.all.to_a } + end + end + + context 'when an existing experiment_subject exists for the given user' do + context 'but it has already been converted' do + let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user, converted_at: 2.days.ago) } + + it 'does not update the converted_at value' do + expect { record_conversion }.not_to change { experiment_subject.converted_at } + end + end + + context 'and it has not yet been converted' do + let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user) } + + it 'updates the converted_at value' do + expect { record_conversion }.to change { experiment_subject.reload.converted_at } + end + end + + context 'with no existing context' do + let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user) } + + it 'updates the context' do + expect { record_conversion }.to change { experiment_subject.reload.context }.to('a' => 42) + end + end + + context 'with an existing context' do + let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user, converted_at: 2.days.ago, context: { b: 1 } ) } + + it 'merges the context' do + expect { record_conversion }.to change { experiment_subject.reload.context }.to('a' => 42, 'b' => 1) + end + end + end + end + describe '#record_subject_and_variant!' do let_it_be(:subject_to_record) { create(:group) } let_it_be(:variant) { :control } diff --git a/spec/models/group/crm_settings_spec.rb b/spec/models/group/crm_settings_spec.rb new file mode 100644 index 00000000000..35fcdca6389 --- /dev/null +++ b/spec/models/group/crm_settings_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Group::CrmSettings do + describe 'associations' do + it { is_expected.to belong_to(:group) } + end + + describe 'validations' do + subject { build(:crm_settings) } + + it { is_expected.to validate_presence_of(:group) } + end +end diff --git a/spec/models/group_group_link_spec.rb b/spec/models/group_group_link_spec.rb index 03cc9d7e64c..034a5c1dfc6 100644 --- a/spec/models/group_group_link_spec.rb +++ b/spec/models/group_group_link_spec.rb @@ -29,32 +29,6 @@ RSpec.describe GroupGroupLink do ]) end end - - describe '.public_or_visible_to_user' do - let!(:user_with_access) { create :user } - let!(:user_without_access) { create :user } - let!(:shared_with_group) { create :group, :private } - let!(:shared_group) { create :group } - let!(:private_group_group_link) { create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group) } - - before do - shared_group.add_owner(user_with_access) - shared_group.add_owner(user_without_access) - shared_with_group.add_developer(user_with_access) - end - - context 'when user can access shared group' do - it 'returns the private group' do - expect(described_class.public_or_visible_to_user(shared_group, user_with_access)).to include(private_group_group_link) - end - end - - context 'when user does not have access to shared group' do - it 'does not return private group' do - expect(described_class.public_or_visible_to_user(shared_group, user_without_access)).not_to include(private_group_group_link) - end - end - end end describe 'validation' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index fed4ee3f3a4..05ee2166245 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Group do include ReloadHelpers + include StubGitlabCalls let!(:group) { create(:group) } @@ -39,6 +40,7 @@ RSpec.describe Group do it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') } it { is_expected.to have_many(:contacts).class_name('CustomerRelations::Contact') } it { is_expected.to have_many(:organizations).class_name('CustomerRelations::Organization') } + it { is_expected.to have_one(:crm_settings) } describe '#members & #requesters' do let(:requester) { create(:user) } @@ -63,6 +65,7 @@ RSpec.describe Group do describe 'validations' do it { is_expected.to validate_presence_of :name } + it { is_expected.not_to allow_value('colon:in:path').for(:path) } # This is to validate that a specially crafted name cannot bypass a pattern match. See !72555 it { is_expected.to allow_value('group test_4').for(:name) } it { is_expected.not_to allow_value('test/../foo').for(:name) } it { is_expected.not_to allow_value('<script>alert("Attack!")</script>').for(:name) } @@ -502,6 +505,10 @@ RSpec.describe Group do it { expect(group.descendants.to_sql).not_to include 'traversal_ids @>' } end + describe '#self_and_hierarchy' do + it { expect(group.self_and_hierarchy.to_sql).not_to include 'traversal_ids @>' } + end + describe '#ancestors' do it { expect(group.ancestors.to_sql).not_to include 'traversal_ids <@' } end @@ -526,6 +533,10 @@ RSpec.describe Group do it { expect(group.descendants.to_sql).to include 'traversal_ids @>' } end + describe '#self_and_hierarchy' do + it { expect(group.self_and_hierarchy.to_sql).to include 'traversal_ids @>' } + end + describe '#ancestors' do it { expect(group.ancestors.to_sql).to include "\"namespaces\".\"id\" = #{group.parent_id}" } @@ -670,6 +681,26 @@ RSpec.describe Group do expect(result).to match_array([internal_group]) end end + + describe 'by_ids_or_paths' do + let(:group_path) { 'group_path' } + let!(:group) { create(:group, path: group_path) } + let(:group_id) { group.id } + + it 'returns matching records based on paths' do + expect(described_class.by_ids_or_paths(nil, [group_path])).to match_array([group]) + end + + it 'returns matching records based on ids' do + expect(described_class.by_ids_or_paths([group_id], nil)).to match_array([group]) + end + + it 'returns matching records based on both paths and ids' do + new_group = create(:group) + + expect(described_class.by_ids_or_paths([new_group.id], [group_path])).to match_array([group, new_group]) + end + end end describe '#to_reference' do @@ -2056,6 +2087,23 @@ RSpec.describe Group do end end + describe '#bots' do + subject { group.bots } + + let_it_be(:group) { create(:group) } + let_it_be(:project_bot) { create(:user, :project_bot) } + let_it_be(:user) { create(:user) } + + before_all do + [project_bot, user].each do |member| + group.add_maintainer(member) + end + end + + it { is_expected.to contain_exactly(project_bot) } + it { is_expected.not_to include(user) } + end + describe '#related_group_ids' do let(:nested_group) { create(:group, parent: group) } let(:shared_with_group) { create(:group, parent: group) } @@ -2492,7 +2540,7 @@ RSpec.describe Group do end end - describe '#default_owner' do + describe '#first_owner' do let(:group) { build(:group) } context 'the group has owners' do @@ -2502,7 +2550,7 @@ RSpec.describe Group do end it 'is the first owner' do - expect(group.default_owner) + expect(group.first_owner) .to eq(group.owners.first) .and be_a(User) end @@ -2517,8 +2565,8 @@ RSpec.describe Group do end it 'is the first owner of the parent' do - expect(group.default_owner) - .to eq(parent.default_owner) + expect(group.first_owner) + .to eq(parent.first_owner) .and be_a(User) end end @@ -2529,7 +2577,7 @@ RSpec.describe Group do end it 'is the group.owner' do - expect(group.default_owner) + expect(group.first_owner) .to eq(group.owner) .and be_a(User) end @@ -2775,4 +2823,330 @@ RSpec.describe Group do end end end + + describe '#dependency_proxy_setting' do + subject(:setting) { group.dependency_proxy_setting } + + it 'builds a new policy if one does not exist', :aggregate_failures do + expect(setting.enabled).to eq(true) + expect(setting).not_to be_persisted + end + + context 'with existing policy' do + before do + group.dependency_proxy_setting.update!(enabled: false) + end + + it 'returns the policy if it already exists', :aggregate_failures do + expect(setting.enabled).to eq(false) + expect(setting).to be_persisted + end + end + end + + describe '#crm_enabled?' do + it 'returns false where no crm_settings exist' do + expect(group.crm_enabled?).to be_falsey + end + + it 'returns false where crm_settings.state is disabled' do + create(:crm_settings, enabled: false, group: group) + + expect(group.crm_enabled?).to be_falsey + end + + it 'returns true where crm_settings.state is enabled' do + create(:crm_settings, enabled: true, group: group) + + expect(group.crm_enabled?).to be_truthy + end + end + describe '.get_ids_by_ids_or_paths' do + let(:group_path) { 'group_path' } + let!(:group) { create(:group, path: group_path) } + let(:group_id) { group.id } + + it 'returns ids matching records based on paths' do + expect(described_class.get_ids_by_ids_or_paths(nil, [group_path])).to match_array([group_id]) + end + + it 'returns ids matching records based on ids' do + expect(described_class.get_ids_by_ids_or_paths([group_id], nil)).to match_array([group_id]) + end + + it 'returns ids matching records based on both paths and ids' do + new_group_id = create(:group).id + + expect(described_class.get_ids_by_ids_or_paths([new_group_id], [group_path])).to match_array([group_id, new_group_id]) + end + end + + describe '#shared_with_group_links_visible_to_user' do + let_it_be(:admin) { create :admin } + let_it_be(:normal_user) { create :user } + let_it_be(:user_with_access) { create :user } + let_it_be(:user_with_parent_access) { create :user } + let_it_be(:user_without_access) { create :user } + let_it_be(:shared_group) { create :group } + let_it_be(:parent_group) { create :group, :private } + let_it_be(:shared_with_private_group) { create :group, :private, parent: parent_group } + let_it_be(:shared_with_internal_group) { create :group, :internal } + let_it_be(:shared_with_public_group) { create :group, :public } + let_it_be(:private_group_group_link) { create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_private_group) } + let_it_be(:internal_group_group_link) { create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_internal_group) } + let_it_be(:public_group_group_link) { create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_public_group) } + + before do + shared_with_private_group.add_developer(user_with_access) + parent_group.add_developer(user_with_parent_access) + end + + context 'when user is admin', :enable_admin_mode do + it 'returns all existing shared group links' do + expect(shared_group.shared_with_group_links_visible_to_user(admin)).to contain_exactly(private_group_group_link, internal_group_group_link, public_group_group_link) + end + end + + context 'when user is nil' do + it 'returns only link of public shared group' do + expect(shared_group.shared_with_group_links_visible_to_user(nil)).to contain_exactly(public_group_group_link) + end + end + + context 'when user has no access to private shared group' do + it 'returns links of internal and public shared groups' do + expect(shared_group.shared_with_group_links_visible_to_user(normal_user)).to contain_exactly(internal_group_group_link, public_group_group_link) + end + end + + context 'when user is member of private shared group' do + it 'returns links of private, internal and public shared groups' do + expect(shared_group.shared_with_group_links_visible_to_user(user_with_access)).to contain_exactly(private_group_group_link, internal_group_group_link, public_group_group_link) + end + end + + context 'when user is inherited member of private shared group' do + it 'returns links of private, internal and public shared groups' do + expect(shared_group.shared_with_group_links_visible_to_user(user_with_parent_access)).to contain_exactly(private_group_group_link, internal_group_group_link, public_group_group_link) + end + end + end + + describe '#enforced_runner_token_expiration_interval and #effective_runner_token_expiration_interval' do + shared_examples 'no enforced expiration interval' do + it { expect(subject.enforced_runner_token_expiration_interval).to be_nil } + end + + shared_examples 'enforced expiration interval' do |enforced_interval:| + it { expect(subject.enforced_runner_token_expiration_interval).to eq(enforced_interval) } + end + + shared_examples 'no effective expiration interval' do + it { expect(subject.effective_runner_token_expiration_interval).to be_nil } + end + + shared_examples 'effective expiration interval' do |effective_interval:| + it { expect(subject.effective_runner_token_expiration_interval).to eq(effective_interval) } + end + + context 'when there is no interval in group settings' do + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a group interval' do + let(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 3.days.to_i) } + + subject { create(:group, namespace_settings: group_settings) } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + # runner_token_expiration_interval should not affect the expiration interval, only + # group_runner_token_expiration_interval should. + context 'when there is a site-wide enforced shared interval' do + before do + stub_application_setting(runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a site-wide enforced group interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 5.days + end + + # project_runner_token_expiration_interval should not affect the expiration interval, only + # group_runner_token_expiration_interval should. + context 'when there is a site-wide enforced project interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + # runner_token_expiration_interval should not affect the expiration interval, only + # subgroup_runner_token_expiration_interval should. + context 'when there is a grandparent group enforced group interval' do + let_it_be(:grandparent_group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:grandparent_group) { create(:group, namespace_settings: grandparent_group_settings) } + let_it_be(:parent_group) { create(:group, parent: grandparent_group) } + let_it_be(:subgroup) { create(:group, parent: parent_group) } + + subject { subgroup } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a grandparent group enforced subgroup interval' do + let_it_be(:grandparent_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:grandparent_group) { create(:group, namespace_settings: grandparent_group_settings) } + let_it_be(:parent_group) { create(:group, parent: grandparent_group) } + let_it_be(:subgroup) { create(:group, parent: parent_group) } + + subject { subgroup } + + it_behaves_like 'enforced expiration interval', enforced_interval: 4.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + # project_runner_token_expiration_interval should not affect the expiration interval, only + # subgroup_runner_token_expiration_interval should. + context 'when there is a grandparent group enforced project interval' do + let_it_be(:grandparent_group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:grandparent_group) { create(:group, namespace_settings: grandparent_group_settings) } + let_it_be(:parent_group) { create(:group, parent: grandparent_group) } + let_it_be(:subgroup) { create(:group, parent: parent_group) } + + subject { subgroup } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a parent group enforced interval overridden by group interval' do + let_it_be(:parent_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 5.days.to_i) } + let_it_be(:parent_group) { create(:group, namespace_settings: parent_group_settings) } + let_it_be(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:subgroup_with_settings) { create(:group, parent: parent_group, namespace_settings: group_settings) } + + subject { subgroup_with_settings } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + + it 'has human-readable expiration intervals' do + expect(subject.enforced_runner_token_expiration_interval_human_readable).to eq('5d') + expect(subject.effective_runner_token_expiration_interval_human_readable).to eq('4d') + end + end + + context 'when site-wide enforced interval overrides group interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 3.days.to_i) + end + + let_it_be(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group_with_settings) { create(:group, namespace_settings: group_settings) } + + subject { group_with_settings } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when group interval overrides site-wide enforced interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group_with_settings) { create(:group, namespace_settings: group_settings) } + + subject { group_with_settings } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + context 'when site-wide enforced interval overrides parent group enforced interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 3.days.to_i) + end + + let_it_be(:parent_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:parent_group) { create(:group, namespace_settings: parent_group_settings) } + let_it_be(:subgroup) { create(:group, parent: parent_group) } + + subject { subgroup } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when parent group enforced interval overrides site-wide enforced interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:parent_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:parent_group) { create(:group, namespace_settings: parent_group_settings) } + let_it_be(:subgroup) { create(:group, parent: parent_group) } + + subject { subgroup } + + it_behaves_like 'enforced expiration interval', enforced_interval: 4.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + # Unrelated groups should not affect the expiration interval. + context 'when there is an enforced group interval in an unrelated group' do + let_it_be(:unrelated_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:unrelated_group) { create(:group, namespace_settings: unrelated_group_settings) } + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + # Subgroups should not affect the parent group expiration interval. + context 'when there is an enforced group interval in a subgroup' do + let_it_be(:subgroup_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:subgroup) { create(:group, parent: group, namespace_settings: subgroup_settings) } + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + end end diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index f0ee9a613d8..ec2eca96755 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -40,6 +40,15 @@ RSpec.describe ProjectHook do end end + describe '#parent' do + it 'returns the associated project' do + project = build(:project) + hook = build(:project_hook, project: project) + + expect(hook.parent).to eq(project) + end + end + describe '#application_context' do let_it_be(:hook) { build(:project_hook) } diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb index 4ce2e729d89..85f433f5f81 100644 --- a/spec/models/hooks/service_hook_spec.rb +++ b/spec/models/hooks/service_hook_spec.rb @@ -31,6 +31,36 @@ RSpec.describe ServiceHook do end end + describe '#parent' do + let(:hook) { build(:service_hook, integration: integration) } + + context 'with a project-level integration' do + let(:project) { build(:project) } + let(:integration) { build(:integration, project: project) } + + it 'returns the associated project' do + expect(hook.parent).to eq(project) + end + end + + context 'with a group-level integration' do + let(:group) { build(:group) } + let(:integration) { build(:integration, :group, group: group) } + + it 'returns the associated group' do + expect(hook.parent).to eq(group) + end + end + + context 'with an instance-level integration' do + let(:integration) { build(:integration, :instance) } + + it 'returns nil' do + expect(hook.parent).to be_nil + end + end + end + describe '#application_context' do let(:hook) { build(:service_hook) } diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 17cb5da977a..89bfb742f5d 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -37,7 +37,7 @@ RSpec.describe SystemHook do let(:project) { create(:project, namespace: user.namespace) } let(:group) { create(:group) } let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jg@example.com', password: 'mydummypass' } + { name: 'John Doe', username: 'jduser', email: 'jg@example.com', password: Gitlab::Password.test_default } end before do diff --git a/spec/models/instance_configuration_spec.rb b/spec/models/instance_configuration_spec.rb index 698d74abf03..a47bc6a5b6d 100644 --- a/spec/models/instance_configuration_spec.rb +++ b/spec/models/instance_configuration_spec.rb @@ -205,7 +205,8 @@ RSpec.describe InstanceConfiguration do group_export_limit: 1018, group_download_export_limit: 1019, group_import_limit: 1020, - raw_blob_request_limit: 1021 + raw_blob_request_limit: 1021, + user_email_lookup_limit: 1022 ) end @@ -228,6 +229,7 @@ RSpec.describe InstanceConfiguration do expect(rate_limits[:group_export_download]).to eq({ enabled: true, requests_per_period: 1019, period_in_seconds: 60 }) expect(rate_limits[:group_import]).to eq({ enabled: true, requests_per_period: 1020, period_in_seconds: 60 }) expect(rate_limits[:raw_blob]).to eq({ enabled: true, requests_per_period: 1021, period_in_seconds: 60 }) + expect(rate_limits[:user_email_lookup]).to eq({ enabled: true, requests_per_period: 1022, period_in_seconds: 60 }) end end end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index de47fb3839a..7bc670302f1 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -33,28 +33,28 @@ RSpec.describe Integration do end with_them do - it 'validates the service' do - expect(build(:service, project_id: project_id, group_id: group_id, instance: instance).valid?).to eq(valid) + it 'validates the integration' do + expect(build(:integration, project_id: project_id, group_id: group_id, instance: instance).valid?).to eq(valid) end end - context 'with existing services' do + context 'with existing integrations' do before_all do - create(:service, :instance) - create(:service, project: project) - create(:service, group: group, project: nil) + create(:integration, :instance) + create(:integration, project: project) + create(:integration, group: group, project: nil) end - it 'allows only one instance service per type' do - expect(build(:service, :instance)).to be_invalid + it 'allows only one instance integration per type' do + expect(build(:integration, :instance)).to be_invalid end - it 'allows only one project service per type' do - expect(build(:service, project: project)).to be_invalid + it 'allows only one project integration per type' do + expect(build(:integration, project: project)).to be_invalid end - it 'allows only one group service per type' do - expect(build(:service, group: group, project: nil)).to be_invalid + it 'allows only one group integration per type' do + expect(build(:integration, group: group, project: nil)).to be_invalid end end end @@ -79,93 +79,85 @@ RSpec.describe Integration do end describe '.by_type' do - let!(:service1) { create(:jira_integration) } - let!(:service2) { create(:jira_integration) } - let!(:service3) { create(:redmine_integration) } + let!(:integration1) { create(:jira_integration) } + let!(:integration2) { create(:jira_integration) } + let!(:integration3) { create(:redmine_integration) } subject { described_class.by_type(type) } context 'when type is "JiraService"' do let(:type) { 'JiraService' } - it { is_expected.to match_array([service1, service2]) } + it { is_expected.to match_array([integration1, integration2]) } end context 'when type is "RedmineService"' do let(:type) { 'RedmineService' } - it { is_expected.to match_array([service3]) } + it { is_expected.to match_array([integration3]) } end end describe '.for_group' do - let!(:service1) { create(:jira_integration, project_id: nil, group_id: group.id) } - let!(:service2) { create(:jira_integration) } + let!(:integration1) { create(:jira_integration, project_id: nil, group_id: group.id) } + let!(:integration2) { create(:jira_integration) } - it 'returns the right group service' do - expect(described_class.for_group(group)).to match_array([service1]) + it 'returns the right group integration' do + expect(described_class.for_group(group)).to match_array([integration1]) end end - describe '.confidential_note_hooks' do - it 'includes services where confidential_note_events is true' do - create(:service, active: true, confidential_note_events: true) + shared_examples 'hook scope' do |hook_type| + describe ".#{hook_type}_hooks" do + it "includes services where #{hook_type}_events is true" do + create(:integration, active: true, "#{hook_type}_events": true) - expect(described_class.confidential_note_hooks.count).to eq 1 - end + expect(described_class.send("#{hook_type}_hooks").count).to eq 1 + end - it 'excludes services where confidential_note_events is false' do - create(:service, active: true, confidential_note_events: false) + it "excludes services where #{hook_type}_events is false" do + create(:integration, active: true, "#{hook_type}_events": false) - expect(described_class.confidential_note_hooks.count).to eq 0 + expect(described_class.send("#{hook_type}_hooks").count).to eq 0 + end end end - describe '.alert_hooks' do - it 'includes services where alert_events is true' do - create(:service, active: true, alert_events: true) - - expect(described_class.alert_hooks.count).to eq 1 - end - - it 'excludes services where alert_events is false' do - create(:service, active: true, alert_events: false) - - expect(described_class.alert_hooks.count).to eq 0 - end - end + include_examples 'hook scope', 'confidential_note' + include_examples 'hook scope', 'alert' + include_examples 'hook scope', 'archive_trace' end describe '#operating?' do - it 'is false when the service is not active' do - expect(build(:service).operating?).to eq(false) + it 'is false when the integration is not active' do + expect(build(:integration).operating?).to eq(false) end - it 'is false when the service is not persisted' do - expect(build(:service, active: true).operating?).to eq(false) + it 'is false when the integration is not persisted' do + expect(build(:integration, active: true).operating?).to eq(false) end - it 'is true when the service is active and persisted' do - expect(create(:service, active: true).operating?).to eq(true) + it 'is true when the integration is active and persisted' do + expect(create(:integration, active: true).operating?).to eq(true) end end describe '#testable?' do context 'when integration is project-level' do - subject { build(:service, project: project) } + subject { build(:integration, project: project) } it { is_expected.to be_testable } end context 'when integration is not project-level' do - subject { build(:service, project: nil) } + subject { build(:integration, project: nil) } it { is_expected.not_to be_testable } end end describe '#test' do - let(:integration) { build(:service, project: project) } + let(:integration) { build(:integration, project: project) } let(:data) { 'test' } it 'calls #execute' do @@ -186,32 +178,32 @@ RSpec.describe Integration do end describe '#project_level?' do - it 'is true when service has a project' do - expect(build(:service, project: project)).to be_project_level + it 'is true when integration has a project' do + expect(build(:integration, project: project)).to be_project_level end - it 'is false when service has no project' do - expect(build(:service, project: nil)).not_to be_project_level + it 'is false when integration has no project' do + expect(build(:integration, project: nil)).not_to be_project_level end end describe '#group_level?' do - it 'is true when service has a group' do - expect(build(:service, group: group)).to be_group_level + it 'is true when integration has a group' do + expect(build(:integration, group: group)).to be_group_level end - it 'is false when service has no group' do - expect(build(:service, group: nil)).not_to be_group_level + it 'is false when integration has no group' do + expect(build(:integration, group: nil)).not_to be_group_level end end describe '#instance_level?' do - it 'is true when service has instance-level integration' do - expect(build(:service, :instance)).to be_instance_level + it 'is true when integration has instance-level integration' do + expect(build(:integration, :instance)).to be_instance_level end - it 'is false when service does not have instance-level integration' do - expect(build(:service, instance: false)).not_to be_instance_level + it 'is false when integration does not have instance-level integration' do + expect(build(:integration, instance: false)).not_to be_instance_level end end @@ -231,19 +223,19 @@ RSpec.describe Integration do end describe '.find_or_initialize_all_non_project_specific' do - shared_examples 'service instances' do - it 'returns the available service instances' do + shared_examples 'integration instances' do + it 'returns the available integration instances' do expect(Integration.find_or_initialize_all_non_project_specific(Integration.for_instance).map(&:to_param)) .to match_array(Integration.available_integration_names(include_project_specific: false)) end - it 'does not create service instances' do + it 'does not create integration instances' do expect { Integration.find_or_initialize_all_non_project_specific(Integration.for_instance) } .not_to change(Integration, :count) end end - it_behaves_like 'service instances' + it_behaves_like 'integration instances' context 'with all existing instances' do before do @@ -252,15 +244,15 @@ RSpec.describe Integration do ) end - it_behaves_like 'service instances' + it_behaves_like 'integration instances' - context 'with a previous existing service (MockCiService) and a new service (Asana)' do + context 'with a previous existing integration (MockCiService) and a new integration (Asana)' do before do Integration.insert({ type: 'MockCiService', instance: true }) Integration.delete_by(type: 'AsanaService', instance: true) end - it_behaves_like 'service instances' + it_behaves_like 'integration instances' end end @@ -269,7 +261,7 @@ RSpec.describe Integration do create(:jira_integration, :instance) end - it_behaves_like 'service instances' + it_behaves_like 'integration instances' end end @@ -320,31 +312,31 @@ RSpec.describe Integration do } end - shared_examples 'service creation from an integration' do - it 'creates a correct service for a project integration' do - service = described_class.build_from_integration(integration, project_id: project.id) + shared_examples 'integration creation from an integration' do + it 'creates a correct integration for a project integration' do + new_integration = described_class.build_from_integration(integration, project_id: project.id) - expect(service).to be_active - expect(service.url).to eq(url) - expect(service.api_url).to eq(api_url) - expect(service.username).to eq(username) - expect(service.password).to eq(password) - expect(service.instance).to eq(false) - expect(service.project).to eq(project) - expect(service.group).to eq(nil) + expect(new_integration).to be_active + expect(new_integration.url).to eq(url) + expect(new_integration.api_url).to eq(api_url) + expect(new_integration.username).to eq(username) + expect(new_integration.password).to eq(password) + expect(new_integration.instance).to eq(false) + expect(new_integration.project).to eq(project) + expect(new_integration.group).to eq(nil) end - it 'creates a correct service for a group integration' do - service = described_class.build_from_integration(integration, group_id: group.id) - - expect(service).to be_active - expect(service.url).to eq(url) - expect(service.api_url).to eq(api_url) - expect(service.username).to eq(username) - expect(service.password).to eq(password) - expect(service.instance).to eq(false) - expect(service.project).to eq(nil) - expect(service.group).to eq(group) + it 'creates a correct integration for a group integration' do + new_integration = described_class.build_from_integration(integration, group_id: group.id) + + expect(new_integration).to be_active + expect(new_integration.url).to eq(url) + expect(new_integration.api_url).to eq(api_url) + expect(new_integration.username).to eq(username) + expect(new_integration.password).to eq(password) + expect(new_integration.instance).to eq(false) + expect(new_integration.project).to eq(nil) + expect(new_integration.group).to eq(group) end end @@ -355,7 +347,7 @@ RSpec.describe Integration do create(:jira_integration, :without_properties_callback, properties: properties.merge(additional: 'something')) end - it_behaves_like 'service creation from an integration' + it_behaves_like 'integration creation from an integration' end context 'when data are stored in separated fields' do @@ -363,7 +355,7 @@ RSpec.describe Integration do create(:jira_integration, data_params.merge(properties: {})) end - it_behaves_like 'service creation from an integration' + it_behaves_like 'integration creation from an integration' end context 'when data are stored in both properties and separated fields' do @@ -374,7 +366,7 @@ RSpec.describe Integration do end end - it_behaves_like 'service creation from an integration' + it_behaves_like 'integration creation from an integration' end end end @@ -565,17 +557,17 @@ RSpec.describe Integration do end describe '.integration_name_to_model' do - it 'returns the model for the given service name' do + it 'returns the model for the given integration name' do expect(described_class.integration_name_to_model('asana')).to eq(Integrations::Asana) end - it 'raises an error if service name is invalid' do + it 'raises an error if integration name is invalid' do expect { described_class.integration_name_to_model('foo') }.to raise_exception(NameError, /uninitialized constant FooService/) end end describe "{property}_changed?" do - let(:service) do + let(:integration) do Integrations::Bamboo.create!( project: project, properties: { @@ -587,35 +579,35 @@ RSpec.describe Integration do end it "returns false when the property has not been assigned a new value" do - service.username = "key_changed" - expect(service.bamboo_url_changed?).to be_falsy + integration.username = "key_changed" + expect(integration.bamboo_url_changed?).to be_falsy end it "returns true when the property has been assigned a different value" do - service.bamboo_url = "http://example.com" - expect(service.bamboo_url_changed?).to be_truthy + integration.bamboo_url = "http://example.com" + expect(integration.bamboo_url_changed?).to be_truthy end it "returns true when the property has been assigned a different value twice" do - service.bamboo_url = "http://example.com" - service.bamboo_url = "http://example.com" - expect(service.bamboo_url_changed?).to be_truthy + integration.bamboo_url = "http://example.com" + integration.bamboo_url = "http://example.com" + expect(integration.bamboo_url_changed?).to be_truthy end it "returns false when the property has been re-assigned the same value" do - service.bamboo_url = 'http://gitlab.com' - expect(service.bamboo_url_changed?).to be_falsy + integration.bamboo_url = 'http://gitlab.com' + expect(integration.bamboo_url_changed?).to be_falsy end it "returns false when the property has been assigned a new value then saved" do - service.bamboo_url = 'http://example.com' - service.save! - expect(service.bamboo_url_changed?).to be_falsy + integration.bamboo_url = 'http://example.com' + integration.save! + expect(integration.bamboo_url_changed?).to be_falsy end end describe "{property}_touched?" do - let(:service) do + let(:integration) do Integrations::Bamboo.create!( project: project, properties: { @@ -627,35 +619,35 @@ RSpec.describe Integration do end it "returns false when the property has not been assigned a new value" do - service.username = "key_changed" - expect(service.bamboo_url_touched?).to be_falsy + integration.username = "key_changed" + expect(integration.bamboo_url_touched?).to be_falsy end it "returns true when the property has been assigned a different value" do - service.bamboo_url = "http://example.com" - expect(service.bamboo_url_touched?).to be_truthy + integration.bamboo_url = "http://example.com" + expect(integration.bamboo_url_touched?).to be_truthy end it "returns true when the property has been assigned a different value twice" do - service.bamboo_url = "http://example.com" - service.bamboo_url = "http://example.com" - expect(service.bamboo_url_touched?).to be_truthy + integration.bamboo_url = "http://example.com" + integration.bamboo_url = "http://example.com" + expect(integration.bamboo_url_touched?).to be_truthy end it "returns true when the property has been re-assigned the same value" do - service.bamboo_url = 'http://gitlab.com' - expect(service.bamboo_url_touched?).to be_truthy + integration.bamboo_url = 'http://gitlab.com' + expect(integration.bamboo_url_touched?).to be_truthy end it "returns false when the property has been assigned a new value then saved" do - service.bamboo_url = 'http://example.com' - service.save! - expect(service.bamboo_url_changed?).to be_falsy + integration.bamboo_url = 'http://example.com' + integration.save! + expect(integration.bamboo_url_changed?).to be_falsy end end describe "{property}_was" do - let(:service) do + let(:integration) do Integrations::Bamboo.create!( project: project, properties: { @@ -667,35 +659,35 @@ RSpec.describe Integration do end it "returns nil when the property has not been assigned a new value" do - service.username = "key_changed" - expect(service.bamboo_url_was).to be_nil + integration.username = "key_changed" + expect(integration.bamboo_url_was).to be_nil end it "returns the previous value when the property has been assigned a different value" do - service.bamboo_url = "http://example.com" - expect(service.bamboo_url_was).to eq('http://gitlab.com') + integration.bamboo_url = "http://example.com" + expect(integration.bamboo_url_was).to eq('http://gitlab.com') end it "returns initial value when the property has been re-assigned the same value" do - service.bamboo_url = 'http://gitlab.com' - expect(service.bamboo_url_was).to eq('http://gitlab.com') + integration.bamboo_url = 'http://gitlab.com' + expect(integration.bamboo_url_was).to eq('http://gitlab.com') end it "returns initial value when the property has been assigned multiple values" do - service.bamboo_url = "http://example.com" - service.bamboo_url = "http://example2.com" - expect(service.bamboo_url_was).to eq('http://gitlab.com') + integration.bamboo_url = "http://example.com" + integration.bamboo_url = "http://example.org" + expect(integration.bamboo_url_was).to eq('http://gitlab.com') end it "returns nil when the property has been assigned a new value then saved" do - service.bamboo_url = 'http://example.com' - service.save! - expect(service.bamboo_url_was).to be_nil + integration.bamboo_url = 'http://example.com' + integration.save! + expect(integration.bamboo_url_was).to be_nil end end - describe 'initialize service with no properties' do - let(:service) do + describe 'initialize integration with no properties' do + let(:integration) do Integrations::Bugzilla.create!( project: project, project_url: 'http://gitlab.example.com' @@ -703,16 +695,16 @@ RSpec.describe Integration do end it 'does not raise error' do - expect { service }.not_to raise_error + expect { integration }.not_to raise_error end it 'sets data correctly' do - expect(service.data_fields.project_url).to eq('http://gitlab.example.com') + expect(integration.data_fields.project_url).to eq('http://gitlab.example.com') end end describe '#api_field_names' do - let(:fake_service) do + let(:fake_integration) do Class.new(Integration) do def fields [ @@ -728,8 +720,8 @@ RSpec.describe Integration do end end - let(:service) do - fake_service.new(properties: [ + let(:integration) do + fake_integration.new(properties: [ { token: 'token-value' }, { api_token: 'api_token-value' }, { key: 'key-value' }, @@ -741,16 +733,16 @@ RSpec.describe Integration do end it 'filters out sensitive fields' do - expect(service.api_field_names).to eq(['safe_field']) + expect(integration.api_field_names).to eq(['safe_field']) end end context 'logging' do - let(:service) { build(:service, project: project) } + let(:integration) { build(:integration, project: project) } let(:test_message) { "test message" } let(:arguments) do { - service_class: service.class.name, + service_class: integration.class.name, project_path: project.full_path, project_id: project.id, message: test_message, @@ -761,20 +753,20 @@ RSpec.describe Integration do it 'logs info messages using json logger' do expect(Gitlab::JsonLogger).to receive(:info).with(arguments) - service.log_info(test_message, additional_argument: 'some argument') + integration.log_info(test_message, additional_argument: 'some argument') end it 'logs error messages using json logger' do expect(Gitlab::JsonLogger).to receive(:error).with(arguments) - service.log_error(test_message, additional_argument: 'some argument') + integration.log_error(test_message, additional_argument: 'some argument') end context 'when project is nil' do let(:project) { nil } let(:arguments) do { - service_class: service.class.name, + service_class: integration.class.name, project_path: nil, project_id: nil, message: test_message, @@ -785,7 +777,7 @@ RSpec.describe Integration do it 'logs info messages using json logger' do expect(Gitlab::JsonLogger).to receive(:info).with(arguments) - service.log_info(test_message, additional_argument: 'some argument') + integration.log_info(test_message, additional_argument: 'some argument') end end end diff --git a/spec/models/integrations/asana_spec.rb b/spec/models/integrations/asana_spec.rb index f7e7eb1b0ae..b6602964182 100644 --- a/spec/models/integrations/asana_spec.rb +++ b/spec/models/integrations/asana_spec.rb @@ -14,27 +14,29 @@ RSpec.describe Integrations::Asana do end describe 'Execute' do - let(:user) { create(:user) } - let(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let(:gid) { "123456789ABCD" } + let(:asana_task) { double(::Asana::Resources::Task) } + let(:asana_integration) { described_class.new } - def create_data_for_commits(*messages) + let(:data) do { object_kind: 'push', ref: 'master', user_name: user.name, - commits: messages.map do |m| + commits: [ { - message: m, + message: message, url: 'https://gitlab.com/' } - end + ] } end before do - @asana = described_class.new - allow(@asana).to receive_messages( + allow(asana_integration).to receive_messages( project: project, project_id: project.id, api_key: 'verySecret', @@ -42,67 +44,79 @@ RSpec.describe Integrations::Asana do ) end - it 'calls Asana integration to create a story' do - data = create_data_for_commits("Message from commit. related to ##{gid}") - expected_message = "#{data[:user_name]} pushed to branch #{data[:ref]} of #{project.full_name} ( #{data[:commits][0][:url]} ): #{data[:commits][0][:message]}" + subject(:execute_integration) { asana_integration.execute(data) } + + context 'when creating a story' do + let(:message) { "Message from commit. related to ##{gid}" } + let(:expected_message) do + "#{user.name} pushed to branch master of #{project.full_name} ( https://gitlab.com/ ): #{message}" + end - d1 = double('Asana::Resources::Task') - expect(d1).to receive(:add_comment).with(text: expected_message) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, gid).once.and_return(d1) + it 'calls Asana integration to create a story' do + expect(asana_task).to receive(:add_comment).with(text: expected_message) + expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, gid).once.and_return(asana_task) - @asana.execute(data) + execute_integration + end end - it 'calls Asana integration to create a story and close a task' do - data = create_data_for_commits('fix #456789') - d1 = double('Asana::Resources::Task') - expect(d1).to receive(:add_comment) - expect(d1).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '456789').once.and_return(d1) + context 'when creating a story and closing a task' do + let(:message) { 'fix #456789' } - @asana.execute(data) + it 'calls Asana integration to create a story and close a task' do + expect(asana_task).to receive(:add_comment) + expect(asana_task).to receive(:update).with(completed: true) + expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '456789').once.and_return(asana_task) + + execute_integration + end end - it 'is able to close via url' do - data = create_data_for_commits('closes https://app.asana.com/19292/956299/42') - d1 = double('Asana::Resources::Task') - expect(d1).to receive(:add_comment) - expect(d1).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '42').once.and_return(d1) + context 'when closing via url' do + let(:message) { 'closes https://app.asana.com/19292/956299/42' } - @asana.execute(data) + it 'calls Asana integration to close via url' do + expect(asana_task).to receive(:add_comment) + expect(asana_task).to receive(:update).with(completed: true) + expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '42').once.and_return(asana_task) + + execute_integration + end end - it 'allows multiple matches per line' do - message = <<-EOF - minor bigfix, refactoring, fixed #123 and Closes #456 work on #789 - ref https://app.asana.com/19292/956299/42 and closing https://app.asana.com/19292/956299/12 - EOF - data = create_data_for_commits(message) - d1 = double('Asana::Resources::Task') - expect(d1).to receive(:add_comment) - expect(d1).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '123').once.and_return(d1) - - d2 = double('Asana::Resources::Task') - expect(d2).to receive(:add_comment) - expect(d2).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '456').once.and_return(d2) - - d3 = double('Asana::Resources::Task') - expect(d3).to receive(:add_comment) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '789').once.and_return(d3) - - d4 = double('Asana::Resources::Task') - expect(d4).to receive(:add_comment) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '42').once.and_return(d4) - - d5 = double('Asana::Resources::Task') - expect(d5).to receive(:add_comment) - expect(d5).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '12').once.and_return(d5) - - @asana.execute(data) + context 'with multiple matches per line' do + let(:message) do + <<-EOF + minor bigfix, refactoring, fixed #123 and Closes #456 work on #789 + ref https://app.asana.com/19292/956299/42 and closing https://app.asana.com/19292/956299/12 + EOF + end + + it 'allows multiple matches per line' do + expect(asana_task).to receive(:add_comment) + expect(asana_task).to receive(:update).with(completed: true) + expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '123').once.and_return(asana_task) + + asana_task_2 = double(Asana::Resources::Task) + expect(asana_task_2).to receive(:add_comment) + expect(asana_task_2).to receive(:update).with(completed: true) + expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '456').once.and_return(asana_task_2) + + asana_task_3 = double(Asana::Resources::Task) + expect(asana_task_3).to receive(:add_comment) + expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '789').once.and_return(asana_task_3) + + asana_task_4 = double(Asana::Resources::Task) + expect(asana_task_4).to receive(:add_comment) + expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '42').once.and_return(asana_task_4) + + asana_task_5 = double(Asana::Resources::Task) + expect(asana_task_5).to receive(:add_comment) + expect(asana_task_5).to receive(:update).with(completed: true) + expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '12').once.and_return(asana_task_5) + + execute_integration + end end end end diff --git a/spec/models/integrations/datadog_spec.rb b/spec/models/integrations/datadog_spec.rb index 9c3ff7aa35b..9856c53a390 100644 --- a/spec/models/integrations/datadog_spec.rb +++ b/spec/models/integrations/datadog_spec.rb @@ -38,6 +38,11 @@ RSpec.describe Integrations::Datadog do let(:pipeline_data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } let(:build_data) { Gitlab::DataBuilder::Build.build(build) } + let(:archive_trace_data) do + create(:ci_job_artifact, :trace, job: build) + + Gitlab::DataBuilder::ArchiveTrace.build(build) + end it_behaves_like Integrations::HasWebHook do let(:integration) { instance } @@ -100,6 +105,13 @@ RSpec.describe Integrations::Datadog do end end + describe '#help' do + subject { instance.help } + + it { is_expected.to be_a(String) } + it { is_expected.not_to be_empty } + end + describe '#hook_url' do subject { instance.hook_url } @@ -161,13 +173,16 @@ RSpec.describe Integrations::Datadog do end before do + stub_feature_flags(datadog_integration_logs_collection: enable_logs_collection) stub_request(:post, expected_hook_url) saved_instance.execute(data) end + let(:enable_logs_collection) { true } + context 'with pipeline data' do let(:data) { pipeline_data } - let(:expected_headers) { { WebHookService::GITLAB_EVENT_HEADER => 'Pipeline Hook' } } + let(:expected_headers) { { ::Gitlab::WebHooks::GITLAB_EVENT_HEADER => 'Pipeline Hook' } } let(:expected_body) { data.with_retried_builds.to_json } it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers, body: expected_body)).to have_been_made } @@ -175,10 +190,24 @@ RSpec.describe Integrations::Datadog do context 'with job data' do let(:data) { build_data } - let(:expected_headers) { { WebHookService::GITLAB_EVENT_HEADER => 'Job Hook' } } + let(:expected_headers) { { ::Gitlab::WebHooks::GITLAB_EVENT_HEADER => 'Job Hook' } } + let(:expected_body) { data.to_json } + + it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers, body: expected_body)).to have_been_made } + end + + context 'with archive trace data' do + let(:data) { archive_trace_data } + let(:expected_headers) { { ::Gitlab::WebHooks::GITLAB_EVENT_HEADER => 'Archive Trace Hook' } } let(:expected_body) { data.to_json } it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers, body: expected_body)).to have_been_made } + + context 'but feature flag disabled' do + let(:enable_logs_collection) { false } + + it { expect(a_request(:post, expected_hook_url)).not_to have_been_made } + end end end end diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 9163a7ef845..e80fa6e3b70 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -937,18 +937,6 @@ RSpec.describe Integrations::Jira do end end - context 'with jira_use_first_ref_by_oid feature flag disabled' do - before do - stub_feature_flags(jira_use_first_ref_by_oid: false) - end - - it 'creates a comment and remote link on Jira' do - expect(subject).to eq(success_message) - expect(WebMock).to have_requested(:post, comment_url).with(body: comment_body).once - expect(WebMock).to have_requested(:post, remote_link_url).once - end - end - it 'tracks usage' do expect(Gitlab::UsageDataCounters::HLLRedisCounter) .to receive(:track_event) diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb index 51b27151ba2..f0007e1203c 100644 --- a/spec/models/internal_id_spec.rb +++ b/spec/models/internal_id_spec.rb @@ -87,7 +87,7 @@ RSpec.describe InternalId do context 'when executed outside of transaction' do it 'increments counter with in_transaction: "false"' do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases + allow(ApplicationRecord.connection).to receive(:transaction_open?) { false } expect(InternalId.internal_id_transactions_total).to receive(:increment) .with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original @@ -146,7 +146,7 @@ RSpec.describe InternalId do let(:value) { 2 } it 'increments counter with in_transaction: "false"' do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases + allow(ApplicationRecord.connection).to receive(:transaction_open?) { false } expect(InternalId.internal_id_transactions_total).to receive(:increment) .with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original @@ -217,7 +217,7 @@ RSpec.describe InternalId do context 'when executed outside of transaction' do it 'increments counter with in_transaction: "false"' do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases + allow(ApplicationRecord.connection).to receive(:transaction_open?) { false } expect(InternalId.internal_id_transactions_total).to receive(:increment) .with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 4cbfa7c7758..c105f6c3439 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Issue do it { is_expected.to belong_to(:iteration) } it { is_expected.to belong_to(:project) } it { is_expected.to have_one(:namespace).through(:project) } - it { is_expected.to belong_to(:work_item_type).class_name('WorkItem::Type') } + it { is_expected.to belong_to(:work_item_type).class_name('WorkItems::Type') } it { is_expected.to belong_to(:moved_to).class_name('Issue') } it { is_expected.to have_one(:moved_from).class_name('Issue') } it { is_expected.to belong_to(:duplicated_to).class_name('Issue') } @@ -238,6 +238,17 @@ RSpec.describe Issue do end end + # TODO: Remove when NOT NULL constraint is added to the relationship + describe '#work_item_type' do + let(:issue) { create(:issue, :incident, project: reusable_project, work_item_type: nil) } + + it 'returns a default type if the legacy issue does not have a work item type associated yet' do + expect(issue.work_item_type_id).to be_nil + expect(issue.issue_type).to eq('incident') + expect(issue.work_item_type).to eq(WorkItems::Type.default_by_type(:incident)) + end + end + describe '#sort' do let(:project) { reusable_project } @@ -1317,28 +1328,10 @@ RSpec.describe Issue do let_it_be(:issue1) { create(:issue, project: project, relative_position: nil) } let_it_be(:issue2) { create(:issue, project: project, relative_position: nil) } - context 'when optimized_issue_neighbor_queries is enabled' do - before do - stub_feature_flags(optimized_issue_neighbor_queries: true) - end - - it_behaves_like "a class that supports relative positioning" do - let_it_be(:project) { reusable_project } - let(:factory) { :issue } - let(:default_params) { { project: project } } - end - end - - context 'when optimized_issue_neighbor_queries is disabled' do - before do - stub_feature_flags(optimized_issue_neighbor_queries: false) - end - - it_behaves_like "a class that supports relative positioning" do - let_it_be(:project) { reusable_project } - let(:factory) { :issue } - let(:default_params) { { project: project } } - end + it_behaves_like "a class that supports relative positioning" do + let_it_be(:project) { reusable_project } + let(:factory) { :issue } + let(:default_params) { { project: project } } end it 'is not blocked for repositioning by default' do @@ -1580,4 +1573,13 @@ RSpec.describe Issue do expect(participant.issue.email_participants_emails_downcase).to match([participant.email.downcase]) end end + + describe '#escalation_status' do + it 'returns the incident_management_issuable_escalation_status association' do + escalation_status = create(:incident_management_issuable_escalation_status) + issue = escalation_status.issue + + expect(issue.escalation_status).to eq(escalation_status) + end + end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index d41a1604211..19459561edf 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -21,6 +21,28 @@ RSpec.describe Key, :mailer do it { is_expected.to allow_value(attributes_for(:ecdsa_key_256)[:key]).for(:key) } it { is_expected.to allow_value(attributes_for(:ed25519_key_256)[:key]).for(:key) } it { is_expected.not_to allow_value('foo-bar').for(:key) } + + context 'key format' do + let(:key) { build(:key) } + + it 'does not allow the key that begins with an algorithm name that is unsupported' do + key.key = 'unsupported-ssh-rsa key' + + key.valid? + + expect(key.errors.of_kind?(:key, :invalid)).to eq(true) + end + + Gitlab::SSHPublicKey.supported_algorithms.each do |supported_algorithm| + it "allows the key that begins with supported algorithm name '#{supported_algorithm}'" do + key.key = "#{supported_algorithm} key" + + key.valid? + + expect(key.errors.of_kind?(:key, :invalid)).to eq(false) + end + end + end end describe "Methods" do diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 7ce32de6edc..1957c58ec81 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -9,6 +9,7 @@ RSpec.describe Member do describe 'Associations' do it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:member_namespace) } it { is_expected.to have_one(:member_task) } end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index e1db1b3cf3e..4005a2ec6da 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1648,10 +1648,7 @@ RSpec.describe MergeRequest, factory_default: :keep do it 'uses template from target project' do request = build(:merge_request, title: 'Fix everything') - request.compare_commits = [ - double(safe_message: 'Commit message', gitaly_commit?: true, merge_commit?: false, description?: false) - ] - subject.target_project.merge_commit_template = '%{title}' + request.target_project.merge_commit_template = '%{title}' expect(request.default_merge_commit_message) .to eq('Fix everything') @@ -3495,84 +3492,6 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - describe "#environments_for" do - let(:project) { create(:project, :repository) } - let(:user) { project.creator } - let(:merge_request) { create(:merge_request, source_project: project) } - let(:source_branch) { merge_request.source_branch } - let(:target_branch) { merge_request.target_branch } - let(:source_oid) { project.commit(source_branch).id } - let(:target_oid) { project.commit(target_branch).id } - - before do - merge_request.source_project.add_maintainer(user) - merge_request.target_project.add_maintainer(user) - end - - context 'with multiple environments' do - let(:environments) { create_list(:environment, 3, project: project) } - - before do - create(:deployment, :success, environment: environments.first, ref: source_branch, sha: source_oid) - create(:deployment, :success, environment: environments.second, ref: target_branch, sha: target_oid) - end - - it 'selects deployed environments' do - expect(merge_request.environments_for(user)).to contain_exactly(environments.first) - end - - it 'selects latest deployed environment' do - latest_environment = create(:environment, project: project) - create(:deployment, :success, environment: latest_environment, ref: source_branch, sha: source_oid) - - expect(merge_request.environments_for(user)).to eq([environments.first, latest_environment]) - expect(merge_request.environments_for(user, latest: true)).to contain_exactly(latest_environment) - end - end - - context 'with environments on source project' do - let(:source_project) { fork_project(project, nil, repository: true) } - - let(:merge_request) do - create(:merge_request, - source_project: source_project, source_branch: 'feature', - target_project: project) - end - - let(:source_environment) { create(:environment, project: source_project) } - - before do - create(:deployment, :success, environment: source_environment, ref: 'feature', sha: merge_request.diff_head_sha) - end - - it 'selects deployed environments', :sidekiq_might_not_need_inline do - expect(merge_request.environments_for(user)).to contain_exactly(source_environment) - end - - context 'with environments on target project' do - let(:target_environment) { create(:environment, project: project) } - - before do - create(:deployment, :success, environment: target_environment, tag: true, sha: merge_request.diff_head_sha) - end - - it 'selects deployed environments', :sidekiq_might_not_need_inline do - expect(merge_request.environments_for(user)).to contain_exactly(source_environment, target_environment) - end - end - end - - context 'without a diff_head_commit' do - before do - expect(merge_request).to receive(:diff_head_commit).and_return(nil) - end - - it 'returns an empty array' do - expect(merge_request.environments_for(user)).to be_empty - end - end - end - describe "#environments" do subject { merge_request.environments } diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index 429727c2360..c9f8a1bcdc2 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -126,57 +126,4 @@ RSpec.describe NamespaceSetting, type: :model do end end end - - describe 'hooks related to group user cap update' do - let(:settings) { create(:namespace_settings, new_user_signups_cap: user_cap) } - let(:group) { create(:group, namespace_settings: settings) } - - before do - allow(group).to receive(:root?).and_return(true) - end - - context 'when updating a group with a user cap' do - let(:user_cap) { nil } - - it 'also sets share_with_group_lock and prevent_sharing_groups_outside_hierarchy to true' do - expect(group.new_user_signups_cap).to be_nil - expect(group.share_with_group_lock).to be_falsey - expect(settings.prevent_sharing_groups_outside_hierarchy).to be_falsey - - settings.update!(new_user_signups_cap: 10) - group.reload - - expect(group.new_user_signups_cap).to eq(10) - expect(group.share_with_group_lock).to be_truthy - expect(settings.reload.prevent_sharing_groups_outside_hierarchy).to be_truthy - end - - it 'has share_with_group_lock and prevent_sharing_groups_outside_hierarchy returning true for descendent groups' do - descendent = create(:group, parent: group) - desc_settings = descendent.namespace_settings - - expect(descendent.share_with_group_lock).to be_falsey - expect(desc_settings.prevent_sharing_groups_outside_hierarchy).to be_falsey - - settings.update!(new_user_signups_cap: 10) - - expect(descendent.reload.share_with_group_lock).to be_truthy - expect(desc_settings.reload.prevent_sharing_groups_outside_hierarchy).to be_truthy - end - end - - context 'when removing a user cap from namespace settings' do - let(:user_cap) { 10 } - - it 'leaves share_with_group_lock and prevent_sharing_groups_outside_hierarchy set to true to the related group' do - expect(group.share_with_group_lock).to be_truthy - expect(settings.prevent_sharing_groups_outside_hierarchy).to be_truthy - - settings.update!(new_user_signups_cap: nil) - - expect(group.reload.share_with_group_lock).to be_truthy - expect(settings.reload.prevent_sharing_groups_outside_hierarchy).to be_truthy - end - end - end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 54327fc70d9..5da0f7a134c 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -28,6 +28,8 @@ RSpec.describe Namespace do it { is_expected.to have_one :onboarding_progress } it { is_expected.to have_one :admin_note } it { is_expected.to have_many :pending_builds } + it { is_expected.to have_one :namespace_route } + it { is_expected.to have_many :namespace_members } describe '#children' do let_it_be(:group) { create(:group) } @@ -1263,6 +1265,32 @@ RSpec.describe Namespace do end end + describe '#use_traversal_ids_for_self_and_hierarchy?' do + let_it_be(:namespace, reload: true) { create(:namespace) } + + subject { namespace.use_traversal_ids_for_self_and_hierarchy? } + + it { is_expected.to eq true } + + it_behaves_like 'disabled feature flag when traversal_ids is blank' + + context 'when use_traversal_ids_for_self_and_hierarchy feature flag is false' do + before do + stub_feature_flags(use_traversal_ids_for_self_and_hierarchy: false) + end + + it { is_expected.to eq false } + end + + context 'when use_traversal_ids? feature flag is false' do + before do + stub_feature_flags(use_traversal_ids: false) + end + + it { is_expected.to eq false } + end + end + describe '#users_with_descendants' do let(:user_a) { create(:user) } let(:user_b) { create(:user) } diff --git a/spec/models/namespaces/project_namespace_spec.rb b/spec/models/namespaces/project_namespace_spec.rb index 4416c49f1bf..47cf866c143 100644 --- a/spec/models/namespaces/project_namespace_spec.rb +++ b/spec/models/namespaces/project_namespace_spec.rb @@ -17,11 +17,11 @@ RSpec.describe Namespaces::ProjectNamespace, type: :model do let_it_be(:project) { create(:project) } let_it_be(:project_namespace) { project.project_namespace } - it 'also deletes the associated project' do + it 'keeps the associated project' do project_namespace.delete expect { project_namespace.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(project.reload.project_namespace).to be_nil end end end diff --git a/spec/models/onboarding_progress_spec.rb b/spec/models/onboarding_progress_spec.rb index deac8d29196..80a39404d10 100644 --- a/spec/models/onboarding_progress_spec.rb +++ b/spec/models/onboarding_progress_spec.rb @@ -131,29 +131,86 @@ RSpec.describe OnboardingProgress do end describe '.register' do - subject(:register_action) { described_class.register(namespace, action) } + context 'for a single action' do + subject(:register_action) { described_class.register(namespace, action) } - context 'when the namespace was onboarded' do - before do - described_class.onboard(namespace) - end + context 'when the namespace was onboarded' do + before do + described_class.onboard(namespace) + end - it 'registers the action for the namespace' do - expect { register_action }.to change { described_class.completed?(namespace, action) }.from(false).to(true) - end + it 'registers the action for the namespace' do + expect { register_action }.to change { described_class.completed?(namespace, action) }.from(false).to(true) + end - context 'when the action does not exist' do - let(:action) { :foo } + it 'does not override timestamp', :aggregate_failures do + expect(described_class.find_by_namespace_id(namespace.id).subscription_created_at).to be_nil + register_action + expect(described_class.find_by_namespace_id(namespace.id).subscription_created_at).not_to be_nil + expect { described_class.register(namespace, action) }.not_to change { described_class.find_by_namespace_id(namespace.id).subscription_created_at } + end + + context 'when the action does not exist' do + let(:action) { :foo } + it 'does not register the action for the namespace' do + expect { register_action }.not_to change { described_class.completed?(namespace, action) }.from(nil) + end + end + end + + context 'when the namespace was not onboarded' do it 'does not register the action for the namespace' do - expect { register_action }.not_to change { described_class.completed?(namespace, action) }.from(nil) + expect { register_action }.not_to change { described_class.completed?(namespace, action) }.from(false) end end end - context 'when the namespace was not onboarded' do - it 'does not register the action for the namespace' do - expect { register_action }.not_to change { described_class.completed?(namespace, action) }.from(false) + context 'for multiple actions' do + let(:action1) { :security_scan_enabled } + let(:action2) { :secure_dependency_scanning_run } + let(:actions) { [action1, action2] } + + subject(:register_action) { described_class.register(namespace, actions) } + + context 'when the namespace was onboarded' do + before do + described_class.onboard(namespace) + end + + it 'registers the actions for the namespace' do + expect { register_action }.to change { + [described_class.completed?(namespace, action1), described_class.completed?(namespace, action2)] + }.from([false, false]).to([true, true]) + end + + it 'does not override timestamp', :aggregate_failures do + described_class.register(namespace, [action1]) + expect(described_class.find_by_namespace_id(namespace.id).security_scan_enabled_at).not_to be_nil + expect(described_class.find_by_namespace_id(namespace.id).secure_dependency_scanning_run_at).to be_nil + + expect { described_class.register(namespace, [action1, action2]) }.not_to change { + described_class.find_by_namespace_id(namespace.id).security_scan_enabled_at + } + expect(described_class.find_by_namespace_id(namespace.id).secure_dependency_scanning_run_at).not_to be_nil + end + + context 'when one of the actions does not exist' do + let(:action2) { :foo } + + it 'does not register any action for the namespace' do + expect { register_action }.not_to change { + [described_class.completed?(namespace, action1), described_class.completed?(namespace, action2)] + }.from([false, nil]) + end + end + end + + context 'when the namespace was not onboarded' do + it 'does not register the action for the namespace' do + expect { register_action }.not_to change { described_class.completed?(namespace, action1) }.from(false) + expect { described_class.register(namespace, action) }.not_to change { described_class.completed?(namespace, action2) }.from(false) + end end end end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index 8617793f41d..a86caa074f1 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -10,6 +10,9 @@ RSpec.describe Packages::PackageFile, type: :model do let_it_be(:package_file3) { create(:package_file, :xml, file_name: 'formatted.zip') } let_it_be(:debian_package) { create(:debian_package, project: project) } + it_behaves_like 'having unique enum values' + it_behaves_like 'destructible', factory: :package_file + describe 'relationships' do it { is_expected.to belong_to(:package) } it { is_expected.to have_one(:conan_file_metadatum) } @@ -138,6 +141,24 @@ RSpec.describe Packages::PackageFile, type: :model do it 'returns the matching file only for Helm packages' do expect(described_class.for_helm_with_channel(project, channel)).to contain_exactly(helm_file2) end + + context 'with package files pending destruction' do + let_it_be(:package_file_pending_destruction) { create(:helm_package_file, :pending_destruction, package: helm_package2, channel: channel) } + + it 'does not return them' do + expect(described_class.for_helm_with_channel(project, channel)).to contain_exactly(helm_file2) + end + + context 'with packages_installable_package_files disabled' do + before do + stub_feature_flags(packages_installable_package_files: false) + end + + it 'returns them' do + expect(described_class.for_helm_with_channel(project, channel)).to contain_exactly(helm_file2, package_file_pending_destruction) + end + end + end end describe '.most_recent!' do @@ -154,15 +175,17 @@ RSpec.describe Packages::PackageFile, type: :model do let_it_be(:package_file3_2) { create(:package_file, :npm, package: package3) } let_it_be(:package_file3_3) { create(:package_file, :npm, package: package3) } + let_it_be(:package_file3_4) { create(:package_file, :npm, :pending_destruction, package: package3) } let_it_be(:package_file4_2) { create(:package_file, :npm, package: package2) } let_it_be(:package_file4_3) { create(:package_file, :npm, package: package2) } let_it_be(:package_file4_4) { create(:package_file, :npm, package: package2) } + let_it_be(:package_file4_4) { create(:package_file, :npm, :pending_destruction, package: package2) } - let(:most_recent_package_file1) { package1.package_files.recent.first } - let(:most_recent_package_file2) { package2.package_files.recent.first } - let(:most_recent_package_file3) { package3.package_files.recent.first } - let(:most_recent_package_file4) { package4.package_files.recent.first } + let(:most_recent_package_file1) { package1.installable_package_files.recent.first } + let(:most_recent_package_file2) { package2.installable_package_files.recent.first } + let(:most_recent_package_file3) { package3.installable_package_files.recent.first } + let(:most_recent_package_file4) { package4.installable_package_files.recent.first } subject { described_class.most_recent_for(packages) } @@ -202,6 +225,24 @@ RSpec.describe Packages::PackageFile, type: :model do it 'returns the most recent package for the selected channel' do expect(subject).to contain_exactly(helm_package_file2) end + + context 'with package files pending destruction' do + let_it_be(:package_file_pending_destruction) { create(:helm_package_file, :pending_destruction, package: helm_package, channel: 'alpha') } + + it 'does not return them' do + expect(subject).to contain_exactly(helm_package_file2) + end + + context 'with packages_installable_package_files disabled' do + before do + stub_feature_flags(packages_installable_package_files: false) + end + + it 'returns them' do + expect(subject).to contain_exactly(package_file_pending_destruction) + end + end + end end end @@ -314,4 +355,25 @@ RSpec.describe Packages::PackageFile, type: :model do end end end + + context 'status scopes' do + let_it_be(:package) { create(:package) } + let_it_be(:default_package_file) { create(:package_file, package: package) } + let_it_be(:pending_destruction_package_file) { create(:package_file, :pending_destruction, package: package) } + + describe '.installable' do + subject { package.installable_package_files } + + it 'does not include non-displayable packages', :aggregate_failures do + is_expected.to include(default_package_file) + is_expected.not_to include(pending_destruction_package_file) + end + end + + describe '.with_status' do + subject { described_class.with_status(:pending_destruction) } + + it { is_expected.to contain_exactly(pending_destruction_package_file) } + end + end end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 44ba6e0e2fd..122340f7bec 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -413,9 +413,17 @@ RSpec.describe Packages::Package, type: :model do it_behaves_like 'validating version to be SemVer compliant for', :terraform_module_package context 'nuget package' do - it_behaves_like 'validating version to be SemVer compliant for', :nuget_package + subject { build_stubbed(:nuget_package) } + it { is_expected.to allow_value('1.2').for(:version) } + it { is_expected.to allow_value('1.2.3').for(:version) } it { is_expected.to allow_value('1.2.3.4').for(:version) } + it { is_expected.to allow_value('1.2.3-beta').for(:version) } + it { is_expected.to allow_value('1.2.3-alpha.3').for(:version) } + it { is_expected.not_to allow_value('1').for(:version) } + it { is_expected.not_to allow_value('1./2.3').for(:version) } + it { is_expected.not_to allow_value('../../../../../1.2.3').for(:version) } + it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) } end end @@ -839,6 +847,7 @@ RSpec.describe Packages::Package, type: :model do end context 'status scopes' do + let_it_be(:default_package) { create(:maven_package, :default) } let_it_be(:hidden_package) { create(:maven_package, :hidden) } let_it_be(:processing_package) { create(:maven_package, :processing) } let_it_be(:error_package) { create(:maven_package, :error) } @@ -856,11 +865,15 @@ RSpec.describe Packages::Package, type: :model do describe '.installable' do subject { described_class.installable } - it 'does not include non-displayable packages', :aggregate_failures do + it 'does not include non-installable packages', :aggregate_failures do is_expected.not_to include(error_package) - is_expected.not_to include(hidden_package) is_expected.not_to include(processing_package) end + + it 'includes installable packages', :aggregate_failures do + is_expected.to include(default_package) + is_expected.to include(hidden_package) + end end describe '.with_status' do diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 8a5b1e73194..0735bf25690 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -336,129 +336,6 @@ RSpec.describe PagesDomain do end end - describe '#update_daemon' do - let_it_be(:project) { create(:project).tap(&:mark_pages_as_deployed) } - - context 'when usage is serverless' do - it 'does not call the UpdatePagesConfigurationService' do - expect(PagesUpdateConfigurationWorker).not_to receive(:perform_async) - - create(:pages_domain, usage: :serverless) - end - end - - it 'runs when the domain is created' do - domain = build(:pages_domain) - - expect(domain).to receive(:update_daemon) - - domain.save! - end - - it 'runs when the domain is destroyed' do - domain = create(:pages_domain) - - expect(domain).to receive(:update_daemon) - - domain.destroy! - end - - it "schedules a PagesUpdateConfigurationWorker" do - expect(PagesUpdateConfigurationWorker).to receive(:perform_async).with(project.id) - - create(:pages_domain, project: project) - end - - context "when the pages aren't deployed" do - let_it_be(:project) { create(:project).tap(&:mark_pages_as_not_deployed) } - - it "does not schedule a PagesUpdateConfigurationWorker" do - expect(PagesUpdateConfigurationWorker).not_to receive(:perform_async).with(project.id) - - create(:pages_domain, project: project) - end - end - - context 'configuration updates when attributes change' do - let_it_be(:project1) { create(:project) } - let_it_be(:project2) { create(:project) } - let_it_be(:domain) { create(:pages_domain) } - - where(:attribute, :old_value, :new_value, :update_expected) do - now = Time.current - future = now + 1.day - - :project | nil | :project1 | true - :project | :project1 | :project1 | false - :project | :project1 | :project2 | true - :project | :project1 | nil | true - - # domain can't be set to nil - :domain | 'a.com' | 'a.com' | false - :domain | 'a.com' | 'b.com' | true - - # verification_code can't be set to nil - :verification_code | 'foo' | 'foo' | false - :verification_code | 'foo' | 'bar' | false - - :verified_at | nil | now | false - :verified_at | now | now | false - :verified_at | now | future | false - :verified_at | now | nil | false - - :enabled_until | nil | now | true - :enabled_until | now | now | false - :enabled_until | now | future | false - :enabled_until | now | nil | true - end - - with_them do - it 'runs if a relevant attribute has changed' do - a = old_value.is_a?(Symbol) ? send(old_value) : old_value - b = new_value.is_a?(Symbol) ? send(new_value) : new_value - - domain.update!(attribute => a) - - if update_expected - expect(domain).to receive(:update_daemon) - else - expect(domain).not_to receive(:update_daemon) - end - - domain.update!(attribute => b) - end - end - - context 'TLS configuration' do - let_it_be(:domain_without_tls) { create(:pages_domain, :without_certificate, :without_key) } - let_it_be(:domain) { create(:pages_domain) } - - let(:cert1) { domain.certificate } - let(:cert2) { cert1 + ' ' } - let(:key1) { domain.key } - let(:key2) { key1 + ' ' } - - it 'updates when added' do - expect(domain_without_tls).to receive(:update_daemon) - - domain_without_tls.update!(key: key1, certificate: cert1) - end - - it 'updates when changed' do - expect(domain).to receive(:update_daemon) - - domain.update!(key: key2, certificate: cert2) - end - - it 'updates when removed' do - expect(domain).to receive(:update_daemon) - - domain.update!(key: nil, certificate: nil) - end - end - end - end - describe '#user_provided_key' do subject { domain.user_provided_key } diff --git a/spec/models/preloaders/environments/deployment_preloader_spec.rb b/spec/models/preloaders/environments/deployment_preloader_spec.rb new file mode 100644 index 00000000000..c1812d45628 --- /dev/null +++ b/spec/models/preloaders/environments/deployment_preloader_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::Environments::DeploymentPreloader do + let_it_be(:user) { create(:user) } + let_it_be(:project, reload: true) { create(:project, :repository) } + + let_it_be(:pipeline) { create(:ci_pipeline, user: user, project: project, sha: project.commit.sha) } + let_it_be(:ci_build_a) { create(:ci_build, user: user, project: project, pipeline: pipeline) } + let_it_be(:ci_build_b) { create(:ci_build, user: user, project: project, pipeline: pipeline) } + let_it_be(:ci_build_c) { create(:ci_build, user: user, project: project, pipeline: pipeline) } + + let_it_be(:environment_a) { create(:environment, project: project, state: :available) } + let_it_be(:environment_b) { create(:environment, project: project, state: :available) } + + before do + create(:deployment, :success, project: project, environment: environment_a, deployable: ci_build_a) + create(:deployment, :success, project: project, environment: environment_a, deployable: ci_build_b) + create(:deployment, :success, project: project, environment: environment_b, deployable: ci_build_c) + end + + def preload_association(association_name) + described_class.new(project.environments) + .execute_with_union(association_name, deployment_associations) + end + + def deployment_associations + { + user: [], + deployable: { + pipeline: { + manual_actions: [] + } + } + } + end + + it 'does not trigger N+1 queries' do + control = ActiveRecord::QueryRecorder.new { preload_association(:last_deployment) } + + ci_build_d = create(:ci_build, user: user, project: project, pipeline: pipeline) + create(:deployment, :success, project: project, environment: environment_b, deployable: ci_build_d) + + expect { preload_association(:last_deployment) }.not_to exceed_query_limit(control) + end + + it 'batch loads the dependent associations' do + preload_association(:last_deployment) + + expect do + project.environments.first.last_deployment.deployable.pipeline.manual_actions + end.not_to exceed_query_limit(0) + end + + # Example query scoped with IN clause for `last_deployment` association preload: + # SELECT DISTINCT ON (environment_id) deployments.* FROM "deployments" WHERE "deployments"."status" IN (1, 2, 3, 4, 6) AND "deployments"."environment_id" IN (35, 34, 33) ORDER BY environment_id, deployments.id DESC + it 'avoids scoping with IN clause during preload' do + control = ActiveRecord::QueryRecorder.new { preload_association(:last_deployment) } + + default_preload_query = control.occurrences_by_line_method.first[1][:occurrences].any? { |i| i.include?('"deployments"."environment_id" IN') } + + expect(default_preload_query).to be(false) + end +end diff --git a/spec/models/project_pages_metadatum_spec.rb b/spec/models/project_pages_metadatum_spec.rb index 31a533e0363..af2f9b94871 100644 --- a/spec/models/project_pages_metadatum_spec.rb +++ b/spec/models/project_pages_metadatum_spec.rb @@ -18,4 +18,15 @@ RSpec.describe ProjectPagesMetadatum do expect(described_class.only_on_legacy_storage).to eq([legacy_storage_project.pages_metadatum]) end end + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:model) do + artifacts_archive = create(:ci_job_artifact, :legacy_archive) + metadatum = artifacts_archive.project.pages_metadatum + metadatum.artifacts_archive = artifacts_archive + metadatum + end + + let!(:parent) { model.artifacts_archive } + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 4e38bf7d3e3..2fe50f8c48a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Project, factory_default: :keep do include GitHelpers include ExternalAuthorizationServiceHelpers include ReloadHelpers + include StubGitlabCalls using RSpec::Parameterized::TableSyntax let_it_be(:namespace) { create_default(:namespace).freeze } @@ -379,6 +380,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:namespace_id) } it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.not_to allow_value('colon:in:path').for(:path) } # This is to validate that a specially crafted name cannot bypass a pattern match. See !72555 it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_length_of(:path).is_at_most(255) } it { is_expected.to validate_length_of(:description).is_at_most(2000) } @@ -1298,7 +1300,7 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#default_owner' do + describe '#first_owner' do let_it_be(:owner) { create(:user) } let_it_be(:namespace) { create(:namespace, owner: owner) } @@ -1306,7 +1308,7 @@ RSpec.describe Project, factory_default: :keep do let(:project) { build(:project, namespace: namespace) } it 'is the namespace owner' do - expect(project.default_owner).to eq(owner) + expect(project.first_owner).to eq(owner) end end @@ -1315,9 +1317,9 @@ RSpec.describe Project, factory_default: :keep do let(:project) { build(:project, group: group, namespace: namespace) } it 'is the group owner' do - allow(group).to receive(:default_owner).and_return(Object.new) + allow(group).to receive(:first_owner).and_return(Object.new) - expect(project.default_owner).to eq(group.default_owner) + expect(project.first_owner).to eq(group.first_owner) end end end @@ -1358,51 +1360,51 @@ RSpec.describe Project, factory_default: :keep do project.reload.has_external_issue_tracker end - it 'is false when external issue tracker service is not active' do - create(:service, project: project, category: 'issue_tracker', active: false) + it 'is false when external issue tracker integration is not active' do + create(:integration, project: project, category: 'issue_tracker', active: false) is_expected.to eq(false) end - it 'is false when other service is active' do - create(:service, project: project, category: 'not_issue_tracker', active: true) + it 'is false when other integration is active' do + create(:integration, project: project, category: 'not_issue_tracker', active: true) is_expected.to eq(false) 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) + context 'when there is an active external issue tracker integration' do + let!(:integration) do + create(:integration, project: project, type: 'JiraService', category: 'issue_tracker', active: true) end specify { is_expected.to eq(true) } - it 'becomes false when external issue tracker service is destroyed' do + it 'becomes false when external issue tracker integration is destroyed' do expect do - Integration.find(service.id).delete + Integration.find(integration.id).delete end.to change { subject }.to(false) end - it 'becomes false when external issue tracker service becomes inactive' do + it 'becomes false when external issue tracker integration becomes inactive' do expect do - service.update_column(:active, false) + integration.update_column(:active, false) end.to change { subject }.to(false) end - 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) + context 'when there are two active external issue tracker integrations' do + let_it_be(:second_integration) do + create(:integration, project: project, type: 'CustomIssueTracker', category: 'issue_tracker', active: true) end - it 'does not become false when external issue tracker service is destroyed' do + it 'does not become false when external issue tracker integration is destroyed' do expect do - Integration.find(service.id).delete + Integration.find(integration.id).delete end.not_to change { subject } end - it 'does not become false when external issue tracker service becomes inactive' do + it 'does not become false when external issue tracker integration becomes inactive' do expect do - service.update_column(:active, false) + integration.update_column(:active, false) end.not_to change { subject } end end @@ -1454,13 +1456,13 @@ RSpec.describe Project, factory_default: :keep do specify { expect(has_external_wiki).to eq(true) } - it 'becomes false if the external wiki service is destroyed' do + it 'becomes false if the external wiki integration is destroyed' do expect do Integration.find(integration.id).delete end.to change { has_external_wiki }.to(false) end - it 'becomes false if the external wiki service becomes inactive' do + it 'becomes false if the external wiki integration becomes inactive' do expect do integration.update_column(:active, false) end.to change { has_external_wiki }.to(false) @@ -4580,11 +4582,25 @@ RSpec.describe Project, factory_default: :keep do include ProjectHelpers let_it_be(:group) { create(:group) } + let_it_be_with_reload(:project) { create(:project, namespace: group) } - let!(:project) { create(:project, project_level, namespace: group ) } let(:user) { create_user_from_membership(project, membership) } - context 'reporter level access' do + subject { described_class.filter_by_feature_visibility(feature, user) } + + shared_examples 'filter respects visibility' do + it 'respects visibility' do + enable_admin_mode!(user) if admin_mode + project.update!(visibility_level: Gitlab::VisibilityLevel.level_value(project_level.to_s)) + update_feature_access_level(project, feature_access_level) + + expected_objects = expected_count == 1 ? [project] : [] + + expect(subject).to eq(expected_objects) + end + end + + context 'with reporter level access' do let(:feature) { MergeRequest } where(:project_level, :feature_access_level, :membership, :admin_mode, :expected_count) do @@ -4592,20 +4608,11 @@ RSpec.describe Project, factory_default: :keep do end with_them do - it "respects visibility" do - enable_admin_mode!(user) if admin_mode - update_feature_access_level(project, feature_access_level) - - expected_objects = expected_count == 1 ? [project] : [] - - expect( - described_class.filter_by_feature_visibility(feature, user) - ).to eq(expected_objects) - end + it_behaves_like 'filter respects visibility' end end - context 'issues' do + context 'with feature issues' do let(:feature) { Issue } where(:project_level, :feature_access_level, :membership, :admin_mode, :expected_count) do @@ -4613,20 +4620,11 @@ RSpec.describe Project, factory_default: :keep do end with_them do - it "respects visibility" do - enable_admin_mode!(user) if admin_mode - update_feature_access_level(project, feature_access_level) - - expected_objects = expected_count == 1 ? [project] : [] - - expect( - described_class.filter_by_feature_visibility(feature, user) - ).to eq(expected_objects) - end + it_behaves_like 'filter respects visibility' end end - context 'wiki' do + context 'with feature wiki' do let(:feature) { :wiki } where(:project_level, :feature_access_level, :membership, :admin_mode, :expected_count) do @@ -4634,20 +4632,11 @@ RSpec.describe Project, factory_default: :keep do end with_them do - it "respects visibility" do - enable_admin_mode!(user) if admin_mode - update_feature_access_level(project, feature_access_level) - - expected_objects = expected_count == 1 ? [project] : [] - - expect( - described_class.filter_by_feature_visibility(feature, user) - ).to eq(expected_objects) - end + it_behaves_like 'filter respects visibility' end end - context 'code' do + context 'with feature code' do let(:feature) { :repository } where(:project_level, :feature_access_level, :membership, :admin_mode, :expected_count) do @@ -4655,16 +4644,7 @@ RSpec.describe Project, factory_default: :keep do end with_them do - it "respects visibility" do - enable_admin_mode!(user) if admin_mode - update_feature_access_level(project, feature_access_level) - - expected_objects = expected_count == 1 ? [project] : [] - - expect( - described_class.filter_by_feature_visibility(feature, user) - ).to eq(expected_objects) - end + it_behaves_like 'filter respects visibility' end end end @@ -6835,7 +6815,7 @@ RSpec.describe Project, factory_default: :keep do describe 'with integrations and chat names' do subject { create(:project) } - let(:integration) { create(:service, project: subject) } + let(:integration) { create(:integration, project: subject) } before do create_list(:chat_name, 5, integration: integration) @@ -7476,6 +7456,258 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#enforced_runner_token_expiration_interval and #effective_runner_token_expiration_interval' do + shared_examples 'no enforced expiration interval' do + it { expect(subject.enforced_runner_token_expiration_interval).to be_nil } + end + + shared_examples 'enforced expiration interval' do |enforced_interval:| + it { expect(subject.enforced_runner_token_expiration_interval).to eq(enforced_interval) } + end + + shared_examples 'no effective expiration interval' do + it { expect(subject.effective_runner_token_expiration_interval).to be_nil } + end + + shared_examples 'effective expiration interval' do |effective_interval:| + it { expect(subject.effective_runner_token_expiration_interval).to eq(effective_interval) } + end + + context 'when there is no interval' do + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a project interval' do + let_it_be(:project) { create(:project, runner_token_expiration_interval: 3.days.to_i) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + # runner_token_expiration_interval should not affect the expiration interval, only + # project_runner_token_expiration_interval should. + context 'when there is a site-wide enforced shared interval' do + before do + stub_application_setting(runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + # group_runner_token_expiration_interval should not affect the expiration interval, only + # project_runner_token_expiration_interval should. + context 'when there is a site-wide enforced group interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a site-wide enforced project interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 5.days + end + + # runner_token_expiration_interval should not affect the expiration interval, only + # project_runner_token_expiration_interval should. + context 'when there is a group-enforced group interval' do + let_it_be(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + # subgroup_runner_token_expiration_interval should not affect the expiration interval, only + # project_runner_token_expiration_interval should. + context 'when there is a group-enforced subgroup interval' do + let_it_be(:group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is an owner group-enforced project interval' do + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 4.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + context 'when there is a grandparent group-enforced interval' do + let_it_be(:grandparent_group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 3.days.to_i) } + let_it_be(:grandparent_group) { create(:group, namespace_settings: grandparent_group_settings) } + let_it_be(:parent_group_settings) { create(:namespace_settings) } + let_it_be(:parent_group) { create(:group, parent: grandparent_group, namespace_settings: parent_group_settings) } + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, parent: parent_group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when there is a parent group-enforced interval overridden by group-enforced interval' do + let_it_be(:parent_group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 5.days.to_i) } + let_it_be(:parent_group) { create(:group, namespace_settings: parent_group_settings) } + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, parent: parent_group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 4.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + context 'when site-wide enforced interval overrides project interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 3.days.to_i) + end + + let_it_be(:project) { create(:project, runner_token_expiration_interval: 4.days.to_i) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when project interval overrides site-wide enforced interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:project) { create(:project, runner_token_expiration_interval: 4.days.to_i) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + + it 'has human-readable expiration intervals' do + expect(subject.enforced_runner_token_expiration_interval_human_readable).to eq('5d') + expect(subject.effective_runner_token_expiration_interval_human_readable).to eq('4d') + end + end + + context 'when site-wide enforced interval overrides group-enforced interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 3.days.to_i) + end + + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when group-enforced interval overrides site-wide enforced interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 4.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + context 'when group-enforced interval overrides project interval' do + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 3.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group, runner_token_expiration_interval: 4.days.to_i) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when project interval overrides group-enforced interval' do + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 5.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group, runner_token_expiration_interval: 4.days.to_i) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + # Unrelated groups should not affect the expiration interval. + context 'when there is an enforced project interval in an unrelated group' do + let_it_be(:unrelated_group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:unrelated_group) { create(:group, namespace_settings: unrelated_group_settings) } + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + # Subgroups should not affect the parent group expiration interval. + context 'when there is an enforced project interval in a subgroup' do + let_it_be(:group) { create(:group) } + let_it_be(:subgroup_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:subgroup) { create(:group, parent: group, namespace_settings: subgroup_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + end + it_behaves_like 'it has loose foreign keys' do let(:factory_name) { :project } end @@ -7551,6 +7783,46 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#context_commits_enabled?' do + let_it_be(:project) { create(:project) } + + subject(:result) { project.context_commits_enabled? } + + context 'when context_commits feature flag is enabled' do + before do + stub_feature_flags(context_commits: true) + end + + it { is_expected.to be_truthy } + end + + context 'when context_commits feature flag is disabled' do + before do + stub_feature_flags(context_commits: false) + end + + it { is_expected.to be_falsey } + end + + context 'when context_commits feature flag is enabled on this project' do + before do + stub_feature_flags(context_commits: project) + end + + it { is_expected.to be_truthy } + end + + context 'when context_commits feature flag is enabled on another project' do + let(:another_project) { create(:project) } + + before do + stub_feature_flags(context_commits: another_project) + end + + it { is_expected.to be_falsey } + end + end + private def finish_job(export_job) diff --git a/spec/models/protectable_dropdown_spec.rb b/spec/models/protectable_dropdown_spec.rb index ab3f455fe63..a256f9e0ab1 100644 --- a/spec/models/protectable_dropdown_spec.rb +++ b/spec/models/protectable_dropdown_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' RSpec.describe ProtectableDropdown do + subject(:dropdown) { described_class.new(project, ref_type) } + let(:project) { create(:project, :repository) } - let(:subject) { described_class.new(project, :branches) } describe 'initialize' do it 'raises ArgumentError for invalid ref type' do @@ -13,34 +14,75 @@ RSpec.describe ProtectableDropdown do end end - describe '#protectable_ref_names' do + shared_examples 'protectable_ref_names' do context 'when project repository is not empty' do - before do - create(:protected_branch, project: project, name: 'master') - end - - it { expect(subject.protectable_ref_names).to include('feature') } - it { expect(subject.protectable_ref_names).not_to include('master') } + it 'includes elements matching a protected ref wildcard' do + is_expected.to include(matching_ref) - it "includes branches matching a protected branch wildcard" do - expect(subject.protectable_ref_names).to include('feature') + factory = ref_type == :branches ? :protected_branch : :protected_tag - create(:protected_branch, name: 'feat*', project: project) + create(factory, name: "#{matching_ref[0]}*", project: project) - subject = described_class.new(project.reload, :branches) + subject = described_class.new(project.reload, ref_type) - expect(subject.protectable_ref_names).to include('feature') + expect(subject.protectable_ref_names).to include(matching_ref) end end context 'when project repository is empty' do let(:project) { create(:project) } - it "returns empty list" do - subject = described_class.new(project, :branches) + it 'returns empty list' do + is_expected.to be_empty + end + end + end + + describe '#protectable_ref_names' do + subject { dropdown.protectable_ref_names } + + context 'for branches' do + let(:ref_type) { :branches } + let(:matching_ref) { 'feature' } - expect(subject.protectable_ref_names).to be_empty + before do + create(:protected_branch, project: project, name: 'master') end + + it { is_expected.to include(matching_ref) } + it { is_expected.not_to include('master') } + + it_behaves_like 'protectable_ref_names' + end + + context 'for tags' do + let(:ref_type) { :tags } + let(:matching_ref) { 'v1.0.0' } + + before do + create(:protected_tag, project: project, name: 'v1.1.0') + end + + it { is_expected.to include(matching_ref) } + it { is_expected.not_to include('v1.1.0') } + + it_behaves_like 'protectable_ref_names' + end + end + + describe '#hash' do + subject { dropdown.hash } + + context 'for branches' do + let(:ref_type) { :branches } + + it { is_expected.to include(id: 'feature', text: 'feature', title: 'feature') } + end + + context 'for tags' do + let(:ref_type) { :tags } + + it { is_expected.to include(id: 'v1.0.0', text: 'v1.0.0', title: 'v1.0.0') } end end end diff --git a/spec/models/ref_matcher_spec.rb b/spec/models/ref_matcher_spec.rb new file mode 100644 index 00000000000..47a6a8b986c --- /dev/null +++ b/spec/models/ref_matcher_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RefMatcher do + subject(:ref_matcher) { described_class.new(ref_pattern) } + + let(:ref_pattern) { 'v1.0' } + + shared_examples 'matching_refs' do + context 'when there is no match' do + let(:ref_pattern) { 'unknown' } + + it { is_expected.to match_array([]) } + end + + context 'when ref pattern is a wildcard' do + let(:ref_pattern) { 'v*' } + + it { is_expected.to match_array(refs) } + end + end + + describe '#matching' do + subject { ref_matcher.matching(refs) } + + context 'when refs are strings' do + let(:refs) { ['v1.0', 'v1.1'] } + + it { is_expected.to match_array([ref_pattern]) } + + it_behaves_like 'matching_refs' + end + + context 'when refs are ref objects' do + let(:matching_ref) { double('tag', name: 'v1.0') } + let(:not_matching_ref) { double('tag', name: 'v1.1') } + let(:refs) { [matching_ref, not_matching_ref] } + + it { is_expected.to match_array([matching_ref]) } + + it_behaves_like 'matching_refs' + end + end + + describe '#matches?' do + subject { ref_matcher.matches?(ref_name) } + + let(:ref_name) { 'v1.0' } + + it { is_expected.to be_truthy } + + context 'when ref_name is empty' do + let(:ref_name) { '' } + + it { is_expected.to be_falsey } + end + + context 'when ref pattern matches wildcard' do + let(:ref_pattern) { 'v*' } + + it { is_expected.to be_truthy } + end + + context 'when ref pattern does not match wildcard' do + let(:ref_pattern) { 'v2.*' } + + it { is_expected.to be_falsey } + end + end + + describe '#wildcard?' do + subject { ref_matcher.wildcard? } + + it { is_expected.to be_falsey } + + context 'when pattern is a wildcard' do + let(:ref_pattern) { 'v*' } + + it { is_expected.to be_truthy } + end + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 96cbdb468aa..e592a4964f5 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2398,17 +2398,6 @@ RSpec.describe Repository do it 'returns nil when tag does not exists' do expect(repository.find_tag('does-not-exist')).to be_nil end - - context 'when find_tag_via_gitaly is disabled' do - it 'fetches all tags' do - stub_feature_flags(find_tag_via_gitaly: false) - - expect(Gitlab::GitalyClient) - .to receive(:call).with(anything, :ref_service, :find_all_tags, anything, anything).and_call_original - - expect(repository.find_tag('v1.1.0').name).to eq('v1.1.0') - end - end end describe '#avatar' do diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index b2fa9c24535..0489a4fb995 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -8,6 +8,7 @@ RSpec.describe Route do describe 'relationships' do it { is_expected.to belong_to(:source) } + it { is_expected.to belong_to(:namespace) } end describe 'validations' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f8cea619233..ac2474ac393 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -83,6 +83,9 @@ RSpec.describe User do it { is_expected.to delegate_method(:registration_objective).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:registration_objective=).to(:user_detail).with_arguments(:args).allow_nil } + + it { is_expected.to delegate_method(:requires_credit_card_verification).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:requires_credit_card_verification=).to(:user_detail).with_arguments(:args).allow_nil } end describe 'associations' do @@ -436,7 +439,7 @@ RSpec.describe User do subject { build(:user) } end - it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :public_email, :notification_email do + it_behaves_like 'an object with email-formatted attributes', :public_email, :notification_email do subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } } end @@ -542,6 +545,13 @@ RSpec.describe User do expect(user).to be_invalid expect(user.errors.messages[:email].first).to eq(expected_error) end + + it 'does not allow user to update email to a non-allowlisted domain' do + user = create(:user, email: "info@test.example.com") + + expect { user.update!(email: "test@notexample.com") } + .to raise_error(StandardError, 'Validation failed: Email is not allowed. Check with your administrator.') + end end context 'when a signup domain is allowed and subdomains are not allowed' do @@ -608,6 +618,13 @@ RSpec.describe User do user = build(:user, email: 'info@example.com', created_by_id: 1) expect(user).to be_valid end + + it 'does not allow user to update email to a denied domain' do + user = create(:user, email: 'info@test.com') + + expect { user.update!(email: 'info@example.com') } + .to raise_error(StandardError, 'Validation failed: Email is not allowed. Check with your administrator.') + end end context 'when a signup domain is denied but a wildcard subdomain is allowed' do @@ -679,6 +696,13 @@ RSpec.describe User do expect(user.errors.messages[:email].first).to eq(expected_error) end + it 'does not allow user to update email to a restricted domain' do + user = create(:user, email: 'info@test.com') + + expect { user.update!(email: 'info@gitlab.com') } + .to raise_error(StandardError, 'Validation failed: Email is not allowed. Check with your administrator.') + end + it 'does accept a valid email address' do user = build(:user, email: 'info@test.com') @@ -1398,7 +1422,7 @@ RSpec.describe User do end describe '#update_tracked_fields!', :clean_gitlab_redis_shared_state do - let(:request) { OpenStruct.new(remote_ip: "127.0.0.1") } + let(:request) { double('request', remote_ip: "127.0.0.1") } let(:user) { create(:user) } it 'writes trackable attributes' do @@ -1481,27 +1505,176 @@ RSpec.describe User do end describe '#confirm' do + let(:expired_confirmation_sent_at) { Date.today - described_class.confirm_within - 7.days } + let(:extant_confirmation_sent_at) { Date.today } + before do allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) end - let(:user) { create(:user, :unconfirmed, unconfirmed_email: 'test@gitlab.com') } + let(:user) do + create(:user, :unconfirmed, unconfirmed_email: 'test@gitlab.com').tap do |user| + user.update!(confirmation_sent_at: confirmation_sent_at) + end + end - it 'returns unconfirmed' do - expect(user.confirmed?).to be_falsey + shared_examples_for 'unconfirmed user' do + it 'returns unconfirmed' do + expect(user.confirmed?).to be_falsey + end end - it 'confirms a user' do - user.confirm - expect(user.confirmed?).to be_truthy + context 'when the confirmation period has expired' do + let(:confirmation_sent_at) { expired_confirmation_sent_at } + + it_behaves_like 'unconfirmed user' + + it 'does not confirm the user' do + user.confirm + + expect(user.confirmed?).to be_falsey + end + + it 'does not add the confirmed primary email to emails' do + user.confirm + + expect(user.emails.confirmed.map(&:email)).not_to include(user.email) + end end - it 'adds the confirmed primary email to emails' do - expect(user.emails.confirmed.map(&:email)).not_to include(user.email) + context 'when the confirmation period has not expired' do + let(:confirmation_sent_at) { extant_confirmation_sent_at } - user.confirm + it_behaves_like 'unconfirmed user' - expect(user.emails.confirmed.map(&:email)).to include(user.email) + it 'confirms a user' do + user.confirm + expect(user.confirmed?).to be_truthy + end + + it 'adds the confirmed primary email to emails' do + expect(user.emails.confirmed.map(&:email)).not_to include(user.email) + + user.confirm + + expect(user.emails.confirmed.map(&:email)).to include(user.email) + end + + context 'when the primary email is already included in user.emails' do + let(:expired_confirmation_sent_at_for_email) { Date.today - Email.confirm_within - 7.days } + let(:extant_confirmation_sent_at_for_email) { Date.today } + + let!(:email) do + create(:email, email: user.unconfirmed_email, user: user).tap do |email| + email.update!(confirmation_sent_at: confirmation_sent_at_for_email) + end + end + + context 'when the confirmation period of the email record has expired' do + let(:confirmation_sent_at_for_email) { expired_confirmation_sent_at_for_email } + + it 'does not confirm the email record' do + user.confirm + + expect(email.reload.confirmed?).to be_falsey + end + end + + context 'when the confirmation period of the email record has not expired' do + let(:confirmation_sent_at_for_email) { extant_confirmation_sent_at_for_email } + + it 'confirms the email record' do + user.confirm + + expect(email.reload.confirmed?).to be_truthy + end + end + end + end + end + + describe '#force_confirm' do + let(:expired_confirmation_sent_at) { Date.today - described_class.confirm_within - 7.days } + let(:extant_confirmation_sent_at) { Date.today } + + let(:user) do + create(:user, :unconfirmed, unconfirmed_email: 'test@gitlab.com').tap do |user| + user.update!(confirmation_sent_at: confirmation_sent_at) + end + end + + shared_examples_for 'unconfirmed user' do + it 'returns unconfirmed' do + expect(user.confirmed?).to be_falsey + end + end + + shared_examples_for 'confirms the user on force_confirm' do + it 'confirms a user' do + user.force_confirm + expect(user.confirmed?).to be_truthy + end + end + + shared_examples_for 'adds the confirmed primary email to emails' do + it 'adds the confirmed primary email to emails' do + expect(user.emails.confirmed.map(&:email)).not_to include(user.email) + + user.force_confirm + + expect(user.emails.confirmed.map(&:email)).to include(user.email) + end + end + + shared_examples_for 'confirms the email record if the primary email was already present in user.emails' do + context 'when the primary email is already included in user.emails' do + let(:expired_confirmation_sent_at_for_email) { Date.today - Email.confirm_within - 7.days } + let(:extant_confirmation_sent_at_for_email) { Date.today } + + let!(:email) do + create(:email, email: user.unconfirmed_email, user: user).tap do |email| + email.update!(confirmation_sent_at: confirmation_sent_at_for_email) + end + end + + shared_examples_for 'confirms the email record' do + it 'confirms the email record' do + user.force_confirm + + expect(email.reload.confirmed?).to be_truthy + end + end + + context 'when the confirmation period of the email record has expired' do + let(:confirmation_sent_at_for_email) { expired_confirmation_sent_at_for_email } + + it_behaves_like 'confirms the email record' + end + + context 'when the confirmation period of the email record has not expired' do + let(:confirmation_sent_at_for_email) { extant_confirmation_sent_at_for_email } + + it_behaves_like 'confirms the email record' + end + end + end + + context 'when the confirmation period has expired' do + let(:confirmation_sent_at) { expired_confirmation_sent_at } + + it_behaves_like 'unconfirmed user' + it_behaves_like 'confirms the user on force_confirm' + it_behaves_like 'adds the confirmed primary email to emails' + it_behaves_like 'confirms the email record if the primary email was already present in user.emails' + end + + context 'when the confirmation period has not expired' do + let(:confirmation_sent_at) { extant_confirmation_sent_at } + + it_behaves_like 'unconfirmed user' + it_behaves_like 'confirms the user on force_confirm' + it_behaves_like 'adds the confirmed primary email to emails' + it_behaves_like 'confirms the email record if the primary email was already present in user.emails' end end @@ -1523,9 +1696,9 @@ RSpec.describe User do describe '#generate_password' do it 'does not generate password by default' do - user = create(:user, password: 'abcdefghe') + user = create(:user, password: Gitlab::Password.test_default) - expect(user.password).to eq('abcdefghe') + expect(user.password).to eq(Gitlab::Password.test_default) end end @@ -1624,6 +1797,29 @@ RSpec.describe User do expect(static_object_token).not_to be_blank expect(user.reload.static_object_token).to eq static_object_token end + + it 'generates an encrypted version of the token' do + user = create(:user, static_object_token: nil) + + expect(user[:static_object_token]).to be_nil + expect(user[:static_object_token_encrypted]).to be_nil + + user.static_object_token + + expect(user[:static_object_token]).to be_nil + expect(user[:static_object_token_encrypted]).to be_present + end + + it 'prefers an encoded version of the token' do + user = create(:user, static_object_token: nil) + + token = user.static_object_token + + user.update_column(:static_object_token, 'Test') + + expect(user.static_object_token).not_to eq('Test') + expect(user.static_object_token).to eq(token) + end end describe 'enabled_static_object_token' do @@ -1862,7 +2058,7 @@ RSpec.describe User do it { expect(user.authorized_groups).to eq([group]) } it { expect(user.owned_groups).to eq([group]) } it { expect(user.namespaces).to contain_exactly(user.namespace, group) } - it { expect(user.manageable_namespaces).to contain_exactly(user.namespace, group) } + it { expect(user.forkable_namespaces).to contain_exactly(user.namespace, group) } context 'with owned groups only' do before do @@ -1876,9 +2072,12 @@ RSpec.describe User do context 'with child groups' do let!(:subgroup) { create(:group, parent: group) } - describe '#manageable_namespaces' do - it 'includes all the namespaces the user can manage' do - expect(user.manageable_namespaces).to contain_exactly(user.namespace, group, subgroup) + describe '#forkable_namespaces' do + it 'includes all the namespaces the user can fork into' do + developer_group = create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) + developer_group.add_developer(user) + + expect(user.forkable_namespaces).to contain_exactly(user.namespace, group, subgroup, developer_group) end end @@ -2592,6 +2791,12 @@ RSpec.describe User do end end + describe '.user_search_minimum_char_limit' do + it 'returns true' do + expect(described_class.user_search_minimum_char_limit).to be(true) + end + end + describe '.find_by_ssh_key_id' do let_it_be(:user) { create(:user) } let_it_be(:key) { create(:key, user: user) } @@ -3768,7 +3973,7 @@ RSpec.describe User do end end - describe '#ci_owned_runners' do + shared_context '#ci_owned_runners' do let(:user) { create(:user) } shared_examples :nested_groups_owner do @@ -4075,6 +4280,16 @@ RSpec.describe User do end end + it_behaves_like '#ci_owned_runners' + + context 'when FF ci_owned_runners_cross_joins_fix is disabled' do + before do + stub_feature_flags(ci_owned_runners_cross_joins_fix: false) + end + + it_behaves_like '#ci_owned_runners' + end + describe '#projects_with_reporter_access_limited_to' do let(:project1) { create(:project) } let(:project2) { create(:project) } @@ -5606,6 +5821,48 @@ RSpec.describe User do end end + describe '#can_log_in_with_non_expired_password?' do + let(:user) { build(:user) } + + subject { user.can_log_in_with_non_expired_password? } + + context 'when user can log in' do + it 'returns true' do + is_expected.to be_truthy + end + + context 'when user with expired password' do + before do + user.password_expires_at = 2.minutes.ago + end + + it 'returns false' do + is_expected.to be_falsey + end + + context 'when password expiration is not applicable' do + context 'when ldap user' do + let(:user) { build(:omniauth_user, provider: 'ldap') } + + it 'returns true' do + is_expected.to be_truthy + end + end + end + end + end + + context 'when user cannot log in' do + context 'when user is blocked' do + let(:user) { build(:user, :blocked) } + + it 'returns false' do + is_expected.to be_falsey + end + end + end + end + describe '#read_only_attribute?' do context 'when synced attributes metadata is present' do it 'delegates to synced_attributes_metadata' do @@ -6303,13 +6560,43 @@ RSpec.describe User do specify { is_expected.to contain_exactly(developer_group2) } end - describe '.get_ids_by_username' do + describe '.get_ids_by_ids_or_usernames' do let(:user_name) { 'user_name' } let!(:user) { create(:user, username: user_name) } let(:user_id) { user.id } it 'returns the id of each record matching username' do - expect(described_class.get_ids_by_username([user_name])).to match_array([user_id]) + expect(described_class.get_ids_by_ids_or_usernames(nil, [user_name])).to match_array([user_id]) + end + + it 'returns the id of each record matching user id' do + expect(described_class.get_ids_by_ids_or_usernames([user_id], nil)).to match_array([user_id]) + end + + it 'return the id for all records matching either user id or user name' do + new_user_id = create(:user).id + + expect(described_class.get_ids_by_ids_or_usernames([new_user_id], [user_name])).to match_array([user_id, new_user_id]) + end + end + + describe '.by_ids_or_usernames' do + let(:user_name) { 'user_name' } + let!(:user) { create(:user, username: user_name) } + let(:user_id) { user.id } + + it 'returns matching records based on username' do + expect(described_class.by_ids_or_usernames(nil, [user_name])).to match_array([user]) + end + + it 'returns matching records based on id' do + expect(described_class.by_ids_or_usernames([user_id], nil)).to match_array([user]) + end + + it 'returns matching records based on both username and id' do + new_user = create(:user) + + expect(described_class.by_ids_or_usernames([new_user.id], [user_name])).to match_array([user, new_user]) end end diff --git a/spec/models/users_statistics_spec.rb b/spec/models/users_statistics_spec.rb index 8553d0bfdb0..add9bd18755 100644 --- a/spec/models/users_statistics_spec.rb +++ b/spec/models/users_statistics_spec.rb @@ -43,7 +43,7 @@ RSpec.describe UsersStatistics do create_list(:user, 2, :bot) create_list(:user, 1, :blocked) - allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false) + allow(described_class.connection).to receive(:transaction_open?).and_return(false) end context 'when successful' do diff --git a/spec/models/work_item/type_spec.rb b/spec/models/work_items/type_spec.rb index cc18558975b..6e9f3210e65 100644 --- a/spec/models/work_item/type_spec.rb +++ b/spec/models/work_items/type_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe WorkItem::Type do +RSpec.describe WorkItems::Type do describe 'modules' do it { is_expected.to include_module(CacheMarkdownField) } end @@ -12,6 +12,22 @@ RSpec.describe WorkItem::Type do it { is_expected.to belong_to(:namespace) } end + describe 'scopes' do + describe 'order_by_name_asc' do + subject { described_class.order_by_name_asc.pluck(:name) } + + before do + # Deletes all so we have control on the entire list of names + described_class.delete_all + create(:work_item_type, name: 'Ztype') + create(:work_item_type, name: 'atype') + create(:work_item_type, name: 'gtype') + end + + it { is_expected.to match(%w[atype gtype Ztype]) } + end + end + describe '#destroy' do let!(:work_item) { create :issue } @@ -19,10 +35,10 @@ RSpec.describe WorkItem::Type do it 'deletes type but not unrelated issues' do type = create(:work_item_type) - expect(WorkItem::Type.count).to eq(6) + expect(WorkItems::Type.count).to eq(6) expect { type.destroy! }.not_to change(Issue, :count) - expect(WorkItem::Type.count).to eq(5) + expect(WorkItems::Type.count).to eq(5) end end @@ -44,6 +60,22 @@ RSpec.describe WorkItem::Type do it { is_expected.not_to allow_value('s' * 256).for(:icon_name) } end + describe 'default?' do + subject { build(:work_item_type, namespace: namespace).default? } + + context 'when namespace is nil' do + let(:namespace) { nil } + + it { is_expected.to be_truthy } + end + + context 'when namespace is present' do + let(:namespace) { build(:namespace) } + + it { is_expected.to be_falsey } + end + end + describe '#name' do it 'strips name' do work_item_type = described_class.new(name: ' label😸 ') |