diff options
Diffstat (limited to 'spec/models')
127 files changed, 3354 insertions, 1301 deletions
diff --git a/spec/models/achievements/achievement_spec.rb b/spec/models/achievements/achievement_spec.rb new file mode 100644 index 00000000000..10c04d184af --- /dev/null +++ b/spec/models/achievements/achievement_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Achievements::Achievement, type: :model, feature_category: :users do + describe 'associations' do + it { is_expected.to belong_to(:namespace).required } + end + + describe 'modules' do + subject { described_class } + + it { is_expected.to include_module(Avatarable) } + end + + describe 'validations' do + subject { create(:achievement) } + + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).case_insensitive.scoped_to([:namespace_id]) } + 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 '#name' do + it 'strips name' do + achievement = described_class.new(name: ' AchievementTest ') + achievement.valid? + + expect(achievement.name).to eq('AchievementTest') + end + end +end diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 9d84279a75e..289408231a9 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -14,6 +14,7 @@ RSpec.describe Appearance do subject(:appearance) { described_class.new } it { expect(appearance.title).to eq('') } + it { expect(appearance.short_title).to eq('') } it { expect(appearance.description).to eq('') } it { expect(appearance.new_project_guidelines).to eq('') } it { expect(appearance.profile_image_guidelines).to eq('') } diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index fd86a784b2d..1454c82c531 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -125,6 +125,8 @@ RSpec.describe ApplicationSetting do it { is_expected.to validate_numericality_of(:max_yaml_depth).only_integer.is_greater_than(0) } it { is_expected.to validate_presence_of(:max_pages_size) } it { is_expected.to validate_presence_of(:max_pages_custom_domains_per_project) } + it { is_expected.to validate_presence_of(:max_terraform_state_size_bytes) } + it { is_expected.to validate_numericality_of(:max_terraform_state_size_bytes).only_integer.is_greater_than_or_equal_to(0) } it 'ensures max_pages_size is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do is_expected.to validate_numericality_of(:max_pages_size).only_integer.is_greater_than_or_equal_to(0) @@ -214,6 +216,10 @@ RSpec.describe ApplicationSetting do it { is_expected.to allow_value(http).for(:jira_connect_proxy_url) } it { is_expected.to allow_value(https).for(:jira_connect_proxy_url) } + it { is_expected.to allow_value(true).for(:bulk_import_enabled) } + it { is_expected.to allow_value(false).for(:bulk_import_enabled) } + it { is_expected.not_to allow_value(nil).for(:bulk_import_enabled) } + context 'when deactivate_dormant_users is enabled' do before do stub_application_setting(deactivate_dormant_users: true) @@ -1283,11 +1289,10 @@ RSpec.describe ApplicationSetting do end end - describe '#instance_review_permitted?', :request_store, :use_clean_rails_memory_store_caching do + describe '#instance_review_permitted?', :request_store, :use_clean_rails_memory_store_caching, :without_license do subject { setting.instance_review_permitted? } before do - allow(License).to receive(:current).and_return(nil) if Gitlab.ee? allow(Rails.cache).to receive(:fetch).and_call_original expect(Rails.cache).to receive(:fetch).with('limited_users_count', anything).and_return( ::ApplicationSetting::INSTANCE_REVIEW_MIN_USERS + users_over_minimum diff --git a/spec/models/badge_spec.rb b/spec/models/badge_spec.rb index f3c95332ca0..79a716c2087 100644 --- a/spec/models/badge_spec.rb +++ b/spec/models/badge_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Badge do - let(:placeholder_url) { 'http://www.example.com/%{project_path}/%{project_id}/%{default_branch}/%{commit_sha}' } + let(:placeholder_url) { 'http://www.example.com/%{project_path}/%{project_id}/%{project_name}/%{default_branch}/%{commit_sha}/%{project_title}' } describe 'validations' do # Requires the let variable url_sym @@ -64,7 +64,7 @@ RSpec.describe Badge do it 'uses the project information to populate the url placeholders' do stub_project_commit_info(project) - expect(badge.public_send("rendered_#{method}", project)).to eq "http://www.example.com/#{project.full_path}/#{project.id}/master/whatever" + expect(badge.public_send("rendered_#{method}", project)).to eq "http://www.example.com/#{project.full_path}/#{project.id}/#{project.path}/master/whatever/#{project.title}" end it 'returns the url if the project used is nil' do diff --git a/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb b/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb index 8d5c7ce84f6..d28fa0bbe97 100644 --- a/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb +++ b/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb @@ -14,224 +14,109 @@ RSpec.describe BlobViewer::MetricsDashboardYml do subject(:viewer) { described_class.new(blob) } - context 'with metrics_dashboard_exhaustive_validations feature flag on' do - before do - stub_feature_flags(metrics_dashboard_exhaustive_validations: true) - end - - context 'when the definition is valid' do + context 'when the definition is valid' do + describe '#valid?' do before do - allow(Gitlab::Metrics::Dashboard::Validator).to receive(:errors).and_return([]) - end - - describe '#valid?' do - it 'calls prepare! on the viewer' do - expect(viewer).to receive(:prepare!) - - viewer.valid? - end - - it 'processes dashboard yaml and returns true', :aggregate_failures do - yml = ::Gitlab::Config::Loader::Yaml.new(data).load_raw! - - expect_next_instance_of(::Gitlab::Config::Loader::Yaml, data) do |loader| - expect(loader).to receive(:load_raw!).and_call_original - end - expect(Gitlab::Metrics::Dashboard::Validator) - .to receive(:errors) - .with(yml, dashboard_path: '.gitlab/dashboards/custom-dashboard.yml', project: project) - .and_call_original - expect(viewer.valid?).to be true - end - end - - describe '#errors' do - it 'returns empty array' do - expect(viewer.errors).to eq [] - end + allow(PerformanceMonitoring::PrometheusDashboard).to receive(:from_json) end - end - context 'when definition is invalid' do - let(:error) { ::Gitlab::Metrics::Dashboard::Validator::Errors::SchemaValidationError.new } - let(:data) do - <<~YAML - dashboard: - YAML - end + it 'calls prepare! on the viewer' do + expect(viewer).to receive(:prepare!) - before do - allow(Gitlab::Metrics::Dashboard::Validator).to receive(:errors).and_return([error]) + viewer.valid? end - describe '#valid?' do - it 'returns false' do - expect(viewer.valid?).to be false - end - end + it 'processes dashboard yaml and returns true', :aggregate_failures do + yml = ::Gitlab::Config::Loader::Yaml.new(data).load_raw! - describe '#errors' do - it 'returns validation errors' do - expect(viewer.errors).to eq ["Dashboard failed schema validation"] + expect_next_instance_of(::Gitlab::Config::Loader::Yaml, data) do |loader| + expect(loader).to receive(:load_raw!).and_call_original end + expect(PerformanceMonitoring::PrometheusDashboard) + .to receive(:from_json) + .with(yml) + .and_call_original + expect(viewer.valid?).to be true end end - context 'when YAML syntax is invalid' do - let(:data) do - <<~YAML - dashboard: 'empty metrics' - panel_groups: - - group: 'Group Title' - YAML - end - - describe '#valid?' do - it 'returns false' do - expect(Gitlab::Metrics::Dashboard::Validator).not_to receive(:errors) - expect(viewer.valid?).to be false - end - end - - describe '#errors' do - it 'returns validation errors' do - expect(viewer.errors).to all be_kind_of String - end - end - end - - context 'when YAML loader raises error' do - let(:data) do - <<~YAML - large yaml file - YAML - end - - before do - allow(::Gitlab::Config::Loader::Yaml).to receive(:new) - .and_raise(::Gitlab::Config::Loader::Yaml::DataTooLargeError, 'The parsed YAML is too big') - end - - it 'is invalid' do - expect(Gitlab::Metrics::Dashboard::Validator).not_to receive(:errors) - expect(viewer.valid?).to be false - end - - it 'returns validation errors' do - expect(viewer.errors).to eq ['The parsed YAML is too big'] + describe '#errors' do + it 'returns empty array' do + expect(viewer.errors).to eq [] end end end - context 'with metrics_dashboard_exhaustive_validations feature flag off' do - before do - stub_feature_flags(metrics_dashboard_exhaustive_validations: false) + context 'when definition is invalid' do + let(:error) { ActiveModel::ValidationError.new(PerformanceMonitoring::PrometheusDashboard.new.tap(&:validate)) } + let(:data) do + <<~YAML + dashboard: + YAML end - context 'when the definition is valid' do - describe '#valid?' do - before do - allow(PerformanceMonitoring::PrometheusDashboard).to receive(:from_json) - end - - it 'calls prepare! on the viewer' do - expect(viewer).to receive(:prepare!) + describe '#valid?' do + it 'returns false' do + expect(PerformanceMonitoring::PrometheusDashboard) + .to receive(:from_json).and_raise(error) - viewer.valid? - end - - it 'processes dashboard yaml and returns true', :aggregate_failures do - yml = ::Gitlab::Config::Loader::Yaml.new(data).load_raw! - - expect_next_instance_of(::Gitlab::Config::Loader::Yaml, data) do |loader| - expect(loader).to receive(:load_raw!).and_call_original - end - expect(PerformanceMonitoring::PrometheusDashboard) - .to receive(:from_json) - .with(yml) - .and_call_original - expect(viewer.valid?).to be true - end - end - - describe '#errors' do - it 'returns empty array' do - expect(viewer.errors).to eq [] - end + expect(viewer.valid?).to be false end end - context 'when definition is invalid' do - let(:error) { ActiveModel::ValidationError.new(PerformanceMonitoring::PrometheusDashboard.new.tap(&:validate)) } - let(:data) do - <<~YAML - dashboard: - YAML - end - - describe '#valid?' do - it 'returns false' do - expect(PerformanceMonitoring::PrometheusDashboard) - .to receive(:from_json).and_raise(error) + describe '#errors' do + it 'returns validation errors' do + allow(PerformanceMonitoring::PrometheusDashboard) + .to receive(:from_json).and_raise(error) - expect(viewer.valid?).to be false - end - end - - describe '#errors' do - it 'returns validation errors' do - allow(PerformanceMonitoring::PrometheusDashboard) - .to receive(:from_json).and_raise(error) - - expect(viewer.errors).to eq error.model.errors.messages.map { |messages| messages.join(': ') } - end + expect(viewer.errors).to eq error.model.errors.messages.map { |messages| messages.join(': ') } end end + end - context 'when YAML syntax is invalid' do - let(:data) do - <<~YAML + context 'when YAML syntax is invalid' do + let(:data) do + <<~YAML dashboard: 'empty metrics' panel_groups: - group: 'Group Title' - YAML - end + YAML + end - describe '#valid?' do - it 'returns false' do - expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:from_json) - expect(viewer.valid?).to be false - end + describe '#valid?' do + it 'returns false' do + expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:from_json) + expect(viewer.valid?).to be false end + end - describe '#errors' do - it 'returns validation errors' do - expect(viewer.errors).to eq ["YAML syntax: (<unknown>): did not find expected key while parsing a block mapping at line 1 column 1"] - end + describe '#errors' do + it 'returns validation errors' do + expect(viewer.errors).to eq ["YAML syntax: (<unknown>): did not find expected key while parsing a block mapping at line 1 column 1"] end end + end - context 'when YAML loader raises error' do - let(:data) do - <<~YAML + context 'when YAML loader raises error' do + let(:data) do + <<~YAML large yaml file - YAML - end + YAML + end - before do - allow(::Gitlab::Config::Loader::Yaml).to( - receive(:new).and_raise(::Gitlab::Config::Loader::Yaml::DataTooLargeError, 'The parsed YAML is too big') - ) - end + before do + allow(::Gitlab::Config::Loader::Yaml).to( + receive(:new).and_raise(::Gitlab::Config::Loader::Yaml::DataTooLargeError, 'The parsed YAML is too big') + ) + end - it 'is invalid' do - expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:from_json) - expect(viewer.valid?).to be false - end + it 'is invalid' do + expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:from_json) + expect(viewer.valid?).to be false + end - it 'returns validation errors' do - expect(viewer.errors).to eq ["YAML syntax: The parsed YAML is too big"] - end + it 'returns validation errors' do + expect(viewer.errors).to eq ["YAML syntax: The parsed YAML is too big"] end end end diff --git a/spec/models/bulk_imports/tracker_spec.rb b/spec/models/bulk_imports/tracker_spec.rb index 1aa76d4dadd..1516ab106cb 100644 --- a/spec/models/bulk_imports/tracker_spec.rb +++ b/spec/models/bulk_imports/tracker_spec.rb @@ -54,13 +54,16 @@ RSpec.describe BulkImports::Tracker, type: :model do it 'returns the not started pipeline trackers from the minimum stage number' do stage_1_tracker = create(:bulk_import_tracker, entity: entity, stage: 1) + stage_1_finished_tracker = create(:bulk_import_tracker, :finished, entity: entity, stage: 1) + stage_1_failed_tracker = create(:bulk_import_tracker, :failed, entity: entity, stage: 1) + stage_1_skipped_tracker = create(:bulk_import_tracker, :skipped, entity: entity, stage: 1) stage_2_tracker = create(:bulk_import_tracker, entity: entity, stage: 2) expect(described_class.next_pipeline_trackers_for(entity.id)) .to include(stage_1_tracker) expect(described_class.next_pipeline_trackers_for(entity.id)) - .not_to include(stage_2_tracker) + .not_to include(stage_2_tracker, stage_1_finished_tracker, stage_1_failed_tracker, stage_1_skipped_tracker) end end diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index df24c92149d..169b00b9c74 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Bridge do +RSpec.describe Ci::Bridge, feature_category: :continuous_integration do let_it_be(:project) { create(:project) } let_it_be(:target_project) { create(:project, name: 'project', namespace: create(:namespace, name: 'my')) } let_it_be(:pipeline) { create(:ci_pipeline, project: project) } @@ -34,6 +34,24 @@ RSpec.describe Ci::Bridge do expect(bridge).to have_one(:downstream_pipeline) end + describe '#sourced_pipelines' do + subject { bridge.sourced_pipelines } + + it 'raises error' do + expect { subject }.to raise_error RuntimeError, 'Ci::Bridge does not have sourced_pipelines association' + end + + context 'when ci_bridge_remove_sourced_pipelines is disabled' do + before do + stub_feature_flags(ci_bridge_remove_sourced_pipelines: false) + end + + it 'returns the sourced_pipelines association' do + expect(bridge.sourced_pipelines).to eq([]) + end + end + end + describe '#retryable?' do let(:bridge) { create(:ci_bridge, :success) } diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index e728ce0f474..8bf3af44be6 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -6,7 +6,6 @@ RSpec.describe Ci::BuildMetadata do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, group: group, build_timeout: 2000) } - let_it_be(:pipeline) do create(:ci_pipeline, project: project, sha: project.commit.id, @@ -14,7 +13,9 @@ RSpec.describe Ci::BuildMetadata do status: 'success') end - let(:job) { create(:ci_build, pipeline: pipeline) } + let_it_be_with_reload(:runner) { create(:ci_runner) } + + let(:job) { create(:ci_build, pipeline: pipeline, runner: runner) } let(:metadata) { job.metadata } it_behaves_like 'having unique enum values' @@ -32,63 +33,110 @@ RSpec.describe Ci::BuildMetadata do end end - context 'when project timeout is set' do - context 'when runner is assigned to the job' do + context 'when job, project and runner timeouts are set' do + context 'when job timeout is lower then runner timeout' do before do - job.update!(runner: runner) + runner.update!(maximum_timeout: 4000) + job.update!(options: { job_timeout: 3000 }) end - context 'when runner timeout is not set' do - let(:runner) { create(:ci_runner, maximum_timeout: nil) } + it_behaves_like 'sets timeout', 'job_timeout_source', 3000 + end - it_behaves_like 'sets timeout', 'project_timeout_source', 2000 + context 'when runner timeout is lower then job timeout' do + before do + runner.update!(maximum_timeout: 2000) + job.update!(options: { job_timeout: 3000 }) end - context 'when runner timeout is lower than project timeout' do - let(:runner) { create(:ci_runner, maximum_timeout: 1900) } + it_behaves_like 'sets timeout', 'runner_timeout_source', 2000 + end + end - it_behaves_like 'sets timeout', 'runner_timeout_source', 1900 + context 'when job, project timeout values are set and runner is assigned' do + context 'when runner has no timeout set' do + before do + runner.update!(maximum_timeout: nil) + job.update!(options: { job_timeout: 3000 }) end - context 'when runner timeout is higher than project timeout' do - let(:runner) { create(:ci_runner, maximum_timeout: 2100) } + it_behaves_like 'sets timeout', 'job_timeout_source', 3000 + end + end - it_behaves_like 'sets timeout', 'project_timeout_source', 2000 + context 'when only job and project timeouts are defined' do + context 'when job timeout is lower then project timeout' do + before do + job.update!(options: { job_timeout: 1000 }) end - end - context 'when job timeout is set' do - context 'when job timeout is higher than project timeout' do - let(:job) { create(:ci_build, pipeline: pipeline, options: { job_timeout: 3000 }) } + it_behaves_like 'sets timeout', 'job_timeout_source', 1000 + end - it_behaves_like 'sets timeout', 'job_timeout_source', 3000 + context 'when project timeout is lower then job timeout' do + before do + job.update!(options: { job_timeout: 3000 }) end - context 'when job timeout is lower than project timeout' do - let(:job) { create(:ci_build, pipeline: pipeline, options: { job_timeout: 1000 }) } + it_behaves_like 'sets timeout', 'job_timeout_source', 3000 + end + end + + context 'when only project and runner timeouts are defined' do + before do + runner.update!(maximum_timeout: 1900) + end + + context 'when runner timeout is lower then project timeout' do + it_behaves_like 'sets timeout', 'runner_timeout_source', 1900 + end - it_behaves_like 'sets timeout', 'job_timeout_source', 1000 + context 'when project timeout is lower then runner timeout' do + before do + runner.update!(maximum_timeout: 2100) end + + it_behaves_like 'sets timeout', 'project_timeout_source', 2000 end + end - context 'when both runner and job timeouts are set' do + context 'when only job and runner timeouts are defined' do + context 'when runner timeout is lower them job timeout' do before do - job.update!(runner: runner) + job.update!(options: { job_timeout: 2000 }) + runner.update!(maximum_timeout: 1900) end - context 'when job timeout is higher than runner timeout' do - let(:job) { create(:ci_build, pipeline: pipeline, options: { job_timeout: 3000 }) } - let(:runner) { create(:ci_runner, maximum_timeout: 2100) } + it_behaves_like 'sets timeout', 'runner_timeout_source', 1900 + end - it_behaves_like 'sets timeout', 'runner_timeout_source', 2100 + context 'when job timeout is lower them runner timeout' do + before do + job.update!(options: { job_timeout: 1000 }) + runner.update!(maximum_timeout: 1900) end - context 'when job timeout is lower than runner timeout' do - let(:job) { create(:ci_build, pipeline: pipeline, options: { job_timeout: 1900 }) } - let(:runner) { create(:ci_runner, maximum_timeout: 2100) } + it_behaves_like 'sets timeout', 'job_timeout_source', 1000 + end + end + + context 'when only job timeout is defined and runner is assigned, but has no timeout set' do + before do + job.update!(options: { job_timeout: 1000 }) + runner.update!(maximum_timeout: nil) + end + + it_behaves_like 'sets timeout', 'job_timeout_source', 1000 + end - it_behaves_like 'sets timeout', 'job_timeout_source', 1900 + context 'when only one timeout value is defined' do + context 'when only project timeout value is defined' do + before do + job.update!(options: { job_timeout: nil }) + runner.update!(maximum_timeout: nil) end + + it_behaves_like 'sets timeout', 'project_timeout_source', 2000 end end end @@ -107,9 +155,7 @@ RSpec.describe Ci::BuildMetadata do } metadata.id_tokens = { TEST_JWT_TOKEN: { - id_token: { - aud: 'https://gitlab.test' - } + aud: 'https://gitlab.test' } } @@ -152,6 +198,29 @@ RSpec.describe Ci::BuildMetadata do end end + describe '#enable_debug_trace!' do + subject { metadata.enable_debug_trace! } + + context 'when debug_trace_enabled is false' do + it 'sets debug_trace_enabled to true' do + subject + + expect(metadata.debug_trace_enabled).to eq(true) + end + end + + context 'when debug_trace_enabled is true' do + before do + metadata.update!(debug_trace_enabled: true) + end + + it 'does not set debug_trace_enabled to true', :aggregate_failures do + expect(described_class).not_to receive(:save!) + expect(metadata.debug_trace_enabled).to eq(true) + end + end + end + describe 'partitioning' do context 'with job' do let(:status) { build(:commit_status, partition_id: 123) } @@ -183,28 +252,6 @@ RSpec.describe Ci::BuildMetadata do end end - describe 'routing table switch' do - context 'with ff disabled' do - before do - stub_feature_flags(ci_partitioning_use_ci_builds_metadata_routing_table: false) - end - - it 'uses the legacy table' do - expect(described_class.table_name).to eq('ci_builds_metadata') - end - end - - context 'with ff enabled' do - before do - stub_feature_flags(ci_partitioning_use_ci_builds_metadata_routing_table: true) - end - - it 'uses the routing table' do - expect(described_class.table_name).to eq('p_ci_builds_metadata') - end - end - end - context 'jsonb fields serialization' do it 'changing other fields does not change config_options' do expect { metadata.id = metadata.id }.not_to change(metadata, :changes) diff --git a/spec/models/ci/build_need_spec.rb b/spec/models/ci/build_need_spec.rb index c2cf9027055..aa1c57d1788 100644 --- a/spec/models/ci/build_need_spec.rb +++ b/spec/models/ci/build_need_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::BuildNeed, model: true do +RSpec.describe Ci::BuildNeed, model: true, feature_category: :continuous_integration do let(:build_need) { build(:ci_build_need) } it { is_expected.to belong_to(:build).class_name('Ci::Processable') } @@ -35,4 +35,62 @@ RSpec.describe Ci::BuildNeed, model: true do end end end + + describe 'partitioning' do + context 'with build' do + let(:build) { FactoryBot.build(:ci_build, partition_id: ci_testing_partition_id) } + let(:build_need) { FactoryBot.build(:ci_build_need, build: build) } + + it 'sets partition_id to the current partition value' do + expect { build_need.valid? }.to change { build_need.partition_id }.to(ci_testing_partition_id) + end + + context 'when it is already set' do + let(:build_need) { FactoryBot.build(:ci_build_need, partition_id: 125) } + + it 'does not change the partition_id value' do + expect { build_need.valid? }.not_to change { build_need.partition_id } + end + end + end + + context 'without build' do + let(:build_need) { FactoryBot.build(:ci_build_need, build: nil) } + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { build_need.valid? }.not_to change { build_need.partition_id } + end + end + + context 'when using bulk_insert' do + include Ci::PartitioningHelpers + + let(:new_pipeline) { create(:ci_pipeline) } + let(:ci_build) { build(:ci_build, pipeline: new_pipeline) } + + before do + stub_current_partition_id + end + + it 'creates build needs successfully', :aggregate_failures do + ci_build.needs_attributes = [ + { name: "build", artifacts: true }, + { name: "build2", artifacts: true }, + { name: "build3", artifacts: true } + ] + + expect(described_class).to receive(:bulk_insert!).and_call_original + + BulkInsertableAssociations.with_bulk_insert do + ci_build.save! + end + + expect(described_class.count).to eq(3) + expect(described_class.first.partition_id).to eq(ci_testing_partition_id) + expect(described_class.second.partition_id).to eq(ci_testing_partition_id) + end + end + end end diff --git a/spec/models/ci/build_pending_state_spec.rb b/spec/models/ci/build_pending_state_spec.rb index a546d2aff65..756180621ec 100644 --- a/spec/models/ci/build_pending_state_spec.rb +++ b/spec/models/ci/build_pending_state_spec.rb @@ -24,4 +24,33 @@ RSpec.describe Ci::BuildPendingState do end end end + + describe 'partitioning' do + context 'with build' do + let(:build) { FactoryBot.build(:ci_build, partition_id: ci_testing_partition_id) } + let(:build_pending_state) { FactoryBot.build(:ci_build_pending_state, build: build) } + + it 'sets partition_id to the current partition value' do + expect { build_pending_state.valid? }.to change { build_pending_state.partition_id }.to(ci_testing_partition_id) + end + + context 'when it is already set' do + let(:build_pending_state) { FactoryBot.build(:ci_build_pending_state, partition_id: 125) } + + it 'does not change the partition_id value' do + expect { build_pending_state.valid? }.not_to change { build_pending_state.partition_id } + end + end + end + + context 'without build' do + let(:build_pending_state) { FactoryBot.build(:ci_build_pending_state, build: nil) } + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { build_pending_state.valid? }.not_to change { build_pending_state.partition_id } + end + end + end end diff --git a/spec/models/ci/build_report_result_spec.rb b/spec/models/ci/build_report_result_spec.rb index 09ea19cf077..90b23d3e824 100644 --- a/spec/models/ci/build_report_result_spec.rb +++ b/spec/models/ci/build_report_result_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::BuildReportResult do - let(:build_report_result) { build(:ci_build_report_result, :with_junit_success) } + let_it_be_with_reload(:build_report_result) { create(:ci_build_report_result, :with_junit_success) } it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:project) } @@ -70,4 +70,34 @@ RSpec.describe Ci::BuildReportResult do expect(build_report_result.tests_skipped).to eq(0) end end + + describe 'partitioning' do + let(:build_report_result) { FactoryBot.build(:ci_build_report_result, build: build) } + + context 'with build' do + let(:build) { FactoryBot.build(:ci_build, partition_id: ci_testing_partition_id) } + + it 'copies the partition_id from build' do + expect { build_report_result.valid? }.to change { build_report_result.partition_id }.to(ci_testing_partition_id) + end + + context 'when it is already set' do + let(:build_report_result) { FactoryBot.build(:ci_build_report_result, partition_id: 125) } + + it 'does not change the partition_id value' do + expect { build_report_result.valid? }.not_to change { build_report_result.partition_id } + end + end + end + + context 'without build' do + subject(:build_report_result) { FactoryBot.build(:ci_build_report_result, build: nil, partition_id: 125) } + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { build_report_result.valid? }.not_to change { build_report_result.partition_id } + end + end + end end diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb index 8dfe854511c..5e1a489ed8b 100644 --- a/spec/models/ci/build_runner_session_spec.rb +++ b/spec/models/ci/build_runner_session_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::BuildRunnerSession, model: true do +RSpec.describe Ci::BuildRunnerSession, model: true, feature_category: :continuous_integration do let!(:build) { create(:ci_build, :with_runner_session) } let(:url) { 'https://new.example.com' } @@ -174,4 +174,20 @@ RSpec.describe Ci::BuildRunnerSession, model: true do end end end + + describe 'partitioning' do + include Ci::PartitioningHelpers + + let(:new_pipeline) { create(:ci_pipeline) } + let(:new_build) { create(:ci_build, pipeline: new_pipeline) } + let(:build_runner_session) { create(:ci_build_runner_session, build: new_build) } + + before do + stub_current_partition_id + end + + it 'assigns the same partition id as the one that build has' do + expect(build_runner_session.partition_id).to eq(ci_testing_partition_id) + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 813b4b3faa6..c978e33bf54 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Build do +RSpec.describe Ci::Build, feature_category: :continuous_integration do include Ci::TemplateHelpers include AfterNextHelpers @@ -1754,8 +1754,8 @@ RSpec.describe Ci::Build do end end - describe '#starts_environment?' do - subject { build.starts_environment? } + describe '#deployment_job?' do + subject { build.deployment_job? } context 'when environment is defined' do before do @@ -2528,20 +2528,24 @@ RSpec.describe Ci::Build do end describe '#ref_slug' do - { - 'master' => 'master', - '1-foo' => '1-foo', - 'fix/1-foo' => 'fix-1-foo', - 'fix-1-foo' => 'fix-1-foo', - 'a' * 63 => 'a' * 63, - 'a' * 64 => 'a' * 63, - 'FOO' => 'foo', - '-' + 'a' * 61 + '-' => 'a' * 61, - '-' + 'a' * 62 + '-' => 'a' * 62, - '-' + 'a' * 63 + '-' => 'a' * 62, - 'a' * 62 + ' ' => 'a' * 62 - }.each do |ref, slug| - it "transforms #{ref} to #{slug}" do + using RSpec::Parameterized::TableSyntax + + where(:ref, :slug) do + 'master' | 'master' + '1-foo' | '1-foo' + 'fix/1-foo' | 'fix-1-foo' + 'fix-1-foo' | 'fix-1-foo' + 'a' * 63 | 'a' * 63 + 'a' * 64 | 'a' * 63 + 'FOO' | 'foo' + '-' + 'a' * 61 + '-' | 'a' * 61 + '-' + 'a' * 62 + '-' | 'a' * 62 + '-' + 'a' * 63 + '-' | 'a' * 62 + 'a' * 62 + ' ' | 'a' * 62 + end + + with_them do + it "transforms ref to slug" do build.ref = ref expect(build.ref_slug).to eq(slug) @@ -2737,6 +2741,7 @@ RSpec.describe Ci::Build do { key: 'CI_PROJECT_PATH', value: project.full_path, public: true, masked: false }, { key: 'CI_PROJECT_PATH_SLUG', value: project.full_path_slug, public: true, masked: false }, { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true, masked: false }, + { key: 'CI_PROJECT_NAMESPACE_ID', value: project.namespace.id.to_s, public: true, masked: false }, { key: 'CI_PROJECT_ROOT_NAMESPACE', value: project.namespace.root_ancestor.path, public: true, masked: false }, { key: 'CI_PROJECT_URL', value: project.web_url, public: true, masked: false }, { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true, masked: false }, @@ -2883,6 +2888,9 @@ RSpec.describe Ci::Build do value: 'var', public: true }] build.environment = 'staging' + + # CI_ENVIRONMENT_NAME is set in predefined_variables when job environment is provided + predefined_variables.insert(20, { key: 'CI_ENVIRONMENT_NAME', value: 'staging', public: true, masked: false }) end it 'matches explicit variables ordering' do @@ -3539,8 +3547,8 @@ RSpec.describe Ci::Build do rsa_key = OpenSSL::PKey::RSA.generate(3072).to_s stub_application_setting(ci_jwt_signing_key: rsa_key) build.metadata.update!(id_tokens: { - 'ID_TOKEN_1' => { id_token: { aud: 'developers' } }, - 'ID_TOKEN_2' => { id_token: { aud: 'maintainers' } } + 'ID_TOKEN_1' => { aud: 'developers' }, + 'ID_TOKEN_2' => { aud: 'maintainers' } }) end @@ -3817,22 +3825,6 @@ RSpec.describe Ci::Build do it 'assigns the token' do expect { build.enqueue }.to change(build, :token).from(nil).to(an_instance_of(String)) end - - context 'with ci_assign_job_token_on_scheduling disabled' do - before do - stub_feature_flags(ci_assign_job_token_on_scheduling: false) - end - - it 'assigns the token on creation' do - expect(build.token).to be_present - end - - it 'does not change the token when enqueuing' do - expect { build.enqueue }.not_to change(build, :token) - - expect(build).to be_pending - end - end end describe 'state transition: pending: :running' do @@ -5442,7 +5434,7 @@ RSpec.describe Ci::Build do it 'delegates to Ci::BuildTraceMetadata' do expect(Ci::BuildTraceMetadata) .to receive(:find_or_upsert_for!) - .with(build.id) + .with(build.id, build.partition_id) build.ensure_trace_metadata! end @@ -5617,4 +5609,72 @@ RSpec.describe Ci::Build do end end end + + describe '#runtime_hooks' do + let(:build1) do + FactoryBot.build(:ci_build, + options: { hooks: { pre_get_sources_script: ["echo 'hello pre_get_sources_script'"] } }) + end + + subject(:runtime_hooks) { build1.runtime_hooks } + + it 'returns an array of hook objects' do + expect(runtime_hooks.size).to eq(1) + expect(runtime_hooks[0].name).to eq('pre_get_sources_script') + expect(runtime_hooks[0].script).to eq(["echo 'hello pre_get_sources_script'"]) + end + end + + describe 'partitioning', :ci_partitionable do + include Ci::PartitioningHelpers + + let(:new_pipeline) { create(:ci_pipeline) } + let(:ci_build) { FactoryBot.build(:ci_build, pipeline: new_pipeline) } + + before do + stub_current_partition_id + end + + it 'assigns partition_id to job variables successfully', :aggregate_failures do + ci_build.job_variables_attributes = [ + { key: 'TEST_KEY', value: 'new value' }, + { key: 'NEW_KEY', value: 'exciting new value' } + ] + + ci_build.save! + + expect(ci_build.job_variables.count).to eq(2) + expect(ci_build.job_variables.first.partition_id).to eq(ci_testing_partition_id) + expect(ci_build.job_variables.second.partition_id).to eq(ci_testing_partition_id) + end + end + + describe 'assigning token', :ci_partitionable do + include Ci::PartitioningHelpers + + let(:new_pipeline) { create(:ci_pipeline) } + let(:ci_build) { create(:ci_build, pipeline: new_pipeline) } + + before do + stub_current_partition_id + end + + it 'includes partition_id as a token prefix' do + prefix = ci_build.token.split('_').first.to_i(16) + + expect(prefix).to eq(ci_testing_partition_id) + end + + context 'when ci_build_partition_id_token_prefix is disabled' do + before do + stub_feature_flags(ci_build_partition_id_token_prefix: false) + end + + it 'does not include partition_id as a token prefix' do + prefix = ci_build.token.split('_').first.to_i(16) + + expect(prefix).not_to eq(ci_testing_partition_id) + end + end + end end diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 3328ed62f15..ac0a18a176d 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -15,13 +15,13 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git described_class.new(build: build, chunk_index: chunk_index, data_store: data_store, raw_data: raw_data) end - it_behaves_like 'having unique enum values' - before do stub_feature_flags(ci_enable_live_trace: true) stub_artifacts_object_storage end + it_behaves_like 'having unique enum values' + def redis_instance { redis: Gitlab::Redis::SharedState, @@ -954,4 +954,33 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git it { is_expected.to eq(value) } end end + + describe 'partitioning' do + context 'with build' do + let(:build) { FactoryBot.build(:ci_build, partition_id: ci_testing_partition_id) } + let(:build_trace_chunk) { FactoryBot.build(:ci_build_trace_chunk, build: build) } + + it 'sets partition_id to the current partition value' do + expect { build_trace_chunk.valid? }.to change { build_trace_chunk.partition_id }.to(ci_testing_partition_id) + end + + context 'when it is already set' do + let(:build_trace_chunk) { FactoryBot.build(:ci_build_trace_chunk, partition_id: 125) } + + it 'does not change the partition_id value' do + expect { build_trace_chunk.valid? }.not_to change { build_trace_chunk.partition_id } + end + end + end + + context 'without build' do + let(:build_trace_chunk) { FactoryBot.build(:ci_build_trace_chunk, build: nil, partition_id: 125) } + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { build_trace_chunk.valid? }.not_to change { build_trace_chunk.partition_id } + end + end + end end diff --git a/spec/models/ci/build_trace_metadata_spec.rb b/spec/models/ci/build_trace_metadata_spec.rb index 120e4289da2..2ab300e4054 100644 --- a/spec/models/ci/build_trace_metadata_spec.rb +++ b/spec/models/ci/build_trace_metadata_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::BuildTraceMetadata do +RSpec.describe Ci::BuildTraceMetadata, feature_category: :continuous_integration do it { is_expected.to belong_to(:build) } it { is_expected.to belong_to(:trace_artifact) } @@ -106,7 +106,7 @@ RSpec.describe Ci::BuildTraceMetadata do let_it_be(:build) { create(:ci_build) } subject(:execute) do - described_class.find_or_upsert_for!(build.id) + described_class.find_or_upsert_for!(build.id, build.partition_id) end it 'creates a new record' do @@ -158,4 +158,22 @@ RSpec.describe Ci::BuildTraceMetadata do it { is_expected.to eq(result) } end end + + describe 'partitioning' do + include Ci::PartitioningHelpers + + let_it_be(:pipeline) { create(:ci_pipeline) } + let_it_be(:build) { create(:ci_build, pipeline: pipeline) } + let(:new_pipeline) { create(:ci_pipeline) } + let(:new_build) { create(:ci_build, pipeline: new_pipeline) } + let(:metadata) { create(:ci_build_trace_metadata, build: new_build) } + + before do + stub_current_partition_id + end + + it 'assigns the same partition id as the one that build has' do + expect(metadata.partition_id).to eq(ci_testing_partition_id) + end + end end diff --git a/spec/models/ci/freeze_period_spec.rb b/spec/models/ci/freeze_period_spec.rb index b9bf1657e28..d8add736d6a 100644 --- a/spec/models/ci/freeze_period_spec.rb +++ b/spec/models/ci/freeze_period_spec.rb @@ -2,16 +2,22 @@ require 'spec_helper' -RSpec.describe Ci::FreezePeriod, type: :model do +RSpec.describe Ci::FreezePeriod, feature_category: :release_orchestration, type: :model do + let_it_be(:project) { create(:project) } + + # Freeze period factory is on a weekend, so we travel in time, in and around that. + let(:friday_2300_time) { Time.utc(2020, 4, 10, 23, 0) } + let(:saturday_1200_time) { Time.utc(2020, 4, 11, 12, 0) } + let(:monday_0700_time) { Time.utc(2020, 4, 13, 7, 0) } + let(:tuesday_0800_time) { Time.utc(2020, 4, 14, 8, 0) } + subject { build(:ci_freeze_period) } it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:project) } - let!(:model) { create(:ci_freeze_period, project: parent) } + let!(:model) { create(:ci_freeze_period, project: parent) } end - let(:invalid_cron) { '0 0 0 * *' } - it { is_expected.to belong_to(:project) } it { is_expected.to respond_to(:freeze_start) } @@ -19,37 +25,142 @@ RSpec.describe Ci::FreezePeriod, type: :model do it { is_expected.to respond_to(:cron_timezone) } describe 'cron validations' do + let(:invalid_cron) { '0 0 0 * *' } + it 'allows valid cron patterns' do - freeze_period = build(:ci_freeze_period) + freeze_period = build_stubbed(:ci_freeze_period) expect(freeze_period).to be_valid end it 'does not allow invalid cron patterns on freeze_start' do - freeze_period = build(:ci_freeze_period, freeze_start: invalid_cron) + freeze_period = build_stubbed(:ci_freeze_period, freeze_start: invalid_cron) expect(freeze_period).not_to be_valid end it 'does not allow invalid cron patterns on freeze_end' do - freeze_period = build(:ci_freeze_period, freeze_end: invalid_cron) + freeze_period = build_stubbed(:ci_freeze_period, freeze_end: invalid_cron) expect(freeze_period).not_to be_valid end it 'does not allow an invalid timezone' do - freeze_period = build(:ci_freeze_period, cron_timezone: 'invalid') + freeze_period = build_stubbed(:ci_freeze_period, cron_timezone: 'invalid') expect(freeze_period).not_to be_valid end context 'when cron contains trailing whitespaces' do it 'strips the attribute' do - freeze_period = build(:ci_freeze_period, freeze_start: ' 0 0 * * * ') + freeze_period = build_stubbed(:ci_freeze_period, freeze_start: ' 0 0 * * * ') expect(freeze_period).to be_valid expect(freeze_period.freeze_start).to eq('0 0 * * *') end end end + + shared_examples 'within freeze period' do |time| + it 'is frozen' do + travel_to(time) do + expect(subject).to eq(Ci::FreezePeriod::STATUS_ACTIVE) + end + end + end + + shared_examples 'outside freeze period' do |time| + it 'is not frozen' do + travel_to(time) do + expect(subject).to eq(Ci::FreezePeriod::STATUS_INACTIVE) + end + end + end + + describe '#status' do + subject { freeze_period.status } + + describe 'single freeze period' do + let(:freeze_period) do + build_stubbed(:ci_freeze_period, project: project) + end + + it_behaves_like 'outside freeze period', Time.utc(2020, 4, 10, 22, 59) + it_behaves_like 'within freeze period', Time.utc(2020, 4, 10, 23, 1) + it_behaves_like 'within freeze period', Time.utc(2020, 4, 13, 6, 59) + it_behaves_like 'outside freeze period', Time.utc(2020, 4, 13, 7, 1) + end + + # See https://gitlab.com/gitlab-org/gitlab/-/issues/370472 + context 'when period overlaps with itself' do + let(:freeze_period) do + build_stubbed(:ci_freeze_period, project: project, freeze_start: '* * * 8 *', freeze_end: '* * * 10 *') + end + + it_behaves_like 'within freeze period', Time.utc(2020, 8, 11, 0, 0) + it_behaves_like 'outside freeze period', Time.utc(2020, 10, 11, 0, 0) + end + end + + shared_examples 'a freeze period method' do + let(:freeze_period) { build_stubbed(:ci_freeze_period, project: project) } + + it 'returns the correct value' do + travel_to(now) do + expect(freeze_period.send(method)).to eq(expected) + end + end + end + + describe '#active?' do + context 'when freeze period status is active' do + it_behaves_like 'a freeze period method' do + let(:now) { saturday_1200_time } + let(:method) { :active? } + let(:expected) { true } + end + end + + context 'when freeze period status is inactive' do + it_behaves_like 'a freeze period method' do + let(:now) { tuesday_0800_time } + let(:method) { :active? } + let(:expected) { false } + end + end + end + + describe '#time_start' do + it_behaves_like 'a freeze period method' do + let(:now) { monday_0700_time } + let(:method) { :time_start } + let(:expected) { friday_2300_time } + end + end + + describe '#next_time_start' do + let(:next_friday_2300_time) { Time.utc(2020, 4, 17, 23, 0) } + + it_behaves_like 'a freeze period method' do + let(:now) { monday_0700_time } + let(:method) { :next_time_start } + let(:expected) { next_friday_2300_time } + end + end + + describe '#time_end_from_now' do + it_behaves_like 'a freeze period method' do + let(:now) { saturday_1200_time } + let(:method) { :time_end_from_now } + let(:expected) { monday_0700_time } + end + end + + describe '#time_end_from_start' do + it_behaves_like 'a freeze period method' do + let(:now) { saturday_1200_time } + let(:method) { :time_end_from_start } + let(:expected) { monday_0700_time } + end + end end diff --git a/spec/models/ci/freeze_period_status_spec.rb b/spec/models/ci/freeze_period_status_spec.rb deleted file mode 100644 index ecbb7af64f7..00000000000 --- a/spec/models/ci/freeze_period_status_spec.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true -require 'spec_helper' - -RSpec.describe Ci::FreezePeriodStatus do - let(:project) { create :project } - # '0 23 * * 5' == "At 23:00 on Friday."", '0 7 * * 1' == "At 07:00 on Monday."" - let(:friday_2300) { '0 23 * * 5' } - let(:monday_0700) { '0 7 * * 1' } - - subject { described_class.new(project: project).execute } - - shared_examples 'within freeze period' do |time| - it 'is frozen' do - travel_to(time) do - expect(subject).to be_truthy - end - end - end - - shared_examples 'outside freeze period' do |time| - it 'is not frozen' do - travel_to(time) do - expect(subject).to be_falsy - end - end - end - - describe 'single freeze period' do - let!(:freeze_period) { create(:ci_freeze_period, project: project, freeze_start: friday_2300, freeze_end: monday_0700) } - - it_behaves_like 'outside freeze period', Time.utc(2020, 4, 10, 22, 59) - - it_behaves_like 'within freeze period', Time.utc(2020, 4, 10, 23, 1) - - it_behaves_like 'within freeze period', Time.utc(2020, 4, 13, 6, 59) - - it_behaves_like 'outside freeze period', Time.utc(2020, 4, 13, 7, 1) - end - - describe 'multiple freeze periods' do - # '30 23 * * 5' == "At 23:30 on Friday."", '0 8 * * 1' == "At 08:00 on Monday."" - let(:friday_2330) { '30 23 * * 5' } - let(:monday_0800) { '0 8 * * 1' } - - let!(:freeze_period_1) { create(:ci_freeze_period, project: project, freeze_start: friday_2300, freeze_end: monday_0700) } - let!(:freeze_period_2) { create(:ci_freeze_period, project: project, freeze_start: friday_2330, freeze_end: monday_0800) } - - it_behaves_like 'outside freeze period', Time.utc(2020, 4, 10, 22, 59) - - it_behaves_like 'within freeze period', Time.utc(2020, 4, 10, 23, 29) - - it_behaves_like 'within freeze period', Time.utc(2020, 4, 11, 10, 0) - - it_behaves_like 'within freeze period', Time.utc(2020, 4, 10, 23, 1) - - it_behaves_like 'within freeze period', Time.utc(2020, 4, 13, 6, 59) - - it_behaves_like 'within freeze period', Time.utc(2020, 4, 13, 7, 59) - - it_behaves_like 'outside freeze period', Time.utc(2020, 4, 13, 8, 1) - end - - # https://gitlab.com/gitlab-org/gitlab/-/issues/370472 - context 'when period overlaps with itself' do - let!(:freeze_period) { create(:ci_freeze_period, project: project, freeze_start: '* * * 8 *', freeze_end: '* * * 10 *') } - - it_behaves_like 'within freeze period', Time.utc(2020, 8, 11, 0, 0) - - it_behaves_like 'outside freeze period', Time.utc(2020, 10, 11, 0, 0) - end -end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 098f8bd4514..18aaab1d1f3 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -356,50 +356,6 @@ RSpec.describe Ci::JobArtifact do end end - describe 'callbacks' do - describe '#schedule_background_upload' do - subject { create(:ci_job_artifact, :archive) } - - context 'when object storage is disabled' do - before do - stub_artifacts_object_storage(enabled: false) - end - - it 'does not schedule the migration' do - expect(ObjectStorage::BackgroundMoveWorker).not_to receive(:perform_async) - - subject - end - end - - context 'when object storage is enabled' do - context 'when background upload is enabled' do - before do - stub_artifacts_object_storage(background_upload: true) - end - - it 'schedules the model for migration' do - expect(ObjectStorage::BackgroundMoveWorker).to receive(:perform_async).with('JobArtifactUploader', described_class.name, :file, kind_of(Numeric)) - - subject - end - end - - context 'when background upload is disabled' do - before do - stub_artifacts_object_storage(background_upload: false) - end - - it 'schedules the model for migration' do - expect(ObjectStorage::BackgroundMoveWorker).not_to receive(:perform_async) - - subject - end - end - end - end - end - context 'creating the artifact' do let(:project) { create(:project) } let(:artifact) { create(:ci_job_artifact, :archive, project: project) } diff --git a/spec/models/ci/job_token/allowlist_spec.rb b/spec/models/ci/job_token/allowlist_spec.rb new file mode 100644 index 00000000000..45083d64393 --- /dev/null +++ b/spec/models/ci/job_token/allowlist_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::JobToken::Allowlist, feature_category: :continuous_integration do + using RSpec::Parameterized::TableSyntax + + let_it_be(:source_project) { create(:project) } + + let(:allowlist) { described_class.new(source_project, direction: direction) } + let(:direction) { :outbound } + + describe '#projects' do + subject(:projects) { allowlist.projects } + + context 'when no projects are added to the scope' do + [:inbound, :outbound].each do |d| + let(:direction) { d } + + it 'returns the project defining the scope' do + expect(projects).to contain_exactly(source_project) + end + end + end + + context 'when projects are added to the scope' do + include_context 'with scoped projects' + + where(:direction, :additional_project) do + :outbound | ref(:outbound_scoped_project) + :inbound | ref(:inbound_scoped_project) + end + + with_them do + it 'returns all projects that can be accessed from a given scope' do + expect(projects).to contain_exactly(source_project, additional_project) + end + end + end + end + + describe '#includes?' do + subject { allowlist.includes?(includes_project) } + + context 'without scoped projects' do + let(:unscoped_project) { build(:project) } + + where(:includes_project, :direction, :result) do + ref(:source_project) | :outbound | false + ref(:source_project) | :inbound | false + ref(:unscoped_project) | :outbound | false + ref(:unscoped_project) | :inbound | false + end + + with_them do + it { is_expected.to be result } + end + end + + context 'with scoped projects' do + include_context 'with scoped projects' + + where(:includes_project, :direction, :result) do + ref(:source_project) | :outbound | false + ref(:source_project) | :inbound | false + ref(:inbound_scoped_project) | :outbound | false + ref(:inbound_scoped_project) | :inbound | true + ref(:outbound_scoped_project) | :outbound | true + ref(:outbound_scoped_project) | :inbound | false + ref(:unscoped_project1) | :outbound | false + ref(:unscoped_project1) | :inbound | false + ref(:unscoped_project2) | :outbound | false + ref(:unscoped_project2) | :inbound | false + end + + with_them do + it { is_expected.to be result } + end + end + end +end diff --git a/spec/models/ci/job_token/project_scope_link_spec.rb b/spec/models/ci/job_token/project_scope_link_spec.rb index 92ed86b55b2..91491733c44 100644 --- a/spec/models/ci/job_token/project_scope_link_spec.rb +++ b/spec/models/ci/job_token/project_scope_link_spec.rb @@ -2,14 +2,14 @@ require 'spec_helper' -RSpec.describe Ci::JobToken::ProjectScopeLink do +RSpec.describe Ci::JobToken::ProjectScopeLink, feature_category: :continuous_integration do + let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + it { is_expected.to belong_to(:source_project) } it { is_expected.to belong_to(:target_project) } it { is_expected.to belong_to(:added_by) } - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project) } - it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:user) } let!(:model) { create(:ci_job_token_project_scope_link, added_by: parent) } @@ -50,8 +50,8 @@ RSpec.describe Ci::JobToken::ProjectScopeLink do end end - describe '.from_project' do - subject { described_class.from_project(project) } + describe '.with_source' do + subject { described_class.with_source(project) } let!(:source_link) { create(:ci_job_token_project_scope_link, source_project: project) } let!(:target_link) { create(:ci_job_token_project_scope_link, target_project: project) } @@ -61,8 +61,8 @@ RSpec.describe Ci::JobToken::ProjectScopeLink do end end - describe '.to_project' do - subject { described_class.to_project(project) } + describe '.with_target' do + subject { described_class.with_target(project) } let!(:source_link) { create(:ci_job_token_project_scope_link, source_project: project) } let!(:target_link) { create(:ci_job_token_project_scope_link, target_project: project) } diff --git a/spec/models/ci/job_token/scope_spec.rb b/spec/models/ci/job_token/scope_spec.rb index 1e3f6d044d2..37c56973506 100644 --- a/spec/models/ci/job_token/scope_spec.rb +++ b/spec/models/ci/job_token/scope_spec.rb @@ -2,58 +2,72 @@ require 'spec_helper' -RSpec.describe Ci::JobToken::Scope do - let_it_be(:project) { create(:project, ci_outbound_job_token_scope_enabled: true).tap(&:save!) } +RSpec.describe Ci::JobToken::Scope, feature_category: :continuous_integration do + let_it_be(:source_project) { create(:project, ci_outbound_job_token_scope_enabled: true) } - let(:scope) { described_class.new(project) } + let(:scope) { described_class.new(source_project) } describe '#all_projects' do subject(:all_projects) { scope.all_projects } context 'when no projects are added to the scope' do it 'returns the project defining the scope' do - expect(all_projects).to contain_exactly(project) + expect(all_projects).to contain_exactly(source_project) end end - context 'when other projects are added to the scope' do - let_it_be(:scoped_project) { create(:project) } - let_it_be(:unscoped_project) { create(:project) } - - let!(:link_in_scope) { create(:ci_job_token_project_scope_link, source_project: project, target_project: scoped_project) } - let!(:link_out_of_scope) { create(:ci_job_token_project_scope_link, target_project: unscoped_project) } + context 'when projects are added to the scope' do + include_context 'with scoped projects' it 'returns all projects that can be accessed from a given scope' do - expect(subject).to contain_exactly(project, scoped_project) + expect(subject).to contain_exactly(source_project, outbound_scoped_project) end end end - describe '#includes?' do - subject { scope.includes?(target_project) } + describe '#allows?' do + subject { scope.allows?(includes_project) } - context 'when param is the project defining the scope' do - let(:target_project) { project } + context 'without scoped projects' do + context 'when self referential' do + let(:includes_project) { source_project } - it { is_expected.to be_truthy } + it { is_expected.to be_truthy } + end end - context 'when param is a project in scope' do - let(:target_link) { create(:ci_job_token_project_scope_link, source_project: project) } - let(:target_project) { target_link.target_project } + context 'with scoped projects' do + include_context 'with scoped projects' - it { is_expected.to be_truthy } - end + context 'when project is in outbound scope' do + let(:includes_project) { outbound_scoped_project } - context 'when param is a project in another scope' do - let(:scope_link) { create(:ci_job_token_project_scope_link) } - let(:target_project) { scope_link.target_project } + it { is_expected.to be_truthy } + end + + context 'when project is in inbound scope' do + let(:includes_project) { inbound_scoped_project } + + it { is_expected.to be_falsey } + end - it { is_expected.to be_falsey } + context 'when project is linked to a different project' do + let(:includes_project) { unscoped_project1 } + + it { is_expected.to be_falsey } + end + + context 'when project is unlinked to a project' do + let(:includes_project) { unscoped_project2 } + + it { is_expected.to be_falsey } + end context 'when project scope setting is disabled' do + let(:includes_project) { unscoped_project1 } + before do - project.ci_outbound_job_token_scope_enabled = false + source_project.ci_outbound_job_token_scope_enabled = false end it 'considers any project to be part of the scope' do diff --git a/spec/models/ci/job_variable_spec.rb b/spec/models/ci/job_variable_spec.rb index 4aebd3283f0..0a65708160a 100644 --- a/spec/models/ci/job_variable_spec.rb +++ b/spec/models/ci/job_variable_spec.rb @@ -2,11 +2,63 @@ require 'spec_helper' -RSpec.describe Ci::JobVariable do - subject { build(:ci_job_variable) } - +RSpec.describe Ci::JobVariable, feature_category: :continuous_integration do it_behaves_like "CI variable" - it { is_expected.to belong_to(:job) } - it { is_expected.to validate_uniqueness_of(:key).scoped_to(:job_id) } + describe 'associations' do + let!(:job_variable) { create(:ci_job_variable) } + + it { is_expected.to belong_to(:job) } + it { is_expected.to validate_uniqueness_of(:key).scoped_to(:job_id) } + end + + describe 'partitioning' do + let(:job_variable) { build(:ci_job_variable, job: ci_build) } + + context 'with build' do + let(:ci_build) { build(:ci_build, partition_id: ci_testing_partition_id) } + + it 'copies the partition_id from build' do + expect { job_variable.valid? }.to change { job_variable.partition_id }.to(ci_testing_partition_id) + end + + context 'when it is already set' do + let(:job_variable) { build(:ci_job_variable, partition_id: 125) } + + it 'does not change the partition_id value' do + expect { job_variable.valid? }.not_to change { job_variable.partition_id } + end + end + end + + context 'without build' do + subject(:job_variable) { build(:ci_job_variable, job: nil, partition_id: 125) } + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { job_variable.valid? }.not_to change { job_variable.partition_id } + end + end + + context 'when using bulk_insert', :ci_partitionable do + include Ci::PartitioningHelpers + + let(:new_pipeline) { create(:ci_pipeline) } + let(:ci_build) { create(:ci_build, pipeline: new_pipeline) } + let(:job_variable_2) { build(:ci_job_variable, job: ci_build) } + + before do + stub_current_partition_id + end + + it 'creates job variables successfully', :aggregate_failures do + described_class.bulk_insert!([job_variable, job_variable_2]) + + expect(described_class.count).to eq(2) + expect(described_class.first.partition_id).to eq(ci_testing_partition_id) + expect(described_class.last.partition_id).to eq(ci_testing_partition_id) + end + end + end end diff --git a/spec/models/ci/pending_build_spec.rb b/spec/models/ci/pending_build_spec.rb index 4bb43233dbd..331522070df 100644 --- a/spec/models/ci/pending_build_spec.rb +++ b/spec/models/ci/pending_build_spec.rb @@ -196,6 +196,28 @@ RSpec.describe Ci::PendingBuild do end end + describe 'partitioning', :ci_partitionable do + include Ci::PartitioningHelpers + + before do + stub_current_partition_id + end + + let(:new_pipeline ) { create(:ci_pipeline, project: pipeline.project) } + let(:new_build) { create(:ci_build, pipeline: new_pipeline) } + + it 'assigns the same partition id as the one that build has', :aggregate_failures do + expect(new_build.partition_id).to eq ci_testing_partition_id + expect(new_build.partition_id).not_to eq pipeline.partition_id + + described_class.upsert_from_build!(build) + described_class.upsert_from_build!(new_build) + + expect(build.reload.queuing_entry.partition_id).to eq pipeline.partition_id + expect(new_build.reload.queuing_entry.partition_id).to eq ci_testing_partition_id + end + end + it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:namespace) } let!(:model) { create(:ci_pending_build, namespace: parent) } diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index b28b61e2b39..9b70f7c2839 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::PipelineSchedule do +RSpec.describe Ci::PipelineSchedule, feature_category: :continuous_integration do let_it_be_with_reload(:project) { create_default(:project) } subject { build(:ci_pipeline_schedule) } @@ -41,6 +41,12 @@ RSpec.describe Ci::PipelineSchedule do expect(pipeline_schedule).not_to be_valid end + it 'does not allow empty variable key' do + pipeline_schedule = build(:ci_pipeline_schedule, variables_attributes: [{ secret_value: 'test_value' }]) + + expect(pipeline_schedule).not_to be_valid + end + context 'when active is false' do it 'does not allow nullified ref' do pipeline_schedule = build(:ci_pipeline_schedule, :inactive, ref: nil) @@ -110,48 +116,18 @@ RSpec.describe Ci::PipelineSchedule do end describe '#set_next_run_at' do - using RSpec::Parameterized::TableSyntax - - where(:worker_cron, :schedule_cron, :plan_limit, :now, :result) do - '0 1 2 3 *' | '0 1 * * *' | nil | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0) - '0 1 2 3 *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0) - '*/5 * * * *' | '*/1 * * * *' | nil | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5) - '*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) - '*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 10).to_i | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 10) - '*/5 * * * *' | '*/1 * * * *' | 200 | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 10) - '*/5 * * * *' | '0 * * * *' | nil | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5) - '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 10).to_i | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) - '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) - '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 2.hours.in_minutes).to_i | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5) - '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) - '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 10).to_i | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) - '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 8).to_i | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) - '*/5 * * * *' | '0 1 1 * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 1, 1, 0) | Time.zone.local(2021, 6, 1, 1, 0) - '*/9 * * * *' | '0 1 1 * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 1, 1, 9) | Time.zone.local(2021, 6, 1, 1, 0) - '*/5 * * * *' | '59 14 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 1, 15, 0) | Time.zone.local(2021, 5, 2, 15, 0) - '*/5 * * * *' | '45 21 1 2 *' | (1.day.in_minutes / 5).to_i | Time.zone.local(2021, 2, 1, 21, 45) | Time.zone.local(2022, 2, 1, 21, 45) - end + let(:now) { Time.zone.local(2021, 3, 2, 1, 0) } + let(:pipeline_schedule) { create(:ci_pipeline_schedule, cron: "0 1 * * *") } - with_them do - let(:pipeline_schedule) { create(:ci_pipeline_schedule, cron: schedule_cron) } + it 'calls fallback method next_run_at if there is no plan limit' do + allow(Settings).to receive(:cron_jobs).and_return({ 'pipeline_schedule_worker' => { 'cron' => "0 1 2 3 *" } }) - before do - allow(Settings).to receive(:cron_jobs) do - { 'pipeline_schedule_worker' => { 'cron' => worker_cron } } - end + travel_to(now) do + expect(pipeline_schedule).to receive(:calculate_next_run_at).and_call_original - create(:plan_limits, :default_plan, ci_daily_pipeline_schedule_triggers: plan_limit) if plan_limit + pipeline_schedule.set_next_run_at - # Setting this here to override initial save with the current time - pipeline_schedule.next_run_at = now - end - - it 'updates next_run_at' do - travel_to(now) do - pipeline_schedule.set_next_run_at - - expect(pipeline_schedule.next_run_at).to eq(result) - end + expect(pipeline_schedule.next_run_at).to eq(Time.zone.local(2022, 3, 2, 1, 0)) end end @@ -288,6 +264,17 @@ RSpec.describe Ci::PipelineSchedule do end end + describe '#worker_cron' do + before do + allow(Settings).to receive(:cron_jobs) + .and_return({ pipeline_schedule_worker: { cron: "* 1 2 3 4" } }.with_indifferent_access) + end + + it "returns cron expression set in Settings" do + expect(subject.worker_cron_expression).to eq("* 1 2 3 4") + end + end + context 'loose foreign key on ci_pipeline_schedules.project_id' do it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:project) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 2c945898e61..b72693d9994 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -219,6 +219,29 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '.for_name' do + subject { described_class.for_name(name) } + + let_it_be(:pipeline1) { create(:ci_pipeline, name: 'Build pipeline') } + let_it_be(:pipeline2) { create(:ci_pipeline, name: 'Chatops pipeline') } + + context 'when name exists' do + let(:name) { 'build Pipeline' } + + it 'performs case insensitive compare' do + is_expected.to contain_exactly(pipeline1) + end + end + + context 'when name does not exist' do + let(:name) { 'absent-name' } + + it 'returns empty' do + is_expected.to be_empty + end + end + end + describe '.created_after' do let_it_be(:old_pipeline) { create(:ci_pipeline, created_at: 1.week.ago) } let_it_be(:pipeline) { create(:ci_pipeline) } @@ -5287,6 +5310,20 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end end + + context 'when the current user is not the bridge user' do + let(:current_user) { create(:user) } + + before do + project.add_maintainer(current_user) + end + + it 'changes bridge user to current user' do + expect { reset_bridge } + .to change { bridge.reload.user } + .from(owner).to(current_user) + end + end end context 'when the user does not have permissions for the processable' do @@ -5305,6 +5342,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do .and not_change { bridge_dependant_dag_job.reload.status } end end + + context 'when the current user is not the bridge user' do + let(:current_user) { create(:user) } + + it 'does not change bridge user' do + expect { reset_bridge } + .to not_change { bridge.reload.user } + end + end end end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index e62e5f84a6d..07fac4ee2f7 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -73,6 +73,7 @@ RSpec.describe Ci::Processable do job_artifacts_network_referee job_artifacts_dotenv job_artifacts_cobertura needs job_artifacts_accessibility job_artifacts_requirements job_artifacts_coverage_fuzzing + job_artifacts_requirements_v2 job_artifacts_api_fuzzing terraform_state_versions job_artifacts_cyclonedx].freeze end @@ -423,8 +424,8 @@ RSpec.describe Ci::Processable do it 'returns all needs attributes' do is_expected.to contain_exactly( - { 'artifacts' => true, 'name' => 'test1', 'optional' => false }, - { 'artifacts' => true, 'name' => 'test2', 'optional' => false } + { 'artifacts' => true, 'name' => 'test1', 'optional' => false, 'partition_id' => build.partition_id }, + { 'artifacts' => true, 'name' => 'test2', 'optional' => false, 'partition_id' => build.partition_id } ) end end diff --git a/spec/models/ci/resource_group_spec.rb b/spec/models/ci/resource_group_spec.rb index e8eccc233db..01acf5194f0 100644 --- a/spec/models/ci/resource_group_spec.rb +++ b/spec/models/ci/resource_group_spec.rb @@ -33,7 +33,13 @@ RSpec.describe Ci::ResourceGroup do end end - describe '#assign_resource_to' do + describe '#assign_resource_to', :ci_partitionable do + include Ci::PartitioningHelpers + + before do + stub_current_partition_id + end + subject { resource_group.assign_resource_to(build) } let(:build) { create(:ci_build) } @@ -41,10 +47,12 @@ RSpec.describe Ci::ResourceGroup do it 'retains resource for the processable' do expect(resource_group.resources.first.processable).to be_nil + expect(resource_group.resources.first.partition_id).to be_nil is_expected.to eq(true) expect(resource_group.resources.first.processable).to eq(build) + expect(resource_group.resources.first.partition_id).to eq(build.partition_id) end context 'when there are no free resources' do @@ -66,7 +74,13 @@ RSpec.describe Ci::ResourceGroup do end end - describe '#release_resource_from' do + describe '#release_resource_from', :ci_partitionable do + include Ci::PartitioningHelpers + + before do + stub_current_partition_id + end + subject { resource_group.release_resource_from(build) } let(:build) { create(:ci_build) } @@ -79,10 +93,12 @@ RSpec.describe Ci::ResourceGroup do it 'releases resource from the build' do expect(resource_group.resources.first.processable).to eq(build) + expect(resource_group.resources.first.partition_id).to eq(build.partition_id) is_expected.to eq(true) expect(resource_group.resources.first.processable).to be_nil + expect(resource_group.resources.first.partition_id).to be_nil end end diff --git a/spec/models/ci/runner_namespace_spec.rb b/spec/models/ci/runner_namespace_spec.rb index 2d1fe11147c..6cbb151d703 100644 --- a/spec/models/ci/runner_namespace_spec.rb +++ b/spec/models/ci/runner_namespace_spec.rb @@ -12,4 +12,27 @@ RSpec.describe Ci::RunnerNamespace do let!(:parent) { model.namespace } end + + describe '.for_runner' do + subject(:for_runner) { described_class.for_runner(runner_ids) } + + let_it_be(:group) { create(:group) } + let_it_be(:runners) { create_list(:ci_runner, 3, :group, groups: [group]) } + + context 'with runner ids' do + let(:runner_ids) { runners[1..2].map(&:id) } + + it 'returns requested runner namespaces' do + is_expected.to eq(runners[1..2].flat_map(&:runner_namespaces)) + end + end + + context 'with runners' do + let(:runner_ids) { runners.first } + + it 'returns requested runner namespaces' do + is_expected.to eq(runners.first.runner_namespaces) + end + end + end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 13eb7086586..803b766c822 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Runner do +RSpec.describe Ci::Runner, feature_category: :runner do include StubGitlabCalls it_behaves_like 'having unique enum values' @@ -701,6 +701,30 @@ RSpec.describe Ci::Runner do it { is_expected.to eq([runner1]) } end + describe '.with_running_builds' do + subject { described_class.with_running_builds } + + let_it_be(:runner1) { create(:ci_runner) } + + context 'with no builds running' do + it { is_expected.to be_empty } + end + + context 'with single build running on runner2' do + let(:runner2) { create(:ci_runner) } + let(:runner3) { create(:ci_runner) } + + before do + project = create(:project, :repository) + pipeline = create(:ci_pipeline, project: project) + create(:ci_build, :running, runner: runner2, pipeline: pipeline) + create(:ci_build, :running, runner: runner3, pipeline: pipeline) + end + + it { is_expected.to contain_exactly(runner2, runner3) } + end + end + describe '#matches_build?' do using RSpec::Parameterized::TableSyntax diff --git a/spec/models/ci/runner_version_spec.rb b/spec/models/ci/runner_version_spec.rb index 7a4b2e8f21e..552b271fe85 100644 --- a/spec/models/ci/runner_version_spec.rb +++ b/spec/models/ci/runner_version_spec.rb @@ -2,16 +2,16 @@ require 'spec_helper' -RSpec.describe Ci::RunnerVersion do - it_behaves_like 'having unique enum values' +RSpec.describe Ci::RunnerVersion, feature_category: :runner_fleet do + let_it_be(:runner_version_recommended) do + create(:ci_runner_version, version: 'abc234', status: :recommended) + end let_it_be(:runner_version_not_available) do create(:ci_runner_version, version: 'abc123', status: :not_available) end - let_it_be(:runner_version_recommended) do - create(:ci_runner_version, version: 'abc234', status: :recommended) - end + it_behaves_like 'having unique enum values' describe '.not_available' do subject { described_class.not_available } @@ -28,11 +28,9 @@ RSpec.describe Ci::RunnerVersion do end it 'contains any valid or unprocessed runner version that is not already recommended' do - is_expected.to match_array([ - runner_version_nil, - runner_version_not_available, - runner_version_available - ]) + is_expected.to match_array( + [runner_version_nil, runner_version_not_available, runner_version_available] + ) end end diff --git a/spec/models/ci/running_build_spec.rb b/spec/models/ci/running_build_spec.rb index d2f74494308..1a5ea044ba3 100644 --- a/spec/models/ci/running_build_spec.rb +++ b/spec/models/ci/running_build_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::RunningBuild do +RSpec.describe Ci::RunningBuild, feature_category: :continuous_integration do let_it_be(:project) { create(:project) } let_it_be(:pipeline) { create(:ci_pipeline, project: project) } @@ -50,6 +50,28 @@ RSpec.describe Ci::RunningBuild do end end + describe 'partitioning', :ci_partitionable do + include Ci::PartitioningHelpers + + before do + stub_current_partition_id + end + + let(:new_pipeline ) { create(:ci_pipeline, project: pipeline.project) } + let(:new_build) { create(:ci_build, :running, pipeline: new_pipeline, runner: runner) } + + it 'assigns the same partition id as the one that build has', :aggregate_failures do + expect(new_build.partition_id).to eq ci_testing_partition_id + expect(new_build.partition_id).not_to eq pipeline.partition_id + + described_class.upsert_shared_runner_build!(build) + described_class.upsert_shared_runner_build!(new_build) + + expect(build.reload.runtime_metadata.partition_id).to eq pipeline.partition_id + expect(new_build.reload.runtime_metadata.partition_id).to eq ci_testing_partition_id + end + end + it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:project) } let!(:model) { create(:ci_running_build, project: parent) } diff --git a/spec/models/ci/secure_file_spec.rb b/spec/models/ci/secure_file_spec.rb index 4413bd8e98b..87077fe2db1 100644 --- a/spec/models/ci/secure_file_spec.rb +++ b/spec/models/ci/secure_file_spec.rb @@ -138,17 +138,48 @@ RSpec.describe Ci::SecureFile do end describe '#update_metadata!' do - it 'assigns the expected metadata when a parsable file is supplied' do + it 'assigns the expected metadata when a parsable .cer file is supplied' do file = create(:ci_secure_file, name: 'file1.cer', file: CarrierWaveStringFile.new(fixture_file('ci_secure_files/sample.cer'))) file.update_metadata! + file.reload + expect(file.expires_at).to eq(DateTime.parse('2022-04-26 19:20:40')) expect(file.metadata['id']).to eq('33669367788748363528491290218354043267') expect(file.metadata['issuer']['CN']).to eq('Apple Worldwide Developer Relations Certification Authority') expect(file.metadata['subject']['OU']).to eq('N7SYAN8PX8') end + it 'assigns the expected metadata when a parsable .p12 file is supplied' do + file = create(:ci_secure_file, name: 'file1.p12', + file: CarrierWaveStringFile.new(fixture_file('ci_secure_files/sample.p12'))) + file.update_metadata! + + file.reload + + expect(file.expires_at).to eq(DateTime.parse('2022-09-21 14:56:00')) + expect(file.metadata['id']).to eq('75949910542696343243264405377658443914') + expect(file.metadata['issuer']['CN']).to eq('Apple Worldwide Developer Relations Certification Authority') + expect(file.metadata['subject']['OU']).to eq('N7SYAN8PX8') + end + + it 'assigns the expected metadata when a parsable .mobileprovision file is supplied' do + file = create(:ci_secure_file, name: 'file1.mobileprovision', + file: CarrierWaveStringFile.new( + fixture_file('ci_secure_files/sample.mobileprovision') + )) + file.update_metadata! + + file.reload + + expect(file.expires_at).to eq(DateTime.parse('2023-08-01 23:15:13')) + expect(file.metadata['id']).to eq('6b9fcce1-b9a9-4b37-b2ce-ec4da2044abf') + expect(file.metadata['platforms'].first).to eq('iOS') + expect(file.metadata['app_name']).to eq('iOS Demo') + expect(file.metadata['app_id']).to eq('match Development com.gitlab.ios-demo') + end + it 'logs an error when something goes wrong with the file parsing' do corrupt_file = create(:ci_secure_file, name: 'file1.cer', file: CarrierWaveStringFile.new('11111111')) message = 'Validation failed: Metadata must be a valid json schema - not enough data.' diff --git a/spec/models/ci/sources/pipeline_spec.rb b/spec/models/ci/sources/pipeline_spec.rb index fdc1c111c40..707872d0a15 100644 --- a/spec/models/ci/sources/pipeline_spec.rb +++ b/spec/models/ci/sources/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Sources::Pipeline do +RSpec.describe Ci::Sources::Pipeline, feature_category: :continuous_integration do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:pipeline) } @@ -31,4 +31,20 @@ RSpec.describe Ci::Sources::Pipeline do let!(:model) { create(:ci_sources_pipeline, project: parent) } end end + + describe 'partitioning', :ci_partitioning do + include Ci::PartitioningHelpers + + let(:new_pipeline) { create(:ci_pipeline) } + let(:source_pipeline) { create(:ci_sources_pipeline, pipeline: new_pipeline) } + + before do + stub_current_partition_id + end + + it 'assigns partition_id and source_partition_id from pipeline and source_job', :aggregate_failures do + expect(source_pipeline.partition_id).to eq(ci_testing_partition_id) + expect(source_pipeline.source_partition_id).to eq(ci_testing_partition_id) + end + end end diff --git a/spec/models/ci/unit_test_failure_spec.rb b/spec/models/ci/unit_test_failure_spec.rb index f9b8c66b603..7ffd103bc7d 100644 --- a/spec/models/ci/unit_test_failure_spec.rb +++ b/spec/models/ci/unit_test_failure_spec.rb @@ -70,4 +70,46 @@ RSpec.describe Ci::UnitTestFailure do end end end + + describe 'partitioning', :ci_partitionable do + let(:project) { FactoryBot.build(:project) } + let(:unit_test) { FactoryBot.build(:ci_unit_test, project: project) } + + context 'with build' do + let(:build) { FactoryBot.build(:ci_build, partition_id: ci_testing_partition_id) } + let(:unit_test_failure) do + FactoryBot.build(:ci_unit_test_failure, build: build, unit_test: unit_test, failed_at: 1.day.ago) + end + + it 'copies the partition_id from build' do + expect { unit_test_failure.valid? }.to change { unit_test_failure.partition_id }.to(ci_testing_partition_id) + end + + context 'when it is already set' do + let(:unit_test_failure) do + FactoryBot.build( + :ci_unit_test_failure, + build: build, + unit_test: unit_test, + failed_at: 1.day.ago, + partition_id: 125 + ) + end + + it 'does not change the partition_id value' do + expect { unit_test_failure.valid? }.not_to change { unit_test_failure.partition_id } + end + end + end + + context 'without build' do + subject(:unit_test_failure) { FactoryBot.build(:ci_unit_test_failure, build: nil, partition_id: 125) } + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { unit_test_failure.valid? }.not_to change { unit_test_failure.partition_id } + end + end + end end diff --git a/spec/models/clusters/agent_token_spec.rb b/spec/models/clusters/agent_token_spec.rb index efa2a3eb09b..74723e3abd8 100644 --- a/spec/models/clusters/agent_token_spec.rb +++ b/spec/models/clusters/agent_token_spec.rb @@ -49,4 +49,12 @@ RSpec.describe Clusters::AgentToken do expect(agent_token.token.length).to be >= 50 end end + + describe '#to_ability_name' do + it 'is :cluster' do + agent_token = build(:cluster_agent_token) + + expect(agent_token.to_ability_name).to eq :cluster + end + end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index e5caa11452e..2be59e5f515 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -5,6 +5,11 @@ require 'spec_helper' RSpec.describe Clusters::Applications::Ingress do let(:ingress) { create(:clusters_applications_ingress) } + before do + allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) + allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) + end + it_behaves_like 'having unique enum values' include_examples 'cluster application core specs', :clusters_applications_ingress @@ -13,11 +18,6 @@ RSpec.describe Clusters::Applications::Ingress do include_examples 'cluster application helm specs', :clusters_applications_ingress include_examples 'cluster application initial status specs' - before do - allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) - allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) - end - describe 'default values' do it { expect(subject.ingress_type).to eq("nginx") } it { expect(subject.version).to eq(described_class::VERSION) } diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index 3914450339a..f6b13f4a93f 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -5,18 +5,18 @@ require 'spec_helper' RSpec.describe Clusters::Applications::Knative do let(:knative) { create(:clusters_applications_knative) } - include_examples 'cluster application core specs', :clusters_applications_knative - include_examples 'cluster application status specs', :clusters_applications_knative - include_examples 'cluster application helm specs', :clusters_applications_knative - include_examples 'cluster application version specs', :clusters_applications_knative - include_examples 'cluster application initial status specs' - before do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) allow(ClusterConfigureIstioWorker).to receive(:perform_async) end + include_examples 'cluster application core specs', :clusters_applications_knative + include_examples 'cluster application status specs', :clusters_applications_knative + include_examples 'cluster application helm specs', :clusters_applications_knative + include_examples 'cluster application version specs', :clusters_applications_knative + include_examples 'cluster application initial status specs' + describe 'associations' do it { is_expected.to have_one(:serverless_domain_cluster).class_name('::Serverless::DomainCluster').with_foreign_key('clusters_applications_knative_id').inverse_of(:knative) } end diff --git a/spec/models/commit_signatures/gpg_signature_spec.rb b/spec/models/commit_signatures/gpg_signature_spec.rb index 1ffaaeba396..75cc5d448df 100644 --- a/spec/models/commit_signatures/gpg_signature_spec.rb +++ b/spec/models/commit_signatures/gpg_signature_spec.rb @@ -23,6 +23,7 @@ RSpec.describe CommitSignatures::GpgSignature do it_behaves_like 'having unique enum values' it_behaves_like 'commit signature' + it_behaves_like 'signature with type checking', :gpg describe 'associations' do it { is_expected.to belong_to(:gpg_key) } @@ -86,9 +87,9 @@ RSpec.describe CommitSignatures::GpgSignature do end end - describe '#user' do + describe '#signed_by_user' do it 'retrieves the gpg_key user' do - expect(signature.user).to eq(gpg_key.user) + expect(signature.signed_by_user).to eq(gpg_key.user) end end end diff --git a/spec/models/commit_signatures/ssh_signature_spec.rb b/spec/models/commit_signatures/ssh_signature_spec.rb index 08530bf6964..629d9c5ec53 100644 --- a/spec/models/commit_signatures/ssh_signature_spec.rb +++ b/spec/models/commit_signatures/ssh_signature_spec.rb @@ -22,6 +22,7 @@ RSpec.describe CommitSignatures::SshSignature do it_behaves_like 'having unique enum values' it_behaves_like 'commit signature' + it_behaves_like 'signature with type checking', :ssh describe 'associations' do it { is_expected.to belong_to(:key).optional } @@ -37,4 +38,10 @@ RSpec.describe CommitSignatures::SshSignature do ).to contain_exactly(signature, another_signature) end end + + describe '#signed_by_user' do + it 'returns the user associated with the SSH key' do + expect(signature.signed_by_user).to eq(ssh_key.user) + end + end end diff --git a/spec/models/commit_signatures/x509_commit_signature_spec.rb b/spec/models/commit_signatures/x509_commit_signature_spec.rb index b971fd078e2..cceb96ec70d 100644 --- a/spec/models/commit_signatures/x509_commit_signature_spec.rb +++ b/spec/models/commit_signatures/x509_commit_signature_spec.rb @@ -23,6 +23,7 @@ RSpec.describe CommitSignatures::X509CommitSignature do it_behaves_like 'having unique enum values' it_behaves_like 'commit signature' + it_behaves_like 'signature with type checking', :x509 describe 'validation' do it { is_expected.to validate_presence_of(:x509_certificate_id) } @@ -37,12 +38,12 @@ RSpec.describe CommitSignatures::X509CommitSignature do let!(:user) { create(:user, email: X509Helpers::User1.certificate_email) } it 'returns user' do - expect(described_class.safe_create!(attributes).user).to eq(user) + expect(described_class.safe_create!(attributes).signed_by_user).to eq(user) end end it 'if email is not assigned to a user, return nil' do - expect(described_class.safe_create!(attributes).user).to be_nil + expect(described_class.safe_create!(attributes).signed_by_user).to be_nil end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index bab6247d4f9..4b5aabe745b 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -828,12 +828,14 @@ eos describe 'signed commits' do let(:gpg_signed_commit) { project.commit_by(oid: '0b4bc9a49b562e85de7cc9e834518ea6828729b9') } let(:x509_signed_commit) { project.commit_by(oid: '189a6c924013fc3fe40d6f1ec1dc20214183bc97') } + let(:ssh_signed_commit) { project.commit_by(oid: '7b5160f9bb23a3d58a0accdbe89da13b96b1ece9') } let(:unsigned_commit) { project.commit_by(oid: '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51') } let!(:commit) { create(:commit, project: project) } it 'returns signature_type properly' do expect(gpg_signed_commit.signature_type).to eq(:PGP) expect(x509_signed_commit.signature_type).to eq(:X509) + expect(ssh_signed_commit.signature_type).to eq(:SSH) expect(unsigned_commit.signature_type).to eq(:NONE) expect(commit.signature_type).to eq(:NONE) end @@ -841,9 +843,24 @@ eos it 'returns has_signature? properly' do expect(gpg_signed_commit.has_signature?).to be_truthy expect(x509_signed_commit.has_signature?).to be_truthy + expect(ssh_signed_commit.has_signature?).to be_truthy expect(unsigned_commit.has_signature?).to be_falsey expect(commit.has_signature?).to be_falsey end + + context 'when feature flag "ssh_commit_signatures" is disabled' do + before do + stub_feature_flags(ssh_commit_signatures: false) + end + + it 'reports no signature' do + expect(ssh_signed_commit).not_to have_signature + end + + it 'does not return signature data' do + expect(ssh_signed_commit.signature).to be_nil + end + end end describe '#has_been_reverted?' do diff --git a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb index 358000ee174..e8d84fe9630 100644 --- a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb +++ b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb @@ -27,6 +27,7 @@ RSpec.describe BatchDestroyDependentAssociations do let_it_be(:build) { create(:ci_build, project: project) } let_it_be(:notification_setting) { create(:notification_setting, project: project) } let_it_be(:note) { create(:note, project: project) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } it 'destroys multiple notes' do create(:note, project: project) @@ -51,9 +52,10 @@ RSpec.describe BatchDestroyDependentAssociations do end it 'excludes associations' do - project.destroy_dependent_associations_in_batches(exclude: [:notes]) + project.destroy_dependent_associations_in_batches(exclude: [:merge_requests]) - expect(Note.count).to eq(1) + expect(MergeRequest.count).to eq(1) + expect(Note.count).to eq(0) expect(Ci::Build.count).to eq(1) expect(User.count).to be > 0 expect(NotificationSetting.count).to eq(User.count) diff --git a/spec/models/concerns/bulk_insertable_associations_spec.rb b/spec/models/concerns/bulk_insertable_associations_spec.rb index 9713f1ce9a4..3187dcd8f93 100644 --- a/spec/models/concerns/bulk_insertable_associations_spec.rb +++ b/spec/models/concerns/bulk_insertable_associations_spec.rb @@ -3,34 +3,41 @@ require 'spec_helper' RSpec.describe BulkInsertableAssociations do - class BulkFoo < ApplicationRecord - include BulkInsertSafe + before do + stub_const('BulkFoo', Class.new(ApplicationRecord)) + stub_const('BulkBar', Class.new(ApplicationRecord)) + stub_const('SimpleBar', Class.new(ApplicationRecord)) + stub_const('BulkParent', Class.new(ApplicationRecord)) - self.table_name = '_test_bulk_foos' + BulkFoo.class_eval do + include BulkInsertSafe - validates :name, presence: true - end + self.table_name = '_test_bulk_foos' - class BulkBar < ApplicationRecord - include BulkInsertSafe + validates :name, presence: true + end - self.table_name = '_test_bulk_bars' - end + BulkBar.class_eval do + include BulkInsertSafe - SimpleBar = Class.new(ApplicationRecord) do - self.table_name = '_test_simple_bars' - end + self.table_name = '_test_bulk_bars' + end + + SimpleBar.class_eval do + self.table_name = '_test_simple_bars' + end - class BulkParent < ApplicationRecord - include BulkInsertableAssociations + BulkParent.class_eval do + include BulkInsertableAssociations - self.table_name = '_test_bulk_parents' + self.table_name = '_test_bulk_parents' - has_many :bulk_foos, class_name: 'BulkFoo' - has_many :bulk_hunks, class_name: 'BulkFoo' - has_many :bulk_bars, class_name: 'BulkBar' - has_many :simple_bars, class_name: 'SimpleBar' # not `BulkInsertSafe` - has_one :bulk_foo # not supported + has_many :bulk_foos, class_name: 'BulkFoo' + has_many :bulk_hunks, class_name: 'BulkFoo' + has_many :bulk_bars, class_name: 'BulkBar' + has_many :simple_bars, class_name: 'SimpleBar' # not `BulkInsertSafe` + has_one :bulk_foo # not supported + end end before(:all) do diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 19b9a1519eb..f85f636ebe5 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -231,7 +231,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do end describe '#updated_cached_html_for' do - let(:thing) { klass.new(description: markdown, description_html: html, cached_markdown_version: cache_version) } + let(:thing) { klass.new(project_id: project.id, namespace_id: project.project_namespace_id, description: markdown, description_html: html, cached_markdown_version: cache_version) } context 'when the markdown cache is outdated' do before do @@ -286,7 +286,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do end shared_examples 'a class with mentionable markdown fields' do - let(:mentionable) { klass.new(description: markdown, description_html: html, title: markdown, title_html: html, cached_markdown_version: cache_version) } + let(:mentionable) { klass.new(project_id: project.id, namespace_id: project.project_namespace_id, description: markdown, description_html: html, title: markdown, title_html: html, cached_markdown_version: cache_version) } context 'when klass is a Mentionable', :aggregate_failures do before do @@ -336,7 +336,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do end context 'when the markdown field also a mentionable attribute' do - let(:thing) { klass.new(description: markdown, description_html: html, cached_markdown_version: cache_version) } + let(:thing) { klass.new(project_id: project.id, namespace_id: project.project_namespace_id, description: markdown, description_html: html, cached_markdown_version: cache_version) } it 'calls #store_mentions!' do expect(thing).to receive(:mentionable_attributes_changed?).and_return(true) @@ -349,7 +349,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do end context 'when the markdown field is not mentionable attribute' do - let(:thing) { klass.new(title: markdown, title_html: html, cached_markdown_version: cache_version) } + let(:thing) { klass.new(project_id: project.id, namespace_id: project.project_namespace_id, title: markdown, title_html: html, cached_markdown_version: cache_version) } it 'does not call #store_mentions!' do expect(thing).not_to receive(:store_mentions!) @@ -363,7 +363,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do end context 'when the markdown field does not exist' do - let(:thing) { klass.new(cached_markdown_version: cache_version) } + let(:thing) { klass.new(project_id: project.id, namespace_id: project.project_namespace_id, cached_markdown_version: cache_version) } it 'does not call #store_mentions!' do expect(thing).not_to receive(:store_mentions!) @@ -376,13 +376,15 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do end context 'for Active record classes' do + let_it_be(:project) { create(:project) } + let(:klass) { ar_class } it_behaves_like 'a class with cached markdown fields' it_behaves_like 'a class with mentionable markdown fields' describe '#attribute_invalidated?' do - let(:thing) { klass.create!(description: markdown, description_html: html, cached_markdown_version: cache_version) } + let(:thing) { klass.create!(project_id: project.id, namespace_id: project.project_namespace_id, description: markdown, description_html: html, cached_markdown_version: cache_version) } it 'returns true when cached_markdown_version is different' do thing.cached_markdown_version += 1 @@ -425,7 +427,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do let(:thing) do # This forces the record to have outdated HTML. We can't use `create` because the `before_create` hook # would re-render the HTML to the latest version - klass.create!.tap do |thing| + klass.create!(project_id: project.id, namespace_id: project.project_namespace_id).tap do |thing| thing.update_columns(description: markdown, description_html: old_html, cached_markdown_version: old_version) end end @@ -441,6 +443,8 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do end context 'for other classes' do + let_it_be(:project) { create(:project) } + let(:klass) { other_class } it_behaves_like 'a class with cached markdown fields' diff --git a/spec/models/concerns/ci/partitionable/partitioned_filter_spec.rb b/spec/models/concerns/ci/partitionable/partitioned_filter_spec.rb new file mode 100644 index 00000000000..bb25d7d1665 --- /dev/null +++ b/spec/models/concerns/ci/partitionable/partitioned_filter_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::Partitionable::PartitionedFilter, :aggregate_failures, feature_category: :continuous_integration do + before do + create_tables(<<~SQL) + CREATE TABLE _test_ci_jobs_metadata ( + id serial NOT NULL, + partition_id int NOT NULL DEFAULT 10, + name text, + PRIMARY KEY (id, partition_id) + ) PARTITION BY LIST(partition_id); + + CREATE TABLE _test_ci_jobs_metadata_1 + PARTITION OF _test_ci_jobs_metadata + FOR VALUES IN (10); + SQL + end + + let(:model) do + Class.new(Ci::ApplicationRecord) do + include Ci::Partitionable::PartitionedFilter + + self.primary_key = :id + self.table_name = :_test_ci_jobs_metadata + + def self.name + 'TestCiJobMetadata' + end + end + end + + let!(:record) { model.create! } + + let(:where_filter) do + /WHERE "_test_ci_jobs_metadata"."id" = #{record.id} AND "_test_ci_jobs_metadata"."partition_id" = 10/ + end + + describe '#save' do + it 'uses id and partition_id' do + record.name = 'test' + recorder = ActiveRecord::QueryRecorder.new { record.save! } + + expect(recorder.log).to include(where_filter) + expect(record.name).to eq('test') + end + end + + describe '#update' do + it 'uses id and partition_id' do + recorder = ActiveRecord::QueryRecorder.new { record.update!(name: 'test') } + + expect(recorder.log).to include(where_filter) + expect(record.name).to eq('test') + end + end + + describe '#delete' do + it 'uses id and partition_id' do + recorder = ActiveRecord::QueryRecorder.new { record.delete } + + expect(recorder.log).to include(where_filter) + expect(model.count).to be_zero + end + end + + describe '#destroy' do + it 'uses id and partition_id' do + recorder = ActiveRecord::QueryRecorder.new { record.destroy! } + + expect(recorder.log).to include(where_filter) + expect(model.count).to be_zero + end + end + + def create_tables(table_sql) + Ci::ApplicationRecord.connection.execute(table_sql) + end +end diff --git a/spec/models/concerns/ci/partitionable_spec.rb b/spec/models/concerns/ci/partitionable_spec.rb index f3d33c971c7..430ef57d493 100644 --- a/spec/models/concerns/ci/partitionable_spec.rb +++ b/spec/models/concerns/ci/partitionable_spec.rb @@ -40,4 +40,28 @@ RSpec.describe Ci::Partitionable do it { expect(ci_model.ancestors).to include(described_class::Switch) } end + + context 'with partitioned options' do + before do + stub_const("#{described_class}::Testing::PARTITIONABLE_MODELS", [ci_model.name]) + + ci_model.include(described_class) + ci_model.partitionable scope: ->(r) { 1 }, partitioned: partitioned + end + + context 'when partitioned is true' do + let(:partitioned) { true } + + it { expect(ci_model.ancestors).to include(described_class::PartitionedFilter) } + it { expect(ci_model).to be_partitioned } + end + + context 'when partitioned is false' do + let(:partitioned) { false } + + it { expect(ci_model.ancestors).not_to include(described_class::PartitionedFilter) } + + it { expect(ci_model).not_to be_partitioned } + end + end end diff --git a/spec/models/concerns/commit_signature_spec.rb b/spec/models/concerns/commit_signature_spec.rb new file mode 100644 index 00000000000..4bba5a6ee41 --- /dev/null +++ b/spec/models/concerns/commit_signature_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe CommitSignature do + describe '#signed_by_user' do + context 'when class does not define the signed_by_user method' do + subject(:implementation) do + Class.new(ActiveRecord::Base) do + self.table_name = 'ssh_signatures' + end.include(described_class).new + end + + it 'raises a NoMethodError with custom message' do + expect do + implementation.signed_by_user + end.to raise_error(NoMethodError, 'must implement `signed_by_user` method') + end + end + end +end diff --git a/spec/models/concerns/counter_attribute_spec.rb b/spec/models/concerns/counter_attribute_spec.rb index 66ccd4559e5..1dd9b78d642 100644 --- a/spec/models/concerns/counter_attribute_spec.rb +++ b/spec/models/concerns/counter_attribute_spec.rb @@ -8,85 +8,70 @@ RSpec.describe CounterAttribute, :counter_attribute, :clean_gitlab_redis_shared_ let(:project_statistics) { create(:project_statistics) } let(:model) { CounterAttributeModel.find(project_statistics.id) } - it_behaves_like CounterAttribute, [:build_artifacts_size, :commit_count] do + it_behaves_like CounterAttribute, [:build_artifacts_size, :commit_count, :packages_size] do let(:model) { CounterAttributeModel.find(project_statistics.id) } end - describe 'after_flush callbacks' do - let(:attribute) { model.class.counter_attributes.first } - - subject { model.flush_increments_to_database!(attribute) } + describe '#counter_attribute_enabled?' do + it 'is true when counter attribute is defined' do + expect(project_statistics.counter_attribute_enabled?(:build_artifacts_size)) + .to be_truthy + end - it 'has registered callbacks' do # defined in :counter_attribute RSpec tag - expect(model.class.after_flush_callbacks.size).to eq(1) + it 'is false when counter attribute is not defined' do + expect(model.counter_attribute_enabled?(:nope)).to be_falsey end - context 'when there are increments to flush' do - before do - model.delayed_increment_counter(attribute, 10) - end + context 'with a conditional counter attribute' do + [true, false].each do |enabled| + context "where the condition evaluates to #{enabled}" do + subject { model.counter_attribute_enabled?(:packages_size) } - it 'executes the callbacks' do - subject + before do + model.allow_package_size_counter = enabled + end - expect(model.flushed).to be_truthy + it { is_expected.to eq(enabled) } + end end end + end - context 'when there are no increments to flush' do - it 'does not execute the callbacks' do - subject + describe '#counter' do + using RSpec::Parameterized::TableSyntax - expect(model.flushed).to be_nil - end + it 'returns the counter for the respective attribute' do + expect(model.counter(:build_artifacts_size).send(:attribute)).to eq(:build_artifacts_size) + expect(model.counter(:commit_count).send(:attribute)).to eq(:commit_count) end - end - describe '.steal_increments' do - let(:increment_key) { 'counters:Model:123:attribute' } - let(:flushed_key) { 'counter:Model:123:attribute:flushed' } - - subject { model.send(:steal_increments, increment_key, flushed_key) } - - where(:increment, :flushed, :result, :flushed_key_present) do - nil | nil | 0 | false - nil | 0 | 0 | false - 0 | 0 | 0 | false - 1 | 0 | 1 | true - 1 | nil | 1 | true - 1 | 1 | 2 | true - 1 | -2 | -1 | true - -1 | 1 | 0 | false + it 'returns the appropriate counter for the attribute' do + expect(model.counter(:build_artifacts_size).class).to eq(Gitlab::Counters::BufferedCounter) + expect(model.counter(:packages_size).class).to eq(Gitlab::Counters::BufferedCounter) + expect(model.counter(:wiki_size).class).to eq(Gitlab::Counters::LegacyCounter) end - with_them do - before do - Gitlab::Redis::SharedState.with do |redis| - redis.set(increment_key, increment) if increment - redis.set(flushed_key, flushed) if flushed - end + context 'with a conditional counter attribute' do + where(:enabled, :expected_counter_class) do + [ + [true, Gitlab::Counters::BufferedCounter], + [false, Gitlab::Counters::LegacyCounter] + ] end - it { is_expected.to eq(result) } - - it 'drops the increment key and creates the flushed key if it does not exist' do - subject + with_them do + before do + model.allow_package_size_counter = enabled + end - Gitlab::Redis::SharedState.with do |redis| - expect(redis.exists?(increment_key)).to eq(false) - expect(redis.exists?(flushed_key)).to eq(flushed_key_present) + it 'returns the appropriate counter based on the condition' do + expect(model.counter(:packages_size).class).to eq(expected_counter_class) end end end - end - - describe '.counter_attribute_enabled?' do - it 'is true when counter attribute is defined' do - expect(CounterAttributeModel.counter_attribute_enabled?(:build_artifacts_size)).to be_truthy - end - it 'is false when counter attribute is not defined' do - expect(CounterAttributeModel.counter_attribute_enabled?(:nope)).to be_falsey + it 'raises error for unknown attribute' do + expect { model.counter(:unknown) }.to raise_error(ArgumentError, 'attribute "unknown" does not exist') end end end diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index b2ea7b22dea..462b28f99be 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe User do specify 'types consistency checks', :aggregate_failures do expect(described_class::USER_TYPES.keys) - .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot visual_review_bot migration_bot automation_bot]) + .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot visual_review_bot migration_bot automation_bot admin_bot]) expect(described_class::USER_TYPES).to include(*described_class::BOT_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::NON_INTERNAL_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::INTERNAL_USER_TYPES) @@ -37,12 +37,6 @@ RSpec.describe User do end end - describe '.bots_without_project_bot' do - it 'includes all bots except project_bot' do - expect(described_class.bots_without_project_bot).to match_array(bots - [project_bot]) - end - end - describe '.non_internal' do it 'includes all non_internal users' do expect(described_class.non_internal).to match_array(non_internal) diff --git a/spec/models/concerns/pg_full_text_searchable_spec.rb b/spec/models/concerns/pg_full_text_searchable_spec.rb index 98b44a2eec2..87f1dc5a27b 100644 --- a/spec/models/concerns/pg_full_text_searchable_spec.rb +++ b/spec/models/concerns/pg_full_text_searchable_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe PgFullTextSearchable do - let(:project) { build(:project) } + let(:project) { build(:project, project_namespace: build(:project_namespace)) } let(:model_class) do Class.new(ActiveRecord::Base) do @@ -12,6 +12,7 @@ RSpec.describe PgFullTextSearchable do self.table_name = 'issues' belongs_to :project + belongs_to :namespace has_one :search_data, class_name: 'Issues::SearchData' before_validation -> { self.work_item_type_id = ::WorkItems::Type.default_issue_type.id } @@ -41,7 +42,7 @@ RSpec.describe PgFullTextSearchable do end describe 'after commit hook' do - let(:model) { model_class.create!(project: project) } + let(:model) { model_class.create!(project: project, namespace: project.project_namespace) } before do model_class.pg_full_text_searchable columns: [{ name: 'title', weight: 'A' }] @@ -76,9 +77,9 @@ RSpec.describe PgFullTextSearchable do end describe '.pg_full_text_search' do - let(:english) { model_class.create!(project: project, title: 'title', description: 'something description english') } - let(:with_accent) { model_class.create!(project: project, title: 'Jürgen', description: 'Ærøskøbing') } - let(:japanese) { model_class.create!(project: project, title: '日本語 title', description: 'another english description') } + let(:english) { model_class.create!(project: project, namespace: project.project_namespace, title: 'title', description: 'something description english') } + let(:with_accent) { model_class.create!(project: project, namespace: project.project_namespace, title: 'Jürgen', description: 'Ærøskøbing') } + let(:japanese) { model_class.create!(project: project, namespace: project.project_namespace, title: '日本語 title', description: 'another english description') } before do model_class.pg_full_text_searchable columns: [{ name: 'title', weight: 'A' }, { name: 'description', weight: 'B' }] @@ -91,7 +92,7 @@ RSpec.describe PgFullTextSearchable do end it 'searches specified columns only' do - matching_object = model_class.create!(project: project, title: 'english', description: 'some description') + matching_object = model_class.create!(project: project, namespace: project.project_namespace, title: 'english', description: 'some description') matching_object.update_search_data! expect(model_class.pg_full_text_search('english', matched_columns: %w(title))).to contain_exactly(matching_object) @@ -115,7 +116,7 @@ RSpec.describe PgFullTextSearchable do end context 'when search term has a URL' do - let(:with_url) { model_class.create!(project: project, title: 'issue with url', description: 'sample url,https://gitlab.com/gitlab-org/gitlab') } + let(:with_url) { model_class.create!(project: project, namespace: project.project_namespace, title: 'issue with url', description: 'sample url,https://gitlab.com/gitlab-org/gitlab') } it 'allows searching by full URL, ignoring the scheme' do with_url.update_search_data! @@ -127,7 +128,7 @@ RSpec.describe PgFullTextSearchable do context 'when search term is a path with underscores' do let(:path) { 'browser_ui/5_package/package_registry/maven/maven_group_level_spec.rb' } - let(:with_underscore) { model_class.create!(project: project, title: 'issue with path', description: "some #{path} other text") } + let(:with_underscore) { model_class.create!(project: project, namespace: project.project_namespace, title: 'issue with path', description: "some #{path} other text") } it 'allows searching by the path' do with_underscore.update_search_data! @@ -137,7 +138,7 @@ RSpec.describe PgFullTextSearchable do end context 'when text has numbers preceded by a dash' do - let(:with_dash) { model_class.create!(project: project, title: 'issue with dash', description: 'ABC-123') } + let(:with_dash) { model_class.create!(project: project, namespace: project.project_namespace, title: 'issue with dash', description: 'ABC-123') } it 'allows searching by numbers only' do with_dash.update_search_data! @@ -148,7 +149,7 @@ RSpec.describe PgFullTextSearchable do end describe '#update_search_data!' do - let(:model) { model_class.create!(project: project, title: 'title', description: 'description') } + let(:model) { model_class.create!(project: project, namespace: project.project_namespace, title: 'title', description: 'description') } before do model_class.pg_full_text_searchable columns: [{ name: 'title', weight: 'A' }, { name: 'description', weight: 'B' }] @@ -162,7 +163,7 @@ RSpec.describe PgFullTextSearchable do end context 'with accented and non-Latin characters' do - let(:model) { model_class.create!(project: project, title: '日本語', description: 'Jürgen') } + let(:model) { model_class.create!(project: project, namespace: project.project_namespace, title: '日本語', description: 'Jürgen') } it 'transliterates accented characters and removes non-Latin ones' do model.update_search_data! @@ -173,7 +174,7 @@ RSpec.describe PgFullTextSearchable do end context 'with long words' do - let(:model) { model_class.create!(project: project, title: 'title ' + 'long/sequence+1' * 4, description: 'description ' + '@user1' * 20) } + let(:model) { model_class.create!(project: project, namespace: project.project_namespace, title: 'title ' + 'long/sequence+1' * 4, description: 'description ' + '@user1' * 20) } it 'strips words that are 50 characters or longer' do model.update_search_data! @@ -197,7 +198,7 @@ RSpec.describe PgFullTextSearchable do context 'with strings that go over tsvector limit', :delete do let(:long_string) { Array.new(30_000) { SecureRandom.hex }.join(' ') } - let(:model) { model_class.create!(project: project, title: 'title', description: long_string) } + let(:model) { model_class.create!(project: project, namespace: project.project_namespace, title: 'title', description: long_string) } it 'does not raise an exception' do expect(Gitlab::AppJsonLogger).to receive(:error).with( @@ -218,6 +219,7 @@ RSpec.describe PgFullTextSearchable do self.table_name = 'issues' belongs_to :project + belongs_to :namespace has_one :search_data, class_name: 'Issues::SearchData' before_validation -> { self.work_item_type_id = ::WorkItems::Type.default_issue_type.id } diff --git a/spec/models/concerns/schedulable_spec.rb b/spec/models/concerns/schedulable_spec.rb index b98dcf1c174..b8084a70046 100644 --- a/spec/models/concerns/schedulable_spec.rb +++ b/spec/models/concerns/schedulable_spec.rb @@ -77,7 +77,7 @@ RSpec.describe Schedulable do end.new end - it 'works' do + it 'raises a NotImplementedError' do expect { schedulable_instance.set_next_run_at }.to raise_error(NotImplementedError) end end diff --git a/spec/models/concerns/sensitive_serializable_hash_spec.rb b/spec/models/concerns/sensitive_serializable_hash_spec.rb index 3c9199ce18f..591a4383a03 100644 --- a/spec/models/concerns/sensitive_serializable_hash_spec.rb +++ b/spec/models/concerns/sensitive_serializable_hash_spec.rb @@ -35,12 +35,6 @@ RSpec.describe SensitiveSerializableHash do expect(model.serializable_hash).not_to include('super_secret') end - context 'unsafe_serialization_hash option' do - it 'includes the field in serializable_hash' do - expect(model.serializable_hash(unsafe_serialization_hash: true)).to include('super_secret') - end - end - it 'does not change parent class attributes_exempt_from_serializable_hash' do expect(test_class.attributes_exempt_from_serializable_hash).to contain_exactly(:super_secret) expect(another_class.attributes_exempt_from_serializable_hash).to contain_exactly(:sub_secret) @@ -65,21 +59,6 @@ RSpec.describe SensitiveSerializableHash do expect(model.as_json).not_to include(attribute) end end - - context 'unsafe_serialization_hash option' do - it 'includes the field in serializable_hash' do - attributes.each do |attribute| - expect(model.attributes).to include(attribute) # double-check the attribute does exist - - # Do not expect binary columns to appear in JSON - next if klass.columns_hash[attribute]&.type == :binary - - expect(model.serializable_hash(unsafe_serialization_hash: true)).to include(attribute) - expect(model.to_json(unsafe_serialization_hash: true)).to include(attribute) - expect(model.as_json(unsafe_serialization_hash: true)).to include(attribute) - end - end - end end end @@ -120,18 +99,6 @@ RSpec.describe SensitiveSerializableHash do expect(model.as_json).not_to include(attribute) end end - - context 'unsafe_serialization_hash option' do - it 'includes the field in serializable_hash' do - attributes.each do |attribute| - expect(model.attributes).to include(attribute) # double-check the attribute does exist - - expect(model.serializable_hash(unsafe_serialization_hash: true)).to include(attribute) - expect(model.to_json(unsafe_serialization_hash: true)).to include(attribute) - expect(model.as_json(unsafe_serialization_hash: true)).to include(attribute) - end - end - end end end diff --git a/spec/models/concerns/signature_type_spec.rb b/spec/models/concerns/signature_type_spec.rb new file mode 100644 index 00000000000..d8e2b617e0e --- /dev/null +++ b/spec/models/concerns/signature_type_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SignatureType do + describe '#type' do + context 'when class does not define a type method' do + subject(:implementation) { Class.new.include(described_class).new } + + it 'raises a NoMethodError with custom message' do + expect { implementation.type }.to raise_error(NoMethodError, 'must implement `type` method') + end + end + end +end diff --git a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb index e0ebb86585a..2df86804f34 100644 --- a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb +++ b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb @@ -237,6 +237,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do expect(subject.set_token(instance, 'my-value')).to eq 'my-value' end end + context 'when encryption is optional' do let(:options) { { encrypted: :optional } } diff --git a/spec/models/concerns/triggerable_hooks_spec.rb b/spec/models/concerns/triggerable_hooks_spec.rb index 90c88c888ff..5682a189c41 100644 --- a/spec/models/concerns/triggerable_hooks_spec.rb +++ b/spec/models/concerns/triggerable_hooks_spec.rb @@ -4,8 +4,10 @@ require 'spec_helper' RSpec.describe TriggerableHooks do before do - class TestableHook < WebHook - include TriggerableHooks + stub_const('TestableHook', Class.new(WebHook)) + + TestableHook.class_eval do + include TriggerableHooks # rubocop:disable RSpec/DescribedClass triggerable_hooks [:push_hooks] end end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 9af53bae204..33d3cabb325 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -879,10 +879,11 @@ RSpec.describe ContainerRepository, :aggregate_failures do it 'updates deletion status attributes' do expect { subject }.to change(repository, :status).from(nil).to('delete_ongoing') .and change(repository, :delete_started_at).from(nil).to(Time.zone.now) + .and change(repository, :status_updated_at).from(nil).to(Time.zone.now) end end - describe '#set_delete_scheduled_status' do + describe '#set_delete_scheduled_status', :freeze_time do let_it_be(:repository) { create(:container_repository, :status_delete_ongoing, delete_started_at: 3.minutes.ago) } subject { repository.set_delete_scheduled_status } @@ -890,6 +891,27 @@ RSpec.describe ContainerRepository, :aggregate_failures do it 'updates delete attributes' do expect { subject }.to change(repository, :status).from('delete_ongoing').to('delete_scheduled') .and change(repository, :delete_started_at).to(nil) + .and change(repository, :status_updated_at).to(Time.zone.now) + end + end + + describe '#status_updated_at', :freeze_time do + let_it_be_with_reload(:repository) { create(:container_repository) } + + %i[delete_scheduled delete_ongoing delete_failed].each do |status| + context "when status is updated to #{status}" do + it 'updates status_changed_at' do + expect { repository.update!(status: status) }.to change(repository, :status_updated_at).from(nil).to(Time.zone.now) + end + end + end + + context 'when status is not changed' do + it 'does not update status_changed_at' do + repository.name = 'different-image' + + expect { repository.save! }.not_to change(repository, :status_updated_at) + end end end @@ -1632,7 +1654,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do stub_application_setting(container_registry_import_target_plan: valid_container_repository.migration_plan) end - it 'works' do + it 'returns valid container repositories' do expect(subject).to contain_exactly(valid_container_repository, valid_container_repository2) end end diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 04763accc42..1b8dd62455e 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe DeployToken do +RSpec.describe DeployToken, feature_category: :continuous_delivery do subject(:deploy_token) { create(:deploy_token) } it { is_expected.to have_many :project_deploy_tokens } @@ -82,9 +82,7 @@ RSpec.describe DeployToken do describe '#ensure_at_least_one_scope' do context 'with at least one scope' do - it 'is valid' do - is_expected.to be_valid - end + it { is_expected.to be_valid } end context 'with no scopes' do diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index daa65f528e9..fb3883820fd 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -925,6 +925,21 @@ RSpec.describe Deployment do end end + describe '#triggered_by?' do + subject { deployment.triggered_by?(user) } + + let(:user) { create(:user) } + let(:deployment) { create(:deployment, user: user) } + + it { is_expected.to eq(true) } + + context 'when deployment triggerer is different' do + let(:deployment) { create(:deployment) } + + it { is_expected.to eq(false) } + end + end + describe '.find_successful_deployment!' do it 'returns a successful deployment' do deploy = create(:deployment, :success) diff --git a/spec/models/design_management/version_spec.rb b/spec/models/design_management/version_spec.rb index 44ecae82174..8c0d7e99ae5 100644 --- a/spec/models/design_management/version_spec.rb +++ b/spec/models/design_management/version_spec.rb @@ -232,7 +232,7 @@ RSpec.describe DesignManagement::Version do context 'there is a single design' do let_it_be(:design) { create(:design) } - shared_examples :a_correctly_categorised_design do |kind, category| + shared_examples 'a correctly categorised design' do |kind, category| let_it_be(:version) { create(:design_version, kind => [design]) } it 'returns a hash with a single key and the single design in that bucket' do @@ -240,9 +240,9 @@ RSpec.describe DesignManagement::Version do end end - it_behaves_like :a_correctly_categorised_design, :created_designs, 'creation' - it_behaves_like :a_correctly_categorised_design, :modified_designs, 'modification' - it_behaves_like :a_correctly_categorised_design, :deleted_designs, 'deletion' + it_behaves_like 'a correctly categorised design', :created_designs, 'creation' + it_behaves_like 'a correctly categorised design', :modified_designs, 'modification' + it_behaves_like 'a correctly categorised design', :deleted_designs, 'deletion' end context 'there are a bunch of different designs in a variety of states' do diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 8a3d43f58e0..2670127442e 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -64,6 +64,34 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end + describe 'preloading deployment associations' do + let!(:environment) { create(:environment, project: project) } + + associations = [:last_deployment, :last_visible_deployment, :upcoming_deployment] + associations.concat Deployment::FINISHED_STATUSES.map { |status| "last_#{status}_deployment".to_sym } + associations.concat Deployment::UPCOMING_STATUSES.map { |status| "last_#{status}_deployment".to_sym } + + context 'raises error for legacy approach' do + let!(:error_pattern) { /Preloading instance dependent scopes is not supported/ } + + subject { described_class.preload(association_name).find_by(id: environment) } + + shared_examples 'raises error' do + it do + expect { subject }.to raise_error(error_pattern) + end + end + + associations.each do |association| + context association.to_s do + let!(:association_name) { association } + + include_examples "raises error" + end + end + end + end + describe 'validate and sanitize external url' do let_it_be_with_refind(:environment) { create(:environment) } @@ -184,7 +212,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end describe ".schedule_to_delete" do - subject { described_class.for_id(deletable_environment).schedule_to_delete } + subject { described_class.id_in(deletable_environment).schedule_to_delete } it "schedules the record for deletion" do freeze_time do @@ -282,6 +310,72 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end + describe '.for_name_like_within_folder' do + subject { project.environments.for_name_like_within_folder(query, limit: limit) } + + let!(:environment) { create(:environment, name: 'review/test-app', project: project) } + let!(:environment_a) { create(:environment, name: 'test-app', project: project) } + let(:query) { 'test' } + let(:limit) { 5 } + + it 'returns a found name' do + is_expected.to contain_exactly(environment) + end + + it 'does not return environment without folder' do + is_expected.not_to include(environment_a) + end + + context 'when query is test-app' do + let(:query) { 'test-app' } + + it 'returns a found name' do + is_expected.to include(environment) + end + end + + context 'when query is test-app-a' do + let(:query) { 'test-app-a' } + + it 'returns empty array' do + is_expected.to be_empty + end + end + + context 'when query is empty string' do + let(:query) { '' } + let!(:environment_b) { create(:environment, name: 'review/test-app-1', project: project) } + + it 'returns only the foldered environments' do + is_expected.to contain_exactly(environment, environment_b) + end + end + + context 'when query is nil' do + let(:query) {} + + it 'raises an error' do + expect { subject }.to raise_error(NoMethodError) + end + end + + context 'when query is partially matched in the middle of environment name' do + let(:query) { 'app' } + + it 'returns empty array' do + is_expected.to be_empty + end + end + + context 'when query contains a wildcard character' do + let(:query) { 'test%' } + + it 'prevents wildcard injection' do + is_expected.to be_empty + end + end + end + describe '.auto_stoppable' do subject { described_class.auto_stoppable(limit) } @@ -1916,4 +2010,23 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end end + + describe '#deploy_freezes' do + let(:environment) { create(:environment, project: project, name: 'staging') } + let(:freeze_period) { create(:ci_freeze_period, project: project) } + + subject { environment.deploy_freezes } + + it 'returns the freeze periods of the associated project' do + expect(subject).to contain_exactly(freeze_period) + end + + it 'caches the freeze periods' do + expect(Gitlab::SafeRequestStore).to receive(:fetch) + .at_least(:once) + .and_return([freeze_period]) + + subject + end + end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 9579c4c2d27..667f3ddff72 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -RSpec.describe Event do +RSpec.describe Event, feature_category: :users do + let_it_be_with_reload(:project) { create(:project) } + describe "Associations" do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:target) } @@ -17,8 +19,6 @@ RSpec.describe Event do end describe 'Callbacks' do - let(:project) { create(:project) } - describe 'after_create :reset_project_activity' do it 'calls the reset_project_activity method' do expect_next_instance_of(described_class) do |instance| @@ -105,7 +105,7 @@ RSpec.describe Event do describe 'created_at' do it 'can find the right event' do time = 1.day.ago - event = create(:event, created_at: time) + event = create(:event, created_at: time, project: project) false_positive = create(:event, created_at: 2.days.ago) found = described_class.created_at(time) @@ -116,11 +116,11 @@ RSpec.describe Event do end describe '.for_fingerprint' do - let_it_be(:with_fingerprint) { create(:event, fingerprint: 'aaa') } + let_it_be(:with_fingerprint) { create(:event, fingerprint: 'aaa', project: project) } before_all do - create(:event) - create(:event, fingerprint: 'bbb') + create(:event, project: project) + create(:event, fingerprint: 'bbb', project: project) end it 'returns none if there is no fingerprint' do @@ -137,28 +137,43 @@ RSpec.describe Event do .to contain_exactly(with_fingerprint) end end + + describe '.contributions' do + let!(:merge_request_event) { create(:event, :created, :for_merge_request, project: project) } + let!(:issue_event) { create(:event, :created, :for_issue, project: project) } + let!(:work_item_event) { create(:event, :created, :for_work_item, project: project) } + let!(:design_event) { create(:design_event, project: project) } + + it 'returns events for MergeRequest, Issue and WorkItem' do + expect(described_class.contributions).to contain_exactly( + merge_request_event, + issue_event, + work_item_event + ) + end + end end describe '#fingerprint' do it 'is unique scoped to target' do - issue = create(:issue) - mr = create(:merge_request) + issue = create(:issue, project: project) + mr = create(:merge_request, source_project: project) - expect { create_list(:event, 2, target: issue, fingerprint: '1234') } + expect { create_list(:event, 2, target: issue, fingerprint: '1234', project: project) } .to raise_error(include('fingerprint')) expect do - create(:event, target: mr, fingerprint: 'abcd') - create(:event, target: issue, fingerprint: 'abcd') - create(:event, target: issue, fingerprint: 'efgh') + create(:event, target: mr, fingerprint: 'abcd', project: project) + create(:event, target: issue, fingerprint: 'abcd', project: project) + create(:event, target: issue, fingerprint: 'efgh', project: project) end.not_to raise_error end end describe "Push event" do - let(:project) { create(:project, :private) } - let(:user) { project.first_owner } - let(:event) { create_push_event(project, user) } + let(:private_project) { create(:project, :private) } + let(:user) { private_project.first_owner } + let(:event) { create_push_event(private_project, user) } it do expect(event.push_action?).to be_truthy @@ -171,8 +186,6 @@ RSpec.describe Event do end describe '#target_title' do - let_it_be(:project) { create(:project) } - let(:author) { project.first_owner } let(:target) { nil } @@ -303,11 +316,11 @@ RSpec.describe Event do end def visible_to_none_except(*roles) - visible_to_none.merge(roles.to_h { |role| [role, true] }) + visible_to_none.merge(roles.index_with { true }) end def visible_to_all_except(*roles) - visible_to_all.merge(roles.to_h { |role| [role, false] }) + visible_to_all.merge(roles.index_with { false }) end shared_examples 'visibility examples' do diff --git a/spec/models/factories_spec.rb b/spec/models/factories_spec.rb index 65b993cca7f..4915c0bd870 100644 --- a/spec/models/factories_spec.rb +++ b/spec/models/factories_spec.rb @@ -44,6 +44,8 @@ RSpec.describe 'factories', :saas do [:ci_pipeline_artifact, :remote_store], # EE [:dast_profile, :with_dast_site_validation], + [:dependency_proxy_manifest, :remote_store], + [:geo_dependency_proxy_manifest_state, any], [:ee_ci_build, :dependency_scanning_report], [:ee_ci_build, :license_scan_v1], [:ee_ci_job_artifact, :v1], diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb index 9d70019734b..fe22b20ecf9 100644 --- a/spec/models/generic_commit_status_spec.rb +++ b/spec/models/generic_commit_status_spec.rb @@ -20,7 +20,7 @@ RSpec.describe GenericCommitStatus do end describe '#name_uniqueness_across_types' do - let(:attributes) { {} } + let(:attributes) { { context: 'default' } } let(:commit_status) { described_class.new(attributes) } let(:status_name) { 'test-job' } @@ -39,7 +39,7 @@ RSpec.describe GenericCommitStatus do end context 'with only a pipeline' do - let(:attributes) { { pipeline: pipeline } } + let(:attributes) { { pipeline: pipeline, context: 'default' } } context 'without name' do it_behaves_like 'it does not have uniqueness errors' @@ -129,32 +129,6 @@ RSpec.describe GenericCommitStatus do end end - describe 'set_default_values' do - before do - generic_commit_status.context = nil - generic_commit_status.stage = nil - generic_commit_status.save! - end - - describe '#context' do - subject { generic_commit_status.context } - - it { is_expected.not_to be_nil } - end - - describe '#stage' do - subject { generic_commit_status.stage } - - it { is_expected.not_to be_nil } - end - - describe '#stage_idx' do - subject { generic_commit_status.stage_idx } - - it { is_expected.not_to be_nil } - end - end - describe '#present' do subject { generic_commit_status.present } diff --git a/spec/models/group_deploy_key_spec.rb b/spec/models/group_deploy_key_spec.rb index dfb4fee593f..3480e72c226 100644 --- a/spec/models/group_deploy_key_spec.rb +++ b/spec/models/group_deploy_key_spec.rb @@ -3,13 +3,13 @@ require 'spec_helper' RSpec.describe GroupDeployKey do + let_it_be(:group) { create(:group) } + let_it_be(:group_deploy_key) { create(:group_deploy_key) } + it { is_expected.to validate_presence_of(:user) } it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:groups) } - let_it_be(:group_deploy_key) { create(:group_deploy_key) } - let_it_be(:group) { create(:group) } - it 'is of type DeployKey' do expect(build(:group_deploy_key).type).to eq('DeployKey') end @@ -30,6 +30,12 @@ RSpec.describe GroupDeployKey do end end + describe '.defined_enums' do + it 'excludes the inherited enum' do + expect(described_class.defined_enums).to eq({}) + end + end + describe '#can_be_edited_for' do let_it_be(:user) { create(:user) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 6ba450b6d57..dfba0470d35 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -40,7 +40,7 @@ RSpec.describe Group do it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') } it { is_expected.to have_many(:contacts).class_name('CustomerRelations::Contact') } it { is_expected.to have_many(:organizations).class_name('CustomerRelations::Organization') } - it { is_expected.to have_many(:protected_branches) } + it { is_expected.to have_many(:protected_branches).inverse_of(:group).with_foreign_key(:namespace_id) } it { is_expected.to have_one(:crm_settings) } it { is_expected.to have_one(:group_feature) } it { is_expected.to have_one(:harbor_integration) } @@ -842,10 +842,12 @@ RSpec.describe Group do it 'returns matching records based on paths' do expect(described_class.by_ids_or_paths(nil, [group_path])).to match_array([group]) + expect(described_class.by_ids_or_paths(nil, [group_path.upcase])).to match_array([group]) end it 'returns matching records based on ids' do expect(described_class.by_ids_or_paths([group_id], nil)).to match_array([group]) + expect(described_class.by_ids_or_paths([group_id], [])).to match_array([group]) end it 'returns matching records based on both paths and ids' do @@ -853,6 +855,13 @@ RSpec.describe Group do expect(described_class.by_ids_or_paths([new_group.id], [group_path])).to match_array([group, new_group]) end + + it 'returns matching records based on full_paths' do + new_group = create(:group, parent: group) + + expect(described_class.by_ids_or_paths(nil, [new_group.full_path])).to match_array([new_group]) + expect(described_class.by_ids_or_paths(nil, [new_group.full_path.upcase])).to match_array([new_group]) + end end describe 'accessible_to_user' do @@ -1955,6 +1964,78 @@ RSpec.describe Group do end end + describe '#self_and_hierarchy_intersecting_with_user_groups' do + let_it_be(:user) { create(:user) } + let(:subject) { group.self_and_hierarchy_intersecting_with_user_groups(user) } + + it 'makes a call to GroupsFinder' do + expect(GroupsFinder).to receive_message_chain(:new, :execute, :unscope) + + subject + end + + context 'when the group is private' do + let_it_be(:group) { create(:group, :private) } + + context 'when the user is not a member of the group' do + it 'is an empty array' do + expect(subject).to eq([]) + end + end + + context 'when the user is a member of the group' do + before do + group.add_developer(user) + end + + it 'is equal to the group' do + expect(subject).to match_array([group]) + end + end + + context 'when the group has a sub group' do + let_it_be(:subgroup) { create(:group, :private, parent: group) } + + context 'when the user is not a member of the subgroup' do + it 'is an empty array' do + expect(subject).to eq([]) + end + end + + context 'when the user is a member of the subgroup' do + before do + subgroup.add_developer(user) + end + + it 'is equal to the group and subgroup' do + expect(subject).to match_array([group, subgroup]) + end + + context 'when the group has an ancestor' do + let_it_be(:ancestor) { create(:group, :private) } + + before do + group.parent = ancestor + group.save! + end + + it 'is equal to the ancestor, group and subgroup' do + expect(subject).to match_array([ancestor, group, subgroup]) + end + end + end + end + end + + context 'when the group is public' do + let_it_be(:group) { create(:group, :public) } + + it 'is equal to the public group regardless of membership' do + expect(subject).to match_array([group]) + end + end + end + describe '#update_two_factor_requirement_for_members' do let_it_be_with_reload(:user) { create(:user) } @@ -2735,7 +2816,7 @@ RSpec.describe Group do end describe 'has_project_with_service_desk_enabled?' do - let_it_be(:group) { create(:group, :private) } + let_it_be_with_refind(:group) { create(:group, :private) } subject { group.has_project_with_service_desk_enabled? } @@ -3294,6 +3375,13 @@ RSpec.describe Group do end end + describe '#work_items_mvc_feature_flag_enabled?' do + it_behaves_like 'checks self and root ancestor feature flag' do + let(:feature_flag) { :work_items_mvc } + let(:feature_flag_method) { :work_items_mvc_feature_flag_enabled? } + end + end + describe '#work_items_mvc_2_feature_flag_enabled?' do it_behaves_like 'checks self and root ancestor feature flag' do let(:feature_flag) { :work_items_mvc_2 } diff --git a/spec/models/hooks/active_hook_filter_spec.rb b/spec/models/hooks/active_hook_filter_spec.rb index 47c0fbdb106..d2a8f89041e 100644 --- a/spec/models/hooks/active_hook_filter_spec.rb +++ b/spec/models/hooks/active_hook_filter_spec.rb @@ -85,23 +85,5 @@ RSpec.describe ActiveHookFilter do it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/feature1' })).to be true } end end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(enhanced_webhook_support_regex: false) - end - - let(:hook) do - build( - :project_hook, - push_events: true, - push_events_branch_filter: '(master)', - branch_filter_strategy: 'regex' - ) - end - - it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false } - it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/(master)' })).to be true } - end end end diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb index 68c284a913c..2ece04c7158 100644 --- a/spec/models/hooks/service_hook_spec.rb +++ b/spec/models/hooks/service_hook_spec.rb @@ -11,6 +11,32 @@ RSpec.describe ServiceHook do it { is_expected.to validate_presence_of(:integration) } end + describe 'executable?' do + let!(:hooks) do + [ + [0, Time.current], + [0, 1.minute.from_now], + [1, 1.minute.from_now], + [3, 1.minute.from_now], + [4, nil], + [4, 1.day.ago], + [4, 1.minute.from_now], + [0, nil], + [0, 1.day.ago], + [1, nil], + [1, 1.day.ago], + [3, nil], + [3, 1.day.ago] + ].map do |(recent_failures, disabled_until)| + create(:service_hook, recent_failures: recent_failures, disabled_until: disabled_until) + end + end + + it 'is always true' do + expect(hooks).to all(be_executable) + end + end + describe 'execute' do let(:hook) { build(:service_hook) } let(:data) { { key: 'value' } } diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 9b55db15f3b..994d5688808 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -31,7 +31,7 @@ RSpec.describe WebHook do it { is_expected.to allow_value({ 'MY_TOKEN' => 'bar' }).for(:url_variables) } it { is_expected.to allow_value({ 'foo2' => 'bar' }).for(:url_variables) } it { is_expected.to allow_value({ 'x' => 'y' }).for(:url_variables) } - it { is_expected.to allow_value({ 'x' => ('a' * 100) }).for(:url_variables) } + it { is_expected.to allow_value({ 'x' => ('a' * 2048) }).for(:url_variables) } it { is_expected.to allow_value({ 'foo' => 'bar', 'bar' => 'baz' }).for(:url_variables) } it { is_expected.to allow_value((1..20).to_h { ["k#{_1}", 'value'] }).for(:url_variables) } it { is_expected.to allow_value({ 'MY-TOKEN' => 'bar' }).for(:url_variables) } @@ -45,7 +45,7 @@ RSpec.describe WebHook do it { is_expected.not_to allow_value({ 'bar' => :baz }).for(:url_variables) } it { is_expected.not_to allow_value({ 'bar' => nil }).for(:url_variables) } it { is_expected.not_to allow_value({ 'foo' => '' }).for(:url_variables) } - it { is_expected.not_to allow_value({ 'foo' => ('a' * 101) }).for(:url_variables) } + it { is_expected.not_to allow_value({ 'foo' => ('a' * 2049) }).for(:url_variables) } it { is_expected.not_to allow_value({ 'has spaces' => 'foo' }).for(:url_variables) } it { is_expected.not_to allow_value({ '' => 'foo' }).for(:url_variables) } it { is_expected.not_to allow_value({ '1foo' => 'foo' }).for(:url_variables) } @@ -237,16 +237,6 @@ RSpec.describe WebHook do it { is_expected.to contain_exactly(:token, :url, :url_variables) } end - describe '.web_hooks_disable_failed?' do - it 'returns true when feature is enabled for parent' do - second_hook = build(:project_hook) - stub_feature_flags(web_hooks_disable_failed: [false, second_hook.project]) - - expect(described_class.web_hooks_disable_failed?(hook)).to eq(false) - expect(described_class.web_hooks_disable_failed?(second_hook)).to eq(true) - end - end - describe 'execute' do let(:data) { { key: 'value' } } let(:hook_name) { 'project hook' } @@ -327,16 +317,6 @@ RSpec.describe WebHook do expect(described_class.where(project_id: project.id).executable).to match_array executables expect(described_class.where(project_id: project.id).disabled).to match_array not_executable end - - context 'when the feature flag is not enabled' do - before do - stub_feature_flags(web_hooks_disable_failed: false) - end - - specify 'enabled is the same as all' do - expect(described_class.where(project_id: project.id).executable).to match_array(executables + not_executable) - end - end end describe '#executable?' do @@ -384,26 +364,6 @@ RSpec.describe WebHook do it 'has the correct state' do expect(web_hook.executable?).to eq(executable) end - - context 'when the feature flag is enabled for a project' do - before do - stub_feature_flags(web_hooks_disable_failed: project) - end - - it 'has the expected value' do - expect(web_hook.executable?).to eq(executable) - end - end - - context 'when the feature flag is not enabled' do - before do - stub_feature_flags(web_hooks_disable_failed: false) - end - - it 'is executable' do - expect(web_hook).to be_executable - end - end end end @@ -643,12 +603,6 @@ RSpec.describe WebHook do it 'is true' do expect(hook).to be_temporarily_disabled end - - it 'is false when `web_hooks_disable_failed` flag is disabled' do - stub_feature_flags(web_hooks_disable_failed: false) - - expect(hook).not_to be_temporarily_disabled - end end end @@ -665,12 +619,6 @@ RSpec.describe WebHook do it 'is true' do expect(hook).to be_permanently_disabled end - - it 'is false when `web_hooks_disable_failed` flag is disabled' do - stub_feature_flags(web_hooks_disable_failed: false) - - expect(hook).not_to be_permanently_disabled - end end end @@ -730,17 +678,9 @@ RSpec.describe WebHook do expect { hook.to_json }.not_to raise_error end - it 'does not error, when serializing unsafe attributes' do - expect { hook.to_json(unsafe_serialization_hash: true) }.not_to raise_error - end - it 'does not contain binary attributes' do expect(hook.to_json).not_to include('encrypted_url_variables') end - - it 'does not contain binary attributes, even when serializing unsafe attributes' do - expect(hook.to_json(unsafe_serialization_hash: true)).not_to include('encrypted_url_variables') - end end describe '#interpolated_url' do diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 4938e1797af..78b30221a24 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -1303,8 +1303,8 @@ RSpec.describe Integration do describe '#async_execute' do let(:integration) { described_class.new(id: 123) } - let(:data) { { object_kind: 'push' } } - let(:supported_events) { %w[push] } + let(:data) { { object_kind: 'build' } } + let(:supported_events) { %w[push build] } subject(:async_execute) { integration.async_execute(data) } diff --git a/spec/models/integrations/base_chat_notification_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index b959ead2cae..67fc09fd8b5 100644 --- a/spec/models/integrations/base_chat_notification_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -34,7 +34,7 @@ RSpec.describe Integrations::BaseChatNotification do webhook: webhook_url ) - WebMock.stub_request(:post, webhook_url) + WebMock.stub_request(:post, webhook_url) if webhook_url.present? subject.active = true end @@ -55,6 +55,33 @@ RSpec.describe Integrations::BaseChatNotification do end end + context 'when webhook is blank' do + let(:webhook_url) { '' } + + it 'returns false' do + expect(chat_integration).not_to receive(:notify) + expect(chat_integration.execute(data)).to be false + end + + context 'when webhook is not required' do + it 'returns true' do + allow(chat_integration).to receive(:requires_webhook?).and_return(false) + + expect(chat_integration).to receive(:notify).and_return(true) + expect(chat_integration.execute(data)).to be true + end + end + end + + context 'when event is not supported' do + it 'returns false' do + allow(chat_integration).to receive(:supported_events).and_return(['foo']) + + expect(chat_integration).not_to receive(:notify) + expect(chat_integration.execute(data)).to be false + end + end + context 'with a project with name containing spaces' do it 'does not remove spaces' do allow(project).to receive(:full_name).and_return('Project Name') diff --git a/spec/models/integrations/base_slack_notification_spec.rb b/spec/models/integrations/base_slack_notification_spec.rb new file mode 100644 index 00000000000..8f7f4e8858d --- /dev/null +++ b/spec/models/integrations/base_slack_notification_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::BaseSlackNotification do + # This spec should only contain tests that cannot be tested through + # `base_slack_notification_shared_examples.rb`. + + describe '#metrics_key_prefix (private method)' do + it 'raises a NotImplementedError error when not defined' do + subclass = Class.new(described_class) + + expect { subclass.new.send(:metrics_key_prefix) }.to raise_error(NotImplementedError) + end + end +end diff --git a/spec/models/integrations/chat_message/pipeline_message_spec.rb b/spec/models/integrations/chat_message/pipeline_message_spec.rb index f3388853b37..413cb097327 100644 --- a/spec/models/integrations/chat_message/pipeline_message_spec.rb +++ b/spec/models/integrations/chat_message/pipeline_message_spec.rb @@ -40,8 +40,6 @@ RSpec.describe Integrations::ChatMessage::PipelineMessage do let(:has_yaml_errors) { false } - it_behaves_like Integrations::ChatMessage - before do test_commit = double("A test commit", committer: args[:user], title: "A test commit message") test_project = build(:project, name: args[:project][:name]) @@ -62,6 +60,8 @@ RSpec.describe Integrations::ChatMessage::PipelineMessage do allow(Gitlab::UrlBuilder).to receive(:build).with(args[:user]).and_return("http://example.gitlab.com/hacker") end + it_behaves_like Integrations::ChatMessage + it 'returns an empty pretext' do expect(subject.pretext).to be_empty end diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb index 6ff6888e0d3..c46face9702 100644 --- a/spec/models/integrations/drone_ci_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -37,7 +37,7 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do end end - shared_context :drone_ci_integration do + shared_context 'drone ci integration' do subject(:drone) do described_class.new( project: project, @@ -116,7 +116,7 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do end it_behaves_like Integrations::HasWebHook do - include_context :drone_ci_integration + include_context 'drone ci integration' let(:integration) { drone } let(:hook_url) { "#{drone_url}/hook?owner=#{project.namespace.full_path}&name=#{project.path}&access_token={token}" } @@ -130,14 +130,14 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do end describe "integration page/path methods" do - include_context :drone_ci_integration + include_context 'drone ci integration' it { expect(drone.build_page(sha, branch)).to eq(build_page) } it { expect(drone.commit_status_path(sha, branch)).to eq(commit_status_path) } end describe '#commit_status' do - include_context :drone_ci_integration + include_context 'drone ci integration' it 'returns the contents of the reactive cache' do stub_reactive_cache(drone, { commit_status: 'foo' }, 'sha', 'ref') @@ -147,7 +147,7 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do end describe '#calculate_reactive_cache' do - include_context :drone_ci_integration + include_context 'drone ci integration' describe '#commit_status' do subject { drone.calculate_reactive_cache(sha, branch)[:commit_status] } @@ -193,7 +193,7 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do end describe "execute" do - include_context :drone_ci_integration + include_context 'drone ci integration' let(:user) { build(:user, username: 'username') } let(:push_sample_data) do diff --git a/spec/models/integrations/flowdock_spec.rb b/spec/models/integrations/flowdock_spec.rb deleted file mode 100644 index daafb1b3958..00000000000 --- a/spec/models/integrations/flowdock_spec.rb +++ /dev/null @@ -1,54 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Integrations::Flowdock do - describe 'Validations' do - context 'when integration is active' do - before do - subject.active = true - end - - it { is_expected.to validate_presence_of(:token) } - end - - context 'when integration is inactive' do - before do - subject.active = false - end - - it { is_expected.not_to validate_presence_of(:token) } - end - end - - describe "Execute" do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:sample_data) { Gitlab::DataBuilder::Push.build_sample(project, user) } - let(:api_url) { 'https://api.flowdock.com/v1/messages' } - - subject(:flowdock_integration) { described_class.new } - - before do - allow(flowdock_integration).to receive_messages( - project_id: project.id, - project: project, - token: 'verySecret' - ) - WebMock.stub_request(:post, api_url) - end - - it "calls FlowDock API" do - flowdock_integration.execute(sample_data) - - sample_data[:commits].each do |commit| - # One request to Flowdock per new commit - next if commit[:id] == sample_data[:before] - - expect(WebMock).to have_requested(:post, api_url).with( - body: /#{commit[:id]}.*#{project.path}/ - ).once - end - end - end -end diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index af1112cf50d..a4ccae459cf 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -469,7 +469,8 @@ RSpec.describe Integrations::Jira do end describe '#client' do - subject do + it 'uses the default GitLab::HTTP timeouts' do + timeouts = Gitlab::HTTP::DEFAULT_TIMEOUT_OPTIONS stub_request(:get, 'http://jira.example.com/foo') expect(Gitlab::HTTP).to receive(:httparty_perform_request) @@ -477,32 +478,6 @@ RSpec.describe Integrations::Jira do jira_integration.client.get('/foo') end - - context 'when the FF :jira_raise_timeouts is enabled' do - let(:timeouts) do - { - open_timeout: 2.minutes, - read_timeout: 2.minutes, - write_timeout: 2.minutes - } - end - - it 'uses custom timeouts' do - subject - end - end - - context 'when the FF :jira_raise_timeouts is disabled' do - before do - stub_feature_flags(jira_raise_timeouts: false) - end - - let(:timeouts) { Gitlab::HTTP::DEFAULT_TIMEOUT_OPTIONS } - - it 'uses the default GitLab::HTTP timeouts' do - subject - end - end end describe '#find_issue' do @@ -612,7 +587,7 @@ RSpec.describe Integrations::Jira do close_issue end - it_behaves_like 'Snowplow event tracking' do + it_behaves_like 'Snowplow event tracking with RedisHLL context' do subject { close_issue } let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } @@ -968,7 +943,7 @@ RSpec.describe Integrations::Jira do subject end - it_behaves_like 'Snowplow event tracking' do + it_behaves_like 'Snowplow event tracking with RedisHLL context' do let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } let(:category) { 'Integrations::Jira' } let(:action) { 'perform_integrations_action' } diff --git a/spec/models/integrations/slack_spec.rb b/spec/models/integrations/slack_spec.rb index a12bc7f4831..218d92ffe05 100644 --- a/spec/models/integrations/slack_spec.rb +++ b/spec/models/integrations/slack_spec.rb @@ -4,5 +4,9 @@ require 'spec_helper' RSpec.describe Integrations::Slack do it_behaves_like Integrations::SlackMattermostNotifier, 'Slack' - it_behaves_like Integrations::BaseSlackNotification, factory: :integrations_slack + it_behaves_like Integrations::BaseSlackNotification, factory: :integrations_slack do + before do + stub_request(:post, integration.webhook) + end + end end diff --git a/spec/models/issue_collection_spec.rb b/spec/models/issue_collection_spec.rb deleted file mode 100644 index 183082bab26..00000000000 --- a/spec/models/issue_collection_spec.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe IssueCollection do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:issue1) { create(:issue, project: project) } - let(:issue2) { create(:issue, project: project) } - let(:collection) { described_class.new([issue1, issue2]) } - - describe '#collection' do - it 'returns the issues in the same order as the input Array' do - expect(collection.collection).to eq([issue1, issue2]) - end - end - - describe '#updatable_by_user' do - context 'using an admin user' do - it 'returns all issues' do - user = create(:admin) - - expect(collection.updatable_by_user(user)).to eq([issue1, issue2]) - end - end - - context 'using a user that has no access to the project' do - it 'returns no issues when the user is not an assignee or author' do - expect(collection.updatable_by_user(user)).to be_empty - end - - it 'returns the issues the user is assigned to' do - issue1.assignees << user - - expect(collection.updatable_by_user(user)).to eq([issue1]) - end - - it 'returns the issues for which the user is the author' do - issue1.author = user - - expect(collection.updatable_by_user(user)).to eq([issue1]) - end - end - - context 'using a user that has reporter access to the project' do - it 'returns the issues of the project' do - project.add_reporter(user) - - expect(collection.updatable_by_user(user)).to eq([issue1, issue2]) - end - end - - # TODO update when we have multiple owners of a project - # https://gitlab.com/gitlab-org/gitlab/-/issues/350605 - context 'using a user that is an owner of a project' do - it 'returns the issues of the project' do - expect(collection.updatable_by_user(project.namespace.owner)) - .to eq([issue1, issue2]) - end - end - end - - describe '#visible_to' do - it 'is an alias for updatable_by_user' do - updatable_by_user = described_class.instance_method(:updatable_by_user) - visible_to = described_class.instance_method(:visible_to) - - expect(visible_to).to eq(updatable_by_user) - end - end -end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index aea8bdaf343..7c147067714 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Issue do +RSpec.describe Issue, feature_category: :project_management do include ExternalAuthorizationServiceHelpers using RSpec::Parameterized::TableSyntax @@ -25,7 +25,7 @@ RSpec.describe Issue do it { is_expected.to have_many(:design_versions) } it { is_expected.to have_one(:sentry_issue) } it { is_expected.to have_one(:alert_management_alert) } - it { is_expected.to have_many(:alert_management_alerts) } + it { is_expected.to have_many(:alert_management_alerts).validate(false) } it { is_expected.to have_many(:resource_milestone_events) } it { is_expected.to have_many(:resource_state_events) } it { is_expected.to have_and_belong_to_many(:prometheus_alert_events) } @@ -138,6 +138,31 @@ RSpec.describe Issue do end end + describe '#allowed_work_item_type_change' do + where(:old_type, :new_type, :is_valid) do + :issue | :incident | true + :incident | :issue | true + :test_case | :issue | true + :issue | :test_case | true + :issue | :task | false + :test_case | :task | false + :incident | :task | false + :task | :issue | false + :task | :incident | false + :task | :test_case | false + end + + with_them do + it 'is possible to change type only between selected types' do + issue = create(:issue, old_type, project: reusable_project) + + issue.work_item_type_id = WorkItems::Type.default_by_type(new_type).id + + expect(issue.valid?).to eq(is_valid) + end + end + end + describe 'confidentiality' do let_it_be(:project) { create(:project) } @@ -257,7 +282,7 @@ RSpec.describe Issue do end context 'when no type was set' do - let_it_be(:issue, refind: true) { build(:issue, project: project, work_item_type: nil).tap { |issue| issue.save!(validate: false) } } + let(:issue) { build(:issue, project: project, work_item_type: nil) } it 'sets a work item type before validation' do expect(issue.work_item_type_id).to be_nil @@ -449,17 +474,6 @@ RSpec.describe Issue do end end - # TODO: Remove when NOT NULL constraint is added to the relationship - describe '#work_item_type' do - let(:issue) { build(:issue, :incident, project: reusable_project, work_item_type: nil).tap { |issue| issue.save!(validate: false) } } - - it 'returns a default type if the legacy issue does not have a work item type associated yet' do - expect(issue.work_item_type_id).to be_nil - expect(issue.issue_type).to eq('incident') - expect(issue.work_item_type).to eq(WorkItems::Type.default_by_type(:incident)) - end - end - describe '#sort' do let(:project) { reusable_project } diff --git a/spec/models/jira_connect_installation_spec.rb b/spec/models/jira_connect_installation_spec.rb index 09a4a7a488c..525690fa6b7 100644 --- a/spec/models/jira_connect_installation_spec.rb +++ b/spec/models/jira_connect_installation_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JiraConnectInstallation do +RSpec.describe JiraConnectInstallation, feature_category: :integrations do describe 'associations' do it { is_expected.to have_many(:subscriptions).class_name('JiraConnectSubscription') } end @@ -124,6 +124,20 @@ RSpec.describe JiraConnectInstallation do end end + describe 'audience_uninstalled_event_url' do + let(:installation) { build(:jira_connect_installation) } + + subject(:audience) { installation.audience_uninstalled_event_url } + + it { is_expected.to eq(nil) } + + context 'when proxy installation' do + let(:installation) { build(:jira_connect_installation, instance_url: 'https://example.com') } + + it { is_expected.to eq('https://example.com/-/jira_connect/events/uninstalled') } + end + end + describe 'proxy?' do let(:installation) { build(:jira_connect_installation) } diff --git a/spec/models/jira_import_state_spec.rb b/spec/models/jira_import_state_spec.rb index 95e9594f885..c31bedd08f3 100644 --- a/spec/models/jira_import_state_spec.rb +++ b/spec/models/jira_import_state_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JiraImportState do +RSpec.describe JiraImportState, feature_category: :integrations do describe "associations" do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:user) } diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index b98c0e8eae0..92f4d6d8531 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Key, :mailer do + it_behaves_like 'having unique enum values' + describe "Associations" do it { is_expected.to belong_to(:user) } end @@ -216,6 +218,20 @@ RSpec.describe Key, :mailer do end end end + + context 'usage type scopes' do + let_it_be(:auth_key) { create(:key, usage_type: :auth) } + let_it_be(:auth_and_signing_key) { create(:key, usage_type: :auth_and_signing) } + let_it_be(:signing_key) { create(:key, usage_type: :signing) } + + it 'auth scope returns auth and auth_and_signing keys' do + expect(described_class.auth).to match_array([auth_key, auth_and_signing_key]) + end + + it 'signing scope returns signing and auth_and_signing keys' do + expect(described_class.signing).to match_array([signing_key, auth_and_signing_key]) + end + end end context 'validation of uniqueness (based on fingerprint uniqueness)' do diff --git a/spec/models/lfs_object_spec.rb b/spec/models/lfs_object_spec.rb index c25d0451f18..e38ffd97eb9 100644 --- a/spec/models/lfs_object_spec.rb +++ b/spec/models/lfs_object_spec.rb @@ -92,65 +92,13 @@ RSpec.describe LfsObject do end end - describe '#schedule_background_upload' do + describe 'storage types' do before do stub_lfs_setting(enabled: true) end subject { create(:lfs_object, :with_file) } - context 'when object storage is disabled' do - before do - stub_lfs_object_storage(enabled: false) - end - - it 'does not schedule the migration' do - expect(ObjectStorage::BackgroundMoveWorker).not_to receive(:perform_async) - - subject - end - end - - context 'when object storage is enabled' do - context 'when background upload is enabled' do - context 'when is licensed' do - before do - stub_lfs_object_storage(background_upload: true) - end - - it 'schedules the model for migration' do - expect(ObjectStorage::BackgroundMoveWorker) - .to receive(:perform_async) - .with('LfsObjectUploader', described_class.name, :file, kind_of(Numeric)) - .once - - subject - end - - it 'schedules the model for migration once' do - expect(ObjectStorage::BackgroundMoveWorker) - .to receive(:perform_async) - .with('LfsObjectUploader', described_class.name, :file, kind_of(Numeric)) - .once - - create(:lfs_object, :with_file) - end - end - end - - context 'when background upload is disabled' do - before do - stub_lfs_object_storage(background_upload: false) - end - - it 'schedules the model for migration' do - expect(ObjectStorage::BackgroundMoveWorker).not_to receive(:perform_async) - - subject - end - end - end - describe 'file is being stored' do subject { create(:lfs_object, :with_file) } diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 2ecd10cccc6..0ebccf1cb65 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -175,26 +175,31 @@ RSpec.describe Member do end context 'member role access level' do - let_it_be(:member) { create(:group_member, access_level: Gitlab::Access::DEVELOPER) } + let_it_be_with_reload(:member) { create(:group_member, access_level: Gitlab::Access::DEVELOPER) } - context 'no member role is associated' do + context 'when no member role is associated' do it 'is valid' do expect(member).to be_valid end end - context 'member role is associated' do + context 'when member role is associated' do let!(:member_role) do - create(:member_role, members: [member], base_access_level: Gitlab::Access::DEVELOPER) + create( + :member_role, + members: [member], + base_access_level: Gitlab::Access::DEVELOPER, + namespace: member.member_namespace + ) end - context 'member role matches access level' do + context 'when member role matches access level' do it 'is valid' do expect(member).to be_valid end end - context 'member role does not match access level' do + context 'when member role does not match access level' do it 'is invalid' do member_role.base_access_level = Gitlab::Access::MAINTAINER @@ -202,13 +207,57 @@ RSpec.describe Member do end end - context 'access_level cannot be changed' do + context 'when access_level is changed' do it 'is invalid' do member.access_level = Gitlab::Access::MAINTAINER expect(member).not_to be_valid - expect(member.errors.full_messages).to include( - "Access level cannot be changed since member is associated with a custom role" + expect(member.errors[:access_level]).to include( + _("cannot be changed since member is associated with a custom role") + ) + end + end + end + end + + context 'member role namespace' do + let_it_be_with_reload(:member) { create(:group_member) } + + context 'when no member role is associated' do + it 'is valid' do + expect(member).to be_valid + end + end + + context 'when member role is associated' do + let_it_be(:member_role) do + create(:member_role, members: [member], namespace: member.group, base_access_level: member.access_level) + end + + context 'when member#member_namespace is a group within hierarchy of member_role#namespace' do + it 'is valid' do + member.member_namespace = create(:group, parent: member_role.namespace) + + expect(member).to be_valid + end + end + + context 'when member#member_namespace is a project within hierarchy of member_role#namespace' do + it 'is valid' do + project = create(:project, group: member_role.namespace) + member.member_namespace = Namespace.find(project.parent_id) + + expect(member).to be_valid + end + end + + context 'when member#member_namespace is outside hierarchy of member_role#namespace' do + it 'is invalid' do + member.member_namespace = create(:group) + + expect(member).not_to be_valid + expect(member.errors[:member_namespace]).to include( + _("must be in same hierarchy as custom role's namespace") ) end end @@ -248,7 +297,7 @@ RSpec.describe Member do accepted_invite_user = build(:user, state: :active) @accepted_invite_member = create(:project_member, :invited, :developer, project: project) - .tap { |u| u.accept_invite!(accepted_invite_user) } + .tap { |u| u.accept_invite!(accepted_invite_user) } requested_user = create(:user).tap { |u| project.request_access(u) } @requested_member = project.requesters.find_by(user_id: requested_user.id) @@ -612,14 +661,14 @@ RSpec.describe Member do subject { described_class.authorizable.to_a } it 'includes the member who has an associated user record,'\ - 'but also having an invite_token' do - member = create(:project_member, - :developer, - :invited, - user: create(:user)) + 'but also having an invite_token' do + member = create(:project_member, + :developer, + :invited, + user: create(:user)) - expect(subject).to include(member) - end + expect(subject).to include(member) + end it { is_expected.to include @owner } it { is_expected.to include @maintainer } @@ -750,9 +799,35 @@ RSpec.describe Member do end describe '#request?' do - subject { create(:project_member, requested_at: Time.current.utc) } + context 'when request for project' do + subject { create(:project_member, requested_at: Time.current.utc) } + + it 'calls notification service but not todo service' do + expect_next_instance_of(NotificationService) do |instance| + expect(instance).to receive(:new_access_request) + end - it { is_expected.to be_request } + expect(TodoService).not_to receive(:new) + + is_expected.to be_request + end + end + + context 'when request for group' do + subject { create(:group_member, requested_at: Time.current.utc) } + + it 'calls notification and todo service' do + expect_next_instance_of(NotificationService) do |instance| + expect(instance).to receive(:new_access_request) + end + + expect_next_instance_of(TodoService) do |instance| + expect(instance).to receive(:create_member_access_request) + end + + is_expected.to be_request + end + end end describe '#pending?' do diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 77bc6d9753f..4ac7ce95b84 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -120,6 +120,40 @@ RSpec.describe GroupMember do end end + describe '#last_owner_of_the_group?' do + context 'when member is an owner' do + let_it_be(:group_member) { build(:group_member, :owner) } + + using RSpec::Parameterized::TableSyntax + + where(:member_last_owner?, :member_last_blocked_owner?, :expected) do + false | false | false + true | false | true + false | true | true + true | true | true + end + + with_them do + it "returns expected" do + allow(group_member.group).to receive(:member_last_owner?).with(group_member).and_return(member_last_owner?) + allow(group_member.group).to receive(:member_last_blocked_owner?) + .with(group_member) + .and_return(member_last_blocked_owner?) + + expect(group_member.last_owner_of_the_group?).to be(expected) + end + end + end + + context 'when member is not an owner' do + let_it_be(:group_member) { build(:group_member, :guest) } + + subject { group_member.last_owner_of_the_group? } + + it { is_expected.to be(false) } + end + end + context 'access levels' do context 'with parent group' do it_behaves_like 'inherited access level as a member of entity' do diff --git a/spec/models/members/member_role_spec.rb b/spec/models/members/member_role_spec.rb index e2691e2e78c..f9d6757bb90 100644 --- a/spec/models/members/member_role_spec.rb +++ b/spec/models/members/member_role_spec.rb @@ -9,39 +9,50 @@ RSpec.describe MemberRole do end describe 'validation' do - subject { described_class.new } + subject(:member_role) { build(:member_role) } it { is_expected.to validate_presence_of(:namespace) } it { is_expected.to validate_presence_of(:base_access_level) } - context 'for namespace' do - subject { build(:member_role) } - + context 'when for namespace' do let_it_be(:root_group) { create(:group) } context 'when namespace is a subgroup' do it 'is invalid' do subgroup = create(:group, parent: root_group) - subject.namespace = subgroup + member_role.namespace = subgroup - expect(subject).to be_invalid + expect(member_role).to be_invalid + expect(member_role.errors[:namespace]).to include( + s_("MemberRole|must be top-level namespace") + ) end end context 'when namespace is a root group' do it 'is valid' do - subject.namespace = root_group + member_role.namespace = root_group - expect(subject).to be_valid + expect(member_role).to be_valid end end context 'when namespace is not present' do it 'is invalid with a different error message' do - subject.namespace = nil + member_role.namespace = nil + + expect(member_role).to be_invalid + expect(member_role.errors[:namespace]).to include(_("can't be blank")) + end + end + + context 'when namespace is outside hierarchy of member' do + it 'creates a validation error' do + member_role.save! + member_role.namespace = create(:group) - expect(subject).to be_invalid - expect(subject.errors.full_messages).to eq(["Namespace can't be blank"]) + expect(member_role).not_to be_valid + expect(member_role.errors[:namespace]).to include(s_("MemberRole|can't be changed")) end end end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index e56c6b38992..d573fde5a74 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -85,6 +85,27 @@ RSpec.describe ProjectMember do end end + describe '#holder_of_the_personal_namespace?' do + let_it_be(:project_member) { build(:project_member) } + + using RSpec::Parameterized::TableSyntax + + where(:personal_namespace_holder?, :expected) do + false | false + true | true + end + + with_them do + it "returns expected" do + allow(project_member.project).to receive(:personal_namespace_holder?) + .with(project_member.user) + .and_return(personal_namespace_holder?) + + expect(project_member.holder_of_the_personal_namespace?).to be(expected) + end + end + end + describe '.import_team' do before do @project_1 = create(:project) diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 22fed716897..a17b90930f0 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -507,6 +507,50 @@ RSpec.describe MergeRequestDiff do end end + describe '#paginated_diffs' do + context 'when no persisted files available' do + before do + diff_with_commits.clean! + end + + it 'returns a Gitlab::Diff::FileCollection::Compare' do + diffs = diff_with_commits.paginated_diffs(1, 10) + + expect(diffs).to be_a(Gitlab::Diff::FileCollection::Compare) + expect(diffs.diff_files.size).to eq(10) + end + end + + context 'when persisted files available' do + it 'returns paginated diffs' do + diffs = diff_with_commits.paginated_diffs(1, 10) + + expect(diffs).to be_a(Gitlab::Diff::FileCollection::PaginatedMergeRequestDiff) + expect(diffs.diff_files.size).to eq(10) + end + + it 'sorts diff files directory first' do + diff_with_commits.update!(sorted: false) # Mark as unsorted so it'll re-order + + # There will be 11 returned, as we have to take into account for new and old paths + expect(diff_with_commits.paginated_diffs(1, 10).diff_paths).to eq( + [ + 'bar/branch-test.txt', + 'custom-highlighting/test.gitlab-custom', + 'encoding/iso8859.txt', + 'files/images/wm.svg', + 'files/js/commit.js.coffee', + 'files/js/commit.coffee', + 'files/lfs/lfs_object.iso', + 'files/ruby/popen.rb', + 'files/ruby/regex.rb', + 'files/.DS_Store', + 'files/whitespace' + ]) + end + end + end + describe '#diffs' do let(:diff_options) { {} } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index cf4f58f558c..05586cbfc64 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -30,6 +30,7 @@ RSpec.describe MergeRequest, factory_default: :keep do it { is_expected.to have_many(:resource_state_events) } it { is_expected.to have_many(:draft_notes) } it { is_expected.to have_many(:reviews).inverse_of(:merge_request) } + it { is_expected.to have_many(:reviewed_by_users).through(:reviews).source(:author) } it { is_expected.to have_one(:cleanup_schedule).inverse_of(:merge_request) } it { is_expected.to have_many(:created_environments).class_name('Environment').inverse_of(:merge_request) } @@ -46,6 +47,20 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(project.merge_requests.find(merge_request.id)).to eq(merge_request) end end + + describe '#reviewed_by_users' do + let!(:merge_request) { create(:merge_request) } + + context 'when the same user has several reviews' do + before do + 2.times { create(:review, merge_request: merge_request, project: merge_request.project, author: merge_request.author) } + end + + it 'returns distinct users' do + expect(merge_request.reviewed_by_users).to match_array([merge_request.author]) + end + end + end end describe '.from_and_to_forks' do @@ -4229,6 +4244,18 @@ RSpec.describe MergeRequest, factory_default: :keep do transition! end + + context 'when transaction is not committed' do + it_behaves_like 'transition not triggering mergeRequestMergeStatusUpdated GraphQL subscription' do + def transition! + MergeRequest.transaction do + super + + raise ActiveRecord::Rollback + end + end + end + end end shared_examples 'for an invalid state transition' do diff --git a/spec/models/milestone_note_spec.rb b/spec/models/milestone_note_spec.rb index db1a7ca05f8..9371cef7540 100644 --- a/spec/models/milestone_note_spec.rb +++ b/spec/models/milestone_note_spec.rb @@ -20,6 +20,8 @@ RSpec.describe MilestoneNote do it 'creates the expected note' do expect(subject.note_html).to include('removed milestone') expect(subject.note_html).not_to include('changed milestone to') + expect(subject.created_at).to eq(event.created_at) + expect(subject.updated_at).to eq(event.created_at) end end end diff --git a/spec/models/ml/candidate_metadata_spec.rb b/spec/models/ml/candidate_metadata_spec.rb new file mode 100644 index 00000000000..94e21a910be --- /dev/null +++ b/spec/models/ml/candidate_metadata_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ml::CandidateMetadata, feature_category: :mlops do + describe 'associations' do + it { is_expected.to belong_to(:candidate) } + end + + describe 'uniqueness of name' do + let_it_be(:metadata) { create(:ml_candidate_metadata, name: 'some_metadata') } + let_it_be(:candidate) { metadata.candidate } + + it 'is unique within candidate' do + expect do + candidate.metadata.create!(name: 'some_metadata', value: 'blah') + end.to raise_error.with_message(/Name 'some_metadata' already taken/) + end + end +end diff --git a/spec/models/ml/candidate_spec.rb b/spec/models/ml/candidate_spec.rb index b35496363fe..9ce411191f0 100644 --- a/spec/models/ml/candidate_spec.rb +++ b/spec/models/ml/candidate_spec.rb @@ -5,11 +5,18 @@ require 'spec_helper' RSpec.describe Ml::Candidate, factory_default: :keep do let_it_be(:candidate) { create(:ml_candidates, :with_metrics_and_params) } + let(:project) { candidate.experiment.project } + describe 'associations' do it { is_expected.to belong_to(:experiment) } it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:params) } it { is_expected.to have_many(:metrics) } + it { is_expected.to have_many(:metadata) } + end + + describe 'default values' do + it { expect(described_class.new.iid).to be_present } end describe '.artifact_root' do @@ -18,8 +25,38 @@ RSpec.describe Ml::Candidate, factory_default: :keep do it { is_expected.to eq("/ml_candidate_#{candidate.iid}/-/") } end - describe 'default values' do - it { expect(described_class.new.iid).to be_present } + describe '.package_name' do + subject { candidate.package_name } + + it { is_expected.to eq("ml_candidate_#{candidate.iid}") } + end + + describe '.package_version' do + subject { candidate.package_version } + + it { is_expected.to eq('-') } + end + + describe '.artifact' do + subject { candidate.artifact } + + context 'when has logged artifacts' do + let(:package) do + create(:generic_package, name: candidate.package_name, version: candidate.package_version, project: project) + end + + it 'returns the package' do + package + + is_expected.to eq(package) + end + end + + context 'when does not have logged artifacts' do + let(:tested_candidate) { create(:ml_candidates, :with_metrics_and_params) } + + it { is_expected.to be_nil } + end end describe '#by_project_id_and_iid' do diff --git a/spec/models/ml/experiment_metadata_spec.rb b/spec/models/ml/experiment_metadata_spec.rb new file mode 100644 index 00000000000..e989d495a1c --- /dev/null +++ b/spec/models/ml/experiment_metadata_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ml::ExperimentMetadata, feature_category: :mlops do + describe 'associations' do + it { is_expected.to belong_to(:experiment) } + end + + describe 'uniqueness of name' do + let_it_be(:metadata) { create(:ml_experiment_metadata, name: 'some_metadata') } + let_it_be(:experiment) { metadata.experiment } + + it 'is unique within experiment' do + expect do + experiment.metadata.create!(name: 'some_metadata', value: 'blah') + end.to raise_error.with_message(/Name 'some_metadata' already taken/) + end + end +end diff --git a/spec/models/ml/experiment_spec.rb b/spec/models/ml/experiment_spec.rb index 789bb3aa88a..52e9f9217f5 100644 --- a/spec/models/ml/experiment_spec.rb +++ b/spec/models/ml/experiment_spec.rb @@ -13,6 +13,7 @@ RSpec.describe Ml::Experiment do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:candidates) } + it { is_expected.to have_many(:metadata) } end describe '#by_project_id_and_iid' do diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index 17c49e13c85..e06a6a30f9a 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -178,6 +178,63 @@ RSpec.describe NamespaceSetting, type: :model do end end + describe '#runner_registration_enabled?' do + context 'when not a subgroup' do + let_it_be(:settings) { create(:namespace_settings) } + let_it_be(:group) { create(:group, namespace_settings: settings) } + + before do + group.update!(runner_registration_enabled: runner_registration_enabled) + end + + context 'when :runner_registration_enabled is false' do + let(:runner_registration_enabled) { false } + + it 'returns false' do + expect(group.runner_registration_enabled?).to be_falsey + end + + it 'does not query the db' do + expect { group.runner_registration_enabled? }.not_to exceed_query_limit(0) + end + end + + context 'when :runner_registration_enabled is true' do + let(:runner_registration_enabled) { true } + + it 'returns true' do + expect(group.runner_registration_enabled?).to be_truthy + end + end + end + + context 'when a group has parent groups' do + let_it_be(:grandparent) { create(:group) } + let_it_be(:parent) { create(:group, parent: grandparent) } + let_it_be(:group) { create(:group, parent: parent) } + + before do + grandparent.update!(runner_registration_enabled: runner_registration_enabled) + end + + context 'when a parent group has runner registration disabled' do + let(:runner_registration_enabled) { false } + + it 'returns false' do + expect(group.runner_registration_enabled?).to be_falsey + end + end + + context 'when all parent groups have runner registration enabled' do + let(:runner_registration_enabled) { true } + + it 'returns true' do + expect(group.runner_registration_enabled?).to be_truthy + end + end + end + end + describe '#delayed_project_removal' do it_behaves_like 'a cascading namespace setting boolean attribute', settings_attribute_name: :delayed_project_removal end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 0516d446945..80721e11049 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -34,6 +34,7 @@ RSpec.describe Namespace do it { is_expected.to have_many :member_roles } it { is_expected.to have_one :cluster_enabled_grant } it { is_expected.to have_many(:work_items) } + it { is_expected.to have_many :achievements } it do is_expected.to have_one(:ci_cd_settings).class_name('NamespaceCiCdSetting').inverse_of(:namespace).autosave(true) @@ -1895,6 +1896,30 @@ RSpec.describe Namespace do end end + describe '#bot_user_namespace?' do + subject { namespace.bot_user_namespace? } + + context 'when owner is a bot user user' do + let(:user) { create(:user, :project_bot) } + let(:namespace) { user.namespace } + + it { is_expected.to be_truthy } + end + + context 'when owner is a non-bot user' do + let(:user) { create(:user) } + let(:namespace) { user.namespace } + + it { is_expected.to be_falsy } + end + + context 'when type is a group' do + let(:namespace) { create(:group) } + + it { is_expected.to be_falsy } + end + end + describe '#aggregation_scheduled?' do let(:namespace) { create(:namespace) } diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 7c71080d63e..328d3ba7dda 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -579,6 +579,7 @@ RSpec.describe Note do expect(commit_note.confidential?).to be_falsy end end + context 'when note is confidential' do it 'is true even when a noteable is not confidential' do issue = create(:issue, confidential: false) diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb index 068166ebb0d..2275bea4c7f 100644 --- a/spec/models/notification_recipient_spec.rb +++ b/spec/models/notification_recipient_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe NotificationRecipient do +RSpec.describe NotificationRecipient, feature_category: :team_planning do let(:user) { create(:user) } let(:project) { create(:project, namespace: user.namespace) } let(:target) { create(:issue, project: project) } diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index c665f738ead..a244ed34e54 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -104,15 +104,9 @@ RSpec.describe Packages::PackageFile, type: :model do let_it_be(:package, reload: true) { create(:package) } context 'when the package file has an explicit size' do - it_behaves_like 'UpdateProjectStatistics' do - subject { build(:package_file, :jar, package: package, size: 42) } - end - end + subject { build(:package_file, :jar, package: package, size: 42) } - context 'when the package file does not have a size' do - it_behaves_like 'UpdateProjectStatistics' do - subject { build(:package_file, package: package, size: nil) } - end + it_behaves_like 'UpdateProjectStatistics', :packages_size end end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 241c585099c..d6f71f2401c 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -708,12 +708,14 @@ RSpec.describe Packages::Package, type: :model do describe '#destroy' do let(:package) { create(:npm_package) } let(:package_file) { package.package_files.first } - let(:project_statistics) { ProjectStatistics.for_project_ids(package.project.id).first } + let(:project_statistics) { package.project.statistics } - it 'affects project statistics' do - expect { package.destroy! } - .to change { project_statistics.reload.packages_size } - .from(package_file.size).to(0) + subject(:destroy!) { package.destroy! } + + it 'updates the project statistics' do + expect(project_statistics).to receive(:increment_counter).with(:packages_size, -package_file.size) + + destroy! end end diff --git a/spec/models/packages/rpm/repository_file_spec.rb b/spec/models/packages/rpm/repository_file_spec.rb index 34347793dd8..1147fd66ac6 100644 --- a/spec/models/packages/rpm/repository_file_spec.rb +++ b/spec/models/packages/rpm/repository_file_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Packages::Rpm::RepositoryFile, type: :model do +RSpec.describe Packages::Rpm::RepositoryFile, type: :model, feature_category: :package_registry do using RSpec::Parameterized::TableSyntax let_it_be(:repository_file) { create(:rpm_repository_file) } @@ -16,6 +16,32 @@ RSpec.describe Packages::Rpm::RepositoryFile, type: :model do it { is_expected.to validate_presence_of(:project) } end + describe '.has_oversized_filelists?' do + let_it_be(:filelists) { create(:rpm_repository_file, :filelists, size: 21.megabytes) } + + subject { described_class.has_oversized_filelists?(project_id: filelists.project_id) } + + context 'when has oversized filelists' do + it { expect(subject).to be true } + end + + context 'when filelists.xml is not oversized' do + before do + filelists.update!(size: 19.megabytes) + end + + it { expect(subject).to be_falsey } + end + + context 'when there is no filelists.xml' do + before do + filelists.destroy! + end + + it { expect(subject).to be_falsey } + end + end + context 'when updating project statistics' do context 'when the package file has an explicit size' do it_behaves_like 'UpdateProjectStatistics' do diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 2d7ee8ba3be..6f684eceaec 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -74,6 +74,12 @@ RSpec.describe Pages::LookupPath do end end + it 'does not recreate source hash' do + expect(deployment.file).to receive(:url_or_file_path).once + + 2.times { lookup_path.source } + end + context 'when deployment is in the local storage' do before do deployment.file.migrate!(::ObjectStorage::Store::LOCAL) diff --git a/spec/models/pages_deployment_spec.rb b/spec/models/pages_deployment_spec.rb index a27d836e2c2..268c5006a88 100644 --- a/spec/models/pages_deployment_spec.rb +++ b/spec/models/pages_deployment_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe PagesDeployment do +RSpec.describe PagesDeployment, feature_category: :pages do let_it_be(:project) { create(:project) } describe 'associations' do diff --git a/spec/models/performance_monitoring/prometheus_dashboard_spec.rb b/spec/models/performance_monitoring/prometheus_dashboard_spec.rb index ee2407f21b6..21b16bdeb17 100644 --- a/spec/models/performance_monitoring/prometheus_dashboard_spec.rb +++ b/spec/models/performance_monitoring/prometheus_dashboard_spec.rb @@ -229,83 +229,37 @@ RSpec.describe PerformanceMonitoring::PrometheusDashboard do allow(Gitlab::Metrics::Dashboard::Finder).to receive(:find_raw).with(project, dashboard_path: path).and_call_original end - context 'metrics_dashboard_exhaustive_validations is on' do - before do - stub_feature_flags(metrics_dashboard_exhaustive_validations: true) - end - - context 'when schema is valid' do - let(:dashboard_schema) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml')) } + context 'when schema is valid' do + let(:dashboard_schema) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml')) } - it 'returns empty array' do - expect(Gitlab::Metrics::Dashboard::Validator).to receive(:errors).with(dashboard_schema, dashboard_path: path, project: project).and_return([]) + it 'returns empty array' do + expect(described_class).to receive(:from_json).with(dashboard_schema) - expect(schema_validation_warnings).to eq [] - end - end - - context 'when schema is invalid' do - let(:dashboard_schema) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/dashboard_missing_panel_groups.yml')) } - - it 'returns array with errors messages' do - error = ::Gitlab::Metrics::Dashboard::Validator::Errors::SchemaValidationError.new - - expect(Gitlab::Metrics::Dashboard::Validator).to receive(:errors).with(dashboard_schema, dashboard_path: path, project: project).and_return([error]) - - expect(schema_validation_warnings).to eq [error.message] - end - end - - context 'when YAML has wrong syntax' do - let(:project) { create(:project, :repository, :custom_repo, files: { path => fixture_file('lib/gitlab/metrics/dashboard/broken_yml_syntax.yml') }) } - - subject(:schema_validation_warnings) { described_class.new(path: path, environment: environment).schema_validation_warnings } - - it 'returns array with errors messages' do - expect(Gitlab::Metrics::Dashboard::Validator).not_to receive(:errors) - - expect(schema_validation_warnings).to eq ['Invalid yaml'] - end + expect(schema_validation_warnings).to eq [] end end - context 'metrics_dashboard_exhaustive_validations is off' do - before do - stub_feature_flags(metrics_dashboard_exhaustive_validations: false) - end - - context 'when schema is valid' do - let(:dashboard_schema) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml')) } - - it 'returns empty array' do - expect(described_class).to receive(:from_json).with(dashboard_schema) - - expect(schema_validation_warnings).to eq [] - end - end - - context 'when schema is invalid' do - let(:dashboard_schema) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/dashboard_missing_panel_groups.yml')) } + context 'when schema is invalid' do + let(:dashboard_schema) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/dashboard_missing_panel_groups.yml')) } - it 'returns array with errors messages' do - instance = described_class.new - instance.errors.add(:test, 'test error') + it 'returns array with errors messages' do + instance = described_class.new + instance.errors.add(:test, 'test error') - expect(described_class).to receive(:from_json).and_raise(ActiveModel::ValidationError.new(instance)) - expect(described_class.new.schema_validation_warnings).to eq ['test: test error'] - end + expect(described_class).to receive(:from_json).and_raise(ActiveModel::ValidationError.new(instance)) + expect(described_class.new.schema_validation_warnings).to eq ['test: test error'] end + end - context 'when YAML has wrong syntax' do - let(:project) { create(:project, :repository, :custom_repo, files: { path => fixture_file('lib/gitlab/metrics/dashboard/broken_yml_syntax.yml') }) } + context 'when YAML has wrong syntax' do + let(:project) { create(:project, :repository, :custom_repo, files: { path => fixture_file('lib/gitlab/metrics/dashboard/broken_yml_syntax.yml') }) } - subject(:schema_validation_warnings) { described_class.new(path: path, environment: environment).schema_validation_warnings } + subject(:schema_validation_warnings) { described_class.new(path: path, environment: environment).schema_validation_warnings } - it 'returns array with errors messages' do - expect(described_class).not_to receive(:from_json) + it 'returns array with errors messages' do + expect(described_class).not_to receive(:from_json) - expect(schema_validation_warnings).to eq ['Invalid yaml'] - end + expect(schema_validation_warnings).to eq ['Invalid yaml'] end end end diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index f9c458b2c80..d4e550657c8 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -200,6 +200,7 @@ RSpec.describe PlanLimits do ci_max_artifact_size_cluster_applications ci_max_artifact_size_secret_detection ci_max_artifact_size_requirements + ci_max_artifact_size_requirements_v2 ci_max_artifact_size_coverage_fuzzing ci_max_artifact_size_api_fuzzing ] diff --git a/spec/models/programming_language_spec.rb b/spec/models/programming_language_spec.rb index b202c10e30b..403cd77c707 100644 --- a/spec/models/programming_language_spec.rb +++ b/spec/models/programming_language_spec.rb @@ -3,6 +3,10 @@ require 'spec_helper' RSpec.describe ProgrammingLanguage do + let_it_be(:ruby) { create(:programming_language, name: 'Ruby') } + let_it_be(:python) { create(:programming_language, name: 'Python') } + let_it_be(:swift) { create(:programming_language, name: 'Swift') } + it { is_expected.to respond_to(:name) } it { is_expected.to respond_to(:color) } @@ -12,10 +16,6 @@ RSpec.describe ProgrammingLanguage do it { is_expected.not_to allow_value("#0z0000").for(:color) } describe '.with_name_case_insensitive scope' do - let_it_be(:ruby) { create(:programming_language, name: 'Ruby') } - let_it_be(:python) { create(:programming_language, name: 'Python') } - let_it_be(:swift) { create(:programming_language, name: 'Swift') } - it 'accepts a single name parameter' do expect(described_class.with_name_case_insensitive('swift')).to( contain_exactly(swift) @@ -28,4 +28,17 @@ RSpec.describe ProgrammingLanguage do ) end end + + describe '.most_popular' do + before do + create_list(:repository_language, 3, programming_language_id: ruby.id) + create(:repository_language, programming_language_id: python.id) + create_list(:repository_language, 2, programming_language_id: swift.id) + ApplicationRecord.connection.execute('analyze repository_languages') + end + + it 'returns the most popular programming languages' do + expect(described_class.most_popular(2)).to(contain_exactly(ruby, swift)) + end + end end diff --git a/spec/models/project_export_job_spec.rb b/spec/models/project_export_job_spec.rb index 653d4d2df27..01b0aaff0ff 100644 --- a/spec/models/project_export_job_spec.rb +++ b/spec/models/project_export_job_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ProjectExportJob, type: :model do +RSpec.describe ProjectExportJob, feature_category: :importers, type: :model do describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:relation_exports) } @@ -13,4 +13,54 @@ RSpec.describe ProjectExportJob, type: :model do it { is_expected.to validate_presence_of(:jid) } it { is_expected.to validate_presence_of(:status) } end + + context 'when pruning expired jobs' do + let_it_be(:old_job_1) { create(:project_export_job, updated_at: 37.months.ago) } + let_it_be(:old_job_2) { create(:project_export_job, updated_at: 12.months.ago) } + let_it_be(:old_job_3) { create(:project_export_job, updated_at: 8.days.ago) } + let_it_be(:fresh_job_1) { create(:project_export_job, updated_at: 1.day.ago) } + let_it_be(:fresh_job_2) { create(:project_export_job, updated_at: 2.days.ago) } + let_it_be(:fresh_job_3) { create(:project_export_job, updated_at: 6.days.ago) } + + let_it_be(:old_relation_export_1) { create(:project_relation_export, project_export_job_id: old_job_1.id) } + let_it_be(:old_relation_export_2) { create(:project_relation_export, project_export_job_id: old_job_2.id) } + let_it_be(:old_relation_export_3) { create(:project_relation_export, project_export_job_id: old_job_3.id) } + let_it_be(:fresh_relation_export_1) { create(:project_relation_export, project_export_job_id: fresh_job_1.id) } + + let_it_be(:old_upload_1) { create(:relation_export_upload, project_relation_export_id: old_relation_export_1.id) } + let_it_be(:old_upload_2) { create(:relation_export_upload, project_relation_export_id: old_relation_export_2.id) } + let_it_be(:old_upload_3) { create(:relation_export_upload, project_relation_export_id: old_relation_export_3.id) } + let_it_be(:fresh_upload_1) do + create( + :relation_export_upload, + project_relation_export_id: fresh_relation_export_1.id + ) + end + + it 'prunes jobs and associations older than 7 days' do + expect { described_class.prune_expired_jobs }.to change { described_class.count }.by(-3) + + expect(described_class.find_by(id: old_job_1.id)).to be_nil + expect(described_class.find_by(id: old_job_2.id)).to be_nil + expect(described_class.find_by(id: old_job_3.id)).to be_nil + + expect(Projects::ImportExport::RelationExport.find_by(id: old_relation_export_1.id)).to be_nil + expect(Projects::ImportExport::RelationExport.find_by(id: old_relation_export_2.id)).to be_nil + expect(Projects::ImportExport::RelationExport.find_by(id: old_relation_export_3.id)).to be_nil + + expect(Projects::ImportExport::RelationExportUpload.find_by(id: old_upload_1.id)).to be_nil + expect(Projects::ImportExport::RelationExportUpload.find_by(id: old_upload_2.id)).to be_nil + expect(Projects::ImportExport::RelationExportUpload.find_by(id: old_upload_3.id)).to be_nil + end + + it 'does not delete associated records for jobs younger than 7 days' do + described_class.prune_expired_jobs + + expect(fresh_job_1.reload).to be_present + expect(fresh_job_2.reload).to be_present + expect(fresh_job_3.reload).to be_present + expect(fresh_relation_export_1.reload).to be_present + expect(fresh_upload_1.reload).to be_present + end + end end diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index dae0f84eda3..fb6aaffdf22 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -291,6 +291,7 @@ RSpec.describe ProjectFeature do end end end + # rubocop:disable Gitlab/FeatureAvailableUsage describe '#feature_available?' do let(:features) { ProjectFeature::FEATURES } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1cae03ae2ae..f33001b9c5b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -21,7 +21,6 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to belong_to(:creator).class_name('User') } it { is_expected.to belong_to(:pool_repository) } it { is_expected.to have_many(:users) } - it { is_expected.to have_many(:integrations) } it { is_expected.to have_many(:events) } it { is_expected.to have_many(:merge_requests) } it { is_expected.to have_many(:merge_request_metrics).class_name('MergeRequest::Metrics') } @@ -58,7 +57,6 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_one(:pipelines_email_integration) } it { is_expected.to have_one(:irker_integration) } it { is_expected.to have_one(:pivotaltracker_integration) } - it { is_expected.to have_one(:flowdock_integration) } it { is_expected.to have_one(:assembla_integration) } it { is_expected.to have_one(:slack_slash_commands_integration) } it { is_expected.to have_one(:mattermost_slash_commands_integration) } @@ -151,6 +149,20 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:project_callouts).class_name('Users::ProjectCallout').with_foreign_key(:project_id) } it { is_expected.to have_many(:pipeline_metadata).class_name('Ci::PipelineMetadata') } it { is_expected.to have_many(:incident_management_timeline_event_tags).class_name('IncidentManagement::TimelineEventTag') } + it { is_expected.to have_many(:integrations) } + it { is_expected.to have_many(:push_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:tag_push_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:issue_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:confidential_issue_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:merge_request_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:note_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:confidential_note_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:job_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:archive_trace_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:pipeline_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:wiki_page_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:deployment_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:alert_hooks_integrations).class_name('Integration') } # GitLab Pages it { is_expected.to have_many(:pages_domains) } @@ -356,6 +368,33 @@ RSpec.describe Project, factory_default: :keep do subject.ci_pipelines end end + + context 'order of the `has_many :notes` association' do + let(:associations_having_dependent_destroy) do + described_class.reflect_on_all_associations(:has_many).select do |assoc| + assoc.options[:dependent] == :destroy + end + end + + let(:associations_having_dependent_destroy_with_issuable_included) do + associations_having_dependent_destroy.select do |association| + association.klass.include?(Issuable) + end + end + + it 'has `has_many :notes` as the first association among all the other associations that'\ + 'includes the `Issuable` module' do + names_of_associations_having_dependent_destroy = associations_having_dependent_destroy.map(&:name) + index_of_has_many_notes_association = names_of_associations_having_dependent_destroy.find_index(:notes) + + associations_having_dependent_destroy_with_issuable_included.each do |issuable_included_association| + index_of_issuable_included_association = + names_of_associations_having_dependent_destroy.find_index(issuable_included_association.name) + + expect(index_of_has_many_notes_association).to be < index_of_issuable_included_association + end + end + end end describe 'modules' do @@ -5759,6 +5798,32 @@ RSpec.describe Project, factory_default: :keep do integration.project.execute_integrations(anything, :merge_request_hooks) end + + it 'does not trigger extra queries when called multiple times' do + integration.project.execute_integrations({}, :push_hooks) + + recorder = ActiveRecord::QueryRecorder.new do + integration.project.execute_integrations({}, :push_hooks) + end + + expect(recorder.count).to be_zero + end + + context 'with cache_project_integrations disabled' do + before do + stub_feature_flags(cache_project_integrations: false) + end + + it 'triggers extra queries when called multiple times' do + integration.project.execute_integrations({}, :push_hooks) + + recorder = ActiveRecord::QueryRecorder.new do + integration.project.execute_integrations({}, :push_hooks) + end + + expect(recorder.count).not_to be_zero + end + end end describe '#has_active_hooks?' do @@ -8156,6 +8221,16 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#work_items_mvc_feature_flag_enabled?' do + let_it_be(:group_project) { create(:project, :in_subgroup) } + + it_behaves_like 'checks parent group feature flag' do + let(:feature_flag_method) { :work_items_mvc_feature_flag_enabled? } + let(:feature_flag) { :work_items_mvc } + let(:subject_project) { group_project } + end + end + describe '#work_items_mvc_2_feature_flag_enabled?' do let_it_be(:group_project) { create(:project, :in_subgroup) } @@ -8370,6 +8445,105 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to be(false) } end + describe '.cascading_with_parent_namespace' do + let_it_be_with_reload(:group) { create(:group, :with_root_storage_statistics) } + let_it_be_with_reload(:subgroup) { create(:group, parent: group) } + let_it_be_with_reload(:project) { create(:project, group: subgroup) } + let_it_be_with_reload(:project_without_group) { create(:project) } + + shared_examples 'cascading settings' do |attribute| + it 'return self value when no parent' do + expect(project_without_group.group).to be_nil + + project_without_group.update!(attribute => true) + expect(project_without_group.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy + expect(project_without_group.public_send("#{attribute}_locked?")).to be_falsey + + project_without_group.update!(attribute => false) + expect(project_without_group.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey + expect(project_without_group.public_send("#{attribute}_locked?")).to be_falsey + end + + it 'return self value when unlocked' do + subgroup.namespace_settings.update!(attribute => false) + group.namespace_settings.update!(attribute => false) + + project.update!(attribute => true) + expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy + expect(project.public_send("#{attribute}_locked?")).to be_falsey + + project.update!(attribute => false) + expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey + expect(project.public_send("#{attribute}_locked?")).to be_falsey + end + + it 'still return self value when locked subgroup' do + subgroup.namespace_settings.update!(attribute => true) + group.namespace_settings.update!(attribute => false) + + project.update!(attribute => true) + expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy + expect(project.public_send("#{attribute}_locked?")).to be_falsey + + project.update!(attribute => false) + expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey + expect(project.public_send("#{attribute}_locked?")).to be_falsey + end + + it 'still return unlocked value when locked group' do + subgroup.namespace_settings.update!(attribute => false) + group.namespace_settings.update!(attribute => true) + + project.update!(attribute => true) + expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy + expect(project.public_send("#{attribute}_locked?")).to be_falsey + + project.update!(attribute => false) + expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey + expect(project.public_send("#{attribute}_locked?")).to be_falsey + end + end + + it_behaves_like 'cascading settings', :only_allow_merge_if_pipeline_succeeds + it_behaves_like 'cascading settings', :allow_merge_on_skipped_pipeline + it_behaves_like 'cascading settings', :only_allow_merge_if_all_discussions_are_resolved + end + + describe '#archived' do + it { expect(subject.archived).to be_falsey } + it { expect(described_class.new(archived: true).archived).to be_truthy } + end + + describe '#resolve_outdated_diff_discussions' do + it { expect(subject.resolve_outdated_diff_discussions).to be_falsey } + + context 'when set explicitly' do + subject { described_class.new(resolve_outdated_diff_discussions: true) } + + it { expect(subject.resolve_outdated_diff_discussions).to be_truthy } + end + end + + describe '#only_allow_merge_if_all_discussions_are_resolved' do + it { expect(subject.only_allow_merge_if_all_discussions_are_resolved).to be_falsey } + + context 'when set explicitly' do + subject { described_class.new(only_allow_merge_if_all_discussions_are_resolved: true) } + + it { expect(subject.only_allow_merge_if_all_discussions_are_resolved).to be_truthy } + end + end + + describe '#remove_source_branch_after_merge' do + it { expect(subject.remove_source_branch_after_merge).to be_truthy } + + context 'when set explicitly' do + subject { described_class.new(remove_source_branch_after_merge: false) } + + it { expect(subject.remove_source_branch_after_merge).to be_falsey } + end + end + private def finish_job(export_job) diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index 9de31ea66e4..a6e2bcf1525 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -467,6 +467,13 @@ RSpec.describe ProjectStatistics do .to change { statistics.reload.storage_size } .by(20) end + + it 'schedules a namespace aggregation worker' do + expect(Namespaces::ScheduleAggregationWorker).to receive(:perform_async) + .with(statistics.project.namespace.id) + + described_class.increment_statistic(project, stat, 20) + end end shared_examples 'a statistic that increases storage_size asynchronously' do @@ -474,7 +481,8 @@ RSpec.describe ProjectStatistics do described_class.increment_statistic(project, stat, 13) Gitlab::Redis::SharedState.with do |redis| - increment = redis.get(statistics.counter_key(stat)) + key = statistics.counter(stat).key + increment = redis.get(key) expect(increment.to_i).to eq(13) end end @@ -482,7 +490,7 @@ RSpec.describe ProjectStatistics do it 'schedules a worker to update the statistic and storage_size async', :sidekiq_inline do expect(FlushCounterIncrementsWorker) .to receive(:perform_in) - .with(CounterAttribute::WORKER_DELAY, described_class.name, statistics.id, stat) + .with(Gitlab::Counters::BufferedCounter::WORKER_DELAY, described_class.name, statistics.id, stat) .and_call_original expect { described_class.increment_statistic(project, stat, 20) } @@ -506,20 +514,20 @@ RSpec.describe ProjectStatistics do context 'when adjusting :packages_size' do let(:stat) { :packages_size } - it_behaves_like 'a statistic that increases storage_size' + it_behaves_like 'a statistic that increases storage_size asynchronously' end context 'when the amount is 0' do it 'does not execute a query' do project - expect { described_class.increment_statistic(project.id, :build_artifacts_size, 0) } + expect { described_class.increment_statistic(project, :build_artifacts_size, 0) } .not_to exceed_query_limit(0) end end context 'when using an invalid column' do it 'raises an error' do - expect { described_class.increment_statistic(project.id, :id, 13) } + expect { described_class.increment_statistic(project, :id, 13) } .to raise_error(ArgumentError, "Cannot increment attribute: id") end end diff --git a/spec/models/projects/build_artifacts_size_refresh_spec.rb b/spec/models/projects/build_artifacts_size_refresh_spec.rb index 21cd8e0b9d4..caff06262d9 100644 --- a/spec/models/projects/build_artifacts_size_refresh_spec.rb +++ b/spec/models/projects/build_artifacts_size_refresh_spec.rb @@ -67,6 +67,8 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do let!(:last_job_artifact_id_on_refresh_start) { create(:ci_job_artifact, project: refresh.project) } + let(:statistics) { refresh.project.statistics } + before do stats = create(:project_statistics, project: refresh.project, build_artifacts_size: 120) stats.increment_counter(:build_artifacts_size, 30) @@ -89,11 +91,11 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do end it 'resets the build artifacts size stats' do - expect { refresh.process! }.to change { refresh.project.statistics.build_artifacts_size }.to(0) + expect { refresh.process! }.to change { statistics.build_artifacts_size }.to(0) end it 'resets the counter attribute to zero' do - expect { refresh.process! }.to change { refresh.project.statistics.get_counter_value(:build_artifacts_size) }.to(0) + expect { refresh.process! }.to change { statistics.counter(:build_artifacts_size).get }.to(0) end end diff --git a/spec/models/projects/forks/divergence_counts_spec.rb b/spec/models/projects/forks/divergence_counts_spec.rb new file mode 100644 index 00000000000..fd69cc0f3e7 --- /dev/null +++ b/spec/models/projects/forks/divergence_counts_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::Forks::DivergenceCounts, feature_category: :source_code_management do + include ProjectForksHelper + + let_it_be(:user) { create(:user) } + + describe '#counts', :use_clean_rails_redis_caching do + let(:source_repo) { create(:project, :repository, :public).repository } + let(:fork_repo) { fork_project(source_repo.project, user, { repository: true }).repository } + let(:fork_branch) { 'fork-branch' } + let(:cache_key) { ['project_forks', fork_repo.project.id, fork_branch, 'divergence_counts'] } + + def expect_cached_counts(value) + counts = described_class.new(fork_repo.project, fork_branch).counts + + ahead, behind = value + expect(counts).to eq({ ahead: ahead, behind: behind }) + + cached_value = [source_repo.commit.sha, fork_repo.commit(fork_branch).sha, value] + expect(Rails.cache.read(cache_key)).to eq(cached_value) + end + + it 'shows how far behind/ahead a fork is from the upstream' do + fork_repo.create_branch(fork_branch) + + expect_cached_counts([0, 0]) + + fork_repo.commit_files( + user, + branch_name: fork_branch, message: 'Committing something', + actions: [{ action: :create, file_path: 'encoding/CHANGELOG', content: 'New file' }] + ) + + expect_cached_counts([1, 0]) + + fork_repo.commit_files( + user, + branch_name: fork_branch, message: 'Committing something else', + actions: [{ action: :create, file_path: 'encoding/ONE-MORE-CHANGELOG', content: 'One more new file' }] + ) + + expect_cached_counts([2, 0]) + + source_repo.commit_files( + user, + branch_name: source_repo.root_ref, message: 'Commit to root ref', + actions: [{ action: :create, file_path: 'encoding/CHANGELOG', content: 'One more' }] + ) + + expect_cached_counts([2, 1]) + + source_repo.commit_files( + user, + branch_name: source_repo.root_ref, message: 'Another commit to root ref', + actions: [{ action: :create, file_path: 'encoding/NEW-CHANGELOG', content: 'One more time' }] + ) + + expect_cached_counts([2, 2]) + + # When the fork is too far ahead + stub_const("#{described_class}::LATEST_COMMITS_COUNT", 1) + fork_repo.commit_files( + user, + branch_name: fork_branch, message: 'Another commit to fork', + actions: [{ action: :create, file_path: 'encoding/TOO-NEW-CHANGELOG', content: 'New file' }] + ) + + expect_cached_counts(nil) + end + + context 'when counts calculated from a branch that exists upstream' do + let(:fork_branch) { 'feature' } + + it 'compares the fork branch to upstream default branch' do + # The branch itself diverges from the upstream default branch + expect_cached_counts([1, 29]) + + source_repo.commit_files( + user, + branch_name: source_repo.root_ref, message: 'Commit to root ref', + actions: [{ action: :create, file_path: 'encoding/CHANGELOG', content: 'New file' }] + ) + + fork_repo.commit_files( + user, + branch_name: fork_branch, message: 'Committing to feature branch', + actions: [{ action: :create, file_path: 'encoding/FEATURE-BRANCH', content: 'New file' }] + ) + + # It takes into account diverged commits from upstream AND from fork + expect_cached_counts([2, 30]) + end + end + end +end diff --git a/spec/models/release_highlight_spec.rb b/spec/models/release_highlight_spec.rb index 3555dfba769..4148452f849 100644 --- a/spec/models/release_highlight_spec.rb +++ b/spec/models/release_highlight_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' -RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache do - let(:fixture_dir_glob) { Dir.glob(File.join(Rails.root, 'spec', 'fixtures', 'whats_new', '*.yml')).grep(/\d*\_(\d*\_\d*)\.yml$/) } +RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache, feature_category: :release_orchestration do + let(:fixture_dir_glob) { Dir.glob(File.join(Rails.root, 'spec', 'fixtures', 'whats_new', '*.yml')).grep(/\d*_(\d*_\d*)\.yml$/) } before do allow(Dir).to receive(:glob).with(Rails.root.join('data', 'whats_new', '*.yml')).and_return(fixture_dir_glob) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index c17e180f282..969a279dd52 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -5,7 +5,9 @@ require 'spec_helper' RSpec.describe Repository do include RepoHelpers - TestBlob = Struct.new(:path) + before do + stub_const('TestBlob', Struct.new(:path)) + end let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :repository) } @@ -164,11 +166,11 @@ RSpec.describe Repository do repository.add_tag(user, annotated_tag_name, 'a48e4fc218069f68ef2e769dd8dfea3991362175', 'test tag message\n') end - it { is_expected.to eq(['v1.0.0', 'v1.1.0', annotated_tag_name]) } - after do repository.rm_tag(user, annotated_tag_name) end + + it { is_expected.to eq(['v1.0.0', 'v1.1.0', annotated_tag_name]) } end end @@ -2224,32 +2226,34 @@ RSpec.describe Repository do describe '#after_change_head' do it 'flushes the method caches' do - expect(repository).to receive(:expire_method_caches).with([ - :size, - :commit_count, - :readme_path, - :contribution_guide, - :changelog, - :license_blob, - :license_licensee, - :license_gitaly, - :gitignore, - :gitlab_ci_yml, - :branch_names, - :tag_names, - :branch_count, - :tag_count, - :avatar, - :exists?, - :root_ref, - :merged_branch_names, - :has_visible_content?, - :issue_template_names_hash, - :merge_request_template_names_hash, - :user_defined_metrics_dashboard_paths, - :xcode_project?, - :has_ambiguous_refs? - ]) + expect(repository).to receive(:expire_method_caches).with( + [ + :size, + :commit_count, + :readme_path, + :contribution_guide, + :changelog, + :license_blob, + :license_licensee, + :license_gitaly, + :gitignore, + :gitlab_ci_yml, + :branch_names, + :tag_names, + :branch_count, + :tag_count, + :avatar, + :exists?, + :root_ref, + :merged_branch_names, + :has_visible_content?, + :issue_template_names_hash, + :merge_request_template_names_hash, + :user_defined_metrics_dashboard_paths, + :xcode_project?, + :has_ambiguous_refs? + ] + ) repository.after_change_head end diff --git a/spec/models/service_desk_setting_spec.rb b/spec/models/service_desk_setting_spec.rb index f99ac84175c..c1ec35732b8 100644 --- a/spec/models/service_desk_setting_spec.rb +++ b/spec/models/service_desk_setting_spec.rb @@ -39,11 +39,16 @@ RSpec.describe ServiceDeskSetting do let_it_be(:project1) { create(:project, name: 'test-one', group: group) } let_it_be(:project2) { create(:project, name: 'one', group: subgroup) } let_it_be(:project_key) { 'key' } - - before_all do + let!(:setting) do create(:service_desk_setting, project: project1, project_key: project_key) end + context 'when project_key exists' do + it 'is valid' do + expect(setting).to be_valid + end + end + context 'when project_key is unique for every project slug' do it 'does not add error' do settings = build(:service_desk_setting, project: project2, project_key: 'otherkey') diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb index 655cfad57c9..050f99fd4d5 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/spec/models/snippet_repository_spec.rb @@ -38,6 +38,9 @@ RSpec.describe SnippetRepository do let(:update_file) { { previous_path: 'README', file_path: 'README', content: 'bar' } } let(:data) { [new_file, move_file, update_file] } + let_it_be(:unnamed_snippet) { { file_path: '', content: 'dummy', action: :create } } + let_it_be(:named_snippet) { { file_path: 'fee.txt', content: 'bar', action: :create } } + it 'returns nil when files argument is empty' do expect(snippet.repository).not_to receive(:commit_files) @@ -210,9 +213,6 @@ RSpec.describe SnippetRepository do end end - let_it_be(:named_snippet) { { file_path: 'fee.txt', content: 'bar', action: :create } } - let_it_be(:unnamed_snippet) { { file_path: '', content: 'dummy', action: :create } } - context 'when existing file has a default name' do let(:default_name) { 'snippetfile1.txt' } let(:new_file) { { file_path: '', content: 'bar' } } diff --git a/spec/models/state_note_spec.rb b/spec/models/state_note_spec.rb index e91150695b0..0afdf6bbcb9 100644 --- a/spec/models/state_note_spec.rb +++ b/spec/models/state_note_spec.rb @@ -19,6 +19,7 @@ RSpec.describe StateNote do it 'contains the expected values' do expect(subject.author).to eq(author) expect(subject.created_at).to eq(event.created_at) + expect(subject.updated_at).to eq(event.created_at) expect(subject.note).to eq(state) end end @@ -33,7 +34,8 @@ RSpec.describe StateNote do it 'contains the expected values' do expect(subject.author).to eq(author) - expect(subject.created_at).to eq(subject.created_at) + expect(subject.created_at).to eq(event.created_at) + expect(subject.updated_at).to eq(event.created_at) expect(subject.note).to eq("closed via commit #{commit.id}") end end @@ -45,6 +47,7 @@ RSpec.describe StateNote do it 'contains the expected values' do expect(subject.author).to eq(author) expect(subject.created_at).to eq(event.created_at) + expect(subject.updated_at).to eq(event.created_at) expect(subject.note).to eq("closed via merge request !#{merge_request.iid}") end end @@ -55,6 +58,7 @@ RSpec.describe StateNote do it 'contains the expected values' do expect(subject.author).to eq(author) expect(subject.created_at).to eq(event.created_at) + expect(subject.updated_at).to eq(event.created_at) expect(subject.note).to eq('resolved the corresponding error and closed the issue') end end @@ -65,6 +69,7 @@ RSpec.describe StateNote do it 'contains the expected values' do expect(subject.author).to eq(author) expect(subject.created_at).to eq(event.created_at) + expect(subject.updated_at).to eq(event.created_at) expect(subject.note).to eq('automatically closed this incident because the alert resolved') end end diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 23ba0be2fbc..221f09dd87f 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -56,6 +56,15 @@ RSpec.describe Todo do expect(subject.body).to eq 'quick fix' end + + it 'returns full path of target when action is member_access_requested' do + group = create(:group) + + subject.target = group + subject.action = Todo::MEMBER_ACCESS_REQUESTED + + expect(subject.body).to eq group.full_path + end end describe '#done' do @@ -182,6 +191,17 @@ RSpec.describe Todo do expect(subject.target_reference).to eq issue.to_reference(full: false) end + + context 'when target is member access requested' do + it 'returns group full path' do + group = create(:group) + + subject.target = group + subject.action = Todo::MEMBER_ACCESS_REQUESTED + + expect(subject.target_reference).to eq group.full_path + end + end end describe '#self_added?' do diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index 04964d36dcd..ed55aca49b7 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -48,6 +48,23 @@ RSpec.describe UserDetail do describe '#website_url' do it { is_expected.to validate_length_of(:website_url).is_at_most(500) } + + it 'only validates the website_url if it is changed' do + user_detail = create(:user_detail) + # `update_attribute` required to bypass current validations + # Validations on `User#website_url` were added after + # there was already data in the database and `UserDetail#website_url` is + # derived from `User#website_url` so this reproduces the state of some of + # our production data + user_detail.update_attribute(:website_url, 'NotAUrl') + + expect(user_detail).to be_valid + + user_detail.website_url = 'AlsoNotAUrl' + + expect(user_detail).not_to be_valid + expect(user_detail.errors.full_messages).to match_array(["Website url is not a valid URL"]) + end end end diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index d76334d7c9e..a6f64c90657 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' RSpec.describe UserPreference do - let(:user_preference) { create(:user_preference) } + let_it_be(:user) { create(:user) } + + let(:user_preference) { create(:user_preference, user: user) } describe 'validations' do describe 'diffs_deletion_color and diffs_addition_color' do @@ -132,10 +134,24 @@ RSpec.describe UserPreference do describe '#tab_width' do it 'is set to 8 by default' do # Intentionally not using factory here to test the constructor. - pref = UserPreference.new + pref = described_class.new + + expect(pref.tab_width).to eq(8) + end + + it 'returns default value when assigning nil' do + pref = described_class.new(tab_width: nil) + expect(pref.tab_width).to eq(8) end + it 'returns default value when the value is NULL' do + pref = create(:user_preference, user: user) + pref.update_column(:tab_width, nil) + + expect(pref.reload.tab_width).to eq(8) + end + it do is_expected.to validate_numericality_of(:tab_width) .only_integer @@ -143,4 +159,141 @@ RSpec.describe UserPreference do .is_less_than_or_equal_to(12) end end + + describe '#tab_width=' do + it 'sets to default value when nil' do + pref = described_class.new(tab_width: nil) + + expect(pref.read_attribute(:tab_width)).to eq(8) + end + + it 'sets user values' do + pref = described_class.new(tab_width: 12) + + expect(pref.read_attribute(:tab_width)).to eq(12) + end + end + + describe '#time_display_relative' do + it 'is set to true by default' do + pref = described_class.new + + expect(pref.time_display_relative).to eq(true) + end + + it 'returns default value when assigning nil' do + pref = described_class.new(time_display_relative: nil) + + expect(pref.time_display_relative).to eq(true) + end + + it 'returns default value when the value is NULL' do + pref = create(:user_preference, user: user) + pref.update_column(:time_display_relative, nil) + + expect(pref.reload.time_display_relative).to eq(true) + end + + it 'returns assigned value' do + pref = described_class.new(time_display_relative: false) + + expect(pref.time_display_relative).to eq(false) + end + end + + describe '#time_display_relative=' do + it 'sets to default value when nil' do + pref = described_class.new(time_display_relative: nil) + + expect(pref.read_attribute(:time_display_relative)).to eq(true) + end + + it 'sets user values' do + pref = described_class.new(time_display_relative: false) + + expect(pref.read_attribute(:time_display_relative)).to eq(false) + end + end + + describe '#time_format_in_24h' do + it 'is set to false by default' do + pref = described_class.new + + expect(pref.time_format_in_24h).to eq(false) + end + + it 'returns default value when assigning nil' do + pref = described_class.new(time_format_in_24h: nil) + + expect(pref.time_format_in_24h).to eq(false) + end + + it 'returns default value when the value is NULL' do + pref = create(:user_preference, user: user) + pref.update_column(:time_format_in_24h, nil) + + expect(pref.reload.time_format_in_24h).to eq(false) + end + + it 'returns assigned value' do + pref = described_class.new(time_format_in_24h: true) + + expect(pref.time_format_in_24h).to eq(true) + end + end + + describe '#time_format_in_24h=' do + it 'sets to default value when nil' do + pref = described_class.new(time_format_in_24h: nil) + + expect(pref.read_attribute(:time_format_in_24h)).to eq(false) + end + + it 'sets user values' do + pref = described_class.new(time_format_in_24h: true) + + expect(pref.read_attribute(:time_format_in_24h)).to eq(true) + end + end + + describe '#render_whitespace_in_code' do + it 'is set to false by default' do + pref = described_class.new + + expect(pref.render_whitespace_in_code).to eq(false) + end + + it 'returns default value when assigning nil' do + pref = described_class.new(render_whitespace_in_code: nil) + + expect(pref.render_whitespace_in_code).to eq(false) + end + + it 'returns default value when the value is NULL' do + pref = create(:user_preference, user: user) + pref.update_column(:render_whitespace_in_code, nil) + + expect(pref.reload.render_whitespace_in_code).to eq(false) + end + + it 'returns assigned value' do + pref = described_class.new(render_whitespace_in_code: true) + + expect(pref.render_whitespace_in_code).to eq(true) + end + end + + describe '#render_whitespace_in_code=' do + it 'sets to default value when nil' do + pref = described_class.new(render_whitespace_in_code: nil) + + expect(pref.read_attribute(:render_whitespace_in_code)).to eq(false) + end + + it 'sets user values' do + pref = described_class.new(render_whitespace_in_code: true) + + expect(pref.read_attribute(:render_whitespace_in_code)).to eq(true) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7207ee0b172..4a66af4ddf1 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -81,6 +81,9 @@ RSpec.describe User do it { is_expected.to delegate_method(:use_legacy_web_ide).to(:user_preference) } it { is_expected.to delegate_method(:use_legacy_web_ide=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:use_new_navigation).to(:user_preference) } + it { is_expected.to delegate_method(:use_new_navigation=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:job_title).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:job_title=).to(:user_detail).with_arguments(:args).allow_nil } @@ -146,6 +149,21 @@ RSpec.describe User do it { is_expected.to have_many(:project_callouts).class_name('Users::ProjectCallout') } it { is_expected.to have_many(:created_projects).dependent(:nullify).class_name('Project') } + describe 'default values' do + let(:user) { described_class.new } + + it { expect(user.admin).to be_falsey } + it { expect(user.external).to eq(Gitlab::CurrentSettings.user_default_external) } + it { expect(user.can_create_group).to eq(Gitlab::CurrentSettings.can_create_group) } + it { expect(user.can_create_team).to be_falsey } + it { expect(user.hide_no_ssh_key).to be_falsey } + it { expect(user.hide_no_password).to be_falsey } + it { expect(user.project_view).to eq('files') } + it { expect(user.notified_of_own_activity).to be_falsey } + it { expect(user.preferred_language).to eq(I18n.default_locale.to_s) } + it { expect(user.theme_id).to eq(described_class.gitlab_config.default_theme) } + end + describe '#user_detail' do it 'does not persist `user_detail` by default' do expect(create(:user).user_detail).not_to be_persisted @@ -345,52 +363,33 @@ RSpec.describe User do context 'check_password_weakness' do let(:weak_password) { "qwertyuiop" } - context 'when feature flag is disabled' do - before do - stub_feature_flags(block_weak_passwords: false) - end - - it 'does not add an error when password is weak' do - expect(Security::WeakPasswords).not_to receive(:weak_for_user?) - - user.password = weak_password - expect(user).to be_valid - end + it 'checks for password weakness when password changes' do + expect(Security::WeakPasswords).to receive(:weak_for_user?) + .with(weak_password, user).and_call_original + user.password = weak_password + expect(user).not_to be_valid end - context 'when feature flag is enabled' do - before do - stub_feature_flags(block_weak_passwords: true) - end - - it 'checks for password weakness when password changes' do - expect(Security::WeakPasswords).to receive(:weak_for_user?) - .with(weak_password, user).and_call_original - user.password = weak_password - expect(user).not_to be_valid - end - - it 'adds an error when password is weak' do - user.password = weak_password - expect(user).not_to be_valid - expect(user.errors).to be_of_kind(:password, 'must not contain commonly used combinations of words and letters') - end + it 'adds an error when password is weak' do + user.password = weak_password + expect(user).not_to be_valid + expect(user.errors).to be_of_kind(:password, 'must not contain commonly used combinations of words and letters') + end - it 'is valid when password is not weak' do - user.password = ::User.random_password - expect(user).to be_valid - end + it 'is valid when password is not weak' do + user.password = ::User.random_password + expect(user).to be_valid + end - it 'is valid when weak password was already set' do - user = build(:user, password: weak_password) - user.save!(validate: false) + it 'is valid when weak password was already set' do + user = build(:user, password: weak_password) + user.save!(validate: false) - expect(Security::WeakPasswords).not_to receive(:weak_for_user?) + expect(Security::WeakPasswords).not_to receive(:weak_for_user?) - # Change an unrelated value - user.name = "Example McExampleFace" - expect(user).to be_valid - end + # Change an unrelated value + user.name = "Example McExampleFace" + expect(user).to be_valid end end end @@ -417,7 +416,7 @@ RSpec.describe User do end it 'falls back to english when I18n.default_locale is not an available language' do - I18n.default_locale = :kl + allow(I18n).to receive(:default_locale) { :kl } default_preferred_language = user.send(:default_preferred_language) expect(user.preferred_language).to eq default_preferred_language @@ -1590,10 +1589,6 @@ RSpec.describe User do let(:expired_confirmation_sent_at) { Date.today - described_class.confirm_within - 7.days } let(:extant_confirmation_sent_at) { Date.today } - before do - allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) - end - let(:user) do create(:user, :unconfirmed, unconfirmed_email: 'test@gitlab.com').tap do |user| user.update!(confirmation_sent_at: confirmation_sent_at) @@ -3090,6 +3085,14 @@ RSpec.describe User do expect(described_class.find_by_ssh_key_id(-1)).to be_nil end end + + it 'does not return a signing-only key', :aggregate_failures do + signing_key = create(:key, usage_type: :signing, user: user) + auth_and_signing_key = create(:key, usage_type: :auth_and_signing, user: user) + + expect(described_class.find_by_ssh_key_id(signing_key.id)).to be_nil + expect(described_class.find_by_ssh_key_id(auth_and_signing_key.id)).to eq(user) + end end shared_examples "find user by login" do @@ -3209,6 +3212,7 @@ RSpec.describe User do expect(described_class.find_by_full_path('unknown')).to eq(nil) 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) @@ -4431,7 +4435,7 @@ RSpec.describe User do shared_context '#ci_owned_runners' do let(:user) { create(:user) } - shared_examples :nested_groups_owner do + shared_examples 'nested groups owner' do context 'when the user is the owner of a multi-level group' do before do set_permissions_for_users @@ -4448,7 +4452,7 @@ RSpec.describe User do end end - shared_examples :group_owner do + shared_examples 'group owner' do context 'when the user is the owner of a one level group' do before do group.add_owner(user) @@ -4464,7 +4468,7 @@ RSpec.describe User do end end - shared_examples :project_owner do + shared_examples 'project owner' do context 'when the user is the owner of a project' do it 'loads the runner belonging to the project' do expect(user.ci_owned_runners).to contain_exactly(runner) @@ -4476,7 +4480,7 @@ RSpec.describe User do end end - shared_examples :project_member do + shared_examples 'project member' do context 'when the user is a maintainer' do before do add_user(:maintainer) @@ -4534,7 +4538,7 @@ RSpec.describe User do end end - shared_examples :group_member do + shared_examples 'group member' do context 'when the user is a maintainer' do before do add_user(:maintainer) @@ -4607,7 +4611,7 @@ RSpec.describe User do let!(:project) { create(:project, namespace: namespace) } let!(:runner) { create(:ci_runner, :project, projects: [project]) } - it_behaves_like :project_owner + it_behaves_like 'project owner' end context 'with group runner in a non owned group' do @@ -4618,14 +4622,14 @@ RSpec.describe User do group.add_member(user, access) end - it_behaves_like :group_member + it_behaves_like 'group member' end context 'with group runner in an owned group' do let!(:group) { create(:group) } let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } - it_behaves_like :group_owner + it_behaves_like 'group owner' end context 'with group runner in an owned group and group runner in a different owner subgroup' do @@ -4640,7 +4644,7 @@ RSpec.describe User do subgroup.add_owner(another_user) end - it_behaves_like :nested_groups_owner + it_behaves_like 'nested groups owner' end context 'with personal project runner in an an owned group and a group runner in that same group' do @@ -4653,7 +4657,7 @@ RSpec.describe User do group.add_owner(user) end - it_behaves_like :nested_groups_owner + it_behaves_like 'nested groups owner' end context 'with personal project runner in an owned group and a group runner in a subgroup' do @@ -4667,7 +4671,7 @@ RSpec.describe User do group.add_owner(user) end - it_behaves_like :nested_groups_owner + it_behaves_like 'nested groups owner' end context 'with personal project runner in an owned group in an owned namespace and a group runner in that group' do @@ -4681,7 +4685,7 @@ RSpec.describe User do group.add_owner(user) end - it_behaves_like :nested_groups_owner + it_behaves_like 'nested groups owner' end context 'with personal project runner in an owned namespace, an owned group, a subgroup and a group runner in that subgroup' do @@ -4696,7 +4700,7 @@ RSpec.describe User do group.add_owner(user) end - it_behaves_like :nested_groups_owner + it_behaves_like 'nested groups owner' end context 'with a project runner that belong to projects that belong to a not owned group' do @@ -4708,7 +4712,7 @@ RSpec.describe User do project.add_member(user, access) end - it_behaves_like :project_member + it_behaves_like 'project member' end context 'with project runners that belong to projects that do not belong to any group' do @@ -4731,7 +4735,7 @@ RSpec.describe User do group.add_member(another_user, :owner) end - it_behaves_like :group_member + it_behaves_like 'group member' end end @@ -5221,6 +5225,10 @@ RSpec.describe User do describe '#invalidate_issue_cache_counts' do let(:user) { build_stubbed(:user) } + before do + stub_feature_flags(limit_assigned_issues_count: false) + end + it 'invalidates cache for issue counter' do cache_mock = double @@ -5230,6 +5238,23 @@ RSpec.describe User do user.invalidate_issue_cache_counts end + + context 'when limit_assigned_issues_count is enabled' do + before do + stub_feature_flags(limit_assigned_issues_count: true) + end + + it 'invalidates cache for issue counter' do + cache_mock = double + + expect(cache_mock).to receive(:delete).with(['users', user.id, 'assigned_open_issues_count']) + expect(cache_mock).to receive(:delete).with(['users', user.id, 'max_assigned_open_issues_count']) + + allow(Rails).to receive(:cache).and_return(cache_mock) + + user.invalidate_issue_cache_counts + end + end end describe '#invalidate_merge_request_cache_counts' do @@ -6155,33 +6180,44 @@ RSpec.describe User do describe '#notification_email_for' do let(:user) { create(:user) } - let(:group) { create(:group) } - subject { user.notification_email_for(group) } + subject { user.notification_email_for(namespace) } - context 'when group is nil' do - let(:group) { nil } + context 'when namespace is nil' do + let(:namespace) { nil } it 'returns global notification email' do is_expected.to eq(user.notification_email_or_default) end end - context 'when group has no notification email set' do - it 'returns global notification email' do - create(:notification_setting, user: user, source: group, notification_email: '') + context 'for group namespace' do + let(:namespace) { create(:group) } - is_expected.to eq(user.notification_email_or_default) + context 'when group has no notification email set' do + it 'returns global notification email' do + create(:notification_setting, user: user, source: namespace, notification_email: '') + + is_expected.to eq(user.notification_email_or_default) + end + end + + context 'when group has notification email set' do + it 'returns group notification email' do + group_notification_email = 'user+group@example.com' + create(:email, :confirmed, user: user, email: group_notification_email) + create(:notification_setting, user: user, source: namespace, notification_email: group_notification_email) + + is_expected.to eq(group_notification_email) + end end end - context 'when group has notification email set' do - it 'returns group notification email' do - group_notification_email = 'user+group@example.com' - create(:email, :confirmed, user: user, email: group_notification_email) - create(:notification_setting, user: user, source: group, notification_email: group_notification_email) + context 'for user namespace' do + let(:namespace) { create(:user_namespace) } - is_expected.to eq(group_notification_email) + it 'returns global notification email' do + is_expected.to eq(user.notification_email_or_default) end end end @@ -6799,7 +6835,8 @@ RSpec.describe User do { user_type: :alert_bot }, { user_type: :support_bot }, { user_type: :security_bot }, - { user_type: :automation_bot } + { user_type: :automation_bot }, + { user_type: :admin_bot } ] end @@ -6881,11 +6918,12 @@ RSpec.describe User do using RSpec::Parameterized::TableSyntax where(:user_type, :expected_result) do - 'human' | true - 'alert_bot' | false - 'support_bot' | false - 'security_bot' | false - 'automation_bot' | false + 'human' | true + 'alert_bot' | false + 'support_bot' | false + 'security_bot' | false + 'automation_bot' | false + 'admin_bot' | false end with_them do @@ -7034,17 +7072,26 @@ RSpec.describe User do it_behaves_like 'bot users', :security_bot it_behaves_like 'bot users', :ghost it_behaves_like 'bot users', :automation_bot + it_behaves_like 'bot users', :admin_bot it_behaves_like 'bot user avatars', :alert_bot, 'alert-bot.png' it_behaves_like 'bot user avatars', :support_bot, 'support-bot.png' it_behaves_like 'bot user avatars', :security_bot, 'security-bot.png' it_behaves_like 'bot user avatars', :automation_bot, 'support-bot.png' + it_behaves_like 'bot user avatars', :admin_bot, 'admin-bot.png' context 'when bot is the support_bot' do subject { described_class.support_bot } it { is_expected.to be_confirmed } end + + context 'when bot is the admin bot' do + subject { described_class.admin_bot } + + it { is_expected.to be_admin } + it { is_expected.to be_confirmed } + end end describe '#confirmation_required_on_sign_in?' do @@ -7307,4 +7354,51 @@ RSpec.describe User do expect(user.account_age_in_days).to be(1) end end + + describe 'state machine and default attributes' do + let(:model) do + Class.new(ApplicationRecord) do + self.table_name = User.table_name + + attribute :external, default: -> { 1 / 0 } + + state_machine :state, initial: :active do + end + end + end + + it 'raises errors by default' do + expect { model }.to raise_error(ZeroDivisionError) + end + + context 'with state machine default attributes override' do + let(:model) do + Class.new(ApplicationRecord) do + self.table_name = User.table_name + + attribute :external, default: -> { 1 / 0 } + + state_machine :state, initial: :active do + def owner_class_attribute_default; end + end + end + end + + it 'does not raise errors' do + expect { model }.not_to raise_error + end + + it 'raises errors when default attributes are used' do + expect { model.new.attributes }.to raise_error(ZeroDivisionError) + end + + it 'does not evaluate default attributes when values are provided' do + expect { model.new(external: false).attributes }.not_to raise_error + end + + it 'sets the state machine default value' do + expect(model.new(external: true).state).to eq('active') + end + end + end end diff --git a/spec/models/users/phone_number_validation_spec.rb b/spec/models/users/phone_number_validation_spec.rb index 2f0fd1d3ac9..7ab461a4346 100644 --- a/spec/models/users/phone_number_validation_spec.rb +++ b/spec/models/users/phone_number_validation_spec.rb @@ -78,4 +78,42 @@ RSpec.describe Users::PhoneNumberValidation do it { is_expected.to eq(false) } end end + + describe '#for_user' do + let_it_be(:user_1) { create(:user) } + let_it_be(:user_2) { create(:user) } + + let_it_be(:phone_number_record_1) { create(:phone_number_validation, user: user_1) } + let_it_be(:phone_number_record_2) { create(:phone_number_validation, user: user_2) } + + context 'when multiple records exist for multiple users' do + it 'returns the correct phone number record for user' do + records = described_class.for_user(user_1.id) + + expect(records.count).to be(1) + expect(records.first).to eq(phone_number_record_1) + end + end + end + + describe '#validated?' do + let_it_be(:user) { create(:user) } + let_it_be(:phone_number_record) { create(:phone_number_validation, user: user) } + + context 'when phone number record is not validated' do + it 'returns false' do + expect(phone_number_record.validated?).to be(false) + end + end + + context 'when phone number record is validated' do + before do + phone_number_record.update!(validated_at: Time.now.utc) + end + + it 'returns true' do + expect(phone_number_record.validated?).to be(true) + end + end + end end diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb index 341f9a9c60f..1c34936c5c2 100644 --- a/spec/models/work_item_spec.rb +++ b/spec/models/work_item_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe WorkItem do +RSpec.describe WorkItem, feature_category: :portfolio_management do let_it_be(:reusable_project) { create(:project) } describe 'associations' do @@ -176,4 +176,59 @@ RSpec.describe WorkItem do end end end + + context 'with hierarchy' do + let_it_be(:type1) { create(:work_item_type, namespace: reusable_project.namespace) } + let_it_be(:type2) { create(:work_item_type, namespace: reusable_project.namespace) } + let_it_be(:type3) { create(:work_item_type, namespace: reusable_project.namespace) } + let_it_be(:type4) { create(:work_item_type, namespace: reusable_project.namespace) } + let_it_be(:hierarchy_restriction1) { create(:hierarchy_restriction, parent_type: type1, child_type: type2) } + let_it_be(:hierarchy_restriction2) { create(:hierarchy_restriction, parent_type: type2, child_type: type2) } + let_it_be(:hierarchy_restriction3) { create(:hierarchy_restriction, parent_type: type2, child_type: type3) } + let_it_be(:hierarchy_restriction4) { create(:hierarchy_restriction, parent_type: type3, child_type: type3) } + let_it_be(:hierarchy_restriction5) { create(:hierarchy_restriction, parent_type: type3, child_type: type4) } + let_it_be(:item1) { create(:work_item, work_item_type: type1, project: reusable_project) } + let_it_be(:item2_1) { create(:work_item, work_item_type: type2, project: reusable_project) } + let_it_be(:item2_2) { create(:work_item, work_item_type: type2, project: reusable_project) } + let_it_be(:item3_1) { create(:work_item, work_item_type: type3, project: reusable_project) } + let_it_be(:item3_2) { create(:work_item, work_item_type: type3, project: reusable_project) } + let_it_be(:item4) { create(:work_item, work_item_type: type4, project: reusable_project) } + let_it_be(:ignored_ancestor) { create(:work_item, work_item_type: type1, project: reusable_project) } + let_it_be(:ignored_descendant) { create(:work_item, work_item_type: type4, project: reusable_project) } + let_it_be(:link1) { create(:parent_link, work_item_parent: item1, work_item: item2_1) } + let_it_be(:link2) { create(:parent_link, work_item_parent: item2_1, work_item: item2_2) } + let_it_be(:link3) { create(:parent_link, work_item_parent: item2_2, work_item: item3_1) } + let_it_be(:link4) { create(:parent_link, work_item_parent: item3_1, work_item: item3_2) } + let_it_be(:link5) { create(:parent_link, work_item_parent: item3_2, work_item: item4) } + + describe '#ancestors' do + it 'returns all ancestors in ascending order' do + expect(item3_1.ancestors).to eq([item2_2, item2_1, item1]) + end + + it 'returns an empty array if there are no ancestors' do + expect(item1.ancestors).to be_empty + end + end + + describe '#same_type_base_and_ancestors' do + it 'returns self and all ancestors of the same type in ascending order' do + expect(item3_2.same_type_base_and_ancestors).to eq([item3_2, item3_1]) + end + + it 'returns self if there are no ancestors of the same type' do + expect(item3_1.same_type_base_and_ancestors).to match_array([item3_1]) + end + end + + describe '#same_type_descendants_depth' do + it 'returns max descendants depth including self' do + expect(item3_1.same_type_descendants_depth).to eq(2) + end + + it 'returns 1 if there are no descendants' do + expect(item1.same_type_descendants_depth).to eq(1) + end + end + end end diff --git a/spec/models/work_items/hierarchy_restriction_spec.rb b/spec/models/work_items/hierarchy_restriction_spec.rb new file mode 100644 index 00000000000..2c4d5d32fb8 --- /dev/null +++ b/spec/models/work_items/hierarchy_restriction_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::HierarchyRestriction do + describe 'associations' do + it { is_expected.to belong_to(:parent_type) } + it { is_expected.to belong_to(:child_type) } + end + + describe 'validations' do + subject { build(:hierarchy_restriction) } + + it { is_expected.to validate_presence_of(:parent_type) } + it { is_expected.to validate_presence_of(:child_type) } + it { is_expected.to validate_uniqueness_of(:child_type).scoped_to(:parent_type_id) } + end +end diff --git a/spec/models/work_items/parent_link_spec.rb b/spec/models/work_items/parent_link_spec.rb index 070b2eef86a..82e79e8fbdf 100644 --- a/spec/models/work_items/parent_link_spec.rb +++ b/spec/models/work_items/parent_link_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -RSpec.describe WorkItems::ParentLink do +RSpec.describe WorkItems::ParentLink, feature_category: :portfolio_management do + let_it_be(:project) { create(:project) } + describe 'associations' do it { is_expected.to belong_to(:work_item) } it { is_expected.to belong_to(:work_item_parent).class_name('WorkItem') } @@ -16,7 +18,6 @@ RSpec.describe WorkItems::ParentLink do it { is_expected.to validate_uniqueness_of(:work_item) } describe 'hierarchy' do - let_it_be(:project) { create(:project) } let_it_be(:issue) { build(:work_item, project: project) } let_it_be(:incident) { build(:work_item, :incident, project: project) } let_it_be(:task1) { build(:work_item, :task, project: project) } @@ -30,18 +31,82 @@ RSpec.describe WorkItems::ParentLink do expect(build(:parent_link, work_item: task1, work_item_parent: incident)).to be_valid end - it 'is not valid if child is not task' do - link = build(:parent_link, work_item: issue) + context 'when assigning to various parent types' do + using RSpec::Parameterized::TableSyntax - expect(link).not_to be_valid - expect(link.errors[:work_item]).to include('only Task can be assigned as a child in hierarchy.') + where(:parent_type_sym, :child_type_sym, :is_valid) do + :issue | :task | true + :incident | :task | true + :task | :issue | false + :issue | :issue | false + :objective | :objective | true + :objective | :key_result | true + :key_result | :objective | false + :key_result | :key_result | false + :objective | :issue | false + :task | :objective | false + end + + with_them do + it 'validates if child can be added to the parent' do + parent_type = WorkItems::Type.default_by_type(parent_type_sym) + child_type = WorkItems::Type.default_by_type(child_type_sym) + parent = build(:work_item, issue_type: parent_type_sym, work_item_type: parent_type, project: project) + child = build(:work_item, issue_type: child_type_sym, work_item_type: child_type, project: project) + link = build(:parent_link, work_item: child, work_item_parent: parent) + + expect(link.valid?).to eq(is_valid) + end + end end - it 'is not valid if parent is task' do - link = build(:parent_link, work_item_parent: task1) + context 'with nested ancestors' do + let_it_be(:type1) { create(:work_item_type, namespace: project.namespace) } + let_it_be(:type2) { create(:work_item_type, namespace: project.namespace) } + let_it_be(:item1) { create(:work_item, work_item_type: type1, project: project) } + let_it_be(:item2) { create(:work_item, work_item_type: type2, project: project) } + let_it_be(:item3) { create(:work_item, work_item_type: type2, project: project) } + let_it_be(:item4) { create(:work_item, work_item_type: type2, project: project) } + let_it_be(:hierarchy_restriction1) { create(:hierarchy_restriction, parent_type: type1, child_type: type2) } + let_it_be(:hierarchy_restriction2) { create(:hierarchy_restriction, parent_type: type2, child_type: type1) } + + let_it_be(:hierarchy_restriction3) do + create(:hierarchy_restriction, parent_type: type2, child_type: type2, maximum_depth: 2) + end - expect(link).not_to be_valid - expect(link.errors[:work_item_parent]).to include('only Issue and Incident can be parent of Task.') + let_it_be(:link1) { create(:parent_link, work_item_parent: item1, work_item: item2) } + let_it_be(:link2) { create(:parent_link, work_item_parent: item3, work_item: item4) } + + describe '#validate_depth' do + it 'is valid if depth is in limit' do + link = build(:parent_link, work_item_parent: item1, work_item: item3) + + expect(link).to be_valid + end + + it 'is not valid when maximum depth is reached' do + link = build(:parent_link, work_item_parent: item2, work_item: item3) + + expect(link).not_to be_valid + expect(link.errors[:work_item]).to include('reached maximum depth') + end + end + + describe '#validate_cyclic_reference' do + it 'is not valid if parent and child are same' do + link1.work_item_parent = item2 + + expect(link1).not_to be_valid + expect(link1.errors[:work_item]).to include('is not allowed to point to itself') + end + + it 'is not valid if child is already in ancestors' do + link = build(:parent_link, work_item_parent: item4, work_item: item3) + + expect(link).not_to be_valid + expect(link.errors[:work_item]).to include('is already present in ancestors') + end + end end it 'is not valid if parent is in other project' do @@ -96,8 +161,26 @@ RSpec.describe WorkItems::ParentLink do end end - context 'with confidential work items' do + describe 'scopes' do let_it_be(:project) { create(:project) } + let_it_be(:issue1) { build(:work_item, project: project) } + let_it_be(:issue2) { build(:work_item, project: project) } + let_it_be(:issue3) { build(:work_item, project: project) } + let_it_be(:task1) { build(:work_item, :task, project: project) } + let_it_be(:task2) { build(:work_item, :task, project: project) } + let_it_be(:link1) { create(:parent_link, work_item_parent: issue1, work_item: task1) } + let_it_be(:link2) { create(:parent_link, work_item_parent: issue2, work_item: task2) } + + describe 'for_parents' do + it 'includes the correct records' do + result = described_class.for_parents([issue1.id, issue2.id, issue3.id]) + + expect(result).to include(link1, link2) + end + end + end + + context 'with confidential work items' do let_it_be(:confidential_child) { create(:work_item, :task, confidential: true, project: project) } let_it_be(:putlic_child) { create(:work_item, :task, project: project) } let_it_be(:confidential_parent) { create(:work_item, confidential: true, project: project) } diff --git a/spec/models/work_items/type_spec.rb b/spec/models/work_items/type_spec.rb index 6685720778a..1ada783385e 100644 --- a/spec/models/work_items/type_spec.rb +++ b/spec/models/work_items/type_spec.rb @@ -43,7 +43,7 @@ RSpec.describe WorkItems::Type do end it 'does not delete type when there are related issues' do - type = create(:work_item_type, work_items: [work_item]) + type = work_item.work_item_type expect { type.destroy! }.to raise_error(ActiveRecord::InvalidForeignKey) expect(Issue.count).to eq(1) @@ -70,11 +70,38 @@ RSpec.describe WorkItems::Type do ::WorkItems::Widgets::Labels, ::WorkItems::Widgets::Assignees, ::WorkItems::Widgets::StartAndDueDate, - ::WorkItems::Widgets::Milestone + ::WorkItems::Widgets::Milestone, + ::WorkItems::Widgets::Notes ) end end + describe '.default_by_type' do + let(:default_issue_type) { described_class.find_by(namespace_id: nil, base_type: :issue) } + + subject { described_class.default_by_type(:issue) } + + it 'returns default work item type by base type without calling importer' do + expect(Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter).not_to receive(:upsert_types) + expect(Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter).not_to receive(:upsert_restrictions) + + expect(subject).to eq(default_issue_type) + end + + context 'when default types are missing' do + before do + described_class.delete_all + end + + it 'creates types and restrictions and returns default work item type by base type' do + expect(Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter).to receive(:upsert_types) + expect(Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter).to receive(:upsert_restrictions) + + expect(subject).to eq(default_issue_type) + end + end + end + describe '#default?' do subject { build(:work_item_type, namespace: namespace).default? } diff --git a/spec/models/work_items/widgets/notes_spec.rb b/spec/models/work_items/widgets/notes_spec.rb new file mode 100644 index 00000000000..cc98f1ebe54 --- /dev/null +++ b/spec/models/work_items/widgets/notes_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::Widgets::Notes, feature_category: :team_planning do + let_it_be(:work_item) { create(:work_item) } + let_it_be(:note) { create(:note, noteable: work_item, project: work_item.project) } + + describe '.type' do + it { expect(described_class.type).to eq(:notes) } + end + + describe '#type' do + it { expect(described_class.new(work_item).type).to eq(:notes) } + end + + describe '#notes' do + it { expect(described_class.new(work_item).notes).to eq(work_item.notes) } + end +end diff --git a/spec/models/zoom_meeting_spec.rb b/spec/models/zoom_meeting_spec.rb index 2b45533035d..d3d75a19fed 100644 --- a/spec/models/zoom_meeting_spec.rb +++ b/spec/models/zoom_meeting_spec.rb @@ -29,6 +29,7 @@ RSpec.describe ZoomMeeting do expect(meetings_added).not_to include(removed_meeting.id) end end + describe '.removed_from_issue' do it 'gets only removed meetings' do meetings_removed = described_class.removed_from_issue.pluck(:id) |