diff options
Diffstat (limited to 'spec/models')
118 files changed, 2584 insertions, 1345 deletions
diff --git a/spec/models/activity_pub/releases_subscription_spec.rb b/spec/models/activity_pub/releases_subscription_spec.rb new file mode 100644 index 00000000000..0c873a5c18a --- /dev/null +++ b/spec/models/activity_pub/releases_subscription_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ActivityPub::ReleasesSubscription, type: :model, feature_category: :release_orchestration do + describe 'factory' do + subject { build(:activity_pub_releases_subscription) } + + it { is_expected.to be_valid } + end + + describe 'associations' do + it { is_expected.to belong_to(:project).optional(false) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:subscriber_url) } + + describe 'subscriber_url' do + subject { build(:activity_pub_releases_subscription) } + + it { is_expected.to validate_uniqueness_of(:subscriber_url).case_insensitive.scoped_to([:project_id]) } + it { is_expected.to allow_value("http://example.com/actor").for(:subscriber_url) } + it { is_expected.not_to allow_values("I'm definitely not a URL").for(:subscriber_url) } + end + + describe 'subscriber_inbox_url' do + subject { build(:activity_pub_releases_subscription) } + + it { is_expected.to validate_uniqueness_of(:subscriber_inbox_url).case_insensitive.scoped_to([:project_id]) } + it { is_expected.to allow_value("http://example.com/actor").for(:subscriber_inbox_url) } + it { is_expected.not_to allow_values("I'm definitely not a URL").for(:subscriber_inbox_url) } + end + + describe 'shared_inbox_url' do + subject { build(:activity_pub_releases_subscription) } + + it { is_expected.to allow_value("http://example.com/actor").for(:shared_inbox_url) } + it { is_expected.not_to allow_values("I'm definitely not a URL").for(:shared_inbox_url) } + end + + describe 'payload' do + it { is_expected.not_to allow_value("string").for(:payload) } + it { is_expected.not_to allow_value(1.0).for(:payload) } + + it do + is_expected.to allow_value({ + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'https://example.com/actor#follow/1', + type: 'Follow', + actor: 'https://example.com/actor', + object: 'http://localhost/user/project/-/releases' + }).for(:payload) + end + end + end + + describe '.find_by_subscriber_url' do + let_it_be(:subscription) { create(:activity_pub_releases_subscription) } + + it 'returns a record if arguments match' do + result = described_class.find_by_subscriber_url(subscription.subscriber_url) + + expect(result).to eq(subscription) + end + + it 'returns a record if arguments match case insensitively' do + result = described_class.find_by_subscriber_url(subscription.subscriber_url.upcase) + + expect(result).to eq(subscription) + end + + it 'returns nil if project does not match' do + result = described_class.find_by_subscriber_url('I really should not exist') + + expect(result).to be(nil) + end + end +end diff --git a/spec/models/ai/service_access_token_spec.rb b/spec/models/ai/service_access_token_spec.rb index d979db4b3d6..d491735e604 100644 --- a/spec/models/ai/service_access_token_spec.rb +++ b/spec/models/ai/service_access_token_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe Ai::ServiceAccessToken, type: :model, feature_category: :application_performance do +RSpec.describe Ai::ServiceAccessToken, type: :model, feature_category: :cloud_connector do describe '.expired', :freeze_time do - let_it_be(:expired_token) { create(:service_access_token, :code_suggestions, :expired) } - let_it_be(:active_token) { create(:service_access_token, :code_suggestions, :active) } + let_it_be(:expired_token) { create(:service_access_token, :expired) } + let_it_be(:active_token) { create(:service_access_token, :active) } it 'selects all expired tokens' do expect(described_class.expired).to match_array([expired_token]) @@ -13,24 +13,14 @@ RSpec.describe Ai::ServiceAccessToken, type: :model, feature_category: :applicat end describe '.active', :freeze_time do - let_it_be(:expired_token) { create(:service_access_token, :code_suggestions, :expired) } - let_it_be(:active_token) { create(:service_access_token, :code_suggestions, :active) } + let_it_be(:expired_token) { create(:service_access_token, :expired) } + let_it_be(:active_token) { create(:service_access_token, :active) } it 'selects all active tokens' do expect(described_class.active).to match_array([active_token]) end end - # There is currently only one category, please expand this test when a new category is added. - describe '.for_category' do - let(:code_suggestions_token) { create(:service_access_token, :code_suggestions) } - let(:category) { :code_suggestions } - - it 'only selects tokens from the selected category' do - expect(described_class.for_category(category)).to match_array([code_suggestions_token]) - end - end - describe '#token' do let(:token_value) { 'Abc' } @@ -47,7 +37,6 @@ RSpec.describe Ai::ServiceAccessToken, type: :model, feature_category: :applicat describe 'validations' do it { is_expected.to validate_presence_of(:token) } - it { is_expected.to validate_presence_of(:category) } it { is_expected.to validate_presence_of(:expires_at) } end end diff --git a/spec/models/alert_management/http_integration_spec.rb b/spec/models/alert_management/http_integration_spec.rb index dc26d0323d7..ef9eaa960f2 100644 --- a/spec/models/alert_management/http_integration_spec.rb +++ b/spec/models/alert_management/http_integration_spec.rb @@ -44,8 +44,8 @@ RSpec.describe AlertManagement::HttpIntegration, feature_category: :incident_man context 'with valid JSON schema' do let(:attribute_mapping) do { - title: { path: %w(a b c), type: 'string', label: 'Title' }, - description: { path: %w(a), type: 'string' } + title: { path: %w[a b c], type: 'string', label: 'Title' }, + description: { path: %w[a], type: 'string' } } end @@ -78,7 +78,7 @@ RSpec.describe AlertManagement::HttpIntegration, feature_category: :incident_man context 'when property has extra attributes' do let(:attribute_mapping) do - { title: { path: %w(a b c), type: 'string', extra: 'property' } } + { title: { path: %w[a b c], type: 'string', extra: 'property' } } end it_behaves_like 'is invalid record' diff --git a/spec/models/analytics/cycle_analytics/value_stream_spec.rb b/spec/models/analytics/cycle_analytics/value_stream_spec.rb index 3b3187e0b51..852ace6a920 100644 --- a/spec/models/analytics/cycle_analytics/value_stream_spec.rb +++ b/spec/models/analytics/cycle_analytics/value_stream_spec.rb @@ -99,4 +99,23 @@ RSpec.describe Analytics::CycleAnalytics::ValueStream, type: :model, feature_cat it { is_expected.to be_custom } end end + + describe '#project' do + subject(:value_stream) do + build(:cycle_analytics_value_stream, name: 'value_stream_1', namespace: namespace).project + end + + context 'when namespace is a project' do + let_it_be(:project) { create(:project) } + let(:namespace) { project.project_namespace } + + it { is_expected.to eq(project) } + end + + context 'when namespace is a group' do + let_it_be(:namespace) { create(:group) } + + it { is_expected.to be_nil } + end + end end diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index b5f47c950b9..ffb46884e5d 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -81,7 +81,7 @@ RSpec.describe Appearance do end end - %i(logo header_logo pwa_icon favicon).each do |logo_type| + %i[logo header_logo pwa_icon favicon].each do |logo_type| it_behaves_like 'logo paths', logo_type end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 78bf410075b..a2d6c60fbd0 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -162,6 +162,8 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_inclusion_of(:user_defaults_to_private_profile).in_array([true, false]) } + it { is_expected.to validate_inclusion_of(:allow_project_creation_for_guest_and_below).in_array([true, false]) } + it { is_expected.to validate_inclusion_of(:deny_all_requests_except_allowed).in_array([true, false]) } it 'ensures max_pages_size is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do @@ -254,11 +256,11 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.not_to allow_value(['']).for(:valid_runner_registrars) } it { is_expected.not_to allow_value(['OBVIOUSLY_WRONG']).for(:valid_runner_registrars) } - it { is_expected.not_to allow_value(%w(project project)).for(:valid_runner_registrars) } + it { is_expected.not_to allow_value(%w[project project]).for(:valid_runner_registrars) } it { is_expected.not_to allow_value([nil]).for(:valid_runner_registrars) } it { is_expected.not_to allow_value(nil).for(:valid_runner_registrars) } it { is_expected.to allow_value([]).for(:valid_runner_registrars) } - it { is_expected.to allow_value(%w(project group)).for(:valid_runner_registrars) } + it { is_expected.to allow_value(%w[project group]).for(:valid_runner_registrars) } it { is_expected.to allow_value(http).for(:jira_connect_proxy_url) } it { is_expected.to allow_value(https).for(:jira_connect_proxy_url) } @@ -820,15 +822,6 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do subject { setting } end - # Upgraded databases will have this sort of content - context 'repository_storages is a String, not an Array' do - before do - described_class.where(id: setting.id).update_all(repository_storages: 'default') - end - - it { expect(setting.repository_storages).to eq(['default']) } - end - context 'auto_devops_domain setting' do context 'when auto_devops_enabled? is true' do before do @@ -865,31 +858,6 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do end end - context 'repository storages' do - before do - storages = { - 'custom1' => 'tmp/tests/custom_repositories_1', - 'custom2' => 'tmp/tests/custom_repositories_2', - 'custom3' => 'tmp/tests/custom_repositories_3' - - } - allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) - end - - describe 'inclusion' do - it { is_expected.to allow_value('custom1').for(:repository_storages) } - it { is_expected.to allow_value(%w(custom2 custom3)).for(:repository_storages) } - it { is_expected.not_to allow_value('alternative').for(:repository_storages) } - it { is_expected.not_to allow_value(%w(alternative custom1)).for(:repository_storages) } - end - - describe 'presence' do - it { is_expected.not_to allow_value([]).for(:repository_storages) } - it { is_expected.not_to allow_value("").for(:repository_storages) } - it { is_expected.not_to allow_value(nil).for(:repository_storages) } - end - end - context 'housekeeping settings' do it { is_expected.not_to allow_value(0).for(:housekeeping_optimize_repository_period) } end diff --git a/spec/models/authentication_event_spec.rb b/spec/models/authentication_event_spec.rb index 17fe10b5b4e..8ce949c737b 100644 --- a/spec/models/authentication_event_spec.rb +++ b/spec/models/authentication_event_spec.rb @@ -37,11 +37,11 @@ RSpec.describe AuthenticationEvent do describe '.providers' do before do - allow(Devise).to receive(:omniauth_providers).and_return(%w(ldapmain google_oauth2)) + allow(Devise).to receive(:omniauth_providers).and_return(%w[ldapmain google_oauth2]) end it 'returns an array of distinct providers' do - expect(described_class.providers).to match_array %w(ldapmain google_oauth2 standard two-factor two-factor-via-u2f-device two-factor-via-webauthn-device) + expect(described_class.providers).to match_array %w[ldapmain google_oauth2 standard two-factor two-factor-via-u2f-device two-factor-via-webauthn-device] end end diff --git a/spec/models/blob_viewer/base_spec.rb b/spec/models/blob_viewer/base_spec.rb index 682b6dc3b1d..91c32d5c7c9 100644 --- a/spec/models/blob_viewer/base_spec.rb +++ b/spec/models/blob_viewer/base_spec.rb @@ -11,7 +11,7 @@ RSpec.describe BlobViewer::Base do Class.new(described_class) do include BlobViewer::ServerSide - self.extensions = %w(pdf) + self.extensions = %w[pdf] self.binary = true self.collapse_limit = 1.megabyte self.size_limit = 5.megabytes @@ -41,7 +41,7 @@ RSpec.describe BlobViewer::Base do context 'when the file type is supported' do before do - viewer_class.file_types = %i(license) + viewer_class.file_types = %i[license] viewer_class.binary = false end diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index 3e98ba0973e..b822786579b 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -248,6 +248,24 @@ RSpec.describe BulkImports::Entity, type: :model, feature_category: :importers d end end + describe '#portable_class' do + context 'when entity is group' do + it 'returns Group class' do + entity = build(:bulk_import_entity, :group_entity) + + expect(entity.portable_class).to eq(Group) + end + end + + context 'when entity is project' do + it 'returns Project class' do + entity = build(:bulk_import_entity, :project_entity) + + expect(entity.portable_class).to eq(Project) + end + end + end + describe '#export_relations_url_path' do context 'when entity is group' do it 'returns group export relations url' do diff --git a/spec/models/bulk_imports/failure_spec.rb b/spec/models/bulk_imports/failure_spec.rb index b3fd60ba348..928f14aaced 100644 --- a/spec/models/bulk_imports/failure_spec.rb +++ b/spec/models/bulk_imports/failure_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::Failure, type: :model do +RSpec.describe BulkImports::Failure, type: :model, feature_category: :importers do let(:failure) { create(:bulk_import_failure) } describe 'associations' do @@ -44,4 +44,18 @@ RSpec.describe BulkImports::Failure, type: :model do end end end + + describe '#exception_message=' do + it 'filters file paths' do + failure = described_class.new + failure.exception_message = 'Failed to read /FILE/PATH' + expect(failure.exception_message).to eq('Failed to read [FILTERED]') + end + + it 'truncates long string' do + failure = described_class.new + failure.exception_message = 'A' * 1000 + expect(failure.exception_message.size).to eq(255) + end + end end diff --git a/spec/models/ci/build_dependencies_spec.rb b/spec/models/ci/build_dependencies_spec.rb index ab32234eba3..f80af7b9dbc 100644 --- a/spec/models/ci/build_dependencies_spec.rb +++ b/spec/models/ci/build_dependencies_spec.rb @@ -107,7 +107,7 @@ RSpec.describe Ci::BuildDependencies, feature_category: :continuous_integration end context 'when dependencies are defined' do - let(:dependencies) { %w(rspec staging) } + let(:dependencies) { %w[rspec staging] } it { is_expected.to contain_exactly(rspec_test, staging) } end @@ -137,7 +137,7 @@ RSpec.describe Ci::BuildDependencies, feature_category: :continuous_integration end context 'when needs and dependencies are defined' do - let(:dependencies) { %w(rspec staging) } + let(:dependencies) { %w[rspec staging] } let(:needs) do [ { name: 'build', artifacts: true }, @@ -150,7 +150,7 @@ RSpec.describe Ci::BuildDependencies, feature_category: :continuous_integration end context 'when needs and dependencies contradict' do - let(:dependencies) { %w(rspec staging) } + let(:dependencies) { %w[rspec staging] } let(:needs) do [ { name: 'build', artifacts: true }, diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2a5d781edc7..2e552c8d524 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -87,7 +87,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def describe 'status' do context 'when transitioning to any state from running' do it 'removes runner_session' do - %w(success drop cancel).each do |event| + %w[success drop cancel].each do |event| build = FactoryBot.create(:ci_build, :running, :with_runner_session, pipeline: pipeline) build.fire_events!(event) @@ -1090,7 +1090,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def let(:options_with_fallback_keys) do { cache: [ - { key: "key", paths: ["public"], policy: "pull-push", fallback_keys: %w(key1 key2) } + { key: "key", paths: ["public"], policy: "pull-push", fallback_keys: %w[key1 key2] } ] } end @@ -1111,8 +1111,8 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def let(:options_with_fallback_keys) do { cache: [ - { key: "key", paths: ["public"], policy: "pull-push", fallback_keys: %w(key3 key4) }, - { key: "key2", paths: ["public"], policy: "pull-push", fallback_keys: %w(key5 key6) } + { key: "key", paths: ["public"], policy: "pull-push", fallback_keys: %w[key3 key4] }, + { key: "key2", paths: ["public"], policy: "pull-push", fallback_keys: %w[key5 key6] } ] } end @@ -1214,11 +1214,11 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def is_expected.to match([ a_hash_including({ key: 'key-1', - fallback_keys: %w(key3-1 key4-1) + fallback_keys: %w[key3-1 key4-1] }), a_hash_including({ key: 'key2-1', - fallback_keys: %w(key5-1 key6-1) + fallback_keys: %w[key5-1 key6-1] }) ]) end @@ -1241,11 +1241,11 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def is_expected.to match([ a_hash_including({ key: 'key-1', - fallback_keys: %w(key3-1 key4-1) + fallback_keys: %w[key3-1 key4-1] }), a_hash_including({ key: 'key2-1', - fallback_keys: %w(key5-1 key6-1) + fallback_keys: %w[key5-1 key6-1] }) ]) end @@ -1320,7 +1320,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def allow(build).to receive(:options).and_return({ cache: [{ key: "key1", - fallback_keys: %w(key2) + fallback_keys: %w[key2] }] }) end @@ -2230,7 +2230,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end context 'when artifacts do not expire' do - it { is_expected.to eq(false) } + it { is_expected.to be_falsey } end context 'when artifacts expire in the future' do @@ -2951,7 +2951,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end context 'when runner is assigned to build' do - let(:runner) { create(:ci_runner, description: 'description', tag_list: %w(docker linux)) } + let(:runner) { create(:ci_runner, description: 'description', tag_list: %w[docker linux]) } before do build.update!(runner: runner) @@ -3952,8 +3952,8 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end context 'when have different tags' do - let(:build_tag_list) { %w(A B) } - let(:tag_list) { %w(C D) } + let(:build_tag_list) { %w[A B] } + let(:tag_list) { %w[C D] } it "does not match a build" do is_expected.not_to contain_exactly(build) @@ -3961,8 +3961,8 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end context 'when have a subset of tags' do - let(:build_tag_list) { %w(A B) } - let(:tag_list) { %w(A B C D) } + let(:build_tag_list) { %w[A B] } + let(:tag_list) { %w[A B C D] } it "does match a build" do is_expected.to contain_exactly(build) @@ -3971,7 +3971,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def context 'when build does not have tags' do let(:build_tag_list) { [] } - let(:tag_list) { %w(C D) } + let(:tag_list) { %w[C D] } it "does match a build" do is_expected.to contain_exactly(build) @@ -3979,8 +3979,8 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end context 'when does not have a subset of tags' do - let(:build_tag_list) { %w(A B C) } - let(:tag_list) { %w(C D) } + let(:build_tag_list) { %w[A B C] } + let(:tag_list) { %w[C D] } it "does not match a build" do is_expected.not_to contain_exactly(build) @@ -3998,7 +3998,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end context 'when does have tags' do - let(:tag_list) { %w(A B) } + let(:tag_list) { %w[A B] } it "does match a build" do is_expected.to contain_exactly(build) @@ -4676,7 +4676,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def describe '#invalid_dependencies' do let!(:pre_stage_job_valid) { create(:ci_build, :manual, pipeline: pipeline, name: 'test1', stage_idx: 0) } let!(:pre_stage_job_invalid) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test2', stage_idx: 1) } - let!(:job) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 2, options: { dependencies: %w(test1 test2) }) } + let!(:job) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 2, options: { dependencies: %w[test1 test2] }) } context 'when pipeline is locked' do before do @@ -5229,16 +5229,34 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def subject { build.doom! } let(:traits) { [] } - let(:build) { create(:ci_build, *traits, pipeline: pipeline) } + let(:build) do + travel(-1.minute) do + create(:ci_build, *traits, pipeline: pipeline) + end + end - it 'updates status and failure_reason', :aggregate_failures do - subject + it 'updates status, failure_reason, finished_at and updated_at', :aggregate_failures do + old_timestamp = build.updated_at + new_timestamp = \ + freeze_time do + Time.current.tap do + subject + end + end + + expect(old_timestamp).not_to eq(new_timestamp) + expect(build.updated_at).to eq(new_timestamp) + expect(build.finished_at).to eq(new_timestamp) expect(build.status).to eq("failed") expect(build.failure_reason).to eq("data_integrity_failure") end - it 'logs a message' do + it 'logs a message and increments the job failure counter', :aggregate_failures do + expect(::Gitlab::Ci::Pipeline::Metrics.job_failure_reason_counter) + .to(receive(:increment)) + .with(reason: :data_integrity_failure) + expect(Gitlab::AppLogger) .to receive(:info) .with(a_hash_including(message: 'Build doomed', class: build.class.name, build_id: build.id)) @@ -5273,12 +5291,20 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def context 'with running builds' do let(:traits) { [:picked] } - it 'drops associated runtime metadata' do + it 'drops associated runtime metadata', :aggregate_failures do subject expect(build.reload.runtime_metadata).not_to be_present end end + + context 'finished builds' do + let(:traits) { [:finished] } + + it 'does not update finished_at' do + expect { subject }.not_to change { build.reload.finished_at } + end + end end it 'does not generate cross DB queries when a record is created via FactoryBot' do diff --git a/spec/models/ci/build_trace_chunks/redis_spec.rb b/spec/models/ci/build_trace_chunks/redis_spec.rb index 0d8cda7b3d8..25a24f4946b 100644 --- a/spec/models/ci/build_trace_chunks/redis_spec.rb +++ b/spec/models/ci/build_trace_chunks/redis_spec.rb @@ -4,223 +4,8 @@ require 'spec_helper' RSpec.describe Ci::BuildTraceChunks::Redis, :clean_gitlab_redis_shared_state do let(:data_store) { described_class.new } + let(:store_trait_with_data) { :redis_with_data } + let(:store_trait_without_data) { :redis_without_data } - describe '#data' do - subject { data_store.data(model) } - - context 'when data exists' do - let(:model) { create(:ci_build_trace_chunk, :redis_with_data, initial_data: 'sample data in redis') } - - it 'returns the data' do - is_expected.to eq('sample data in redis') - end - end - - context 'when data does not exist' do - let(:model) { create(:ci_build_trace_chunk, :redis_without_data) } - - it 'returns nil' do - is_expected.to be_nil - end - end - end - - describe '#set_data' do - subject { data_store.set_data(model, data) } - - let(:data) { 'abc123' } - - context 'when data exists' do - let(:model) { create(:ci_build_trace_chunk, :redis_with_data, initial_data: 'sample data in redis') } - - it 'overwrites data' do - expect(data_store.data(model)).to eq('sample data in redis') - - subject - - expect(data_store.data(model)).to eq('abc123') - end - end - - context 'when data does not exist' do - let(:model) { create(:ci_build_trace_chunk, :redis_without_data) } - - it 'sets new data' do - expect(data_store.data(model)).to be_nil - - subject - - expect(data_store.data(model)).to eq('abc123') - end - end - end - - describe '#append_data' do - context 'when valid offset is used with existing data' do - let(:model) { create(:ci_build_trace_chunk, :redis_with_data, initial_data: 'abcd') } - - it 'appends data' do - expect(data_store.data(model)).to eq('abcd') - - length = data_store.append_data(model, '12345', 4) - - expect(length).to eq 9 - expect(data_store.data(model)).to eq('abcd12345') - end - end - - context 'when data does not exist yet' do - let(:model) { create(:ci_build_trace_chunk, :redis_without_data) } - - it 'sets new data' do - expect(data_store.data(model)).to be_nil - - length = data_store.append_data(model, 'abc', 0) - - expect(length).to eq 3 - expect(data_store.data(model)).to eq('abc') - end - end - - context 'when data needs to be truncated' do - let(:model) { create(:ci_build_trace_chunk, :redis_with_data, initial_data: '12345678') } - - it 'appends data and truncates stored value' do - expect(data_store.data(model)).to eq('12345678') - - length = data_store.append_data(model, 'ab', 4) - - expect(length).to eq 6 - expect(data_store.data(model)).to eq('1234ab') - end - end - - context 'when invalid offset is provided' do - let(:model) { create(:ci_build_trace_chunk, :redis_with_data, initial_data: 'abc') } - - it 'raises an exception' do - length = data_store.append_data(model, '12345', 4) - - expect(length).to be_negative - end - end - - context 'when trace contains multi-byte UTF8 characters' do - let(:model) { create(:ci_build_trace_chunk, :redis_with_data, initial_data: 'aüc') } - - it 'appends data' do - length = data_store.append_data(model, '1234', 4) - - data_store.data(model).then do |new_data| - expect(new_data.bytesize).to eq 8 - expect(new_data).to eq 'aüc1234' - end - - expect(length).to eq 8 - end - end - - context 'when trace contains non-UTF8 characters' do - let(:model) { create(:ci_build_trace_chunk, :redis_with_data, initial_data: "a\255c") } - - it 'appends data' do - length = data_store.append_data(model, '1234', 3) - - data_store.data(model).then do |new_data| - expect(new_data.bytesize).to eq 7 - end - - expect(length).to eq 7 - end - end - end - - describe '#delete_data' do - subject { data_store.delete_data(model) } - - context 'when data exists' do - let(:model) { create(:ci_build_trace_chunk, :redis_with_data, initial_data: 'sample data in redis') } - - it 'deletes data' do - expect(data_store.data(model)).to eq('sample data in redis') - - subject - - expect(data_store.data(model)).to be_nil - end - end - - context 'when data does not exist' do - let(:model) { create(:ci_build_trace_chunk, :redis_without_data) } - - it 'does nothing' do - expect(data_store.data(model)).to be_nil - - subject - - expect(data_store.data(model)).to be_nil - end - end - end - - describe '#size' do - context 'when data exists' do - let(:model) { create(:ci_build_trace_chunk, :redis_with_data, initial_data: 'üabcd') } - - it 'returns data bytesize correctly' do - expect(data_store.size(model)).to eq 6 - end - end - - context 'when data does not exist' do - let(:model) { create(:ci_build_trace_chunk, :redis_without_data) } - - it 'returns zero' do - expect(data_store.size(model)).to be_zero - end - end - end - - describe '#keys' do - subject { data_store.keys(relation) } - - let(:build) { create(:ci_build) } - let(:relation) { build.trace_chunks } - - before do - create(:ci_build_trace_chunk, :redis_with_data, chunk_index: 0, build: build) - create(:ci_build_trace_chunk, :redis_with_data, chunk_index: 1, build: build) - end - - it 'returns keys' do - is_expected.to eq([[build.id, 0], [build.id, 1]]) - end - end - - describe '#delete_keys' do - subject { data_store.delete_keys(keys) } - - let(:build) { create(:ci_build) } - let(:relation) { build.trace_chunks } - let(:keys) { data_store.keys(relation) } - - before do - create(:ci_build_trace_chunk, :redis_with_data, chunk_index: 0, build: build) - create(:ci_build_trace_chunk, :redis_with_data, chunk_index: 1, build: build) - end - - it 'deletes multiple data' do - Gitlab::Redis::SharedState.with do |redis| - expect(redis.exists?("gitlab:ci:trace:#{build.id}:chunks:0")).to eq(true) - expect(redis.exists?("gitlab:ci:trace:#{build.id}:chunks:1")).to eq(true) - end - - subject - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.exists?("gitlab:ci:trace:#{build.id}:chunks:0")).to eq(false) - expect(redis.exists?("gitlab:ci:trace:#{build.id}:chunks:1")).to eq(false) - end - end - end + it_behaves_like 'CI build trace chunk redis', Gitlab::Redis::SharedState end diff --git a/spec/models/ci/build_trace_chunks/redis_trace_chunks_spec.rb b/spec/models/ci/build_trace_chunks/redis_trace_chunks_spec.rb new file mode 100644 index 00000000000..eb2226e39ca --- /dev/null +++ b/spec/models/ci/build_trace_chunks/redis_trace_chunks_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::BuildTraceChunks::RedisTraceChunks, :clean_gitlab_redis_trace_chunks, + feature_category: :continuous_integration do + let(:data_store) { described_class.new } + let(:store_trait_with_data) { :redis_trace_chunks_with_data } + let(:store_trait_without_data) { :redis_trace_chunks_without_data } + + it_behaves_like 'CI build trace chunk redis', Gitlab::Redis::TraceChunks +end diff --git a/spec/models/ci/catalog/components_project_spec.rb b/spec/models/ci/catalog/components_project_spec.rb index d7e0ee2079c..79e1a113e47 100644 --- a/spec/models/ci/catalog/components_project_spec.rb +++ b/spec/models/ci/catalog/components_project_spec.rb @@ -5,32 +5,29 @@ require 'spec_helper' RSpec.describe Ci::Catalog::ComponentsProject, feature_category: :pipeline_composition do using RSpec::Parameterized::TableSyntax - let_it_be(:files) do - { - 'templates/secret-detection.yml' => "spec:\n inputs:\n website:\n---\nimage: alpine_1", - 'templates/dast/template.yml' => 'image: alpine_2', - 'templates/template.yml' => 'image: alpine_3', - 'templates/blank-yaml.yml' => '', - 'templates/dast/sub-folder/template.yml' => 'image: alpine_4', - 'tests/test.yml' => 'image: alpine_5', - 'README.md' => 'Read me' - } - end - - let_it_be(:project) do - create( - :project, :custom_repo, - description: 'Simple, complex, and other components', - files: files - ) - end - + let_it_be(:project) { create(:project, :catalog_resource_with_components) } let_it_be(:catalog_resource) { create(:ci_catalog_resource, project: project) } let(:components_project) { described_class.new(project, project.default_branch) } describe '#fetch_component_paths' do - it 'retrieves all the paths for valid components' do + context 'when there are invalid paths' do + let(:project) do + create(:project, :small_repo, description: 'description', + files: { 'templates/secrets.yml' => '', + 'tests/test.yml' => 'this is invalid', + 'README.md' => 'this is not ok' } + ) + end + + it 'does not retrieve the invalid path(s) and only retrieves the valid one(s)' do + paths = components_project.fetch_component_paths(project.default_branch) + + expect(paths).to contain_exactly('templates/secrets.yml') + end + end + + it 'retrieves all the valid paths for components' do paths = components_project.fetch_component_paths(project.default_branch) expect(paths).to contain_exactly( @@ -38,6 +35,12 @@ RSpec.describe Ci::Catalog::ComponentsProject, feature_category: :pipeline_compo 'templates/template.yml' ) end + + it 'does not fetch more paths than the limit' do + paths = components_project.fetch_component_paths(project.default_branch, limit: 1) + + expect(paths.size).to eq(1) + end end describe '#extract_component_name' do @@ -50,7 +53,11 @@ RSpec.describe Ci::Catalog::ComponentsProject, feature_category: :pipeline_compo context 'with valid component paths' do where(:path, :name) do 'templates/secret-detection.yml' | 'secret-detection' + 'templates/secret_detection.yml' | 'secret_detection' + 'templates/secret_detection123.yml' | 'secret_detection123' + 'templates/secret-detection-123.yml' | 'secret-detection-123' 'templates/dast/template.yml' | 'dast' + 'templates/d-a-s_t/template.yml' | 'd-a-s_t' 'templates/template.yml' | 'template' 'templates/blank-yaml.yml' | 'blank-yaml' end diff --git a/spec/models/ci/catalog/listing_spec.rb b/spec/models/ci/catalog/listing_spec.rb index 7524d908252..7a1e12165ac 100644 --- a/spec/models/ci/catalog/listing_spec.rb +++ b/spec/models/ci/catalog/listing_spec.rb @@ -4,109 +4,179 @@ require 'spec_helper' RSpec.describe Ci::Catalog::Listing, feature_category: :pipeline_composition do let_it_be(:namespace) { create(:group) } - let_it_be(:project_1) { create(:project, namespace: namespace, name: 'X Project') } - let_it_be(:project_2) { create(:project, namespace: namespace, name: 'B Project') } - let_it_be(:project_3) { create(:project, namespace: namespace, name: 'A Project') } - let_it_be(:project_4) { create(:project) } + let_it_be(:project_x) { create(:project, namespace: namespace, name: 'X Project') } + let_it_be(:project_a) { create(:project, :public, namespace: namespace, name: 'A Project') } + let_it_be(:project_noaccess) { create(:project, namespace: namespace, name: 'C Project') } + let_it_be(:project_ext) { create(:project, :public, name: 'TestProject') } let_it_be(:user) { create(:user) } - let(:list) { described_class.new(namespace, user) } + let_it_be(:project_b) do + create(:project, namespace: namespace, name: 'B Project', description: 'Rspec test framework') + end - describe '#new' do - context 'when namespace is not a root namespace' do - let(:namespace) { create(:group, :nested) } + let(:list) { described_class.new(user) } - it 'raises an exception' do - expect { list }.to raise_error(ArgumentError, 'Namespace is not a root namespace') - end - end + before_all do + project_x.add_reporter(user) + project_b.add_reporter(user) + project_a.add_reporter(user) + project_ext.add_reporter(user) end describe '#resources' do - subject(:resources) { list.resources } + subject(:resources) { list.resources(**params) } + + context 'when user is anonymous' do + let(:user) { nil } + let(:params) { {} } + + let!(:resource_1) { create(:ci_catalog_resource, project: project_a) } + let!(:resource_2) { create(:ci_catalog_resource, project: project_ext) } + let!(:resource_3) { create(:ci_catalog_resource, project: project_b) } + + it 'returns only resources for public projects' do + is_expected.to contain_exactly(resource_1, resource_2) + end + + context 'when sorting is provided' do + let(:params) { { sort: :name_desc } } + + it 'returns only resources for public projects sorted by name DESC' do + is_expected.to contain_exactly(resource_2, resource_1) + end + end + end + + context 'when search params are provided' do + let(:params) { { search: 'test' } } + + let!(:resource_1) { create(:ci_catalog_resource, project: project_a) } + let!(:resource_2) { create(:ci_catalog_resource, project: project_ext) } + let!(:resource_3) { create(:ci_catalog_resource, project: project_b) } - context 'when the user has access to all projects in the namespace' do - before do - namespace.add_developer(user) + it 'returns the resources that match the search params' do + is_expected.to contain_exactly(resource_2, resource_3) end - context 'when the namespace has no catalog resources' do + context 'when search term is too small' do + let(:params) { { search: 'te' } } + it { is_expected.to be_empty } end + end - context 'when the namespace has catalog resources' do - let_it_be(:today) { Time.zone.now } - let_it_be(:yesterday) { today - 1.day } - let_it_be(:tomorrow) { today + 1.day } + context 'when namespace is provided' do + let(:params) { { namespace: namespace } } - let_it_be(:resource) { create(:ci_catalog_resource, project: project_1, latest_released_at: yesterday) } - let_it_be(:resource_2) { create(:ci_catalog_resource, project: project_2, latest_released_at: today) } - let_it_be(:resource_3) { create(:ci_catalog_resource, project: project_3, latest_released_at: nil) } + context 'when namespace is not a root namespace' do + let(:namespace) { create(:group, :nested) } - let_it_be(:other_namespace_resource) do - create(:ci_catalog_resource, project: project_4, latest_released_at: tomorrow) + it 'raises an exception' do + expect { resources }.to raise_error(ArgumentError, 'Namespace is not a root namespace') end + end - it 'contains only catalog resources for projects in that namespace' do - is_expected.to contain_exactly(resource, resource_2, resource_3) + context 'when the user has access to all projects in the namespace' do + context 'when the namespace has no catalog resources' do + it { is_expected.to be_empty } end - context 'with a sort parameter' do - subject(:resources) { list.resources(sort: sort) } + context 'when the namespace has catalog resources' do + let_it_be(:today) { Time.zone.now } + let_it_be(:yesterday) { today - 1.day } + let_it_be(:tomorrow) { today + 1.day } - context 'when the sort is name ascending' do - let_it_be(:sort) { :name_asc } + let_it_be(:resource_1) do + create(:ci_catalog_resource, project: project_x, latest_released_at: yesterday, created_at: today) + end - it 'contains catalog resources for projects sorted by name ascending' do - is_expected.to eq([resource_3, resource_2, resource]) - end + let_it_be(:resource_2) do + create(:ci_catalog_resource, project: project_b, latest_released_at: today, created_at: yesterday) end - context 'when the sort is name descending' do - let_it_be(:sort) { :name_desc } + let_it_be(:resource_3) do + create(:ci_catalog_resource, project: project_a, latest_released_at: nil, created_at: tomorrow) + end - it 'contains catalog resources for projects sorted by name descending' do - is_expected.to eq([resource, resource_2, resource_3]) - end + let_it_be(:other_namespace_resource) do + create(:ci_catalog_resource, project: project_ext, latest_released_at: tomorrow) end - context 'when the sort is latest_released_at ascending' do - let_it_be(:sort) { :latest_released_at_asc } + it 'contains only catalog resources for projects in that namespace' do + is_expected.to contain_exactly(resource_1, resource_2, resource_3) + end - it 'contains catalog resources sorted by latest_released_at ascending with nulls last' do - is_expected.to eq([resource, resource_2, resource_3]) + context 'with a sort parameter' do + let(:params) { { namespace: namespace, sort: sort } } + + context 'when the sort is created_at ascending' do + let_it_be(:sort) { :created_at_asc } + + it 'contains catalog resources sorted by created_at ascending' do + is_expected.to eq([resource_2, resource_1, resource_3]) + end + end + + context 'when the sort is created_at descending' do + let_it_be(:sort) { :created_at_desc } + + it 'contains catalog resources sorted by created_at descending' do + is_expected.to eq([resource_3, resource_1, resource_2]) + end end - end - context 'when the sort is latest_released_at descending' do - let_it_be(:sort) { :latest_released_at_desc } + context 'when the sort is name ascending' do + let_it_be(:sort) { :name_asc } - it 'contains catalog resources sorted by latest_released_at descending with nulls last' do - is_expected.to eq([resource_2, resource, resource_3]) + it 'contains catalog resources for projects sorted by name ascending' do + is_expected.to eq([resource_3, resource_2, resource_1]) + end + end + + context 'when the sort is name descending' do + let_it_be(:sort) { :name_desc } + + it 'contains catalog resources for projects sorted by name descending' do + is_expected.to eq([resource_1, resource_2, resource_3]) + end + end + + context 'when the sort is latest_released_at ascending' do + let_it_be(:sort) { :latest_released_at_asc } + + it 'contains catalog resources sorted by latest_released_at ascending with nulls last' do + is_expected.to eq([resource_1, resource_2, resource_3]) + end + end + + context 'when the sort is latest_released_at descending' do + let_it_be(:sort) { :latest_released_at_desc } + + it 'contains catalog resources sorted by latest_released_at descending with nulls last' do + is_expected.to eq([resource_2, resource_1, resource_3]) + end end end end end - end - context 'when the user only has access to some projects in the namespace' do - let!(:resource_1) { create(:ci_catalog_resource, project: project_1) } - let!(:resource_2) { create(:ci_catalog_resource, project: project_2) } + context 'when the user only has access to some projects in the namespace' do + let!(:accessible_resource) { create(:ci_catalog_resource, project: project_x) } + let!(:inaccessible_resource) { create(:ci_catalog_resource, project: project_noaccess) } - before do - project_1.add_developer(user) - project_2.add_guest(user) + it 'only returns catalog resources for projects the user has access to' do + is_expected.to contain_exactly(accessible_resource) + end end - it 'only returns catalog resources for projects the user has access to' do - is_expected.to contain_exactly(resource_1) - end - end + context 'when the user does not have access to the namespace' do + let!(:project) { create(:project) } + let!(:resource) { create(:ci_catalog_resource, project: project) } - context 'when the user does not have access to the namespace' do - let!(:resource) { create(:ci_catalog_resource, project: project_1) } + let(:namespace) { project.namespace } - it { is_expected.to be_empty } + it { is_expected.to be_empty } + end end end end diff --git a/spec/models/ci/catalog/resource_spec.rb b/spec/models/ci/catalog/resource_spec.rb index 4ce1433e015..098772b1ea9 100644 --- a/spec/models/ci/catalog/resource_spec.rb +++ b/spec/models/ci/catalog/resource_spec.rb @@ -7,10 +7,10 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do let_it_be(:yesterday) { today - 1.day } let_it_be(:tomorrow) { today + 1.day } - let_it_be(:project) { create(:project, name: 'A') } + let_it_be_with_reload(:project) { create(:project, name: 'A') } let_it_be(:project_2) { build(:project, name: 'Z') } - let_it_be(:project_3) { build(:project, name: 'L') } - let_it_be(:resource) { create(:ci_catalog_resource, project: project, latest_released_at: tomorrow) } + let_it_be(:project_3) { build(:project, name: 'L', description: 'Z') } + let_it_be_with_reload(:resource) { create(:ci_catalog_resource, project: project, latest_released_at: tomorrow) } let_it_be(:resource_2) { create(:ci_catalog_resource, project: project_2, latest_released_at: today) } let_it_be(:resource_3) { create(:ci_catalog_resource, project: project_3, latest_released_at: nil) } @@ -19,12 +19,16 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do let_it_be(:release3) { create(:release, project: project, released_at: tomorrow) } it { is_expected.to belong_to(:project) } - it { is_expected.to have_many(:components).class_name('Ci::Catalog::Resources::Component') } + + it do + is_expected.to( + have_many(:components).class_name('Ci::Catalog::Resources::Component').with_foreign_key(:catalog_resource_id) + ) + end + it { is_expected.to have_many(:versions).class_name('Ci::Catalog::Resources::Version') } it { is_expected.to delegate_method(:avatar_path).to(:project) } - it { is_expected.to delegate_method(:description).to(:project) } - it { is_expected.to delegate_method(:name).to(:project) } it { is_expected.to delegate_method(:star_count).to(:project) } it { is_expected.to delegate_method(:forks_count).to(:project) } @@ -38,6 +42,14 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do end end + describe '.search' do + it 'returns catalog resources whose name or description match the search term' do + resources = described_class.search('Z') + + expect(resources).to contain_exactly(resource_2, resource_3) + end + end + describe '.order_by_created_at_desc' do it 'returns catalog resources sorted by descending created at' do ordered_resources = described_class.order_by_created_at_desc @@ -46,20 +58,40 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do end end + describe '.order_by_created_at_asc' do + it 'returns catalog resources sorted by ascending created at' do + ordered_resources = described_class.order_by_created_at_asc + + expect(ordered_resources.to_a).to eq([resource, resource_2, resource_3]) + end + end + describe '.order_by_name_desc' do - it 'returns catalog resources sorted by descending name' do - ordered_resources = described_class.order_by_name_desc + subject(:ordered_resources) { described_class.order_by_name_desc } + it 'returns catalog resources sorted by descending name' do expect(ordered_resources.pluck(:name)).to eq(%w[Z L A]) end + + it 'returns catalog resources sorted by descending name with nulls last' do + resource.update!(name: nil) + + expect(ordered_resources.pluck(:name)).to eq(['Z', 'L', nil]) + end end describe '.order_by_name_asc' do - it 'returns catalog resources sorted by ascending name' do - ordered_resources = described_class.order_by_name_asc + subject(:ordered_resources) { described_class.order_by_name_asc } + it 'returns catalog resources sorted by ascending name' do expect(ordered_resources.pluck(:name)).to eq(%w[A L Z]) end + + it 'returns catalog resources sorted by ascending name with nulls last' do + resource.update!(name: nil) + + expect(ordered_resources.pluck(:name)).to eq(['L', 'Z', nil]) + end end describe '.order_by_latest_released_at_desc' do @@ -78,21 +110,92 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do end end - describe '#versions' do - it 'returns releases ordered by released date descending' do - expect(resource.versions).to eq([release3, release2, release1]) + describe '#state' do + it 'defaults to draft' do + expect(resource.state).to eq('draft') end end - describe '#latest_version' do - it 'returns the latest release' do - expect(resource.latest_version).to eq(release3) + describe '#publish!' do + context 'when the catalog resource is in draft state' do + it 'updates the state of the catalog resource to published' do + expect(resource.state).to eq('draft') + + resource.publish! + + expect(resource.reload.state).to eq('published') + end + end + + context 'when a catalog resource already has a published state' do + it 'leaves the state as published' do + resource.update!(state: 'published') + + resource.publish! + + expect(resource.state).to eq('published') + end end end - describe '#state' do - it 'defaults to draft' do - expect(resource.state).to eq('draft') + describe '#unpublish!' do + context 'when the catalog resource is in published state' do + it 'updates the state to draft' do + resource.update!(state: :published) + expect(resource.state).to eq('published') + + resource.unpublish! + + expect(resource.reload.state).to eq('draft') + end + end + + context 'when the catalog resource is already in draft state' do + it 'leaves the state as draft' do + expect(resource.state).to eq('draft') + + resource.unpublish! + + expect(resource.reload.state).to eq('draft') + end + end + end + + describe 'sync with project' do + shared_examples 'denormalized columns of the catalog resource match the project' do + it do + expect(resource.name).to eq(project.name) + expect(resource.description).to eq(project.description) + expect(resource.visibility_level).to eq(project.visibility_level) + end + end + + context 'when the catalog resource is created' do + it_behaves_like 'denormalized columns of the catalog resource match the project' + end + + context 'when the project name is updated' do + before do + project.update!(name: 'My new project name') + end + + it_behaves_like 'denormalized columns of the catalog resource match the project' + end + + context 'when the project description is updated' do + before do + project.update!(description: 'My new description') + end + + it_behaves_like 'denormalized columns of the catalog resource match the project' + end + + context 'when the project visibility_level is updated' do + before do + project.update!(visibility_level: 10) + end + + it_behaves_like 'denormalized columns of the catalog resource match the project' end end end diff --git a/spec/models/ci/catalog/resources/component_spec.rb b/spec/models/ci/catalog/resources/component_spec.rb index e8c92ce0788..2ee91175920 100644 --- a/spec/models/ci/catalog/resources/component_spec.rb +++ b/spec/models/ci/catalog/resources/component_spec.rb @@ -9,6 +9,23 @@ RSpec.describe Ci::Catalog::Resources::Component, type: :model, feature_category it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:version).class_name('Ci::Catalog::Resources::Version') } + it_behaves_like 'a BulkInsertSafe model', described_class do + let_it_be(:project) { create(:project, :readme, description: 'project description') } + let_it_be(:catalog_resource) { create(:ci_catalog_resource, project: project) } + let_it_be(:version) { create(:ci_catalog_resource_version, project: project) } + + let(:valid_items_for_bulk_insertion) do + build_list(:ci_catalog_resource_component, 10) do |component| + component.catalog_resource = catalog_resource + component.version = version + component.project = project + component.created_at = Time.zone.now + end + end + + let(:invalid_items_for_bulk_insertion) { [] } + end + describe 'validations' do it { is_expected.to validate_presence_of(:catalog_resource) } it { is_expected.to validate_presence_of(:project) } diff --git a/spec/models/ci/catalog/resources/version_spec.rb b/spec/models/ci/catalog/resources/version_spec.rb index e93176e466a..7114d2b6709 100644 --- a/spec/models/ci/catalog/resources/version_spec.rb +++ b/spec/models/ci/catalog/resources/version_spec.rb @@ -3,14 +3,105 @@ require 'spec_helper' RSpec.describe Ci::Catalog::Resources::Version, type: :model, feature_category: :pipeline_composition do + include_context 'when there are catalog resources with versions' + it { is_expected.to belong_to(:release) } it { is_expected.to belong_to(:catalog_resource).class_name('Ci::Catalog::Resource') } it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:components).class_name('Ci::Catalog::Resources::Component') } + it { is_expected.to delegate_method(:name).to(:release) } + it { is_expected.to delegate_method(:description).to(:release) } + it { is_expected.to delegate_method(:tag).to(:release) } + it { is_expected.to delegate_method(:sha).to(:release) } + it { is_expected.to delegate_method(:released_at).to(:release) } + it { is_expected.to delegate_method(:author_id).to(:release) } + describe 'validations' do it { is_expected.to validate_presence_of(:release) } it { is_expected.to validate_presence_of(:catalog_resource) } it { is_expected.to validate_presence_of(:project) } end + + describe '.for_catalog resources' do + it 'returns versions for the given catalog resources' do + versions = described_class.for_catalog_resources([resource1, resource2]) + + expect(versions).to match_array([v1_0, v1_1, v2_0, v2_1]) + 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 + + expect(versions).to eq([v2_1, v2_0, v1_1, v1_0]) + end + end + + describe '.order_by_created_at_desc' do + it 'returns versions ordered by created_at descending' do + versions = described_class.order_by_created_at_desc + + expect(versions).to eq([v1_0, v1_1, v2_0, v2_1]) + end + end + + describe '.order_by_released_at_asc' do + it 'returns versions ordered by released_at ascending' do + versions = described_class.order_by_released_at_asc + + expect(versions).to eq([v1_0, v1_1, v2_0, v2_1]) + end + end + + describe '.order_by_released_at_desc' do + it 'returns versions ordered by released_at descending' do + versions = described_class.order_by_released_at_desc + + expect(versions).to eq([v2_1, v2_0, v1_1, v1_0]) + end + end + + describe '.latest' do + subject { described_class.latest } + + it 'returns the latest version by released date' do + is_expected.to eq(v2_1) + end + + context 'when there are no versions' do + it 'returns nil' do + resource1.versions.delete_all(:delete_all) + resource2.versions.delete_all(:delete_all) + + is_expected.to be_nil + end + end + end + + describe '.latest_for_catalog resources' do + subject { described_class.latest_for_catalog_resources([resource1, resource2]) } + + it 'returns the latest version for each catalog resource' do + is_expected.to match_array([v1_1, v2_1]) + end + + context 'when one catalog resource does not have versions' do + it 'returns the latest version of only the catalog resource with versions' do + resource1.versions.delete_all(:delete_all) + + is_expected.to match_array([v2_1]) + end + end + + context 'when no catalog resource has versions' do + it 'returns empty response' do + resource1.versions.delete_all(:delete_all) + resource2.versions.delete_all(:delete_all) + + is_expected.to be_empty + end + end + end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 498af80dbb6..48d46824c11 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -207,7 +207,7 @@ RSpec.describe Ci::JobArtifact, feature_category: :build_artifacts do subject { described_class.associated_file_types_for(file_type) } where(:file_type, :result) do - 'codequality' | %w(codequality) + 'codequality' | %w[codequality] 'quality' | nil end diff --git a/spec/models/ci/job_token/scope_spec.rb b/spec/models/ci/job_token/scope_spec.rb index 7aa861a3dab..d41286f5a45 100644 --- a/spec/models/ci/job_token/scope_spec.rb +++ b/spec/models/ci/job_token/scope_spec.rb @@ -88,24 +88,6 @@ RSpec.describe Ci::JobToken::Scope, feature_category: :continuous_integration, f end end - RSpec.shared_examples 'enforces outbound scope only' do - include_context 'with accessible and inaccessible projects' - - where(:accessed_project, :result) do - ref(:current_project) | true - ref(:inbound_allowlist_project) | false - ref(:unscoped_project1) | false - ref(:unscoped_project2) | false - ref(:outbound_allowlist_project) | true - ref(:inbound_accessible_project) | false - ref(:fully_accessible_project) | true - end - - with_them do - it { is_expected.to eq(result) } - end - end - describe 'accessible?' do subject { scope.accessible?(accessed_project) } @@ -121,6 +103,7 @@ RSpec.describe Ci::JobToken::Scope, feature_category: :continuous_integration, f ref(:outbound_allowlist_project) | false ref(:inbound_accessible_project) | false ref(:fully_accessible_project) | true + ref(:unscoped_public_project) | false end with_them do @@ -147,6 +130,7 @@ RSpec.describe Ci::JobToken::Scope, feature_category: :continuous_integration, f ref(:outbound_allowlist_project) | false ref(:inbound_accessible_project) | true ref(:fully_accessible_project) | true + ref(:unscoped_public_project) | false end with_them do @@ -160,7 +144,34 @@ RSpec.describe Ci::JobToken::Scope, feature_category: :continuous_integration, f current_project.update!(ci_outbound_job_token_scope_enabled: true) end - include_examples 'enforces outbound scope only' + include_context 'with accessible and inaccessible projects' + + where(:accessed_project, :result) do + ref(:current_project) | true + ref(:inbound_allowlist_project) | false + ref(:unscoped_project1) | false + ref(:unscoped_project2) | false + ref(:outbound_allowlist_project) | true + ref(:inbound_accessible_project) | false + ref(:fully_accessible_project) | true + ref(:unscoped_public_project) | true + end + + with_them do + it { is_expected.to eq(result) } + end + + context "with FF restrict_ci_job_token_for_public_and_internal_projects disabled" do + before do + stub_feature_flags(restrict_ci_job_token_for_public_and_internal_projects: false) + end + + let(:accessed_project) { unscoped_public_project } + + it "restricts public and internal outbound projects not in allowlist" do + is_expected.to eq(false) + end + end end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 887ec48ec8f..9696ba7b3ee 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -157,6 +157,106 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end end + describe 'unlocking pipelines based on state transition' do + let(:ci_ref) { create(:ci_ref) } + let(:unlock_previous_pipelines_worker_spy) { class_spy(::Ci::Refs::UnlockPreviousPipelinesWorker) } + + before do + stub_const('Ci::Refs::UnlockPreviousPipelinesWorker', unlock_previous_pipelines_worker_spy) + stub_feature_flags(ci_stop_unlock_pipelines: false) + end + + shared_examples 'not unlocking pipelines' do |event:| + context "on #{event}" do + let(:pipeline) { create(:ci_pipeline, ci_ref_id: ci_ref.id, locked: :artifacts_locked) } + + it 'does not unlock previous pipelines' do + pipeline.fire_events!(event) + + expect(unlock_previous_pipelines_worker_spy).not_to have_received(:perform_async) + end + end + end + + shared_examples 'unlocking pipelines' do |event:| + context "on #{event}" do + before do + pipeline.fire_events!(event) + end + + let(:pipeline) { create(:ci_pipeline, ci_ref_id: ci_ref.id, locked: :artifacts_locked) } + + it 'unlocks previous pipelines' do + expect(unlock_previous_pipelines_worker_spy).to have_received(:perform_async).with(ci_ref.id) + end + end + end + + context 'when transitioning to unlockable states' do + before do + pipeline.run + end + + it_behaves_like 'unlocking pipelines', event: :succeed + it_behaves_like 'unlocking pipelines', event: :drop + it_behaves_like 'unlocking pipelines', event: :skip + it_behaves_like 'unlocking pipelines', event: :cancel + it_behaves_like 'unlocking pipelines', event: :block + + context 'and ci_stop_unlock_pipelines is enabled' do + before do + stub_feature_flags(ci_stop_unlock_pipelines: true) + end + + it_behaves_like 'not unlocking pipelines', event: :succeed + it_behaves_like 'not unlocking pipelines', event: :drop + it_behaves_like 'not unlocking pipelines', event: :skip + it_behaves_like 'not unlocking pipelines', event: :cancel + it_behaves_like 'not unlocking pipelines', event: :block + end + + context 'and ci_unlock_non_successful_pipelines is disabled' do + before do + stub_feature_flags(ci_unlock_non_successful_pipelines: false) + end + + it_behaves_like 'unlocking pipelines', event: :succeed + it_behaves_like 'not unlocking pipelines', event: :drop + it_behaves_like 'not unlocking pipelines', event: :skip + it_behaves_like 'not unlocking pipelines', event: :cancel + it_behaves_like 'not unlocking pipelines', event: :block + + context 'and ci_stop_unlock_pipelines is enabled' do + before do + stub_feature_flags(ci_stop_unlock_pipelines: true) + end + + it_behaves_like 'not unlocking pipelines', event: :succeed + it_behaves_like 'not unlocking pipelines', event: :drop + it_behaves_like 'not unlocking pipelines', event: :skip + it_behaves_like 'not unlocking pipelines', event: :cancel + it_behaves_like 'not unlocking pipelines', event: :block + end + end + end + + context 'when transitioning to a non-unlockable state' do + before do + pipeline.enqueue + end + + it_behaves_like 'not unlocking pipelines', event: :run + + context 'and ci_unlock_non_successful_pipelines is disabled' do + before do + stub_feature_flags(ci_unlock_non_successful_pipelines: false) + end + + it_behaves_like 'not unlocking pipelines', event: :run + end + end + end + describe 'pipeline age metric' do let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } @@ -220,6 +320,34 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end end + describe '.with_unlockable_status' do + let_it_be(:project) { create(:project) } + + let!(:pipeline) { create(:ci_pipeline, project: project, status: status) } + + subject(:result) { described_class.with_unlockable_status } + + described_class::UNLOCKABLE_STATUSES.map(&:to_s).each do |s| + context "when pipeline status is #{s}" do + let(:status) { s } + + it 'includes the pipeline in the result' do + expect(result).to include(pipeline) + end + end + end + + (Ci::HasStatus::AVAILABLE_STATUSES - described_class::UNLOCKABLE_STATUSES.map(&:to_s)).each do |s| + context "when pipeline status is #{s}" do + let(:status) { s } + + it 'does excludes the pipeline in the result' do + expect(result).not_to include(pipeline) + end + end + end + end + describe '.processables' do let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } @@ -231,7 +359,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end it 'has an association with processable CI/CD entities' do - pipeline.processables.pluck('name').yield_self do |processables| + pipeline.processables.pluck('name').then do |processables| expect(processables).to match_array %w[build bridge] end end @@ -1303,7 +1431,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: describe '#stages_names' do it 'returns a valid names of stages' do - expect(pipeline.stages_names).to eq(%w(build test deploy)) + expect(pipeline.stages_names).to eq(%w[build test deploy]) end end end @@ -1900,11 +2028,23 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end end - context 'when only_allow_merge_if_pipeline_succeeds? returns false' do + context 'when only_allow_merge_if_pipeline_succeeds? returns false and widget_pipeline_pass_subscription_update disabled' do let(:only_allow_merge_if_pipeline_succeeds?) { false } + before do + stub_feature_flags(widget_pipeline_pass_subscription_update: false) + end + it_behaves_like 'state transition not triggering GraphQL subscription mergeRequestMergeStatusUpdated' end + + context 'when only_allow_merge_if_pipeline_succeeds? returns false and widget_pipeline_pass_subscription_update enabled' do + let(:only_allow_merge_if_pipeline_succeeds?) { false } + + it_behaves_like 'triggers GraphQL subscription mergeRequestMergeStatusUpdated' do + let(:action) { pipeline.succeed } + end + end end context 'when pipeline has merge requests' do @@ -2640,7 +2780,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: subject(:latest_successful_for_refs) { described_class.latest_successful_for_refs(refs) } context 'when refs are specified' do - let(:refs) { %w(first_ref second_ref third_ref) } + let(:refs) { %w[first_ref second_ref third_ref] } before do create(:ci_empty_pipeline, id: 1001, status: :success, ref: 'first_ref', sha: 'sha') @@ -2819,7 +2959,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: subject { described_class.bridgeable_statuses } it { is_expected.to be_an(Array) } - it { is_expected.not_to include('created', 'waiting_for_resource', 'preparing', 'pending') } + it { is_expected.to contain_exactly('running', 'success', 'failed', 'canceled', 'skipped', 'manual', 'scheduled') } end describe '#status', :sidekiq_inline do @@ -3176,6 +3316,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: %i[ enqueue request_resource + wait_for_callback prepare run skip @@ -5578,25 +5719,4 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end end end - - describe '#reduced_build_attributes_list_for_rules?' do - subject { pipeline.reduced_build_attributes_list_for_rules? } - - let(:pipeline) { build_stubbed(:ci_pipeline, project: project, user: user) } - - it { is_expected.to be_truthy } - - it 'memoizes the result' do - expect { subject } - .to change { pipeline.strong_memoized?(:reduced_build_attributes_list_for_rules?) } - end - - context 'with the FF disabled' do - before do - stub_feature_flags(reduced_build_attributes_list_for_rules: false) - end - - it { is_expected.to be_falsey } - end - end end diff --git a/spec/models/ci/ref_spec.rb b/spec/models/ci/ref_spec.rb index 75071a17fa9..2727c7701b8 100644 --- a/spec/models/ci/ref_spec.rb +++ b/spec/models/ci/ref_spec.rb @@ -7,61 +7,6 @@ RSpec.describe Ci::Ref, feature_category: :continuous_integration do it { is_expected.to belong_to(:project) } - describe 'state machine transitions' do - context 'unlock artifacts transition' do - let(:ci_ref) { create(:ci_ref) } - let(:unlock_previous_pipelines_worker_spy) { class_spy(::Ci::Refs::UnlockPreviousPipelinesWorker) } - - before do - stub_const('Ci::Refs::UnlockPreviousPipelinesWorker', unlock_previous_pipelines_worker_spy) - end - - context 'pipeline is locked' do - let!(:pipeline) { create(:ci_pipeline, ci_ref_id: ci_ref.id, locked: :artifacts_locked) } - - where(:initial_state, :action, :count) do - :unknown | :succeed! | 1 - :unknown | :do_fail! | 0 - :success | :succeed! | 1 - :success | :do_fail! | 0 - :failed | :succeed! | 1 - :failed | :do_fail! | 0 - :fixed | :succeed! | 1 - :fixed | :do_fail! | 0 - :broken | :succeed! | 1 - :broken | :do_fail! | 0 - :still_failing | :succeed | 1 - :still_failing | :do_fail | 0 - end - - with_them do - context "when transitioning states" do - before do - status_value = Ci::Ref.state_machines[:status].states[initial_state].value - ci_ref.update!(status: status_value) - end - - it 'calls pipeline complete unlock artifacts service' do - ci_ref.send(action) - - expect(unlock_previous_pipelines_worker_spy).to have_received(:perform_async).exactly(count).times - end - end - end - end - - context 'pipeline is unlocked' do - let!(:pipeline) { create(:ci_pipeline, ci_ref_id: ci_ref.id, locked: :unlocked) } - - it 'does not unlock pipelines' do - ci_ref.succeed! - - expect(unlock_previous_pipelines_worker_spy).not_to have_received(:perform_async) - end - end - end - end - describe '.ensure_for' do let_it_be(:project) { create(:project, :repository) } @@ -241,4 +186,117 @@ RSpec.describe Ci::Ref, feature_category: :continuous_integration do let!(:model) { create(:ci_ref, project: parent) } end end + + describe '#last_successful_ci_source_pipeline' do + let_it_be(:ci_ref) { create(:ci_ref) } + + let(:ci_source) { Enums::Ci::Pipeline.sources[:push] } + let(:dangling_source) { Enums::Ci::Pipeline.sources[:parent_pipeline] } + + subject(:result) { ci_ref.last_successful_ci_source_pipeline } + + context 'when there are no successful CI source pipelines' do + let!(:running_ci_source) { create(:ci_pipeline, :running, ci_ref: ci_ref, source: ci_source) } + let!(:successful_dangling_source) { create(:ci_pipeline, :success, ci_ref: ci_ref, source: dangling_source) } + + it { is_expected.to be_nil } + end + + context 'when there are successful CI source pipelines' do + context 'and the latest pipeline is a successful CI source pipeline' do + let!(:failed_ci_source) { create(:ci_pipeline, :failed, ci_ref: ci_ref, source: ci_source) } + let!(:successful_dangling_source) { create(:ci_pipeline, :success, ci_ref: ci_ref, source: dangling_source, child_of: failed_ci_source) } + let!(:successful_ci_source) { create(:ci_pipeline, :success, ci_ref: ci_ref, source: ci_source) } + + it 'returns the last successful CI source pipeline' do + expect(result).to eq(successful_ci_source) + end + end + + context 'and there is a newer successful dangling source pipeline than the successful CI source pipelines' do + let!(:successful_ci_source_1) { create(:ci_pipeline, :success, ci_ref: ci_ref, source: ci_source) } + let!(:successful_ci_source_2) { create(:ci_pipeline, :success, ci_ref: ci_ref, source: ci_source) } + let!(:failed_ci_source) { create(:ci_pipeline, :failed, ci_ref: ci_ref, source: ci_source) } + let!(:successful_dangling_source) { create(:ci_pipeline, :success, ci_ref: ci_ref, source: dangling_source, child_of: failed_ci_source) } + + it 'returns the last successful CI source pipeline' do + expect(result).to eq(successful_ci_source_2) + end + + context 'and the newer successful dangling source is a child of a successful CI source pipeline' do + let!(:parent_ci_source) { create(:ci_pipeline, :success, ci_ref: ci_ref, source: ci_source) } + let!(:successful_child_source) { create(:ci_pipeline, :success, ci_ref: ci_ref, source: dangling_source, child_of: parent_ci_source) } + + it 'returns the parent pipeline instead' do + expect(result).to eq(parent_ci_source) + end + end + end + end + end + + describe '#last_unlockable_ci_source_pipeline' do + let(:ci_source) { Enums::Ci::Pipeline.sources[:push] } + let(:dangling_source) { Enums::Ci::Pipeline.sources[:parent_pipeline] } + + let_it_be(:project) { create(:project) } + let_it_be(:ci_ref) { create(:ci_ref, project: project) } + + subject(:result) { ci_ref.last_unlockable_ci_source_pipeline } + + context 'when there are unlockable pipelines in the ref' do + context 'and the last CI source pipeline in the ref is unlockable' do + let!(:unlockable_ci_source_1) { create(:ci_pipeline, :success, project: project, ci_ref: ci_ref, source: ci_source) } + let!(:unlockable_ci_source_2) { create(:ci_pipeline, :blocked, project: project, ci_ref: ci_ref, source: ci_source) } + + it 'returns the CI source pipeline' do + expect(result).to eq(unlockable_ci_source_2) + end + + context 'and it has unlockable child pipelines' do + let!(:child) { create(:ci_pipeline, :success, project: project, ci_ref: ci_ref, source: dangling_source, child_of: unlockable_ci_source_2) } + let!(:child_2) { create(:ci_pipeline, :success, project: project, ci_ref: ci_ref, source: dangling_source, child_of: unlockable_ci_source_2) } + + it 'returns the parent CI source pipeline' do + expect(result).to eq(unlockable_ci_source_2) + end + end + + context 'and it has a non-unlockable child pipeline' do + let!(:child) { create(:ci_pipeline, :running, project: project, ci_ref: ci_ref, source: dangling_source, child_of: unlockable_ci_source_2) } + + it 'returns the parent CI source pipeline' do + expect(result).to eq(unlockable_ci_source_2) + end + end + end + + context 'and the last CI source pipeline in the ref is not unlockable' do + let!(:unlockable_ci_source) { create(:ci_pipeline, :skipped, project: project, ci_ref: ci_ref, source: ci_source) } + let!(:unlockable_dangling_source) { create(:ci_pipeline, :success, project: project, ci_ref: ci_ref, source: dangling_source, child_of: unlockable_ci_source) } + let!(:non_unlockable_ci_source) { create(:ci_pipeline, :running, project: project, ci_ref: ci_ref, source: ci_source) } + let!(:non_unlockable_ci_source_2) { create(:ci_pipeline, :running, project: project, ci_ref: ci_ref, source: ci_source) } + + it 'returns the last unlockable CI source pipeline before it' do + expect(result).to eq(unlockable_ci_source) + end + + context 'and it has unlockable child pipelines' do + let!(:child) { create(:ci_pipeline, :success, project: project, ci_ref: ci_ref, source: dangling_source, child_of: non_unlockable_ci_source) } + let!(:child_2) { create(:ci_pipeline, :success, project: project, ci_ref: ci_ref, source: dangling_source, child_of: non_unlockable_ci_source) } + + it 'returns the last unlockable CI source pipeline before it' do + expect(result).to eq(unlockable_ci_source) + end + end + end + end + + context 'when there are no unlockable pipelines in the ref' do + let!(:non_unlockable_pipeline) { create(:ci_pipeline, :running, project: project, ci_ref: ci_ref, source: ci_source) } + let!(:pipeline_from_another_ref) { create(:ci_pipeline, :success, source: ci_source) } + + it { is_expected.to be_nil } + end + end end diff --git a/spec/models/ci/resource_group_spec.rb b/spec/models/ci/resource_group_spec.rb index 6d518d5c874..9e98cc884de 100644 --- a/spec/models/ci/resource_group_spec.rb +++ b/spec/models/ci/resource_group_spec.rb @@ -118,7 +118,7 @@ RSpec.describe Ci::ResourceGroup do let!(:resource_group) { create(:ci_resource_group, process_mode: process_mode, project: project) } - Ci::HasStatus::STATUSES_ENUM.keys.each do |status| # rubocop:diable RSpec/UselessDynamicDefinition + Ci::HasStatus::STATUSES_ENUM.each_key do |status| # rubocop:disable RSpec/UselessDynamicDefinition -- `status` used in `let` let!("build_1_#{status}") { create(:ci_build, pipeline: pipeline_1, status: status, resource_group: resource_group) } let!("build_2_#{status}") { create(:ci_build, pipeline: pipeline_2, status: status, resource_group: resource_group) } end diff --git a/spec/models/ci/runner_manager_spec.rb b/spec/models/ci/runner_manager_spec.rb index bc1d1a0cc49..01275ffd31c 100644 --- a/spec/models/ci/runner_manager_spec.rb +++ b/spec/models/ci/runner_manager_spec.rb @@ -413,4 +413,68 @@ RSpec.describe Ci::RunnerManager, feature_category: :runner_fleet, type: :model 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 3a3ef072b28..bb9ac084ed6 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -524,6 +524,39 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do end end + describe '.with_creator_id' do + subject { described_class.with_creator_id('1') } + + let_it_be(:runner1) { create(:ci_runner, creator_id: 2) } + let_it_be(:runner2) { create(:ci_runner, creator_id: 1) } + 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 + is_expected.to contain_exactly(runner2, runner3) + end + end + + describe '.with_version_prefix' do + subject { described_class.with_version_prefix('15.11.') } + + let_it_be(:runner1) { create(:ci_runner) } + let_it_be(:runner2) { create(:ci_runner) } + let_it_be(:runner3) { create(:ci_runner) } + + before_all do + create(:ci_runner_machine, runner: runner1, version: '15.11.0') + create(:ci_runner_machine, runner: runner2, version: '15.9.0') + create(:ci_runner_machine, runner: runner3, version: '15.9.0') + # Add another runner_machine to runner3 to ensure edge case is handled (searching multiple machines in a single runner) + create(:ci_runner_machine, runner: runner3, version: '15.11.5') + end + + it 'returns runners containing runner managers with versions starting with 15.11.' do + is_expected.to contain_exactly(runner1, runner3) + end + end + describe '.stale', :freeze_time do subject { described_class.stale } @@ -739,7 +772,7 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do end context 'when runner has tags' do - let(:tag_list) { %w(bb cc) } + let(:tag_list) { %w[bb cc] } shared_examples 'tagged build picker' do it 'can handle build with matching tags' do @@ -1026,7 +1059,7 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do end def value_in_queues - Gitlab::Redis::SharedState.with do |redis| + Gitlab::Redis::Workhorse.with do |redis| runner_queue_key = runner.send(:runner_queue_key) redis.get(runner_queue_key) end diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index 1be50083cd4..4951f57fe6f 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -166,6 +166,18 @@ RSpec.describe Ci::Stage, :models, feature_category: :continuous_integration do end end + context 'when build is waiting for callback' do + before do + create(:ci_build, :waiting_for_callback, stage_id: stage.id) + end + + it 'updates status to waiting for callback' do + expect { stage.update_legacy_status } + .to change { stage.reload.status } + .to 'waiting_for_callback' + end + end + context 'when stage is skipped because is empty' do it 'updates status to skipped' do expect { stage.update_legacy_status } diff --git a/spec/models/clusters/agent_spec.rb b/spec/models/clusters/agent_spec.rb index 6201b7b1861..062d5062658 100644 --- a/spec/models/clusters/agent_spec.rb +++ b/spec/models/clusters/agent_spec.rb @@ -21,7 +21,7 @@ RSpec.describe Clusters::Agent, feature_category: :deployment_management do describe 'scopes' do describe '.ordered_by_name' do - let(:names) { %w(agent-d agent-b agent-a agent-c) } + let(:names) { %w[agent-d agent-b agent-a agent-c] } subject { described_class.ordered_by_name } diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index c32abaf50f5..79a81977611 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Clusters::Platforms::Kubernetes do it { is_expected.to be_kind_of(Gitlab::Kubernetes) } it { is_expected.to respond_to :ca_pem } - it { is_expected.to validate_exclusion_of(:namespace).in_array(%w(gitlab-managed-apps)) } + it { is_expected.to validate_exclusion_of(:namespace).in_array(%w[gitlab-managed-apps]) } it { is_expected.to validate_presence_of(:api_url) } it { is_expected.to validate_presence_of(:token) } diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb index 334833e884b..fe8c28d7251 100644 --- a/spec/models/commit_range_spec.rb +++ b/spec/models/commit_range_spec.rb @@ -77,7 +77,7 @@ RSpec.describe CommitRange do describe '#to_param' do it 'includes the correct keys' do - expect(range.to_param.keys).to eq %i(from to) + expect(range.to_param.keys).to eq %i[from to] end it 'includes the correct values for a three-dot range' do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 7ab43611108..e873a59b54a 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -23,13 +23,13 @@ RSpec.describe Commit do shared_examples '.lazy checks' do context 'when the commits are found' do let(:oids) do - %w( + %w[ 498214de67004b1da3d820901307bed2a68a8ef6 c642fe9b8b9f28f9225d7ea953fe14e74748d53b 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 048721d90c449b244b7b4c53a9186b04330174ec 281d3a76f31c812dbf48abce82ccf6860adedd81 - ) + ] end subject { oids.map { |oid| described_class.lazy(container, oid) } } @@ -707,16 +707,6 @@ eos it_behaves_like "#uri_type" end - describe '#uri_type with Rugged enabled', :enable_rugged do - it 'calls out to the Rugged implementation' do - allow_any_instance_of(Rugged::Tree).to receive(:path).with('files/html').and_call_original - - commit.uri_type('files/html') - end - - it_behaves_like '#uri_type' - end - describe '.diff_max_files' do subject(:diff_max_files) { described_class.diff_max_files } diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index e9257b08bca..618dd3a3f77 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -27,7 +27,7 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do it { is_expected.to belong_to(:auto_canceled_by) } it { is_expected.to validate_presence_of(:name) } - it { is_expected.to validate_inclusion_of(:status).in_array(%w(pending running failed success canceled)) } + it { is_expected.to validate_inclusion_of(:status).in_array(%w[pending running failed success canceled]) } it { is_expected.to validate_length_of(:stage).is_at_most(255) } it { is_expected.to validate_length_of(:ref).is_at_most(255) } @@ -44,24 +44,6 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do it { is_expected.not_to be_retried } it { expect(described_class.primary_key).to eq('id') } - describe '.switch_table_names' do - before do - stub_env('USE_CI_BUILDS_ROUTING_TABLE', flag_value) - end - - context 'with the env flag disabled' do - let(:flag_value) { 'false' } - - it { expect(described_class.switch_table_names).to eq(:ci_builds) } - end - - context 'with the env flag enabled' do - let(:flag_value) { 'true' } - - it { expect(described_class.switch_table_names).to eq(:p_ci_builds) } - end - end - describe '#author' do subject { commit_status.author } @@ -174,7 +156,7 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do describe '.cancelable' do subject { described_class.cancelable } - %i[running pending waiting_for_resource preparing created scheduled].each do |status| + %i[running pending waiting_for_resource waiting_for_callback preparing created scheduled].each do |status| context "when #{status} commit status" do let!(:commit_status) { create(:commit_status, status, pipeline: pipeline) } diff --git a/spec/models/compare_spec.rb b/spec/models/compare_spec.rb index 2206ed7bfe8..78610d002b2 100644 --- a/spec/models/compare_spec.rb +++ b/spec/models/compare_spec.rb @@ -129,13 +129,13 @@ RSpec.describe Compare, feature_category: :source_code_management do it 'returns affected file paths, without duplication' do expect(subject.modified_paths).to contain_exactly( - *%w{ + *%w[ foo/for_move.txt foo/bar/for_move.txt foo/for_create.txt foo/for_delete.txt foo/for_edit.txt - }) + ]) end end diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index fcd0d0c05f4..828b75aceb0 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -102,7 +102,7 @@ RSpec.describe Awardable do end it "includes unused thumbs buttons by default" do - expect(note_without_downvote.grouped_awards.keys.sort).to eq %w(thumbsdown thumbsup) + expect(note_without_downvote.grouped_awards.keys.sort).to eq %w[thumbsdown thumbsup] end it "doesn't include unused thumbs buttons when disabled in project" do @@ -114,7 +114,7 @@ RSpec.describe Awardable do it "includes unused thumbs buttons when enabled in project" do note_without_downvote.project.show_default_award_emojis = true - expect(note_without_downvote.grouped_awards.keys.sort).to eq %w(thumbsdown thumbsup) + expect(note_without_downvote.grouped_awards.keys.sort).to eq %w[thumbsdown thumbsup] end it "doesn't include unused thumbs buttons in summary" do @@ -124,11 +124,11 @@ RSpec.describe Awardable do it "includes used thumbs buttons when disabled in project" do note_with_downvote.project.show_default_award_emojis = false - expect(note_with_downvote.grouped_awards.keys).to eq %w(thumbsdown) + expect(note_with_downvote.grouped_awards.keys).to eq %w[thumbsdown] end it "includes used thumbs buttons in summary" do - expect(note_with_downvote.grouped_awards(with_thumbs: false).keys).to eq %w(thumbsdown) + expect(note_with_downvote.grouped_awards(with_thumbs: false).keys).to eq %w[thumbsdown] end end end diff --git a/spec/models/concerns/case_sensitivity_spec.rb b/spec/models/concerns/case_sensitivity_spec.rb index 6e624c687c4..a8e52904873 100644 --- a/spec/models/concerns/case_sensitivity_spec.rb +++ b/spec/models/concerns/case_sensitivity_spec.rb @@ -21,11 +21,11 @@ RSpec.describe CaseSensitivity do end it 'finds multiple instances by a single attribute regardless of case' do - expect(model.iwhere(path: %w(MODEL-1 model-2))).to contain_exactly(model_1, model_2) + expect(model.iwhere(path: %w[MODEL-1 model-2])).to contain_exactly(model_1, model_2) end it 'finds instances by multiple attributes' do - expect(model.iwhere(path: %w(MODEL-1 model-2), name: 'model 1')) + expect(model.iwhere(path: %w[MODEL-1 model-2], name: 'model 1')) .to contain_exactly(model_1) end @@ -34,7 +34,7 @@ RSpec.describe CaseSensitivity do end it 'builds a query using LOWER' do - query = model.iwhere(path: %w(MODEL-1 model-2), name: 'model 1').to_sql + query = model.iwhere(path: %w[MODEL-1 model-2], name: 'model 1').to_sql expected_query = <<~QRY.strip SELECT \"namespaces\".* FROM \"namespaces\" WHERE (LOWER(\"namespaces\".\"path\") IN (LOWER('MODEL-1'), LOWER('model-2'))) AND (LOWER(\"namespaces\".\"name\") = LOWER('model 1')) QRY diff --git a/spec/models/concerns/ci/has_status_spec.rb b/spec/models/concerns/ci/has_status_spec.rb index 5e0a430aa13..95f17c4f854 100644 --- a/spec/models/concerns/ci/has_status_spec.rb +++ b/spec/models/concerns/ci/has_status_spec.rb @@ -55,6 +55,22 @@ RSpec.describe Ci::HasStatus, feature_category: :continuous_integration do it { is_expected.to eq 'waiting_for_resource' } end + context 'all waiting for callback' do + let!(:statuses) do + [create(type, status: :waiting_for_callback), create(type, status: :waiting_for_callback)] + end + + it { is_expected.to eq 'waiting_for_callback' } + end + + context 'at least one waiting for callback' do + let!(:statuses) do + [create(type, status: :success), create(type, status: :waiting_for_callback)] + end + + it { is_expected.to eq 'waiting_for_callback' } + end + context 'all preparing' do let!(:statuses) do [create(type, status: :preparing), create(type, status: :preparing)] @@ -225,7 +241,7 @@ RSpec.describe Ci::HasStatus, feature_category: :continuous_integration do end end - %i[created waiting_for_resource preparing running pending success + %i[created waiting_for_callback waiting_for_resource preparing running pending success failed canceled skipped].each do |status| it_behaves_like 'having a job', status end @@ -271,7 +287,7 @@ RSpec.describe Ci::HasStatus, feature_category: :continuous_integration do describe '.alive' do subject { CommitStatus.alive } - %i[running pending waiting_for_resource preparing created].each do |status| + %i[running pending waiting_for_callback waiting_for_resource preparing created].each do |status| it_behaves_like 'containing the job', status end @@ -283,7 +299,7 @@ RSpec.describe Ci::HasStatus, feature_category: :continuous_integration do describe '.alive_or_scheduled' do subject { CommitStatus.alive_or_scheduled } - %i[running pending waiting_for_resource preparing created scheduled].each do |status| + %i[running pending waiting_for_callback waiting_for_resource preparing created scheduled].each do |status| it_behaves_like 'containing the job', status end @@ -319,7 +335,7 @@ RSpec.describe Ci::HasStatus, feature_category: :continuous_integration do describe '.cancelable' do subject { CommitStatus.cancelable } - %i[running pending waiting_for_resource preparing created scheduled].each do |status| + %i[running pending waiting_for_callback waiting_for_resource preparing created scheduled].each do |status| it_behaves_like 'containing the job', status end diff --git a/spec/models/concerns/ci/partitionable/switch_spec.rb b/spec/models/concerns/ci/partitionable/switch_spec.rb index 0041a33e50e..c6e2ed265bd 100644 --- a/spec/models/concerns/ci/partitionable/switch_spec.rb +++ b/spec/models/concerns/ci/partitionable/switch_spec.rb @@ -31,8 +31,6 @@ RSpec.describe Ci::Partitionable::Switch, :aggregate_failures do end before do - allow(ActiveSupport::DescendantsTracker).to receive(:store_inherited) - create_tables(<<~SQL) CREATE TABLE _test_ci_jobs_metadata( id serial NOT NULL PRIMARY KEY, @@ -78,6 +76,15 @@ RSpec.describe Ci::Partitionable::Switch, :aggregate_failures do ) end + # the models defined here are leaked to other tests through + # `ActiveRecord::Base.descendants` and we need to counter the side effects + # from this. We redefine the method so that we don't check the FF existence + # outside of the examples here. + # `ActiveSupport::DescendantsTracker.clear` doesn't work with cached classes. + after do + model.define_singleton_method(:routing_table_enabled?) { false } + end + it { expect(model).not_to be_routing_class } it { expect(partitioned_model).to be_routing_class } diff --git a/spec/models/concerns/enums/sbom_spec.rb b/spec/models/concerns/enums/sbom_spec.rb index 41670880630..e2f56cc637d 100644 --- a/spec/models/concerns/enums/sbom_spec.rb +++ b/spec/models/concerns/enums/sbom_spec.rb @@ -22,7 +22,8 @@ RSpec.describe Enums::Sbom, feature_category: :dependency_management do :apk | 9 :rpm | 10 :deb | 11 - :cbl_mariner | 12 + 'cbl-mariner' | 12 + :wolfi | 13 'unknown-pkg-manager' | 0 'Python (unknown)' | 0 end diff --git a/spec/models/concerns/featurable_spec.rb b/spec/models/concerns/featurable_spec.rb index bf104fe1b30..97ad4fc8bdd 100644 --- a/spec/models/concerns/featurable_spec.rb +++ b/spec/models/concerns/featurable_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Featurable do self.table_name = 'project_features' - set_available_features %i(feature1 feature2 feature3) + set_available_features %i[feature1 feature2 feature3] def feature1_access_level Featurable::DISABLED diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index 54614ec2b21..effb588e55f 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe User, feature_category: :system_access do - User::USER_TYPES.keys.each do |type| # rubocop:disable RSpec/UselessDynamicDefinition + User::USER_TYPES.each_key do |type| # rubocop:disable RSpec/UselessDynamicDefinition -- `type` used in `let` let_it_be(type) { create(:user, username: type, user_type: type) } end let(:bots) { User::BOT_USER_TYPES.map { |type| public_send(type) } } diff --git a/spec/models/concerns/ignorable_columns_spec.rb b/spec/models/concerns/ignorable_columns_spec.rb index c97dc606159..339f06f9c45 100644 --- a/spec/models/concerns/ignorable_columns_spec.rb +++ b/spec/models/concerns/ignorable_columns_spec.rb @@ -14,13 +14,13 @@ RSpec.describe IgnorableColumns do it 'adds columns to ignored_columns' do expect do subject.ignore_columns(:name, :created_at, remove_after: '2019-12-01', remove_with: '12.6') - end.to change { subject.ignored_columns }.from([]).to(%w(name created_at)) + end.to change { subject.ignored_columns }.from([]).to(%w[name created_at]) end it 'adds columns to ignored_columns (array version)' do expect do subject.ignore_columns(%i[name created_at], remove_after: '2019-12-01', remove_with: '12.6') - end.to change { subject.ignored_columns }.from([]).to(%w(name created_at)) + end.to change { subject.ignored_columns }.from([]).to(%w[name created_at]) end it 'requires remove_after attribute to be set' do diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 705f8f46a90..d61a465b39f 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -597,7 +597,7 @@ RSpec.describe Issuable, feature_category: :team_planning do expect(builder).to receive(:build).with( user: user, changes: hash_including( - 'severity' => %w(unknown low) + 'severity' => %w[unknown low] )) issue.to_hook_data(user, old_associations: { severity: 'unknown' }) @@ -618,7 +618,7 @@ RSpec.describe Issuable, feature_category: :team_planning do expect(builder).to receive(:build).with( user: user, changes: hash_including( - 'escalation_status' => %i(triggered acknowledged) + 'escalation_status' => %i[triggered acknowledged] )) issue.to_hook_data(user, old_associations: { escalation_status: :triggered }) diff --git a/spec/models/concerns/pg_full_text_searchable_spec.rb b/spec/models/concerns/pg_full_text_searchable_spec.rb index 059df64f7d0..8e3b65cf125 100644 --- a/spec/models/concerns/pg_full_text_searchable_spec.rb +++ b/spec/models/concerns/pg_full_text_searchable_spec.rb @@ -18,7 +18,7 @@ RSpec.describe PgFullTextSearchable, feature_category: :global_search do before_validation -> { self.work_item_type_id = ::WorkItems::Type.default_issue_type.id } def persist_pg_full_text_search_vector(search_vector) - Issues::SearchData.upsert({ project_id: project_id, issue_id: id, search_vector: search_vector }, unique_by: %i(project_id issue_id)) + Issues::SearchData.upsert({ project_id: project_id, issue_id: id, search_vector: search_vector }, unique_by: %i[project_id issue_id]) end def self.name @@ -95,7 +95,7 @@ RSpec.describe PgFullTextSearchable, feature_category: :global_search do matching_object = model_class.create!(project: project, namespace: project.project_namespace, title: 'english', description: 'some description') matching_object.update_search_data! - expect(model_class.pg_full_text_search('english', matched_columns: %w(title))).to contain_exactly(matching_object) + expect(model_class.pg_full_text_search('english', matched_columns: %w[title])).to contain_exactly(matching_object) end it 'uses prefix matching' do diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb index f168bedc8eb..46390fa735b 100644 --- a/spec/models/concerns/project_features_compatibility_spec.rb +++ b/spec/models/concerns/project_features_compatibility_spec.rb @@ -4,12 +4,12 @@ require 'spec_helper' RSpec.describe ProjectFeaturesCompatibility do let(:project) { create(:project) } - let(:features_enabled) { %w(issues wiki builds merge_requests snippets security_and_compliance) } + let(:features_enabled) { %w[issues wiki builds merge_requests snippets security_and_compliance] } let(:features) do - features_enabled + %w( + features_enabled + %w[ repository pages operations container_registry package_registry environments feature_flags releases monitor infrastructure - ) + ] end # We had issues_enabled, snippets_enabled, builds_enabled, merge_requests_enabled and issues_enabled fields on projects table diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 039b9e574fe..324759fc7ee 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -176,7 +176,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do describe '.reactive_cache_worker_finder' do context 'with default reactive_cache_worker_finder' do - let(:args) { %w(other args) } + let(:args) { %w[other args] } before do allow(instance.class).to receive(:find_by).with(id: instance.id) @@ -192,7 +192,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do end context 'with custom reactive_cache_worker_finder' do - let(:args) { %w(arg1 arg2) } + let(:args) { %w[arg1 arg2] } let(:instance) { custom_finder_cache_test.new(666, &calculation) } let(:custom_finder_cache_test) do diff --git a/spec/models/concerns/reset_on_column_errors_spec.rb b/spec/models/concerns/reset_on_column_errors_spec.rb index 38ba0f447f5..96bee128f7e 100644 --- a/spec/models/concerns/reset_on_column_errors_spec.rb +++ b/spec/models/concerns/reset_on_column_errors_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ResetOnColumnErrors, :delete, feature_category: :shared do +RSpec.describe ResetOnColumnErrors, :delete, feature_category: :shared, query_analyzers: false do let(:test_reviewer_model) do Class.new(ApplicationRecord) do self.table_name = '_test_reviewers_table' diff --git a/spec/models/concerns/sortable_spec.rb b/spec/models/concerns/sortable_spec.rb index f1ae89f33af..98f4ab4f521 100644 --- a/spec/models/concerns/sortable_spec.rb +++ b/spec/models/concerns/sortable_spec.rb @@ -14,14 +14,14 @@ RSpec.describe Sortable do it 'allows secondary ordering by id ascending' do orders = arel_orders(sorted_relation.with_order_id_asc) - expect(orders.map { |arel| arel.expr.name }).to eq(%w(created_at id)) + expect(orders.map { |arel| arel.expr.name }).to eq(%w[created_at id]) expect(orders).to all(be_kind_of(Arel::Nodes::Ascending)) end it 'allows secondary ordering by id descending' do orders = arel_orders(sorted_relation.with_order_id_desc) - expect(orders.map { |arel| arel.expr.name }).to eq(%w(created_at id)) + expect(orders.map { |arel| arel.expr.name }).to eq(%w[created_at id]) expect(orders.first).to be_kind_of(Arel::Nodes::Ascending) expect(orders.last).to be_kind_of(Arel::Nodes::Descending) end @@ -123,24 +123,24 @@ RSpec.describe Sortable do let!(:group4) { create(:group, name: 'bbb', id: 4, created_at: ref_time, updated_at: ref_time - 15.seconds) } it 'sorts groups by id' do - expect(ordered_group_names('id_asc')).to eq(%w(aa AAA BB bbb)) - expect(ordered_group_names('id_desc')).to eq(%w(bbb BB AAA aa)) + expect(ordered_group_names('id_asc')).to eq(%w[aa AAA BB bbb]) + expect(ordered_group_names('id_desc')).to eq(%w[bbb BB AAA aa]) end it 'sorts groups by name via case-insensitive comparision' do - expect(ordered_group_names('name_asc')).to eq(%w(aa AAA BB bbb)) - expect(ordered_group_names('name_desc')).to eq(%w(bbb BB AAA aa)) + expect(ordered_group_names('name_asc')).to eq(%w[aa AAA BB bbb]) + expect(ordered_group_names('name_desc')).to eq(%w[bbb BB AAA aa]) end it 'sorts groups by created_at' do - expect(ordered_group_names('created_asc')).to eq(%w(aa AAA BB bbb)) - expect(ordered_group_names('created_desc')).to eq(%w(bbb BB AAA aa)) - expect(ordered_group_names('created_date')).to eq(%w(bbb BB AAA aa)) + expect(ordered_group_names('created_asc')).to eq(%w[aa AAA BB bbb]) + expect(ordered_group_names('created_desc')).to eq(%w[bbb BB AAA aa]) + expect(ordered_group_names('created_date')).to eq(%w[bbb BB AAA aa]) end it 'sorts groups by updated_at' do - expect(ordered_group_names('updated_asc')).to eq(%w(bbb BB AAA aa)) - expect(ordered_group_names('updated_desc')).to eq(%w(aa AAA BB bbb)) + expect(ordered_group_names('updated_asc')).to eq(%w[bbb BB AAA aa]) + expect(ordered_group_names('updated_desc')).to eq(%w[aa AAA BB bbb]) end end end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 822e2817d84..9bd20676c0e 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -260,7 +260,7 @@ RSpec.describe Ci::Build, 'TokenAuthenticatable' do it 'persists a new token' do build.save! - build.token.yield_self do |previous_token| + build.token.then do |previous_token| build.reset_token! expect(build.token).not_to eq previous_token diff --git a/spec/models/concerns/use_sql_function_for_primary_key_lookups_spec.rb b/spec/models/concerns/use_sql_function_for_primary_key_lookups_spec.rb new file mode 100644 index 00000000000..f6f53c9aad5 --- /dev/null +++ b/spec/models/concerns/use_sql_function_for_primary_key_lookups_spec.rb @@ -0,0 +1,181 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe UseSqlFunctionForPrimaryKeyLookups, feature_category: :groups_and_projects do + let_it_be(:project) { create(:project) } + let_it_be(:another_project) { create(:project) } + + let(:model) do + Class.new(ApplicationRecord) do + self.table_name = :projects + + include UseSqlFunctionForPrimaryKeyLookups + end + end + + context 'when the use_sql_functions_for_primary_key_lookups FF is on' do + before do + stub_feature_flags(use_sql_functions_for_primary_key_lookups: true) + end + + it 'loads the correct record' do + expect(model.find(project.id).id).to eq(project.id) + end + + it 'uses the fuction-based finder query' do + query = <<~SQL + SELECT "projects".* FROM find_projects_by_id(#{project.id})#{' '} + "projects" WHERE ("projects"."id" IS NOT NULL) LIMIT 1 + SQL + query_log = ActiveRecord::QueryRecorder.new { model.find(project.id) }.log + + expect(query_log).to match_array(include(query.tr("\n", ''))) + end + + it 'uses query cache', :use_sql_query_cache do + query = <<~SQL + SELECT "projects".* FROM find_projects_by_id(#{project.id})#{' '} + "projects" WHERE ("projects"."id" IS NOT NULL) LIMIT 1 + SQL + + recorder = ActiveRecord::QueryRecorder.new do + model.find(project.id) + model.find(project.id) + model.find(project.id) + end + + expect(recorder.data.each_value.first[:count]).to eq(1) + expect(recorder.cached).to include(query.tr("\n", '')) + end + + context 'when the model has ignored columns' do + around do |example| + model.ignored_columns = %i[path] + example.run + model.ignored_columns = [] + end + + it 'enumerates the column names' do + column_list = model.columns.map do |column| + %("projects"."#{column.name}") + end.join(', ') + + expect(column_list).not_to include(%("projects"."path")) + + query = <<~SQL + SELECT #{column_list} FROM find_projects_by_id(#{project.id})#{' '} + "projects" WHERE ("projects"."id" IS NOT NULL) LIMIT 1 + SQL + query_log = ActiveRecord::QueryRecorder.new { model.find(project.id) }.log + + expect(query_log).to match_array(include(query.tr("\n", ''))) + end + end + + context 'when there are scope attributes' do + let(:scoped_model) do + Class.new(model) do + default_scope { where.not(path: nil) } # rubocop: disable Cop/DefaultScope -- Needed for testing a specific case + end + end + + it 'loads the correct record' do + expect(scoped_model.find(project.id).id).to eq(project.id) + end + + it 'does not use the function-based finder query' do + query_log = ActiveRecord::QueryRecorder.new { scoped_model.find(project.id) }.log + + expect(query_log).not_to include(match(/find_projects_by_id/)) + end + end + + context 'when there are multiple arguments' do + it 'loads the correct records' do + expect(model.find(project.id, another_project.id).map(&:id)).to match_array([project.id, another_project.id]) + end + + it 'does not use the function-based finder query' do + query_log = ActiveRecord::QueryRecorder.new { model.find(project.id, another_project.id) }.log + + expect(query_log).not_to include(match(/find_projects_by_id/)) + end + end + + context 'when there is block given' do + it 'loads the correct records' do + expect(model.find(0) { |p| p.path == project.path }.id).to eq(project.id) + end + + it 'does not use the function-based finder query' do + query_log = ActiveRecord::QueryRecorder.new { model.find(0) { |p| p.path == project.path } }.log + + expect(query_log).not_to include(match(/find_projects_by_id/)) + end + end + + context 'when there is no primary key defined' do + let(:model_without_pk) do + Class.new(model) do + def self.primary_key + nil + end + end + end + + it 'raises ActiveRecord::UnknownPrimaryKey' do + expect { model_without_pk.find(0) }.to raise_error ActiveRecord::UnknownPrimaryKey + end + end + + context 'when id is provided as an array' do + it 'returns the correct record as an array' do + expect(model.find([project.id]).map(&:id)).to eq([project.id]) + end + + it 'does use the function-based finder query' do + query_log = ActiveRecord::QueryRecorder.new { model.find([project.id]) }.log + + expect(query_log).to include(match(/find_projects_by_id/)) + end + + context 'when array has multiple elements' do + it 'does not use the function-based finder query' do + query_log = ActiveRecord::QueryRecorder.new { model.find([project.id, another_project.id]) }.log + + expect(query_log).not_to include(match(/find_projects_by_id/)) + end + end + end + + context 'when the provided id is null' do + it 'raises ActiveRecord::RecordNotFound' do + expect { model.find(nil) }.to raise_error ActiveRecord::RecordNotFound, "Couldn't find without an ID" + end + end + + context 'when the provided id is not a string that can cast to numeric' do + it 'raises ActiveRecord::RecordNotFound' do + expect { model.find('foo') }.to raise_error ActiveRecord::RecordNotFound, "Couldn't find with 'id'=foo" + end + end + end + + context 'when the use_sql_functions_for_primary_key_lookups FF is off' do + before do + stub_feature_flags(use_sql_functions_for_primary_key_lookups: false) + end + + it 'loads the correct record' do + expect(model.find(project.id).id).to eq(project.id) + end + + it 'uses the SQL-based finder query' do + expected_query = %(SELECT "projects".* FROM \"projects\" WHERE "projects"."id" = #{project.id} LIMIT 1) + query_log = ActiveRecord::QueryRecorder.new { model.find(project.id) }.log + + expect(query_log).to match_array(include(expected_query)) + end + end +end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 93fe070e5c4..027fd20462b 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -675,6 +675,101 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont end end + describe '#tags_page' do + let_it_be(:page_size) { 100 } + let_it_be(:before) { 'before' } + let_it_be(:last) { 'last' } + let_it_be(:sort) { '-name' } + let_it_be(:name) { 'repo' } + + subject do + repository.tags_page(before: before, last: last, sort: sort, name: name, page_size: page_size) + end + + before do + allow(repository).to receive(:migrated?).and_return(true) + end + + 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) + + subject + end + + context 'with a call to tags' do + let_it_be(:tags_response) do + [ + { + name: '0.1.0', + digest: 'sha256:6c3c624b58dbbcd3c0dd82b4c53f04194d1247c6eebdaab7c610cf7d6670', + 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 + }, + { + name: 'latest', + digest: 'sha256:6c3c624b58dbbcd3c0dd82b4c53f04191247c6eebdaab7c610cf7d66709b3', + config_digest: 'sha256:66b1132a0173910b01ee694462c99efbe1b9ab5bf8083231232312', + media_type: 'application/vnd.oci.image.manifest.v1+json', + size_bytes: 1234567892, + created_at: 10.minutes.ago, + updated_at: 10.minutes.ago + } + ] + end + + let_it_be(:response_body) do + { + pagination: { + previous: { uri: URI('/test?before=prev-cursor') }, + next: { uri: URI('/test?last=next-cursor') } + }, + response_body: ::Gitlab::Json.parse(tags_response.to_json) + } + end + + before do + allow(repository.gitlab_api_client).to receive(:tags).and_return(response_body) + end + + it 'returns tags and parses the previous and next cursors' do + return_value = subject + + expect(return_value[:pagination]).to eq(response_body[:pagination]) + + return_value[:tags].each_with_index do |tag, index| + expected_revision = tags_response[index][:config_digest].to_s.split(':')[1] + + expect(tag.is_a?(ContainerRegistry::Tag)).to eq(true) + expect(tag).to have_attributes( + repository: repository, + name: tags_response[index][:name], + digest: tags_response[index][:digest], + total_size: tags_response[index][:size_bytes], + 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) + ) + end + end + end + + context 'calling on a non migrated repository' do + before do + allow(repository).to receive(:migrated?).and_return(false) + end + + it 'raises an Argument error' do + expect { repository.tags_page }.to raise_error(ArgumentError, 'not a migrated repository') + end + end + end + describe '#tags_count' do it 'returns the count of tags' do expect(repository.tags_count).to eq(1) @@ -720,7 +815,7 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont end end - describe '#delete_tag_by_name' do + describe '#delete_tag' do let(:repository) do create( :container_repository, @@ -733,22 +828,22 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont context 'when action succeeds' do it 'returns status that indicates success' do expect(repository.client) - .to receive(:delete_repository_tag_by_name) + .to receive(:delete_repository_tag_by_digest) .with(repository.path, "latest") .and_return(true) - expect(repository.delete_tag_by_name('latest')).to be_truthy + expect(repository.delete_tag('latest')).to be_truthy end end context 'when action fails' do it 'returns status that indicates failure' do expect(repository.client) - .to receive(:delete_repository_tag_by_name) + .to receive(:delete_repository_tag_by_digest) .with(repository.path, "latest") .and_return(false) - expect(repository.delete_tag_by_name('latest')).to be_falsey + expect(repository.delete_tag('latest')).to be_falsey end end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 639b149e2ae..ee48e8cac6c 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -1336,7 +1336,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do let(:job_status) { :created } it_behaves_like 'gracefully handling error' do - let(:error_message) { %{Status cannot transition via \"create\"} } + let(:error_message) { %(Status cannot transition via \"create\") } end end @@ -1344,7 +1344,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do let(:job_status) { :manual } it_behaves_like 'gracefully handling error' do - let(:error_message) { %{Status cannot transition via \"block\"} } + let(:error_message) { %(Status cannot transition via \"block\") } end end @@ -1374,7 +1374,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do let(:job_status) { :created } it_behaves_like 'gracefully handling error' do - let(:error_message) { %{Status cannot transition via \"create\"} } + let(:error_message) { %(Status cannot transition via \"create\") } end end @@ -1382,7 +1382,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do let(:job_status) { :manual } it_behaves_like 'gracefully handling error' do - let(:error_message) { %{Status cannot transition via \"block\"} } + let(:error_message) { %(Status cannot transition via \"block\") } end end @@ -1390,7 +1390,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do let(:job_status) { :running } it_behaves_like 'gracefully handling error' do - let(:error_message) { %{Status cannot transition via \"run\"} } + let(:error_message) { %(Status cannot transition via \"run\") } end end diff --git a/spec/models/diff_viewer/base_spec.rb b/spec/models/diff_viewer/base_spec.rb index 57c62788ee9..8ab7b090928 100644 --- a/spec/models/diff_viewer/base_spec.rb +++ b/spec/models/diff_viewer/base_spec.rb @@ -13,7 +13,7 @@ RSpec.describe DiffViewer::Base do Class.new(described_class) do include DiffViewer::ServerSide - self.extensions = %w(jpg) + self.extensions = %w[jpg] self.binary = true self.collapse_limit = 1.megabyte self.size_limit = 5.megabytes @@ -55,7 +55,7 @@ RSpec.describe DiffViewer::Base do before do allow(diff_file).to receive(:renamed_file?).and_return(true) - viewer_class.extensions = %w(notjpg) + viewer_class.extensions = %w[notjpg] end it 'returns false' do diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index b76063bfa1a..ecb8f72470d 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -89,7 +89,7 @@ RSpec.describe Email do end context 'when the confirmation period has expired' do - let(:confirmation_sent_at) { expired_confirmation_sent_at } + let(:confirmation_sent_at) { expired_confirmation_sent_at } it_behaves_like 'unconfirmed email' @@ -101,7 +101,7 @@ RSpec.describe Email do end context 'when the confirmation period has not expired' do - let(:confirmation_sent_at) { extant_confirmation_sent_at } + let(:confirmation_sent_at) { extant_confirmation_sent_at } it_behaves_like 'unconfirmed email' @@ -138,7 +138,7 @@ RSpec.describe Email do end context 'when the confirmation period has expired' do - let(:confirmation_sent_at) { expired_confirmation_sent_at } + let(:confirmation_sent_at) { expired_confirmation_sent_at } it_behaves_like 'unconfirmed email' it_behaves_like 'confirms the email on force_confirm' diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index dcfee7fcc8c..33142922670 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -166,6 +166,37 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ end end + describe '#long_stopping?' do + subject { environment1.long_stopping? } + + let(:long_ago) { (described_class::LONG_STOP + 1.day).ago } + let(:not_long_ago) { (described_class::LONG_STOP - 1.day).ago } + + context 'when a stopping environment has not been updated recently' do + let!(:environment1) { create(:environment, state: 'stopping', project: project, updated_at: long_ago) } + + it { is_expected.to eq(true) } + end + + context 'when a stopping environment has been updated recently' do + let!(:environment1) { create(:environment, state: 'stopping', project: project, updated_at: not_long_ago) } + + it { is_expected.to eq(false) } + end + + context 'when a non stopping environment has not been updated recently' do + let!(:environment1) { create(:environment, project: project, updated_at: long_ago) } + + it { is_expected.to eq(false) } + end + + context 'when a non stopping environment has been updated recently' do + let!(:environment1) { create(:environment, project: project, updated_at: not_long_ago) } + + it { is_expected.to eq(false) } + end + end + describe ".stopped_review_apps" do let_it_be(:project) { create(:project, :repository) } let_it_be(:old_stopped_review_env) { create(:environment, :with_review_app, :stopped, created_at: 31.days.ago, project: project) } @@ -406,6 +437,47 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ end end + describe '.long_stopping' do + subject { described_class.long_stopping } + + let_it_be(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + let(:long) { (described_class::LONG_STOP + 1.day).ago } + let(:short) { (described_class::LONG_STOP - 1.day).ago } + + context 'when a stopping environment has not been updated recently' do + before do + environment.update!(state: :stopping, updated_at: long) + end + + it { is_expected.to eq([environment]) } + end + + context 'when a stopping environment has been updated recently' do + before do + environment.update!(state: :stopping, updated_at: short) + end + + it { is_expected.to be_empty } + end + + context 'when a non stopping environment has not been updated recently' do + before do + environment.update!(state: :available, updated_at: long) + end + + it { is_expected.to be_empty } + end + + context 'when a non stopping environment has been updated recently' do + before do + environment.update!(state: :available, updated_at: short) + end + + it { is_expected.to be_empty } + end + end + describe '.pluck_names' do subject { described_class.pluck_names } @@ -1361,7 +1433,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ end context 'reactive cache has pod data' do - let(:cache_data) { Hash(pods: %w(pod1 pod2)) } + let(:cache_data) { Hash(pods: %w[pod1 pod2]) } before do stub_reactive_cache(environment, cache_data) @@ -1390,9 +1462,9 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ it 'returns cache data from the deployment platform' do expect(environment.deployment_platform).to receive(:calculate_reactive_cache_for) - .with(environment).and_return(pods: %w(pod1 pod2)) + .with(environment).and_return(pods: %w[pod1 pod2]) - is_expected.to eq(pods: %w(pod1 pod2)) + is_expected.to eq(pods: %w[pod1 pod2]) end context 'environment does not have terminals available' do @@ -1863,8 +1935,8 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ end context 'cached rollout status is present' do - let(:pods) { %w(pod1 pod2) } - let(:deployments) { %w(deployment1 deployment2) } + let(:pods) { %w[pod1 pod2] } + let(:deployments) { %w[deployment1 deployment2] } before do stub_reactive_cache(environment, pods: pods, deployments: deployments) diff --git a/spec/models/group_label_spec.rb b/spec/models/group_label_spec.rb index 701348baf48..40019fdc94c 100644 --- a/spec/models/group_label_spec.rb +++ b/spec/models/group_label_spec.rb @@ -34,7 +34,7 @@ RSpec.describe GroupLabel do end it 'uses id when name contains double quote' do - label = create(:label, name: %q{"irony"}) + label = create(:label, name: %q("irony")) expect(label.to_reference(format: :name)).to eq "~#{label.id}" end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 96ef36a5b75..2bca73545d0 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -1659,6 +1659,12 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end + it 'returns true for a user in parent group' do + subgroup = create(:group, parent: group) + + expect(subgroup.member?(user)).to be_truthy + end + context 'in shared group' do let(:shared_group) { create(:group) } let(:member_shared) { create(:user) } @@ -2053,29 +2059,6 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end - describe '#project_users_with_descendants' do - let(:user_a) { create(:user) } - let(:user_b) { create(:user) } - let(:user_c) { create(:user) } - - let(:group) { create(:group) } - let(:nested_group) { create(:group, parent: group) } - let(:deep_nested_group) { create(:group, parent: nested_group) } - let(:project_a) { create(:project, namespace: group) } - let(:project_b) { create(:project, namespace: nested_group) } - let(:project_c) { create(:project, namespace: deep_nested_group) } - - it 'returns members of all projects in group and subgroups' do - project_a.add_developer(user_a) - project_b.add_developer(user_b) - project_c.add_developer(user_c) - - expect(group.project_users_with_descendants).to contain_exactly(user_a, user_b, user_c) - expect(nested_group.project_users_with_descendants).to contain_exactly(user_b, user_c) - expect(deep_nested_group.project_users_with_descendants).to contain_exactly(user_c) - end - end - describe '#refresh_members_authorized_projects' do let_it_be(:group) { create(:group, :nested) } let_it_be(:parent_group_user) { create(:user) } @@ -2953,18 +2936,21 @@ RSpec.describe Group, feature_category: :groups_and_projects do end describe '.ids_with_disabled_email' do - let_it_be(:parent_1) { create(:group, emails_disabled: true) } + let_it_be(:parent_1) { create(:group) } let_it_be(:child_1) { create(:group, parent: parent_1) } - let_it_be(:parent_2) { create(:group, emails_disabled: false) } + let_it_be(:parent_2) { create(:group) } let_it_be(:child_2) { create(:group, parent: parent_2) } - let_it_be(:other_group) { create(:group, emails_disabled: false) } + let_it_be(:other_group) { create(:group) } shared_examples 'returns namespaces with disabled email' do subject(:group_ids_where_email_is_disabled) { described_class.ids_with_disabled_email([child_1, child_2, other_group]) } - it { is_expected.to eq(Set.new([child_1.id])) } + it do + parent_1.update_attribute(:emails_enabled, false) + is_expected.to eq(Set.new([child_1.id])) + end end it_behaves_like 'returns namespaces with disabled email' diff --git a/spec/models/instance_configuration_spec.rb b/spec/models/instance_configuration_spec.rb index 346f743e8ef..6fbb9245885 100644 --- a/spec/models/instance_configuration_spec.rb +++ b/spec/models/instance_configuration_spec.rb @@ -36,7 +36,7 @@ RSpec.describe InstanceConfiguration do result = subject.settings[:ssh_algorithms_hashes] - expect(result.map { |a| a[:name] }).to match_array(%w(DSA ECDSA ED25519 RSA)) + expect(result.map { |a| a[:name] }).to match_array(%w[DSA ECDSA ED25519 RSA]) end it 'does not include disabled algorithm' do @@ -45,7 +45,7 @@ RSpec.describe InstanceConfiguration do result = subject.settings[:ssh_algorithms_hashes] - expect(result.map { |a| a[:name] }).to match_array(%w(ECDSA ED25519 RSA)) + expect(result.map { |a| a[:name] }).to match_array(%w[ECDSA ED25519 RSA]) end def pub_file(exist: true) diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index d7b69546de6..5af6a592c66 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -157,6 +157,18 @@ RSpec.describe Integration, feature_category: :integrations do include_examples 'hook scope', 'incident' end + describe '.title' do + it 'raises an error' do + expect { described_class.title }.to raise_error(NotImplementedError) + end + end + + describe '.description' do + it 'raises an error' do + expect { described_class.description }.to raise_error(NotImplementedError) + end + end + describe '#operating?' do it 'is false when the integration is not active' do expect(build(:integration).operating?).to eq(false) @@ -976,7 +988,7 @@ RSpec.describe Integration, feature_category: :integrations do subject { described_class.available_integration_names } before do - allow(described_class).to receive(:integration_names).and_return(%w(foo)) + allow(described_class).to receive(:integration_names).and_return(%w[foo]) allow(described_class).to receive(:project_specific_integration_names).and_return(['bar']) allow(described_class).to receive(:dev_integration_names).and_return(['baz']) end @@ -1315,6 +1327,7 @@ RSpec.describe Integration, feature_category: :integrations do describe '#async_execute' do let(:integration) { described_class.new(id: 123) } let(:data) { { object_kind: 'build' } } + let(:serialized_data) { data.deep_stringify_keys } let(:supported_events) { %w[push build] } subject(:async_execute) { integration.async_execute(data) } @@ -1324,7 +1337,7 @@ RSpec.describe Integration, feature_category: :integrations do end it 'queues a Integrations::ExecuteWorker' do - expect(Integrations::ExecuteWorker).to receive(:perform_async).with(integration.id, data) + expect(Integrations::ExecuteWorker).to receive(:perform_async).with(integration.id, serialized_data) async_execute end diff --git a/spec/models/integrations/base_chat_notification_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index 497f2f1e7c9..9ad37f40fbc 100644 --- a/spec/models/integrations/base_chat_notification_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -347,12 +347,6 @@ RSpec.describe Integrations::BaseChatNotification, feature_category: :integratio end end - describe '#help' do - it 'raises an error' do - expect { subject.help }.to raise_error(NotImplementedError) - end - end - describe '#event_channel_name' do it 'returns the channel field name for the given event' do expect(subject.event_channel_name(:event)).to eq('event_channel') diff --git a/spec/models/integrations/buildkite_spec.rb b/spec/models/integrations/buildkite_spec.rb index f3231d50eae..ce31c0b20a3 100644 --- a/spec/models/integrations/buildkite_spec.rb +++ b/spec/models/integrations/buildkite_spec.rb @@ -50,7 +50,7 @@ RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching, f describe '.supported_events' do it 'supports push, merge_request, and tag_push events' do - expect(integration.supported_events).to eq %w(push merge_request tag_push) + expect(integration.supported_events).to eq %w[push merge_request tag_push] end end diff --git a/spec/models/integrations/campfire_spec.rb b/spec/models/integrations/campfire_spec.rb index 38d3d89cdbf..19f819252f8 100644 --- a/spec/models/integrations/campfire_spec.rb +++ b/spec/models/integrations/campfire_spec.rb @@ -48,7 +48,7 @@ RSpec.describe Integrations::Campfire, feature_category: :integrations do ) @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) @rooms_url = 'https://project-name.campfirenow.com/rooms.json' - @auth = %w(verySecret X) + @auth = %w[verySecret X] @headers = { 'Content-Type' => 'application/json; charset=utf-8' } end diff --git a/spec/models/integrations/integration_list_spec.rb b/spec/models/integrations/integration_list_spec.rb index b7ccbcecf6b..4bb7b100bc0 100644 --- a/spec/models/integrations/integration_list_spec.rb +++ b/spec/models/integrations/integration_list_spec.rb @@ -12,10 +12,10 @@ RSpec.describe Integrations::IntegrationList, feature_category: :integrations do describe '#to_array' do it 'returns array of Integration, columns, and values' do - expect(subject.to_array).to eq([ + expect(subject.to_array).to match_array([ Integration, %w[active category project_id], - [['true', 'common', projects.first.id], ['true', 'common', projects.second.id]] + contain_exactly(['true', 'common', projects.first.id], ['true', 'common', projects.second.id]) ]) end end diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index c87128db221..af021c51035 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -603,6 +603,17 @@ RSpec.describe Integrations::Jira, feature_category: :integrations do jira_integration.client.get('/foo') end + context 'when a custom read_timeout option is passed as an argument' do + it 'uses the default GitLab::HTTP timeouts plus a custom read_timeout' do + expected_timeouts = Gitlab::HTTP::DEFAULT_TIMEOUT_OPTIONS.merge(read_timeout: 2.minutes, timeout: 2.minutes) + + expect(Gitlab::HTTP_V2::Client).to receive(:httparty_perform_request) + .with(Net::HTTP::Get, '/foo', hash_including(expected_timeouts)).and_call_original + + jira_integration.client(read_timeout: 2.minutes).get('/foo') + end + end + context 'with basic auth' do before do jira_integration.jira_auth_type = 0 @@ -719,10 +730,10 @@ RSpec.describe Integrations::Jira, feature_category: :integrations do allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return(issue_key) allow(JIRA::Resource::Remotelink).to receive(:all).and_return([]) - WebMock.stub_request(:get, issue_url).with(basic_auth: %w(jira-username jira-password)) - WebMock.stub_request(:post, transitions_url).with(basic_auth: %w(jira-username jira-password)) - WebMock.stub_request(:post, comment_url).with(basic_auth: %w(jira-username jira-password)) - WebMock.stub_request(:post, remote_link_url).with(basic_auth: %w(jira-username jira-password)) + WebMock.stub_request(:get, issue_url).with(basic_auth: %w[jira-username jira-password]) + WebMock.stub_request(:post, transitions_url).with(basic_auth: %w[jira-username jira-password]) + WebMock.stub_request(:post, comment_url).with(basic_auth: %w[jira-username jira-password]) + WebMock.stub_request(:post, remote_link_url).with(basic_auth: %w[jira-username jira-password]) end let(:external_issue) { ExternalIssue.new('JIRA-123', project) } @@ -864,7 +875,7 @@ RSpec.describe Integrations::Jira, feature_category: :integrations do it 'logs exception when transition id is not valid' do allow(jira_integration).to receive(:log_exception) - WebMock.stub_request(:post, transitions_url).with(basic_auth: %w(jira-username jira-password)).and_raise("Bad Request") + WebMock.stub_request(:post, transitions_url).with(basic_auth: %w[jira-username jira-password]).and_raise("Bad Request") close_issue @@ -973,7 +984,7 @@ RSpec.describe Integrations::Jira, feature_category: :integrations do context 'when a transition fails' do before do - WebMock.stub_request(:post, transitions_url).with(basic_auth: %w(jira-username jira-password)).to_return do |request| + WebMock.stub_request(:post, transitions_url).with(basic_auth: %w[jira-username jira-password]).to_return do |request| { status: request.body.include?('"id":"2"') ? 500 : 200 } end end diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index c4c7202fae0..1537b10ba03 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -308,7 +308,7 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do def stub_post_to_build_queue(branch:) teamcity_full_url = "#{teamcity_url}/httpAuth/app/rest/buildQueue" body ||= %(<build branchName=\"#{branch}\"><buildType id=\"foo\"/></build>) - auth = %w(mic password) + auth = %w[mic password] stub_full_request(teamcity_full_url, method: :post).with( basic_auth: auth, @@ -320,7 +320,7 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do end def stub_request(status: 200, body: nil, build_status: 'success') - auth = %w(mic password) + auth = %w[mic password] body ||= %({"build":{"status":"#{build_status}","id":"666"}}) diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index e7a5a53c6a0..6c8603d7b4c 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -373,10 +373,10 @@ RSpec.describe Issue, feature_category: :team_planning do describe '.simple_sorts' do it 'includes all keys' do expect(described_class.simple_sorts.keys).to include( - *%w(created_asc created_at_asc created_date created_desc created_at_desc + *%w[created_asc created_at_asc created_date created_desc created_at_desc closest_future_date closest_future_date_asc due_date due_date_asc due_date_desc id_asc id_desc relative_position relative_position_asc updated_desc updated_asc - updated_at_asc updated_at_desc title_asc title_desc)) + updated_at_asc updated_at_desc title_asc title_desc]) end end @@ -390,7 +390,7 @@ RSpec.describe Issue, feature_category: :team_planning do end it 'returns issues with the given issue types' do - expect(described_class.with_issue_type(%w(issue incident))) + expect(described_class.with_issue_type(%w[issue incident])) .to contain_exactly(issue, incident) end @@ -439,7 +439,7 @@ RSpec.describe Issue, feature_category: :team_planning do end it 'returns issues without the given issue types' do - expect(described_class.without_issue_type(%w(issue incident))) + expect(described_class.without_issue_type(%w[issue incident])) .to contain_exactly(task) end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index fdd8a610fe4..b4941c71d6a 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -591,6 +591,22 @@ RSpec.describe Member, feature_category: :groups_and_projects do it { is_expected.not_to include @member_with_minimal_access } it { is_expected.not_to include awaiting_group_member } it { is_expected.not_to include awaiting_project_member } + + context 'when minimal_access is true' do + subject { described_class.without_invites_and_requests(minimal_access: true) } + + it { is_expected.to include @owner } + it { is_expected.to include @maintainer } + it { is_expected.not_to include @invited_member } + it { is_expected.to include @accepted_invite_member } + it { is_expected.not_to include @requested_member } + it { is_expected.to include @accepted_request_member } + it { is_expected.to include @blocked_maintainer } + it { is_expected.to include @blocked_developer } + it { is_expected.to include @member_with_minimal_access } + it { is_expected.not_to include awaiting_group_member } + it { is_expected.not_to include awaiting_project_member } + end end describe '.connected_to_user' do diff --git a/spec/models/members/members/members_with_parents_spec.rb b/spec/models/members/members/members_with_parents_spec.rb new file mode 100644 index 00000000000..46c934c932f --- /dev/null +++ b/spec/models/members/members/members_with_parents_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Members::MembersWithParents, feature_category: :groups_and_projects do + let_it_be(:group) { create(:group, :nested) } + let_it_be(:maintainer) { group.parent.add_maintainer(create(:user)) } + let_it_be(:developer) { group.add_developer(create(:user)) } + let_it_be(:pending_maintainer) { create(:group_member, :awaiting, :maintainer, group: group.parent) } + let_it_be(:pending_developer) { create(:group_member, :awaiting, :developer, group: group) } + let_it_be(:invited_member) { create(:group_member, :invited, group: group) } + let_it_be(:inactive_developer) { group.add_developer(create(:user, :deactivated)) } + let_it_be(:minimal_access) { create(:group_member, :minimal_access, group: group) } + + describe '#all_members' do + subject(:all_members) { described_class.new(group).all_members } + + it 'returns all members for group and group parents' do + expect(all_members).to contain_exactly( + developer, + maintainer, + pending_maintainer, + pending_developer, + invited_member, + inactive_developer, + minimal_access + ) + end + end + + describe '#members' do + let(:arguments) { {} } + + subject(:members) { described_class.new(group).members(**arguments) } + + using Rspec::Parameterized::TableSyntax + + where(:arguments, :expected_members) do + [ + [ + {}, + lazy { [developer, maintainer, inactive_developer] } + ], + [ + # minimal access is Premium, so in FOSS we will not include minimal access member + { minimal_access: true }, + lazy { [developer, maintainer, inactive_developer] } + ], + [ + { active_users: true }, + lazy { [developer, maintainer] } + ] + ] + end + + with_them do + it 'returns expected members' do + expect(members).to contain_exactly(*expected_members) + expect(members).not_to include(*(group.members - expected_members)) + end + end + + context 'when active_users: true and minimal_access: true' do + let(:arguments) { { active_users: true, minimal_access: true } } + + it 'raises an error' do + expect { members }.to raise_error(ArgumentError) + end + end + + context 'with group sharing' do + let_it_be(:shared_with_group) { create(:group) } + + let_it_be(:shared_with_group_maintainer) do + shared_with_group.add_maintainer(create(:user)) + end + + let_it_be(:shared_with_group_developer) do + shared_with_group.add_developer(create(:user)) + end + + before do + create(:group_group_link, shared_group: group, shared_with_group: shared_with_group) + end + + it 'returns shared with group members' do + expect(members).to(include(shared_with_group_maintainer)) + expect(members).to(include(shared_with_group_developer)) + end + end + end +end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index a2b5bde8890..a9725a796bf 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -51,6 +51,50 @@ RSpec.describe ProjectMember, feature_category: :groups_and_projects do end end + describe '.permissible_access_level_roles_for_project_access_token' do + let_it_be(:owner) { create(:user) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:developer) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + + before do + project.add_owner(owner) + project.add_maintainer(maintainer) + project.add_developer(developer) + end + + subject(:access_levels) { described_class.permissible_access_level_roles_for_project_access_token(user, project) } + + context 'when member can manage owners' do + let(:user) { owner } + + it 'returns Gitlab::Access.options_with_owner' do + expect(access_levels).to eq(Gitlab::Access.options_with_owner) + end + end + + context 'when member cannot manage owners' do + let(:user) { maintainer } + + it 'returns Gitlab::Access.options' do + expect(access_levels).to eq(Gitlab::Access.options) + end + end + + context 'when the user is a developer' do + let(:user) { developer } + + it 'returns Gitlab::Access.options' do + expect(access_levels).to eq({ + "Guest" => 10, + "Reporter" => 20, + "Developer" => 30 + }) + end + end + end + describe '#real_source_type' do subject { create(:project_member).real_source_type } diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 806ce3f21b5..bcab2029942 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -1090,7 +1090,7 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end it 'returns affected file paths' do - expect(subject.modified_paths).to eq(%w{foo bar baz}) + expect(subject.modified_paths).to eq(%w[foo bar baz]) end context "when fallback_on_overflow is true" do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 40f85c92851..d3c32da2842 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -725,22 +725,35 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end - describe '.recent_target_branches' do + describe '.recent_target_branches and .recent_source_branches' do + def create_mr(source_branch, target_branch, status, remove_source_branch = false) + if remove_source_branch + create(:merge_request, status, :remove_source_branch, source_project: project, + target_branch: target_branch, source_branch: source_branch) + else + create(:merge_request, status, source_project: project, + target_branch: target_branch, source_branch: source_branch) + end + end + let(:project) { create(:project) } - let!(:merge_request1) { create(:merge_request, :opened, source_project: project, target_branch: 'feature') } - let!(:merge_request2) { create(:merge_request, :closed, source_project: project, target_branch: 'merge-test') } - let!(:merge_request3) { create(:merge_request, :opened, source_project: project, target_branch: 'fix') } - let!(:merge_request4) { create(:merge_request, :closed, source_project: project, target_branch: 'feature') } + let!(:merge_request1) { create_mr('source1', 'target1', :opened) } + let!(:merge_request2) { create_mr('source2', 'target2', :closed) } + let!(:merge_request3) { create_mr('source3', 'target3', :opened) } + let!(:merge_request4) { create_mr('source4', 'target1', :closed) } + let!(:merge_request5) { create_mr('source5', 'target4', :merged, true) } before do merge_request1.update_columns(updated_at: 1.day.since) merge_request2.update_columns(updated_at: 2.days.since) merge_request3.update_columns(updated_at: 3.days.since) merge_request4.update_columns(updated_at: 4.days.since) + merge_request5.update_columns(updated_at: 5.days.since) end - it 'returns target branches sort by updated at desc' do - expect(described_class.recent_target_branches).to match_array(%w[feature merge-test fix]) + it 'returns branches sort by updated at desc' do + expect(described_class.recent_target_branches).to match_array(%w[target1 target2 target3 target4]) + expect(described_class.recent_source_branches).to match_array(%w[source1 source2 source3 source4 source5]) end end @@ -3324,6 +3337,7 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev context 'with skip_ci_check option' do before do + allow(subject.project).to receive(:only_allow_merge_if_pipeline_succeeds?).and_return(true) allow(subject).to receive_messages(check_mergeability: nil, can_be_merged?: true, broken?: false) end @@ -3345,6 +3359,8 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev context 'with skip_discussions_check option' do before do + allow(subject.project).to receive(:only_allow_merge_if_all_discussions_are_resolved?).and_return(true) + allow(subject).to receive_messages( mergeable_ci_state?: true, check_mergeability: nil, @@ -3380,6 +3396,8 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev context 'with skip_rebase_check option' do before do + allow(subject.project).to receive(:ff_merge_must_be_possible?).and_return(true) + allow(subject).to receive_messages( mergeable_state?: true, check_mergeability: nil, @@ -3525,6 +3543,7 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev context 'when failed' do context 'when #mergeable_ci_state? is false' do before do + allow(subject.project).to receive(:only_allow_merge_if_pipeline_succeeds?) { true } allow(subject).to receive(:mergeable_ci_state?) { false } end @@ -3539,6 +3558,7 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev context 'when #mergeable_discussions_state? is false' do before do + allow(subject.project).to receive(:only_allow_merge_if_all_discussions_are_resolved?) { true } allow(subject).to receive(:mergeable_discussions_state?) { false } end @@ -6016,12 +6036,10 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev allow(merge_request_diff).to receive(:patch_id_sha).and_return(nil) allow(merge_request).to receive(:diff_refs).and_return(diff_refs) - allow_next_instance_of(Repository) do |repo| - allow(repo) - .to receive(:get_patch_id) - .with(diff_refs.base_sha, diff_refs.head_sha) - .and_return(patch_id) - end + allow(merge_request.project.repository) + .to receive(:get_patch_id) + .with(diff_refs.base_sha, diff_refs.head_sha) + .and_return(patch_id) end it { is_expected.to eq(patch_id) } @@ -6065,4 +6083,54 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev expect(merge_request.all_mergeability_checks_results).to eq(result.payload[:results]) end end + + describe '#only_allow_merge_if_pipeline_succeeds?' do + let(:merge_request) { build_stubbed(:merge_request) } + + subject(:result) { merge_request.only_allow_merge_if_pipeline_succeeds? } + + before do + allow(merge_request.project) + .to receive(:only_allow_merge_if_pipeline_succeeds?) + .with(inherit_group_setting: true) + .and_return(only_allow_merge_if_pipeline_succeeds?) + end + + context 'when associated project only_allow_merge_if_pipeline_succeeds? returns true' do + let(:only_allow_merge_if_pipeline_succeeds?) { true } + + it { is_expected.to eq(true) } + end + + context 'when associated project only_allow_merge_if_pipeline_succeeds? returns false' do + let(:only_allow_merge_if_pipeline_succeeds?) { false } + + it { is_expected.to eq(false) } + end + end + + describe '#only_allow_merge_if_all_discussions_are_resolved?' do + let(:merge_request) { build_stubbed(:merge_request) } + + subject(:result) { merge_request.only_allow_merge_if_all_discussions_are_resolved? } + + before do + allow(merge_request.project) + .to receive(:only_allow_merge_if_all_discussions_are_resolved?) + .with(inherit_group_setting: true) + .and_return(only_allow_merge_if_all_discussions_are_resolved?) + end + + context 'when associated project only_allow_merge_if_all_discussions_are_resolved? returns true' do + let(:only_allow_merge_if_all_discussions_are_resolved?) { true } + + it { is_expected.to eq(true) } + end + + context 'when associated project only_allow_merge_if_all_discussions_are_resolved? returns false' do + let(:only_allow_merge_if_all_discussions_are_resolved?) { false } + + it { is_expected.to eq(false) } + end + end end diff --git a/spec/models/ml/candidate_spec.rb b/spec/models/ml/candidate_spec.rb index fa19b723ee2..d5b71e2c3f7 100644 --- a/spec/models/ml/candidate_spec.rb +++ b/spec/models/ml/candidate_spec.rb @@ -16,6 +16,7 @@ RSpec.describe Ml::Candidate, factory_default: :keep, feature_category: :mlops d it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:package) } it { is_expected.to belong_to(:ci_build).class_name('Ci::Build') } + it { is_expected.to belong_to(:model_version).class_name('Ml::ModelVersion') } it { is_expected.to have_many(:params) } it { is_expected.to have_many(:metrics) } it { is_expected.to have_many(:metadata) } @@ -35,6 +36,45 @@ RSpec.describe Ml::Candidate, factory_default: :keep, feature_category: :mlops d it { expect(described_class.new.eid).to be_present } end + describe 'validation' do + let_it_be(:model) { create(:ml_models, project: candidate.project) } + let_it_be(:model_version1) { create(:ml_model_versions, model: model) } + let_it_be(:model_version2) { create(:ml_model_versions, model: model) } + let_it_be(:validation_candidate) do + create(:ml_candidates, model_version: model_version1, project: candidate.project) + end + + let(:params) do + { + model_version: nil + } + end + + subject(:errors) do + candidate = described_class.new(**params) + candidate.validate + candidate.errors + end + + describe 'model_version' do + context 'when model_version is nil' do + it { expect(errors).not_to include(:model_version_id) } + end + + context 'when no other candidate is associated to the model_version' do + let(:params) { { model_version: model_version2 } } + + it { expect(errors).not_to include(:model_version_id) } + end + + context 'when another candidate has model_version_id' do + let(:params) { { model_version: validation_candidate.model_version } } + + it { expect(errors).to include(:model_version_id) } + end + end + end + describe '.destroy' do let_it_be(:candidate_to_destroy) do create(:ml_candidates, :with_metrics_and_params, :with_metadata, :with_artifact) diff --git a/spec/models/ml/model_metadata_spec.rb b/spec/models/ml/model_metadata_spec.rb new file mode 100644 index 00000000000..f06c7a2ce50 --- /dev/null +++ b/spec/models/ml/model_metadata_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ml::ModelMetadata, feature_category: :mlops do + describe 'associations' do + it { is_expected.to belong_to(:model).required } + end + + describe 'validations' do + let_it_be(:metadata) { create(:ml_model_metadata, name: 'some_metadata') } + let_it_be(:model) { metadata.model } + + it 'is unique within the model' do + expect do + model.metadata.create!(name: 'some_metadata', value: 'blah') + end.to raise_error.with_message(/Name 'some_metadata' already taken/) + end + + it 'a model is required' do + expect do + described_class.create!(name: 'some_metadata', value: 'blah') + end.to raise_error.with_message(/Model must exist/) + end + end +end diff --git a/spec/models/ml/model_spec.rb b/spec/models/ml/model_spec.rb index e22989f3ce2..ae7c3f163f3 100644 --- a/spec/models/ml/model_spec.rb +++ b/spec/models/ml/model_spec.rb @@ -14,6 +14,7 @@ RSpec.describe Ml::Model, feature_category: :mlops do it { is_expected.to belong_to(:project) } it { is_expected.to have_one(:default_experiment) } it { is_expected.to have_many(:versions) } + it { is_expected.to have_many(:metadata) } it { is_expected.to have_one(:latest_version).class_name('Ml::ModelVersion').inverse_of(:model) } end @@ -77,45 +78,11 @@ RSpec.describe Ml::Model, feature_category: :mlops do end end - describe '.find_or_create' do - subject(:find_or_create) { described_class.find_or_create(project, name, experiment) } + describe '.including_project' do + subject { described_class.including_project } - let(:name) { existing_model.name } - let(:project) { existing_model.project } - let(:experiment) { default_experiment } - - context 'when model name does not exist in the project' do - let(:name) { 'new_model' } - let(:experiment) { build(:ml_experiments, name: name, project: project) } - - it 'creates a model', :aggregate_failures do - expect { find_or_create }.to change { Ml::Model.count }.by(1) - - expect(find_or_create.name).to eq(name) - expect(find_or_create.project).to eq(project) - expect(find_or_create.default_experiment).to eq(experiment) - end - end - - context 'when model name exists but project is different' do - let(:project) { create(:project) } - let(:experiment) { build(:ml_experiments, name: name, project: project) } - - it 'creates a model', :aggregate_failures do - expect { find_or_create }.to change { Ml::Model.count }.by(1) - - expect(find_or_create.name).to eq(name) - expect(find_or_create.project).to eq(project) - expect(find_or_create.default_experiment).to eq(experiment) - end - end - - context 'when model exists' do - it 'fetches existing model', :aggregate_failures do - expect { find_or_create }.not_to change { Ml::Model.count } - - expect(find_or_create).to eq(existing_model) - end + it 'loads latest version' do + expect(subject.first.association_cached?(:project)).to be(true) end end diff --git a/spec/models/ml/model_version_spec.rb b/spec/models/ml/model_version_spec.rb index 83639fca9e1..95d4a545f52 100644 --- a/spec/models/ml/model_version_spec.rb +++ b/spec/models/ml/model_version_spec.rb @@ -18,6 +18,7 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do it { is_expected.to belong_to(:project) } 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') } end describe 'validation' do @@ -26,11 +27,15 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do build_stubbed(:ml_model_package, project: base_project, version: valid_version, name: model1.name) end + let_it_be(:valid_description) { 'Valid description' } + let(:package) { valid_package } let(:version) { valid_version } + let(:description) { valid_description } subject(:errors) do - mv = described_class.new(version: version, model: model1, package: package, project: model1.project) + mv = described_class.new(version: version, model: model1, package: package, project: model1.project, + description: description) mv.validate mv.errors end @@ -60,6 +65,14 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do end end + describe 'description' do + context 'when description is too large' do + let(:description) { 'a' * 501 } + + it { expect(errors).to include(:description) } + end + end + describe 'model' do context 'when project is different' do before do @@ -91,8 +104,9 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do let(:version) { existing_model_version.version } let(:package) { nil } + let(:description) { 'Some description' } - subject(:find_or_create) { described_class.find_or_create!(model1, version, package) } + subject(:find_or_create) { described_class.find_or_create!(model1, version, package, description) } context 'if model version exists' do it 'returns the model version', :aggregate_failures do @@ -111,11 +125,66 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do expect(model_version.version).to eq(version) expect(model_version.model).to eq(model1) + expect(model_version.description).to eq(description) expect(model_version.package).to eq(package) end end end + describe '#by_project_id_and_id' do + let(:id) { model_version1.id } + let(:project_id) { model_version1.project.id } + + subject { described_class.by_project_id_and_id(project_id, id) } + + context 'if exists' do + it { is_expected.to eq(model_version1) } + end + + context 'if id has no match' do + let(:id) { non_existing_record_id } + + it { is_expected.to be(nil) } + end + + context 'if project id does not match' do + let(:project_id) { non_existing_record_id } + + it { is_expected.to be(nil) } + end + end + + describe '.by_project_id_name_and_version' do + let(:version) { model_version1.version } + let(:project_id) { model_version1.project.id } + let(:model_name) { model_version1.model.name } + let_it_be(:latest_version) { create(:ml_model_versions, model: model_version1.model) } + + subject { described_class.by_project_id_name_and_version(project_id, model_name, version) } + + context 'if exists' do + it { is_expected.to eq(model_version1) } + end + + context 'if id has no match' do + let(:version) { non_existing_record_id } + + it { is_expected.to be(nil) } + end + + context 'if project id does not match' do + let(:project_id) { non_existing_record_id } + + it { is_expected.to be(nil) } + end + + context 'if model name does not match' do + let(:model_name) { non_existing_record_id } + + it { is_expected.to be(nil) } + end + end + describe '.order_by_model_id_id_desc' do subject { described_class.order_by_model_id_id_desc } diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index e9822d97447..c7449e047b0 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -216,32 +216,54 @@ RSpec.describe NamespaceSetting, feature_category: :groups_and_projects, type: : end end - describe '#emails_enabled?' do - context 'when a group has no parent' - let(:settings) { create(:namespace_settings, emails_enabled: true) } - let(:grandparent) { create(:group) } + context 'when a group has parent groups' do + let(:grandparent) { create(:group, namespace_settings: settings) } let(:parent) { create(:group, parent: grandparent) } - let(:group) { create(:group, parent: parent, namespace_settings: settings) } + let!(:group) { create(:group, parent: parent) } - context 'when the groups setting is changed' do - it 'returns false when the attribute is false' do - group.update_attribute(:emails_disabled, true) + context "when a parent group has disabled diff previews" do + let(:settings) { create(:namespace_settings, show_diff_preview_in_email: false) } - expect(group.emails_enabled?).to be_falsey + it 'returns false' do + expect(group.show_diff_preview_in_email?).to be_falsey end end - context 'when a group has a parent' do - it 'returns true when no parent has disabled emails' do - expect(group.emails_enabled?).to be_truthy + context 'when all parent groups have enabled diff previews' do + let(:settings) { create(:namespace_settings, show_diff_preview_in_email: true) } + + it 'returns true' do + expect(group.show_diff_preview_in_email?).to be_truthy end + end + end + end - context 'when ancestor emails are disabled' do - it 'returns false' do - grandparent.update_attribute(:emails_disabled, true) + describe '#emails_enabled?' do + context 'when a group has no parent' + let(:settings) { create(:namespace_settings, emails_enabled: true) } + let(:grandparent) { create(:group) } + let(:parent) { create(:group, parent: grandparent) } + let(:group) { create(:group, parent: parent, namespace_settings: settings) } - expect(group.emails_enabled?).to be_falsey - end + context 'when the groups setting is changed' do + it 'returns false when the attribute is false' do + group.update_attribute(:emails_enabled, false) + + expect(group.emails_enabled?).to be_falsey + end + end + + context 'when a group has a parent' do + it 'returns true when no parent has disabled emails' do + expect(group.emails_enabled?).to be_truthy + end + + context 'when ancestor emails are disabled' do + it 'returns false' do + grandparent.update_attribute(:emails_enabled, false) + + expect(group.emails_enabled?).to be_falsey end end end @@ -251,19 +273,19 @@ RSpec.describe NamespaceSetting, feature_category: :groups_and_projects, type: : let(:parent) { create(:group, parent: grandparent) } let!(:group) { create(:group, parent: parent) } - context "when a parent group has disabled diff previews" do - let(:settings) { create(:namespace_settings, show_diff_preview_in_email: false) } + context "when a parent group has emails disabled" do + let(:settings) { create(:namespace_settings, emails_enabled: false) } it 'returns false' do - expect(group.show_diff_preview_in_email?).to be_falsey + expect(group.emails_enabled?).to be_falsey end end - context 'when all parent groups have enabled diff previews' do - let(:settings) { create(:namespace_settings, show_diff_preview_in_email: true) } + context 'when all parent groups have emails enabled' do + let(:settings) { create(:namespace_settings, emails_enabled: true) } it 'returns true' do - expect(group.show_diff_preview_in_email?).to be_truthy + expect(group.emails_enabled?).to be_truthy end end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 9974aac3c6c..85569a68252 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -569,6 +569,48 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end end end + + describe "#default_branch_protection_settings" do + let(:default_branch_protection_defaults) { {} } + let(:namespace_setting) { create(:namespace_settings, default_branch_protection_defaults: default_branch_protection_defaults) } + let(:namespace) { create(:namespace, namespace_settings: namespace_setting) } + let(:group) { create(:group, namespace_settings: namespace_setting) } + + before do + stub_application_setting(default_branch_protection_defaults: Gitlab::Access::BranchProtection.protected_against_developer_pushes) + end + + context 'for a namespace' do + it 'returns the instance level setting' do + expected_settings = Gitlab::Access::BranchProtection.protected_against_developer_pushes.deep_stringify_keys + settings = namespace.default_branch_protection_settings.to_hash + + expect(settings).to eq(expected_settings) + end + end + + context 'for a group' do + context 'that has not altered the default value' do + it 'returns the instance level setting' do + expected_settings = Gitlab::Access::BranchProtection.protected_against_developer_pushes.deep_stringify_keys + settings = group.default_branch_protection_settings.to_hash + + expect(settings).to eq(expected_settings) + end + end + + context 'that has altered the default value' do + let(:default_branch_protection_defaults) { Gitlab::Access::BranchProtection.protected_fully.deep_stringify_keys } + + it 'returns the group level setting' do + expected_settings = default_branch_protection_defaults + settings = group.default_branch_protection_settings.to_hash + + expect(settings).to eq(expected_settings) + end + end + end + end end describe "Respond to" do @@ -1863,20 +1905,22 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do describe '#emails_disabled?' do context 'when not a subgroup' do + let(:group) { create(:group) } + it 'returns false' do - group = create(:group, emails_disabled: false) + group.update_attribute(:emails_enabled, true) expect(group.emails_disabled?).to be_falsey end it 'returns true' do - group = create(:group, emails_disabled: true) + group.update_attribute(:emails_enabled, false) expect(group.emails_disabled?).to be_truthy end it 'does not query the db when there is no parent group' do - group = create(:group, emails_disabled: true) + group.update_attribute(:emails_enabled, false) expect { group.emails_disabled? }.not_to exceed_query_limit(0) end @@ -1903,7 +1947,8 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do describe '#emails_enabled?' do context 'without a persisted namespace_setting object' do - let(:group) { build(:group, emails_disabled: false) } + let(:group_settings) { create(:namespace_settings) } + let(:group) { build(:group, emails_disabled: false, namespace_settings: group_settings) } it "is the opposite of emails_disabled" do expect(group.emails_enabled?).to be_truthy @@ -1928,7 +1973,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do parent_2 = create(:group) # No projects create(:project, group: child_1_1).tap do |project| - project.pages_metadatum.update!(deployed: true) + create(:pages_deployment, project: project) end create(:project, group: child_1_1) diff --git a/spec/models/namespace_statistics_spec.rb b/spec/models/namespace_statistics_spec.rb index ac747b70a9f..ee3b4765dba 100644 --- a/spec/models/namespace_statistics_spec.rb +++ b/spec/models/namespace_statistics_spec.rb @@ -171,7 +171,7 @@ RSpec.describe NamespaceStatistics do context 'when other columns are updated' do it 'does not enqueue the job to update root storage statistics' do - columns_to_update = NamespaceStatistics.columns_hash.reject { |k, _| %w(id namespace_id).include?(k) || k.include?('_size') }.keys + columns_to_update = NamespaceStatistics.columns_hash.reject { |k, _| %w[id namespace_id].include?(k) || k.include?('_size') }.keys columns_to_update.each { |c| statistics[c] = 10 } expect(statistics).not_to receive(:update_root_storage_statistics) diff --git a/spec/models/network/graph_spec.rb b/spec/models/network/graph_spec.rb index d0c73d6285c..3bee7225df5 100644 --- a/spec/models/network/graph_spec.rb +++ b/spec/models/network/graph_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' RSpec.describe Network::Graph, feature_category: :source_code_management do let(:project) { create(:project, :repository) } - let!(:note_on_commit) { create(:note_on_commit, project: project) } describe '#initialize' do let(:graph) do @@ -14,16 +13,6 @@ RSpec.describe Network::Graph, feature_category: :source_code_management do it 'has initialized' do expect(graph).to be_a(described_class) end - - context 'when disable_network_graph_note_counts is disabled' do - before do - stub_feature_flags(disable_network_graph_notes_count: false) - end - - it 'initializes the notes hash' do - expect(graph.notes).to eq({ note_on_commit.commit_id => 1 }) - end - end end describe '#commits' do diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb index 2275bea4c7f..f19c0a68f87 100644 --- a/spec/models/notification_recipient_spec.rb +++ b/spec/models/notification_recipient_spec.rb @@ -14,7 +14,7 @@ RSpec.describe NotificationRecipient, feature_category: :team_planning do context 'when emails are disabled' do it 'returns false if group disabled' do - expect(project.namespace).to receive(:emails_disabled?).and_return(true) + expect(project.namespace).to receive(:emails_enabled?).and_return(false) expect(recipient).to receive(:emails_disabled?).and_call_original expect(recipient.notifiable?).to eq false end @@ -28,7 +28,7 @@ RSpec.describe NotificationRecipient, feature_category: :team_planning do context 'when emails are enabled' do it 'returns true if group enabled' do - expect(project.namespace).to receive(:emails_disabled?).and_return(false) + expect(project.namespace).to receive(:emails_enabled?).and_return(true) expect(recipient).to receive(:emails_disabled?).and_call_original expect(recipient.notifiable?).to eq true end diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb index 2f9f04fd3e6..0670002135c 100644 --- a/spec/models/organizations/organization_spec.rb +++ b/spec/models/organizations/organization_spec.rb @@ -181,4 +181,13 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel 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' + + expect(Gitlab::UrlBuilder).to receive(:build).with(organization, only_path: nil).and_return(web_url) + expect(organization.web_url).to eq(web_url) + end + end end diff --git a/spec/models/packages/npm/metadata_cache_spec.rb b/spec/models/packages/npm/metadata_cache_spec.rb index 94b41ab6a5e..3a6c87a4244 100644 --- a/spec/models/packages/npm/metadata_cache_spec.rb +++ b/spec/models/packages/npm/metadata_cache_spec.rb @@ -148,4 +148,40 @@ RSpec.describe Packages::Npm::MetadataCache, type: :model, feature_category: :pa end end end + + describe '.stale' do + let_it_be(:npm_metadata_cache) { create(:npm_metadata_cache) } + let_it_be(:npm_metadata_cache_stale) { create(:npm_metadata_cache, :stale) } + + subject { described_class.stale } + + it { is_expected.to contain_exactly(npm_metadata_cache_stale) } + end + + describe '.pending_destruction' do + let_it_be(:npm_metadata_cache) { create(:npm_metadata_cache) } + let_it_be(:npm_metadata_cache_stale_default) { create(:npm_metadata_cache, :stale) } + let_it_be(:npm_metadata_cache_stale_processing) { create(:npm_metadata_cache, :stale, :processing) } + + subject { described_class.pending_destruction } + + it { is_expected.to contain_exactly(npm_metadata_cache_stale_default) } + end + + describe '.next_pending_destruction' do + let_it_be(:npm_metadata_cache1) { create(:npm_metadata_cache, created_at: 1.month.ago, updated_at: 1.day.ago) } + let_it_be(:npm_metadata_cache2) { create(:npm_metadata_cache, created_at: 1.year.ago, updated_at: 1.year.ago) } + + let_it_be(:npm_metadata_cache3) do + create(:npm_metadata_cache, :stale, created_at: 2.years.ago, updated_at: 1.month.ago) + end + + let_it_be(:npm_metadata_cache4) do + create(:npm_metadata_cache, :stale, created_at: 3.years.ago, updated_at: 2.weeks.ago) + end + + it 'returns the oldest pending destruction item based on updated_at' do + expect(described_class.next_pending_destruction(order_by: :updated_at)).to eq(npm_metadata_cache3) + end + end end diff --git a/spec/models/packages/nuget/symbol_spec.rb b/spec/models/packages/nuget/symbol_spec.rb index 52e95c11939..f43f3a3bdeb 100644 --- a/spec/models/packages/nuget/symbol_spec.rb +++ b/spec/models/packages/nuget/symbol_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Packages::Nuget::Symbol, type: :model, feature_category: :package subject(:symbol) { create(:nuget_symbol) } it { is_expected.to be_a FileStoreMounter } + it { is_expected.to be_a ShaAttribute } describe 'relationships' do it { is_expected.to belong_to(:package).inverse_of(:nuget_symbols) } diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index e113218e828..8e3b97e55f3 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -1196,7 +1196,7 @@ RSpec.describe Packages::Package, type: :model, feature_category: :package_regis it { is_expected.to eq([]) } context 'with tags' do - let(:tags) { %w(tag1 tag2 tag3) } + let(:tags) { %w[tag1 tag2 tag3] } before do tags.each { |t| create(:packages_tag, name: t, package: package) } diff --git a/spec/models/packages/protection/rule_spec.rb b/spec/models/packages/protection/rule_spec.rb index 320c265239c..3f0aefa945a 100644 --- a/spec/models/packages/protection/rule_spec.rb +++ b/spec/models/packages/protection/rule_spec.rb @@ -34,6 +34,32 @@ RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :pack it { is_expected.to validate_presence_of(:package_name_pattern) } 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) } + 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.')) + ) + } + end end describe '#package_type' do @@ -51,14 +77,13 @@ RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :pack context 'with different package name patterns' do where(:package_name_pattern, :expected_pattern_query) do - '@my-scope/my-package' | '@my-scope/my-package' - '*@my-scope/my-package-with-wildcard-start' | '%@my-scope/my-package-with-wildcard-start' - '@my-scope/my-package-with-wildcard-end*' | '@my-scope/my-package-with-wildcard-end%' - '@my-scope/*my-package-with-wildcard-inbetween' | '@my-scope/%my-package-with-wildcard-inbetween' - '**@my-scope/**my-package-with-wildcard-multiple**' | '%%@my-scope/%%my-package-with-wildcard-multiple%%' - '@my-scope/my-package-with_____underscore' | '@my-scope/my-package-with\_\_\_\_\_underscore' - '@my-scope/my-package-with-percent-sign-%' | '@my-scope/my-package-with-percent-sign-\%' - '@my-scope/my-package-with-regex-characters.+' | '@my-scope/my-package-with-regex-characters.+' + '@my-scope/my-package' | '@my-scope/my-package' + '@my-scope/*my-package-with-wildcard-start' | '@my-scope/%my-package-with-wildcard-start' + '@my-scope/my-package-with-wildcard-end*' | '@my-scope/my-package-with-wildcard-end%' + '@my-scope/my-package*with-wildcard-inbetween' | '@my-scope/my-package%with-wildcard-inbetween' + '@my-scope/**my-package-**-with-wildcard-multiple**' | '@my-scope/%%my-package-%%-with-wildcard-multiple%%' + '@my-scope/my-package-with_____underscore' | '@my-scope/my-package-with\_\_\_\_\_underscore' + '@my-scope/my-package-with-regex-characters.+' | '@my-scope/my-package-with-regex-characters.+' end with_them do @@ -74,7 +99,7 @@ RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :pack end let_it_be(:ppr_with_wildcard_start) do - create(:package_protection_rule, package_name_pattern: '*@my-scope/my_package-with-wildcard-start') + create(:package_protection_rule, package_name_pattern: '@my-scope/*my_package-with-wildcard-start') end let_it_be(:ppr_with_wildcard_end) do @@ -82,11 +107,11 @@ RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :pack end let_it_be(:ppr_with_wildcard_inbetween) do - create(:package_protection_rule, package_name_pattern: '@my-scope/*my_package-with-wildcard-inbetween') + create(:package_protection_rule, package_name_pattern: '@my-scope/my_package*with-wildcard-inbetween') end let_it_be(:ppr_with_wildcard_multiples) do - create(:package_protection_rule, package_name_pattern: '**@my-scope/**my_package-with-wildcard-multiple**') + create(:package_protection_rule, package_name_pattern: '@my-scope/**my_package**with-wildcard-multiple**') end let_it_be(:ppr_with_underscore) do @@ -103,46 +128,47 @@ RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :pack context 'with several package protection rule scenarios' do where(:package_name, :expected_package_protection_rules) do - '@my-scope/my_package' | [ref(:package_protection_rule)] - '@my-scope/my2package' | [] - '@my-scope/my_package-2' | [] + '@my-scope/my_package' | [ref(:package_protection_rule)] + '@my-scope/my2package' | [] + '@my-scope/my_package-2' | [] # With wildcard pattern at the start - '@my-scope/my_package-with-wildcard-start' | [ref(:ppr_with_wildcard_start)] - '@my-scope/my_package-with-wildcard-start-any' | [] - 'prefix-@my-scope/my_package-with-wildcard-start' | [ref(:ppr_with_wildcard_start)] - 'prefix-@my-scope/my_package-with-wildcard-start-any' | [] + '@my-scope/my_package-with-wildcard-start' | [ref(:ppr_with_wildcard_start)] + '@my-scope/my_package-with-wildcard-start-any' | [] + '@my-scope/any-my_package-with-wildcard-start' | [ref(:ppr_with_wildcard_start)] + '@my-scope/any-my_package-with-wildcard-start-any' | [] # With wildcard pattern at the end - '@my-scope/my_package-with-wildcard-end' | [ref(:ppr_with_wildcard_end)] - '@my-scope/my_package-with-wildcard-end:1234567890' | [ref(:ppr_with_wildcard_end)] - 'prefix-@my-scope/my_package-with-wildcard-end' | [] - 'prefix-@my-scope/my_package-with-wildcard-end:1234567890' | [] + '@my-scope/my_package-with-wildcard-end' | [ref(:ppr_with_wildcard_end)] + '@my-scope/my_package-with-wildcard-end:1234567890' | [ref(:ppr_with_wildcard_end)] + '@my-scope/any-my_package-with-wildcard-end' | [] + '@my-scope/any-my_package-with-wildcard-end:1234567890' | [] # With wildcard pattern inbetween - '@my-scope/my_package-with-wildcard-inbetween' | [ref(:ppr_with_wildcard_inbetween)] - '@my-scope/any-my_package-with-wildcard-inbetween' | [ref(:ppr_with_wildcard_inbetween)] - '@my-scope/any-my_package-my_package-wildcard-inbetween-any' | [] + '@my-scope/my_packagewith-wildcard-inbetween' | [ref(:ppr_with_wildcard_inbetween)] + '@my-scope/my_package-any-with-wildcard-inbetween' | [ref(:ppr_with_wildcard_inbetween)] + '@my-scope/any-my_package-my_package-wildcard-inbetween-any' | [] # With multiple wildcard pattern are used - '@my-scope/my_package-with-wildcard-multiple' | [ref(:ppr_with_wildcard_multiples)] - 'prefix-@my-scope/any-my_package-with-wildcard-multiple-any' | [ref(:ppr_with_wildcard_multiples)] - '****@my-scope/****my_package-with-wildcard-multiple****' | [ref(:ppr_with_wildcard_multiples)] - 'prefix-@other-scope/any-my_package-with-wildcard-multiple-any' | [] + '@my-scope/my_packagewith-wildcard-multiple' | [ref(:ppr_with_wildcard_multiples)] + '@my-scope/any-my_package-any-with-wildcard-multiple-any' | [ref(:ppr_with_wildcard_multiples)] + '@my-scope/****my_package****with-wildcard-multiple****' | [ref(:ppr_with_wildcard_multiples)] + '@other-scope/any-my_package-with-wildcard-multiple-any' | [] # With underscore - '@my-scope/my_package-with_____underscore' | [ref(:ppr_with_underscore)] - '@my-scope/my_package-with_any_underscore' | [] + '@my-scope/my_package-with_____underscore' | [ref(:ppr_with_underscore)] + '@my-scope/my_package-with_any_underscore' | [] - '@my-scope/my_package-with-regex-characters.+' | [ref(:ppr_with_regex_characters)] - '@my-scope/my_package-with-regex-characters.' | [] - '@my-scope/my_package-with-regex-characters' | [] - '@my-scope/my_package-with-regex-characters-any' | [] + # With regex pattern + '@my-scope/my_package-with-regex-characters.+' | [ref(:ppr_with_regex_characters)] + '@my-scope/my_package-with-regex-characters.' | [] + '@my-scope/my_package-with-regex-characters' | [] + '@my-scope/my_package-with-regex-characters-any' | [] # Special cases - nil | [] - '' | [] - 'any_package' | [] + nil | [] + '' | [] + 'any_package' | [] end with_them do @@ -209,7 +235,7 @@ RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :pack ) end - describe "with different users and protection levels" do + describe 'with different users and protection levels' do # rubocop:disable Layout/LineLength where(:project, :access_level, :package_name, :package_type, :push_protected) do ref(:project_with_ppr) | Gitlab::Access::REPORTER | '@my-scope/my-package-stage-sha-1234' | :npm | true diff --git a/spec/models/packages/pypi/metadatum_spec.rb b/spec/models/packages/pypi/metadatum_spec.rb index 6c83c4ed143..6c93f84124f 100644 --- a/spec/models/packages/pypi/metadatum_spec.rb +++ b/spec/models/packages/pypi/metadatum_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Packages::Pypi::Metadatum, type: :model do +RSpec.describe Packages::Pypi::Metadatum, type: :model, feature_category: :package_registry do describe 'relationships' do it { is_expected.to belong_to(:package) } end @@ -9,8 +9,29 @@ RSpec.describe Packages::Pypi::Metadatum, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:package) } it { is_expected.to allow_value('').for(:required_python) } - it { is_expected.not_to allow_value(nil).for(:required_python) } - it { is_expected.not_to allow_value('a' * 256).for(:required_python) } + it { is_expected.to validate_length_of(:required_python).is_at_most(described_class::MAX_REQUIRED_PYTHON_LENGTH) } + it { is_expected.to allow_value('').for(:keywords) } + it { is_expected.to allow_value(nil).for(:keywords) } + it { is_expected.to validate_length_of(:keywords).is_at_most(described_class::MAX_KEYWORDS_LENGTH) } + it { is_expected.to allow_value('').for(:metadata_version) } + it { is_expected.to allow_value(nil).for(:metadata_version) } + it { is_expected.to validate_length_of(:metadata_version).is_at_most(described_class::MAX_METADATA_VERSION_LENGTH) } + it { is_expected.to allow_value('').for(:author_email) } + it { is_expected.to allow_value(nil).for(:author_email) } + it { is_expected.to validate_length_of(:author_email).is_at_most(described_class::MAX_AUTHOR_EMAIL_LENGTH) } + it { is_expected.to allow_value('').for(:summary) } + it { is_expected.to allow_value(nil).for(:summary) } + it { is_expected.to validate_length_of(:summary).is_at_most(described_class::MAX_SUMMARY_LENGTH) } + it { is_expected.to allow_value('').for(:description) } + it { is_expected.to allow_value(nil).for(:description) } + it { is_expected.to validate_length_of(:description).is_at_most(described_class::MAX_DESCRIPTION_LENGTH) } + it { is_expected.to allow_value('').for(:description_content_type) } + it { is_expected.to allow_value(nil).for(:description_content_type) } + + it { + is_expected.to validate_length_of(:description_content_type) + .is_at_most(described_class::MAX_DESCRIPTION_CONTENT_TYPE) + } describe '#pypi_package_type' do it 'will not allow a package with a different package_type' do diff --git a/spec/models/packages/tag_spec.rb b/spec/models/packages/tag_spec.rb index bc03c34f56b..6842d1946e5 100644 --- a/spec/models/packages/tag_spec.rb +++ b/spec/models/packages/tag_spec.rb @@ -5,6 +5,23 @@ RSpec.describe Packages::Tag, type: :model, feature_category: :package_registry let!(:project) { create(:project) } let!(:package) { create(:npm_package, version: '1.0.2', project: project, updated_at: 3.days.ago) } + describe '#ensure_project_id' do + it 'sets the project_id before saving' do + tag = build(:packages_tag) + expect(tag.project_id).to be_nil + tag.save! + expect(tag.project_id).not_to be_nil + expect(tag.project_id).to eq(tag.package.project_id) + end + + it 'does not override the project_id if set' do + another_project = create(:project) + tag = build(:packages_tag, project_id: another_project.id) + tag.save! + expect(tag.project_id).to eq(another_project.id) + end + end + describe 'relationships' do it { is_expected.to belong_to(:package).inverse_of(:tags) } end @@ -61,7 +78,7 @@ RSpec.describe Packages::Tag, type: :model, feature_category: :package_registry end context 'with multiple names' do - let(:name) { %w(tag1 tag3) } + let(:name) { %w[tag1 tag3] } it { is_expected.to contain_exactly(tag1, tag3) } end diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 08ba823f8fa..570c369016b 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -55,12 +55,7 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end context 'when there is pages deployment' do - let(:deployment) { create(:pages_deployment, project: project) } - - before do - project.mark_pages_as_deployed - project.pages_metadatum.update!(pages_deployment: deployment) - end + let!(:deployment) { create(:pages_deployment, project: project) } it 'uses deployment from object storage' do freeze_time do @@ -75,12 +70,6 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end end - it 'does not recreate source hash' do - expect(deployment.file).to receive(:url_or_file_path).once - - 2.times { lookup_path.source } - end - context 'when deployment is in the local storage' do before do deployment.file.migrate!(::ObjectStorage::Store::LOCAL) @@ -159,12 +148,7 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end context 'when there is a deployment' do - let(:deployment) { create(:pages_deployment, project: project, root_directory: 'foo') } - - before do - project.mark_pages_as_deployed - project.pages_metadatum.update!(pages_deployment: deployment) - end + let!(:deployment) { create(:pages_deployment, project: project, root_directory: 'foo') } it 'returns the deployment\'s root_directory' do expect(lookup_path.root_directory).to eq('foo') diff --git a/spec/models/pages_deployment_spec.rb b/spec/models/pages_deployment_spec.rb index e74c7ee8612..1e6f8b33a86 100644 --- a/spec/models/pages_deployment_spec.rb +++ b/spec/models/pages_deployment_spec.rb @@ -71,54 +71,62 @@ RSpec.describe PagesDeployment, feature_category: :pages do end end - describe '.deactivate_deployments_older_than', :freeze_time do - let!(:other_project_deployment) do - create(:pages_deployment) - end + describe '.deactivate_all', :freeze_time do + let!(:deployment) { create(:pages_deployment, project: project, updated_at: 5.minutes.ago) } + let!(:nil_path_prefix_deployment) { create(:pages_deployment, project: project, path_prefix: nil) } + let!(:empty_path_prefix_deployment) { create(:pages_deployment, project: project, path_prefix: '') } - let!(:other_path_prefix_deployment) do - create(:pages_deployment, project: project, path_prefix: 'other') - end + let!(:other_project_deployment) { create(:pages_deployment) } + let!(:deactivated_deployment) { create(:pages_deployment, project: project, deleted_at: 5.minutes.ago) } - let!(:deactivated_deployment) do - create(:pages_deployment, project: project, deleted_at: 5.minutes.ago) + it 'updates only older deployments for the same project and path prefix' do + expect { described_class.deactivate_all(project) } + .to change { deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and change { deployment.reload.updated_at }.to(Time.zone.now) + .and change { nil_path_prefix_deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and change { empty_path_prefix_deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and not_change { other_project_deployment.reload.deleted_at } + .and not_change { deactivated_deployment.reload.deleted_at } end + end + + describe '.deactivate_deployments_older_than', :freeze_time do + let!(:nil_path_prefix_deployment) { create(:pages_deployment, project: project, path_prefix: nil) } + let!(:empty_path_prefix_deployment) { create(:pages_deployment, project: project, path_prefix: '') } + let!(:older_deployment) { create(:pages_deployment, project: project, updated_at: 5.minutes.ago) } + let!(:reference_deployment) { create(:pages_deployment, project: project, updated_at: 5.minutes.ago) } + let!(:newer_deployment) { create(:pages_deployment, project: project, updated_at: 5.minutes.ago) } + + let!(:other_project_deployment) { create(:pages_deployment) } + let!(:other_path_prefix_deployment) { create(:pages_deployment, project: project, path_prefix: 'other') } + let!(:deactivated_deployment) { create(:pages_deployment, project: project, deleted_at: 5.minutes.ago) } it 'updates only older deployments for the same project and path prefix' do - deployment1 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) - deployment2 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) - deployment3 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) - - expect { described_class.deactivate_deployments_older_than(deployment2) } - .to change { deployment1.reload.deleted_at } - .from(nil).to(Time.zone.now) - .and change { deployment1.reload.updated_at } - .to(Time.zone.now) - - expect(deployment2.reload.deleted_at).to be_nil - expect(deployment3.reload.deleted_at).to be_nil - expect(other_project_deployment.deleted_at).to be_nil - expect(other_path_prefix_deployment.reload.deleted_at).to be_nil - expect(deactivated_deployment.reload.deleted_at).to eq(5.minutes.ago) + expect { described_class.deactivate_deployments_older_than(reference_deployment) } + .to change { older_deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and change { older_deployment.reload.updated_at }.to(Time.zone.now) + .and change { nil_path_prefix_deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and change { empty_path_prefix_deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and not_change { reference_deployment.reload.deleted_at } + .and not_change { newer_deployment.reload.deleted_at } + .and not_change { other_project_deployment.reload.deleted_at } + .and not_change { other_path_prefix_deployment.reload.deleted_at } + .and not_change { deactivated_deployment.reload.deleted_at } end it 'updates only older deployments for the same project with the given time' do - deployment1 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) - deployment2 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) - deployment3 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) time = 30.minutes.from_now - expect { described_class.deactivate_deployments_older_than(deployment2, time: time) } - .to change { deployment1.reload.deleted_at } - .from(nil).to(time) - .and change { deployment1.reload.updated_at } - .to(Time.zone.now) - - expect(deployment2.reload.deleted_at).to be_nil - expect(deployment3.reload.deleted_at).to be_nil - expect(other_project_deployment.deleted_at).to be_nil - expect(other_path_prefix_deployment.reload.deleted_at).to be_nil - expect(deactivated_deployment.reload.deleted_at).to eq(5.minutes.ago) + expect { described_class.deactivate_deployments_older_than(reference_deployment, time: time) } + .to change { older_deployment.reload.deleted_at }.from(nil).to(time) + .and change { older_deployment.reload.updated_at }.to(Time.zone.now) + .and change { nil_path_prefix_deployment.reload.deleted_at }.from(nil).to(time) + .and change { empty_path_prefix_deployment.reload.deleted_at }.from(nil).to(time) + .and not_change { reference_deployment.reload.deleted_at } + .and not_change { newer_deployment.reload.deleted_at } + .and not_change { other_project_deployment.reload.deleted_at } + .and not_change { other_path_prefix_deployment.reload.deleted_at } + .and not_change { deactivated_deployment.reload.deleted_at } end end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 5a4eca11f71..7aa5cf993dc 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -84,20 +84,20 @@ RSpec.describe PagesDomain, feature_category: :pages do attributes = attributes_for(:pages_domain) cert, key = attributes.fetch_values(:certificate, :key) - true | nil | nil | false | %i(certificate key) + true | nil | nil | false | %i[certificate key] true | nil | nil | true | [] - true | cert | nil | false | %i(key) - true | cert | nil | true | %i(key) - true | nil | key | false | %i(certificate key) - true | nil | key | true | %i(key) + true | cert | nil | false | %i[key] + true | cert | nil | true | %i[key] + true | nil | key | false | %i[certificate key] + true | nil | key | true | %i[key] true | cert | key | false | [] true | cert | key | true | [] false | nil | nil | false | [] false | nil | nil | true | [] - false | cert | nil | false | %i(key) - false | cert | nil | true | %i(key) - false | nil | key | false | %i(key) - false | nil | key | true | %i(key) + false | cert | nil | false | %i[key] + false | cert | nil | true | %i[key] + false | nil | key | false | %i[key] + false | nil | key | true | %i[key] false | cert | key | false | [] false | cert | key | true | [] end @@ -288,8 +288,8 @@ RSpec.describe PagesDomain, feature_category: :pages do end end - describe '#has_intermediates?' do - subject { domain.has_intermediates? } + describe '#has_valid_intermediates?' do + subject { domain.has_valid_intermediates? } context 'for self signed' do let(:domain) { build(:pages_domain) } @@ -312,6 +312,14 @@ RSpec.describe PagesDomain, feature_category: :pages do it { is_expected.to be_truthy } end + + context 'for chain with unknown root CA' do + # In cases where users use an origin certificate the CA does not necessarily need to be in + # the trust store, eg. in the case of Cloudflare Origin Certs. + let(:domain) { build(:pages_domain, :with_untrusted_root_ca_in_chain) } + + it { is_expected.to be_truthy } + end end describe '#expired?' do diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 7437e9b463e..7665f4dbde4 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -365,7 +365,7 @@ RSpec.describe PersonalAccessToken, feature_category: :system_access do describe '.simple_sorts' do it 'includes overridden keys' do - expect(described_class.simple_sorts.keys).to include(*%w(expires_at_asc_id_desc)) + expect(described_class.simple_sorts.keys).to include(*%w[expires_at_asc_id_desc]) end end diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index 39e77df1900..c0a78ff2f53 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -53,7 +53,7 @@ RSpec.describe ProjectFeature, feature_category: :groups_and_projects do end it "does not allow repository related features have higher level" do - features = %w(builds merge_requests) + features = %w[builds merge_requests] project_feature = project.project_feature features.each do |feature| @@ -64,7 +64,7 @@ RSpec.describe ProjectFeature, feature_category: :groups_and_projects do end end - it_behaves_like 'access level validation', ProjectFeature::FEATURES - %i(pages package_registry) do + it_behaves_like 'access level validation', ProjectFeature::FEATURES - %i[pages package_registry] do let(:container_features) { project.project_feature } end diff --git a/spec/models/project_feature_usage_spec.rb b/spec/models/project_feature_usage_spec.rb deleted file mode 100644 index 3765a2b37a7..00000000000 --- a/spec/models/project_feature_usage_spec.rb +++ /dev/null @@ -1,173 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ProjectFeatureUsage, type: :model do - describe '.jira_dvcs_integrations_enabled_count' do - it 'returns count of projects with Jira DVCS Cloud enabled' do - create(:project).feature_usage.log_jira_dvcs_integration_usage - create(:project).feature_usage.log_jira_dvcs_integration_usage - - expect(described_class.with_jira_dvcs_integration_enabled.count).to eq(2) - end - - it 'returns count of projects with Jira DVCS Server enabled' do - create(:project).feature_usage.log_jira_dvcs_integration_usage(cloud: false) - create(:project).feature_usage.log_jira_dvcs_integration_usage(cloud: false) - - expect(described_class.with_jira_dvcs_integration_enabled(cloud: false).count).to eq(2) - end - end - - describe '#log_jira_dvcs_integration_usage' do - let(:project) { create(:project) } - - subject { project.feature_usage } - - context 'when the feature usage has not been created yet' do - it 'logs Jira DVCS Cloud last sync' do - freeze_time do - subject.log_jira_dvcs_integration_usage - - expect(subject.jira_dvcs_server_last_sync_at).to be_nil - expect(subject.jira_dvcs_cloud_last_sync_at).to be_like_time(Time.current) - end - end - - it 'logs Jira DVCS Server last sync' do - freeze_time do - subject.log_jira_dvcs_integration_usage(cloud: false) - - expect(subject.jira_dvcs_server_last_sync_at).to be_like_time(Time.current) - expect(subject.jira_dvcs_cloud_last_sync_at).to be_nil - end - end - end - - context 'when the feature usage already exists' do - let(:today) { Time.current.beginning_of_day } - let(:project) { create(:project) } - - subject { project.feature_usage } - - where(:cloud, :timestamp_field) do - [ - [true, :jira_dvcs_cloud_last_sync_at], - [false, :jira_dvcs_server_last_sync_at] - ] - end - - with_them do - context 'when Jira DVCS Cloud last sync has not been logged' do - before do - travel_to today - 3.days do - subject.log_jira_dvcs_integration_usage(cloud: !cloud) - end - end - - it 'logs Jira DVCS Cloud last sync' do - freeze_time do - subject.log_jira_dvcs_integration_usage(cloud: cloud) - - expect(subject.reload.send(timestamp_field)).to be_like_time(Time.current) - end - end - end - - context 'when Jira DVCS Cloud last sync was logged today' do - let(:last_updated) { today + 1.hour } - - before do - travel_to last_updated do - subject.log_jira_dvcs_integration_usage(cloud: cloud) - end - end - - it 'does not log Jira DVCS Cloud last sync' do - travel_to today + 2.hours do - subject.log_jira_dvcs_integration_usage(cloud: cloud) - - expect(subject.reload.send(timestamp_field)).to be_like_time(last_updated) - end - end - end - - context 'when Jira DVCS Cloud last sync was logged yesterday' do - let(:last_updated) { today - 2.days } - - before do - travel_to last_updated do - subject.log_jira_dvcs_integration_usage(cloud: cloud) - end - end - - it 'logs Jira DVCS Cloud last sync' do - travel_to today + 1.hour do - subject.log_jira_dvcs_integration_usage(cloud: cloud) - - expect(subject.reload.send(timestamp_field)).to be_like_time(today + 1.hour) - end - end - end - end - end - - context 'when log_jira_dvcs_integration_usage is called simultaneously for the same project' do - it 'logs the latest call' do - feature_usage = project.feature_usage - feature_usage.log_jira_dvcs_integration_usage - first_logged_at = feature_usage.jira_dvcs_cloud_last_sync_at - - travel_to(1.hour.from_now) do - ProjectFeatureUsage.new(project_id: project.id).log_jira_dvcs_integration_usage - end - - expect(feature_usage.reload.jira_dvcs_cloud_last_sync_at).to be > first_logged_at - end - end - end - - context 'ProjectFeatureUsage with DB Load Balancing', :request_store do - describe '#log_jira_dvcs_integration_usage' do - let!(:project) { create(:project) } - - subject { project.feature_usage } - - context 'database load balancing is configured' do - before do - ::Gitlab::Database::LoadBalancing::Session.clear_session - end - - it 'logs Jira DVCS Cloud last sync' do - freeze_time do - subject.log_jira_dvcs_integration_usage - - expect(subject.jira_dvcs_server_last_sync_at).to be_nil - expect(subject.jira_dvcs_cloud_last_sync_at).to be_like_time(Time.current) - end - end - - it 'does not stick to primary' do - expect(::Gitlab::Database::LoadBalancing::Session.current).not_to be_performed_write - expect(::Gitlab::Database::LoadBalancing::Session.current).not_to be_using_primary - - subject.log_jira_dvcs_integration_usage - - expect(::Gitlab::Database::LoadBalancing::Session.current).to be_performed_write - expect(::Gitlab::Database::LoadBalancing::Session.current).not_to be_using_primary - end - end - - context 'database load balancing is not cofigured' do - it 'logs Jira DVCS Cloud last sync' do - freeze_time do - subject.log_jira_dvcs_integration_usage - - expect(subject.jira_dvcs_server_last_sync_at).to be_nil - expect(subject.jira_dvcs_cloud_last_sync_at).to be_like_time(Time.current) - end - end - end - end - end -end diff --git a/spec/models/project_label_spec.rb b/spec/models/project_label_spec.rb index 62839f5fb4f..01df58ee615 100644 --- a/spec/models/project_label_spec.rb +++ b/spec/models/project_label_spec.rb @@ -89,7 +89,7 @@ RSpec.describe ProjectLabel do end it 'uses id when name contains double quote' do - label = create(:label, name: %q{"irony"}) + label = create(:label, name: %q("irony")) expect(label.to_reference(format: :name)).to eq "~#{label.id}" end end diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 719e51018ac..8ad232b7e0c 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -214,7 +214,7 @@ RSpec.describe ProjectSetting, type: :model, feature_category: :groups_and_proje context 'when emails are enabled in parent group' do before do - allow(project.namespace).to receive(:emails_disabled?).and_return(false) + allow(project.namespace).to receive(:emails_enabled?).and_return(true) end it 'returns true' do diff --git a/spec/models/project_snippet_spec.rb b/spec/models/project_snippet_spec.rb index 3d1c87771f3..5ce9499c420 100644 --- a/spec/models/project_snippet_spec.rb +++ b/spec/models/project_snippet_spec.rb @@ -2,11 +2,24 @@ require 'spec_helper' -RSpec.describe ProjectSnippet do +RSpec.describe ProjectSnippet, feature_category: :source_code_management do describe "Associations" do it { is_expected.to belong_to(:project) } end + describe 'scopes' do + describe '.by_project' do + subject { described_class.by_project(project) } + + let_it_be(:project) { create(:project) } + let_it_be(:snippet1) { create(:project_snippet, project: project) } + let_it_be(:snippet2) { create(:project_snippet, project: build(:project)) } + let_it_be(:snippet3) { create(:personal_snippet) } + + it { is_expected.to contain_exactly(snippet1) } + end + end + describe "Validation" do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_inclusion_of(:secret).in_array([false]) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c27ed2cc82c..3ea5f6ea0ae 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1136,24 +1136,24 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it { is_expected.to delegate_method(:npm_package_requests_forwarding).to(:namespace) } describe 'read project settings' do - %i( + %i[ show_default_award_emojis show_default_award_emojis? warn_about_potentially_unwanted_characters warn_about_potentially_unwanted_characters? enforce_auth_checks_on_uploads enforce_auth_checks_on_uploads? - ).each do |method| + ].each do |method| it { is_expected.to delegate_method(method).to(:project_setting).allow_nil } end end describe 'write project settings' do - %i( + %i[ show_default_award_emojis= warn_about_potentially_unwanted_characters= enforce_auth_checks_on_uploads= - ).each do |method| + ].each do |method| it { is_expected.to delegate_method(method).to(:project_setting).with_arguments(:args).allow_nil } end end @@ -1177,12 +1177,13 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr let(:exclude_attributes) do # Skip attributes defined in EE code - %w( + %w[ merge_pipelines_enabled merge_trains_enabled auto_rollback_enabled merge_trains_skip_train_allowed - ) + restrict_pipeline_cancellation_role + ] end end @@ -2127,28 +2128,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe 'sorting by name' do - let_it_be(:project1) { create(:project, name: 'A') } - let_it_be(:project2) { create(:project, name: 'Z') } - let_it_be(:project3) { create(:project, name: 'L') } - - context 'when using .sort_by_name_desc' do - it 'reorders the projects by descending name order' do - projects = described_class.sorted_by_name_desc - - expect(projects.pluck(:name)).to eq(%w[Z L A]) - end - end - - context 'when using .sort_by_name_asc' do - it 'reorders the projects by ascending name order' do - projects = described_class.sorted_by_name_asc - - expect(projects.pluck(:name)).to eq(%w[A L Z]) - end - end - end - describe '.with_shared_runners_enabled' do subject { described_class.with_shared_runners_enabled } @@ -2841,7 +2820,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr context "will return false if pages is deployed even if onboarding_complete is false" do before do project.pages_metadatum.update_column(:onboarding_complete, false) - project.pages_metadatum.update_column(:deployed, true) + create(:pages_deployment, project: project) end it { is_expected.to be_falsey } @@ -2855,7 +2834,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr context 'if pages are deployed' do before do - project.pages_metadatum.update_column(:deployed, true) + create(:pages_deployment, project: project) end it { is_expected.to be_truthy } @@ -4309,7 +4288,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end context 'when project has a deployment platform' do - let(:platform_variables) { %w(platform variables) } + let(:platform_variables) { %w[platform variables] } let(:deployment_platform) { double } before do @@ -7142,7 +7121,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr project.check_personal_projects_limit expect(project.errors[:limit_reached].first) - .to match(/Personal project creation is not allowed/) + .to eq('You cannot create projects in your personal namespace. Contact your GitLab administrator.') end end @@ -7155,7 +7134,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr project.check_personal_projects_limit expect(project.errors[:limit_reached].first) - .to match(/Your project limit is 5 projects/) + .to eq("You've reached your limit of 5 projects created. Contact your GitLab administrator.") end end end @@ -7219,126 +7198,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '#mark_pages_as_deployed' do - let(:project) { create(:project) } - - it "works when artifacts_archive is missing" do - project.mark_pages_as_deployed - - expect(project.pages_metadatum.reload.deployed).to eq(true) - end - - it "creates new record and sets deployed to true if none exists yet" do - project.pages_metadatum.destroy! - project.reload - - project.mark_pages_as_deployed - - expect(project.pages_metadatum.reload.deployed).to eq(true) - end - - it "updates the existing record and sets deployed to true and records artifact archive" do - pages_metadatum = project.pages_metadatum - pages_metadatum.update!(deployed: false) - - expect do - project.mark_pages_as_deployed - end.to change { pages_metadatum.reload.deployed }.from(false).to(true) - end - end - - describe '#mark_pages_as_not_deployed' do - let(:project) { create(:project) } - - it "creates new record and sets deployed to false if none exists yet" do - project.pages_metadatum.destroy! - project.reload - - project.mark_pages_as_not_deployed - - expect(project.pages_metadatum.reload.deployed).to eq(false) - end - - it "updates the existing record and sets deployed to false and clears artifacts_archive" do - pages_metadatum = project.pages_metadatum - pages_metadatum.update!(deployed: true) - - expect do - project.mark_pages_as_not_deployed - end.to change { pages_metadatum.reload.deployed }.from(true).to(false) - end - end - - describe '#update_pages_deployment!' do - let(:project) { create(:project) } - let(:deployment) { create(:pages_deployment, project: project) } - - it "creates new metadata record if none exists yet and sets deployment" do - project.pages_metadatum.destroy! - project.reload - - project.update_pages_deployment!(deployment) - - expect(project.pages_metadatum.pages_deployment).to eq(deployment) - end - - it "updates the existing metadara record with deployment" do - expect do - project.update_pages_deployment!(deployment) - end.to change { project.pages_metadatum.reload.pages_deployment }.from(nil).to(deployment) - end - end - - describe '#set_first_pages_deployment!' do - let(:project) { create(:project) } - let(:deployment) { create(:pages_deployment, project: project) } - - it "creates new metadata record if none exists yet and sets deployment" do - project.pages_metadatum.destroy! - project.reload - - project.set_first_pages_deployment!(deployment) - - expect(project.pages_metadatum.reload.pages_deployment).to eq(deployment) - expect(project.pages_metadatum.reload.deployed).to eq(true) - end - - it "updates the existing metadara record with deployment" do - expect do - project.set_first_pages_deployment!(deployment) - end.to change { project.pages_metadatum.reload.pages_deployment }.from(nil).to(deployment) - - expect(project.pages_metadatum.reload.deployed).to eq(true) - end - - it 'only updates metadata for this project' do - other_project = create(:project) - - expect do - project.set_first_pages_deployment!(deployment) - end.not_to change { other_project.pages_metadatum.reload.pages_deployment }.from(nil) - - expect(other_project.pages_metadatum.reload.deployed).to eq(false) - end - - it 'does nothing if metadata already references some deployment' do - existing_deployment = create(:pages_deployment, project: project) - project.set_first_pages_deployment!(existing_deployment) - - expect do - project.set_first_pages_deployment!(deployment) - end.not_to change { project.pages_metadatum.reload.pages_deployment }.from(existing_deployment) - end - - it 'marks project as not deployed if deployment is nil' do - project.mark_pages_as_deployed - - expect do - project.set_first_pages_deployment!(nil) - end.to change { project.pages_metadatum.reload.deployed }.from(true).to(false) - end - end - describe '#has_pool_repository?' do it 'returns false when it does not have a pool repository' do subject = create(:project, :repository) @@ -7402,7 +7261,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it 'returns only projects that have pages deployed' do _project_without_pages = create(:project) project_with_pages = create(:project) - project_with_pages.mark_pages_as_deployed + create(:pages_deployment, project: project_with_pages) expect(described_class.with_pages_deployed).to contain_exactly(project_with_pages) end @@ -9141,6 +9000,66 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end + # TODO: Remove/update this spec after background syncing is implemented. See https://gitlab.com/gitlab-org/gitlab/-/issues/429376. + describe '#update_catalog_resource' do + let_it_be_with_reload(:project) { create(:project, name: 'My project name', description: 'My description') } + let_it_be_with_reload(:resource) { create(:ci_catalog_resource, project: project) } + + shared_examples 'name, description, and visibility_level of the catalog resource match the project' do + it do + expect(project).to receive(:update_catalog_resource).once.and_call_original + + project.save! + + expect(resource.name).to eq(project.name) + expect(resource.description).to eq(project.description) + expect(resource.visibility_level).to eq(project.visibility_level) + end + end + + context 'when the project name is updated' do + before do + project.name = 'My new project name' + end + + it_behaves_like 'name, description, and visibility_level of the catalog resource match the project' + end + + context 'when the project description is updated' do + before do + project.description = 'My new description' + end + + it_behaves_like 'name, description, and visibility_level of the catalog resource match the project' + end + + context 'when the project visibility_level is updated' do + before do + project.visibility_level = 10 + end + + it_behaves_like 'name, description, and visibility_level of the catalog resource match the project' + end + + context 'when neither the project name, description, nor visibility_level are updated' do + it 'does not call update_catalog_resource' do + expect(project).not_to receive(:update_catalog_resource) + + project.update!(path: 'path') + end + end + + context 'when the project does not have a catalog resource' do + let_it_be(:project2) { create(:project) } + + it 'does not call update_catalog_resource' do + expect(project2).not_to receive(:update_catalog_resource) + + project.update!(name: 'name') + end + end + end + private def finish_job(export_job) diff --git a/spec/models/projects/repository_storage_move_spec.rb b/spec/models/projects/repository_storage_move_spec.rb index ab0ad81f77a..c5fbc92176f 100644 --- a/spec/models/projects/repository_storage_move_spec.rb +++ b/spec/models/projects/repository_storage_move_spec.rb @@ -3,33 +3,11 @@ require 'spec_helper' RSpec.describe Projects::RepositoryStorageMove, type: :model do - let_it_be_with_refind(:project) { create(:project) } - it_behaves_like 'handles repository moves' do - let(:container) { project } + let_it_be_with_refind(:container) { create(:project) } + let(:repository_storage_factory_key) { :project_repository_storage_move } let(:error_key) { :project } let(:repository_storage_worker) { Projects::UpdateRepositoryStorageWorker } end - - describe 'state transitions' do - let(:storage) { 'test_second_storage' } - - before do - stub_storage_settings(storage => { 'path' => 'tmp/tests/extra_storage' }) - end - - context 'when started' do - subject(:storage_move) { create(:project_repository_storage_move, :started, container: project, destination_storage_name: storage) } - - context 'and transits to replicated' do - it 'sets the repository storage and marks the container as writable' do - storage_move.finish_replication! - - expect(project.repository_storage).to eq(storage) - expect(project).not_to be_repository_read_only - end - end - end - end end diff --git a/spec/models/projects/topic_spec.rb b/spec/models/projects/topic_spec.rb index 568a4166de7..b3a55ccd370 100644 --- a/spec/models/projects/topic_spec.rb +++ b/spec/models/projects/topic_spec.rb @@ -76,7 +76,7 @@ RSpec.describe Projects::Topic do describe '#find_by_name_case_insensitive' do it 'returns topic with case insensitive name' do - %w(topic TOPIC Topic).each do |name| + %w[topic TOPIC Topic].each do |name| expect(described_class.find_by_name_case_insensitive(name)).to eq(topic) end end diff --git a/spec/models/prometheus_metric_spec.rb b/spec/models/prometheus_metric_spec.rb index a20f4edcf4a..c8a95aef8a6 100644 --- a/spec/models/prometheus_metric_spec.rb +++ b/spec/models/prometheus_metric_spec.rb @@ -94,16 +94,16 @@ RSpec.describe PrometheusMetric do describe '#required_metrics' do where(:group, :required_metrics) do - :nginx_ingress_vts | %w(nginx_upstream_responses_total nginx_upstream_response_msecs_avg) - :nginx_ingress | %w(nginx_ingress_controller_requests nginx_ingress_controller_ingress_upstream_latency_seconds_sum) - :ha_proxy | %w(haproxy_frontend_http_requests_total haproxy_frontend_http_responses_total) - :aws_elb | %w(aws_elb_request_count_sum aws_elb_latency_average aws_elb_httpcode_backend_5_xx_sum) - :nginx | %w(nginx_server_requests nginx_server_requestMsec) - :kubernetes | %w(container_memory_usage_bytes container_cpu_usage_seconds_total) - :business | %w() - :response | %w() - :system | %w() - :cluster_health | %w(container_memory_usage_bytes container_cpu_usage_seconds_total) + :nginx_ingress_vts | %w[nginx_upstream_responses_total nginx_upstream_response_msecs_avg] + :nginx_ingress | %w[nginx_ingress_controller_requests nginx_ingress_controller_ingress_upstream_latency_seconds_sum] + :ha_proxy | %w[haproxy_frontend_http_requests_total haproxy_frontend_http_responses_total] + :aws_elb | %w[aws_elb_request_count_sum aws_elb_latency_average aws_elb_httpcode_backend_5_xx_sum] + :nginx | %w[nginx_server_requests nginx_server_requestMsec] + :kubernetes | %w[container_memory_usage_bytes container_cpu_usage_seconds_total] + :business | %w[] + :response | %w[] + :system | %w[] + :cluster_health | %w[container_memory_usage_bytes container_cpu_usage_seconds_total] end with_them do diff --git a/spec/models/releases/link_spec.rb b/spec/models/releases/link_spec.rb index c4c9fba32d9..5d264af695b 100644 --- a/spec/models/releases/link_spec.rb +++ b/spec/models/releases/link_spec.rb @@ -82,7 +82,7 @@ RSpec.describe Releases::Link do describe 'supported protocols' do where(:protocol) do - %w(http https ftp) + %w[http https ftp] end with_them do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 2265d1b39af..606c4ea05b9 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -743,7 +743,7 @@ RSpec.describe Repository, feature_category: :source_code_management do describe "#merged_branch_names", :clean_gitlab_redis_cache do subject { repository.merged_branch_names(branch_names) } - let(:branch_names) { %w(test beep boop definitely_merged) } + let(:branch_names) { %w[test beep boop definitely_merged] } let(:already_merged) { Set.new(["definitely_merged"]) } let(:write_hash) do @@ -1621,16 +1621,16 @@ RSpec.describe Repository, feature_category: :source_code_management do where(:branch_names, :tag_names, :result) do nil | nil | false - %w() | %w() | false - %w(a b) | %w() | false - %w() | %w(c d) | false - %w(a b) | %w(c d) | false - %w(a/b) | %w(c/d) | false - %w(a b) | %w(c d a/z) | true - %w(a b c/z) | %w(c d) | true - %w(a/b/z) | %w(a/b) | false # we only consider refs ambiguous before the first slash - %w(a/b/z) | %w(a/b a) | true - %w(ab) | %w(abc/d a b) | false + %w[] | %w[] | false + %w[a b] | %w[] | false + %w[] | %w[c d] | false + %w[a b] | %w[c d] | false + %w[a/b] | %w[c/d] | false + %w[a b] | %w[c d a/z] | true + %w[a b c/z] | %w[c d] | true + %w[a/b/z] | %w[a/b] | false # we only consider refs ambiguous before the first slash + %w[a/b/z] | %w[a/b a] | true + %w[ab] | %w[abc/d a b] | false end with_them do @@ -2596,7 +2596,7 @@ RSpec.describe Repository, feature_category: :source_code_management do describe '#expire_branches_cache' do it 'expires the cache' do expect(repository).to receive(:expire_method_caches) - .with(%i(branch_names merged_branch_names branch_count has_visible_content? has_ambiguous_refs?)) + .with(%i[branch_names merged_branch_names branch_count has_visible_content? has_ambiguous_refs?]) .and_call_original expect_next_instance_of(ProtectedBranches::CacheService) do |cache_service| @@ -2630,7 +2630,7 @@ RSpec.describe Repository, feature_category: :source_code_management do describe '#expire_tags_cache' do it 'expires the cache' do expect(repository).to receive(:expire_method_caches) - .with(%i(tag_names tag_count has_ambiguous_refs?)) + .with(%i[tag_names tag_count has_ambiguous_refs?]) .and_call_original repository.expire_tags_cache @@ -2889,7 +2889,7 @@ RSpec.describe Repository, feature_category: :source_code_management do describe '#expire_statistics_caches' do it 'expires the caches' do expect(repository).to receive(:expire_method_caches) - .with(%i(size recent_objects_size commit_count)) + .with(%i[size recent_objects_size commit_count]) repository.expire_statistics_caches end @@ -3001,10 +3001,6 @@ RSpec.describe Repository, feature_category: :source_code_management do it_behaves_like '#tree' - describe '#tree? with Rugged enabled', :enable_rugged do - it_behaves_like '#tree' - end - describe '#size' do context 'with a non-existing repository' do it 'returns 0' do @@ -3090,33 +3086,13 @@ RSpec.describe Repository, feature_category: :source_code_management do describe '#refresh_method_caches' do it 'refreshes the caches of the given types' do expect(repository).to receive(:expire_method_caches) - .with(%i(readme_path license_blob license_gitaly)) + .with(%i[readme_path license_blob license_gitaly]) expect(repository).to receive(:readme_path) expect(repository).to receive(:license_blob) expect(repository).to receive(:license_gitaly) - repository.refresh_method_caches(%i(readme license)) - end - end - - describe '#gitlab_ci_yml_for' do - let(:project) { create(:project, :repository) } - - before do - repository.create_file(User.last, '.gitlab-ci.yml', 'CONTENT', message: 'Add .gitlab-ci.yml', branch_name: 'master') - end - - context 'when there is a .gitlab-ci.yml at the commit' do - it 'returns the content' do - expect(repository.gitlab_ci_yml_for(repository.commit.sha)).to eq('CONTENT') - end - end - - context 'when there is no .gitlab-ci.yml at the commit' do - it 'returns nil' do - expect(repository.gitlab_ci_yml_for(repository.commit.parent.sha)).to be_nil - end + repository.refresh_method_caches(%i[readme license]) end end @@ -3236,16 +3212,6 @@ RSpec.describe Repository, feature_category: :source_code_management do end end - describe '#ancestor? with Rugged enabled', :enable_rugged do - it 'calls out to the Rugged implementation' do - allow_any_instance_of(Rugged).to receive(:merge_base).with(repository.commit.id, Gitlab::Git::BLANK_SHA).and_call_original - - repository.ancestor?(repository.commit.id, Gitlab::Git::BLANK_SHA) - end - - it_behaves_like '#ancestor?' - end - describe '#archive_metadata' do let(:ref) { 'master' } let(:storage_path) { '/tmp' } @@ -3842,6 +3808,13 @@ RSpec.describe Repository, feature_category: :source_code_management do it 'returns nil' do expect(repository.get_patch_id('HEAD', 'HEAD')).to be_nil end + + it 'does not report the exception' do + expect(Gitlab::ErrorTracking) + .not_to receive(:track_exception) + + repository.get_patch_id('HEAD', 'HEAD') + end end context 'when a Gitlab::Git::CommandError is raised' do @@ -3851,7 +3824,7 @@ RSpec.describe Repository, feature_category: :source_code_management do end it 'returns nil' do - expect(repository.get_patch_id('HEAD', 'HEAD')).to be_nil + expect(repository.get_patch_id('HEAD~', 'HEAD')).to be_nil end it 'reports the exception' do @@ -3860,11 +3833,11 @@ RSpec.describe Repository, feature_category: :source_code_management do .with( instance_of(Gitlab::Git::CommandError), project_id: repository.project.id, - old_revision: 'HEAD', + old_revision: 'HEAD~', new_revision: 'HEAD' ) - repository.get_patch_id('HEAD', 'HEAD') + repository.get_patch_id('HEAD~', 'HEAD') end end diff --git a/spec/models/service_desk/custom_email_credential_spec.rb b/spec/models/service_desk/custom_email_credential_spec.rb index a990b77128e..dbf47a8f6a7 100644 --- a/spec/models/service_desk/custom_email_credential_spec.rb +++ b/spec/models/service_desk/custom_email_credential_spec.rb @@ -55,6 +55,39 @@ RSpec.describe ServiceDesk::CustomEmailCredential, feature_category: :service_de end end + describe '#delivery_options' do + let(:expected_attributes) do + { + address: 'smtp.example.com', + domain: 'example.com', + user_name: 'user@example.com', + port: 587, + password: 'supersecret', + authentication: nil + } + end + + let(:setting) { build_stubbed(:service_desk_setting, project: project, custom_email: 'user@example.com') } + + subject { credential.delivery_options } + + before do + # credential.service_desk_setting is delegated to project and we only use build_stubbed + project.service_desk_setting = setting + end + + it { is_expected.to include(expected_attributes) } + + context 'when authentication is set' do + before do + credential.smtp_authentication = 'login' + expected_attributes[:authentication] = 'login' + end + + it { is_expected.to include(expected_attributes) } + end + end + describe 'associations' do it { is_expected.to belong_to(:project) } diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb index 8048f255272..fe854f5d3ba 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/spec/models/snippet_repository_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe SnippetRepository do +RSpec.describe SnippetRepository, feature_category: :snippets do let_it_be(:user) { create(:user) } let(:snippet) { create(:personal_snippet, :repository, author: user) } @@ -68,6 +68,8 @@ RSpec.describe SnippetRepository do expect(update_file_blob).not_to be_nil end + expect(described_class.sticking).to receive(:stick) + expect do snippet_repository.multi_files_action(user, data, **commit_opts) end.not_to raise_error diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index ec2dfb2634f..ff1c5959cb0 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Snippet do +RSpec.describe Snippet, feature_category: :source_code_management do include FakeBlobHelpers describe 'modules' do @@ -26,6 +26,22 @@ RSpec.describe Snippet do it { is_expected.to have_many(:repository_storage_moves).class_name('Snippets::RepositoryStorageMove').inverse_of(:container) } end + describe 'scopes' do + describe '.with_repository_storage_moves' do + subject { described_class.with_repository_storage_moves } + + let_it_be(:snippet) { create(:project_snippet) } + + it { is_expected.to be_empty } + + context 'when associated repository storage move exists' do + let!(:snippet_repository_storage_move) { create(:snippet_repository_storage_move, container: snippet) } + + it { is_expected.to match_array([snippet]) } + end + end + end + describe 'validation' do it { is_expected.to validate_presence_of(:author) } @@ -614,7 +630,7 @@ RSpec.describe Snippet do context 'when file does not exist' do it 'removes nil values from the blobs array' do - allow(snippet).to receive(:list_files).and_return(%w(LICENSE non_existent_snippet_file)) + allow(snippet).to receive(:list_files).and_return(%w[LICENSE non_existent_snippet_file]) blobs = snippet.blobs expect(blobs.count).to eq 1 diff --git a/spec/models/terraform/state_spec.rb b/spec/models/terraform/state_spec.rb index fc0a6432149..df8051ebbc6 100644 --- a/spec/models/terraform/state_spec.rb +++ b/spec/models/terraform/state_spec.rb @@ -21,7 +21,7 @@ RSpec.describe Terraform::State, feature_category: :infrastructure_as_code do describe '.ordered_by_name' do let_it_be(:project) { create(:project) } - let(:names) { %w(state_d state_b state_a state_c) } + let(:names) { %w[state_d state_b state_a state_c] } subject { described_class.ordered_by_name } diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 27e2060a94b..ff38edb73b6 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -266,7 +266,7 @@ RSpec.describe Upload do it 'updates project statistics when upload is added' do expect(ProjectCacheWorker).to receive(:perform_async) - .with(project.id, [], [:uploads_size]) + .with(project.id, [], ['uploads_size']) subject.save! end @@ -275,7 +275,7 @@ RSpec.describe Upload do subject.save! expect(ProjectCacheWorker).to receive(:perform_async) - .with(project.id, [], [:uploads_size]) + .with(project.id, [], ['uploads_size']) subject.destroy! end diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index 428fd5470c3..b443988cde9 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -59,6 +59,27 @@ RSpec.describe UserDetail do end end + describe '#mastodon' do + it { is_expected.to validate_length_of(:mastodon).is_at_most(500) } + + context 'when mastodon is set' do + let_it_be(:user_detail) { create(:user_detail) } + + it 'accepts a valid mastodon username' do + user_detail.mastodon = '@robin@example.com' + + expect(user_detail).to be_valid + end + + it 'throws an error when mastodon username format is wrong' do + user_detail.mastodon = '@robin' + + expect(user_detail).not_to be_valid + expect(user_detail.errors.full_messages).to match_array([_('Mastodon must contain only a mastodon username.')]) + end + end + end + describe '#location' do it { is_expected.to validate_length_of(:location).is_at_most(500) } end @@ -97,6 +118,7 @@ RSpec.describe UserDetail do discord: '1234567890123456789', linkedin: 'linkedin', location: 'location', + mastodon: '@robin@example.com', organization: 'organization', skype: 'skype', twitter: 'twitter', @@ -117,6 +139,7 @@ RSpec.describe UserDetail do it_behaves_like 'prevents `nil` value', :discord it_behaves_like 'prevents `nil` value', :linkedin it_behaves_like 'prevents `nil` value', :location + it_behaves_like 'prevents `nil` value', :mastodon it_behaves_like 'prevents `nil` value', :organization it_behaves_like 'prevents `nil` value', :skype it_behaves_like 'prevents `nil` value', :twitter diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 947d83badf6..fe229ce836f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -44,6 +44,9 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to delegate_method(:time_display_relative).to(:user_preference) } it { is_expected.to delegate_method(:time_display_relative=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:time_display_format).to(:user_preference) } + it { is_expected.to delegate_method(:time_display_format=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:show_whitespace_in_diffs).to(:user_preference) } it { is_expected.to delegate_method(:show_whitespace_in_diffs=).to(:user_preference).with_arguments(:args) } @@ -113,6 +116,9 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to delegate_method(:linkedin).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:linkedin=).to(:user_detail).with_arguments(:args).allow_nil } + it { is_expected.to delegate_method(:mastodon).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:mastodon=).to(:user_detail).with_arguments(:args).allow_nil } + it { is_expected.to delegate_method(:twitter).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:twitter=).to(:user_detail).with_arguments(:args).allow_nil } @@ -130,6 +136,9 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to delegate_method(:email_reset_offered_at).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:email_reset_offered_at=).to(:user_detail).with_arguments(:args).allow_nil } + + it { is_expected.to delegate_method(:project_authorizations_recalculated_at).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:project_authorizations_recalculated_at=).to(:user_detail).with_arguments(:args).allow_nil } end describe 'associations' do @@ -1277,7 +1286,7 @@ RSpec.describe User, feature_category: :user_profile do user = create(:user, username: 'CaMeLcAsEd') user2 = create(:user, username: 'UPPERCASE') - expect(described_class.by_username(%w(CAMELCASED uppercase))) + expect(described_class.by_username(%w[CAMELCASED uppercase])) .to contain_exactly(user, user2) end @@ -1416,6 +1425,16 @@ RSpec.describe User, feature_category: :user_profile do 'ORDER BY "users"."current_sign_in_at" ASC NULLS LAST') end end + + describe '.trusted' do + let_it_be(:trusted_user1) { create(:user, :trusted) } + let_it_be(:trusted_user2) { create(:user, :trusted) } + let_it_be(:user3) { create(:user) } + + it 'returns only the trusted users' do + expect(described_class.trusted).to match_array([trusted_user1, trusted_user2]) + end + end end context 'strip attributes' do @@ -1824,7 +1843,7 @@ RSpec.describe User, feature_category: :user_profile do end context 'when the confirmation period has expired' do - let(:confirmation_sent_at) { expired_confirmation_sent_at } + let(:confirmation_sent_at) { expired_confirmation_sent_at } it_behaves_like 'unconfirmed user' @@ -1842,7 +1861,7 @@ RSpec.describe User, feature_category: :user_profile do end context 'when the confirmation period has not expired' do - let(:confirmation_sent_at) { extant_confirmation_sent_at } + let(:confirmation_sent_at) { extant_confirmation_sent_at } it_behaves_like 'unconfirmed user' @@ -2033,7 +2052,7 @@ RSpec.describe User, feature_category: :user_profile do end context 'when the confirmation period has expired' do - let(:confirmation_sent_at) { expired_confirmation_sent_at } + let(:confirmation_sent_at) { expired_confirmation_sent_at } it_behaves_like 'unconfirmed user' it_behaves_like 'confirms the user on force_confirm' @@ -2855,6 +2874,12 @@ RSpec.describe User, feature_category: :user_profile do expect(described_class.filter_items('wop')).to include user end + + it 'filters by trusted' do + expect(described_class).to receive(:trusted).and_return([user]) + + expect(described_class.filter_items('trusted')).to include user + end end describe '.without_projects' do @@ -3261,6 +3286,9 @@ RSpec.describe User, feature_category: :user_profile do end describe 'username matching' do + let_it_be(:named_john) { create(:user, name: 'John', username: 'abcd') } + let_it_be(:username_john) { create(:user, name: 'John Doe', username: 'john') } + it 'returns users with a matching username' do expect(described_class.search(user.username)).to eq([user, user2]) end @@ -3281,6 +3309,10 @@ RSpec.describe User, feature_category: :user_profile do expect(described_class.search(user2.username.upcase)).to eq([user2]) end + it 'returns users with an exact matching username first' do + expect(described_class.search('John')).to eq([username_john, named_john]) + end + it 'returns users with a exact matching username shorter than 3 chars' do expect(described_class.search(user3.username)).to eq([user3]) end @@ -5814,37 +5846,37 @@ RSpec.describe User, feature_category: :user_profile do context 'oauth user' do it 'returns true if name can be synced' do - stub_omniauth_setting(sync_profile_attributes: %w(name location)) + stub_omniauth_setting(sync_profile_attributes: %w[name location]) expect(user.sync_attribute?(:name)).to be_truthy end it 'returns true if email can be synced' do - stub_omniauth_setting(sync_profile_attributes: %w(name email)) + stub_omniauth_setting(sync_profile_attributes: %w[name email]) expect(user.sync_attribute?(:email)).to be_truthy end it 'returns true if location can be synced' do - stub_omniauth_setting(sync_profile_attributes: %w(location email)) + stub_omniauth_setting(sync_profile_attributes: %w[location email]) expect(user.sync_attribute?(:email)).to be_truthy end it 'returns false if name can not be synced' do - stub_omniauth_setting(sync_profile_attributes: %w(location email)) + stub_omniauth_setting(sync_profile_attributes: %w[location email]) expect(user.sync_attribute?(:name)).to be_falsey end it 'returns false if email can not be synced' do - stub_omniauth_setting(sync_profile_attributes: %w(location name)) + stub_omniauth_setting(sync_profile_attributes: %w[location name]) expect(user.sync_attribute?(:email)).to be_falsey end it 'returns false if location can not be synced' do - stub_omniauth_setting(sync_profile_attributes: %w(name email)) + stub_omniauth_setting(sync_profile_attributes: %w[name email]) expect(user.sync_attribute?(:location)).to be_falsey end @@ -5875,7 +5907,7 @@ RSpec.describe User, feature_category: :user_profile do it 'returns true for email and location if ldap user and location declared as syncable' do allow(user).to receive(:ldap_user?).and_return(true) - stub_omniauth_setting(sync_profile_attributes: %w(location)) + stub_omniauth_setting(sync_profile_attributes: %w[location]) expect(user.sync_attribute?(:name)).to be_falsey expect(user.sync_attribute?(:email)).to be_truthy diff --git a/spec/models/guest_spec.rb b/spec/models/users/anonymous_spec.rb index 975b64cb855..f6151be6184 100644 --- a/spec/models/guest_spec.rb +++ b/spec/models/users/anonymous_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Guest do +RSpec.describe Users::Anonymous, feature_category: :system_access do let_it_be(:public_project, reload: true) { create(:project, :public) } let_it_be(:private_project) { create(:project, :private) } let_it_be(:internal_project) { create(:project, :internal) } diff --git a/spec/models/users/credit_card_validation_spec.rb b/spec/models/users/credit_card_validation_spec.rb index 7faddb2384c..ae75020c768 100644 --- a/spec/models/users/credit_card_validation_spec.rb +++ b/spec/models/users/credit_card_validation_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Users::CreditCardValidation, feature_category: :user_profile do + include CryptoHelpers + it { is_expected.to belong_to(:user) } it { is_expected.to validate_length_of(:holder_name).is_at_most(50) } @@ -206,12 +208,12 @@ RSpec.describe Users::CreditCardValidation, feature_category: :user_profile do context 'when last_digits has a blank value' do let(:last_digits) { ' ' } - it { expect { save_credit_card_validation }.not_to change { credit_card_validation.last_digits_hash } } + it { expect(credit_card_validation).to be_invalid } end context 'when last_digits has a value' do let(:last_digits) { 1111 } - let(:expected_last_digits_hash) { Gitlab::CryptoHelper.sha256(last_digits) } + let(:expected_last_digits_hash) { sha256(last_digits) } it 'assigns correct last_digits_hash value' do expect { save_credit_card_validation }.to change { @@ -240,7 +242,7 @@ RSpec.describe Users::CreditCardValidation, feature_category: :user_profile do context 'when holder_name has a value' do let(:holder_name) { 'John Smith' } - let(:expected_holder_name_hash) { Gitlab::CryptoHelper.sha256(holder_name.downcase) } + let(:expected_holder_name_hash) { sha256(holder_name.downcase) } it 'lowercases holder_name and assigns correct holder_name_hash value' do expect { save_credit_card_validation }.to change { @@ -269,7 +271,7 @@ RSpec.describe Users::CreditCardValidation, feature_category: :user_profile do context 'when network has a value' do let(:network) { 'Visa' } - let(:expected_network_hash) { Gitlab::CryptoHelper.sha256(network.downcase) } + let(:expected_network_hash) { sha256(network.downcase) } it 'lowercases network and assigns correct network_hash value' do expect { save_credit_card_validation }.to change { @@ -298,7 +300,7 @@ RSpec.describe Users::CreditCardValidation, feature_category: :user_profile do context 'when expiration_date has a value' do let(:expiration_date) { 1.year.from_now.to_date } - let(:expected_expiration_date_hash) { Gitlab::CryptoHelper.sha256(expiration_date.to_s) } + let(:expected_expiration_date_hash) { sha256(expiration_date.to_s) } it 'assigns correct expiration_date_hash value' do expect { save_credit_card_validation }.to change { diff --git a/spec/models/users/group_visit_spec.rb b/spec/models/users/group_visit_spec.rb index 63c4631ad7d..241cb537fad 100644 --- a/spec/models/users/group_visit_spec.rb +++ b/spec/models/users/group_visit_spec.rb @@ -7,10 +7,6 @@ RSpec.describe Users::GroupVisit, feature_category: :navigation do let_it_be(:user) { create(:user) } let_it_be(:base_time) { DateTime.now } - before do - described_class.create!(entity_id: entity.id, user_id: user.id, visited_at: base_time) - end - it_behaves_like 'namespace visits model' it_behaves_like 'cleanup by a loose foreign key' do @@ -22,4 +18,25 @@ RSpec.describe Users::GroupVisit, feature_category: :navigation do let!(:model) { create(:group_visit, entity_id: entity.id, user_id: user.id, visited_at: base_time) } let!(:parent) { user } end + + describe '#frecent_groups' do + let_it_be(:group1) { create(:group) } + let_it_be(:group2) { create(:group) } + + before do + [ + [group1.id, 1.day.ago], + [group2.id, 2.days.ago] + ].each do |id, datetime| + described_class.create!(entity_id: id, user_id: user.id, visited_at: datetime) + end + end + + it "returns the associated frecently visited groups" do + expect(described_class.frecent_groups(user_id: user.id)).to eq([ + group1, + group2 + ]) + end + end end diff --git a/spec/models/users/phone_number_validation_spec.rb b/spec/models/users/phone_number_validation_spec.rb index 7ab461a4346..e41719d8ca3 100644 --- a/spec/models/users/phone_number_validation_spec.rb +++ b/spec/models/users/phone_number_validation_spec.rb @@ -2,7 +2,10 @@ require 'spec_helper' -RSpec.describe Users::PhoneNumberValidation do +RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resiliency do + let_it_be(:user) { create(:user) } + let_it_be(:banned_user) { create(:user, :banned) } + it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:banned_user) } @@ -31,9 +34,6 @@ RSpec.describe Users::PhoneNumberValidation do let_it_be(:international_dial_code) { 1 } let_it_be(:phone_number) { '555' } - let_it_be(:user) { create(:user) } - let_it_be(:banned_user) { create(:user, :banned) } - subject(:related_to_banned_user?) do described_class.related_to_banned_user?(international_dial_code, phone_number) end @@ -79,25 +79,25 @@ RSpec.describe Users::PhoneNumberValidation do end end - describe '#for_user' do - let_it_be(:user_1) { create(:user) } - let_it_be(:user_2) { create(:user) } + describe 'scopes' do + let_it_be(:another_user) { create(:user) } - let_it_be(:phone_number_record_1) { create(:phone_number_validation, user: user_1) } - let_it_be(:phone_number_record_2) { create(:phone_number_validation, user: user_2) } + let_it_be(:phone_number_record_1) { create(:phone_number_validation, user: user, telesign_reference_xid: 'target') } + let_it_be(:phone_number_record_2) { create(:phone_number_validation, user: another_user) } - context 'when multiple records exist for multiple users' do - it 'returns the correct phone number record for user' do - records = described_class.for_user(user_1.id) + describe '#for_user' do + context 'when multiple records exist for multiple users' do + it 'returns the correct phone number record for user' do + records = described_class.for_user(user.id) - expect(records.count).to be(1) - expect(records.first).to eq(phone_number_record_1) + expect(records.count).to be(1) + expect(records.first).to eq(phone_number_record_1) + end end end end describe '#validated?' do - let_it_be(:user) { create(:user) } let_it_be(:phone_number_record) { create(:phone_number_validation, user: user) } context 'when phone number record is not validated' do @@ -116,4 +116,20 @@ RSpec.describe Users::PhoneNumberValidation do end end end + + describe '.by_reference_id' do + let_it_be(:phone_number_record) { create(:phone_number_validation) } + + let(:ref_id) { phone_number_record.telesign_reference_xid } + + subject { described_class.by_reference_id(ref_id) } + + it { is_expected.to eq phone_number_record } + + context 'when there is no matching record' do + let(:ref_id) { 'does-not-exist' } + + it { is_expected.to be_nil } + end + end end diff --git a/spec/models/users/project_visit_spec.rb b/spec/models/users/project_visit_spec.rb index 38747bd6462..50e9ef02fd8 100644 --- a/spec/models/users/project_visit_spec.rb +++ b/spec/models/users/project_visit_spec.rb @@ -7,10 +7,6 @@ RSpec.describe Users::ProjectVisit, feature_category: :navigation do let_it_be(:user) { create(:user) } let_it_be(:base_time) { DateTime.now } - before do - described_class.create!(entity_id: entity.id, user_id: user.id, visited_at: base_time) - end - it_behaves_like 'namespace visits model' it_behaves_like 'cleanup by a loose foreign key' do @@ -22,4 +18,25 @@ RSpec.describe Users::ProjectVisit, feature_category: :navigation do let!(:model) { create(:project_visit, entity_id: entity.id, user_id: user.id, visited_at: base_time) } let!(:parent) { user } end + + describe '#frecent_projects' do + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + + before do + [ + [project1.id, 1.day.ago], + [project2.id, 2.days.ago] + ].each do |id, datetime| + described_class.create!(entity_id: id, user_id: user.id, visited_at: datetime) + end + end + + it "returns the associated frecently visited projects" do + expect(described_class.frecent_projects(user_id: user.id)).to eq([ + project1, + project2 + ]) + end + end end diff --git a/spec/models/vs_code/settings/vs_code_setting_spec.rb b/spec/models/vs_code/settings/vs_code_setting_spec.rb index d22cc815877..d61e89dc54b 100644 --- a/spec/models/vs_code/settings/vs_code_setting_spec.rb +++ b/spec/models/vs_code/settings/vs_code_setting_spec.rb @@ -11,10 +11,18 @@ RSpec.describe VsCode::Settings::VsCodeSetting, feature_category: :web_ide do it { is_expected.to validate_presence_of(:content) } end + describe 'validates the uniqueness of attributes' do + it { is_expected.to validate_uniqueness_of(:setting_type).scoped_to([:user_id]) } + end + describe 'relationship validation' do it { is_expected.to belong_to(:user) } end + describe 'settings type validation' do + it { is_expected.to validate_inclusion_of(:setting_type).in_array(VsCode::Settings::SETTINGS_TYPES) } + end + describe '.by_setting_type' do subject { described_class.by_setting_type('settings') } diff --git a/spec/models/web_ide_terminal_spec.rb b/spec/models/web_ide_terminal_spec.rb index fc30bc18f68..505b7531db4 100644 --- a/spec/models/web_ide_terminal_spec.rb +++ b/spec/models/web_ide_terminal_spec.rb @@ -45,7 +45,7 @@ RSpec.describe WebIdeTerminal do end it 'returns services aliases' do - expect(subject.services).to eq %w(postgres docker) + expect(subject.services).to eq %w[postgres docker] end end @@ -55,7 +55,7 @@ RSpec.describe WebIdeTerminal do end it 'returns all aliases' do - expect(subject.services).to eq %w(postgres docker ruby) + expect(subject.services).to eq %w[postgres docker ruby] end end @@ -71,7 +71,7 @@ RSpec.describe WebIdeTerminal do context 'when no image nor services' do let(:config) do - { script: %w(echo) } + { script: %w[echo] } end it 'returns an empty array' do diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 2e1cb9d3d9b..f3cf8966b9a 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -58,6 +58,7 @@ RSpec.describe WikiPage, feature_category: :wiki do let(:front_matter) { { title: 'Foo', slugs: %w[slug_a slug_b] } } it { expect(wiki_page.front_matter).to eq(front_matter) } + it { expect(wiki_page.front_matter_title).to eq(front_matter[:title]) } end context 'the wiki page has front matter' do @@ -1054,4 +1055,28 @@ RSpec.describe WikiPage, feature_category: :wiki do ) end end + + describe "#human_title" do + context "with front matter title" do + let(:front_matter_title) { "abc" } + let(:content_with_front_matter_title) { "---\ntitle: #{front_matter_title}\n---\nHome Page" } + let(:wiki_page) { create(:wiki_page, container: container, content: content_with_front_matter_title) } + + context "when wiki_front_matter_title enabled" do + it 'returns the front matter title' do + expect(wiki_page.human_title).to eq front_matter_title + end + end + + context "when wiki_front_matter_title disabled" do + before do + stub_feature_flags(wiki_front_matter_title: false) + end + + it 'returns the page title' do + expect(wiki_page.human_title).to eq wiki_page.title + end + end + end + end end diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb index 3294d53e364..476d346db10 100644 --- a/spec/models/work_item_spec.rb +++ b/spec/models/work_item_spec.rb @@ -402,6 +402,12 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do end end + describe '#linked_items_keyset_order' do + subject { described_class.linked_items_keyset_order } + + it { is_expected.to eq('"issue_links"."id" ASC') } + end + context 'with hierarchy' do let_it_be(:type1) { create(:work_item_type, namespace: reusable_project.namespace) } let_it_be(:type2) { create(:work_item_type, namespace: reusable_project.namespace) } diff --git a/spec/models/zoom_meeting_spec.rb b/spec/models/zoom_meeting_spec.rb index d3d75a19fed..b67a9d4a2ff 100644 --- a/spec/models/zoom_meeting_spec.rb +++ b/spec/models/zoom_meeting_spec.rb @@ -65,7 +65,7 @@ RSpec.describe ZoomMeeting do context 'with non-Zoom URL' do before do - subject.url = %{https://non-zoom.url} + subject.url = %(https://non-zoom.url) end include_examples 'invalid Zoom URL' @@ -73,7 +73,7 @@ RSpec.describe ZoomMeeting do context 'with multiple Zoom-URLs' do before do - subject.url = %{https://zoom.us/j/123 https://zoom.us/j/456} + subject.url = %(https://zoom.us/j/123 https://zoom.us/j/456) end include_examples 'invalid Zoom URL' |