diff options
Diffstat (limited to 'spec/models')
67 files changed, 1279 insertions, 815 deletions
diff --git a/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb b/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb index 3e6d4ebd0a2..c0d5b9203b8 100644 --- a/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb +++ b/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb @@ -8,4 +8,6 @@ RSpec.describe Analytics::CycleAnalytics::IssueStageEvent do it { is_expected.to validate_presence_of(:group_id) } it { is_expected.to validate_presence_of(:project_id) } it { is_expected.to validate_presence_of(:start_event_timestamp) } + + it_behaves_like 'StageEventModel' end diff --git a/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb b/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb index 244c5c70286..82a7e66d62a 100644 --- a/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb +++ b/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb @@ -8,4 +8,6 @@ RSpec.describe Analytics::CycleAnalytics::MergeRequestStageEvent do it { is_expected.to validate_presence_of(:group_id) } it { is_expected.to validate_presence_of(:project_id) } it { is_expected.to validate_presence_of(:start_event_timestamp) } + + it_behaves_like 'StageEventModel' end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index efb92ddaea0..f0212da3041 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -194,7 +194,7 @@ RSpec.describe ApplicationRecord do end context 'with database load balancing' do - let(:session) { double(:session) } + let(:session) { Gitlab::Database::LoadBalancing::Session.new } before do allow(::Gitlab::Database::LoadBalancing::Session).to receive(:current).and_return(session) diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 3e264867703..8ad83da61f3 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -77,6 +77,9 @@ 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(: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(: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) } @@ -946,6 +949,10 @@ RSpec.describe ApplicationSetting do throttle_unauthenticated_files_api_period_in_seconds throttle_authenticated_files_api_requests_per_period throttle_authenticated_files_api_period_in_seconds + throttle_unauthenticated_deprecated_api_requests_per_period + throttle_unauthenticated_deprecated_api_period_in_seconds + throttle_authenticated_deprecated_api_requests_per_period + throttle_authenticated_deprecated_api_period_in_seconds throttle_authenticated_git_lfs_requests_per_period throttle_authenticated_git_lfs_period_in_seconds ] diff --git a/spec/models/bulk_import_spec.rb b/spec/models/bulk_import_spec.rb index 4cfec6b20b7..ea002a7b174 100644 --- a/spec/models/bulk_import_spec.rb +++ b/spec/models/bulk_import_spec.rb @@ -21,4 +21,18 @@ RSpec.describe BulkImport, type: :model do expect(described_class.all_human_statuses).to contain_exactly('created', 'started', 'finished', 'failed') end end + + describe '.min_gl_version_for_project' do + it { expect(described_class.min_gl_version_for_project_migration).to be_a(Gitlab::VersionInfo) } + it { expect(described_class.min_gl_version_for_project_migration.to_s).to eq('14.4.0') } + end + + describe '#source_version_info' do + it 'returns source_version as Gitlab::VersionInfo' do + bulk_import = build(:bulk_import, source_version: '9.13.2') + + expect(bulk_import.source_version_info).to be_a(Gitlab::VersionInfo) + expect(bulk_import.source_version_info.to_s).to eq(bulk_import.source_version) + end + end end diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index c1cbe61885f..278d7f4bc56 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -179,7 +179,7 @@ RSpec.describe BulkImports::Entity, type: :model do entity = create(:bulk_import_entity, :group_entity) entity.create_pipeline_trackers! - expect(entity.trackers.count).to eq(BulkImports::Groups::Stage.pipelines.count) + expect(entity.trackers.count).to eq(BulkImports::Groups::Stage.new(entity.bulk_import).pipelines.count) expect(entity.trackers.map(&:pipeline_name)).to include(BulkImports::Groups::Pipelines::GroupPipeline.to_s) end end @@ -189,7 +189,7 @@ RSpec.describe BulkImports::Entity, type: :model do entity = create(:bulk_import_entity, :project_entity) entity.create_pipeline_trackers! - expect(entity.trackers.count).to eq(BulkImports::Projects::Stage.pipelines.count) + expect(entity.trackers.count).to eq(BulkImports::Projects::Stage.new(entity.bulk_import).pipelines.count) expect(entity.trackers.map(&:pipeline_name)).to include(BulkImports::Projects::Pipelines::ProjectPipeline.to_s) end end @@ -207,4 +207,40 @@ RSpec.describe BulkImports::Entity, type: :model do expect(entity.pipeline_exists?('BulkImports::Groups::Pipelines::InexistentPipeline')).to eq(false) end end + + describe '#pluralized_name' do + context 'when entity is group' do + it 'returns groups' do + entity = build(:bulk_import_entity, :group_entity) + + expect(entity.pluralized_name).to eq('groups') + end + end + + context 'when entity is project' do + it 'returns projects' do + entity = build(:bulk_import_entity, :project_entity) + + expect(entity.pluralized_name).to eq('projects') + end + end + end + + describe '#export_relations_url_path' do + context 'when entity is group' do + it 'returns group export relations url' do + entity = build(:bulk_import_entity, :group_entity) + + expect(entity.export_relations_url_path).to eq("/groups/#{entity.encoded_source_full_path}/export_relations") + end + end + + context 'when entity is project' do + it 'returns project export relations url' do + entity = build(:bulk_import_entity, :project_entity) + + expect(entity.export_relations_url_path).to eq("/projects/#{entity.encoded_source_full_path}/export_relations") + end + end + end end diff --git a/spec/models/bulk_imports/file_transfer/group_config_spec.rb b/spec/models/bulk_imports/file_transfer/group_config_spec.rb index 1e566a7b042..8660114b719 100644 --- a/spec/models/bulk_imports/file_transfer/group_config_spec.rb +++ b/spec/models/bulk_imports/file_transfer/group_config_spec.rb @@ -23,10 +23,8 @@ RSpec.describe BulkImports::FileTransfer::GroupConfig do end describe '#export_path' do - it 'returns correct export path' do - expect(::Gitlab::ImportExport).to receive(:storage_path).and_return('storage_path') - - expect(subject.export_path).to eq("storage_path/#{exportable.full_path}/#{hex}") + it 'returns tmpdir location' do + expect(subject.export_path).to include(File.join(Dir.tmpdir, 'bulk_imports')) 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 db037528ec1..3bd79333f0c 100644 --- a/spec/models/bulk_imports/file_transfer/project_config_spec.rb +++ b/spec/models/bulk_imports/file_transfer/project_config_spec.rb @@ -23,10 +23,8 @@ RSpec.describe BulkImports::FileTransfer::ProjectConfig do end describe '#export_path' do - it 'returns correct export path' do - expect(::Gitlab::ImportExport).to receive(:storage_path).and_return('storage_path') - - expect(subject.export_path).to eq("storage_path/#{exportable.disk_path}/#{hex}") + it 'returns tmpdir location' do + expect(subject.export_path).to include(File.join(Dir.tmpdir, 'bulk_imports')) end end @@ -51,4 +49,46 @@ RSpec.describe BulkImports::FileTransfer::ProjectConfig do expect(subject.relation_excluded_keys('project')).to include('creator_id') end end + + describe '#tree_relation?' do + context 'when it is a tree relation' do + it 'returns true' do + expect(subject.tree_relation?('labels')).to eq(true) + end + end + + context 'when it is not a tree relation' do + it 'returns false' do + expect(subject.tree_relation?('example')).to eq(false) + end + end + end + + describe '#file_relation?' do + context 'when it is a file relation' do + it 'returns true' do + expect(subject.file_relation?('uploads')).to eq(true) + end + end + + context 'when it is not a file relation' do + it 'returns false' do + expect(subject.file_relation?('example')).to eq(false) + end + end + end + + describe '#tree_relation_definition_for' do + it 'returns relation definition' do + expected = { service_desk_setting: { except: [:outgoing_name, :file_template_project_id], include: [] } } + + expect(subject.tree_relation_definition_for('service_desk_setting')).to eq(expected) + end + + context 'when relation is not tree relation' do + it 'returns' do + expect(subject.tree_relation_definition_for('example')).to be_nil + end + end + end end diff --git a/spec/models/bulk_imports/tracker_spec.rb b/spec/models/bulk_imports/tracker_spec.rb index 7f0a7d4f1ae..a72b628e329 100644 --- a/spec/models/bulk_imports/tracker_spec.rb +++ b/spec/models/bulk_imports/tracker_spec.rb @@ -66,7 +66,8 @@ RSpec.describe BulkImports::Tracker, type: :model do describe '#pipeline_class' do it 'returns the pipeline class' do - pipeline_class = BulkImports::Groups::Stage.pipelines.first[1] + bulk_import = create(:bulk_import) + pipeline_class = BulkImports::Groups::Stage.new(bulk_import).pipelines.first[1] tracker = create(:bulk_import_tracker, pipeline_name: pipeline_class) expect(tracker.pipeline_class).to eq(pipeline_class) diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 6dd3c40f228..8f1ae9c5f02 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -17,6 +17,8 @@ RSpec.describe Ci::Bridge do { trigger: { project: 'my/project', branch: 'master' } } end + it { is_expected.to respond_to(:runner_features) } + it 'has many sourced pipelines' do expect(bridge).to have_many(:sourced_pipelines) end @@ -76,7 +78,7 @@ RSpec.describe Ci::Bridge do bridge.enqueue! - expect(::Ci::CreateCrossProjectPipelineWorker.jobs.last['args']).to eq([bridge.id]) + expect(::Ci::CreateDownstreamPipelineWorker.jobs.last['args']).to eq([bridge.id]) end end @@ -85,7 +87,7 @@ RSpec.describe Ci::Bridge do bridge.enqueue_waiting_for_resource! - expect(::Ci::CreateCrossProjectPipelineWorker.jobs.last['args']).to eq([bridge.id]) + expect(::Ci::CreateDownstreamPipelineWorker.jobs.last['args']).to match_array([bridge.id]) end it 'raises error when the status is failed' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 1e06d566c80..2ebf75a1d8a 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -29,11 +29,13 @@ RSpec.describe Ci::Build do it { is_expected.to have_one(:deployment) } it { is_expected.to have_one(:runner_session) } it { is_expected.to have_one(:trace_metadata) } + it { is_expected.to have_many(:terraform_state_versions).dependent(:nullify).inverse_of(:build) } it { is_expected.to validate_presence_of(:ref) } it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } + it { is_expected.to respond_to(:runner_features) } it { is_expected.to delegate_method(:merge_request?).to(:pipeline) } it { is_expected.to delegate_method(:merge_request_ref?).to(:pipeline) } @@ -345,10 +347,10 @@ RSpec.describe Ci::Build do end describe '#stick_build_if_status_changed' do - it 'sticks the build if the status changed', :db_load_balancing do + it 'sticks the build if the status changed' do job = create(:ci_build, :pending) - expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) + expect(ApplicationRecord.sticking).to receive(:stick) .with(:build, job.id) job.update!(status: :running) @@ -1288,7 +1290,7 @@ RSpec.describe Ci::Build do end end - describe 'state transition as a deployable' do + shared_examples_for 'state transition as a deployable' do subject { build.send(event) } let!(:build) { create(:ci_build, :with_deployment, :start_review_app, project: project, pipeline: pipeline) } @@ -1397,6 +1399,36 @@ RSpec.describe Ci::Build do end end + it_behaves_like 'state transition as a deployable' do + context 'when transits to running' do + let(:event) { :run! } + + context 'when deployment is already running state' do + before do + build.deployment.success! + end + + it 'does not change deployment status and tracks an error' do + expect(Gitlab::ErrorTracking) + .to receive(:track_exception).with( + instance_of(Deployment::StatusSyncError), deployment_id: deployment.id, build_id: build.id) + + with_cross_database_modification_prevented do + expect { subject }.not_to change { deployment.reload.status } + end + end + end + end + end + + context 'when update_deployment_after_transaction_commit feature flag is disabled' do + before do + stub_feature_flags(update_deployment_after_transaction_commit: false) + end + + it_behaves_like 'state transition as a deployable' + end + describe '#on_stop' do subject { build.on_stop } @@ -3946,7 +3978,7 @@ RSpec.describe Ci::Build do end it 'can drop the build' do - expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) + expect(Gitlab::ErrorTracking).to receive(:track_exception) expect { build.drop! }.not_to raise_error @@ -5288,4 +5320,10 @@ RSpec.describe Ci::Build do expect(build.reload.queuing_entry).not_to be_present end end + + it 'does not generate cross DB queries when a record is created via FactoryBot' do + with_cross_database_modification_prevented do + create(:ci_build) + end + end end diff --git a/spec/models/ci/build_trace_metadata_spec.rb b/spec/models/ci/build_trace_metadata_spec.rb index 5e4645c5dc4..120e4289da2 100644 --- a/spec/models/ci/build_trace_metadata_spec.rb +++ b/spec/models/ci/build_trace_metadata_spec.rb @@ -88,14 +88,16 @@ RSpec.describe Ci::BuildTraceMetadata do describe '#track_archival!' do let(:trace_artifact) { create(:ci_job_artifact) } let(:metadata) { create(:ci_build_trace_metadata) } + let(:checksum) { SecureRandom.hex } it 'stores the artifact id and timestamp' do expect(metadata.trace_artifact_id).to be_nil - metadata.track_archival!(trace_artifact.id) + metadata.track_archival!(trace_artifact.id, checksum) metadata.reload expect(metadata.trace_artifact_id).to eq(trace_artifact.id) + expect(metadata.checksum).to eq(checksum) expect(metadata.archived_at).to be_like_time(Time.current) end end @@ -131,4 +133,29 @@ RSpec.describe Ci::BuildTraceMetadata do end end end + + describe '#remote_checksum_valid?' do + using RSpec::Parameterized::TableSyntax + + let(:metadata) do + build(:ci_build_trace_metadata, + checksum: checksum, + remote_checksum: remote_checksum) + end + + subject { metadata.remote_checksum_valid? } + + where(:checksum, :remote_checksum, :result) do + nil | nil | false + nil | 'a' | false + 'a' | nil | false + 'a' | 'b' | false + 'b' | 'a' | false + 'a' | 'a' | true + end + + with_them do + it { is_expected.to eq(result) } + end + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 1007d64438f..98b55ccb76b 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -35,6 +35,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it { is_expected.to have_many(:sourced_pipelines) } it { is_expected.to have_many(:triggered_pipelines) } it { is_expected.to have_many(:pipeline_artifacts) } + it { is_expected.to have_many(:package_build_infos).dependent(:nullify).inverse_of(:pipeline) } + it { is_expected.to have_many(:package_file_build_infos).dependent(:nullify).inverse_of(:pipeline) } it { is_expected.to have_one(:chat_data) } it { is_expected.to have_one(:source_pipeline) } @@ -1219,32 +1221,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do %w(test success), %w(deploy running)]) end - - context 'when commit status is retried' do - let!(:old_commit_status) do - create(:commit_status, pipeline: pipeline, - stage: 'build', - name: 'mac', - stage_idx: 0, - status: 'success') - end - - context 'when FF ci_remove_update_retried_from_process_pipeline is disabled' do - before do - stub_feature_flags(ci_remove_update_retried_from_process_pipeline: false) - - Ci::ProcessPipelineService - .new(pipeline) - .execute - end - - it 'ignores the previous state' do - expect(statuses).to eq([%w(build success), - %w(test success), - %w(deploy running)]) - end - end - end end context 'when there is a stage with warnings' do @@ -2906,121 +2882,30 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end - describe '#execute_hooks' do + describe 'hooks trigerring' do let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } - let!(:build_a) { create_build('a', 0) } - let!(:build_b) { create_build('b', 0) } - - let!(:hook) do - create(:project_hook, pipeline_events: enabled) - end - - before do - WebHookWorker.drain - end - - context 'with pipeline hooks enabled' do - let(:enabled) { true } - - before do - stub_full_request(hook.url, method: :post) - end - - context 'with multiple builds', :sidekiq_inline do - context 'when build is queued' do - before do - build_a.reload.enqueue - build_b.reload.enqueue - end - - it 'receives a pending event once' do - expect(WebMock).to have_requested_pipeline_hook('pending').once - end - - it 'builds hook data once' do - create(:pipelines_email_integration) - - expect(Gitlab::DataBuilder::Pipeline).to receive(:build).once.and_call_original - - pipeline.execute_hooks - end - end - - context 'when build is run' do - before do - build_a.reload.enqueue - build_a.reload.run! - build_b.reload.enqueue - build_b.reload.run! - end - - it 'receives a running event once' do - expect(WebMock).to have_requested_pipeline_hook('running').once - end - end - - context 'when all builds succeed' do - before do - build_a.success - - # We have to reload build_b as this is in next stage and it gets triggered by PipelineProcessWorker - build_b.reload.success - end - - it 'receives a success event once' do - expect(WebMock).to have_requested_pipeline_hook('success').once - end - end + %i[ + enqueue + request_resource + prepare + run + skip + drop + succeed + cancel + block + delay + ].each do |action| + context "when pipeline action is #{action}" do + let(:pipeline_action) { action } - context 'when stage one failed' do - let!(:build_b) { create_build('b', 1) } - - before do - build_a.drop - end + it 'schedules a new PipelineHooksWorker job' do + expect(PipelineHooksWorker).to receive(:perform_async).with(pipeline.id) - it 'receives a failed event once' do - expect(WebMock).to have_requested_pipeline_hook('failed').once - end + pipeline.reload.public_send(pipeline_action) end - - def have_requested_pipeline_hook(status) - have_requested(:post, stubbed_hostname(hook.url)).with do |req| - json_body = Gitlab::Json.parse(req.body) - json_body['object_attributes']['status'] == status && - json_body['builds'].length == 2 - end - end - end - end - - context 'with pipeline hooks disabled' do - let(:enabled) { false } - - before do - build_a.enqueue - build_b.enqueue - end - - it 'did not execute pipeline_hook after touched' do - expect(WebMock).not_to have_requested(:post, hook.url) end - - it 'does not build hook data' do - expect(Gitlab::DataBuilder::Pipeline).not_to receive(:build) - - pipeline.execute_hooks - end - end - - def create_build(name, stage_idx) - create(:ci_build, - :created, - pipeline: pipeline, - name: name, - stage: "stage:#{stage_idx}", - stage_idx: stage_idx) end end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 0a43f785598..ac1a8247aaa 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -147,13 +147,20 @@ RSpec.describe Ci::Processable do end it 'releases a resource when build finished' do - expect(build.resource_group).to receive(:release_resource_from).with(build).and_call_original + expect(build.resource_group).to receive(:release_resource_from).with(build).and_return(true).and_call_original expect(Ci::ResourceGroups::AssignResourceFromResourceGroupWorker).to receive(:perform_async).with(build.resource_group_id) build.enqueue_waiting_for_resource! build.success! end + it 're-checks the resource group even if the processable does not retain a resource' do + expect(build.resource_group).to receive(:release_resource_from).with(build).and_return(false).and_call_original + expect(Ci::ResourceGroups::AssignResourceFromResourceGroupWorker).to receive(:perform_async).with(build.resource_group_id) + + build.success! + end + context 'when build has prerequisites' do before do allow(build).to receive(:any_unmet_prerequisites?) { true } diff --git a/spec/models/ci/resource_group_spec.rb b/spec/models/ci/resource_group_spec.rb index 50a786419f2..aae16157fbf 100644 --- a/spec/models/ci/resource_group_spec.rb +++ b/spec/models/ci/resource_group_spec.rb @@ -85,4 +85,61 @@ RSpec.describe Ci::ResourceGroup do end end end + + describe '#upcoming_processables' do + subject { resource_group.upcoming_processables } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:pipeline_1) { create(:ci_pipeline, project: project) } + let_it_be(:pipeline_2) { create(:ci_pipeline, project: project) } + + let!(:resource_group) { create(:ci_resource_group, process_mode: process_mode, project: project) } + + Ci::HasStatus::STATUSES_ENUM.keys.each do |status| + let!("build_1_#{status}") { create(:ci_build, pipeline: pipeline_1, status: status, resource_group: resource_group) } + let!("build_2_#{status}") { create(:ci_build, pipeline: pipeline_2, status: status, resource_group: resource_group) } + end + + context 'when process mode is unordered' do + let(:process_mode) { :unordered } + + it 'returns correct jobs in an indeterministic order' do + expect(subject).to contain_exactly(build_1_waiting_for_resource, build_2_waiting_for_resource) + end + end + + context 'when process mode is oldest_first' do + let(:process_mode) { :oldest_first } + + it 'returns correct jobs in a specific order' do + expect(subject[0]).to eq(build_1_waiting_for_resource) + expect(subject[1..2]).to contain_exactly(build_1_created, build_1_scheduled) + expect(subject[3]).to eq(build_2_waiting_for_resource) + expect(subject[4..5]).to contain_exactly(build_2_created, build_2_scheduled) + end + end + + context 'when process mode is newest_first' do + let(:process_mode) { :newest_first } + + it 'returns correct jobs in a specific order' do + expect(subject[0]).to eq(build_2_waiting_for_resource) + expect(subject[1..2]).to contain_exactly(build_2_created, build_2_scheduled) + expect(subject[3]).to eq(build_1_waiting_for_resource) + expect(subject[4..5]).to contain_exactly(build_1_created, build_1_scheduled) + end + end + + context 'when process mode is unknown' do + let(:process_mode) { :unordered } + + before do + resource_group.update_column(:process_mode, 3) + end + + it 'returns empty' do + is_expected.to be_empty + end + end + end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 31e854c852e..826332268c5 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -5,6 +5,20 @@ require 'spec_helper' RSpec.describe Ci::Runner do it_behaves_like 'having unique enum values' + describe 'groups association' do + # Due to other assoctions such as projects this whole spec is allowed to + # generate cross-database queries. So we have this temporary spec to + # validate that at least groups association does not generate cross-DB + # queries. + it 'does not create a cross-database query' do + runner = create(:ci_runner, :group) + + with_cross_joins_prevented do + expect(runner.groups.count).to eq(1) + end + end + end + describe 'validation' do it { is_expected.to validate_presence_of(:access_level) } it { is_expected.to validate_presence_of(:runner_type) } @@ -257,7 +271,7 @@ RSpec.describe Ci::Runner do expect(subject).to be_truthy expect(runner).to be_project_type - expect(runner.projects).to eq([project]) + expect(runner.runner_projects.pluck(:project_id)).to match_array([project.id]) expect(runner.only_for?(project)).to be_truthy end end @@ -383,10 +397,7 @@ RSpec.describe Ci::Runner do it 'sticks the runner to the primary and calls the original method' do runner = create(:ci_runner) - allow(Gitlab::Database::LoadBalancing).to receive(:enable?) - .and_return(true) - - expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) + expect(ApplicationRecord.sticking).to receive(:stick) .with(:runner, runner.id) expect(Gitlab::Workhorse).to receive(:set_key_and_notify) @@ -724,7 +735,7 @@ RSpec.describe Ci::Runner do context 'with invalid runner' do before do - runner.projects = [] + runner.runner_projects.delete_all end it 'still updates redis cache and database' do diff --git a/spec/models/clusters/agents/group_authorization_spec.rb b/spec/models/clusters/agents/group_authorization_spec.rb index 2a99fb26e3f..baeb8f5464e 100644 --- a/spec/models/clusters/agents/group_authorization_spec.rb +++ b/spec/models/clusters/agents/group_authorization_spec.rb @@ -7,4 +7,10 @@ RSpec.describe Clusters::Agents::GroupAuthorization do it { is_expected.to belong_to(:group).class_name('::Group').required } it { expect(described_class).to validate_jsonb_schema(['config']) } + + describe '#config_project' do + let(:record) { create(:agent_group_authorization) } + + it { expect(record.config_project).to eq(record.agent.project) } + end end diff --git a/spec/models/clusters/agents/implicit_authorization_spec.rb b/spec/models/clusters/agents/implicit_authorization_spec.rb index 69aa55a350e..2d6c3ddb426 100644 --- a/spec/models/clusters/agents/implicit_authorization_spec.rb +++ b/spec/models/clusters/agents/implicit_authorization_spec.rb @@ -9,6 +9,6 @@ RSpec.describe Clusters::Agents::ImplicitAuthorization do it { expect(subject.agent).to eq(agent) } it { expect(subject.agent_id).to eq(agent.id) } - it { expect(subject.project).to eq(agent.project) } + it { expect(subject.config_project).to eq(agent.project) } it { expect(subject.config).to be_nil } end diff --git a/spec/models/clusters/agents/project_authorization_spec.rb b/spec/models/clusters/agents/project_authorization_spec.rb index 134c70739ac..9ba259356c7 100644 --- a/spec/models/clusters/agents/project_authorization_spec.rb +++ b/spec/models/clusters/agents/project_authorization_spec.rb @@ -7,4 +7,10 @@ RSpec.describe Clusters::Agents::ProjectAuthorization do it { is_expected.to belong_to(:project).class_name('Project').required } it { expect(described_class).to validate_jsonb_schema(['config']) } + + describe '#config_project' do + let(:record) { create(:agent_project_authorization) } + + it { expect(record.config_project).to eq(record.agent.project) } + end end diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 43e2eab3b9d..788430d53d3 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -96,8 +96,9 @@ RSpec.describe Clusters::Applications::Runner do it 'creates a project runner' do subject + runner_projects = Project.where(id: runner.runner_projects.pluck(:project_id)) expect(runner).to be_project_type - expect(runner.projects).to eq [project] + expect(runner_projects).to match_array [project] end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 63fe6923630..ac0ae17f8f7 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -799,7 +799,7 @@ eos describe '#work_in_progress?' do [ 'squash! ', 'fixup! ', 'wip: ', 'WIP: ', '[WIP] ', - 'draft: ', 'Draft - ', '[Draft] ', '(draft) ', 'Draft: ' + 'draft: ', '[Draft] ', '(draft) ', 'Draft: ' ].each do |wip_prefix| it "detects the '#{wip_prefix}' prefix" do commit.message = "#{wip_prefix}#{commit.message}" @@ -814,22 +814,18 @@ eos expect(commit).to be_work_in_progress end - it "detects WIP for a commit just saying 'draft'" do + it "does not detect WIP for a commit just saying 'draft'" do commit.message = "draft" - expect(commit).to be_work_in_progress - end - - it "doesn't detect WIP for a commit that begins with 'FIXUP! '" do - commit.message = "FIXUP! #{commit.message}" - expect(commit).not_to be_work_in_progress end - it "doesn't detect WIP for words starting with WIP" do - commit.message = "Wipout #{commit.message}" + ["FIXUP!", "Draft - ", "Wipeout"].each do |draft_prefix| + it "doesn't detect '#{draft_prefix}' at the start of the title as a draft" do + commit.message = "#{draft_prefix} #{commit.message}" - expect(commit).not_to be_work_in_progress + expect(commit).not_to be_work_in_progress + end end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 7134a387e65..20afddd8470 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -123,6 +123,16 @@ RSpec.describe CommitStatus do end end + describe '.scheduled_at_before' do + let!(:never_scheduled) { create(:commit_status) } + let!(:stale_scheduled) { create(:commit_status, scheduled_at: 1.day.ago) } + let!(:fresh_scheduled) { create(:commit_status, scheduled_at: 1.minute.ago) } + + subject { CommitStatus.scheduled_at_before(1.hour.ago) } + + it { is_expected.to contain_exactly(stale_scheduled) } + end + describe '#processed' do subject { commit_status.processed } diff --git a/spec/models/concerns/bulk_insert_safe_spec.rb b/spec/models/concerns/bulk_insert_safe_spec.rb index 209ee1264d5..172986c142c 100644 --- a/spec/models/concerns/bulk_insert_safe_spec.rb +++ b/spec/models/concerns/bulk_insert_safe_spec.rb @@ -17,6 +17,7 @@ RSpec.describe BulkInsertSafe do t.binary :sha_value, null: false, limit: 20 t.jsonb :jsonb_value, null: false t.belongs_to :bulk_insert_parent_item, foreign_key: true, null: true + t.timestamps null: true t.index :name, unique: true end @@ -179,29 +180,26 @@ RSpec.describe BulkInsertSafe do end context 'with returns option set' do + let(:items) { bulk_insert_item_class.valid_list(1) } + + subject(:bulk_insert) { bulk_insert_item_class.bulk_insert!(items, returns: returns) } + context 'when is set to :ids' do - it 'return an array with the primary key values for all inserted records' do - items = bulk_insert_item_class.valid_list(1) + let(:returns) { :ids } - expect(bulk_insert_item_class.bulk_insert!(items, returns: :ids)).to contain_exactly(a_kind_of(Integer)) - end + it { is_expected.to contain_exactly(a_kind_of(Integer)) } end context 'when is set to nil' do - it 'returns an empty array' do - items = bulk_insert_item_class.valid_list(1) + let(:returns) { nil } - expect(bulk_insert_item_class.bulk_insert!(items, returns: nil)).to eq([]) - end + it { is_expected.to eq([]) } end - context 'when is set to anything else' do - it 'raises an error' do - items = bulk_insert_item_class.valid_list(1) + context 'when is set to a list of attributes' do + let(:returns) { [:id, :sha_value] } - expect { bulk_insert_item_class.bulk_insert!([items], returns: [:id, :name]) } - .to raise_error(ArgumentError, "returns needs to be :ids or nil") - end + it { is_expected.to contain_exactly([a_kind_of(Integer), '2fd4e1c67a2d28fced849ee1bb76e7391b93eb12']) } end end end @@ -228,10 +226,20 @@ RSpec.describe BulkInsertSafe do end describe '.bulk_upsert!' do + subject(:bulk_upsert) { bulk_insert_item_class.bulk_upsert!([new_object], unique_by: %w[name]) } + it 'updates existing object' do - bulk_insert_item_class.bulk_upsert!([new_object], unique_by: %w[name]) + expect { bulk_upsert }.to change { existing_object.reload.secret_value }.to('new value') + end - expect(existing_object.reload.secret_value).to eq('new value') + context 'when the `created_at` attribute is provided' do + before do + new_object.created_at = 10.days.from_now + end + + it 'does not change the existing `created_at` value' do + expect { bulk_upsert }.not_to change { existing_object.reload.created_at } + end end end end @@ -250,7 +258,7 @@ RSpec.describe BulkInsertSafe do it 'successfully inserts an item' do expect(ActiveRecord::InsertAll).to receive(:new) .with( - bulk_insert_items_with_composite_pk_class, [new_object.as_json], on_duplicate: :raise, returning: false, unique_by: %w[id name] + bulk_insert_items_with_composite_pk_class.insert_all_proxy_class, [new_object.as_json], on_duplicate: :raise, returning: false, unique_by: %w[id name] ).and_call_original expect { bulk_insert_items_with_composite_pk_class.bulk_insert!([new_object]) }.to( diff --git a/spec/models/concerns/checksummable_spec.rb b/spec/models/concerns/checksummable_spec.rb index 3a0387333e8..93a65605b50 100644 --- a/spec/models/concerns/checksummable_spec.rb +++ b/spec/models/concerns/checksummable_spec.rb @@ -13,11 +13,19 @@ RSpec.describe Checksummable do end end - describe ".hexdigest" do + describe ".sha256_hexdigest" do it 'returns the SHA256 sum of the file' do expected = Digest::SHA256.file(__FILE__).hexdigest - expect(subject.hexdigest(__FILE__)).to eq(expected) + expect(subject.sha256_hexdigest(__FILE__)).to eq(expected) + end + end + + describe ".md5_hexdigest" do + it 'returns the MD5 sum of the file' do + expected = Digest::MD5.file(__FILE__).hexdigest + + expect(subject.md5_hexdigest(__FILE__)).to eq(expected) end end end diff --git a/spec/models/concerns/ci/has_status_spec.rb b/spec/models/concerns/ci/has_status_spec.rb index 0709a050056..9dfc7d84f89 100644 --- a/spec/models/concerns/ci/has_status_spec.rb +++ b/spec/models/concerns/ci/has_status_spec.rb @@ -363,6 +363,18 @@ RSpec.describe Ci::HasStatus do it_behaves_like 'not containing the job', status end end + + describe '.waiting_for_resource_or_upcoming' do + subject { CommitStatus.waiting_for_resource_or_upcoming } + + %i[created scheduled waiting_for_resource].each do |status| + it_behaves_like 'containing the job', status + end + + %i[running failed success canceled].each do |status| + it_behaves_like 'not containing the job', status + end + end end describe '::DEFAULT_STATUS' do diff --git a/spec/models/concerns/vulnerability_finding_helpers_spec.rb b/spec/models/concerns/vulnerability_finding_helpers_spec.rb new file mode 100644 index 00000000000..023ecccb520 --- /dev/null +++ b/spec/models/concerns/vulnerability_finding_helpers_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe VulnerabilityFindingHelpers do + let(:cls) do + Class.new do + include VulnerabilityFindingHelpers + + attr_accessor :report_type + + def initialize(report_type) + @report_type = report_type + end + end + end + + describe '#requires_manual_resolution?' do + it 'returns false if the finding does not require manual resolution' do + expect(cls.new('sast').requires_manual_resolution?).to eq(false) + end + + it 'returns true when the finding requires manual resolution' do + expect(cls.new('secret_detection').requires_manual_resolution?).to eq(true) + end + end +end diff --git a/spec/models/customer_relations/contact_spec.rb b/spec/models/customer_relations/contact_spec.rb index b19554dd67e..298d5db3ab9 100644 --- a/spec/models/customer_relations/contact_spec.rb +++ b/spec/models/customer_relations/contact_spec.rb @@ -6,6 +6,7 @@ RSpec.describe CustomerRelations::Contact, type: :model do describe 'associations' do it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:organization).optional } + it { is_expected.to have_and_belong_to_many(:issues) } end describe 'validations' do diff --git a/spec/models/dependency_proxy/blob_spec.rb b/spec/models/dependency_proxy/blob_spec.rb index 3797f6184fe..3c54d3126a8 100644 --- a/spec/models/dependency_proxy/blob_spec.rb +++ b/spec/models/dependency_proxy/blob_spec.rb @@ -2,17 +2,16 @@ require 'spec_helper' RSpec.describe DependencyProxy::Blob, type: :model do + it_behaves_like 'ttl_expirable' + describe 'relationships' do it { is_expected.to belong_to(:group) } end - it_behaves_like 'having unique enum values' - describe 'validations' do it { is_expected.to validate_presence_of(:group) } it { is_expected.to validate_presence_of(:file) } it { is_expected.to validate_presence_of(:file_name) } - it { is_expected.to validate_presence_of(:status) } end describe '.total_size' do diff --git a/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb b/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb index 2906ea7b774..9f6358e1286 100644 --- a/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb +++ b/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb @@ -20,4 +20,13 @@ RSpec.describe DependencyProxy::ImageTtlGroupPolicy, type: :model do it { is_expected.to validate_numericality_of(:ttl).allow_nil.is_greater_than(0) } end end + + describe '.enabled' do + it 'returns policies that are enabled' do + enabled_policy = create(:image_ttl_group_policy) + create(:image_ttl_group_policy, :disabled) + + expect(described_class.enabled).to contain_exactly(enabled_policy) + end + end end diff --git a/spec/models/dependency_proxy/manifest_spec.rb b/spec/models/dependency_proxy/manifest_spec.rb index 2a085b3613b..e7f0889345a 100644 --- a/spec/models/dependency_proxy/manifest_spec.rb +++ b/spec/models/dependency_proxy/manifest_spec.rb @@ -2,18 +2,17 @@ require 'spec_helper' RSpec.describe DependencyProxy::Manifest, type: :model do + it_behaves_like 'ttl_expirable' + describe 'relationships' do it { is_expected.to belong_to(:group) } end - it_behaves_like 'having unique enum values' - describe 'validations' do it { is_expected.to validate_presence_of(:group) } it { is_expected.to validate_presence_of(:file) } it { is_expected.to validate_presence_of(:file_name) } it { is_expected.to validate_presence_of(:digest) } - it { is_expected.to validate_presence_of(:status) } end describe 'file is being stored' do diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index a0e5e9cbfe4..f9a05fbb06f 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -456,18 +456,6 @@ RSpec.describe Deployment do end end - describe 'with_deployable' do - subject { described_class.with_deployable } - - it 'retrieves deployments with deployable builds' do - with_deployable = create(:deployment) - create(:deployment, deployable: nil) - create(:deployment, deployable_type: 'CommitStatus', deployable_id: non_existing_record_id) - - is_expected.to contain_exactly(with_deployable) - end - end - describe 'visible' do subject { described_class.visible } @@ -613,6 +601,26 @@ RSpec.describe Deployment do end end + describe '.builds' do + let!(:deployment1) { create(:deployment) } + let!(:deployment2) { create(:deployment) } + let!(:deployment3) { create(:deployment) } + + subject { described_class.builds } + + it 'retrieves builds for the deployments' do + is_expected.to match_array( + [deployment1.deployable, deployment2.deployable, deployment3.deployable]) + end + + it 'does not fetch the null deployable_ids' do + deployment3.update!(deployable_id: nil, deployable_type: nil) + + is_expected.to match_array( + [deployment1.deployable, deployment2.deployable]) + end + end + describe '#previous_deployment' do using RSpec::Parameterized::TableSyntax @@ -757,7 +765,7 @@ RSpec.describe Deployment do expect(Deployments::LinkMergeRequestWorker).to receive(:perform_async) expect(Deployments::HooksWorker).to receive(:perform_async) - deploy.update_status('success') + expect(deploy.update_status('success')).to eq(true) end it 'updates finished_at when transitioning to a finished status' do @@ -767,6 +775,139 @@ RSpec.describe Deployment do expect(deploy.read_attribute(:finished_at)).to eq(Time.current) end end + + it 'tracks an exception if an invalid status transition is detected' do + expect(Gitlab::ErrorTracking) + .to receive(:track_exception) + .with(instance_of(described_class::StatusUpdateError), deployment_id: deploy.id) + + expect(deploy.update_status('running')).to eq(false) + end + + it 'tracks an exception if an invalid argument' do + expect(Gitlab::ErrorTracking) + .to receive(:track_exception) + .with(instance_of(described_class::StatusUpdateError), deployment_id: deploy.id) + + expect(deploy.update_status('created')).to eq(false) + end + end + + describe '#sync_status_with' do + subject { deployment.sync_status_with(ci_build) } + + let_it_be(:project) { create(:project, :repository) } + + let(:deployment) { create(:deployment, project: project, status: deployment_status) } + let(:ci_build) { create(:ci_build, project: project, status: build_status) } + + shared_examples_for 'synchronizing deployment' do + it 'changes deployment status' do + expect(Gitlab::ErrorTracking).not_to receive(:track_exception) + + is_expected.to eq(true) + + expect(deployment.status).to eq(build_status.to_s) + expect(deployment.errors).to be_empty + end + end + + shared_examples_for 'gracefully handling error' do + it 'tracks an exception' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).with( + instance_of(described_class::StatusSyncError), + deployment_id: deployment.id, + build_id: ci_build.id) + + is_expected.to eq(false) + + expect(deployment.status).to eq(deployment_status.to_s) + expect(deployment.errors.full_messages).to include(error_message) + end + end + + shared_examples_for 'ignoring build' do + it 'does not change deployment status' do + expect(Gitlab::ErrorTracking).not_to receive(:track_exception) + + is_expected.to eq(false) + + expect(deployment.status).to eq(deployment_status.to_s) + expect(deployment.errors).to be_empty + end + end + + context 'with created deployment' do + let(:deployment_status) { :created } + + context 'with running build' do + let(:build_status) { :running } + + it_behaves_like 'synchronizing deployment' + end + + context 'with finished build' do + let(:build_status) { :success } + + it_behaves_like 'synchronizing deployment' + end + + context 'with unrelated build' do + let(:build_status) { :waiting_for_resource } + + it_behaves_like 'ignoring build' + end + end + + context 'with running deployment' do + let(:deployment_status) { :running } + + context 'with running build' do + let(:build_status) { :running } + + it_behaves_like 'gracefully handling error' do + let(:error_message) { %Q{Status cannot transition via \"run\"} } + end + end + + context 'with finished build' do + let(:build_status) { :success } + + it_behaves_like 'synchronizing deployment' + end + + context 'with unrelated build' do + let(:build_status) { :waiting_for_resource } + + it_behaves_like 'ignoring build' + end + end + + context 'with finished deployment' do + let(:deployment_status) { :success } + + context 'with running build' do + let(:build_status) { :running } + + it_behaves_like 'gracefully handling error' do + let(:error_message) { %Q{Status cannot transition via \"run\"} } + end + end + + context 'with finished build' do + let(:build_status) { :success } + + it_behaves_like 'gracefully handling error' do + let(:error_message) { %Q{Status cannot transition via \"succeed\"} } + end + end + + context 'with unrelated build' do + let(:build_status) { :waiting_for_resource } + + it_behaves_like 'ignoring build' + end + end end describe '#valid_sha' do diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index e3e9d1f7a71..08c639957d3 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -801,38 +801,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do expect(query_count).to eq(0) end end - - context 'when the feature for disable_join is disabled' do - let(:pipeline) { create(:ci_pipeline, project: project) } - let(:ci_build) { create(:ci_build, project: project, pipeline: pipeline) } - - before do - stub_feature_flags(environment_last_visible_pipeline_disable_joins: false) - create(:deployment, :failed, project: project, environment: environment, deployable: ci_build) - end - - context 'for preload' do - it 'executes the original association instead of override' do - environment.reload - ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_deployable: []]) - - expect_any_instance_of(Deployment).not_to receive(:deployable) - - query_count = ActiveRecord::QueryRecorder.new do - expect(subject.id).to eq(ci_build.id) - end.count - - expect(query_count).to eq(0) - end - end - - context 'for direct call' do - it 'executes the original association instead of override' do - expect_any_instance_of(Deployment).not_to receive(:deployable) - expect(subject.id).to eq(ci_build.id) - end - end - end end describe '#last_visible_pipeline' do @@ -963,40 +931,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do expect(query_count).to eq(0) end end - - context 'when the feature for disable_join is disabled' do - let(:pipeline) { create(:ci_pipeline, project: project) } - let(:ci_build) { create(:ci_build, project: project, pipeline: pipeline) } - - before do - stub_feature_flags(environment_last_visible_pipeline_disable_joins: false) - create(:deployment, :failed, project: project, environment: environment, deployable: ci_build) - end - - subject { environment.last_visible_pipeline } - - context 'for preload' do - it 'executes the original association instead of override' do - environment.reload - ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_pipeline: []]) - - expect_any_instance_of(Ci::Build).not_to receive(:pipeline) - - query_count = ActiveRecord::QueryRecorder.new do - expect(subject.id).to eq(pipeline.id) - end.count - - expect(query_count).to eq(0) - end - end - - context 'for direct call' do - it 'executes the original association instead of override' do - expect_any_instance_of(Ci::Build).not_to receive(:pipeline) - expect(subject.id).to eq(pipeline.id) - end - end - end end describe '#upcoming_deployment' do diff --git a/spec/models/error_tracking/error_spec.rb b/spec/models/error_tracking/error_spec.rb index 5543392b624..9b8a81c6372 100644 --- a/spec/models/error_tracking/error_spec.rb +++ b/spec/models/error_tracking/error_spec.rb @@ -81,6 +81,13 @@ RSpec.describe ErrorTracking::Error, type: :model do end describe '#to_sentry_detailed_error' do - it { expect(error.to_sentry_detailed_error).to be_kind_of(Gitlab::ErrorTracking::DetailedError) } + let_it_be(:event) { create(:error_tracking_error_event, error: error) } + + subject { error.to_sentry_detailed_error } + + it { is_expected.to be_kind_of(Gitlab::ErrorTracking::DetailedError) } + it { expect(subject.integrated).to be_truthy } + it { expect(subject.first_release_version).to eq('db853d7') } + it { expect(subject.last_release_version).to eq('db853d7') } end end diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 29255e53fcf..d17541b4a6c 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -79,6 +79,46 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end + describe 'Callbacks' do + describe 'after_save :create_client_key!' do + subject { build(:project_error_tracking_setting, :integrated, project: project) } + + context 'no client key yet' do + it 'creates a new client key' do + expect { subject.save! }.to change { ErrorTracking::ClientKey.count }.by(1) + end + + context 'sentry backend' do + before do + subject.integrated = false + end + + it 'does not create a new client key' do + expect { subject.save! }.not_to change { ErrorTracking::ClientKey.count } + end + end + + context 'feature disabled' do + before do + subject.enabled = false + end + + it 'does not create a new client key' do + expect { subject.save! }.not_to change { ErrorTracking::ClientKey.count } + end + end + end + + context 'client key already exists' do + let!(:client_key) { create(:error_tracking_client_key, project: project) } + + it 'does not create a new client key' do + expect { subject.save! }.not_to change { ErrorTracking::ClientKey.count } + end + end + end + end + describe '.extract_sentry_external_url' do subject { described_class.extract_sentry_external_url(sentry_url) } @@ -494,4 +534,10 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do it { expect(subject.sentry_enabled).to eq(sentry_enabled) } end end + + describe '#gitlab_dsn' do + let!(:client_key) { create(:error_tracking_client_key, project: project) } + + it { expect(subject.gitlab_dsn).to eq(client_key.sentry_dsn) } + end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index e8aebe35302..e88abc21ef2 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -36,6 +36,7 @@ RSpec.describe Group do it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::GroupDistribution').dependent(:destroy) } it { is_expected.to have_many(:daily_build_group_report_results).class_name('Ci::DailyBuildGroupReportResult') } it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout').with_foreign_key(:group_id) } + it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') } describe '#members & #requesters' do let(:requester) { create(:user) } @@ -2369,7 +2370,7 @@ RSpec.describe Group do let_it_be(:project) { create(:project, group: group, shared_runners_enabled: true) } let_it_be(:project_2) { create(:project, group: sub_group_2, shared_runners_enabled: true) } - subject { group.update_shared_runners_setting!('disabled_and_unoverridable') } + subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_AND_UNOVERRIDABLE) } it 'disables shared Runners for all descendant groups and projects' do expect { subject_and_reload(group, sub_group, sub_group_2, project, project_2) } @@ -2395,7 +2396,7 @@ RSpec.describe Group do end context 'disabled_with_override' do - subject { group.update_shared_runners_setting!('disabled_with_override') } + subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_WITH_OVERRIDE) } context 'top level group' do let_it_be(:group) { create(:group, :shared_runners_disabled) } @@ -2607,17 +2608,29 @@ RSpec.describe Group do end describe '.ids_with_disabled_email' do - let!(:parent_1) { create(:group, emails_disabled: true) } - let!(:child_1) { create(:group, parent: parent_1) } + let_it_be(:parent_1) { create(:group, emails_disabled: true) } + let_it_be(:child_1) { create(:group, parent: parent_1) } - let!(:parent_2) { create(:group, emails_disabled: false) } - let!(:child_2) { create(:group, parent: parent_2) } + let_it_be(:parent_2) { create(:group, emails_disabled: false) } + let_it_be(:child_2) { create(:group, parent: parent_2) } - let!(:other_group) { create(:group, emails_disabled: false) } + let_it_be(:other_group) { create(:group, emails_disabled: false) } - subject(:group_ids_where_email_is_disabled) { described_class.ids_with_disabled_email([child_1, child_2, other_group]) } + shared_examples 'returns namespaces with disabled email' do + subject(:group_ids_where_email_is_disabled) { described_class.ids_with_disabled_email([child_1, child_2, other_group]) } - it { is_expected.to eq(Set.new([child_1.id])) } + it { is_expected.to eq(Set.new([child_1.id])) } + end + + it_behaves_like 'returns namespaces with disabled email' + + context 'when feature flag :linear_group_ancestor_scopes is disabled' do + before do + stub_feature_flags(linear_group_ancestor_scopes: false) + end + + it_behaves_like 'returns namespaces with disabled email' + end end describe '.timelogs' do diff --git a/spec/models/instance_configuration_spec.rb b/spec/models/instance_configuration_spec.rb index 551e6e7572c..cc0b69e3526 100644 --- a/spec/models/instance_configuration_spec.rb +++ b/spec/models/instance_configuration_spec.rb @@ -31,6 +31,23 @@ RSpec.describe InstanceConfiguration do expect(result.size).to eq(InstanceConfiguration::SSH_ALGORITHMS.size) end + it 'includes all algorithms' do + stub_pub_file(pub_file) + + result = subject.settings[:ssh_algorithms_hashes] + + expect(result.map { |a| a[:name] }).to match_array(%w(DSA ECDSA ED25519 RSA)) + end + + it 'does not include disabled algorithm' do + Gitlab::CurrentSettings.current_application_settings.update!(dsa_key_restriction: ApplicationSetting::FORBIDDEN_KEY_VALUE) + stub_pub_file(pub_file) + + result = subject.settings[:ssh_algorithms_hashes] + + expect(result.map { |a| a[:name] }).to match_array(%w(ECDSA ED25519 RSA)) + end + def pub_file(exist: true) path = exist ? 'spec/fixtures/ssh_host_example_key.pub' : 'spec/fixtures/ssh_host_example_key.pub.random' @@ -175,6 +192,9 @@ RSpec.describe InstanceConfiguration do throttle_authenticated_packages_api_enabled: true, throttle_authenticated_packages_api_requests_per_period: 1011, throttle_authenticated_packages_api_period_in_seconds: 1012, + throttle_authenticated_git_lfs_enabled: true, + throttle_authenticated_git_lfs_requests_per_period: 1022, + throttle_authenticated_git_lfs_period_in_seconds: 1023, issues_create_limit: 1013, notes_create_limit: 1014, project_export_limit: 1015, @@ -196,6 +216,7 @@ RSpec.describe InstanceConfiguration do expect(rate_limits[:protected_paths]).to eq({ enabled: true, requests_per_period: 1007, period_in_seconds: 1008 }) expect(rate_limits[:unauthenticated_packages_api]).to eq({ enabled: false, requests_per_period: 1009, period_in_seconds: 1010 }) expect(rate_limits[:authenticated_packages_api]).to eq({ enabled: true, requests_per_period: 1011, period_in_seconds: 1012 }) + expect(rate_limits[:authenticated_git_lfs_api]).to eq({ enabled: true, requests_per_period: 1022, period_in_seconds: 1023 }) expect(rate_limits[:issue_creation]).to eq({ enabled: true, requests_per_period: 1013, period_in_seconds: 60 }) expect(rate_limits[:note_creation]).to eq({ enabled: true, requests_per_period: 1014, period_in_seconds: 60 }) expect(rate_limits[:project_export]).to eq({ enabled: true, requests_per_period: 1015, period_in_seconds: 60 }) diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 8a06f7fac99..1a83d948fcf 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -14,7 +14,6 @@ RSpec.describe Integration do it { is_expected.to have_one(:service_hook).inverse_of(:integration).with_foreign_key(:service_id) } it { is_expected.to have_one(:issue_tracker_data).autosave(true).inverse_of(:integration).with_foreign_key(:service_id).class_name('Integrations::IssueTrackerData') } it { is_expected.to have_one(:jira_tracker_data).autosave(true).inverse_of(:integration).with_foreign_key(:service_id).class_name('Integrations::JiraTrackerData') } - it { is_expected.to have_one(:open_project_tracker_data).autosave(true).inverse_of(:integration).with_foreign_key(:service_id).class_name('Integrations::OpenProjectTrackerData') } end describe 'validations' do diff --git a/spec/models/integrations/open_project_spec.rb b/spec/models/integrations/open_project_spec.rb deleted file mode 100644 index 789911acae8..00000000000 --- a/spec/models/integrations/open_project_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Integrations::OpenProject do - describe 'Validations' do - context 'when integration is active' do - before do - subject.active = true - end - - it { is_expected.to validate_presence_of(:url) } - it { is_expected.to validate_presence_of(:token) } - it { is_expected.to validate_presence_of(:project_identifier_code) } - - it_behaves_like 'issue tracker integration URL attribute', :url - it_behaves_like 'issue tracker integration URL attribute', :api_url - end - - context 'when integration is inactive' do - before do - subject.active = false - end - - it { is_expected.not_to validate_presence_of(:url) } - it { is_expected.not_to validate_presence_of(:token) } - it { is_expected.not_to validate_presence_of(:project_identifier_code) } - end - end -end diff --git a/spec/models/integrations/open_project_tracker_data_spec.rb b/spec/models/integrations/open_project_tracker_data_spec.rb deleted file mode 100644 index 41c913f978c..00000000000 --- a/spec/models/integrations/open_project_tracker_data_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Integrations::OpenProjectTrackerData do - describe 'associations' do - it { is_expected.to belong_to(:integration) } - end - - describe 'closed_status_id' do - it 'returns the set value' do - expect(build(:open_project_tracker_data).closed_status_id).to eq('15') - end - - it 'returns the default value if not set' do - expect(build(:open_project_tracker_data, closed_status_id: nil).closed_status_id).to eq('13') - end - end -end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 1747972e8ae..4319407706e 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -34,6 +34,7 @@ RSpec.describe Issue do it { is_expected.to have_many(:issue_email_participants) } it { is_expected.to have_many(:timelogs).autosave(true) } it { is_expected.to have_one(:incident_management_issuable_escalation_status) } + it { is_expected.to have_and_belong_to_many(:customer_relations_contacts) } describe 'versions.most_recent' do it 'returns the most recent version' do @@ -222,17 +223,15 @@ RSpec.describe Issue do end end - describe '#order_by_position_and_priority' do + describe '#order_by_relative_position' do let(:project) { reusable_project } - let(:p1) { create(:label, title: 'P1', project: project, priority: 1) } - let(:p2) { create(:label, title: 'P2', project: project, priority: 2) } - let!(:issue1) { create(:labeled_issue, project: project, labels: [p1]) } - let!(:issue2) { create(:labeled_issue, project: project, labels: [p2]) } + let!(:issue1) { create(:issue, project: project) } + let!(:issue2) { create(:issue, project: project) } let!(:issue3) { create(:issue, project: project, relative_position: -200) } let!(:issue4) { create(:issue, project: project, relative_position: -100) } it 'returns ordered list' do - expect(project.issues.order_by_position_and_priority) + expect(project.issues.order_by_relative_position) .to match [issue3, issue4, issue1, issue2] end end @@ -1505,6 +1504,26 @@ RSpec.describe Issue do end end + describe '#supports_move_and_clone?' do + let_it_be(:project) { create(:project) } + let_it_be_with_refind(:issue) { create(:incident, project: project) } + + where(:issue_type, :supports_move_and_clone) do + :issue | true + :incident | true + end + + with_them do + before do + issue.update!(issue_type: issue_type) + end + + it do + expect(issue.supports_move_and_clone?).to eq(supports_move_and_clone) + end + end + end + describe '#email_participants_emails' do let_it_be(:issue) { create(:issue) } diff --git a/spec/models/loose_foreign_keys/deleted_record_spec.rb b/spec/models/loose_foreign_keys/deleted_record_spec.rb deleted file mode 100644 index db2f8b4d2d3..00000000000 --- a/spec/models/loose_foreign_keys/deleted_record_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe LooseForeignKeys::DeletedRecord do - let_it_be(:deleted_record_1) { described_class.create!(created_at: 1.day.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 5) } - let_it_be(:deleted_record_2) { described_class.create!(created_at: 3.days.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 1) } - let_it_be(:deleted_record_3) { described_class.create!(created_at: 5.days.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 3) } - let_it_be(:deleted_record_4) { described_class.create!(created_at: 10.days.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 1) } # duplicate - - # skip created_at because it gets truncated after insert - def map_attributes(records) - records.pluck(:deleted_table_name, :deleted_table_primary_key_value) - end - - describe 'partitioning strategy' do - it 'has retain_non_empty_partitions option' do - expect(described_class.partitioning_strategy.retain_non_empty_partitions).to eq(true) - end - end - - describe '.load_batch' do - it 'loads records and orders them by creation date' do - records = described_class.load_batch(4) - - expect(map_attributes(records)).to eq([['projects', 1], ['projects', 3], ['projects', 1], ['projects', 5]]) - end - - it 'supports configurable batch size' do - records = described_class.load_batch(2) - - expect(map_attributes(records)).to eq([['projects', 1], ['projects', 3]]) - end - end - - describe '.delete_records' do - it 'deletes exactly one record' do - described_class.delete_records([deleted_record_2]) - - expect(described_class.count).to eq(3) - expect(described_class.find_by(created_at: deleted_record_2.created_at)).to eq(nil) - end - - it 'deletes two records' do - described_class.delete_records([deleted_record_2, deleted_record_4]) - - expect(described_class.count).to eq(2) - end - - it 'deletes all records' do - described_class.delete_records([deleted_record_1, deleted_record_2, deleted_record_3, deleted_record_4]) - - expect(described_class.count).to eq(0) - end - end -end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 3f7f69ff34e..afe78adc547 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -7,11 +7,11 @@ RSpec.describe Member do using RSpec::Parameterized::TableSyntax - describe "Associations" do + describe 'Associations' do it { is_expected.to belong_to(:user) } end - describe "Validation" do + describe 'Validation' do subject { described_class.new(access_level: Member::GUEST) } it { is_expected.to validate_presence_of(:user) } @@ -28,7 +28,7 @@ RSpec.describe Member do subject { build(:project_member) } end - context "when an invite email is provided" do + context 'when an invite email is provided' do let_it_be(:project) { create(:project) } let(:member) { build(:project_member, source: project, invite_email: "user@example.com", user: nil) } @@ -37,34 +37,36 @@ RSpec.describe Member do expect(member).to be_valid end - it "requires a valid invite email" do + it 'requires a valid invite email' do member.invite_email = "nope" expect(member).not_to be_valid end - it "requires a unique invite email scoped to this source" do + it 'requires a unique invite email scoped to this source' do create(:project_member, source: member.source, invite_email: member.invite_email) expect(member).not_to be_valid end end - context "when an invite email is not provided" do + context 'when an invite email is not provided' do let(:member) { build(:project_member) } - it "requires a user" do + it 'requires a user' do member.user = nil expect(member).not_to be_valid end - it "is valid otherwise" do + it 'is valid otherwise' do expect(member).to be_valid end end context 'with admin signup restrictions' do + let(:expected_message) { _('is not allowed for this group. Check with your administrator.') } + context 'when allowed domains for signup is enabled' do before do stub_application_setting(domain_allowlist: ['example.com']) @@ -74,7 +76,7 @@ RSpec.describe Member do member = build(:group_member, :invited, invite_email: 'info@gitlab.com') expect(member).not_to be_valid - expect(member.errors.messages[:user].first).to eq(_('domain is not authorized for sign-up.')) + expect(member.errors.messages[:user].first).to eq(expected_message) end end @@ -88,7 +90,7 @@ RSpec.describe Member do member = build(:group_member, :invited, invite_email: 'denylist@example.org') expect(member).not_to be_valid - expect(member.errors.messages[:user].first).to eq(_('is not from an allowed domain.')) + expect(member.errors.messages[:user].first).to eq(expected_message) end end @@ -102,18 +104,18 @@ RSpec.describe Member do member = build(:group_member, :invited, invite_email: 'info@gitlab.com') expect(member).not_to be_valid - expect(member.errors.messages[:user].first).to eq(_('is not allowed. Try again with a different email address, or contact your GitLab admin.')) + expect(member.errors.messages[:user].first).to eq(expected_message) end end end - context "when a child member inherits its access level" do + context 'when a child member inherits its access level' do let(:user) { create(:user) } let(:member) { create(:group_member, :developer, user: user) } let(:child_group) { create(:group, parent: member.group) } let(:child_member) { build(:group_member, group: child_group, user: user) } - it "requires a higher level" do + it 'requires a higher level' do child_member.access_level = GroupMember::REPORTER child_member.validate @@ -123,7 +125,7 @@ RSpec.describe Member do # Membership in a subgroup confers certain access rights, such as being # able to merge or push code to protected branches. - it "is valid with an equal level" do + it 'is valid with an equal level' do child_member.access_level = GroupMember::DEVELOPER child_member.validate @@ -131,7 +133,7 @@ RSpec.describe Member do expect(child_member).to be_valid end - it "is valid with a higher level" do + it 'is valid with a higher level' do child_member.access_level = GroupMember::MAINTAINER child_member.validate @@ -167,6 +169,8 @@ RSpec.describe Member do describe 'Scopes & finders' do let_it_be(:project) { create(:project, :public) } let_it_be(:group) { create(:group) } + let_it_be(:blocked_pending_approval_user) { create(:user, :blocked_pending_approval ) } + let_it_be(:blocked_pending_approval_project_member) { create(:project_member, :invited, :developer, project: project, invite_email: blocked_pending_approval_user.email) } before_all do @owner_user = create(:user).tap { |u| group.add_owner(u) } @@ -536,9 +540,28 @@ RSpec.describe Member do it { is_expected.to eq [example_member] } end end + + describe '.with_invited_user_state' do + subject(:with_invited_user_state) { described_class.with_invited_user_state } + + it { is_expected.to include @owner } + it { is_expected.to include @maintainer } + it { is_expected.to include @invited_member } + it { is_expected.to include @accepted_invite_member } + it { is_expected.to include @requested_member } + it { is_expected.to include @accepted_request_member } + + context 'with invited pending members' do + it 'includes invited user state' do + invited_pending_members = with_invited_user_state.select { |m| m.invited_user_state.present? } + expect(invited_pending_members.count).to eq 1 + expect(invited_pending_members).to include blocked_pending_approval_project_member + end + end + end end - describe "Delegate methods" do + describe 'Delegate methods' do it { is_expected.to respond_to(:user_name) } it { is_expected.to respond_to(:user_email) } end @@ -608,29 +631,29 @@ RSpec.describe Member do end end - describe "#accept_invite!" do + describe '#accept_invite!' do let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } let(:user) { create(:user) } - it "resets the invite token" do + it 'resets the invite token' do member.accept_invite!(user) expect(member.invite_token).to be_nil end - it "sets the invite accepted timestamp" do + it 'sets the invite accepted timestamp' do member.accept_invite!(user) expect(member.invite_accepted_at).not_to be_nil end - it "sets the user" do + it 'sets the user' do member.accept_invite!(user) expect(member.user).to eq(user) end - it "calls #after_accept_invite" do + it 'calls #after_accept_invite' do expect(member).to receive(:after_accept_invite) member.accept_invite!(user) @@ -657,26 +680,26 @@ RSpec.describe Member do end end - describe "#decline_invite!" do + describe '#decline_invite!' do let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } - it "destroys the member" do + it 'destroys the member' do member.decline_invite! expect(member).to be_destroyed end - it "calls #after_decline_invite" do + it 'calls #after_decline_invite' do expect(member).to receive(:after_decline_invite) member.decline_invite! end end - describe "#generate_invite_token" do + describe '#generate_invite_token' do let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } - it "sets the invite token" do + it 'sets the invite token' do expect { member.generate_invite_token }.to change { member.invite_token } end end @@ -684,12 +707,12 @@ RSpec.describe Member do describe 'generate invite token on create' do let!(:member) { build(:project_member, invite_email: "user@example.com") } - it "sets the invite token" do + it 'sets the invite token' do expect { member.save! }.to change { member.invite_token }.to(kind_of(String)) end context 'when invite was already accepted' do - it "does not set invite token" do + it 'does not set invite token' do member.invite_accepted_at = 1.day.ago expect { member.save! }.not_to change { member.invite_token }.from(nil) @@ -744,7 +767,7 @@ RSpec.describe Member do end end - describe "#invite_to_unknown_user?" do + describe '#invite_to_unknown_user?' do subject { member.invite_to_unknown_user? } let(:member) { create(:project_member, invite_email: "user@example.com", invite_token: '1234', user: user) } @@ -762,7 +785,7 @@ RSpec.describe Member do end end - describe "destroying a record", :delete do + describe 'destroying a record', :delete, :sidekiq_inline do it "refreshes user's authorized projects" do project = create(:project, :private) user = create(:user) diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 1704d5adb96..ca846cf9e8e 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -244,11 +244,16 @@ RSpec.describe ProjectMember do project.add_user(user, Gitlab::Access::GUEST) end - it 'changes access level' do + it 'changes access level', :sidekiq_inline do expect { action }.to change { user.can?(:guest_access, project) }.from(true).to(false) end - it_behaves_like 'calls AuthorizedProjectUpdate::ProjectRecalculatePerUserService to recalculate authorizations' + it 'calls AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker to recalculate authorizations' do + expect(AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker).to receive(:perform_async).with(project.id, user.id) + + action + end + it_behaves_like 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' end @@ -298,7 +303,7 @@ RSpec.describe ProjectMember do project.add_user(user, Gitlab::Access::GUEST) end - it 'changes access level' do + it 'changes access level', :sidekiq_inline do expect { action }.to change { user.can?(:guest_access, project) }.from(true).to(false) end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 06ca88644b7..d871453e062 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1348,7 +1348,7 @@ RSpec.describe MergeRequest, factory_default: :keep do [ 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP: [WIP] WIP:', - 'draft:', 'Draft: ', '[Draft]', '[DRAFT] ', 'Draft - ' + 'draft:', 'Draft: ', '[Draft]', '[DRAFT] ' ].each do |wip_prefix| it "detects the '#{wip_prefix}' prefix" do subject.title = "#{wip_prefix}#{subject.title}" @@ -1357,16 +1357,27 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + [ + "WIP ", "(WIP)", + "draft", "Draft", "Draft -", "draft - ", "Draft ", "draft " + ].each do |draft_prefix| + it "doesn't detect '#{draft_prefix}' at the start of the title as a draft" do + subject.title = "#{draft_prefix}#{subject.title}" + + expect(subject.work_in_progress?).to eq false + end + end + it "detects merge request title just saying 'wip'" do subject.title = "wip" expect(subject.work_in_progress?).to eq true end - it "detects merge request title just saying 'draft'" do + it "does not detect merge request title just saying 'draft'" do subject.title = "draft" - expect(subject.work_in_progress?).to eq true + expect(subject.work_in_progress?).to eq false end it 'does not detect WIP in the middle of the title' do @@ -1428,7 +1439,7 @@ RSpec.describe MergeRequest, factory_default: :keep do [ 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', '[WIP] WIP: [WIP] WIP:', - 'draft:', 'Draft: ', '[Draft]', '[DRAFT] ', 'Draft - ' + 'draft:', 'Draft: ', '[Draft]', '[DRAFT] ' ].each do |wip_prefix| it "removes the '#{wip_prefix}' prefix" do wipless_title = subject.title @@ -3078,7 +3089,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - describe '#mergeable_state?' do + shared_examples 'for mergeable_state' do subject { create(:merge_request) } it 'checks if merge request can be merged' do @@ -3119,33 +3130,61 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when failed' do - context 'when #mergeable_ci_state? is false' do - before do - allow(subject).to receive(:mergeable_ci_state?) { false } - end + shared_examples 'failed skip_ci_check' do + context 'when #mergeable_ci_state? is false' do + before do + allow(subject).to receive(:mergeable_ci_state?) { false } + end - it 'returns false' do - expect(subject.mergeable_state?).to be_falsey + it 'returns false' do + expect(subject.mergeable_state?).to be_falsey + end + + it 'returns true when skipping ci check' do + expect(subject.mergeable_state?(skip_ci_check: true)).to be(true) + end end - it 'returns true when skipping ci check' do - expect(subject.mergeable_state?(skip_ci_check: true)).to be(true) + context 'when #mergeable_discussions_state? is false' do + before do + allow(subject).to receive(:mergeable_discussions_state?) { false } + end + + it 'returns false' do + expect(subject.mergeable_state?).to be_falsey + end + + it 'returns true when skipping discussions check' do + expect(subject.mergeable_state?(skip_discussions_check: true)).to be(true) + end end end - context 'when #mergeable_discussions_state? is false' do + context 'when improved_mergeability_checks is on' do + it_behaves_like 'failed skip_ci_check' + end + + context 'when improved_mergeability_checks is off' do before do - allow(subject).to receive(:mergeable_discussions_state?) { false } + stub_feature_flags(improved_mergeability_checks: false) end - it 'returns false' do - expect(subject.mergeable_state?).to be_falsey - end + it_behaves_like 'failed skip_ci_check' + end + end + end - it 'returns true when skipping discussions check' do - expect(subject.mergeable_state?(skip_discussions_check: true)).to be(true) - end + describe '#mergeable_state?' do + context 'when merge state caching is on' do + it_behaves_like 'for mergeable_state' + end + + context 'when merge state caching is off' do + before do + stub_feature_flags(mergeability_caching: false) end + + it_behaves_like 'for mergeable_state' end end diff --git a/spec/models/namespace/traversal_hierarchy_spec.rb b/spec/models/namespace/traversal_hierarchy_spec.rb index 2cd66f42458..d7b0ee888c0 100644 --- a/spec/models/namespace/traversal_hierarchy_spec.rb +++ b/spec/models/namespace/traversal_hierarchy_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Namespace::TraversalHierarchy, type: :model do - let_it_be(:root, reload: true) { create(:group, :with_hierarchy) } + let!(:root) { create(:group, :with_hierarchy) } describe '.for_namespace' do let(:hierarchy) { described_class.for_namespace(group) } @@ -62,7 +62,12 @@ RSpec.describe Namespace::TraversalHierarchy, type: :model do it { expect(hierarchy.incorrect_traversal_ids).to be_empty } - it_behaves_like 'hierarchy with traversal_ids' + it_behaves_like 'hierarchy with traversal_ids' do + before do + subject + end + end + it_behaves_like 'locked row' do let(:recorded_queries) { ActiveRecord::QueryRecorder.new } let(:row) { root } diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index c1cc8fc3e88..429727c2360 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -11,6 +11,8 @@ RSpec.describe NamespaceSetting, type: :model do it { is_expected.to belong_to(:namespace) } end + it { is_expected.to define_enum_for(:jobs_to_be_done).with_values([:basics, :move_repository, :code_storage, :exploring, :ci, :other]).with_suffix } + describe "validations" do describe "#default_branch_name_content" do let_it_be(:group) { create(:group) } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 51a26d82daa..c201d89947e 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Namespace do include GitHelpers include ReloadHelpers + let_it_be(:group_sti_name) { Group.sti_name } + let_it_be(:project_sti_name) { Namespaces::ProjectNamespace.sti_name } + let_it_be(:user_sti_name) { Namespaces::UserNamespace.sti_name } + let!(:namespace) { create(:namespace, :with_namespace_settings) } let(:gitlab_shell) { Gitlab::Shell.new } let(:repository_storage) { 'default' } @@ -38,20 +42,22 @@ RSpec.describe Namespace do context 'validating the parent of a namespace' do using RSpec::Parameterized::TableSyntax + # rubocop:disable Lint/BinaryOperatorWithIdenticalOperands where(:parent_type, :child_type, :error) do - nil | 'User' | nil - nil | 'Group' | nil - nil | 'Project' | 'must be set for a project namespace' - 'Project' | 'User' | 'project namespace cannot be the parent of another namespace' - 'Project' | 'Group' | 'project namespace cannot be the parent of another namespace' - 'Project' | 'Project' | 'project namespace cannot be the parent of another namespace' - 'Group' | 'User' | 'cannot not be used for user namespace' - 'Group' | 'Group' | nil - 'Group' | 'Project' | nil - 'User' | 'User' | 'cannot not be used for user namespace' - 'User' | 'Group' | 'user namespace cannot be the parent of another namespace' - 'User' | 'Project' | nil - end + nil | ref(:user_sti_name) | nil + nil | ref(:group_sti_name) | nil + nil | ref(:project_sti_name) | 'must be set for a project namespace' + ref(:project_sti_name) | ref(:user_sti_name) | 'project namespace cannot be the parent of another namespace' + ref(:project_sti_name) | ref(:group_sti_name) | 'project namespace cannot be the parent of another namespace' + ref(:project_sti_name) | ref(:project_sti_name) | 'project namespace cannot be the parent of another namespace' + ref(:group_sti_name) | ref(:user_sti_name) | 'cannot not be used for user namespace' + ref(:group_sti_name) | ref(:group_sti_name) | nil + ref(:group_sti_name) | ref(:project_sti_name) | nil + ref(:user_sti_name) | ref(:user_sti_name) | 'cannot not be used for user namespace' + ref(:user_sti_name) | ref(:group_sti_name) | 'user namespace cannot be the parent of another namespace' + ref(:user_sti_name) | ref(:project_sti_name) | nil + end + # rubocop:enable Lint/BinaryOperatorWithIdenticalOperands with_them do it 'validates namespace parent' do @@ -127,39 +133,77 @@ RSpec.describe Namespace do end context 'top-level group' do - let(:group) { build(:group, path: 'tree') } + let(:group) { build(:namespace, path: 'tree') } it { expect(group).to be_valid } end end - describe '1 char path length' do - it 'does not allow to create one' do - namespace = build(:namespace, path: 'j') + describe 'path validator' do + using RSpec::Parameterized::TableSyntax + + let_it_be(:parent) { create(:namespace) } - expect(namespace).not_to be_valid - expect(namespace.errors[:path].first).to eq('is too short (minimum is 2 characters)') + # rubocop:disable Lint/BinaryOperatorWithIdenticalOperands + where(:namespace_type, :path, :valid) do + ref(:project_sti_name) | 'j' | true + ref(:project_sti_name) | 'path.' | true + ref(:project_sti_name) | 'blob' | false + ref(:group_sti_name) | 'j' | false + ref(:group_sti_name) | 'path.' | false + ref(:group_sti_name) | 'blob' | true + ref(:user_sti_name) | 'j' | false + ref(:user_sti_name) | 'path.' | false + ref(:user_sti_name) | 'blob' | true end + # rubocop:enable Lint/BinaryOperatorWithIdenticalOperands - it 'does not allow to update one' do - namespace = create(:namespace) - namespace.update(path: 'j') + with_them do + it 'validates namespace path' do + parent_namespace = parent if namespace_type == Namespaces::ProjectNamespace.sti_name + namespace = build(:namespace, type: namespace_type, parent: parent_namespace, path: path) - expect(namespace).not_to be_valid - expect(namespace.errors[:path].first).to eq('is too short (minimum is 2 characters)') + expect(namespace.valid?).to be(valid) + end end + end - it 'allows updating other attributes for existing record' do - namespace = build(:namespace, path: 'j', owner: create(:user)) - namespace.save(validate: false) - namespace.reload + describe '1 char path length' do + context 'with user namespace' do + let(:namespace) { build(:namespace) } - expect(namespace.path).to eq('j') + it 'does not allow to update path to single char' do + namespace.save! - namespace.update(name: 'something new') + namespace.path = 'j' - expect(namespace).to be_valid - expect(namespace.name).to eq('something new') + expect(namespace).not_to be_valid + expect(namespace.errors[:path].first).to eq('is too short (minimum is 2 characters)') + end + + it 'allows updating other attributes for existing record' do + namespace.save! + namespace.update_attribute(:path, 'j') + namespace.reload + + expect(namespace.path).to eq('j') + + namespace.update(name: 'something new') + + expect(namespace).to be_valid + expect(namespace.name).to eq('something new') + end + end + + context 'with project namespace' do + let(:namespace) { build(:project_namespace) } + + it 'allows to update path to single char' do + namespace = create(:project_namespace) + namespace.update(path: 'j') + + expect(namespace).to be_valid + end end end end @@ -170,55 +214,53 @@ RSpec.describe Namespace do let(:namespace) { Namespace.find(create(:namespace, type: namespace_type, parent: parent).id) } context 'creating a Group' do - let(:namespace_type) { 'Group' } + let(:namespace_type) { group_sti_name } - it 'is valid' do + it 'is the correct type of namespace' do expect(namespace).to be_a(Group) expect(namespace.kind).to eq('group') - expect(namespace.group?).to be_truthy + expect(namespace.group_namespace?).to be_truthy end end context 'creating a ProjectNamespace' do - let(:namespace_type) { 'Project' } + let(:namespace_type) { project_sti_name } let(:parent) { create(:group) } - it 'is valid' do + it 'is the correct type of namespace' do expect(Namespace.find(namespace.id)).to be_a(Namespaces::ProjectNamespace) expect(namespace.kind).to eq('project') - expect(namespace.project?).to be_truthy + expect(namespace.project_namespace?).to be_truthy end end context 'creating a UserNamespace' do - let(:namespace_type) { 'User' } + let(:namespace_type) { user_sti_name } - it 'is valid' do - # TODO: We create a normal Namespace until - # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68894 is ready - expect(Namespace.find(namespace.id)).to be_a(Namespace) + it 'is the correct type of namespace' do + expect(Namespace.find(namespace.id)).to be_a(Namespaces::UserNamespace) expect(namespace.kind).to eq('user') - expect(namespace.user?).to be_truthy + expect(namespace.user_namespace?).to be_truthy end end context 'creating a default Namespace' do let(:namespace_type) { nil } - it 'is valid' do + it 'is the correct type of namespace' do expect(Namespace.find(namespace.id)).to be_a(Namespace) expect(namespace.kind).to eq('user') - expect(namespace.user?).to be_truthy + expect(namespace.user_namespace?).to be_truthy end end context 'creating an unknown Namespace type' do let(:namespace_type) { 'One' } - it 'defaults to a Namespace' do + it 'creates a default Namespace' do expect(Namespace.find(namespace.id)).to be_a(Namespace) expect(namespace.kind).to eq('user') - expect(namespace.user?).to be_truthy + expect(namespace.user_namespace?).to be_truthy end end end @@ -257,6 +299,15 @@ RSpec.describe Namespace do expect(described_class.sorted_by_similarity_and_parent_id_desc('Namespace')).to eq([namespace2, namespace1, namespace2sub, namespace1sub, namespace]) end end + + describe '.without_project_namespaces' do + let_it_be(:user_namespace) { create(:user_namespace) } + let_it_be(:project_namespace) { create(:project_namespace) } + + it 'excludes project namespaces' do + expect(described_class.without_project_namespaces).to match_array([namespace, namespace1, namespace2, namespace1sub, namespace2sub, user_namespace, project_namespace.parent]) + end + end end describe 'delegate' do @@ -428,9 +479,9 @@ RSpec.describe Namespace do end describe '.search' do - let_it_be(:first_group) { build(:group, name: 'my first namespace', path: 'old-path').tap(&:save!) } - let_it_be(:parent_group) { build(:group, name: 'my parent namespace', path: 'parent-path').tap(&:save!) } - let_it_be(:second_group) { build(:group, name: 'my second namespace', path: 'new-path', parent: parent_group).tap(&:save!) } + let_it_be(:first_group) { create(:group, name: 'my first namespace', path: 'old-path') } + let_it_be(:parent_group) { create(:group, name: 'my parent namespace', path: 'parent-path') } + let_it_be(:second_group) { create(:group, name: 'my second namespace', path: 'new-path', parent: parent_group) } let_it_be(:project_with_same_path) { create(:project, id: second_group.id, path: first_group.path) } it 'returns namespaces with a matching name' do @@ -1558,8 +1609,8 @@ RSpec.describe Namespace do end end - describe '#user?' do - subject { namespace.user? } + describe '#user_namespace?' do + subject { namespace.user_namespace? } context 'when type is a user' do let(:user) { create(:user) } @@ -1745,10 +1796,10 @@ RSpec.describe Namespace do using RSpec::Parameterized::TableSyntax where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :shared_runners_setting) do - true | true | 'enabled' - true | false | 'enabled' - false | true | 'disabled_with_override' - false | false | 'disabled_and_unoverridable' + true | true | Namespace::SR_ENABLED + true | false | Namespace::SR_ENABLED + false | true | Namespace::SR_DISABLED_WITH_OVERRIDE + false | false | Namespace::SR_DISABLED_AND_UNOVERRIDABLE end with_them do @@ -1764,15 +1815,15 @@ RSpec.describe Namespace do using RSpec::Parameterized::TableSyntax where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :other_setting, :result) do - true | true | 'enabled' | false - true | true | 'disabled_with_override' | true - true | true | 'disabled_and_unoverridable' | true - false | true | 'enabled' | false - false | true | 'disabled_with_override' | false - false | true | 'disabled_and_unoverridable' | true - false | false | 'enabled' | false - false | false | 'disabled_with_override' | false - false | false | 'disabled_and_unoverridable' | false + true | true | Namespace::SR_ENABLED | false + true | true | Namespace::SR_DISABLED_WITH_OVERRIDE | true + true | true | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | true + false | true | Namespace::SR_ENABLED | false + false | true | Namespace::SR_DISABLED_WITH_OVERRIDE | false + false | true | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | true + false | false | Namespace::SR_ENABLED | false + false | false | Namespace::SR_DISABLED_WITH_OVERRIDE | false + false | false | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | false end with_them do diff --git a/spec/models/namespaces/user_namespace_spec.rb b/spec/models/namespaces/user_namespace_spec.rb new file mode 100644 index 00000000000..7c00a597756 --- /dev/null +++ b/spec/models/namespaces/user_namespace_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# Main user namespace functionality it still in `Namespace`, so most +# of the specs are in `namespace_spec.rb`. +# UserNamespace specific specs will end up being migrated here. +RSpec.describe Namespaces::UserNamespace, type: :model do + describe 'validations' do + it { is_expected.to validate_presence_of(:owner) } + end +end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 5e3773513f1..0dd77967f25 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -108,6 +108,34 @@ RSpec.describe Note do end describe 'callbacks' do + describe '#keep_around_commit' do + let!(:noteable) { create(:issue) } + + it "calls #keep_around_commit normally" do + note = build(:note, project: noteable.project, noteable: noteable) + + expect(note).to receive(:keep_around_commit) + + note.save! + end + + it "skips #keep_around_commit if 'skip_keep_around_commits' is true" do + note = build(:note, project: noteable.project, noteable: noteable, skip_keep_around_commits: true) + + expect(note).not_to receive(:keep_around_commit) + + note.save! + end + + it "skips #keep_around_commit if 'importing' is true" do + note = build(:note, project: noteable.project, noteable: noteable, importing: true) + + expect(note).not_to receive(:keep_around_commit) + + note.save! + end + end + describe '#notify_after_create' do it 'calls #after_note_created on the noteable' do noteable = create(:issue) diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb index d689632e2b4..e709470b312 100644 --- a/spec/models/operations/feature_flag_spec.rb +++ b/spec/models/operations/feature_flag_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Operations::FeatureFlag do describe 'associations' do it { is_expected.to belong_to(:project) } - it { is_expected.to have_many(:scopes) } + it { is_expected.to have_many(:strategies) } end describe '.reference_pattern' do @@ -52,17 +52,6 @@ RSpec.describe Operations::FeatureFlag do it { is_expected.to define_enum_for(:version).with_values(new_version_flag: 2) } context 'a version 2 feature flag' do - it 'is invalid if associated with Operations::FeatureFlagScope models' do - project = create(:project) - feature_flag = described_class.new({ name: 'test', project: project, version: 2, - scopes_attributes: [{ environment_scope: '*', active: false }] }) - - expect(feature_flag.valid?).to eq(false) - expect(feature_flag.errors.messages).to eq({ - version_associations: ["version 2 feature flags may not have scopes"] - }) - end - it 'is valid if associated with Operations::FeatureFlags::Strategy models' do project = create(:project) feature_flag = described_class.create!({ name: 'test', project: project, version: 2, @@ -81,18 +70,6 @@ RSpec.describe Operations::FeatureFlag do end end - describe 'the default scope' do - let_it_be(:project) { create(:project) } - - context 'with a version 2 feature flag' do - it 'does not create a default scope' do - feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [], version: 2 }) - - expect(feature_flag.scopes).to eq([]) - end - end - end - describe '.enabled' do subject { described_class.enabled } @@ -187,26 +164,4 @@ RSpec.describe Operations::FeatureFlag do expect(subject.hook_attrs).to eq(hook_attrs) end end - - describe "#execute_hooks" do - let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } - let_it_be(:feature_flag) { create(:operations_feature_flag, project: project) } - - it 'does not execute the hook when feature_flag event is disabled' do - create(:project_hook, project: project, feature_flag_events: false) - expect(WebHookWorker).not_to receive(:perform_async) - - feature_flag.execute_hooks(user) - feature_flag.touch - end - - it 'executes hook when feature_flag event is enabled' do - hook = create(:project_hook, project: project, feature_flag_events: true) - expect(WebHookWorker).to receive(:perform_async).with(hook.id, an_instance_of(Hash), 'feature_flag_hooks') - - feature_flag.execute_hooks(user) - feature_flag.touch - end - end end diff --git a/spec/models/packages/helm/file_metadatum_spec.rb b/spec/models/packages/helm/file_metadatum_spec.rb index c7c17b157e4..995179b391d 100644 --- a/spec/models/packages/helm/file_metadatum_spec.rb +++ b/spec/models/packages/helm/file_metadatum_spec.rb @@ -31,8 +31,8 @@ RSpec.describe Packages::Helm::FileMetadatum, type: :model do it 'validates #channel', :aggregate_failures do is_expected.to validate_presence_of(:channel) - is_expected.to allow_value('a' * 63).for(:channel) - is_expected.not_to allow_value('a' * 64).for(:channel) + is_expected.to allow_value('a' * 255).for(:channel) + is_expected.not_to allow_value('a' * 256).for(:channel) is_expected.to allow_value('release').for(:channel) is_expected.to allow_value('my-repo').for(:channel) diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 99e5769fc1f..2573c01d686 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -1184,7 +1184,7 @@ RSpec.describe Packages::Package, type: :model do end context 'with an already existing build info' do - let_it_be(:build_info) { create(:packages_build_info, package: package, pipeline: pipeline) } + let_it_be(:build_info) { create(:package_build_info, package: package, pipeline: pipeline) } it 'does not create a build info' do expect { subject }.not_to change { ::Packages::BuildInfo.count } diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 7b997f0d4e1..2b6ed9a9927 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -94,7 +94,7 @@ RSpec.describe PagesDomain do with_them do it "is adds the expected errors" do - expect(pages_domain.errors.keys).to eq errors_on + expect(pages_domain.errors.attribute_names).to eq errors_on end end end @@ -155,7 +155,7 @@ RSpec.describe PagesDomain do it "adds error to certificate" do domain.valid? - expect(domain.errors.keys).to contain_exactly(:key, :certificate) + expect(domain.errors.attribute_names).to contain_exactly(:key, :certificate) end end @@ -165,7 +165,7 @@ RSpec.describe PagesDomain do domain.valid? - expect(domain.errors.keys).to contain_exactly(:key) + expect(domain.errors.attribute_names).to contain_exactly(:key) end end end @@ -287,6 +287,19 @@ RSpec.describe PagesDomain do it { is_expected.to be_truthy } end + + # The LetsEncrypt DST Root CA X3 expired on 2021-09-30, but the + # cross-sign in ISRG Root X1 enables it to function provided a chain + # of trust can be established with the system store. See: + # + # 1. https://community.letsencrypt.org/t/production-chain-changes/150739 + # 2. https://letsencrypt.org/2020/12/21/extending-android-compatibility.html + # 3. https://www.openssl.org/blog/blog/2021/09/13/LetsEncryptRootCertExpire/ + context 'with a LetsEncrypt bundle with an expired DST Root CA X3' do + let(:domain) { build(:pages_domain, :letsencrypt_expired_x3_root) } + + it { is_expected.to be_truthy } + end end describe '#expired?' do diff --git a/spec/models/preloaders/merge_requests_preloader_spec.rb b/spec/models/preloaders/merge_requests_preloader_spec.rb deleted file mode 100644 index 7108de2e491..00000000000 --- a/spec/models/preloaders/merge_requests_preloader_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Preloaders::MergeRequestsPreloader do - describe '#execute' do - let_it_be_with_refind(:merge_requests) { create_list(:merge_request, 3) } - let_it_be(:upvotes) { merge_requests.each { |m| create(:award_emoji, :upvote, awardable: m) } } - - it 'does not make n+1 queries' do - described_class.new(merge_requests).execute - - control = ActiveRecord::QueryRecorder.new(skip_cached: false) do - # expectations make sure the queries execute - merge_requests.each do |m| - expect(m.target_project.project_feature).not_to be_nil - expect(m.lazy_upvotes_count).to eq(1) - end - end - - # 1 query for BatchLoader to load all upvotes at once - expect(control.count).to eq(1) - end - - it 'runs extra queries without preloading' do - control = ActiveRecord::QueryRecorder.new(skip_cached: false) do - # expectations make sure the queries execute - merge_requests.each do |m| - expect(m.target_project.project_feature).not_to be_nil - expect(m.lazy_upvotes_count).to eq(1) - end - end - - # 4 queries per merge request = - # 1 to load merge request - # 1 to load project - # 1 to load project_feature - # 1 to load upvotes count - expect(control.count).to eq(4 * merge_requests.size) - end - end -end diff --git a/spec/models/product_analytics_event_spec.rb b/spec/models/product_analytics_event_spec.rb index 286729b8398..801e6dd5e10 100644 --- a/spec/models/product_analytics_event_spec.rb +++ b/spec/models/product_analytics_event_spec.rb @@ -36,17 +36,6 @@ RSpec.describe ProductAnalyticsEvent, type: :model do it { expect(described_class.count_by_graph('platform', 30.days)).to eq({ 'app' => 1, 'mobile' => 1, 'web' => 2 }) } end - describe '.by_category_and_action' do - let_it_be(:event) { create(:product_analytics_event, se_category: 'catA', se_action: 'actA') } - - before do - create(:product_analytics_event, se_category: 'catA', se_action: 'actB') - create(:product_analytics_event, se_category: 'catB', se_action: 'actA') - end - - it { expect(described_class.by_category_and_action('catA', 'actA')).to match_array([event]) } - end - describe '.count_collector_tstamp_by_day' do let_it_be(:time_now) { Time.zone.now } let_it_be(:time_ago) { Time.zone.now - 5.days } diff --git a/spec/models/project_feature_usage_spec.rb b/spec/models/project_feature_usage_spec.rb index 698c5374e88..3765a2b37a7 100644 --- a/spec/models/project_feature_usage_spec.rb +++ b/spec/models/project_feature_usage_spec.rb @@ -133,10 +133,8 @@ RSpec.describe ProjectFeatureUsage, type: :model do subject { project.feature_usage } - context 'database load balancing is configured', :db_load_balancing do + context 'database load balancing is configured' do before do - allow(ActiveRecord::Base).to receive(:connection).and_return(::Gitlab::Database::LoadBalancing.proxy) - ::Gitlab::Database::LoadBalancing::Session.clear_session end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3989ddc31e8..10220448936 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -140,6 +140,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:error_tracking_client_keys).class_name('ErrorTracking::ClientKey') } it { is_expected.to have_many(:pending_builds).class_name('Ci::PendingBuild') } it { is_expected.to have_many(:ci_feature_usages).class_name('Projects::CiFeatureUsage') } + it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') } # GitLab Pages it { is_expected.to have_many(:pages_domains) } @@ -494,23 +495,6 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#merge_requests_author_approval' do - where(:attribute_value, :return_value) do - true | true - false | false - nil | false - end - - with_them do - let(:project) { create(:project, merge_requests_author_approval: attribute_value) } - - it 'returns expected value' do - expect(project.merge_requests_author_approval).to eq(return_value) - expect(project.merge_requests_author_approval?).to eq(return_value) - end - end - end - describe '#all_pipelines' do let_it_be(:project) { create(:project) } @@ -3066,7 +3050,7 @@ RSpec.describe Project, factory_default: :keep do let(:project) { create(:project) } it 'marks the location with project ID' do - expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:mark_primary_write_location).with(:project, project.id) + expect(ApplicationRecord.sticking).to receive(:mark_primary_write_location).with(:project, project.id) project.mark_primary_write_location end @@ -6303,24 +6287,18 @@ RSpec.describe Project, factory_default: :keep do describe 'validation #changing_shared_runners_enabled_is_allowed' do where(:shared_runners_setting, :project_shared_runners_enabled, :valid_record) do - 'enabled' | true | true - 'enabled' | false | true - 'disabled_with_override' | true | true - 'disabled_with_override' | false | true - 'disabled_and_unoverridable' | true | false - 'disabled_and_unoverridable' | false | true + :shared_runners_enabled | true | true + :shared_runners_enabled | false | true + :disabled_with_override | true | true + :disabled_with_override | false | true + :disabled_and_unoverridable | true | false + :disabled_and_unoverridable | false | true end with_them do - let(:group) { create(:group) } + let(:group) { create(:group, shared_runners_setting) } let(:project) { build(:project, namespace: group, shared_runners_enabled: project_shared_runners_enabled) } - before do - allow_next_found_instance_of(Group) do |group| - allow(group).to receive(:shared_runners_setting).and_return(shared_runners_setting) - end - end - it 'validates the configuration' do expect(project.valid?).to eq(valid_record) @@ -7239,35 +7217,6 @@ RSpec.describe Project, factory_default: :keep do expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) end end - - context 'during ExtractProjectTopicsIntoSeparateTable migration' do - before do - topic_a = ActsAsTaggableOn::Tag.find_or_create_by!(name: 'topicA') - topic_b = ActsAsTaggableOn::Tag.find_or_create_by!(name: 'topicB') - - project.reload.topics_acts_as_taggable = [topic_a, topic_b] - project.save! - project.reload - end - - it 'topic_list returns correct string array' do - expect(project.topic_list).to eq(%w[topicA topicB topic1 topic2 topic3]) - end - - it 'topics returns correct topic records' do - expect(project.topics.map(&:class)).to eq([ActsAsTaggableOn::Tag, ActsAsTaggableOn::Tag, Projects::Topic, Projects::Topic, Projects::Topic]) - expect(project.topics.map(&:name)).to eq(%w[topicA topicB topic1 topic2 topic3]) - end - - it 'topic_list= sets new topics and removes old topics' do - project.topic_list = 'new-topic1, new-topic2' - project.save! - project.reload - - expect(project.topics.map(&:class)).to eq([Projects::Topic, Projects::Topic]) - expect(project.topics.map(&:name)).to eq(%w[new-topic1 new-topic2]) - end - end end shared_examples 'all_runners' do diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index ba769e830fd..ead6238b2f4 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -294,15 +294,17 @@ RSpec.describe ProjectStatistics do describe '#update_lfs_objects_size' do let!(:lfs_object1) { create(:lfs_object, size: 23.megabytes) } let!(:lfs_object2) { create(:lfs_object, size: 34.megabytes) } + let!(:lfs_object3) { create(:lfs_object, size: 34.megabytes) } let!(:lfs_objects_project1) { create(:lfs_objects_project, project: project, lfs_object: lfs_object1) } let!(:lfs_objects_project2) { create(:lfs_objects_project, project: project, lfs_object: lfs_object2) } + let!(:lfs_objects_project3) { create(:lfs_objects_project, project: project, lfs_object: lfs_object3) } before do statistics.update_lfs_objects_size end it "stores the size of related LFS objects" do - expect(statistics.lfs_objects_size).to eq 57.megabytes + expect(statistics.lfs_objects_size).to eq 91.megabytes end end diff --git a/spec/models/projects/topic_spec.rb b/spec/models/projects/topic_spec.rb index 409dc932709..397c65f4d5c 100644 --- a/spec/models/projects/topic_spec.rb +++ b/spec/models/projects/topic_spec.rb @@ -3,12 +3,18 @@ require 'spec_helper' RSpec.describe Projects::Topic do - let_it_be(:topic, reload: true) { create(:topic) } + let_it_be(:topic, reload: true) { create(:topic, name: 'topic') } subject { topic } it { expect(subject).to be_valid } + describe 'modules' do + subject { described_class } + + it { is_expected.to include_module(Avatarable) } + end + describe 'associations' do it { is_expected.to have_many(:project_topics) } it { is_expected.to have_many(:projects) } @@ -18,5 +24,76 @@ RSpec.describe Projects::Topic do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name) } it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to validate_length_of(:description).is_at_most(1024) } + end + + describe 'scopes' do + describe 'order_by_total_projects_count' do + let!(:topic1) { create(:topic, name: 'topicB') } + let!(:topic2) { create(:topic, name: 'topicC') } + let!(:topic3) { create(:topic, name: 'topicA') } + let!(:project1) { create(:project, topic_list: 'topicC, topicA, topicB') } + let!(:project2) { create(:project, topic_list: 'topicC, topicA') } + let!(:project3) { create(:project, topic_list: 'topicC') } + + it 'sorts topics by total_projects_count' do + topics = described_class.order_by_total_projects_count + + expect(topics.map(&:name)).to eq(%w[topicC topicA topicB topic]) + end + end + + describe 'reorder_by_similarity' do + let!(:topic1) { create(:topic, name: 'my-topic') } + let!(:topic2) { create(:topic, name: 'other') } + let!(:topic3) { create(:topic, name: 'topic2') } + + it 'sorts topics by similarity' do + topics = described_class.reorder_by_similarity('topic') + + expect(topics.map(&:name)).to eq(%w[topic my-topic topic2 other]) + end + end + end + + describe '#search' do + it 'returns topics with a matching name' do + expect(described_class.search(topic.name)).to eq([topic]) + end + + it 'returns topics with a partially matching name' do + expect(described_class.search(topic.name[0..2])).to eq([topic]) + end + + it 'returns topics with a matching name regardless of the casing' do + expect(described_class.search(topic.name.upcase)).to eq([topic]) + end + end + + describe '#avatar_type' do + it "is true if avatar is image" do + topic.update_attribute(:avatar, 'uploads/avatar.png') + expect(topic.avatar_type).to be_truthy + end + + it "is false if avatar is html page" do + topic.update_attribute(:avatar, 'uploads/avatar.html') + topic.avatar_type + + expect(topic.errors.added?(:avatar, "file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico, webp")).to be true + end + end + + describe '#avatar_url' do + context 'when avatar file is uploaded' do + before do + topic.update!(avatar: fixture_file_upload("spec/fixtures/dk.png")) + end + + it 'shows correct avatar url' do + expect(topic.avatar_url).to eq(topic.avatar.url) + expect(topic.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, topic.avatar.url].join) + end + end end end diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index 019c01af672..587a9683a8e 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -308,4 +308,15 @@ RSpec.describe ProtectedBranch do expect(described_class.by_name('')).to be_empty end end + + describe '.get_ids_by_name' do + let(:branch_name) { 'branch_name' } + let!(:protected_branch) { create(:protected_branch, name: branch_name) } + let(:branch_id) { protected_branch.id } + + it 'returns the id for each protected branch matching name' do + expect(described_class.get_ids_by_name([branch_name])) + .to match_array([branch_id]) + end + end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index dc55214c1dd..7bad907cf90 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -71,7 +71,7 @@ RSpec.describe Repository do let(:feature_flag) { true } before do - stub_feature_flags(gitaly_tags_finder: feature_flag) + stub_feature_flags(tags_finder_gitaly: feature_flag) end context 'name_desc' do diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb index 40a28b9e0cc..e8a933d2277 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/spec/models/snippet_repository_spec.rb @@ -115,6 +115,7 @@ RSpec.describe SnippetRepository do allow(snippet).to receive(:repository).and_return(repo) allow(repo).to receive(:ls_files).and_return([]) allow(repo).to receive(:root_ref).and_return('master') + allow(repo).to receive(:empty?).and_return(false) end it 'infers the commit action based on the parameters if not present' do diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 6bac5e31435..0ac684cd04c 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -242,4 +242,28 @@ RSpec.describe Upload do it { expect(subject.uploader_context).to match(a_hash_including(secret: 'secret', identifier: 'file.txt')) } end + + describe '#update_project_statistics' do + let_it_be(:project) { create(:project) } + + subject do + create(:upload, model: project) + end + + it 'updates project statistics when upload is added' do + expect(ProjectCacheWorker).to receive(:perform_async) + .with(project.id, [], [:uploads_size]) + + subject.save! + end + + it 'updates project statistics when upload is removed' do + subject.save! + + expect(ProjectCacheWorker).to receive(:perform_async) + .with(project.id, [], [:uploads_size]) + + subject.destroy! + end + end end diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index ba7ea3f7ce2..9189b9a1469 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe UserDetail do it { is_expected.to belong_to(:user) } + it { is_expected.to define_enum_for(:registration_objective).with_values([:basics, :move_repository, :code_storage, :exploring, :ci, :other, :joining_team]).with_suffix } describe 'validations' do describe '#job_title' do diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index 5806f123871..d4491aacd9f 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -80,12 +80,6 @@ RSpec.describe UserPreference do end end - describe '#timezone' do - it 'returns server time as default' do - expect(user_preference.timezone).to eq(Time.zone.tzinfo.name) - end - end - describe '#tab_width' do it 'is set to 8 by default' do # Intentionally not using factory here to test the constructor. diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ca4c38d4663..db805a804c8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -79,6 +79,9 @@ RSpec.describe User do it { is_expected.to delegate_method(:bio).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:bio=).to(:user_detail).with_arguments(:args).allow_nil } + + 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 } end describe 'associations' do @@ -123,7 +126,7 @@ RSpec.describe User do it { is_expected.to have_many(:callouts).class_name('UserCallout') } it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout') } - describe "#user_detail" do + describe '#user_detail' do it 'does not persist `user_detail` by default' do expect(create(:user).user_detail).not_to be_persisted end @@ -160,25 +163,25 @@ RSpec.describe User do end end - describe "#abuse_report" do + describe '#abuse_report' do let(:current_user) { create(:user) } let(:other_user) { create(:user) } it { is_expected.to have_one(:abuse_report) } - it "refers to the abuse report whose user_id is the current user" do + it 'refers to the abuse report whose user_id is the current user' do abuse_report = create(:abuse_report, reporter: other_user, user: current_user) expect(current_user.abuse_report).to eq(abuse_report) end - it "does not refer to the abuse report whose reporter_id is the current user" do + it 'does not refer to the abuse report whose reporter_id is the current user' do create(:abuse_report, reporter: current_user, user: other_user) expect(current_user.abuse_report).to be_nil end - it "does not update the user_id of an abuse report when the user is updated" do + it 'does not update the user_id of an abuse report when the user is updated' do abuse_report = create(:abuse_report, reporter: current_user, user: other_user) current_user.block @@ -343,8 +346,9 @@ RSpec.describe User do it 'falls back to english when I18n.default_locale is not an available language' do I18n.default_locale = :kl + default_preferred_language = user.send(:default_preferred_language) - expect(user.preferred_language).to eq 'en' + expect(user.preferred_language).to eq default_preferred_language end end end @@ -374,7 +378,7 @@ RSpec.describe User do end context 'when username is changed' do - let(:user) { build_stubbed(:user, username: 'old_path', namespace: build_stubbed(:namespace)) } + let(:user) { build_stubbed(:user, username: 'old_path', namespace: build_stubbed(:user_namespace)) } it 'validates move_dir is allowed for the namespace' do expect(user.namespace).to receive(:any_project_has_container_registry_tags?).and_return(true) @@ -401,7 +405,7 @@ RSpec.describe User do user = build(:user, username: "test.#{type}") expect(user).not_to be_valid - expect(user.errors.full_messages).to include('Username ending with a file extension is not allowed.') + expect(user.errors.full_messages).to include('Username ending with a reserved file extension is not allowed.') expect(build(:user, username: "test#{type}")).to be_valid end end @@ -490,6 +494,8 @@ RSpec.describe User do end describe 'email' do + let(:expected_error) { _('is not allowed for sign-up. Check with your administrator.') } + context 'when no signup domains allowed' do before do stub_application_setting(domain_allowlist: []) @@ -533,7 +539,7 @@ RSpec.describe User do it 'rejects example@test.com' do user = build(:user, email: "example@test.com") expect(user).to be_invalid - expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.')) + expect(user.errors.messages[:email].first).to eq(expected_error) end end @@ -550,13 +556,13 @@ RSpec.describe User do it 'rejects info@test.example.com' do user = build(:user, email: "info@test.example.com") expect(user).to be_invalid - expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.')) + expect(user.errors.messages[:email].first).to eq(expected_error) end it 'rejects example@test.com' do user = build(:user, email: "example@test.com") expect(user).to be_invalid - expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.')) + expect(user.errors.messages[:email].first).to eq(expected_error) end it 'accepts example@test.com when added by another user' do @@ -594,7 +600,7 @@ RSpec.describe User do it 'rejects info@example.com' do user = build(:user, email: 'info@example.com') expect(user).not_to be_valid - expect(user.errors.messages[:email].first).to eq(_('is not from an allowed domain.')) + expect(user.errors.messages[:email].first).to eq(expected_error) end it 'accepts info@example.com when added by another user' do @@ -628,7 +634,7 @@ RSpec.describe User do it 'rejects info@example.com' do user = build(:user, email: 'info@example.com') expect(user).not_to be_valid - expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.')) + expect(user.errors.messages[:email].first).to eq(expected_error) end end end @@ -669,7 +675,7 @@ RSpec.describe User do user = build(:user, email: 'info@gitlab.com') expect(user).not_to be_valid - expect(user.errors.messages[:email].first).to eq(_('is not allowed. Try again with a different email address, or contact your GitLab admin.')) + expect(user.errors.messages[:email].first).to eq(expected_error) end it 'does accept a valid email address' do @@ -715,7 +721,7 @@ RSpec.describe User do end end - describe "scopes" do + describe 'scopes' do context 'blocked users' do let_it_be(:active_user) { create(:user) } let_it_be(:blocked_user) { create(:user, :blocked) } @@ -753,8 +759,8 @@ RSpec.describe User do end end - describe ".with_two_factor" do - it "returns users with 2fa enabled via OTP" do + describe '.with_two_factor' do + it 'returns users with 2fa enabled via OTP' do user_with_2fa = create(:user, :two_factor_via_otp) user_without_2fa = create(:user) users_with_two_factor = described_class.with_two_factor.pluck(:id) @@ -763,8 +769,8 @@ RSpec.describe User do expect(users_with_two_factor).not_to include(user_without_2fa.id) end - shared_examples "returns the right users" do |trait| - it "returns users with 2fa enabled via hardware token" do + shared_examples 'returns the right users' do |trait| + it 'returns users with 2fa enabled via hardware token' do user_with_2fa = create(:user, trait) user_without_2fa = create(:user) users_with_two_factor = described_class.with_two_factor.pluck(:id) @@ -773,7 +779,7 @@ RSpec.describe User do expect(users_with_two_factor).not_to include(user_without_2fa.id) end - it "returns users with 2fa enabled via OTP and hardware token" do + it 'returns users with 2fa enabled via OTP and hardware token' do user_with_2fa = create(:user, :two_factor_via_otp, trait) user_without_2fa = create(:user) users_with_two_factor = described_class.with_two_factor.pluck(:id) @@ -791,17 +797,17 @@ RSpec.describe User do end end - describe "and U2F" do + describe 'and U2F' do it_behaves_like "returns the right users", :two_factor_via_u2f end - describe "and WebAuthn" do + describe 'and WebAuthn' do it_behaves_like "returns the right users", :two_factor_via_webauthn end end - describe ".without_two_factor" do - it "excludes users with 2fa enabled via OTP" do + describe '.without_two_factor' do + it 'excludes users with 2fa enabled via OTP' do user_with_2fa = create(:user, :two_factor_via_otp) user_without_2fa = create(:user) users_without_two_factor = described_class.without_two_factor.pluck(:id) @@ -810,8 +816,8 @@ RSpec.describe User do expect(users_without_two_factor).not_to include(user_with_2fa.id) end - describe "and u2f" do - it "excludes users with 2fa enabled via U2F" do + describe 'and u2f' do + it 'excludes users with 2fa enabled via U2F' do user_with_2fa = create(:user, :two_factor_via_u2f) user_without_2fa = create(:user) users_without_two_factor = described_class.without_two_factor.pluck(:id) @@ -820,7 +826,7 @@ RSpec.describe User do expect(users_without_two_factor).not_to include(user_with_2fa.id) end - it "excludes users with 2fa enabled via OTP and U2F" do + it 'excludes users with 2fa enabled via OTP and U2F' do user_with_2fa = create(:user, :two_factor_via_otp, :two_factor_via_u2f) user_without_2fa = create(:user) users_without_two_factor = described_class.without_two_factor.pluck(:id) @@ -830,8 +836,8 @@ RSpec.describe User do end end - describe "and webauthn" do - it "excludes users with 2fa enabled via WebAuthn" do + describe 'and webauthn' do + it 'excludes users with 2fa enabled via WebAuthn' do user_with_2fa = create(:user, :two_factor_via_webauthn) user_without_2fa = create(:user) users_without_two_factor = described_class.without_two_factor.pluck(:id) @@ -840,7 +846,7 @@ RSpec.describe User do expect(users_without_two_factor).not_to include(user_with_2fa.id) end - it "excludes users with 2fa enabled via OTP and WebAuthn" do + it 'excludes users with 2fa enabled via OTP and WebAuthn' do user_with_2fa = create(:user, :two_factor_via_otp, :two_factor_via_webauthn) user_without_2fa = create(:user) users_without_two_factor = described_class.without_two_factor.pluck(:id) @@ -1073,7 +1079,7 @@ RSpec.describe User do end end - describe "Respond to" do + describe 'Respond to' do it { is_expected.to respond_to(:admin?) } it { is_expected.to respond_to(:name) } it { is_expected.to respond_to(:external?) } @@ -1095,7 +1101,7 @@ RSpec.describe User do let(:user) { create(:user) } let(:external_user) { create(:user, external: true) } - it "sets other properties as well" do + it 'sets other properties as well' do expect(external_user.can_create_team).to be_falsey expect(external_user.can_create_group).to be_falsey expect(external_user.projects_limit).to be 0 @@ -1514,7 +1520,7 @@ RSpec.describe User do end describe '#generate_password' do - it "does not generate password by default" do + it 'does not generate password by default' do user = create(:user, password: 'abcdefghe') expect(user.password).to eq('abcdefghe') @@ -1882,14 +1888,14 @@ RSpec.describe User do describe 'deactivating a user' do let(:user) { create(:user, name: 'John Smith') } - context "an active user" do - it "can be deactivated" do + context 'an active user' do + it 'can be deactivated' do user.deactivate expect(user.deactivated?).to be_truthy end - context "when user deactivation emails are disabled" do + context 'when user deactivation emails are disabled' do before do stub_application_setting(user_deactivation_emails_enabled: false) end @@ -1900,7 +1906,7 @@ RSpec.describe User do end end - context "when user deactivation emails are enabled" do + context 'when user deactivation emails are enabled' do it 'sends deactivated user an email' do expect_next_instance_of(NotificationService) do |notification| allow(notification).to receive(:user_deactivated).with(user.name, user.notification_email_or_default) @@ -1911,12 +1917,12 @@ RSpec.describe User do end end - context "a user who is blocked" do + context 'a user who is blocked' do before do user.block end - it "cannot be deactivated" do + it 'cannot be deactivated' do user.deactivate expect(user.reload.deactivated?).to be_falsy @@ -2083,7 +2089,7 @@ RSpec.describe User do describe 'with defaults' do let(:user) { described_class.new } - it "applies defaults to user" do + it 'applies defaults to user' do expect(user.projects_limit).to eq(Gitlab.config.gitlab.default_projects_limit) expect(user.can_create_group).to eq(Gitlab.config.gitlab.default_can_create_group) expect(user.theme_id).to eq(Gitlab.config.gitlab.default_theme) @@ -2095,7 +2101,7 @@ RSpec.describe User do describe 'with default overrides' do let(:user) { described_class.new(projects_limit: 123, can_create_group: false, can_create_team: true) } - it "applies defaults to user" do + it 'applies defaults to user' do expect(user.projects_limit).to eq(123) expect(user.can_create_group).to be_falsey expect(user.theme_id).to eq(1) @@ -2114,7 +2120,7 @@ RSpec.describe User do stub_application_setting(user_default_external: true) end - it "creates external user by default" do + it 'creates external user by default' do user = create(:user) expect(user.external).to be_truthy @@ -2123,7 +2129,7 @@ RSpec.describe User do end describe 'with default overrides' do - it "creates a non-external user" do + it 'creates a non-external user' do user = create(:user, external: false) expect(user.external).to be_falsey @@ -2139,7 +2145,7 @@ RSpec.describe User do } protocol_and_expectation.each do |protocol, expected| - it "has correct require_ssh_key?" do + it 'has correct require_ssh_key?' do stub_application_setting(enabled_git_access_protocol: protocol) user = build(:user) @@ -2542,71 +2548,79 @@ RSpec.describe User do end describe '.find_by_full_path' do - let!(:user) { create(:user) } + using RSpec::Parameterized::TableSyntax - context 'with a route matching the given path' do - let!(:route) { user.namespace.route } + # TODO: this `where/when` can be removed in issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070 + # At that point we only need to check `user_namespace` + where(namespace_type: [:namespace, :user_namespace]) - it 'returns the user' do - expect(described_class.find_by_full_path(route.path)).to eq(user) - end + with_them do + let!(:user) { create(:user, namespace: create(namespace_type)) } - it 'is case-insensitive' do - expect(described_class.find_by_full_path(route.path.upcase)).to eq(user) - expect(described_class.find_by_full_path(route.path.downcase)).to eq(user) - end - end + context 'with a route matching the given path' do + let!(:route) { user.namespace.route } - context 'with a redirect route matching the given path' do - let!(:redirect_route) { user.namespace.redirect_routes.create!(path: 'foo') } + it 'returns the user' do + expect(described_class.find_by_full_path(route.path)).to eq(user) + end - context 'without the follow_redirects option' do - it 'returns nil' do - expect(described_class.find_by_full_path(redirect_route.path)).to eq(nil) + it 'is case-insensitive' do + expect(described_class.find_by_full_path(route.path.upcase)).to eq(user) + expect(described_class.find_by_full_path(route.path.downcase)).to eq(user) end end - context 'with the follow_redirects option set to true' do - it 'returns the user' do - expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to eq(user) + context 'with a redirect route matching the given path' do + let!(:redirect_route) { user.namespace.redirect_routes.create!(path: 'foo') } + + context 'without the follow_redirects option' do + it 'returns nil' do + expect(described_class.find_by_full_path(redirect_route.path)).to eq(nil) + end end - it 'is case-insensitive' do - expect(described_class.find_by_full_path(redirect_route.path.upcase, follow_redirects: true)).to eq(user) - expect(described_class.find_by_full_path(redirect_route.path.downcase, follow_redirects: true)).to eq(user) + context 'with the follow_redirects option set to true' do + it 'returns the user' do + expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to eq(user) + end + + it 'is case-insensitive' do + expect(described_class.find_by_full_path(redirect_route.path.upcase, follow_redirects: true)).to eq(user) + expect(described_class.find_by_full_path(redirect_route.path.downcase, follow_redirects: true)).to eq(user) + end end end - end - context 'without a route or a redirect route matching the given path' do - context 'without the follow_redirects option' do - it 'returns nil' do - expect(described_class.find_by_full_path('unknown')).to eq(nil) + context 'without a route or a redirect route matching the given path' do + context 'without the follow_redirects option' do + it 'returns nil' do + expect(described_class.find_by_full_path('unknown')).to eq(nil) + end end - end - context 'with the follow_redirects option set to true' do - it 'returns nil' do - expect(described_class.find_by_full_path('unknown', follow_redirects: true)).to eq(nil) + context 'with the follow_redirects option set to true' do + it 'returns nil' do + expect(described_class.find_by_full_path('unknown', follow_redirects: true)).to eq(nil) + end end end - end - context 'with a group route matching the given path' do - let!(:group) { create(:group, path: 'group_path') } + context 'with a group route matching the given path' do + let!(:group) { create(:group, path: 'group_path') } - context 'when the group namespace has an owner_id (legacy data)' do - before do - group.update!(owner_id: user.id) - end + context 'when the group namespace has an owner_id (legacy data)' do + before do + group.update!(owner_id: user.id) + end - it 'returns nil' do - expect(described_class.find_by_full_path('group_path')).to eq(nil) + it 'returns nil' do + expect(described_class.find_by_full_path('group_path')).to eq(nil) + end end - end - context 'when the group namespace does not have an owner_id' do - it 'returns nil' do - expect(described_class.find_by_full_path('group_path')).to eq(nil) + context 'when the group namespace does not have an owner_id' do + it 'returns nil' do + expect(described_class.find_by_full_path('group_path')).to eq(nil) + end end end end @@ -2615,7 +2629,7 @@ RSpec.describe User do describe 'all_ssh_keys' do it { is_expected.to have_many(:keys).dependent(:destroy) } - it "has all ssh keys" do + it 'has all ssh keys' do user = create :user key = create :key, key: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD33bWLBxu48Sev9Fert1yzEO4WGcWglWF7K/AwblIUFselOt/QdOL9DSjpQGxLagO1s9wl53STIO8qGS4Ms0EJZyIXOEFMjFJ5xmjSy+S37By4sG7SsltQEHMxtbtFOaW5LV2wCrX+rUsRNqLMamZjgjcPO0/EgGCXIGMAYW4O7cwGZdXWYIhQ1Vwy+CsVMDdPkPgBXqK7nR/ey8KMs8ho5fMNgB5hBw/AL9fNGhRw3QTD6Q12Nkhl4VZES2EsZqlpNnJttnPdp847DUsT6yuLRlfiQfz5Cn9ysHFdXObMN5VYIiPFwHeYCZp1X2S4fDZooRE8uOLTfxWHPXwrhqSH", user_id: user.id @@ -2651,10 +2665,10 @@ RSpec.describe User do end end - describe "#clear_avatar_caches" do + describe '#clear_avatar_caches' do let(:user) { create(:user) } - it "clears the avatar cache when saving" do + it 'clears the avatar cache when saving' do allow(user).to receive(:avatar_changed?).and_return(true) expect(Gitlab::AvatarCache).to receive(:delete_by_email).with(*user.verified_emails) @@ -3180,7 +3194,7 @@ RSpec.describe User do end end - describe "#last_active_at" do + describe '#last_active_at' do let(:last_activity_on) { 5.days.ago.to_date } let(:current_sign_in_at) { 8.days.ago } @@ -3218,7 +3232,7 @@ RSpec.describe User do end end - describe "#can_be_deactivated?" do + describe '#can_be_deactivated?' do let(:activity) { {} } let(:user) { create(:user, name: 'John Smith', **activity) } let(:day_within_minium_inactive_days_threshold) { User::MINIMUM_INACTIVE_DAYS.pred.days.ago } @@ -3236,7 +3250,7 @@ RSpec.describe User do end end - context "a user who is not active" do + context 'a user who is not active' do before do user.block end @@ -3277,7 +3291,7 @@ RSpec.describe User do end end - describe "#contributed_projects" do + describe '#contributed_projects' do subject { create(:user) } let!(:project1) { create(:project) } @@ -3292,11 +3306,11 @@ RSpec.describe User do project2.add_maintainer(subject) end - it "includes IDs for projects the user has pushed to" do + it 'includes IDs for projects the user has pushed to' do expect(subject.contributed_projects).to include(project1) end - it "includes IDs for projects the user has had merge requests merged into" do + it 'includes IDs for projects the user has had merge requests merged into' do expect(subject.contributed_projects).to include(project3) end @@ -3390,7 +3404,7 @@ RSpec.describe User do end end - describe "#recent_push" do + describe '#recent_push' do let(:user) { build(:user) } let(:project) { build(:project) } let(:event) { build(:push_event) } @@ -3554,7 +3568,7 @@ RSpec.describe User do expect(user.authorized_projects).to include(project) end - it "includes personal projects user has been given access to" do + it 'includes personal projects user has been given access to' do user1 = create(:user) user2 = create(:user) project = create(:project, :private, namespace: user1.namespace) @@ -3564,7 +3578,7 @@ RSpec.describe User do expect(user2.authorized_projects).to include(project) end - it "includes projects of groups user has been added to" do + it 'includes projects of groups user has been added to' do group = create(:group) project = create(:project, group: group) user = create(:user) @@ -3574,7 +3588,7 @@ RSpec.describe User do expect(user.authorized_projects).to include(project) end - it "does not include projects of groups user has been removed from" do + it 'does not include projects of groups user has been removed from', :sidekiq_inline do group = create(:group) project = create(:project, group: group) user = create(:user) @@ -3599,7 +3613,7 @@ RSpec.describe User do expect(user.authorized_projects).to include(project) end - it "does not include destroyed projects user had access to" do + it 'does not include destroyed projects user had access to' do user1 = create(:user) user2 = create(:user) project = create(:project, :private, namespace: user1.namespace) @@ -3613,7 +3627,7 @@ RSpec.describe User do expect(user2.authorized_projects).not_to include(project) end - it "does not include projects of destroyed groups user had access to" do + it 'does not include projects of destroyed groups user had access to' do group = create(:group) project = create(:project, namespace: group) user = create(:user) @@ -3841,7 +3855,7 @@ RSpec.describe User do end context 'with runner in a personal project' do - let!(:namespace) { create(:namespace, owner: user) } + let!(:namespace) { create(:user_namespace, owner: user) } let!(:project) { create(:project, namespace: namespace) } let!(:runner) { create(:ci_runner, :project, projects: [project]) } @@ -3909,7 +3923,7 @@ RSpec.describe User do end context 'with personal project runner in an owned group in an owned namespace and a group runner in that group' do - let!(:namespace) { create(:namespace, owner: user) } + let!(:namespace) { create(:user_namespace, owner: user) } let!(:group) { create(:group) } let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } let!(:project) { create(:project, namespace: namespace, group: group) } @@ -3923,7 +3937,7 @@ RSpec.describe User do end context 'with personal project runner in an owned namespace, an owned group, a subgroup and a group runner in that subgroup' do - let!(:namespace) { create(:namespace, owner: user) } + let!(:namespace) { create(:user_namespace, owner: user) } let!(:group) { create(:group) } let!(:subgroup) { create(:group, parent: group) } let!(:group_runner) { create(:ci_runner, :group, groups: [subgroup]) } @@ -4166,7 +4180,7 @@ RSpec.describe User do expect(user.admin).to be true end - it "accepts string values in addition to symbols" do + it 'accepts string values in addition to symbols' do user.access_level = 'admin' expect(user.access_level).to eq(:admin) @@ -4247,7 +4261,7 @@ RSpec.describe User do expect(ghost.user_type).to eq 'ghost' end - it "does not create a second ghost user if one is already present" do + it 'does not create a second ghost user if one is already present' do expect do described_class.ghost described_class.ghost @@ -4256,7 +4270,7 @@ RSpec.describe User do end context "when a regular user exists with the username 'ghost'" do - it "creates a ghost user with a non-conflicting username" do + it 'creates a ghost user with a non-conflicting username' do create(:user, username: 'ghost') ghost = described_class.ghost @@ -4266,7 +4280,7 @@ RSpec.describe User do end context "when a regular user exists with the email 'ghost@example.com'" do - it "creates a ghost user with a non-conflicting email" do + it 'creates a ghost user with a non-conflicting email' do create(:user, email: 'ghost@example.com') ghost = described_class.ghost @@ -4605,6 +4619,7 @@ RSpec.describe User do user.save! expect(user.namespace).not_to be_nil + expect(user.namespace).to be_kind_of(Namespaces::UserNamespace) end it 'creates the namespace setting' do @@ -4746,7 +4761,7 @@ RSpec.describe User do it { is_expected.to be true } end - context 'when email and username aren\'t changed' do + context "when email and username aren't changed" do before do user.name = 'new_name' end @@ -5057,26 +5072,26 @@ RSpec.describe User do subject { user.required_terms_not_accepted? } - context "when terms are not enforced" do + context 'when terms are not enforced' do it { is_expected.to be_falsey } end - context "when terms are enforced" do + context 'when terms are enforced' do before do enforce_terms end - it "is not accepted by the user" do + it 'is not accepted by the user' do expect(subject).to be_truthy end - it "is accepted by the user" do + it 'is accepted by the user' do accept_terms(user) expect(subject).to be_falsey end - it "auto accepts the term for project bots" do + it 'auto accepts the term for project bots' do expect(project_bot.required_terms_not_accepted?).to be_falsey end end @@ -6165,4 +6180,14 @@ RSpec.describe User do it_behaves_like 'groups_with_developer_maintainer_project_access examples' end end + + describe '.get_ids_by_username' 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]) + end + end end diff --git a/spec/models/users/credit_card_validation_spec.rb b/spec/models/users/credit_card_validation_spec.rb index fb9f6e35038..d2b4f5ebd65 100644 --- a/spec/models/users/credit_card_validation_spec.rb +++ b/spec/models/users/credit_card_validation_spec.rb @@ -4,4 +4,22 @@ require 'spec_helper' RSpec.describe Users::CreditCardValidation do it { is_expected.to belong_to(:user) } + + it { is_expected.to validate_length_of(:holder_name).is_at_most(26) } + it { is_expected.to validate_numericality_of(:last_digits).is_less_than_or_equal_to(9999) } + + describe '.similar_records' do + let(:card_details) { subject.attributes.slice(:expiration_date, :last_digits, :holder_name) } + + subject(:credit_card_validation) { create(:credit_card_validation) } + + let!(:match1) { create(:credit_card_validation, card_details) } + let!(:other1) { create(:credit_card_validation, card_details.merge(last_digits: 9)) } + let!(:match2) { create(:credit_card_validation, card_details) } + let!(:other2) { create(:credit_card_validation, card_details.merge(holder_name: 'foo bar')) } + + it 'returns records with matching credit card, ordered by credit_card_validated_at' do + expect(subject.similar_records).to eq([match2, match1, subject]) + end + end end |