diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-16 13:42:19 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-16 13:42:19 +0300 |
commit | 84d1bd786125c1c14a3ba5f63e38a4cc736a9027 (patch) | |
tree | f550fa965f507077e20dbb6d61a8269a99ef7107 /spec/models | |
parent | 3a105e36e689f7b75482236712f1a47fd5a76814 (diff) |
Add latest changes from gitlab-org/gitlab@16-8-stable-eev16.8.0-rc42
Diffstat (limited to 'spec/models')
74 files changed, 2875 insertions, 986 deletions
diff --git a/spec/models/analytics/cycle_analytics/aggregation_spec.rb b/spec/models/analytics/cycle_analytics/aggregation_spec.rb index e69093f454a..5494df84d68 100644 --- a/spec/models/analytics/cycle_analytics/aggregation_spec.rb +++ b/spec/models/analytics/cycle_analytics/aggregation_spec.rb @@ -131,39 +131,52 @@ RSpec.describe Analytics::CycleAnalytics::Aggregation, type: :model, feature_cat end describe '#safe_create_for_namespace' do - let_it_be(:group) { create(:group) } - let_it_be(:subgroup) { create(:group, parent: group) } + context 'when group namespace is provided' do + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } - it 'creates the aggregation record' do - record = described_class.safe_create_for_namespace(group) - - expect(record).to be_persisted - end - - context 'when non top-level group is given' do - it 'creates the aggregation record for the top-level group' do - record = described_class.safe_create_for_namespace(subgroup) + it 'creates the aggregation record' do + record = described_class.safe_create_for_namespace(group) expect(record).to be_persisted end - end - context 'when the record is already present' do - it 'does nothing' do - described_class.safe_create_for_namespace(group) + context 'when non top-level group is given' do + it 'creates the aggregation record for the top-level group' do + record = described_class.safe_create_for_namespace(subgroup) + + expect(record).to be_persisted + end + end - expect do + context 'when the record is already present' do + it 'does nothing' do described_class.safe_create_for_namespace(group) - described_class.safe_create_for_namespace(subgroup) - end.not_to change { described_class.count } + + expect do + described_class.safe_create_for_namespace(group) + described_class.safe_create_for_namespace(subgroup) + end.not_to change { described_class.count } + end + end + + context 'when the aggregation was disabled for some reason' do + it 're-enables the aggregation' do + create(:cycle_analytics_aggregation, enabled: false, namespace: group) + + aggregation = described_class.safe_create_for_namespace(group) + + expect(aggregation).to be_enabled + end end end - context 'when the aggregation was disabled for some reason' do - it 're-enables the aggregation' do - create(:cycle_analytics_aggregation, enabled: false, namespace: group) + context 'when personal namespace is provided' do + let_it_be(:user2) { create(:user) } + let_it_be(:project) { create(:project, :public, namespace: user2.namespace) } - aggregation = described_class.safe_create_for_namespace(group) + it 'is successful' do + aggregation = described_class.safe_create_for_namespace(user2.namespace) expect(aggregation).to be_enabled end diff --git a/spec/models/analytics/cycle_analytics/stage_spec.rb b/spec/models/analytics/cycle_analytics/stage_spec.rb index 54ae0feca2c..abe041ae5d6 100644 --- a/spec/models/analytics/cycle_analytics/stage_spec.rb +++ b/spec/models/analytics/cycle_analytics/stage_spec.rb @@ -81,20 +81,4 @@ RSpec.describe Analytics::CycleAnalytics::Stage, feature_category: :value_stream expect(current_event_pairs).to eq(expected_event_pairs) end end - - it_behaves_like 'database events tracking' do - let(:namespace) { create(:group) } - let(:value_stream) { create(:cycle_analytics_value_stream) } - let(:record) { described_class.create!(stage_params) } - let(:update_params) { { name: 'st 2' } } - let(:stage_params) do - { - namespace: namespace, - name: 'st1', - start_event_identifier: :merge_request_created, - end_event_identifier: :merge_request_merged, - group_value_stream_id: value_stream.id - } - end - end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index d16a78be533..b4003469ebb 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -27,6 +27,8 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { expect(setting.max_decompressed_archive_size).to eq(25600) } it { expect(setting.decompress_archive_file_timeout).to eq(210) } it { expect(setting.bulk_import_concurrent_pipeline_batch_limit).to eq(25) } + it { expect(setting.allow_project_creation_for_guest_and_below).to eq(true) } + it { expect(setting.members_delete_limit).to eq(60) } end describe 'validations' do @@ -57,6 +59,8 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do } end + it { expect(described_class).to validate_jsonb_schema(['application_setting_rate_limits']) } + it { is_expected.to allow_value(nil).for(:home_page_url) } it { is_expected.to allow_value(http).for(:home_page_url) } it { is_expected.to allow_value(https).for(:home_page_url) } @@ -101,65 +105,18 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.not_to allow_value(nil).for(:protected_paths_for_get_request) } it { is_expected.to allow_value([]).for(:protected_paths_for_get_request) } - it { is_expected.to allow_value(3).for(:push_event_hooks_limit) } - it { is_expected.not_to allow_value('three').for(:push_event_hooks_limit) } - it { is_expected.not_to allow_value(nil).for(:push_event_hooks_limit) } - - it { is_expected.to allow_value(3).for(:push_event_activities_limit) } - it { is_expected.not_to allow_value('three').for(:push_event_activities_limit) } - it { is_expected.not_to allow_value(nil).for(:push_event_activities_limit) } - - it { is_expected.to validate_numericality_of(:container_registry_delete_tags_service_timeout).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to validate_numericality_of(:container_registry_cleanup_tags_service_max_list_size).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to validate_numericality_of(:container_registry_data_repair_detail_worker_max_concurrency).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to validate_numericality_of(:container_registry_expiration_policies_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_inclusion_of(:container_registry_expiration_policies_caching).in_array([true, false]) } - it { is_expected.to validate_numericality_of(:container_registry_import_max_tags_count).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to validate_numericality_of(:container_registry_import_max_retries).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to validate_numericality_of(:container_registry_import_start_max_retries).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to validate_numericality_of(:container_registry_import_max_step_duration).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to validate_numericality_of(:container_registry_pre_import_timeout).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to validate_numericality_of(:container_registry_import_timeout).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_pre_import_tags_rate).is_greater_than_or_equal_to(0) } - it { is_expected.not_to allow_value(nil).for(:container_registry_data_repair_detail_worker_max_concurrency) } - it { is_expected.not_to allow_value(nil).for(:container_registry_import_max_tags_count) } - it { is_expected.not_to allow_value(nil).for(:container_registry_import_max_retries) } - it { is_expected.not_to allow_value(nil).for(:container_registry_import_start_max_retries) } - it { is_expected.not_to allow_value(nil).for(:container_registry_import_max_step_duration) } - it { is_expected.not_to allow_value(nil).for(:container_registry_pre_import_timeout) } - it { is_expected.not_to allow_value(nil).for(:container_registry_import_timeout) } it { is_expected.not_to allow_value(nil).for(:container_registry_pre_import_tags_rate) } it { is_expected.to allow_value(1.5).for(:container_registry_pre_import_tags_rate) } it { is_expected.to validate_presence_of(:container_registry_import_target_plan) } it { is_expected.to validate_presence_of(:container_registry_import_created_before) } - it { is_expected.to validate_numericality_of(:decompress_archive_file_timeout).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.not_to allow_value(nil).for(:decompress_archive_file_timeout) } - - it { is_expected.to validate_numericality_of(:dependency_proxy_ttl_group_policy_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.not_to allow_value(nil).for(:dependency_proxy_ttl_group_policy_worker_capacity) } - - it { is_expected.to validate_numericality_of(:packages_cleanup_package_file_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.not_to allow_value(nil).for(:packages_cleanup_package_file_worker_capacity) } - - it { is_expected.to validate_numericality_of(:package_registry_cleanup_policies_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.not_to allow_value(nil).for(:package_registry_cleanup_policies_worker_capacity) } - - it { is_expected.to validate_numericality_of(:snippet_size_limit).only_integer.is_greater_than(0) } it { is_expected.to validate_numericality_of(:wiki_page_max_content_bytes).only_integer.is_greater_than_or_equal_to(1024) } it { is_expected.to validate_inclusion_of(:wiki_asciidoc_allow_uri_includes).in_array([true, false]) } - it { is_expected.to validate_presence_of(:max_artifacts_size) } - it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } - it { is_expected.to validate_presence_of(:max_yaml_size_bytes) } - it { is_expected.to validate_numericality_of(:max_yaml_size_bytes).only_integer.is_greater_than(0) } - it { is_expected.to validate_presence_of(:max_yaml_depth) } - 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 { is_expected.to validate_inclusion_of(:user_defaults_to_private_profile).in_array([true, false]) } @@ -174,40 +131,12 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do .is_less_than(::Gitlab::Pages::MAX_SIZE / 1.megabyte) end - it 'ensures max_pages_custom_domains_per_project is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do - is_expected - .to validate_numericality_of(:max_pages_custom_domains_per_project) - .only_integer - .is_greater_than_or_equal_to(0) - end - - it { is_expected.to validate_presence_of(:jobs_per_stage_page_size) } - it { is_expected.to validate_numericality_of(:jobs_per_stage_page_size).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.not_to allow_value(7).for(:minimum_password_length) } it { is_expected.not_to allow_value(129).for(:minimum_password_length) } it { is_expected.not_to allow_value(nil).for(:minimum_password_length) } it { is_expected.not_to allow_value('abc').for(:minimum_password_length) } it { is_expected.to allow_value(10).for(:minimum_password_length) } - it { is_expected.to allow_value(300).for(:issues_create_limit) } - it { is_expected.not_to allow_value('three').for(:issues_create_limit) } - it { is_expected.not_to allow_value(nil).for(:issues_create_limit) } - it { is_expected.not_to allow_value(10.5).for(:issues_create_limit) } - it { is_expected.not_to allow_value(-1).for(:issues_create_limit) } - - it { is_expected.to allow_value(0).for(:raw_blob_request_limit) } - it { is_expected.not_to allow_value('abc').for(:raw_blob_request_limit) } - it { is_expected.not_to allow_value(nil).for(:raw_blob_request_limit) } - it { is_expected.not_to allow_value(10.5).for(:raw_blob_request_limit) } - it { is_expected.not_to allow_value(-1).for(:raw_blob_request_limit) } - - it { is_expected.to allow_value(0).for(:pipeline_limit_per_project_user_sha) } - it { is_expected.not_to allow_value('abc').for(:pipeline_limit_per_project_user_sha) } - it { is_expected.not_to allow_value(nil).for(:pipeline_limit_per_project_user_sha) } - it { is_expected.not_to allow_value(10.5).for(:pipeline_limit_per_project_user_sha) } - it { is_expected.not_to allow_value(-1).for(:pipeline_limit_per_project_user_sha) } - it { is_expected.not_to allow_value(false).for(:hashed_storage_enabled) } it { is_expected.to allow_value('default' => 0).for(:repository_storages_weighted) } @@ -219,15 +148,6 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.not_to allow_value('default' => 101).for(:repository_storages_weighted).with_message("value for 'default' must be between 0 and 100") } it { is_expected.not_to allow_value('default' => 100, shouldntexist: 50).for(:repository_storages_weighted).with_message("can't include: shouldntexist") } - %i[notes_create_limit search_rate_limit search_rate_limit_unauthenticated users_get_by_id_limit - projects_api_rate_limit_unauthenticated gitlab_shell_operation_limit].each do |setting| - it { is_expected.to allow_value(400).for(setting) } - it { is_expected.not_to allow_value('two').for(setting) } - it { is_expected.not_to allow_value(nil).for(setting) } - it { is_expected.not_to allow_value(5.5).for(setting) } - it { is_expected.not_to allow_value(-2).for(setting) } - end - def many_usernames(num = 100) Array.new(num) { |i| "username#{i}" } end @@ -280,23 +200,132 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_inclusion_of(:silent_mode_enabled).in_array([true, false]) } - it { is_expected.to allow_value(0).for(:ci_max_includes) } - it { is_expected.to allow_value(200).for(:ci_max_includes) } - it { is_expected.not_to allow_value('abc').for(:ci_max_includes) } - it { is_expected.not_to allow_value(nil).for(:ci_max_includes) } - it { is_expected.not_to allow_value(10.5).for(:ci_max_includes) } - it { is_expected.not_to allow_value(-1).for(:ci_max_includes) } + context 'for non-null integer attributes starting from 0' do + where(:attribute) do + %i[ + bulk_import_max_download_file_size + ci_max_includes + ci_max_total_yaml_size_bytes + container_registry_cleanup_tags_service_max_list_size + container_registry_data_repair_detail_worker_max_concurrency + container_registry_delete_tags_service_timeout + container_registry_expiration_policies_worker_capacity + container_registry_import_max_retries + container_registry_import_max_step_duration + container_registry_import_max_tags_count + container_registry_import_start_max_retries + container_registry_import_timeout + container_registry_pre_import_timeout + decompress_archive_file_timeout + dependency_proxy_ttl_group_policy_worker_capacity + gitlab_shell_operation_limit + inactive_projects_min_size_mb + issues_create_limit + jobs_per_stage_page_size + max_decompressed_archive_size + max_export_size + max_import_remote_file_size + max_import_size + max_pages_custom_domains_per_project + max_terraform_state_size_bytes + members_delete_limit + notes_create_limit + package_registry_cleanup_policies_worker_capacity + packages_cleanup_package_file_worker_capacity + pipeline_limit_per_project_user_sha + projects_api_rate_limit_unauthenticated + raw_blob_request_limit + search_rate_limit + search_rate_limit_unauthenticated + session_expire_delay + sidekiq_job_limiter_compression_threshold_bytes + sidekiq_job_limiter_limit_bytes + terminal_max_session_time + users_get_by_id_limit + ] + end - it { is_expected.to allow_value(0).for(:ci_max_total_yaml_size_bytes) } - it { is_expected.to allow_value(200).for(:ci_max_total_yaml_size_bytes) } - it { is_expected.not_to allow_value('abc').for(:ci_max_total_yaml_size_bytes) } - it { is_expected.not_to allow_value(nil).for(:ci_max_total_yaml_size_bytes) } - it { is_expected.not_to allow_value(10.5).for(:ci_max_total_yaml_size_bytes) } - it { is_expected.not_to allow_value(-1).for(:ci_max_total_yaml_size_bytes) } + with_them do + it { is_expected.to validate_numericality_of(attribute).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.not_to allow_value(nil).for(attribute) } + end + end - it { is_expected.to validate_inclusion_of(:remember_me_enabled).in_array([true, false]) } + context 'for non-null numerical attributes starting from 0' do + where(:attribute) do + %i[ + push_event_hooks_limit + push_event_activities_limit + ] + end - it { is_expected.to validate_numericality_of(:namespace_aggregation_schedule_lease_duration_in_seconds).only_integer.is_greater_than(0) } + with_them do + it { is_expected.to validate_numericality_of(attribute).is_greater_than_or_equal_to(0) } + it { is_expected.not_to allow_value(nil).for(attribute) } + end + end + + context 'for non-null integer attributes starting from 1' do + where(:attribute) do + %i[ + max_attachment_size + max_artifacts_size + container_registry_token_expire_delay + housekeeping_optimize_repository_period + bulk_import_concurrent_pipeline_batch_limit + snippet_size_limit + max_yaml_size_bytes + max_yaml_depth + namespace_aggregation_schedule_lease_duration_in_seconds + throttle_unauthenticated_api_requests_per_period + throttle_unauthenticated_api_period_in_seconds + throttle_unauthenticated_requests_per_period + throttle_unauthenticated_period_in_seconds + throttle_unauthenticated_packages_api_requests_per_period + throttle_unauthenticated_packages_api_period_in_seconds + throttle_unauthenticated_files_api_requests_per_period + throttle_unauthenticated_files_api_period_in_seconds + throttle_unauthenticated_deprecated_api_requests_per_period + throttle_unauthenticated_deprecated_api_period_in_seconds + throttle_authenticated_api_requests_per_period + throttle_authenticated_api_period_in_seconds + throttle_authenticated_git_lfs_requests_per_period + throttle_authenticated_git_lfs_period_in_seconds + throttle_authenticated_web_requests_per_period + throttle_authenticated_web_period_in_seconds + throttle_authenticated_packages_api_requests_per_period + throttle_authenticated_packages_api_period_in_seconds + throttle_authenticated_files_api_requests_per_period + throttle_authenticated_files_api_period_in_seconds + throttle_authenticated_deprecated_api_requests_per_period + throttle_authenticated_deprecated_api_period_in_seconds + throttle_protected_paths_requests_per_period + throttle_protected_paths_period_in_seconds + project_jobs_api_rate_limit + ] + end + + with_them do + it { is_expected.to validate_numericality_of(attribute).only_integer.is_greater_than(0) } + it { is_expected.not_to allow_value(nil).for(attribute) } + end + end + + context 'for null integer attributes starting from 1' do + where(:attribute) do + %i[ + failed_login_attempts_unlock_period_in_minutes + external_pipeline_validation_service_timeout + max_login_attempts + ] + end + + with_them do + it { is_expected.to validate_numericality_of(attribute).only_integer.is_greater_than(0).allow_nil } + end + end + + it { is_expected.to validate_inclusion_of(:remember_me_enabled).in_array([true, false]) } it { is_expected.to validate_inclusion_of(:instance_level_code_suggestions_enabled).in_array([true, false]) } @@ -586,66 +615,6 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do end end - it { is_expected.to validate_presence_of(:max_attachment_size) } - - specify do - is_expected.to validate_numericality_of(:max_attachment_size) - .only_integer - .is_greater_than(0) - end - - it { is_expected.to validate_presence_of(:max_export_size) } - - specify do - is_expected.to validate_numericality_of(:max_export_size) - .only_integer - .is_greater_than_or_equal_to(0) - end - - it { is_expected.to validate_presence_of(:max_import_size) } - - specify do - is_expected.to validate_numericality_of(:max_import_size) - .only_integer - .is_greater_than_or_equal_to(0) - end - - it { is_expected.to validate_presence_of(:max_import_remote_file_size) } - - specify do - is_expected.to validate_numericality_of(:max_import_remote_file_size) - .only_integer - .is_greater_than_or_equal_to(0) - end - - it { is_expected.to validate_presence_of(:bulk_import_max_download_file_size) } - - specify do - is_expected.to validate_numericality_of(:bulk_import_max_download_file_size) - .only_integer - .is_greater_than_or_equal_to(0) - end - - it { is_expected.to validate_presence_of(:max_decompressed_archive_size) } - - specify do - is_expected.to validate_numericality_of(:max_decompressed_archive_size) - .only_integer - .is_greater_than_or_equal_to(0) - end - - specify do - is_expected.to validate_numericality_of(:failed_login_attempts_unlock_period_in_minutes) - .only_integer - .is_greater_than(0) - end - - specify do - is_expected.to validate_numericality_of(:max_login_attempts) - .only_integer - .is_greater_than(0) - end - specify do is_expected.to validate_numericality_of(:local_markdown_version) .only_integer @@ -879,10 +848,6 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do end end - context 'housekeeping settings' do - it { is_expected.not_to allow_value(0).for(:housekeeping_optimize_repository_period) } - end - context 'gitaly timeouts' do it "validates that the default_timeout is lower than the max_request_duration" do is_expected.to validate_numericality_of(:gitaly_timeout_default) @@ -1002,8 +967,8 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it 'the credentials are valid when the private key can be read and matches the certificate' do tls_attributes = [:external_auth_client_key_pass, - :external_auth_client_key, - :external_auth_client_cert] + :external_auth_client_key, + :external_auth_client_cert] setting.external_auth_client_key = File.read('spec/fixtures/passphrase_x509_certificate_pk.key') setting.external_auth_client_key_pass = '5iveL!fe' @@ -1215,43 +1180,6 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do end end - context 'throttle_* settings' do - where(:throttle_setting) do - %i[ - throttle_unauthenticated_api_requests_per_period - throttle_unauthenticated_api_period_in_seconds - throttle_unauthenticated_requests_per_period - throttle_unauthenticated_period_in_seconds - throttle_authenticated_api_requests_per_period - throttle_authenticated_api_period_in_seconds - throttle_authenticated_web_requests_per_period - throttle_authenticated_web_period_in_seconds - throttle_unauthenticated_packages_api_requests_per_period - throttle_unauthenticated_packages_api_period_in_seconds - throttle_authenticated_packages_api_requests_per_period - throttle_authenticated_packages_api_period_in_seconds - throttle_unauthenticated_files_api_requests_per_period - throttle_unauthenticated_files_api_period_in_seconds - throttle_authenticated_files_api_requests_per_period - throttle_authenticated_files_api_period_in_seconds - throttle_unauthenticated_deprecated_api_requests_per_period - throttle_unauthenticated_deprecated_api_period_in_seconds - throttle_authenticated_deprecated_api_requests_per_period - throttle_authenticated_deprecated_api_period_in_seconds - throttle_authenticated_git_lfs_requests_per_period - throttle_authenticated_git_lfs_period_in_seconds - ] - end - - with_them do - it { is_expected.to allow_value(3).for(throttle_setting) } - it { is_expected.not_to allow_value(-3).for(throttle_setting) } - it { is_expected.not_to allow_value(0).for(throttle_setting) } - it { is_expected.not_to allow_value('three').for(throttle_setting) } - it { is_expected.not_to allow_value(nil).for(throttle_setting) } - end - end - context 'sidekiq job limiter settings' do it 'has the right defaults', :aggregate_failures do expect(setting.sidekiq_job_limiter_mode).to eq('compress') @@ -1262,8 +1190,6 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do end it { is_expected.to allow_value('track').for(:sidekiq_job_limiter_mode) } - it { is_expected.to validate_numericality_of(:sidekiq_job_limiter_compression_threshold_bytes).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to validate_numericality_of(:sidekiq_job_limiter_limit_bytes).only_integer.is_greater_than_or_equal_to(0) } end context 'prometheus settings' do @@ -1352,13 +1278,6 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do .with_message("must be a value between 0 and 1") end end - - describe 'bulk_import_concurrent_pipeline_batch_limit' do - it do - is_expected.to validate_numericality_of(:bulk_import_concurrent_pipeline_batch_limit) - .is_greater_than(0) - end - end end context 'restrict creating duplicates' do @@ -1714,8 +1633,6 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_numericality_of(:inactive_projects_delete_after_months).is_greater_than(0) } - it { is_expected.to validate_numericality_of(:inactive_projects_min_size_mb).is_greater_than_or_equal_to(0) } - it "deletes the redis key used for tracking inactive projects deletion warning emails when setting is updated", :clean_gitlab_redis_shared_state do Gitlab::Redis::SharedState.with do |redis| diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index ce143a1aa33..014b050a5b5 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -307,8 +307,7 @@ RSpec.describe BulkImports::Entity, type: :model, feature_category: :importers d import = build(:bulk_import, source_version: '16.2.0') entity = build(:bulk_import_entity, :project_entity, bulk_import: import) - expect(entity.export_relations_url_path(batched: true)) - .to eq("/projects/#{entity.source_xid}/export_relations?batched=true") + expect(entity.export_relations_url_path).to eq("/projects/#{entity.source_xid}/export_relations?batched=true") end end @@ -316,8 +315,7 @@ RSpec.describe BulkImports::Entity, type: :model, feature_category: :importers d it 'returns export relations url' do entity = build(:bulk_import_entity) - expect(entity.export_relations_url_path(batched: true)) - .to eq("/groups/#{entity.source_xid}/export_relations") + expect(entity.export_relations_url_path).to eq("/groups/#{entity.source_xid}/export_relations") end end end diff --git a/spec/models/bulk_imports/failure_spec.rb b/spec/models/bulk_imports/failure_spec.rb index 928f14aaced..bbb5ad52fe1 100644 --- a/spec/models/bulk_imports/failure_spec.rb +++ b/spec/models/bulk_imports/failure_spec.rb @@ -58,4 +58,20 @@ RSpec.describe BulkImports::Failure, type: :model, feature_category: :importers expect(failure.exception_message.size).to eq(255) end end + + describe '#source_title=' do + it 'truncates title to 255 characters' do + failure = described_class.new + failure.source_title = 'A' * 1000 + expect(failure.source_title.size).to eq(255) + end + end + + describe '#source_url=' do + it 'truncates url to 255 characters' do + failure = described_class.new + failure.source_url = 'A' * 1000 + expect(failure.source_url.size).to eq(255) + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 18c7e57d464..d7e91f44e75 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -68,10 +68,17 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def it { is_expected.to delegate_method(:legacy_detached_merge_request_pipeline?).to(:pipeline) } describe 'associations' do + it { is_expected.to belong_to(:project_mirror) } + it 'has a bidirectional relationship with projects' do expect(described_class.reflect_on_association(:project).has_inverse?).to eq(:builds) expect(Project.reflect_on_association(:builds).has_inverse?).to eq(:project) end + + it 'has a bidirectional relationship with project mirror' do + expect(described_class.reflect_on_association(:project_mirror).has_inverse?).to eq(:builds) + expect(Ci::ProjectMirror.reflect_on_association(:builds).has_inverse?).to eq(:project_mirror) + end end describe 'callbacks' do @@ -325,14 +332,15 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def describe '.with_exposed_artifacts' do subject { described_class.with_exposed_artifacts } - let!(:job1) { create(:ci_build, pipeline: pipeline) } + let_it_be(:job1) { create(:ci_build, pipeline: pipeline) } + let_it_be(:job3) { create(:ci_build, pipeline: pipeline) } + let!(:job2) { create(:ci_build, options: options, pipeline: pipeline) } - let!(:job3) { create(:ci_build, pipeline: pipeline) } - context 'when some jobs have exposed artifacs and some not' do + context 'when some jobs have exposed artifacts and some not' do let(:options) { { artifacts: { expose_as: 'test', paths: ['test'] } } } - before do + before_all do job1.ensure_metadata.update!(has_exposed_artifacts: nil) job3.ensure_metadata.update!(has_exposed_artifacts: false) end @@ -356,10 +364,10 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def let(:artifact_scope) { Ci::JobArtifact.where(file_type: 'archive') } - let!(:build_1) { create(:ci_build, :artifacts, pipeline: pipeline) } - let!(:build_2) { create(:ci_build, :codequality_reports, pipeline: pipeline) } - let!(:build_3) { create(:ci_build, :test_reports, pipeline: pipeline) } - let!(:build_4) { create(:ci_build, :artifacts, pipeline: pipeline) } + let_it_be(:build_1) { create(:ci_build, :artifacts, pipeline: pipeline) } + let_it_be(:build_2) { create(:ci_build, :codequality_reports, pipeline: pipeline) } + let_it_be(:build_3) { create(:ci_build, :test_reports, pipeline: pipeline) } + let_it_be(:build_4) { create(:ci_build, :artifacts, pipeline: pipeline) } it 'returns artifacts matching the given scope' do expect(builds).to contain_exactly(build_1, build_4) @@ -383,10 +391,10 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end describe '.with_needs' do - let!(:build) { create(:ci_build, pipeline: pipeline) } - let!(:build_b) { create(:ci_build, pipeline: pipeline) } - let!(:build_need_a) { create(:ci_build_need, build: build) } - let!(:build_need_b) { create(:ci_build_need, build: build_b) } + let_it_be(:build) { create(:ci_build, pipeline: pipeline) } + let_it_be(:build_b) { create(:ci_build, pipeline: pipeline) } + let_it_be(:build_need_a) { create(:ci_build_need, build: build) } + let_it_be(:build_need_b) { create(:ci_build_need, build: build_b) } context 'when passing build name' do subject { described_class.with_needs(build_need_a.name) } @@ -421,6 +429,33 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end end + describe '.belonging_to_runner_manager' do + subject { described_class.belonging_to_runner_manager(runner_manager) } + + let_it_be(:runner) { create(:ci_runner, :group, groups: [group]) } + let_it_be(:build_b) { create(:ci_build, :success) } + + context 'with runner_manager of runner associated with build' do + let!(:runner_manager) { create(:ci_runner_machine, runner: runner) } + let!(:runner_manager_build) { create(:ci_runner_machine_build, build: build, runner_manager: runner_manager) } + + it { is_expected.to contain_exactly(build) } + end + + context 'with runner_manager of runner not associated with build' do + let!(:runner_manager) { create(:ci_runner_machine, runner: instance_runner) } + let!(:instance_runner) { create(:ci_runner, :with_runner_manager) } + + it { is_expected.to be_empty } + end + + context 'with nil runner_manager' do + let(:runner_manager) { nil } + + it { is_expected.to be_empty } + end + end + describe '#stick_build_if_status_changed' do it 'sticks the build if the status changed' do job = create(:ci_build, :pending, pipeline: pipeline) @@ -3262,6 +3297,34 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end end + context 'for the diffblue_cover integration' do + context 'when active' do + let_it_be(:diffblue_cover_integration) { create(:diffblue_cover_integration, active: true) } + + let(:diffblue_cover_variables) do + [ + { key: 'DIFFBLUE_LICENSE_KEY', value: diffblue_cover_integration.diffblue_license_key, masked: true, public: false }, + { key: 'DIFFBLUE_ACCESS_TOKEN_NAME', value: diffblue_cover_integration.diffblue_access_token_name, masked: true, public: false }, + { key: 'DIFFBLUE_ACCESS_TOKEN', value: diffblue_cover_integration.diffblue_access_token_secret, masked: true, public: false } + ] + end + + it 'includes diffblue_cover variables' do + is_expected.to include(*diffblue_cover_variables) + end + end + + context 'when inactive' do + let_it_be(:diffblue_cover_integration) { create(:diffblue_cover_integration, active: false) } + + it 'does not include diffblue_cover variables' do + expect(subject.find { |v| v[:key] == 'DIFFBLUE_LICENSE_KEY' }).to be_nil + expect(subject.find { |v| v[:key] == 'DIFFBLUE_ACCESS_TOKEN_NAME' }).to be_nil + expect(subject.find { |v| v[:key] == 'DIFFBLUE_ACCESS_TOKEN' }).to be_nil + end + end + end + context 'for the google_play integration' do before do allow(build.pipeline).to receive(:protected_ref?).and_return(pipeline_protected_ref) @@ -5507,10 +5570,11 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def stub_current_partition_id end - it 'includes partition_id as a token prefix' do - prefix = ci_build.token.split('_').first.to_i(16) + it 'includes partition_id in the token prefix' do + prefix = ci_build.token.match(/^glcbt-([\h]+)_/) + partition_prefix = prefix[1].to_i(16) - expect(prefix).to eq(ci_testing_partition_id) + expect(partition_prefix).to eq(ci_testing_partition_id) end end @@ -5648,7 +5712,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def it 'generates a token' do expect { ci_build.enqueue } - .to change { ci_build.token }.from(nil).to(a_string_starting_with(partition_id_prefix_in_16_bit_encode)) + .to change { ci_build.token }.from(nil).to(a_string_starting_with("glcbt-#{partition_id_prefix_in_16_bit_encode}")) end end @@ -5665,4 +5729,51 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end end end + + describe '#prefix_and_partition_for_token' do + # 100.to_s(16) -> 64 + let(:ci_build) { described_class.new(partition_id: 100) } + + shared_examples 'partition prefix' do + it 'is prefixed with partition_id' do + ci_build.ensure_token + expect(ci_build.token).to match(/^64_[\w-]{20}$/) + end + end + + shared_examples 'static and partition prefixes' do + it 'is prefixed with static string and partition id' do + ci_build.ensure_token + expect(ci_build.token).to match(/^glcbt-64_[\w-]{20}$/) + end + end + + it_behaves_like 'static and partition prefixes' + + context 'when feature flag is globally disabled' do + before do + stub_feature_flags(prefix_ci_build_tokens: false) + end + + it_behaves_like 'partition prefix' + + context 'when enabled for a different project' do + let_it_be(:project) { create(:project) } + + before do + stub_feature_flags(prefix_ci_build_tokens: project) + end + + it_behaves_like 'partition prefix' + end + + context 'when enabled for the project' do + before do + stub_feature_flags(prefix_ci_build_tokens: ci_build.project) + end + + it_behaves_like 'static and partition prefixes' + end + end + end end diff --git a/spec/models/ci/catalog/resources/version_spec.rb b/spec/models/ci/catalog/resources/version_spec.rb index aafd51699b5..bacaa6355fe 100644 --- a/spec/models/ci/catalog/resources/version_spec.rb +++ b/spec/models/ci/catalog/resources/version_spec.rb @@ -28,6 +28,23 @@ RSpec.describe Ci::Catalog::Resources::Version, type: :model, feature_category: end end + describe '.by_name' do + it 'returns the version that matches the name' do + versions = described_class.by_name('v1.0') + + expect(versions.count).to eq(1) + expect(versions.first.name).to eq('v1.0') + end + + context 'when no version matches the name' do + it 'returns empty response' do + versions = described_class.by_name('does_not_exist') + + expect(versions).to be_empty + end + end + end + describe '.order_by_created_at_asc' do it 'returns versions ordered by created_at ascending' do versions = described_class.order_by_created_at_asc @@ -127,9 +144,9 @@ RSpec.describe Ci::Catalog::Resources::Version, type: :model, feature_category: describe '#name' do it 'is equivalent to release.tag' do - release_v1_0.update!(name: 'Release v1.0') + v1_0.release.update!(name: 'Release v1.0') - expect(v1_0.name).to eq(release_v1_0.tag) + expect(v1_0.name).to eq(v1_0.release.tag) end end @@ -142,10 +159,17 @@ RSpec.describe Ci::Catalog::Resources::Version, type: :model, feature_category: context 'when the sha is nil' do it 'returns nil' do - release_v1_0.update!(sha: nil) + v1_0.release.update!(sha: nil) is_expected.to be_nil end end end + + describe '#readme' do + it 'returns the correct readme for the version' do + expect(v1_0.readme.data).to include('Readme v1.0') + expect(v1_1.readme.data).to include('Readme v1.1') + end + end end diff --git a/spec/models/ci/instance_variable_spec.rb b/spec/models/ci/instance_variable_spec.rb index 0ef1dfbd55c..9a6a78ee5f4 100644 --- a/spec/models/ci/instance_variable_spec.rb +++ b/spec/models/ci/instance_variable_spec.rb @@ -113,4 +113,10 @@ RSpec.describe Ci::InstanceVariable do end end end + + describe "description" do + it { is_expected.to allow_values('').for(:description) } + it { is_expected.to allow_values(nil).for(:description) } + it { is_expected.to validate_length_of(:description).is_at_most(255) } + end end diff --git a/spec/models/ci/namespace_mirror_spec.rb b/spec/models/ci/namespace_mirror_spec.rb index 63e6e9e6b26..8db8fd4e067 100644 --- a/spec/models/ci/namespace_mirror_spec.rb +++ b/spec/models/ci/namespace_mirror_spec.rb @@ -21,6 +21,16 @@ RSpec.describe Ci::NamespaceMirror do ) end + describe 'associations' do + it { is_expected.to belong_to(:namespace) } + it { is_expected.to have_many(:project_mirrors) } + + it 'has a bidirectional relationship with project mirrors' do + expect(described_class.reflect_on_association(:project_mirrors).has_inverse?).to eq(:namespace_mirror) + expect(Ci::ProjectMirror.reflect_on_association(:namespace_mirror).has_inverse?).to eq(:project_mirrors) + end + end + context 'scopes' do describe '.by_group_and_descendants' do let_it_be(:another_group) { create(:group) } diff --git a/spec/models/ci/pipeline_artifact_spec.rb b/spec/models/ci/pipeline_artifact_spec.rb index eb89c7af208..1cb99ec22b9 100644 --- a/spec/models/ci/pipeline_artifact_spec.rb +++ b/spec/models/ci/pipeline_artifact_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::PipelineArtifact, type: :model do +RSpec.describe Ci::PipelineArtifact, type: :model, feature_category: :build_artifacts do let(:coverage_report) { create(:ci_pipeline_artifact, :with_coverage_report) } describe 'associations' do @@ -309,4 +309,19 @@ RSpec.describe Ci::PipelineArtifact, type: :model do let!(:model) { create(:ci_pipeline_artifact, project: parent) } end end + + describe 'partitioning', :ci_partitionable do + include Ci::PartitioningHelpers + + let(:pipeline) { create(:ci_pipeline) } + let(:pipeline_artifact) { create(:ci_pipeline_artifact, pipeline: pipeline) } + + before do + stub_current_partition_id + end + + it 'assigns the same partition id as the one that pipeline has' do + expect(pipeline_artifact.partition_id).to eq(ci_testing_partition_id) + end + end end diff --git a/spec/models/ci/pipeline_chat_data_spec.rb b/spec/models/ci/pipeline_chat_data_spec.rb new file mode 100644 index 00000000000..4c9dc7edd88 --- /dev/null +++ b/spec/models/ci/pipeline_chat_data_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PipelineChatData, type: :model, feature_category: :continuous_integration do + it { is_expected.to belong_to(:chat_name) } + it { is_expected.to belong_to(:pipeline) } + + it { is_expected.to validate_presence_of(:pipeline_id) } + it { is_expected.to validate_presence_of(:chat_name_id) } + it { is_expected.to validate_presence_of(:response_url) } + + describe 'partitioning', :ci_partitionable do + include Ci::PartitioningHelpers + + let(:pipeline) { create(:ci_pipeline) } + let(:pipeline_chat_data) { create(:ci_pipeline_chat_data, pipeline: pipeline) } + + before do + stub_current_partition_id + end + + it 'assigns the same partition id as the one that pipeline has' do + expect(pipeline_chat_data.partition_id).to eq(ci_testing_partition_id) + end + end +end diff --git a/spec/models/ci/pipeline_config_spec.rb b/spec/models/ci/pipeline_config_spec.rb index 3d033d33df3..3368c40fb57 100644 --- a/spec/models/ci/pipeline_config_spec.rb +++ b/spec/models/ci/pipeline_config_spec.rb @@ -2,9 +2,24 @@ require 'spec_helper' -RSpec.describe Ci::PipelineConfig, type: :model do +RSpec.describe Ci::PipelineConfig, type: :model, feature_category: :continuous_integration do it { is_expected.to belong_to(:pipeline) } it { is_expected.to validate_presence_of(:pipeline) } it { is_expected.to validate_presence_of(:content) } + + describe 'partitioning', :ci_partitionable do + include Ci::PartitioningHelpers + + let(:pipeline) { create(:ci_pipeline) } + let(:pipeline_config) { create(:ci_pipeline_config, pipeline: pipeline) } + + before do + stub_current_partition_id + end + + it 'assigns the same partition id as the one that pipeline has' do + expect(pipeline_config.partition_id).to eq(ci_testing_partition_id) + end + end end diff --git a/spec/models/ci/pipeline_metadata_spec.rb b/spec/models/ci/pipeline_metadata_spec.rb index 1a426118063..efe180b1db3 100644 --- a/spec/models/ci/pipeline_metadata_spec.rb +++ b/spec/models/ci/pipeline_metadata_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Ci::PipelineMetadata, feature_category: :pipeline_composition do is_expected.to define_enum_for( :auto_cancel_on_new_commit ).with_values( - conservative: 0, interruptible: 1, disabled: 2 + conservative: 0, interruptible: 1, none: 2 ).with_prefix end @@ -27,4 +27,19 @@ RSpec.describe Ci::PipelineMetadata, feature_category: :pipeline_composition do ).with_prefix end end + + describe 'partitioning', :ci_partitionable do + include Ci::PartitioningHelpers + + let(:pipeline) { create(:ci_pipeline) } + let(:pipeline_metadata) { create(:ci_pipeline_metadata, pipeline: pipeline) } + + before do + stub_current_partition_id + end + + it 'assigns the same partition id as the one that pipeline has' do + expect(pipeline_metadata.partition_id).to eq(ci_testing_partition_id) + end + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 024d3ae4240..52c3792ac93 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2349,14 +2349,14 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end it 'runs on a branch update push' do - expect(pipeline.before_sha).not_to be Gitlab::Git::BLANK_SHA + expect(pipeline.before_sha).not_to be Gitlab::Git::SHA1_BLANK_SHA expect(pipeline.branch_updated?).to be true end end context 'when pipeline does not have before SHA' do before do - pipeline.update!(before_sha: Gitlab::Git::BLANK_SHA) + pipeline.update!(before_sha: Gitlab::Git::SHA1_BLANK_SHA) end it 'does not run on a branch updating push' do @@ -2384,7 +2384,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: context 'when either old or new revision is missing' do before do - pipeline.update!(before_sha: Gitlab::Git::BLANK_SHA) + pipeline.update!(before_sha: Gitlab::Git::SHA1_BLANK_SHA) end it 'returns nil' do @@ -5727,4 +5727,36 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end end end + + describe '#auto_cancel_on_new_commit' do + let_it_be_with_reload(:pipeline) { create(:ci_pipeline, project: project) } + + subject(:auto_cancel_on_new_commit) { pipeline.auto_cancel_on_new_commit } + + context 'when pipeline_metadata is not present' do + it { is_expected.to eq('conservative') } + end + + context 'when pipeline_metadata is present' do + before_all do + create(:ci_pipeline_metadata, project: pipeline.project, pipeline: pipeline) + end + + context 'when auto_cancel_on_new_commit is nil' do + before do + pipeline.pipeline_metadata.auto_cancel_on_new_commit = nil + end + + it { is_expected.to eq('conservative') } + end + + context 'when auto_cancel_on_new_commit is a valid value' do + before do + pipeline.pipeline_metadata.auto_cancel_on_new_commit = 'interruptible' + end + + it { is_expected.to eq('interruptible') } + end + end + end end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 5d457c4f213..d74441f93a6 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -83,7 +83,7 @@ RSpec.describe Ci::Processable, feature_category: :continuous_integration do let(:ignore_accessors) do %i[type namespace lock_version target_url base_tags trace_sections - commit_id deployment erased_by_id project_id + commit_id deployment erased_by_id project_id project_mirror runner_id tag_taggings taggings tags trigger_request_id user_id auto_canceled_by_id retried failure_reason sourced_pipelines sourced_pipeline artifacts_file_store artifacts_metadata_store diff --git a/spec/models/ci/project_mirror_spec.rb b/spec/models/ci/project_mirror_spec.rb index 5ef520b4230..491ae353ffe 100644 --- a/spec/models/ci/project_mirror_spec.rb +++ b/spec/models/ci/project_mirror_spec.rb @@ -8,6 +8,22 @@ RSpec.describe Ci::ProjectMirror do let!(:project) { create(:project, namespace: group2) } + describe 'associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:namespace_mirror) } + it { is_expected.to have_many(:builds) } + + it 'has a bidirectional relationship with namespace mirror' do + expect(described_class.reflect_on_association(:namespace_mirror).has_inverse?).to eq(:project_mirrors) + expect(Ci::NamespaceMirror.reflect_on_association(:project_mirrors).has_inverse?).to eq(:namespace_mirror) + end + + it 'has a bidirectional relationship with builds' do + expect(described_class.reflect_on_association(:builds).has_inverse?).to eq(:project_mirror) + expect(Ci::Build.reflect_on_association(:project_mirror).has_inverse?).to eq(:builds) + end + end + context 'scopes' do let_it_be(:another_project) { create(:project, namespace: group1) } diff --git a/spec/models/ci/runner_manager_spec.rb b/spec/models/ci/runner_manager_spec.rb index 02a72afe0c6..4fdfbae997d 100644 --- a/spec/models/ci/runner_manager_spec.rb +++ b/spec/models/ci/runner_manager_spec.rb @@ -41,20 +41,70 @@ RSpec.describe Ci::RunnerManager, feature_category: :fleet_visibility, type: :mo end end - describe '.stale', :freeze_time do - subject { described_class.stale.ids } + describe 'status scopes' do + let_it_be(:runner) { create(:ci_runner, :instance) } - let!(:runner_manager1) { create(:ci_runner_machine, :stale) } - let!(:runner_manager2) { create(:ci_runner_machine, :stale, contacted_at: nil) } - let!(:runner_manager3) { create(:ci_runner_machine, created_at: 6.months.ago, contacted_at: Time.current) } - let!(:runner_manager4) { create(:ci_runner_machine, created_at: 5.days.ago) } - let!(:runner_manager5) do - create(:ci_runner_machine, created_at: (7.days - 1.second).ago, contacted_at: (7.days - 1.second).ago) + let_it_be(:offline_runner_manager) { create(:ci_runner_machine, runner: runner, contacted_at: 2.hours.ago) } + let_it_be(:online_runner_manager) { create(:ci_runner_machine, runner: runner, contacted_at: 1.second.ago) } + let_it_be(:never_contacted_runner_manager) { create(:ci_runner_machine, runner: runner, contacted_at: nil) } + + describe '.online' do + subject(:runner_managers) { described_class.online } + + it 'returns online runner managers' do + expect(runner_managers).to contain_exactly(online_runner_manager) + end + end + + describe '.offline' do + subject(:runner_managers) { described_class.offline } + + it 'returns offline runner managers' do + expect(runner_managers).to contain_exactly(offline_runner_manager) + end end - it 'returns stale runner managers' do - is_expected.to match_array([runner_manager1.id, runner_manager2.id]) + describe '.never_contacted' do + subject(:runner_managers) { described_class.never_contacted } + + it 'returns never contacted runner managers' do + expect(runner_managers).to contain_exactly(never_contacted_runner_manager) + end end + + describe '.stale', :freeze_time do + subject { described_class.stale } + + let!(:stale_runner_manager1) do + create( + :ci_runner_machine, + runner: runner, + created_at: described_class.stale_deadline - 1.second, + contacted_at: nil + ) + end + + let!(:stale_runner_manager2) do + create( + :ci_runner_machine, + runner: runner, + created_at: 8.days.ago, + contacted_at: described_class.stale_deadline - 1.second + ) + end + + it 'returns stale runner managers' do + is_expected.to contain_exactly(stale_runner_manager1, stale_runner_manager2) + end + end + + include_examples 'runner with status scope' + end + + describe '.available_statuses' do + subject { described_class.available_statuses } + + it { is_expected.to eq(%w[online offline never_contacted stale]) } end describe '.online_contact_time_deadline', :freeze_time do @@ -72,23 +122,60 @@ RSpec.describe Ci::RunnerManager, feature_category: :fleet_visibility, type: :mo describe '.for_runner' do subject(:runner_managers) { described_class.for_runner(runner_arg) } - let_it_be(:runner1) { create(:ci_runner) } - let_it_be(:runner_manager11) { create(:ci_runner_machine, runner: runner1) } - let_it_be(:runner_manager12) { create(:ci_runner_machine, runner: runner1) } + let_it_be(:runner_a) { create(:ci_runner) } + let_it_be(:runner_manager_a1) { create(:ci_runner_machine, runner: runner_a) } + let_it_be(:runner_manager_a2) { create(:ci_runner_machine, runner: runner_a) } context 'with single runner' do - let(:runner_arg) { runner1 } + let(:runner_arg) { runner_a } - it { is_expected.to contain_exactly(runner_manager11, runner_manager12) } + it { is_expected.to contain_exactly(runner_manager_a1, runner_manager_a2) } end context 'with multiple runners' do - let(:runner_arg) { [runner1, runner2] } + let(:runner_arg) { [runner_a, runner_b] } - let_it_be(:runner2) { create(:ci_runner) } - let_it_be(:runner_manager2) { create(:ci_runner_machine, runner: runner2) } + let_it_be(:runner_b) { create(:ci_runner) } + let_it_be(:runner_manager_b1) { create(:ci_runner_machine, runner: runner_b) } - it { is_expected.to contain_exactly(runner_manager11, runner_manager12, runner_manager2) } + it { is_expected.to contain_exactly(runner_manager_a1, runner_manager_a2, runner_manager_b1) } + end + end + + describe '.with_system_xid' do + subject(:runner_managers) { described_class.with_system_xid(system_xid) } + + let_it_be(:runner_a) { create(:ci_runner) } + let_it_be(:runner_b) { create(:ci_runner) } + let_it_be(:runner_manager_a1) { create(:ci_runner_machine, runner: runner_a, system_xid: 'id1') } + let_it_be(:runner_manager_a2) { create(:ci_runner_machine, runner: runner_a, system_xid: 'id2') } + let_it_be(:runner_manager_b1) { create(:ci_runner_machine, runner: runner_b, system_xid: 'id1') } + + context 'with single system id' do + let(:system_xid) { 'id2' } + + it { is_expected.to contain_exactly(runner_manager_a2) } + end + + context 'with multiple system ids' do + let(:system_xid) { %w[id1 id2] } + + it { is_expected.to contain_exactly(runner_manager_a1, runner_manager_a2, runner_manager_b1) } + end + + context 'when chained with another scope' do + subject(:runner_managers) { described_class.for_runner(runner).with_system_xid(system_xid) } + + let(:runner) { runner_a } + let(:system_xid) { 'id1' } + + it { is_expected.to contain_exactly(runner_manager_a1) } + + context 'with another runner' do + let(:runner) { runner_b } + + it { is_expected.to contain_exactly(runner_manager_b1) } + end end end @@ -96,18 +183,18 @@ RSpec.describe Ci::RunnerManager, feature_category: :fleet_visibility, type: :mo let!(:runner_version1) { create(:ci_runner_version, version: '16.0.0', status: :recommended) } let!(:runner_version2) { create(:ci_runner_version, version: '16.0.1', status: :available) } - let!(:runner1) { create(:ci_runner) } - let!(:runner2) { create(:ci_runner) } - let!(:runner_manager11) { create(:ci_runner_machine, runner: runner1, version: runner_version1.version) } - let!(:runner_manager12) { create(:ci_runner_machine, runner: runner1, version: runner_version2.version) } - let!(:runner_manager2) { create(:ci_runner_machine, runner: runner2, version: runner_version2.version) } + let!(:runner_a) { create(:ci_runner) } + let!(:runner_b) { create(:ci_runner) } + let!(:runner_manager_a1) { create(:ci_runner_machine, runner: runner_a, version: runner_version1.version) } + let!(:runner_manager_a2) { create(:ci_runner_machine, runner: runner_a, version: runner_version2.version) } + let!(:runner_manager_b1) { create(:ci_runner_machine, runner: runner_b, version: runner_version2.version) } subject { described_class.aggregate_upgrade_status_by_runner_id } it 'contains aggregate runner upgrade status by runner ID' do is_expected.to eq({ - runner1.id => :recommended, - runner2.id => :available + runner_a.id => :recommended, + runner_b.id => :available }) end end @@ -139,6 +226,108 @@ RSpec.describe Ci::RunnerManager, feature_category: :fleet_visibility, type: :mo it { is_expected.to eq([runner_manager2, runner_manager1]) } end + describe '.with_upgrade_status' do + subject(:scope) { described_class.with_upgrade_status(upgrade_status) } + + let_it_be(:runner_manager_14_0_0) { create(:ci_runner_machine, version: '14.0.0') } + let_it_be(:runner_manager_14_1_0) { create(:ci_runner_machine, version: '14.1.0') } + let_it_be(:runner_manager_14_1_1) { create(:ci_runner_machine, version: '14.1.1') } + + before_all do + create(:ci_runner_version, version: '14.0.0', status: :available) + create(:ci_runner_version, version: '14.1.0', status: :recommended) + create(:ci_runner_version, version: '14.1.1', status: :unavailable) + end + + context 'as :unavailable' do + let(:upgrade_status) { :unavailable } + + it 'returns runners with runner managers whose version is assigned :unavailable' do + is_expected.to contain_exactly(runner_manager_14_1_1) + end + end + + context 'as :available' do + let(:upgrade_status) { :available } + + it 'returns runners with runner managers whose version is assigned :available' do + is_expected.to contain_exactly(runner_manager_14_0_0) + end + end + + context 'as :recommended' do + let(:upgrade_status) { :recommended } + + it 'returns runners with runner managers whose version is assigned :recommended' do + is_expected.to contain_exactly(runner_manager_14_1_0) + end + end + end + + describe '.with_version_prefix' do + subject { described_class.with_version_prefix(version_prefix) } + + let_it_be(:runner_manager1) { create(:ci_runner_machine, version: '15.11.0') } + let_it_be(:runner_manager2) { create(:ci_runner_machine, version: '15.9.0') } + let_it_be(:runner_manager3) { create(:ci_runner_machine, version: '15.11.5') } + + context 'with a prefix string of "15."' do + let(:version_prefix) { "15." } + + it 'returns runner managers' do + is_expected.to contain_exactly(runner_manager1, runner_manager2, runner_manager3) + end + end + + context 'with a prefix string of "15"' do + let(:version_prefix) { "15" } + + it 'returns runner managers' do + is_expected.to contain_exactly(runner_manager1, runner_manager2, runner_manager3) + end + end + + context 'with a prefix string of "15.11."' do + let(:version_prefix) { "15.11." } + + it 'returns runner managers' do + is_expected.to contain_exactly(runner_manager1, runner_manager3) + end + end + + context 'with a prefix string of "15.11"' do + let(:version_prefix) { "15.11" } + + it 'returns runner managers' do + is_expected.to contain_exactly(runner_manager1, runner_manager3) + end + end + + context 'with a prefix string of "15.9"' do + let(:version_prefix) { "15.9" } + + it 'returns runner managers' do + is_expected.to contain_exactly(runner_manager2) + end + end + + context 'with a prefix string of "15.11.5"' do + let(:version_prefix) { "15.11.5" } + + it 'returns runner managers' do + is_expected.to contain_exactly(runner_manager3) + end + end + + context 'with a malformed prefix of "V2"' do + let(:version_prefix) { "V2" } + + it 'returns no runner managers' do + is_expected.to be_empty + end + end + end + describe '#status', :freeze_time do let(:runner_manager) { build(:ci_runner_machine, created_at: 8.days.ago) } @@ -375,106 +564,4 @@ RSpec.describe Ci::RunnerManager, feature_category: :fleet_visibility, type: :mo it { is_expected.to contain_exactly build } end end - - describe '.with_upgrade_status' do - subject(:scope) { described_class.with_upgrade_status(upgrade_status) } - - let_it_be(:runner_manager_14_0_0) { create(:ci_runner_machine, version: '14.0.0') } - let_it_be(:runner_manager_14_1_0) { create(:ci_runner_machine, version: '14.1.0') } - let_it_be(:runner_manager_14_1_1) { create(:ci_runner_machine, version: '14.1.1') } - - before_all do - create(:ci_runner_version, version: '14.0.0', status: :available) - create(:ci_runner_version, version: '14.1.0', status: :recommended) - create(:ci_runner_version, version: '14.1.1', status: :unavailable) - end - - context 'as :unavailable' do - let(:upgrade_status) { :unavailable } - - it 'returns runners with runner managers whose version is assigned :unavailable' do - is_expected.to contain_exactly(runner_manager_14_1_1) - end - end - - context 'as :available' do - let(:upgrade_status) { :available } - - it 'returns runners with runner managers whose version is assigned :available' do - is_expected.to contain_exactly(runner_manager_14_0_0) - end - end - - context 'as :recommended' do - let(:upgrade_status) { :recommended } - - it 'returns runners with runner managers whose version is assigned :recommended' do - is_expected.to contain_exactly(runner_manager_14_1_0) - end - end - end - - describe '.with_version_prefix' do - subject { described_class.with_version_prefix(version_prefix) } - - let_it_be(:runner_manager1) { create(:ci_runner_machine, version: '15.11.0') } - let_it_be(:runner_manager2) { create(:ci_runner_machine, version: '15.9.0') } - let_it_be(:runner_manager3) { create(:ci_runner_machine, version: '15.11.5') } - - context 'with a prefix string of "15."' do - let(:version_prefix) { "15." } - - it 'returns runner managers' do - is_expected.to contain_exactly(runner_manager1, runner_manager2, runner_manager3) - end - end - - context 'with a prefix string of "15"' do - let(:version_prefix) { "15" } - - it 'returns runner managers' do - is_expected.to contain_exactly(runner_manager1, runner_manager2, runner_manager3) - end - end - - context 'with a prefix string of "15.11."' do - let(:version_prefix) { "15.11." } - - it 'returns runner managers' do - is_expected.to contain_exactly(runner_manager1, runner_manager3) - end - end - - context 'with a prefix string of "15.11"' do - let(:version_prefix) { "15.11" } - - it 'returns runner managers' do - is_expected.to contain_exactly(runner_manager1, runner_manager3) - end - end - - context 'with a prefix string of "15.9"' do - let(:version_prefix) { "15.9" } - - it 'returns runner managers' do - is_expected.to contain_exactly(runner_manager2) - end - end - - context 'with a prefix string of "15.11.5"' do - let(:version_prefix) { "15.11.5" } - - it 'returns runner managers' do - is_expected.to contain_exactly(runner_manager3) - end - end - - context 'with a malformed prefix of "V2"' do - let(:version_prefix) { "V2" } - - it 'returns no runner managers' do - is_expected.to be_empty - end - end - end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index bb9ac084ed6..d4f7db3bddd 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -532,7 +532,7 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do let_it_be(:runner3) { create(:ci_runner, creator_id: 1) } let_it_be(:runner4) { create(:ci_runner, creator_id: nil) } - it 'returns runners with creator_id \'1\'' do + it "returns runners with creator_id '1'" do is_expected.to contain_exactly(runner2, runner3) end end @@ -557,19 +557,6 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do end end - describe '.stale', :freeze_time do - subject { described_class.stale } - - let!(:runner1) { create(:ci_runner, :instance, created_at: 4.months.ago, contacted_at: 3.months.ago + 1.second) } - let!(:runner2) { create(:ci_runner, :instance, created_at: 4.months.ago, contacted_at: 3.months.ago) } - let!(:runner3) { create(:ci_runner, :instance, created_at: 3.months.ago, contacted_at: nil) } - let!(:runner4) { create(:ci_runner, :instance, created_at: 2.months.ago, contacted_at: nil) } - - it 'returns stale runners' do - is_expected.to match_array([runner2, runner3]) - end - end - describe '#stale?', :clean_gitlab_redis_cache, :freeze_time do let(:runner) { build(:ci_runner, :instance) } @@ -632,15 +619,6 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do end end - describe '.online', :freeze_time do - subject { described_class.online } - - let!(:runner1) { create(:ci_runner, :instance, contacted_at: 2.hours.ago) } - let!(:runner2) { create(:ci_runner, :instance, contacted_at: 1.second.ago) } - - it { is_expected.to match_array([runner2]) } - end - describe '#online?', :clean_gitlab_redis_cache, :freeze_time do let(:runner) { build(:ci_runner, :instance) } @@ -715,15 +693,6 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do end end - describe '.offline' do - subject { described_class.offline } - - let!(:runner1) { create(:ci_runner, :instance, contacted_at: 2.hours.ago) } - let!(:runner2) { create(:ci_runner, :instance, contacted_at: 1.second.ago) } - - it { is_expected.to eq([runner1]) } - end - describe '.with_running_builds' do subject { described_class.with_running_builds } @@ -1229,6 +1198,46 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do end end + describe '#clear_heartbeat', :freeze_time do + let!(:runner) { create(:ci_runner, :project, version: '15.0.0') } + let(:heartbeat_values) do + { + version: '15.0.1', + platform: 'darwin', + architecture: '18-bit', + ip_address: '1.1.1.1', + executor: 'shell', + revision: 'sha', + config: { 'gpus' => 'all' } + } + end + + let(:expected_attributes) { heartbeat_values.except(:executor).merge(executor_type: 'shell') } + let(:expected_cleared_attributes) { expected_attributes.to_h { |key, _| [key, nil] }.merge(config: {}) } + + it 'clears contacted at and other attributes' do + expect do + runner.heartbeat(heartbeat_values) + end.to change { runner.reload.contacted_at }.from(nil).to(Time.current) + .and change { runner.reload.uncached_contacted_at }.from(nil).to(Time.current) + + expected_attributes.each do |key, value| + expect(runner.public_send(key)).to eq(value) + expect(runner.read_attribute(key)).to eq(value) + end + + expect do + runner.clear_heartbeat + end.to change { runner.reload.contacted_at }.from(Time.current).to(nil) + .and change { runner.reload.uncached_contacted_at }.from(Time.current).to(nil) + + expected_cleared_attributes.each do |key, value| + expect(runner.public_send(key)).to eq(value) + expect(runner.read_attribute(key)).to eq(value) + end + end + end + describe '#destroy' do let(:runner) { create(:ci_runner) } @@ -2126,4 +2135,102 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do end end end + + describe 'status scopes' do + let_it_be(:online_runner) { create(:ci_runner, :instance, contacted_at: 1.second.ago) } + let_it_be(:offline_runner) { create(:ci_runner, :instance, contacted_at: 2.hours.ago) } + let_it_be(:never_contacted_runner) { create(:ci_runner, :instance, contacted_at: nil) } + + describe '.online' do + subject(:runners) { described_class.online } + + it 'returns online runners' do + expect(runners).to contain_exactly(online_runner) + end + end + + describe '.offline' do + subject(:runners) { described_class.offline } + + it 'returns offline runners' do + expect(runners).to contain_exactly(offline_runner) + end + end + + describe '.never_contacted' do + subject(:runners) { described_class.never_contacted } + + it 'returns never contacted runners' do + expect(runners).to contain_exactly(never_contacted_runner) + end + end + + describe '.stale', :freeze_time do + subject { described_class.stale } + + let!(:stale_runner1) do + create(:ci_runner, :instance, created_at: described_class.stale_deadline - 1.second, contacted_at: nil) + end + + let!(:stale_runner2) do + create(:ci_runner, :instance, created_at: 4.months.ago, contacted_at: described_class.stale_deadline - 1.second) + end + + it 'returns stale runners' do + is_expected.to contain_exactly(stale_runner1, stale_runner2) + end + end + + include_examples 'runner with status scope' + end + + describe '.available_statuses' do + subject { described_class.available_statuses } + + it { is_expected.to eq(%w[active paused online offline never_contacted stale]) } + end + + describe '.online_contact_time_deadline', :freeze_time do + subject { described_class.online_contact_time_deadline } + + it { is_expected.to eq(2.hours.ago) } + end + + describe '.stale_deadline', :freeze_time do + subject { described_class.stale_deadline } + + it { is_expected.to eq(3.months.ago) } + end + + describe '.with_runner_type' do + subject { described_class.with_runner_type(runner_type) } + + let_it_be(:instance_runner) { create(:ci_runner, :instance) } + let_it_be(:group_runner) { create(:ci_runner, :group) } + let_it_be(:project_runner) { create(:ci_runner, :project) } + + context 'with instance_type' do + let(:runner_type) { 'instance_type' } + + it { is_expected.to contain_exactly(instance_runner) } + end + + context 'with group_type' do + let(:runner_type) { 'group_type' } + + it { is_expected.to contain_exactly(group_runner) } + end + + context 'with project_type' do + let(:runner_type) { 'project_type' } + + it { is_expected.to contain_exactly(project_runner) } + end + + context 'with invalid runner type' do + let(:runner_type) { 'invalid runner type' } + + it { is_expected.to contain_exactly(instance_runner, group_runner, project_runner) } + end + end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 5fc5bbd41ff..a95f56ea714 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -576,9 +576,9 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching, it 'avoids N+1 queries' do another_project = create(:project) - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do described_class.ancestor_clusters_for_clusterable(another_project, hierarchy_order: hierarchy_order) - end.count + end cluster2 = create(:cluster, :provided_by_gcp, :group) child2 = cluster2.group @@ -587,7 +587,7 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching, expect do described_class.ancestor_clusters_for_clusterable(project, hierarchy_order: hierarchy_order) - end.not_to exceed_query_limit(control_count) + end.not_to exceed_query_limit(control) end context 'for a group' do diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb index be80aced3fd..5db417f8032 100644 --- a/spec/models/commit_collection_spec.rb +++ b/spec/models/commit_collection_spec.rb @@ -210,7 +210,7 @@ RSpec.describe CommitCollection, feature_category: :source_code_management do it 'returns the original commit if the commit could not be lazy loaded' do collection = described_class.new(project, [hash_commit]) - unexisting_lazy_commit = Commit.lazy(project, Gitlab::Git::BLANK_SHA) + unexisting_lazy_commit = Commit.lazy(project, Gitlab::Git::SHA1_BLANK_SHA) expect(Commit).to receive(:lazy).with(project, hash_commit.id).and_return(unexisting_lazy_commit) diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 618dd3a3f77..7c4917596a0 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -583,7 +583,7 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do end it 'returns blank sha' do - is_expected.to eq(Gitlab::Git::BLANK_SHA) + is_expected.to eq(Gitlab::Git::SHA1_BLANK_SHA) end end diff --git a/spec/models/concerns/commit_signature_spec.rb b/spec/models/concerns/commit_signature_spec.rb index 4bba5a6ee41..9a4ac165ac1 100644 --- a/spec/models/concerns/commit_signature_spec.rb +++ b/spec/models/concerns/commit_signature_spec.rb @@ -2,15 +2,15 @@ require 'spec_helper' -RSpec.describe CommitSignature do +RSpec.describe CommitSignature, feature_category: :source_code_management do + subject(:implementation) do + Class.new(ActiveRecord::Base) do + self.table_name = 'ssh_signatures' + end.include(described_class).new + end + 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 @@ -18,4 +18,12 @@ RSpec.describe CommitSignature do end end end + + describe 'enums' do + it 'defines enums for verification statuses' do + is_expected.to define_enum_for(:verification_status).with_values( + ::Enums::CommitSignature.verification_statuses + ) + end + end end diff --git a/spec/models/concerns/database_event_tracking_spec.rb b/spec/models/concerns/database_event_tracking_spec.rb deleted file mode 100644 index a99b4737537..00000000000 --- a/spec/models/concerns/database_event_tracking_spec.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe DatabaseEventTracking, :snowplow, feature_category: :service_ping do - before do - allow(Gitlab::Tracking).to receive(:database_event).and_call_original - end - - let(:test_class) do - Class.new(ActiveRecord::Base) do - include DatabaseEventTracking - - self.table_name = 'application_setting_terms' - - self::SNOWPLOW_ATTRIBUTES = %w[id].freeze # rubocop:disable RSpec/LeakyConstantDeclaration - end - end - - subject(:create_test_class_record) { test_class.create!(id: 1, terms: "") } - - context 'if event emmiter failed' do - before do - allow(Gitlab::Tracking).to receive(:database_event).and_raise(StandardError) # rubocop:disable RSpec/ExpectGitlabTracking - end - - it 'tracks the exception' do - expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) - - create_test_class_record - end - end - - describe 'event tracking' do - let(:category) { test_class.to_s } - let(:event) { 'database_event' } - - it 'when created' do - create_test_class_record - - expect_snowplow_event( - tracking_method: :database_event, - category: category, - action: "#{event}_create", - label: 'application_setting_terms', - property: 'create', - namespace: nil, - project: nil, - "id" => 1 - ) - end - - it 'when updated' do - create_test_class_record - test_class.first.update!(id: 3) - - expect_snowplow_event( - tracking_method: :database_event, - category: category, - action: "#{event}_update", - label: 'application_setting_terms', - property: 'update', - namespace: nil, - project: nil, - "id" => 3 - ) - end - - it 'when destroyed' do - create_test_class_record - test_class.first.destroy! - - expect_snowplow_event( - tracking_method: :database_event, - category: category, - action: "#{event}_destroy", - label: 'application_setting_terms', - property: 'destroy', - namespace: nil, - project: nil, - "id" => 1 - ) - end - end -end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index e71392f7bbc..a9149b0eebe 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -82,15 +82,15 @@ RSpec.shared_examples 'routable resource' do end end - context 'on the usage of `use_includes` parameter' do + context 'on the usage of `preload_routes` parameter' do let_it_be(:klass) { record.class.to_s.downcase } let_it_be(:record_3) { create(:"#{klass}") } let_it_be(:record_4) { create(:"#{klass}") } - context 'when use_includes: true' do + context 'when preload_routes: true' do it 'includes route information when loading records' do - control_count = ActiveRecord::QueryRecorder.new do - described_class.where_full_path_in([record.full_path, record_2.full_path], use_includes: true) + control = ActiveRecord::QueryRecorder.new do + described_class.where_full_path_in([record.full_path, record_2.full_path], preload_routes: true) .map(&:route) end @@ -101,16 +101,16 @@ RSpec.shared_examples 'routable resource' do record_2.full_path, record_3.full_path, record_4.full_path - ], use_includes: true) + ], preload_routes: true) .map(&:route) - end.to issue_same_number_of_queries_as(control_count) + end.to issue_same_number_of_queries_as(control) end end - context 'when use_includes: false' do + context 'when preload_routes: false' do it 'does not include route information when loading records' do control_count = ActiveRecord::QueryRecorder.new do - described_class.where_full_path_in([record.full_path, record_2.full_path], use_includes: false) + described_class.where_full_path_in([record.full_path, record_2.full_path], preload_routes: false) .map(&:route) end @@ -121,7 +121,7 @@ RSpec.shared_examples 'routable resource' do record_2.full_path, record_3.full_path, record_4.full_path - ], use_includes: false) + ], preload_routes: false) .map(&:route) end.not_to issue_same_number_of_queries_as(control_count) end @@ -130,14 +130,6 @@ RSpec.shared_examples 'routable resource' do end it_behaves_like '.where_full_path_in', :aggregate_failures - - context 'when the `optimize_where_full_path_in` feature flag is turned OFF' do - before do - stub_feature_flags(optimize_where_full_path_in: false) - end - - it_behaves_like '.where_full_path_in', :aggregate_failures - end end RSpec.shared_examples 'routable resource with parent' do diff --git a/spec/models/container_registry/protection/rule_spec.rb b/spec/models/container_registry/protection/rule_spec.rb index 1706fcf76ae..848b844ed64 100644 --- a/spec/models/container_registry/protection/rule_spec.rb +++ b/spec/models/container_registry/protection/rule_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe ContainerRegistry::Protection::Rule, type: :model, feature_category: :container_registry do + using RSpec::Parameterized::TableSyntax + it_behaves_like 'having unique enum values' describe 'relationships' do @@ -51,4 +53,192 @@ RSpec.describe ContainerRegistry::Protection::Rule, type: :model, feature_catego it { is_expected.to validate_presence_of(:push_protected_up_to_access_level) } end end + + describe '.for_repository_path' do + let_it_be(:container_registry_protection_rule) do + create(:container_registry_protection_rule, repository_path_pattern: 'my-scope/my_container') + end + + let_it_be(:protection_rule_with_wildcard_start) do + create(:container_registry_protection_rule, repository_path_pattern: '*my-scope/my_container-with-wildcard-start') + end + + let_it_be(:protection_rule_with_wildcard_end) do + create(:container_registry_protection_rule, repository_path_pattern: 'my-scope/my_container-with-wildcard-end*') + end + + let_it_be(:protection_rule_with_wildcard_middle) do + create(:container_registry_protection_rule, + repository_path_pattern: 'my-scope/*my_container-with-wildcard-middle') + end + + let_it_be(:protection_rule_with_wildcard_double) do + create(:container_registry_protection_rule, + repository_path_pattern: '**my-scope/**my_container-with-wildcard-double**') + end + + let_it_be(:protection_rule_with_underscore) do + create(:container_registry_protection_rule, repository_path_pattern: 'my-scope/my_container-with_____underscore') + end + + let_it_be(:protection_rule_with_regex_chars) do + create(:container_registry_protection_rule, repository_path_pattern: 'my-scope/my_container-with-regex-chars.+') + end + + let(:repository_path) { container_registry_protection_rule.repository_path_pattern } + + subject { described_class.for_repository_path(repository_path) } + + context 'with several container registry protection rule scenarios' do + where(:repository_path, :expected_container_registry_protection_rules) do + 'my-scope/my_container' | [ref(:container_registry_protection_rule)] + 'my-scope/my2container' | [] + 'my-scope/my_container-2' | [] + + # With wildcard pattern at the start + 'my-scope/my_container-with-wildcard-start' | [ref(:protection_rule_with_wildcard_start)] + 'my-scope/my_container-with-wildcard-start-any' | [] + 'prefix-my-scope/my_container-with-wildcard-start' | [ref(:protection_rule_with_wildcard_start)] + 'prefix-my-scope/my_container-with-wildcard-start-any' | [] + + # With wildcard pattern at the end + 'my-scope/my_container-with-wildcard-end' | [ref(:protection_rule_with_wildcard_end)] + 'my-scope/my_container-with-wildcard-end:1234567890' | [ref(:protection_rule_with_wildcard_end)] + 'prefix-my-scope/my_container-with-wildcard-end' | [] + 'prefix-my-scope/my_container-with-wildcard-end:1234567890' | [] + + # With wildcard pattern in the middle + 'my-scope/my_container-with-wildcard-middle' | [ref(:protection_rule_with_wildcard_middle)] + 'my-scope/any-my_container-with-wildcard-middle' | [ref(:protection_rule_with_wildcard_middle)] + 'my-scope/any-my_container-my_container-wildcard-middle-any' | [] + + # With double wildcard pattern + 'my-scope/my_container-with-wildcard-double' | [ref(:protection_rule_with_wildcard_double)] + 'prefix-my-scope/any-my_container-with-wildcard-double-any' | [ref(:protection_rule_with_wildcard_double)] + '****my-scope/****my_container-with-wildcard-double****' | [ref(:protection_rule_with_wildcard_double)] + 'prefix-@other-scope/any-my_container-with-wildcard-double-any' | [] + + # With underscore + 'my-scope/my_container-with_____underscore' | [ref(:protection_rule_with_underscore)] + 'my-scope/my_container-with_any_underscore' | [] + + 'my-scope/my_container-with-regex-chars.+' | [ref(:protection_rule_with_regex_chars)] + 'my-scope/my_container-with-regex-chars.' | [] + 'my-scope/my_container-with-regex-chars' | [] + 'my-scope/my_container-with-regex-chars-any' | [] + + # Special cases + nil | [] + '' | [] + 'any_container' | [] + end + + with_them do + it { is_expected.to match_array(expected_container_registry_protection_rules) } + end + end + + context 'with multiple matching container registry protection rules' do + let!(:container_registry_protection_rule_second_match) do + create(:container_registry_protection_rule, repository_path_pattern: "#{repository_path}*") + end + + it { + is_expected.to contain_exactly(container_registry_protection_rule_second_match, + container_registry_protection_rule) + } + end + end + + describe '.for_push_exists?' do + subject do + project + .container_registry_protection_rules + .for_push_exists?( + access_level: access_level, + repository_path: repository_path + ) + end + + context 'when the repository path matches multiple protection rules' do + # The abbreviation `crpr` stands for container registry protection rule + let_it_be(:project_with_crpr) { create(:project) } + let_it_be(:project_without_crpr) { create(:project) } + + let_it_be(:protection_rule_for_developer) do + create(:container_registry_protection_rule, + repository_path_pattern: 'my-scope/my-container-stage*', + project: project_with_crpr, + push_protected_up_to_access_level: :developer + ) + end + + let_it_be(:protection_rule_for_maintainer) do + create(:container_registry_protection_rule, + repository_path_pattern: 'my-scope/my-container-prod*', + project: project_with_crpr, + push_protected_up_to_access_level: :maintainer + ) + end + + let_it_be(:protection_rule_for_owner) do + create(:container_registry_protection_rule, + repository_path_pattern: 'my-scope/my-container-release*', + project: project_with_crpr, + push_protected_up_to_access_level: :owner + ) + end + + let_it_be(:protection_rule_overlapping_for_developer) do + create(:container_registry_protection_rule, + repository_path_pattern: 'my-scope/my-container-*', + project: project_with_crpr, + push_protected_up_to_access_level: :developer + ) + end + + where(:project, :access_level, :repository_path, :for_push_exists) do + ref(:project_with_crpr) | Gitlab::Access::REPORTER | 'my-scope/my-container-stage-sha-1234' | true + ref(:project_with_crpr) | Gitlab::Access::DEVELOPER | 'my-scope/my-container-stage-sha-1234' | true + ref(:project_with_crpr) | Gitlab::Access::MAINTAINER | 'my-scope/my-container-stage-sha-1234' | false + ref(:project_with_crpr) | Gitlab::Access::MAINTAINER | 'my-scope/my-container-stage-sha-1234' | false + ref(:project_with_crpr) | Gitlab::Access::OWNER | 'my-scope/my-container-stage-sha-1234' | false + ref(:project_with_crpr) | Gitlab::Access::ADMIN | 'my-scope/my-container-stage-sha-1234' | false + + ref(:project_with_crpr) | Gitlab::Access::DEVELOPER | 'my-scope/my-container-prod-sha-1234' | true + ref(:project_with_crpr) | Gitlab::Access::MAINTAINER | 'my-scope/my-container-prod-sha-1234' | true + ref(:project_with_crpr) | Gitlab::Access::OWNER | 'my-scope/my-container-prod-sha-1234' | false + ref(:project_with_crpr) | Gitlab::Access::ADMIN | 'my-scope/my-container-prod-sha-1234' | false + + ref(:project_with_crpr) | Gitlab::Access::DEVELOPER | 'my-scope/my-container-release-v1' | true + ref(:project_with_crpr) | Gitlab::Access::OWNER | 'my-scope/my-container-release-v1' | true + ref(:project_with_crpr) | Gitlab::Access::ADMIN | 'my-scope/my-container-release-v1' | false + + ref(:project_with_crpr) | Gitlab::Access::DEVELOPER | 'my-scope/my-container-any-suffix' | true + ref(:project_with_crpr) | Gitlab::Access::MAINTAINER | 'my-scope/my-container-any-suffix' | false + ref(:project_with_crpr) | Gitlab::Access::OWNER | 'my-scope/my-container-any-suffix' | false + + # For non-matching repository_path + ref(:project_with_crpr) | Gitlab::Access::DEVELOPER | 'my-scope/non-matching-container' | false + + # For no access level + ref(:project_with_crpr) | Gitlab::Access::NO_ACCESS | 'my-scope/my-container-prod-sha-1234' | true + + # Edge cases + ref(:project_with_crpr) | 0 | '' | false + ref(:project_with_crpr) | nil | nil | false + ref(:project_with_crpr) | Gitlab::Access::DEVELOPER | nil | false + ref(:project_with_crpr) | nil | 'my-scope/non-matching-container' | false + + # For projects that have no container registry protection rules + ref(:project_without_crpr) | Gitlab::Access::DEVELOPER | 'my-scope/my-container-prod-sha-1234' | false + ref(:project_without_crpr) | Gitlab::Access::MAINTAINER | 'my-scope/my-container-prod-sha-1234' | false + ref(:project_without_crpr) | Gitlab::Access::OWNER | 'my-scope/my-container-prod-sha-1234' | false + end + + with_them do + it { is_expected.to eq for_push_exists } + end + end + end end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index fb32c796016..084e2dd7bd5 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -693,12 +693,16 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont it 'calls GitlabApiClient#tags and passes parameters' do allow(repository.gitlab_api_client).to receive(:tags).and_return({}) expect(repository.gitlab_api_client).to receive(:tags).with( - repository.path, page_size: page_size, before: before, last: last, sort: sort, name: name) + repository.path, page_size: page_size, before: before, last: last, sort: sort, name: name, referrers: nil) subject end context 'with a call to tags' do + let_it_be(:created_at) { 15.minutes.ago } + let_it_be(:updated_at) { 10.minutes.ago } + let_it_be(:published_at) { 5.minutes.ago } + let_it_be(:tags_response) do [ { @@ -707,8 +711,15 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont config_digest: 'sha256:66b1132a0173910b01ee69583bbf2f7f1e4462c99efbe1b9ab5bf', media_type: 'application/vnd.oci.image.manifest.v1+json', size_bytes: 1234567890, - created_at: 5.minutes.ago, - updated_at: 5.minutes.ago + created_at: created_at, + updated_at: updated_at, + published_at: published_at, + referrers: [ + { + artifactType: 'application/vnd.example+type', + digest: 'sha256:57d3be92c2f857566ecc7f9306a80021c0a7fa631e0ef5146957235aea859961' + } + ] }, { name: 'latest', @@ -716,8 +727,9 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont config_digest: nil, media_type: 'application/vnd.oci.image.manifest.v1+json', size_bytes: 1234567892, - created_at: 10.minutes.ago, - updated_at: 10.minutes.ago + created_at: created_at, + updated_at: updated_at, + published_at: published_at } ] end @@ -753,8 +765,17 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont revision: expected_revision, short_revision: expected_revision[0..8], created_at: DateTime.rfc3339(tags_response[index][:created_at].rfc3339), - updated_at: DateTime.rfc3339(tags_response[index][:updated_at].rfc3339) + updated_at: DateTime.rfc3339(tags_response[index][:updated_at].rfc3339), + published_at: DateTime.rfc3339(tags_response[index][:published_at].rfc3339) ) + + Array(tag.referrers).each_with_index do |ref, ref_index| + expect(ref.is_a?(ContainerRegistry::Referrer)).to eq(true) + expect(ref).to have_attributes( + artifact_type: tags_response[index][:referrers][ref_index][:artifactType], + digest: tags_response[index][:referrers][ref_index][:digest] + ) + end end end end @@ -1148,9 +1169,9 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont end end - describe '.find_or_create_from_path' do + describe '.find_or_create_from_path!' do let(:repository) do - described_class.find_or_create_from_path(ContainerRegistry::Path.new(path)) + described_class.find_or_create_from_path!(ContainerRegistry::Path.new(path)) end let(:repository_path) { ContainerRegistry::Path.new(path) } @@ -1239,7 +1260,7 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont Thread.new do true while wait_for_it - described_class.find_or_create_from_path(path) + described_class.find_or_create_from_path!(path) end end wait_for_it = false diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index cb2c38c15e0..d260e75871d 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -56,6 +56,8 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do let(:scope_attrs) { { project: project } } let(:usage) { :deployments } end + + it { is_expected.to include_module(EachBatch) } end describe '.stoppable' do @@ -878,7 +880,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do context 'when the SHA for the deployment does not exist in the repo' do it 'returns false' do - deployment.update!(sha: Gitlab::Git::BLANK_SHA) + deployment.update!(sha: Gitlab::Git::SHA1_BLANK_SHA) commit = project.commit expect(deployment.includes_commit?(commit.id)).to be false @@ -1537,6 +1539,18 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do expect(project.commit(deployment.ref_path)).not_to be_nil end end + + it 'does not trigger N+1 queries' do + project = create(:project, :repository) + environment = create(:environment, project: project) + create(:deployment, environment: environment, project: project) + + control = ActiveRecord::QueryRecorder.new { project.deployments.fast_destroy_all } + + create_list(:deployment, 2, environment: environment, project: project) + + expect { project.deployments.fast_destroy_all }.not_to exceed_query_limit(control) + end end describe '#update_merge_request_metrics!' do diff --git a/spec/models/design_management/design_spec.rb b/spec/models/design_management/design_spec.rb index 98e5399f737..882aaffa818 100644 --- a/spec/models/design_management/design_spec.rb +++ b/spec/models/design_management/design_spec.rb @@ -447,7 +447,7 @@ RSpec.describe DesignManagement::Design, feature_category: :design_management do let(:versions_count) { 1 } it 'builds diff refs based on the empty tree if there was only one version' do - expect(design.diff_refs.base_sha).to eq(Gitlab::Git::BLANK_SHA) + expect(design.diff_refs.base_sha).to eq(Gitlab::Git::SHA1_BLANK_SHA) expect(design.diff_refs.head_sha).to eq(design.diff_refs.head_sha) end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 1fafa64a535..26a9a364ea6 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -1603,6 +1603,27 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end + describe '#owned_by?' do + let!(:invited_group_member) { create(:group_member, :owner, :invited, group: group) } + + before do + @members = setup_group_members(group) + end + + it 'returns true for owner' do + expect(group.owned_by?(@members[:owner])).to eq(true) + end + + it 'returns false for developer' do + expect(group.owned_by?(@members[:developer])).to eq(false) + end + + it 'returns false when nil is passed' do + expect(invited_group_member.user).to eq(nil) + expect(group.owned_by?(invited_group_member.user)).to eq(false) + end + end + def setup_group_members(group) members = { owner: create(:user), @@ -1642,6 +1663,54 @@ RSpec.describe Group, feature_category: :groups_and_projects do it { expect(subject.parent).to be_kind_of(described_class) } end + describe '#has_user?' do + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:user) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:invited_group_member) { create(:group_member, :owner, :invited, group: group) } + + subject { group.has_user?(user) } + + context 'when the user is a member' do + before_all do + group.add_developer(user) + end + + it { is_expected.to be_truthy } + it { expect(group.has_user?(user2)).to be_falsey } + + it 'returns false for subgroup' do + expect(subgroup.has_user?(user)).to be_falsey + end + end + + context 'when the user is a member with minimal access' do + before_all do + group.add_member(user, GroupMember::MINIMAL_ACCESS) + end + + it { is_expected.to be_falsey } + end + + context 'when the user has requested membership' do + before_all do + create(:group_member, :developer, :access_request, user: user, source: group) + end + + it 'returns false' do + expect(subject).to be_falsey + end + end + + context 'when the user is an invited member' do + it 'returns false when nil is passed' do + expect(invited_group_member.user).to eq(nil) + expect(group.has_user?(invited_group_member.user)).to be_falsey + end + end + end + describe '#member?' do let_it_be(:group) { create(:group) } let_it_be(:user) { create(:user) } @@ -2442,9 +2511,15 @@ RSpec.describe Group, feature_category: :groups_and_projects do subject(:highest_group_member) { nested_group_2.highest_group_member(user) } context 'when the user is not a member of any group in the hierarchy' do - it 'returns nil' do - expect(highest_group_member).to be_nil + it { is_expected.to be_nil } + end + + context 'when access request to group is pending' do + before do + create(:group_member, requested_at: Time.current.utc, source: nested_group, user: user) end + + it { is_expected.to be_nil } end context 'when the user is only a member of one group in the hierarchy' do @@ -3168,6 +3243,48 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end + describe '.descendant_groups_counts' do + let_it_be(:parent) { create(:group) } + let_it_be(:group) { create(:group, parent: parent) } + let_it_be(:project) { create(:project, namespace: parent) } + + subject(:descendant_groups_counts) { described_class.id_in(parent).descendant_groups_counts } + + it 'return a hash of group id and descendant groups count without projects' do + expect(descendant_groups_counts).to eq({ parent.id => 1 }) + end + end + + describe '.projects_counts' do + let_it_be(:parent) { create(:group) } + let_it_be(:group) { create(:group, parent: parent) } + let_it_be(:project) { create(:project, namespace: parent) } + let_it_be(:archived_project) { create(:project, :archived, namespace: parent) } + + subject(:projects_counts) { described_class.id_in(parent).projects_counts } + + it 'return a hash of group id and projects count without counting archived projects' do + expect(projects_counts).to eq({ parent.id => 1 }) + end + end + + describe '.group_members_counts' do + let_it_be(:parent) { create(:group) } + let_it_be(:group) { create(:group, parent: parent) } + + before_all do + create(:group_member, group: parent) + create(:group_member, group: parent, requested_at: Time.current) + create(:group_member, group: group) + end + + subject(:group_members_counts) { described_class.id_in(parent).group_members_counts } + + it 'return a hash of group id and approved direct group members' do + expect(group_members_counts).to eq({ parent.id => 1 }) + end + end + describe '#shared_with_group_links_visible_to_user' do let_it_be(:admin) { create :admin } let_it_be(:normal_user) { create :user } diff --git a/spec/models/integrations/chat_message/push_message_spec.rb b/spec/models/integrations/chat_message/push_message_spec.rb index a9d0f801406..ba619f655ec 100644 --- a/spec/models/integrations/chat_message/push_message_spec.rb +++ b/spec/models/integrations/chat_message/push_message_spec.rb @@ -72,7 +72,7 @@ RSpec.describe Integrations::ChatMessage::PushMessage do let(:args) do { after: 'after', - before: Gitlab::Git::BLANK_SHA, + before: Gitlab::Git::SHA1_BLANK_SHA, project_name: 'project_name', ref: 'refs/tags/new_tag', user_name: 'test.user', @@ -112,7 +112,7 @@ RSpec.describe Integrations::ChatMessage::PushMessage do context 'removed tag' do let(:args) do { - after: Gitlab::Git::BLANK_SHA, + after: Gitlab::Git::SHA1_BLANK_SHA, before: 'before', project_name: 'project_name', ref: 'refs/tags/new_tag', @@ -152,7 +152,7 @@ RSpec.describe Integrations::ChatMessage::PushMessage do context 'new branch' do before do - args[:before] = Gitlab::Git::BLANK_SHA + args[:before] = Gitlab::Git::SHA1_BLANK_SHA end context 'without markdown' do @@ -185,7 +185,7 @@ RSpec.describe Integrations::ChatMessage::PushMessage do context 'removed branch' do before do - args[:after] = Gitlab::Git::BLANK_SHA + args[:after] = Gitlab::Git::SHA1_BLANK_SHA end context 'without markdown' do diff --git a/spec/models/integrations/diffblue_cover_spec.rb b/spec/models/integrations/diffblue_cover_spec.rb new file mode 100644 index 00000000000..c1a98cc2fbd --- /dev/null +++ b/spec/models/integrations/diffblue_cover_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::DiffblueCover, feature_category: :integrations do + let_it_be(:project) { build(:project) } + + subject(:integration) { build(:diffblue_cover_integration, project: project) } + + describe 'Validations' do + context 'when active' do + before do + integration.active = true + end + + it { is_expected.to validate_presence_of(:diffblue_license_key) } + it { is_expected.to validate_presence_of(:diffblue_access_token_name) } + it { is_expected.to validate_presence_of(:diffblue_access_token_secret) } + end + + context 'when inactive' do + before do + integration.active = false + end + + it { is_expected.not_to validate_presence_of(:diffblue_license_key) } + it { is_expected.not_to validate_presence_of(:diffblue_access_token_name) } + it { is_expected.not_to validate_presence_of(:diffblue_access_token_secret) } + end + end + + describe '#avatar_url' do + it 'returns the avatar image path' do + expect(integration.avatar_url).to eq(ActionController::Base.helpers.image_path( + 'illustrations/third-party-logos/integrations-logos/diffblue.svg' + )) + end + end + + describe '#ci-vars' do + let(:ci_vars) do + [ + { key: 'DIFFBLUE_LICENSE_KEY', value: '1234-ABCD-DCBA-4321', public: false, masked: true }, + { key: 'DIFFBLUE_ACCESS_TOKEN_NAME', value: 'Diffblue CI', public: false, masked: true }, + { key: 'DIFFBLUE_ACCESS_TOKEN', + value: 'glpat-00112233445566778899', public: false, masked: true } # gitleaks:allow + ] + end + + context 'when active' do + before do + integration.active = true + end + + it 'returns the required pipeline vars' do + expect(integration.ci_variables).to match_array(ci_vars) + end + end + + context 'when inactive' do + before do + integration.active = false + end + + it 'does not return the required pipeline vars' do + expect(integration.ci_variables).to be_empty + end + end + end + + describe '#diffblue_link' do + it { expect(described_class.diffblue_link).to include("https://www.diffblue.com/try-cover/gitlab/") } + end +end diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index 1537b10ba03..2294e687296 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -246,7 +246,7 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do end it 'returns nil when ref is blank' do - data[:after] = Gitlab::Git::BLANK_SHA + data[:after] = Gitlab::Git::SHA1_BLANK_SHA expect(integration.execute(data)).to be_nil end diff --git a/spec/models/issue_email_participant_spec.rb b/spec/models/issue_email_participant_spec.rb index 8ddc9a5f478..760af974275 100644 --- a/spec/models/issue_email_participant_spec.rb +++ b/spec/models/issue_email_participant_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe IssueEmailParticipant do +RSpec.describe IssueEmailParticipant, feature_category: :service_desk do describe "Associations" do it { is_expected.to belong_to(:issue) } end @@ -27,4 +27,18 @@ RSpec.describe IssueEmailParticipant do expect(subject).to be_invalid end end + + describe 'Scopes' do + describe '.with_emails' do + let!(:participant) { create(:issue_email_participant, email: 'user@example.com') } + let!(:participant1) { create(:issue_email_participant, email: 'user1@example.com') } + let!(:participant2) { create(:issue_email_participant, email: 'user2@example.com') } + + it 'returns only participant with matching emails' do + expect(described_class.with_emails([participant.email, participant1.email])).to match_array( + [participant, participant1] + ) + end + end + end end diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 4862b0b0453..2c73178ee63 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -208,20 +208,6 @@ RSpec.describe Label, feature_category: :team_planning do end describe 'scopes' do - describe '.on_board' do - let(:board) { create(:board, project: project) } - let!(:list1) { create(:list, board: board, label: development) } - let!(:list2) { create(:list, board: board, label: testing) } - - let!(:development) { create(:label, project: project, name: 'Development') } - let!(:testing) { create(:label, project: project, name: 'Testing') } - let!(:regression) { create(:label, project: project, name: 'Regression') } - - it 'returns only the board labels' do - expect(described_class.on_board(board.id)).to match_array([development, testing]) - end - end - describe '.with_lock_on_merge' do let(:label) { create(:label, project: project, name: 'Label') } let(:label_locked) { create(:label, project: project, name: 'Label locked', lock_on_merge: true) } diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index db2ae319bc9..bd74af9b7e8 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -735,6 +735,30 @@ RSpec.describe Member, feature_category: :groups_and_projects do it { is_expected.to respond_to(:user_email) } end + describe 'callbacks' do + describe '#send_invite' do + context 'with an invited group member' do + it 'sends an invite email' do + expect_next_instance_of(NotificationService) do |instance| + expect(instance).to receive(:invite_member) + end + + create(:group_member, :invited) + end + end + + context 'with an uninvited member' do + it 'does not send an invite email' do + expect_next_instance_of(NotificationService) do |instance| + expect(instance).not_to receive(:invite_member) + end + + create(:group_member) + end + end + end + end + describe '.valid_email?' do it 'is a valid email format' do expect(described_class.valid_email?('foo')).to eq(false) @@ -898,6 +922,40 @@ RSpec.describe Member, feature_category: :groups_and_projects do expect(member.invite_token).not_to be_nil expect_any_instance_of(described_class).not_to receive(:after_accept_invite) end + + context 'when after accepting invite' do + include NotificationHelpers + + let_it_be(:group) { create(:group, require_two_factor_authentication: true) } + let_it_be(:member, reload: true) { create(:group_member, :invited, source: group) } + let_it_be(:email) { member.invite_email } + let(:user) { build(:user, email: email) } + + it 'enqueues an email to user' do + member.accept_invite!(user) + + expect_enqueud_email(member.real_source_type, member.id, mail: 'member_invite_accepted_email') + end + + it 'calls updates the two factor requirement' do + expect(user).to receive(:require_two_factor_authentication_from_group).and_call_original + + member.accept_invite!(user) + + expect(user.require_two_factor_authentication_from_group).to be_truthy + end + + context 'when member source is a project' do + let_it_be(:project) { create(:project, namespace: group) } + let_it_be(:member) { create(:project_member, :invited, source: project, invite_email: email) } + + it 'calls updates the two factor requirement' do + expect(user).not_to receive(:require_two_factor_authentication_from_group) + + member.accept_invite!(user) + end + end + end end describe '#decline_invite!' do @@ -1026,6 +1084,110 @@ RSpec.describe Member, feature_category: :groups_and_projects do end end + context 'for updating organization_users' do + let_it_be(:group) { create(:group, :with_organization) } + let(:member) { create(:group_member, source: group) } + let(:update_organization_users_enabled) { true } + + before do + stub_feature_flags(update_organization_users: update_organization_users_enabled) + end + + context 'when update_organization_users is enabled' do + it 'inserts new record on member creation' do + expect { member }.to change { Organizations::OrganizationUser.count }.by(1) + record_attrs = { organization: group.organization, user: member.user, access_level: :default } + expect(Organizations::OrganizationUser.exists?(record_attrs)).to be(true) + end + + context 'when user already exists in the organization_users' do + context 'for an already existing default organization_user' do + let_it_be(:project) { create(:project, group: group, organization: group.organization) } + + before do + member + end + + it 'does not insert a new record in organization_users' do + expect do + create(:project_member, :owner, source: project, user: member.user) + end.not_to change { Organizations::OrganizationUser.count } + + expect( + Organizations::OrganizationUser.exists?( + organization: project.organization, user: member.user, access_level: :default + ) + ).to be(true) + end + + it 'does not update timestamps' do + travel_to(1.day.from_now) do + expect do + create(:project_member, :owner, source: project, user: member.user) + end.not_to change { Organizations::OrganizationUser.last.updated_at } + end + end + end + + context 'for an already existing owner organization_user' do + let_it_be(:user) { create(:user) } + let_it_be(:common_attrs) { { organization: group.organization, user: user } } + + before_all do + create(:organization_user, :owner, common_attrs) + end + + it 'does not insert a new record in organization_users nor update the access_level' do + expect do + create(:group_member, :owner, source: group, user: user) + end.not_to change { Organizations::OrganizationUser.count } + + expect( + Organizations::OrganizationUser.exists?(common_attrs.merge(access_level: :default)) + ).to be(false) + expect( + Organizations::OrganizationUser.exists?(common_attrs.merge(access_level: :owner)) + ).to be(true) + end + end + end + + context 'when updating the organization_users is not successful' do + it 'rolls back the member creation' do + allow(Organizations::OrganizationUser).to receive(:upsert).once.and_raise(ActiveRecord::StatementTimeout) + + expect { member }.to raise_error(ActiveRecord::StatementTimeout) + expect(Organizations::OrganizationUser.exists?(organization: group.organization)).to be(false) + expect(group.group_members).to be_empty + end + end + end + + shared_examples_for 'does not create an organization_user entry' do + specify do + expect { member }.not_to change { Organizations::OrganizationUser.count } + end + end + + context 'when update_organization_users is disabled' do + let(:update_organization_users_enabled) { false } + + it_behaves_like 'does not create an organization_user entry' + end + + context 'when member is an invite' do + let(:member) { create(:group_member, :invited, source: group) } + + it_behaves_like 'does not create an organization_user entry' + end + + context 'when organization does not exist' do + let(:member) { create(:group_member) } + + it_behaves_like 'does not create an organization_user entry' + end + end + context 'when after_commit :update_highest_role' do let_it_be(:user) { create(:user) } @@ -1071,6 +1233,132 @@ RSpec.describe Member, feature_category: :groups_and_projects do end end + context 'when after_update :post_update_hook' do + let_it_be(:member) { create(:group_member, :developer) } + + context 'when access_level is changed' do + it 'calls NotificationService.update_member' do + expect(NotificationService).to receive_message_chain(:new, :updated_member_access_level).with(member) + + member.update_attribute(:access_level, Member::MAINTAINER) + end + + it 'does not send an email when the access level has not changed' do + expect(NotificationService).not_to receive(:new) + + member.touch + end + end + + context 'when expiration is changed' do + it 'calls the notification service when membership expiry has changed' do + expect(NotificationService).to receive_message_chain(:new, :updated_member_expiration).with(member) + + member.update!(expires_at: 5.days.from_now) + end + end + end + + context 'when after_create :post_create_hook' do + include NotificationHelpers + + let_it_be(:source) { create(:group) } + let(:member) { create(:group_member, source: source) } + + subject(:create_member) { member } + + shared_examples_for 'invokes a notification' do + it 'enqueues an email to user' do + create_member + + expect_delivery_jobs_count(1) + expect_enqueud_email(member.real_source_type, member.id, mail: 'member_access_granted_email') + end + end + + shared_examples_for 'performs all the common hooks' do + it_behaves_like 'invokes a notification' + + it 'creates an event' do + expect { create_member }.to change { Event.count }.by(1) + end + end + + it 'calls the system hook service' do + expect_next_instance_of(SystemHooksService) do |instance| + expect(instance).to receive(:execute_hooks_for).with(an_instance_of(GroupMember), :create) + end + + create_member + end + + context 'when source is a group' do + it_behaves_like 'invokes a notification' + + it 'does not create an event' do + expect { create_member }.not_to change { Event.count } + end + end + + context 'when source is a project' do + context 'when source is a personal project' do + let_it_be(:namespace) { create(:namespace) } + + context 'when member is the owner of the namespace' do + subject(:create_member) { create(:project, namespace: namespace) } + + it 'does not enqueue an email' do + create_member + + expect_delivery_jobs_count(0) + end + + it 'does not create an event' do + expect { create_member }.not_to change { Event.count } + end + end + + context 'when member is not the namespace owner' do + let_it_be(:project) { create(:project, namespace: namespace) } + let(:member) { create(:project_member, source: project) } + + subject(:create_member) { member } + + it_behaves_like 'performs all the common hooks' + end + end + + context 'when source is not a personal project' do + let_it_be(:project) { create(:project, namespace: create(:group)) } + let(:member) { create(:project_member, source: project) } + + subject(:create_member) { member } + + it_behaves_like 'performs all the common hooks' + end + end + end + + context 'when after_create :update_two_factor_requirement' do + it 'calls update_two_factor_requirement after creation' do + user = create(:user) + + expect(user).to receive(:update_two_factor_requirement) + + create(:group_member, user: user) + end + end + + context 'when after_destroy :update_two_factor_requirement' do + it 'calls update_two_factor_requirement after deletion' do + group_member = create(:group_member) + + expect(group_member.user).to receive(:update_two_factor_requirement) + + group_member.destroy! + end + end + describe 'log_invitation_token_cleanup' do let_it_be(:project) { create :project } diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 7307e76272d..3b23f3661f8 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -33,10 +33,6 @@ RSpec.describe GroupMember, feature_category: :cell do end end - describe 'delegations' do - it { is_expected.to delegate_method(:update_two_factor_requirement).to(:user).allow_nil } - end - describe '.access_level_roles' do it 'returns Gitlab::Access.options_with_owner' do expect(described_class.access_level_roles).to eq(Gitlab::Access.options_with_owner) @@ -67,22 +63,6 @@ RSpec.describe GroupMember, feature_category: :cell do it { is_expected.to eq 'Group' } end - describe '#update_two_factor_requirement' do - it 'is called after creation and deletion' do - user = create :user - group = create :group - group_member = build :group_member, user: user, group: group - - expect(user).to receive(:update_two_factor_requirement) - - group_member.save! - - expect(user).to receive(:update_two_factor_requirement) - - group_member.destroy! - end - end - describe '#destroy' do context 'for an orphaned member' do let!(:orphaned_group_member) do @@ -95,21 +75,6 @@ RSpec.describe GroupMember, feature_category: :cell do end end - describe '#after_accept_invite' do - it 'calls #update_two_factor_requirement' do - email = 'foo@email.com' - user = build(:user, email: email) - group = create(:group, require_two_factor_authentication: true) - group_member = create(:group_member, group: group, invite_token: '1234', invite_email: email) - - expect(user).to receive(:require_two_factor_authentication_from_group).and_call_original - - group_member.accept_invite!(user) - - expect(user.require_two_factor_authentication_from_group).to be_truthy - end - end - describe '#last_owner_of_the_group?' do let_it_be(:parent_group) { create(:group) } let_it_be(:group) { create(:group, parent: parent_group) } @@ -202,18 +167,6 @@ RSpec.describe GroupMember, feature_category: :cell do end end - context 'when group member expiration date is updated' do - let_it_be(:group_member) { create(:group_member) } - - it 'emails the user that their group membership expiry has changed' do - expect_next_instance_of(NotificationService) do |notification| - allow(notification).to receive(:updated_group_member_expiration).with(group_member) - end - - group_member.update!(expires_at: 5.days.from_now) - end - end - describe 'refresh_member_authorized_projects' do context 'when importing' do it 'does not refresh' do @@ -288,18 +241,4 @@ RSpec.describe GroupMember, feature_category: :cell do it_behaves_like 'calls AuthorizedProjectsWorker inline to recalculate authorizations' end end - - context 'group member welcome email', :sidekiq_inline, :saas do - let_it_be(:group) { create(:group) } - - let(:user) { create(:user) } - - it 'schedules plain welcome to the group email' do - expect_next_instance_of(NotificationService) do |notification| - expect(notification).to receive(:new_group_member) - end - - group.add_developer(user) - end - end end diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb index e9e4956dc41..8d1d503b323 100644 --- a/spec/models/merge_request/metrics_spec.rb +++ b/spec/models/merge_request/metrics_spec.rb @@ -93,12 +93,4 @@ RSpec.describe MergeRequest::Metrics do end end end - - it_behaves_like 'database events tracking', feature_category: :service_ping do - let(:merge_request) { create(:merge_request) } - - let(:record) { merge_request.metrics } - let(:namespace) { nil } - let(:update_params) { { pipeline_id: 1, updated_at: Date.tomorrow } } - end end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 2e68cd9e74a..e31a4399646 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -53,6 +53,28 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') } it { expect(subject.patch_id_sha).to eq('1e05e04d4c2a6414d9d4ab38208511a3bbe715f2') } + it 'calls GraphqlTriggers.merge_request_diff_generated' do + merge_request = create(:merge_request, :skip_diff_creation) + + expect(GraphqlTriggers).to receive(:merge_request_diff_generated).with(merge_request) + + merge_request.create_merge_request_diff + end + + context 'when merge_request_diff_generated_subscription flag is disabled' do + before do + stub_feature_flags(merge_request_diff_generated_subscription: false) + end + + it 'does not call GraphqlTriggers.merge_request_diff_generated' do + merge_request = create(:merge_request, :skip_diff_creation) + + expect(GraphqlTriggers).not_to receive(:merge_request_diff_generated) + + merge_request.create_merge_request_diff + end + end + context 'when diff_type is merge_head' do let_it_be(:merge_request) { create(:merge_request) } @@ -488,6 +510,28 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end end + shared_examples_for 'perform generated files check' do + context 'when collapse_generated option is given' do + let(:diff_options) do + super().merge(collapse_generated: true) + end + + it 'checks generated files' do + diffs = diff_with_commits.diffs_in_batch(1, 10, diff_options: diff_options) + + expect(diffs.diff_files.first.generated?).to be false + end + end + + context 'when collapse_generated option is not given' do + it 'does not check generated files' do + diffs = diff_with_commits.diffs_in_batch(1, 10, diff_options: diff_options) + + expect(diffs.diff_files.first.generated?).to be nil + end + end + end + context 'when no persisted files available' do before do diff_with_commits.clean! @@ -501,6 +545,7 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end it_behaves_like 'fetching full diffs' + it_behaves_like 'perform generated files check' end end @@ -540,6 +585,8 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do { ignore_whitespace_change: true } end + it_behaves_like 'perform generated files check' + it 'returns pagination data from MergeRequestDiffBatch' do diffs = diff_with_commits.diffs_in_batch(1, 10, diff_options: diff_options) file_count = diff_with_commits.merge_request_diff_files.count @@ -561,11 +608,35 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end describe '#paginated_diffs' do + shared_examples 'diffs with generated files check' do + context 'when collapse_generated_diff_files FF is enabled' do + it 'checks generated files' do + diffs = diff_with_commits.paginated_diffs(1, 10) + + expect(diffs.diff_files.first.generated?).not_to be_nil + end + end + + context 'when collapse_generated_diff_files FF is disabled' do + before do + stub_feature_flags(collapse_generated_diff_files: false) + end + + it 'does not check generated files' do + diffs = diff_with_commits.paginated_diffs(1, 10) + + expect(diffs.diff_files.first.generated?).to be_nil + end + end + end + context 'when no persisted files available' do before do diff_with_commits.clean! end + it_behaves_like 'diffs with generated files check' + it 'returns a Gitlab::Diff::FileCollection::Compare' do diffs = diff_with_commits.paginated_diffs(1, 10) @@ -575,6 +646,8 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end context 'when persisted files available' do + it_behaves_like 'diffs with generated files check' + it 'returns paginated diffs' do diffs = diff_with_commits.paginated_diffs(1, 10) @@ -911,11 +984,19 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end context 'handling generated files' do - let(:project) { create(:project, :repository) } + let(:project) do + create(:project, :custom_repo, files: { + '.gitattributes' => '*.txt gitlab-generated' + }) + end + + let(:generated_file_name_manual) { 'generated.txt' } + let(:generated_file_name_auto) { 'package-lock.json' } + let(:regular_file_name) { 'regular.rb' } + let(:target_branch) { project.default_branch } let(:source_branch) { 'test-generated-diff-file' } - let(:generated_file_name) { 'generated.txt' } - let(:regular_file_name) { 'regular.rb' } + let(:merge_request) do create( :merge_request, @@ -931,20 +1012,34 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end before do - project.repository.update_file( + project.repository.create_branch(source_branch, target_branch) + + project.repository.create_file( project.creator, - '.gitattributes', - '*.txt gitlab-generated', - message: 'Update', - branch_name: target_branch) + generated_file_name_manual, + 'updated generated content', + message: 'Update generated file', + branch_name: source_branch) - create_file_in_repo(project, target_branch, source_branch, generated_file_name, "generated text\n") - create_file_in_repo(project, source_branch, source_branch, regular_file_name, "something else\n") + project.repository.create_file( + project.creator, + generated_file_name_auto, + 'updated generated content', + message: 'Update generated file', + branch_name: source_branch) + + project.repository.create_file( + project.creator, + regular_file_name, + 'updated regular content', + message: "Update regular file", + branch_name: source_branch) end context 'with collapse_generated_diff_files feature flag' do it 'sets generated field correctly' do - expect(diff_files.find_by(new_path: generated_file_name)).to be_generated + expect(diff_files.find_by(new_path: generated_file_name_manual)).to be_generated + expect(diff_files.find_by(new_path: generated_file_name_auto)).to be_generated expect(diff_files.find_by(new_path: regular_file_name)).not_to be_generated end end @@ -955,7 +1050,8 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end it 'sets generated field correctly' do - expect(diff_files.find_by(new_path: generated_file_name)).not_to be_generated + expect(diff_files.find_by(new_path: generated_file_name_auto)).not_to be_generated + expect(diff_files.find_by(new_path: generated_file_name_manual)).not_to be_generated expect(diff_files.find_by(new_path: regular_file_name)).not_to be_generated end end @@ -1206,7 +1302,7 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do it 'returns false if passed commits do not exist' do expect(subject.includes_any_commits?([])).to eq(false) - expect(subject.includes_any_commits?([Gitlab::Git::BLANK_SHA])).to eq(false) + expect(subject.includes_any_commits?([Gitlab::Git::SHA1_BLANK_SHA])).to eq(false) end it 'returns true if passed commits exists' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 737884425f7..797ab5be235 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -5044,10 +5044,36 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end describe '#should_be_rebased?' do - it 'returns false for the same source and target branches' do - merge_request = build_stubbed(:merge_request, source_project: project, target_project: project) + let(:merge_request) { build_stubbed(:merge_request) } + + subject { merge_request.should_be_rebased? } + + context 'when the same source and target branches' do + let(:merge_request) { build_stubbed(:merge_request, source_project: project, target_project: project) } + + it { is_expected.to be_falsey } + end + + context 'when the project is using ff merge method' do + before do + allow(merge_request.target_project).to receive(:ff_merge_must_be_possible?).and_return(true) + end + + context 'when the mr needs to be rebased to merge' do + before do + allow(merge_request).to receive(:ff_merge_possible?).and_return(false) + end + + it { is_expected.to be_truthy } + end - expect(merge_request.should_be_rebased?).to be_falsey + context 'when the MR can be merged without rebase' do + before do + allow(merge_request).to receive(:ff_merge_possible?).and_return(true) + end + + it { is_expected.to be_falsey } + end end end @@ -6128,6 +6154,34 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end + describe '#allows_multiple_assignees?' do + let(:merge_request) { build_stubbed(:merge_request) } + + subject(:allows_multiple_assignees?) { merge_request.allows_multiple_assignees? } + + before do + allow(merge_request.project) + .to receive(:allows_multiple_merge_request_assignees?) + .and_return(false) + end + + it { is_expected.to eq(false) } + end + + describe '#allows_multiple_reviewers?' do + let(:merge_request) { build_stubbed(:merge_request) } + + subject(:allows_multiple_reviewers?) { merge_request.allows_multiple_reviewers? } + + before do + allow(merge_request.project) + .to receive(:allows_multiple_merge_request_reviewers?) + .and_return(false) + end + + it { is_expected.to eq(false) } + end + describe '#previous_diff' do let(:merge_request) { create(:merge_request, :skip_diff_creation) } diff --git a/spec/models/ml/candidate_metric_spec.rb b/spec/models/ml/candidate_metric_spec.rb index 9f9a6e8e3ba..9ceaa83a6fa 100644 --- a/spec/models/ml/candidate_metric_spec.rb +++ b/spec/models/ml/candidate_metric_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ml::CandidateMetric do +RSpec.describe Ml::CandidateMetric, feature_category: :mlops do describe 'associations' do it { is_expected.to belong_to(:candidate) } end diff --git a/spec/models/ml/candidate_param_spec.rb b/spec/models/ml/candidate_param_spec.rb index ff38e471219..89232b10855 100644 --- a/spec/models/ml/candidate_param_spec.rb +++ b/spec/models/ml/candidate_param_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ml::CandidateParam do +RSpec.describe Ml::CandidateParam, feature_category: :mlops do describe 'associations' do it { is_expected.to belong_to(:candidate) } end diff --git a/spec/models/ml/experiment_spec.rb b/spec/models/ml/experiment_spec.rb index 36bdb611833..1864c04d2fd 100644 --- a/spec/models/ml/experiment_spec.rb +++ b/spec/models/ml/experiment_spec.rb @@ -37,6 +37,20 @@ RSpec.describe Ml::Experiment, feature_category: :mlops do end end + describe '.by_project' do + subject { described_class.by_project(exp.project) } + + it { is_expected.to match_array([exp, exp2]) } + end + + describe '.including_project' do + subject { described_class.including_project } + + it 'loads latest version' do + expect(subject.first.association_cached?(:project)).to be(true) + end + end + describe '#by_project_id_and_iid' do subject { described_class.by_project_id_and_iid(exp.project_id, iid) } diff --git a/spec/models/ml/model_metadata_spec.rb b/spec/models/ml/model_metadata_spec.rb index f06c7a2ce50..0afc7bb7a2e 100644 --- a/spec/models/ml/model_metadata_spec.rb +++ b/spec/models/ml/model_metadata_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' RSpec.describe Ml::ModelMetadata, feature_category: :mlops do describe 'associations' do it { is_expected.to belong_to(:model).required } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to validate_length_of(:value).is_at_most(5000) } end describe 'validations' do diff --git a/spec/models/ml/model_version_metadata_spec.rb b/spec/models/ml/model_version_metadata_spec.rb new file mode 100644 index 00000000000..7c8ffb9b0d7 --- /dev/null +++ b/spec/models/ml/model_version_metadata_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ml::ModelVersionMetadata, feature_category: :mlops do + describe 'associations' do + it { is_expected.to belong_to(:model_version).required } + it { is_expected.to belong_to(:project).required } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to validate_length_of(:value).is_at_most(5000) } + end + + describe 'validations' do + let_it_be(:metadata) { create(:ml_model_version_metadata, name: 'some_metadata') } + let_it_be(:model_version) { metadata.model_version } + + it 'is unique within the model version' do + expect do + model_version.metadata.create!(name: 'some_metadata', value: 'blah') + end.to raise_error.with_message(/Name 'some_metadata' already taken/) + end + + it 'a model version is required' do + expect do + described_class.create!(name: 'some_metadata', value: 'blah') + end.to raise_error.with_message(/Model version must exist/) + end + end +end diff --git a/spec/models/ml/model_version_spec.rb b/spec/models/ml/model_version_spec.rb index 95d4a545f52..9db9f7e34ab 100644 --- a/spec/models/ml/model_version_spec.rb +++ b/spec/models/ml/model_version_spec.rb @@ -19,6 +19,7 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do it { is_expected.to belong_to(:model) } it { is_expected.to belong_to(:package).class_name('Packages::MlModel::Package') } it { is_expected.to have_one(:candidate).class_name('Ml::Candidate') } + it { is_expected.to have_many(:metadata) } end describe 'validation' do @@ -99,6 +100,34 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do end end + describe '#add_metadata' do + it 'accepts an array of metadata and persists it to the model version' do + input = [ + { project_id: base_project.id, key: 'tag1', value: 'value1' }, + { project_id: base_project.id, key: 'tag2', value: 'value2' } + ] + + expect { model_version1.add_metadata(input) }.to change { model_version1.metadata.count }.by(2) + end + + it 'raises an error when duplicate key names are supplied' do + input = [ + { project_id: base_project.id, key: 'tag1', value: 'value1' }, + { project_id: base_project.id, key: 'tag1', value: 'value2' } + ] + + expect { model_version1.add_metadata(input) }.to raise_error(ActiveRecord::RecordInvalid) + end + + it 'raises an error when validation fails' do + input = [ + { project_id: base_project.id, key: nil, value: 'value1' } + ] + + expect { model_version1.add_metadata(input) }.to raise_error(ActiveRecord::RecordInvalid) + end + end + describe '#find_or_create!' do let_it_be(:existing_model_version) { create(:ml_model_versions, model: model1, version: '1.0.0') } diff --git a/spec/models/namespace/package_setting_spec.rb b/spec/models/namespace/package_setting_spec.rb index f06490d7999..e326e8cace8 100644 --- a/spec/models/namespace/package_setting_spec.rb +++ b/spec/models/namespace/package_setting_spec.rb @@ -12,13 +12,21 @@ RSpec.describe Namespace::PackageSetting, feature_category: :package_registry do describe '#maven_duplicates_allowed' do it { is_expected.to validate_inclusion_of(:maven_duplicates_allowed).in_array([true, false]) } - it { is_expected.to validate_inclusion_of(:generic_duplicates_allowed).in_array([true, false]) } - it { is_expected.to validate_inclusion_of(:nuget_duplicates_allowed).in_array([true, false]) } + it { is_expected.to validate_length_of(:maven_duplicate_exception_regex).is_at_most(255) } end it { is_expected.to allow_value(true, false).for(:nuget_symbol_server_enabled) } it { is_expected.not_to allow_value(nil).for(:nuget_symbol_server_enabled) } + it { is_expected.to validate_inclusion_of(:generic_duplicates_allowed).in_array([true, false]) } + it { is_expected.to validate_length_of(:generic_duplicate_exception_regex).is_at_most(255) } + it { is_expected.to validate_inclusion_of(:nuget_duplicates_allowed).in_array([true, false]) } + it { is_expected.to validate_length_of(:nuget_duplicate_exception_regex).is_at_most(255) } + + it { is_expected.to allow_value(true, false).for(:terraform_module_duplicates_allowed) } + it { is_expected.not_to allow_value(nil).for(:terraform_module_duplicates_allowed) } + it { is_expected.to validate_length_of(:terraform_module_duplicate_exception_regex).is_at_most(255) } + describe 'regex values' do let_it_be(:package_settings) { create(:namespace_package_setting) } @@ -39,6 +47,50 @@ RSpec.describe Namespace::PackageSetting, feature_category: :package_registry do end end + describe 'scopes' do + describe '.namespace_id_in' do + let_it_be(:package_settings) { create(:namespace_package_setting) } + let_it_be(:other_package_settings) { create(:namespace_package_setting) } + + subject { described_class.namespace_id_in([package_settings.namespace_id]) } + + it { is_expected.to eq([package_settings]) } + end + + describe '.with_terraform_module_duplicates_allowed_or_exception_regex' do + let_it_be(:package_settings) { create(:namespace_package_setting) } + + subject { described_class.with_terraform_module_duplicates_allowed_or_exception_regex } + + context 'when terraform_module_duplicates_allowed is true' do + before do + package_settings.update_column(:terraform_module_duplicates_allowed, true) + end + + it { is_expected.to eq([package_settings]) } + end + + context 'when terraform_module_duplicate_exception_regex is present' do + before do + package_settings.update_column(:terraform_module_duplicate_exception_regex, 'foo') + end + + it { is_expected.to eq([package_settings]) } + end + + context 'when terraform_module_duplicates_allowed is false and terraform_module_duplicate_exception_regex is empty' do + before do + package_settings.update_columns( + terraform_module_duplicates_allowed: false, + terraform_module_duplicate_exception_regex: '' + ) + end + + it { is_expected.to be_empty } + end + end + end + describe '#duplicates_allowed?' do using RSpec::Parameterized::TableSyntax @@ -46,9 +98,14 @@ RSpec.describe Namespace::PackageSetting, feature_category: :package_registry do context 'package types with package_settings' do # As more package types gain settings they will be added to this list - %i[maven_package generic_package nuget_package].each do |format| - context "with package_type:#{format}" do - let_it_be(:package) { create(format, name: 'foo', version: '1.0.0-beta') } + [ + { format: :maven_package, package_name: 'foo' }, + { format: :generic_package, package_name: 'foo' }, + { format: :nuget_package, package_name: 'foo' }, + { format: :terraform_module_package, package_name: 'foo/bar' } + ].each do |type| + context "with package_type: #{type[:format]}" do + let_it_be(:package) { create(type[:format], name: type[:package_name], version: '1.0.0-beta') } let_it_be(:package_type) { package.package_type } let_it_be(:package_setting) { package.project.namespace.package_settings } @@ -61,7 +118,7 @@ RSpec.describe Namespace::PackageSetting, feature_category: :package_registry do end with_them do - context "for #{format}" do + context "for #{type[:format]}" do before do package_setting.update!( "#{package_type}_duplicates_allowed" => duplicates_allowed, diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 0e6513764f5..67b8931f0c5 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -39,6 +39,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do it { is_expected.to have_many(:namespace_commit_emails).class_name('Users::NamespaceCommitEmail') } it { is_expected.to have_many(:cycle_analytics_stages) } it { is_expected.to have_many(:value_streams) } + it { is_expected.to have_many(:non_archived_projects).class_name('Project') } it do is_expected.to have_one(:ci_cd_settings).class_name('NamespaceCiCdSetting').inverse_of(:namespace).autosave(true) @@ -896,12 +897,14 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do it 'does not cause N+1 query in fetching registries' do stub_container_registry_tags(repository: :any, tags: []) - control_count = ActiveRecord::QueryRecorder.new { namespace.any_project_has_container_registry_tags? }.count + control = ActiveRecord::QueryRecorder.new { namespace.any_project_has_container_registry_tags? } other_repositories = create_list(:container_repository, 2) create(:project, namespace: namespace, container_repositories: other_repositories) - expect { namespace.first_project_with_container_registry_tags }.not_to exceed_query_limit(control_count + 1) + expect do + namespace.first_project_with_container_registry_tags + end.not_to exceed_query_limit(control).with_threshold(1) end end @@ -1130,6 +1133,28 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end end + describe '.gfm_autocomplete_search' do + let_it_be(:parent_group) { create(:group, path: 'parent', name: 'Parent') } + let_it_be(:group_1) { create(:group, parent: parent_group, path: 'somepath', name: 'Your Group') } + let_it_be(:group_2) { create(:group, path: 'noparent', name: 'My Group') } + + it 'returns partial matches on full path' do + expect(described_class.gfm_autocomplete_search('parent/som')).to eq([group_1]) + end + + it 'returns matches on full name across multiple words' do + expect(described_class.gfm_autocomplete_search('yourgr')).to eq([group_1]) + end + + it 'prioritizes sorting of matches that start with the query' do + expect(described_class.gfm_autocomplete_search('pare')).to eq([parent_group, group_1, group_2]) + end + + it 'falls back to sorting by full path' do + expect(described_class.gfm_autocomplete_search('group')).to eq([group_2, group_1]) + end + end + describe '.with_statistics' do let_it_be(:namespace) { create(:namespace) } diff --git a/spec/models/namespaces/descendants_spec.rb b/spec/models/namespaces/descendants_spec.rb new file mode 100644 index 00000000000..6c153c3307b --- /dev/null +++ b/spec/models/namespaces/descendants_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespaces::Descendants, feature_category: :database do + describe 'associations' do + it { is_expected.to belong_to(:namespace) } + end + + describe 'validations' do + subject(:namespace_descendants) { create(:namespace_descendants) } + + it { is_expected.to validate_uniqueness_of(:namespace_id) } + end + + describe 'factory' do + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } + + let_it_be(:project1) { create(:project, group: subgroup) } + let_it_be(:project2) { create(:project, group: group) } + + it 'up to date descendant record for a group' do + descendants = create(:namespace_descendants, namespace: group) + + expect(descendants).to have_attributes( + self_and_descendant_group_ids: [group.id, subgroup.id], + all_project_ids: [project1.id, project2.id], + traversal_ids: [group.id] + ) + end + + it 'creates up-to-date descendant record for a subgroup' do + descendants = create(:namespace_descendants, namespace: subgroup) + + expect(descendants).to have_attributes( + self_and_descendant_group_ids: [subgroup.id], + all_project_ids: [project1.id], + traversal_ids: [group.id, subgroup.id] + ) + end + end + + describe '.expire_for' do + it 'sets the outdated_at column for the given namespace ids' do + freeze_time do + expire_time = Time.current + + group1 = create(:group).tap do |g| + create(:namespace_descendants, namespace: g).reload.update!(outdated_at: nil) + end + group2 = create(:group, parent: group1).tap { |g| create(:namespace_descendants, namespace: g) } + group3 = create(:group, parent: group1) + + group4 = create(:group).tap do |g| + create(:namespace_descendants, namespace: g).reload.update!(outdated_at: nil) + end + + described_class.expire_for([group1.id, group2.id, group3.id]) + + expect(group1.namespace_descendants.outdated_at).to eq(expire_time) + expect(group2.namespace_descendants.outdated_at).to eq(expire_time) + expect(group3.namespace_descendants).to be_nil + expect(group4.namespace_descendants.outdated_at).to be_nil + end + end + end +end diff --git a/spec/models/namespaces/traversal/cached_spec.rb b/spec/models/namespaces/traversal/cached_spec.rb new file mode 100644 index 00000000000..8263e28bb98 --- /dev/null +++ b/spec/models/namespaces/traversal/cached_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespaces::Traversal::Cached, feature_category: :database do + let_it_be_with_refind(:old_parent) { create(:group) } + let_it_be_with_refind(:new_parent) { create(:group) } + let_it_be_with_refind(:group) { create(:group, parent: old_parent) } + let_it_be_with_refind(:subgroup) { create(:group, parent: group) } + + context 'when the namespace_descendants_cache_expiration feature flag is off' do + let!(:cache) { create(:namespace_descendants, namespace: group) } + + before do + stub_feature_flags(namespace_descendants_cache_expiration: false) + end + + it 'does not invalidate the cache' do + expect { group.update!(parent: new_parent) }.not_to change { cache.reload.outdated_at } + end + + context 'when the group is deleted' do + it 'invalidates the cache' do + expect { group.destroy! }.not_to change { cache.reload.outdated_at } + end + end + end + + context 'when no cached records are present' do + it 'does nothing' do + group.parent = new_parent + + expect { group.save! }.not_to change { Namespaces::Descendants.all.to_a } + end + end + + context 'when the namespace record is UserNamespace' do + it 'does nothing' do + # we won't use the optimization for UserNamespace + namespace = create(:user_namespace) + cache = create(:namespace_descendants, namespace: namespace) + + expect { namespace.destroy! }.not_to change { cache.reload.outdated_at } + end + end + + context 'when cached record is present' do + let!(:cache) { create(:namespace_descendants, namespace: group) } + + it 'invalidates the cache' do + expect { group.update!(parent: new_parent) }.to change { cache.reload.outdated_at }.from(nil) + end + + it 'does not invalidate the cache of subgroups' do + subgroup_cache = create(:namespace_descendants, namespace: subgroup) + + expect { group.update!(parent: new_parent) }.not_to change { subgroup_cache.reload.outdated_at } + end + + context 'when a new subgroup is added' do + it 'invalidates the cache' do + expect { create(:group, parent: group) }.to change { cache.reload.outdated_at } + end + end + + context 'when a new project is added' do + it 'invalidates the cache' do + expect { create(:project, group: group) }.to change { cache.reload.outdated_at } + end + end + end + + context 'when parent group has cached record' do + it 'invalidates the parent cache' do + old_parent_cache = create(:namespace_descendants, namespace: old_parent) + new_parent_cache = create(:namespace_descendants, namespace: new_parent) + + group.update!(parent: new_parent) + + expect(old_parent_cache.reload.outdated_at).not_to be_nil + expect(new_parent_cache.reload.outdated_at).not_to be_nil + end + end + + context 'when group is destroyed' do + it 'invalidates the cache' do + cache = create(:namespace_descendants, namespace: group) + + expect { group.destroy! }.to change { cache.reload.outdated_at }.from(nil) + end + + context 'when parent group has cached record' do + it 'invalidates the parent cache' do + old_parent_cache = create(:namespace_descendants, namespace: old_parent) + new_parent_cache = create(:namespace_descendants, namespace: new_parent) + + group.destroy! + + expect(old_parent_cache.reload.outdated_at).not_to be_nil + expect(new_parent_cache.reload.outdated_at).to be_nil # no change + end + end + end +end diff --git a/spec/models/note_diff_file_spec.rb b/spec/models/note_diff_file_spec.rb index 1ece1dfea59..5d9381b5886 100644 --- a/spec/models/note_diff_file_spec.rb +++ b/spec/models/note_diff_file_spec.rb @@ -32,7 +32,7 @@ RSpec.describe NoteDiffFile do end it 'excludes note diff files with the wrong sha' do - found = described_class.referencing_sha(Gitlab::Git::BLANK_SHA, project_id: project.id) + found = described_class.referencing_sha(Gitlab::Git::SHA1_BLANK_SHA, project_id: project.id) expect(found).to be_empty end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 5aa3ac3a2ea..59795059642 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -448,13 +448,13 @@ RSpec.describe Note, feature_category: :team_planning do # Project authorization checks are cached, establish a baseline retrieve_participants - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do retrieve_participants end create(:note_on_commit, project: note.project, note: 'another note', noteable_id: commit.id) - expect { retrieve_participants }.not_to exceed_query_limit(control_count) + expect { retrieve_participants }.not_to exceed_query_limit(control) end end diff --git a/spec/models/onboarding/completion_spec.rb b/spec/models/onboarding/completion_spec.rb index dd7648f7799..b9c9b994736 100644 --- a/spec/models/onboarding/completion_spec.rb +++ b/spec/models/onboarding/completion_spec.rb @@ -42,43 +42,19 @@ RSpec.describe Onboarding::Completion, feature_category: :onboarding do describe '#completed?' do subject(:completed?) { described_class.new(project).completed?(column) } - context 'when code_added' do - let(:column) { :code_added } + let(:column) { :code_added_at } + let(:completed_actions) { { code_added_at: code_added_at_timestamp } } - context 'when commit_count > 1' do - let(:project) { build(:project, :stubbed_commit_count, namespace: namespace) } + context 'when the action has been completed' do + let(:code_added_at_timestamp) { Time.current } - it { is_expected.to eq(true) } - end - - context 'when branch_count > 1' do - let(:project) { build(:project, :stubbed_branch_count, namespace: namespace) } - - it { is_expected.to eq(true) } - end - - context 'when empty repository' do - let(:project) { build(:project, namespace: namespace) } - - it { is_expected.to eq(false) } - end + it { is_expected.to eq(true) } end - context 'when secure_dast_run' do - let(:column) { :secure_dast_run_at } - let(:completed_actions) { { secure_dast_run_at: secure_dast_run_at } } - - context 'when is completed' do - let(:secure_dast_run_at) { Time.current } - - it { is_expected.to eq(true) } - end - - context 'when is not completed' do - let(:secure_dast_run_at) { nil } + context 'when the action has not been completed' do + let(:code_added_at_timestamp) { nil } - it { is_expected.to eq(false) } - end + it { is_expected.to eq(false) } end end end diff --git a/spec/models/organizations/organization_detail_spec.rb b/spec/models/organizations/organization_detail_spec.rb index 3f44a9cc637..dd49274e7dd 100644 --- a/spec/models/organizations/organization_detail_spec.rb +++ b/spec/models/organizations/organization_detail_spec.rb @@ -16,6 +16,15 @@ RSpec.describe Organizations::OrganizationDetail, type: :model, feature_category let(:model) { create(:organization_detail) } end + describe '#description_html' do + let_it_be(:model) { create(:organization_detail, description: '### Foo **Bar**') } + let(:expected_description) { ' Foo <strong>Bar</strong> ' } + + subject { model.description_html } + + it { is_expected.to eq_no_sourcepos(expected_description) } + end + context 'with uploads' do it_behaves_like 'model with uploads', false do let(:model_object) { create(:organization_detail) } diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb index 756024b6437..7a3c743eddd 100644 --- a/spec/models/organizations/organization_spec.rb +++ b/spec/models/organizations/organization_spec.rb @@ -59,8 +59,10 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel describe 'delegations' do it { is_expected.to delegate_method(:description).to(:organization_detail) } + it { is_expected.to delegate_method(:description_html).to(:organization_detail) } it { is_expected.to delegate_method(:avatar).to(:organization_detail) } it { is_expected.to delegate_method(:avatar_url).to(:organization_detail) } + it { is_expected.to delegate_method(:remove_avatar!).to(:organization_detail) } end describe 'nested attributes' do @@ -202,6 +204,32 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel end end + describe '#owner?' do + let_it_be(:user) { create(:user) } + + subject { organization.owner?(user) } + + context 'when user is an owner' do + before do + create(:organization_user, :owner, organization: organization, user: user) + end + + it { is_expected.to eq true } + end + + context 'when user is not an owner' do + before do + create(:organization_user, organization: organization, user: user) + end + + it { is_expected.to eq false } + end + + context 'when user is not an organization user' do + it { is_expected.to eq false } + end + end + describe '#web_url' do it 'returns web url from `Gitlab::UrlBuilder`' do web_url = 'http://127.0.0.1:3000/-/organizations/default' diff --git a/spec/models/organizations/organization_user_spec.rb b/spec/models/organizations/organization_user_spec.rb index 392ffa1b5be..c3416c93ec9 100644 --- a/spec/models/organizations/organization_user_spec.rb +++ b/spec/models/organizations/organization_user_spec.rb @@ -7,4 +7,41 @@ RSpec.describe Organizations::OrganizationUser, type: :model, feature_category: it { is_expected.to belong_to(:organization).inverse_of(:organization_users).required } it { is_expected.to belong_to(:user).inverse_of(:organization_users).required } end + + describe 'validations' do + subject { build(:organization_user) } + + it { is_expected.to define_enum_for(:access_level).with_values(described_class.access_levels) } + it { is_expected.to validate_presence_of(:access_level) } + it { is_expected.to validate_uniqueness_of(:user).scoped_to(:organization_id) } + + it 'does not allow invalid enum value' do + expect { build(:organization_user, access_level: '_invalid_') }.to raise_error(ArgumentError) + end + end + + context 'with loose foreign key on organization_users.organization_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let_it_be(:parent) { create(:organization) } + let_it_be(:model) { create(:organization_user, organization: parent) } + end + end + + context 'with loose foreign key on organization_users.user_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let_it_be(:parent) { create(:user) } + let_it_be(:model) { create(:organization_user, user: parent) } + end + end + + describe '.owners' do + it 'returns the owners of the organization' do + organization_user = create(:organization_user, :owner) + create(:organization_user) + + expect(described_class.owners).to match([organization_user]) + end + end + + it_behaves_like 'having unique enum values' end diff --git a/spec/models/packages/protection/rule_spec.rb b/spec/models/packages/protection/rule_spec.rb index 3f0aefa945a..03d0440f0d9 100644 --- a/spec/models/packages/protection/rule_spec.rb +++ b/spec/models/packages/protection/rule_spec.rb @@ -35,30 +35,32 @@ RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :pack it { is_expected.to validate_uniqueness_of(:package_name_pattern).scoped_to(:project_id, :package_type) } it { is_expected.to validate_length_of(:package_name_pattern).is_at_most(255) } - [ - '@my-scope/my-package', - '@my-scope/*my-package-with-wildcard-inbetween', - '@my-scope/*my-package-with-wildcard-start', - '@my-scope/my-*package-*with-wildcard-multiple-*', - '@my-scope/my-package-with_____underscore', - '@my-scope/my-package-with-regex-characters.+', - '@my-scope/my-package-with-wildcard-end*' - ].each do |package_name_pattern| - it { is_expected.to allow_value(package_name_pattern).for(:package_name_pattern) } + where(:package_name_pattern, :allowed) do + '@my-scope/my-package' | true + '@my-scope/*my-package-with-wildcard-inbetween' | true + '@my-scope/*my-package-with-wildcard-start' | true + '@my-scope/my-*package-*with-wildcard-multiple-*' | true + '@my-scope/my-package-with_____underscore' | true + '@my-scope/my-package-with-regex-characters.+' | true + '@my-scope/my-package-with-wildcard-end*' | true + + '@my-scope/my-package-with-percent-sign-%' | false + '*@my-scope/my-package-with-wildcard-start' | false + '@my-scope/my-package-with-backslash-\*' | false end - [ - '@my-scope/my-package-with-percent-sign-%', - '*@my-scope/my-package-with-wildcard-start', - '@my-scope/my-package-with-backslash-\*' - ].each do |package_name_pattern| - it { - is_expected.not_to( - allow_value(package_name_pattern) - .for(:package_name_pattern) - .with_message(_('should be a valid NPM package name with optional wildcard characters.')) - ) - } + with_them do + if params[:allowed] + it { is_expected.to allow_value(package_name_pattern).for(:package_name_pattern) } + else + it { + is_expected.not_to( + allow_value(package_name_pattern) + .for(:package_name_pattern) + .with_message(_('should be a valid NPM package name with optional wildcard characters.')) + ) + } + end end end diff --git a/spec/models/preloaders/commit_status_preloader_spec.rb b/spec/models/preloaders/commit_status_preloader_spec.rb index 85ea784335c..0453b6267ed 100644 --- a/spec/models/preloaders/commit_status_preloader_spec.rb +++ b/spec/models/preloaders/commit_status_preloader_spec.rb @@ -21,13 +21,13 @@ RSpec.describe Preloaders::CommitStatusPreloader do it 'prevents N+1 for specified relations', :use_sql_query_cache do execute - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do call_each_relation(statuses.sample(3)) end expect do call_each_relation(statuses) - end.to issue_same_number_of_queries_as(control_count) + end.to issue_same_number_of_queries_as(control) end private diff --git a/spec/models/project_authorizations/changes_spec.rb b/spec/models/project_authorizations/changes_spec.rb index d6ccfccbcbe..9c2686e82f6 100644 --- a/spec/models/project_authorizations/changes_spec.rb +++ b/spec/models/project_authorizations/changes_spec.rb @@ -28,48 +28,81 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro end shared_examples_for 'publishes AuthorizationsChangedEvent' do - it 'publishes a AuthorizationsChangedEvent event with project id' do - project_ids.each do |project_id| - project_data = { project_id: project_id } - project_event = instance_double('::ProjectAuthorizations::AuthorizationsChangedEvent', data: project_data) + it 'does not publish a AuthorizationsChangedEvent event' do + expect(::Gitlab::EventStore).not_to receive(:publish) + .with(an_instance_of(::ProjectAuthorizations::AuthorizationsChangedEvent)) - allow(::ProjectAuthorizations::AuthorizationsChangedEvent).to receive(:new) - .with(data: project_data) - .and_return(project_event) + apply_project_authorization_changes + end - allow(::Gitlab::EventStore).to receive(:publish) - expect(::Gitlab::EventStore).to receive(:publish).with(project_event) + context 'when feature flag "add_policy_approvers_to_rules" is disabled' do + before do + stub_feature_flags(add_policy_approvers_to_rules: false) end - apply_project_authorization_changes + it 'publishes a AuthorizationsChangedEvent event with project id' do + allow(::Gitlab::EventStore).to receive(:publish) + project_ids.each do |project_id| + project_data = { project_id: project_id } + project_event = instance_double('::ProjectAuthorizations::AuthorizationsChangedEvent', data: project_data) + + allow(::ProjectAuthorizations::AuthorizationsChangedEvent).to receive(:new) + .with(data: project_data) + .and_return(project_event) + + expect(::Gitlab::EventStore).to receive(:publish).with(project_event) + end + + apply_project_authorization_changes + end end end shared_examples_for 'publishes AuthorizationsRemovedEvent' do it 'publishes a AuthorizationsRemovedEvent event with project id' do - project_ids.each do |project_id| + allow(::Gitlab::EventStore).to receive(:publish_group) + project_events = project_ids.map do |project_id| project_data = { project_id: project_id, user_ids: user_ids } project_event = instance_double('::ProjectAuthorizations::AuthorizationsRemovedEvent', data: project_data) allow(::ProjectAuthorizations::AuthorizationsRemovedEvent).to receive(:new) .with(data: project_data) .and_return(project_event) + project_event + end + expect(::Gitlab::EventStore).to receive(:publish_group).with(project_events) - allow(::Gitlab::EventStore).to receive(:publish) - expect(::Gitlab::EventStore).to receive(:publish).with(project_event) + apply_project_authorization_changes + end + end + + shared_examples_for 'publishes AuthorizationsAddedEvent' do + it 'publishes a AuthorizationsAddedEvent event with project id' do + allow(::Gitlab::EventStore).to receive(:publish_group) + project_events = project_ids.map do |project_id| + project_data = { project_id: project_id, user_ids: user_ids } + project_event = instance_double('::ProjectAuthorizations::AuthorizationsAddedEvent', data: project_data) + + allow(::ProjectAuthorizations::AuthorizationsAddedEvent).to receive(:new) + .with(data: project_data) + .and_return(project_event) + project_event end + expect(::Gitlab::EventStore).to receive(:publish_group).with(project_events) apply_project_authorization_changes end - context 'when feature flag "user_approval_rules_removal" is disabled' do + context 'when feature flag "add_policy_approvers_to_rules" is disabled' do before do - stub_feature_flags(user_approval_rules_removal: false) + stub_feature_flags(add_policy_approvers_to_rules: false) end - it 'does not publish a AuthorizationsRemovedEvent event' do + it 'does not publish a AuthorizationsAddedEvent event' do expect(::Gitlab::EventStore).not_to( - receive(:publish).with(an_instance_of(::ProjectAuthorizations::AuthorizationsRemovedEvent)) + receive(:publish_group).with(array_including( + an_instance_of(::ProjectAuthorizations::AuthorizationsAddedEvent)) + ) ) apply_project_authorization_changes @@ -88,8 +121,23 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro shared_examples_for 'does not publish AuthorizationsRemovedEvent' do it 'does not publish a AuthorizationsRemovedEvent event' do - expect(::Gitlab::EventStore).not_to receive(:publish) - .with(an_instance_of(::ProjectAuthorizations::AuthorizationsRemovedEvent)) + expect(::Gitlab::EventStore).not_to( + receive(:publish_group).with( + array_including(an_instance_of(::ProjectAuthorizations::AuthorizationsRemovedEvent)) + ) + ) + + apply_project_authorization_changes + end + end + + shared_examples_for 'does not publish AuthorizationsAddedEvent' do + it 'does not publish a AuthorizationsAddedEvent event' do + expect(::Gitlab::EventStore).not_to( + receive(:publish_group).with( + array_including(an_instance_of(::ProjectAuthorizations::AuthorizationsAddedEvent)) + ) + ) apply_project_authorization_changes end @@ -101,6 +149,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro let_it_be(:project_2) { create(:project) } let_it_be(:project_3) { create(:project) } let(:project_ids) { [project_1.id, project_2.id, project_3.id] } + let(:user_ids) { [user.id] } let(:authorizations_to_add) do [ @@ -155,6 +204,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'logs the detail', batch_size: 2 it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'publishes AuthorizationsAddedEvent' it_behaves_like 'does not publish AuthorizationsRemovedEvent' context 'when the GitLab installation does not have a replica database configured' do @@ -166,6 +216,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'inserts the rows in batches, as per the `per_batch` size, without a delay between batches' it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'publishes AuthorizationsAddedEvent' it_behaves_like 'does not publish AuthorizationsRemovedEvent' end end @@ -178,6 +229,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'inserts the rows in batches, as per the `per_batch` size, without a delay between batches' it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'publishes AuthorizationsAddedEvent' it_behaves_like 'does not publish AuthorizationsRemovedEvent' end end @@ -242,6 +294,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'logs the detail', batch_size: 2 it_behaves_like 'publishes AuthorizationsChangedEvent' it_behaves_like 'publishes AuthorizationsRemovedEvent' + it_behaves_like 'does not publish AuthorizationsAddedEvent' context 'when the GitLab installation does not have a replica database configured' do before do @@ -253,6 +306,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' it_behaves_like 'publishes AuthorizationsRemovedEvent' + it_behaves_like 'does not publish AuthorizationsAddedEvent' end end @@ -265,6 +319,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' it_behaves_like 'publishes AuthorizationsRemovedEvent' + it_behaves_like 'does not publish AuthorizationsAddedEvent' end context 'when the user_ids list is empty' do @@ -273,6 +328,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'does not removes project authorizations of the users in the current project' it_behaves_like 'does not publish AuthorizationsChangedEvent' it_behaves_like 'does not publish AuthorizationsRemovedEvent' + it_behaves_like 'does not publish AuthorizationsAddedEvent' end context 'when the user_ids list is nil' do @@ -281,6 +337,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'does not removes project authorizations of the users in the current project' it_behaves_like 'does not publish AuthorizationsChangedEvent' it_behaves_like 'does not publish AuthorizationsRemovedEvent' + it_behaves_like 'does not publish AuthorizationsAddedEvent' end end @@ -344,6 +401,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'logs the detail', batch_size: 2 it_behaves_like 'publishes AuthorizationsChangedEvent' it_behaves_like 'publishes AuthorizationsRemovedEvent' + it_behaves_like 'does not publish AuthorizationsAddedEvent' context 'when the GitLab installation does not have a replica database configured' do before do @@ -355,6 +413,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' it_behaves_like 'publishes AuthorizationsRemovedEvent' + it_behaves_like 'does not publish AuthorizationsAddedEvent' end end @@ -367,6 +426,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' it_behaves_like 'publishes AuthorizationsRemovedEvent' + it_behaves_like 'does not publish AuthorizationsAddedEvent' end context 'when the project_ids list is empty' do @@ -375,6 +435,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'does not removes any project authorizations from the current user' it_behaves_like 'does not publish AuthorizationsChangedEvent' it_behaves_like 'does not publish AuthorizationsRemovedEvent' + it_behaves_like 'does not publish AuthorizationsAddedEvent' end context 'when the user_ids list is nil' do @@ -383,6 +444,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'does not removes any project authorizations from the current user' it_behaves_like 'does not publish AuthorizationsChangedEvent' it_behaves_like 'does not publish AuthorizationsRemovedEvent' + it_behaves_like 'does not publish AuthorizationsAddedEvent' end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c256c4f10f8..1743c9bd89d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1121,6 +1121,8 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end it { is_expected.to delegate_method(:members).to(:team).with_prefix(true) } + it { is_expected.to delegate_method(:has_user?).to(:team) } + it { is_expected.to delegate_method(:member?).to(:team) } it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).allow_nil } it { is_expected.to delegate_method(:root_ancestor).to(:namespace).allow_nil } it { is_expected.to delegate_method(:certificate_based_clusters_enabled?).to(:namespace).allow_nil } @@ -2340,11 +2342,11 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it 'avoids n + 1', :aggregate_failures do create(:prometheus_integration) run_test = -> { described_class.include_integration(:prometheus_integration).map(&:prometheus_integration) } - control_count = ActiveRecord::QueryRecorder.new { run_test.call } + control = ActiveRecord::QueryRecorder.new { run_test.call } create(:prometheus_integration) expect(run_test.call.count).to eq(2) - expect { run_test.call }.not_to exceed_query_limit(control_count) + expect { run_test.call }.not_to exceed_query_limit(control) end end @@ -6591,17 +6593,17 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr let_it_be(:subject) { create(:project) } it 'avoids N+1 database queries' do - control_count = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_integrations }.count + control = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_integrations } - expect(control_count).to be <= 4 + expect(control.count).to be <= 4 end it 'avoids N+1 database queries with more available integrations' do allow(Integration).to receive(:available_integration_names).and_return(%w[pushover]) - control_count = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_integrations } + control = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_integrations } allow(Integration).to receive(:available_integration_names).and_call_original - expect { subject.find_or_initialize_integrations }.not_to exceed_query_limit(control_count) + expect { subject.find_or_initialize_integrations }.not_to exceed_query_limit(control) end context 'with disabled integrations' do @@ -6648,11 +6650,11 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it 'avoids N+1 database queries' do allow(Integration).to receive(:available_integration_names).and_return(%w[prometheus pushover]) - control_count = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_integration('prometheus') }.count + control = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_integration('prometheus') } allow(Integration).to receive(:available_integration_names).and_call_original - expect { subject.find_or_initialize_integration('prometheus') }.not_to exceed_query_limit(control_count) + expect { subject.find_or_initialize_integration('prometheus') }.not_to exceed_query_limit(control) end it 'returns nil if integration is disabled' do @@ -7978,6 +7980,24 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) end + + it 'assigns slug value for new topics' do + topic = create(:topic, name: 'old topic', title: 'old topic', slug: nil) + project.topic_list = topic.name + project.save! + + project.topic_list = 'old topic, new topic' + expect { expect(project.save).to be true }.to change { Projects::Topic.count }.by(1) + + topics = project.reset.topics + expect(topics.map(&:name)).to match_array(['old topic', 'new topic']) + + old_topic = topics.first + new_topic = topics.last + + expect(old_topic.slug).to be_nil + expect(new_topic.slug).to eq('newtopic') + end end context 'public topics counter' do @@ -8970,6 +8990,30 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end + describe '#allows_multiple_merge_request_assignees?' do + let(:project) { build_stubbed(:project) } + + subject(:allows_multiple_merge_request_assignees?) { project.allows_multiple_merge_request_assignees? } + + it { is_expected.to eq(false) } + end + + describe '#allows_multiple_merge_request_reviewers?' do + let(:project) { build_stubbed(:project) } + + subject(:allows_multiple_merge_request_reviewers?) { project.allows_multiple_merge_request_reviewers? } + + it { is_expected.to eq(false) } + end + + describe '#on_demand_dast_available?' do + let_it_be(:project) { create(:project) } + + subject(:on_demand_dast_available?) { project.on_demand_dast_available? } + + it { is_expected.to be_falsy } + end + private def finish_job(export_job) diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index 211ac257c53..d21d29aa469 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -647,4 +647,22 @@ RSpec.describe ProjectStatistics do end end end + + describe '#export_size' do + it 'does not include artifacts & packages size' do + statistics.update!( + repository_size: 3.gigabytes, + wiki_size: 3.gigabytes, + lfs_objects_size: 3.gigabytes, + build_artifacts_size: 3.gigabytes, + packages_size: 3.gigabytes, + snippets_size: 3.gigabytes, + uploads_size: 3.gigabytes + ) + + statistics.refresh_storage_size! + + expect(statistics.reload.export_size).to eq(15.gigabytes) + end + end end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 10a2e967b14..cd721b9f163 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -341,22 +341,60 @@ RSpec.describe ProjectTeam, feature_category: :groups_and_projects do end end + describe '#has_user?' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, namespace: group) } + let_it_be(:user) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:invited_project_member) { create(:project_member, :owner, :invited, project: project) } + + subject { project.team.has_user?(user) } + + context 'when the user is a member' do + before_all do + project.add_developer(user) + end + + it { is_expected.to be_truthy } + it { expect(group.has_user?(user2)).to be_falsey } + end + + context 'when user is a member with minimal access' do + before_all do + project.add_member(user, GroupMember::MINIMAL_ACCESS) + end + + it { is_expected.to be_falsey } + end + + context 'when user is not a direct member of the project' do + before_all do + create(:group_member, :developer, user: user, source: group) + end + + it { is_expected.to be_falsey } + end + + context 'when the user is an invited member' do + it 'returns false when nil is passed' do + expect(invited_project_member.user).to eq(nil) + expect(project.team.has_user?(invited_project_member.user)).to be_falsey + end + end + end + describe "#human_max_access" do - it 'returns Maintainer role' do - user = create(:user) - group = create(:group) - project = create(:project, namespace: group) + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, namespace: group) } + it 'returns Maintainer role' do group.add_maintainer(user) expect(project.team.human_max_access(user.id)).to eq 'Maintainer' end it 'returns Owner role' do - user = create(:user) - group = create(:group) - project = create(:project, namespace: group) - group.add_owner(user) expect(project.team.human_max_access(user.id)).to eq 'Owner' diff --git a/spec/models/projects/project_topic_spec.rb b/spec/models/projects/project_topic_spec.rb index c7a989040c7..634c391a25a 100644 --- a/spec/models/projects/project_topic_spec.rb +++ b/spec/models/projects/project_topic_spec.rb @@ -12,5 +12,6 @@ RSpec.describe Projects::ProjectTopic do describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:topic) } + it { is_expected.to validate_uniqueness_of(:topic_id).scoped_to(:project_id) } end end diff --git a/spec/models/projects/topic_spec.rb b/spec/models/projects/topic_spec.rb index ebe53f3761d..e322fbbbcc3 100644 --- a/spec/models/projects/topic_spec.rb +++ b/spec/models/projects/topic_spec.rb @@ -32,6 +32,21 @@ RSpec.describe Projects::Topic do it { is_expected.not_to allow_value("new\nline").for(:name).with_message(name_format_message) } it { is_expected.not_to allow_value("new\rline").for(:name).with_message(name_format_message) } it { is_expected.not_to allow_value("new\vline").for(:name).with_message(name_format_message) } + + context 'for slug' do + let(:slug_format_message) { "can contain only letters, digits, '_', '-', '.'" } + + it { is_expected.to validate_length_of(:slug).is_at_most(255) } + it { is_expected.to validate_uniqueness_of(:slug).case_insensitive } + + it { is_expected.not_to allow_value("new\nline").for(:slug).with_message(slug_format_message) } + it { is_expected.not_to allow_value("space value").for(:slug).with_message(slug_format_message) } + it { is_expected.not_to allow_value("$special_symbol_value").for(:slug).with_message(slug_format_message) } + + it { is_expected.to allow_value("underscored_value").for(:slug) } + it { is_expected.to allow_value("hypened-value").for(:slug) } + it { is_expected.to allow_value("dotted.value").for(:slug) } + end end describe 'scopes' do diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index bff9f73e44a..4a4cb1ae46a 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -156,6 +156,7 @@ RSpec.describe Release, feature_category: :release_orchestration do describe 'latest releases' do let_it_be(:yesterday) { Time.zone.now - 1.day } + let_it_be(:today) { Time.zone.now } let_it_be(:tomorrow) { Time.zone.now + 1.day } let_it_be(:project2) { create(:project) } @@ -176,6 +177,14 @@ RSpec.describe Release, feature_category: :release_orchestration do create(:release, project: project2, released_at: tomorrow, created_at: yesterday) end + let_it_be(:project2_release3) do + create(:release, project: project2, released_at: today, created_at: yesterday) + end + + let_it_be(:project2_release4) do + create(:release, project: project2, released_at: today, created_at: yesterday, release_published_at: today) + end + let(:args) { {} } describe '.latest' do @@ -240,6 +249,16 @@ RSpec.describe Release, feature_category: :release_orchestration do end end end + + describe '.waiting_for_publish_event' do + let(:releases) { [project2_release3] } + + subject(:waiting) { described_class.waiting_for_publish_event } + + it "find today's releases not yet published" do + expect(waiting).to match_array(releases) + end + end end describe '#assets_count' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index eeb0bbb8e7d..ca2ee447b4c 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -661,7 +661,7 @@ RSpec.describe Repository, feature_category: :source_code_management do describe '#blob_at' do context 'blank sha' do - subject { repository.blob_at(Gitlab::Git::BLANK_SHA, '.gitignore') } + subject { repository.blob_at(Gitlab::Git::SHA1_BLANK_SHA, '.gitignore') } it { is_expected.to be_nil } end @@ -3226,8 +3226,8 @@ RSpec.describe Repository, feature_category: :source_code_management do end it 'returns false for invalid commit IDs' do - expect(repository.ancestor?(commit.id, Gitlab::Git::BLANK_SHA)).to eq(false) - expect(repository.ancestor?(Gitlab::Git::BLANK_SHA, commit.id)).to eq(false) + expect(repository.ancestor?(commit.id, Gitlab::Git::SHA1_BLANK_SHA)).to eq(false) + expect(repository.ancestor?(Gitlab::Git::SHA1_BLANK_SHA, commit.id)).to eq(false) end end diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb index 8cc89578e0e..27a39672994 100644 --- a/spec/models/resource_label_event_spec.rb +++ b/spec/models/resource_label_event_spec.rb @@ -3,12 +3,13 @@ require 'spec_helper' RSpec.describe ResourceLabelEvent, feature_category: :team_planning, type: :model do - let_it_be(:project) { create(:project, :repository) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, group: group) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:merge_request) { create(:merge_request, source_project: project) } let_it_be(:label) { create(:label, project: project) } - subject { build(:resource_label_event, issue: issue, label: label) } + subject(:resource_label_event) { build(:resource_label_event, issue: issue, label: label) } it_behaves_like 'having unique enum values' @@ -95,6 +96,63 @@ RSpec.describe ResourceLabelEvent, feature_category: :team_planning, type: :mode end end + describe '#reference_html' do + subject { Nokogiri::HTML.fragment(label_event.reference_html).css('a').first.attr('href') } + + before do + label_event.refresh_invalid_reference + end + + context 'when resource event belongs to a group level issue' do + let(:group_label) { create(:group_label, group: group) } + let(:label_event) do + group_issue = create(:issue, :group_level, namespace: group) + + create(:resource_label_event, issue: group_issue, label: group_label) + end + + it { is_expected.to eq(Gitlab::Routing.url_helpers.group_work_items_path(group, label_name: group_label.title)) } + end + + context 'when resource event belongs to a project level issue' do + let(:label_event) { resource_label_event } + + it { is_expected.to eq(Gitlab::Routing.url_helpers.project_issues_path(project, label_name: label.title)) } + end + + context 'when resource event belongs to a merge request' do + let(:label_event) { create(:resource_label_event, merge_request: merge_request, label: label) } + + it do + is_expected.to eq(Gitlab::Routing.url_helpers.project_merge_requests_path(project, label_name: label.title)) + end + end + end + + describe '#group' do + subject { build_stubbed(:resource_label_event, **issuable_attributes).group } + + context 'when issuable is a merge request' do + let(:issuable_attributes) { { merge_request: merge_request } } + + it { is_expected.to be_nil } + end + + context 'when issuable is an issue' do + context 'when issue exists at the project level' do + let(:issuable_attributes) { { issue: issue } } + + it { is_expected.to be_nil } + end + + context 'when issue exists at the group level' do + let(:issuable_attributes) { { issue: build_stubbed(:issue, :group_level, namespace: group) } } + + it { is_expected.to eq(group) } + end + end + end + describe '.visible_to_user?' do let_it_be(:user) { create(:user) } let_it_be(:issue_project) { create(:project) } diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index 7cada013636..8a791a19dec 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -74,7 +74,7 @@ RSpec.describe Route do let!(:another_group) { create(:group, path: 'gittlab', name: 'gitllab') } let!(:another_group_nested) { create(:group, path: 'git_lab', name: 'git_lab', parent: another_group) } - context 'path update' do + shared_examples_for 'path update' do context 'when route name is set' do before do route.update!(path: 'bar') @@ -116,7 +116,7 @@ RSpec.describe Route do end end - context 'name update' do + shared_examples_for 'name update' do it 'updates children routes with new path' do route.update!(name: 'bar') @@ -134,6 +134,18 @@ RSpec.describe Route do .to change { route.name }.from(nil).to('bar') end end + + it_behaves_like 'path update' + it_behaves_like 'name update' + + context 'when the feature flag `batch_route_updates` if turned off' do + before do + stub_feature_flags(batch_route_updates: false) + end + + it_behaves_like 'path update' + it_behaves_like 'name update' + end end describe '#create_redirect_for_old_path' do diff --git a/spec/models/time_tracking/timelog_category_spec.rb b/spec/models/time_tracking/timelog_category_spec.rb index ac2fb651134..d07ba29091c 100644 --- a/spec/models/time_tracking/timelog_category_spec.rb +++ b/spec/models/time_tracking/timelog_category_spec.rb @@ -2,9 +2,10 @@ require 'spec_helper' -RSpec.describe TimeTracking::TimelogCategory, type: :model do +RSpec.describe TimeTracking::TimelogCategory, feature_category: :team_planning do describe 'associations' do it { is_expected.to belong_to(:namespace).with_foreign_key('namespace_id') } + it { is_expected.to have_many(:timelogs) } end describe 'default values' do diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb index aee2c4ded19..dc87aea0cb4 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Timelog, feature_category: :team_planning do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:issue).touch(true) } it { is_expected.to belong_to(:merge_request).touch(true) } + it { is_expected.to belong_to(:timelog_category).optional(true) } it { is_expected.to be_valid } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cc0ea69401e..7014c9e685f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -187,7 +187,6 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to have_many(:merge_request_assignees).inverse_of(:assignee) } it { is_expected.to have_many(:merge_request_reviewers).inverse_of(:reviewer) } it { is_expected.to have_many(:created_custom_emoji).inverse_of(:creator) } - it { is_expected.to have_many(:in_product_marketing_emails) } it { is_expected.to have_many(:timelogs) } it { is_expected.to have_many(:callouts).class_name('Users::Callout') } it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout') } @@ -223,6 +222,17 @@ RSpec.describe User, feature_category: :user_profile do is_expected.to have_many(:alert_assignees).class_name('::AlertManagement::AlertAssignee').inverse_of(:assignee) end + describe 'organizations association' do + it 'does not create a cross-database query' do + user = create(:user) + create(:organization_user, user: user) + + with_cross_joins_prevented do + expect(user.organizations.count).to eq(1) + end + end + end + describe 'default values' do let(:user) { described_class.new } @@ -616,38 +626,33 @@ RSpec.describe User, feature_category: :user_profile do end end - describe 'username' do + shared_examples 'username validations' do it 'validates presence' do expect(subject).to validate_presence_of(:username) end - it 'rejects denied names' do - user = build(:user, username: 'dashboard') - - expect(user).not_to be_valid - expect(user.errors.messages[:username]).to eq ['dashboard is a reserved name'] - end - - it 'allows child names' do - user = build(:user, username: 'avatar') + context 'when username is reserved' do + let(:username) { 'dashboard' } - expect(user).to be_valid + it 'rejects denied names' do + expect(user).not_to be_valid + expect(user.errors.messages[:username]).to eq ['dashboard is a reserved name'] + end end - it 'allows wildcard names' do - user = build(:user, username: 'blob') + context 'when username is a child' do + let(:username) { 'avatar' } - expect(user).to be_valid + it 'allows child names' do + expect(user).to be_valid + end end - context 'when username is changed' do - let(:user) { build_stubbed(:user, username: 'old_path', namespace: build_stubbed(:user_namespace)) } + context 'when username is a wildcard' do + let(:username) { 'blob' } - it 'validates move_dir is allowed for the namespace' do - expect(user.namespace).to receive(:any_project_has_container_registry_tags?).and_return(true) - user.username = 'new_path' - expect(user).to be_invalid - expect(user.errors.messages[:username].first).to eq(_('cannot be changed if a personal project has container registry tags.')) + it 'allows wildcard names' do + expect(user).to be_valid end end @@ -656,25 +661,59 @@ RSpec.describe User, feature_category: :user_profile do let!(:other_user) { create(:user, username: username) } it 'is invalid' do - user = build(:user, username: username) - expect(user).not_to be_valid expect(user.errors.full_messages).to eq(['Username has already been taken']) end end - it 'validates format' do - Mime::EXTENSION_LOOKUP.keys.each do |type| - user = build(:user, username: "test.#{type}") + Mime::EXTENSION_LOOKUP.keys.each do |type| + context 'with extension format' do + let(:username) { "test.#{type}" } - expect(user).not_to be_valid - expect(user.errors.full_messages).to include('Username ending with a reserved file extension is not allowed.') - expect(build(:user, username: "test#{type}")).to be_valid + it do + expect(user).not_to be_valid + expect(user.errors.full_messages).to include('Username ending with a reserved file extension is not allowed.') + end + end + + context 'when suffixed by extension type' do + let(:username) { "test#{type}" } + + it do + expect(user).to be_valid + end end end + end + + context 'when creating user' do + let(:user) { build(:user, username: username) } + + include_examples 'username validations' + end + + context 'when updating user' do + let(:user) { create(:user) } + + before do + user.username = username if defined?(username) + end + + include_examples 'username validations' + + context 'when personal project has container registry tags' do + let(:user) { build_stubbed(:user, username: 'old_path', namespace: build_stubbed(:user_namespace)) } + + before do + expect(user.namespace).to receive(:any_project_has_container_registry_tags?).and_return(true) + end - it 'validates format on updated record' do - expect(create(:user).update(username: 'profile.html')).to be_falsey + it 'validates move_dir is allowed for the namespace' do + user.username = 'new_path' + + expect(user).to be_invalid + expect(user.errors.messages[:username].first).to eq(_('cannot be changed if a personal project has container registry tags.')) + end end end @@ -1434,6 +1473,20 @@ RSpec.describe User, feature_category: :user_profile do end end + describe '.ordered_by_id_desc' do + let_it_be(:first_user) { create(:user) } + let_it_be(:second_user) { create(:user) } + + it 'generates the order SQL in descending order' do + expect(described_class.ordered_by_id_desc.to_sql).to include( + 'ORDER BY "users"."id" DESC') + end + + it 'sorts users correctly' do + expect(described_class.ordered_by_id_desc).to eq([second_user, first_user]) + end + end + describe '.trusted' do let_it_be(:trusted_user1) { create(:user, :trusted) } let_it_be(:trusted_user2) { create(:user, :trusted) } @@ -3368,6 +3421,27 @@ RSpec.describe User, feature_category: :user_profile do end end + describe '.gfm_autocomplete_search' do + let_it_be(:user_1) { create(:user, username: 'someuser', name: 'John Doe') } + let_it_be(:user_2) { create(:user, username: 'userthomas', name: 'Thomas Person') } + + it 'returns partial matches on username' do + expect(described_class.gfm_autocomplete_search('some')).to eq([user_1]) + end + + it 'returns matches on name across multiple words' do + expect(described_class.gfm_autocomplete_search('johnd')).to eq([user_1]) + end + + it 'prioritizes sorting of matches that start with the query' do + expect(described_class.gfm_autocomplete_search('user')).to eq([user_2, user_1]) + end + + it 'falls back to sorting by username' do + expect(described_class.gfm_autocomplete_search('ser')).to eq([user_1, user_2]) + end + end + describe '.user_search_minimum_char_limit' do it 'returns true' do expect(described_class.user_search_minimum_char_limit).to be(true) @@ -3624,6 +3698,66 @@ RSpec.describe User, feature_category: :user_profile do end end + describe '#can_create_project?' do + let(:user) { create(:user) } + + context "when projects_limit_left is 0" do + before do + allow(user).to receive(:projects_limit_left).and_return(0) + end + + it "returns false" do + expect(user.can_create_project?).to be_falsey + end + end + + context "when projects_limit_left is > 0" do + before do + allow(user).to receive(:projects_limit_left).and_return(1) + end + + context "with allow_project_creation_for_guest_and_below default value of true" do + it "returns true" do + expect(user.can_create_project?).to be_truthy + end + end + + context "when Gitlab::CurrentSettings.allow_project_creation_for_guest_and_below is false" do + before do + stub_application_setting(allow_project_creation_for_guest_and_below: false) + end + + [ + Gitlab::Access::NO_ACCESS, + Gitlab::Access::MINIMAL_ACCESS, + Gitlab::Access::GUEST + ].each do |role| + context "when users highest role is #{role}" do + it "returns false" do + allow(user).to receive(:highest_role).and_return(role) + expect(user.can_create_project?).to be_falsey + end + end + end + + [ + Gitlab::Access::REPORTER, + Gitlab::Access::DEVELOPER, + Gitlab::Access::MAINTAINER, + Gitlab::Access::OWNER, + Gitlab::Access::ADMIN + ].each do |role| + context "when users highest role is #{role}" do + it "returns true" do + allow(user).to receive(:highest_role).and_return(role) + expect(user.can_create_project?).to be_truthy + end + end + end + end + end + end + describe '#all_emails' do let(:user) { create(:user) } let!(:unconfirmed_secondary_email) { create(:email, user: user) } @@ -4468,13 +4602,13 @@ RSpec.describe User, feature_category: :user_profile do it 'avoids N+1 queries' do fresh_user = described_class.find(user.id) - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do fresh_user.solo_owned_groups - end.count + end create(:group).add_owner(user) - expect { solo_owned_groups }.not_to exceed_query_limit(control_count) + expect { solo_owned_groups }.not_to exceed_query_limit(control) end end end @@ -5693,27 +5827,6 @@ RSpec.describe User, feature_category: :user_profile do expect(user.namespace).to be_nil end - - context 'when create_personal_ns_outside_model feature flag is disabled' do - before do - stub_feature_flags(create_personal_ns_outside_model: false) - end - - it 'creates the namespace' do - expect(user.namespace).to be_nil - - user.save! - - expect(user.namespace).to be_present - expect(user.namespace).to be_kind_of(Namespaces::UserNamespace) - end - - it 'creates the namespace setting' do - user.save! - - expect(user.namespace.namespace_settings).to be_persisted - end - end end context 'for an existing user' do diff --git a/spec/models/users/in_product_marketing_email_spec.rb b/spec/models/users/in_product_marketing_email_spec.rb deleted file mode 100644 index b1642383e42..00000000000 --- a/spec/models/users/in_product_marketing_email_spec.rb +++ /dev/null @@ -1,137 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Users::InProductMarketingEmail, type: :model, feature_category: :onboarding do - let(:track) { :create } - let(:series) { 0 } - - describe 'associations' do - it { is_expected.to belong_to(:user) } - end - - describe 'validations' do - subject { build(:in_product_marketing_email) } - - it { is_expected.to validate_presence_of(:user) } - - context 'when track+series email' do - it { is_expected.to validate_presence_of(:track) } - it { is_expected.to validate_presence_of(:series) } - - it { - is_expected.to validate_uniqueness_of(:user_id) - .scoped_to([:track, :series]).with_message('track series email has already been sent') - } - end - end - - describe '.without_track_and_series' do - let_it_be(:user) { create(:user) } - - subject(:without_track_and_series) { User.merge(described_class.without_track_and_series(track, series)) } - - before do - create(:in_product_marketing_email, track: :create, series: 0, user: user) - create(:in_product_marketing_email, track: :create, series: 1, user: user) - create(:in_product_marketing_email, track: :verify, series: 0, user: user) - end - - context 'when given track and series already exists' do - it { expect(without_track_and_series).to be_empty } - end - - context 'when track does not exist' do - let(:track) { :trial } - - it { expect(without_track_and_series).to eq [user] } - end - - context 'when series does not exist' do - let(:series) { 2 } - - it { expect(without_track_and_series).to eq [user] } - end - - context 'when no track or series for a user exists' do - let(:track) { :create } - let(:series) { 0 } - let(:other_user) { create(:user) } - - it { expect(without_track_and_series).to eq [other_user] } - end - end - - describe '.for_user_with_track_and_series' do - let_it_be(:user) { create(:user) } - let_it_be(:in_product_marketing_email) { create(:in_product_marketing_email, series: 0, track: 0, user: user) } - - subject(:for_user_with_track_and_series) do - described_class.for_user_with_track_and_series(user, track, series).first - end - - context 'when record for user with given track and series exists' do - it { is_expected.to eq(in_product_marketing_email) } - end - - context 'when user is different' do - let(:user) { build_stubbed(:user) } - - it { is_expected.to be_nil } - end - - context 'when track is different' do - let(:track) { 1 } - - it { is_expected.to be_nil } - end - - context 'when series is different' do - let(:series) { 1 } - - it { is_expected.to be_nil } - end - end - - describe '.save_cta_click' do - let(:user) { create(:user) } - - subject(:save_cta_click) { described_class.save_cta_click(user, track, series) } - - context 'when there is no record' do - it 'does not error' do - expect { save_cta_click }.not_to raise_error - end - end - - context 'when there is no record for the track and series' do - it 'does not perform an update' do - other_email = create(:in_product_marketing_email, user: user, track: :verify, series: 2, cta_clicked_at: nil) - - expect { save_cta_click }.not_to change { other_email.reload } - end - end - - context 'when there is a record for the track and series' do - it 'saves the cta click date' do - email = create(:in_product_marketing_email, user: user, track: track, series: series, cta_clicked_at: nil) - - freeze_time do - expect { save_cta_click }.to change { email.reload.cta_clicked_at }.from(nil).to(Time.zone.now) - end - end - - context 'when cta_clicked_at is already set' do - it 'does not update' do - create(:in_product_marketing_email, user: user, track: track, series: series, cta_clicked_at: Time.zone.now) - - expect_next_found_instance_of(described_class) do |record| - expect(record).not_to receive(:update) - end - - save_cta_click - end - 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 15bbb507dee..eb73fc31dac 100644 --- a/spec/models/users/phone_number_validation_spec.rb +++ b/spec/models/users/phone_number_validation_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resiliency do + using RSpec::Parameterized::TableSyntax + let_it_be(:user) { create(:user) } let_it_be(:banned_user) { create(:user, :banned) } @@ -14,12 +16,12 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie it { is_expected.to validate_presence_of(:international_dial_code) } - it { + it do is_expected.to validate_numericality_of(:international_dial_code) .only_integer .is_greater_than_or_equal_to(1) .is_less_than_or_equal_to(999) - } + end it { is_expected.to validate_presence_of(:phone_number) } it { is_expected.to validate_length_of(:phone_number).is_at_most(12) } @@ -30,6 +32,27 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie it { is_expected.to validate_length_of(:telesign_reference_xid).is_at_most(255) } + describe '#similar_records' do + let_it_be(:phone_number_validation) { create(:phone_number_validation, :validated) } + + let_it_be(:phone_number) do + phone_number_validation.attributes.with_indifferent_access.slice( + :international_dial_code, :phone_number + ) + end + + let_it_be(:match) { create(:phone_number_validation, :validated, phone_number) } + let_it_be(:unvalidated_match) { create(:phone_number_validation, phone_number) } + + let_it_be(:non_match_1) { create(:phone_number_validation, phone_number.merge(international_dial_code: 81)) } + let_it_be(:non_match_2) { create(:phone_number_validation, phone_number.merge(phone_number: '5555555555')) } + + it 'returns matches with the same international dialing code and phone number' do + expect(phone_number_validation.similar_records).to match_array([unvalidated_match, match, + phone_number_validation]) + end + end + describe '.related_to_banned_user?' do let_it_be(:international_dial_code) { 1 } let_it_be(:phone_number) { '555' } @@ -41,7 +64,12 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie context 'when banned user has the same international dial code and phone number' do context 'and the matching record has not been verified' do before do - create(:phone_number_validation, user: banned_user) + create( + :phone_number_validation, + user: banned_user, + international_dial_code: international_dial_code, + phone_number: phone_number + ) end it { is_expected.to eq(false) } @@ -49,7 +77,13 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie context 'and the matching record has been verified' do before do - create(:phone_number_validation, :validated, user: banned_user) + create( + :phone_number_validation, + :validated, + user: banned_user, + international_dial_code: international_dial_code, + phone_number: phone_number + ) end it { is_expected.to eq(true) } @@ -58,7 +92,14 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie context 'when banned user has the same international dial code and phone number, but different country code' do before do - create(:phone_number_validation, :validated, user: banned_user, country: 'CA') + create( + :phone_number_validation, + :validated, + user: banned_user, + international_dial_code: international_dial_code, + phone_number: phone_number, + country: 'CA' + ) end it { is_expected.to eq(true) } @@ -66,7 +107,13 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie context 'when banned user does not have the same international dial code' do before do - create(:phone_number_validation, :validated, user: banned_user, international_dial_code: 61) + create( + :phone_number_validation, + :validated, + user: banned_user, + international_dial_code: 81, + phone_number: phone_number + ) end it { is_expected.to eq(false) } @@ -74,7 +121,13 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie context 'when banned user does not have the same phone number' do before do - create(:phone_number_validation, :validated, user: banned_user, phone_number: '666') + create( + :phone_number_validation, + :validated, + user: banned_user, + international_dial_code: international_dial_code, + phone_number: '666' + ) end it { is_expected.to eq(false) } @@ -82,7 +135,13 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie context 'when not-banned user has the same international dial code and phone number' do before do - create(:phone_number_validation, :validated, user: user) + create( + :phone_number_validation, + :validated, + user: user, + international_dial_code: international_dial_code, + phone_number: phone_number + ) end it { is_expected.to eq(false) } @@ -105,6 +164,57 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie end end end + + describe '.similar_to' do + subject(:similar_to) { described_class.similar_to(phone_number_validation) } + + let_it_be(:international_dial_code) { 44 } + let_it_be(:phone_number) { '111' } + + let_it_be(:phone_number_validation) do + create(:phone_number_validation, + :validated, + international_dial_code: international_dial_code, + phone_number: phone_number + ) + end + + let_it_be(:match) do + create(:phone_number_validation, + :validated, + international_dial_code: phone_number_validation.international_dial_code, + phone_number: phone_number_validation.phone_number + ) + end + + let_it_be(:non_match_1) do + create(:phone_number_validation, + :validated, + international_dial_code: phone_number_validation.international_dial_code, + phone_number: '222' + ) + end + + let_it_be(:non_match_2) do + create(:phone_number_validation, + :validated, + international_dial_code: 81, + phone_number: phone_number_validation.phone_number + ) + end + + let_it_be(:non_match_3) do + create(:phone_number_validation, + :validated, + international_dial_code: 82, + phone_number: '333' + ) + end + + it 'returns only records with the same international dialing code and phone number' do + expect(similar_to).to match_array([phone_number_validation, match]) + end + end end describe '#validated?' do @@ -142,4 +252,43 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie it { is_expected.to be_nil } end end + + describe '.sms_send_allowed_after' do + let_it_be(:record) { create(:phone_number_validation, sms_send_count: 0) } + + subject(:result) { record.sms_send_allowed_after } + + context 'when there are no attempts yet' do + it { is_expected.to be_nil } + end + + context 'when sms_send_wait_time feature flag is disabled' do + let_it_be(:record) { create(:phone_number_validation, sms_send_count: 1) } + + before do + stub_feature_flags(sms_send_wait_time: false) + end + + it { is_expected.to be_nil } + end + + where(:attempt_number, :expected_delay) do + 2 | 1.minute + 3 | 3.minutes + 4 | 5.minutes + 5 | 10.minutes + 6 | 10.minutes + end + + with_them do + it 'returns the correct delayed timestamp value' do + freeze_time do + record.update!(sms_send_count: attempt_number - 1, sms_sent_at: Time.current) + + expected_result = Time.current + expected_delay + expect(result).to eq expected_result + end + end + end + end end diff --git a/spec/models/work_items/hierarchy_restriction_spec.rb b/spec/models/work_items/hierarchy_restriction_spec.rb index 2c4d5d32fb8..890c007b6cd 100644 --- a/spec/models/work_items/hierarchy_restriction_spec.rb +++ b/spec/models/work_items/hierarchy_restriction_spec.rb @@ -15,4 +15,24 @@ RSpec.describe WorkItems::HierarchyRestriction do it { is_expected.to validate_presence_of(:child_type) } it { is_expected.to validate_uniqueness_of(:child_type).scoped_to(:parent_type_id) } end + + describe '#clear_parent_type_cache!' do + subject(:hierarchy_restriction) { build(:hierarchy_restriction) } + + context 'when a hierarchy restriction is saved' do + it 'calls #clear_reactive_cache! on parent type' do + expect(hierarchy_restriction.parent_type).to receive(:clear_reactive_cache!).once + + hierarchy_restriction.save! + end + end + + context 'when a hierarchy restriction is destroyed' do + it 'calls #clear_reactive_cache! on parent type' do + expect(hierarchy_restriction.parent_type).to receive(:clear_reactive_cache!).once + + hierarchy_restriction.destroy! + end + end + end end diff --git a/spec/models/work_items/widget_definition_spec.rb b/spec/models/work_items/widget_definition_spec.rb index da772eec39c..1540ee57ff4 100644 --- a/spec/models/work_items/widget_definition_spec.rb +++ b/spec/models/work_items/widget_definition_spec.rb @@ -26,7 +26,9 @@ RSpec.describe WorkItems::WidgetDefinition, feature_category: :team_planning do ::WorkItems::Widgets::HealthStatus, ::WorkItems::Widgets::Progress, ::WorkItems::Widgets::RequirementLegacy, - ::WorkItems::Widgets::TestReports + ::WorkItems::Widgets::TestReports, + ::WorkItems::Widgets::Color, + ::WorkItems::Widgets::RolledupDates ] end |