diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-17 14:33:21 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-17 14:33:21 +0300 |
commit | 7021455bd1ed7b125c55eb1b33c5a01f2bc55ee0 (patch) | |
tree | 5bdc2229f5198d516781f8d24eace62fc7e589e9 /spec/models | |
parent | 185b095e93520f96e9cfc31d9c3e69b498cdab7c (diff) |
Add latest changes from gitlab-org/gitlab@15-6-stable-eev15.6.0-rc42
Diffstat (limited to 'spec/models')
149 files changed, 2902 insertions, 2176 deletions
diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index 5d316f7cff2..3665f13015e 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -260,7 +260,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do redis.set("session:gitlab:#{rack_session.private_id}", '') redis.set(session_key, serialized_session) - redis.sadd(lookup_key, active_session_lookup_key) + redis.sadd?(lookup_key, active_session_lookup_key) end end @@ -338,7 +338,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do session_private_id = Rack::Session::SessionId.new(session_public_id).private_id active_session = ActiveSession.new(session_private_id: session_private_id) redis.set(key_name(user.id, session_private_id), dump_session(active_session)) - redis.sadd(lookup_key, session_private_id) + redis.sadd?(lookup_key, session_private_id) end # setup for unrelated user @@ -347,7 +347,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do active_session = ActiveSession.new(session_private_id: session_private_id) redis.set(key_name(unrelated_user_id, session_private_id), dump_session(active_session)) - redis.sadd(described_class.lookup_key_name(unrelated_user_id), session_private_id) + redis.sadd?(described_class.lookup_key_name(unrelated_user_id), session_private_id) end end @@ -372,7 +372,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do Gitlab::Redis::Sessions.with do |redis| redis.set(key_name(user.id, impersonated_session_id), dump_session(ActiveSession.new(session_id: Rack::Session::SessionId.new(impersonated_session_id), is_impersonated: true))) - redis.sadd(lookup_key, impersonated_session_id) + redis.sadd?(lookup_key, impersonated_session_id) end expect { ActiveSession.destroy_all_but_current(user, request.session) }.to change { ActiveSession.session_ids_for_user(user.id).size }.from(3).to(2) @@ -418,8 +418,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do it 'removes obsolete lookup entries' do Gitlab::Redis::Sessions.with do |redis| redis.set(session_key, '') - redis.sadd(lookup_key, current_session_id) - redis.sadd(lookup_key, '59822c7d9fcdfa03725eff41782ad97d') + redis.sadd(lookup_key, [current_session_id, '59822c7d9fcdfa03725eff41782ad97d']) end ActiveSession.cleanup(user) @@ -445,7 +444,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do key_name(user.id, number), dump_session(ActiveSession.new(session_id: number.to_s, updated_at: number.days.ago)) ) - redis.sadd(lookup_key, number.to_s) + redis.sadd?(lookup_key, number.to_s) end end end @@ -477,7 +476,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do it 'removes obsolete lookup entries even without active session' do Gitlab::Redis::Sessions.with do |redis| - redis.sadd(lookup_key, "#{max_number_of_sessions_plus_two + 1}") + redis.sadd?(lookup_key, (max_number_of_sessions_plus_two + 1).to_s) end ActiveSession.cleanup(user) @@ -534,7 +533,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do key_name(user.id, number), dump_session(ActiveSession.new(session_private_id: number.to_s, updated_at: number.days.ago)) ) - redis.sadd(lookup_key, number.to_s) + redis.sadd?(lookup_key, number.to_s) end end end @@ -601,11 +600,10 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do dump_session(ActiveSession.new(session_id: number.to_s, updated_at: number.days.ago)) ) - redis.sadd(lookup_key, number.to_s) + redis.sadd?(lookup_key, number.to_s) end - redis.sadd(lookup_key, (active_count + 1).to_s) - redis.sadd(lookup_key, (active_count + 2).to_s) + redis.sadd?(lookup_key, [(active_count + 1).to_s, (active_count + 2).to_s]) end end diff --git a/spec/models/alert_management/http_integration_spec.rb b/spec/models/alert_management/http_integration_spec.rb index f88a66a7c27..b453b3a82e0 100644 --- a/spec/models/alert_management/http_integration_spec.rb +++ b/spec/models/alert_management/http_integration_spec.rb @@ -13,6 +13,11 @@ RSpec.describe AlertManagement::HttpIntegration do it { is_expected.to belong_to(:project) } end + describe 'default values' do + it { expect(described_class.new.endpoint_identifier).to be_present } + it { expect(described_class.new(endpoint_identifier: 'test').endpoint_identifier).to eq('test') } + end + describe 'validations' do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:name) } @@ -124,10 +129,6 @@ RSpec.describe AlertManagement::HttpIntegration do end context 'when unsaved' do - context 'when unassigned' do - it_behaves_like 'valid token' - end - context 'when assigned' do include_context 'assign token', 'random_token' diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 2817e177d28..9d84279a75e 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -10,6 +10,20 @@ RSpec.describe Appearance do it { is_expected.to have_many(:uploads) } + describe 'default values' do + subject(:appearance) { described_class.new } + + it { expect(appearance.title).to eq('') } + it { expect(appearance.description).to eq('') } + it { expect(appearance.new_project_guidelines).to eq('') } + it { expect(appearance.profile_image_guidelines).to eq('') } + it { expect(appearance.header_message).to eq('') } + it { expect(appearance.footer_message).to eq('') } + it { expect(appearance.message_background_color).to eq('#E75E40') } + it { expect(appearance.message_font_color).to eq('#FFFFFF') } + it { expect(appearance.email_header_and_footer_enabled).to eq(false) } + end + describe '#single_appearance_row' do it 'adds an error when more than 1 row exists' do create(:appearance) diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 77bb6b502b5..fd86a784b2d 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -17,6 +17,14 @@ RSpec.describe ApplicationSetting do it { expect(setting.uuid).to be_present } it { expect(setting).to have_db_column(:auto_devops_enabled) } + describe 'default values' do + subject(:setting) { described_class.new } + + it { expect(setting.id).to eq(1) } + it { expect(setting.repository_storages_weighted).to eq({}) } + it { expect(setting.kroki_formats).to eq({}) } + end + describe 'validations' do let(:http) { 'http://example.com' } let(:https) { 'https://example.com' } @@ -203,6 +211,9 @@ RSpec.describe ApplicationSetting do 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(http).for(:jira_connect_proxy_url) } + it { is_expected.to allow_value(https).for(:jira_connect_proxy_url) } + context 'when deactivate_dormant_users is enabled' do before do stub_application_setting(deactivate_dormant_users: true) @@ -261,6 +272,7 @@ RSpec.describe ApplicationSetting do end it { is_expected.not_to allow_value('http://localhost:9000').for(:grafana_url) } + it { is_expected.not_to allow_value('http://localhost:9000').for(:jira_connect_proxy_url) } end context 'with invalid grafana URL' do @@ -1121,6 +1133,11 @@ RSpec.describe ApplicationSetting do it { is_expected.to validate_presence_of(:error_tracking_api_url) } end end + + context 'for default_preferred_language' do + it { is_expected.to allow_value(*Gitlab::I18n.available_locales).for(:default_preferred_language) } + it { is_expected.not_to allow_value(nil, '', 'invalid_locale').for(:default_preferred_language) } + end end context 'restrict creating duplicates' do diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index b0bfdabe13c..8fdc9852f6e 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -27,6 +27,13 @@ RSpec.describe BroadcastMessage do it { is_expected.to validate_inclusion_of(:target_access_levels).in_array(described_class::ALLOWED_TARGET_ACCESS_LEVELS) } end + describe 'default values' do + subject(:message) { described_class.new } + + it { expect(message.color).to eq('#E75E40') } + it { expect(message.font).to eq('#FFFFFF') } + end + shared_examples 'time constrainted' do |broadcast_type| it 'returns message if time match' do message = create(:broadcast_message, broadcast_type: broadcast_type) diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 44a6bec0130..df24c92149d 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -27,6 +27,8 @@ RSpec.describe Ci::Bridge do it_behaves_like 'has ID tokens', :ci_bridge + it_behaves_like 'a retryable job' + it 'has one downstream pipeline' do expect(bridge).to have_one(:sourced_pipeline) expect(bridge).to have_one(:downstream_pipeline) @@ -35,18 +37,8 @@ RSpec.describe Ci::Bridge do describe '#retryable?' do let(:bridge) { create(:ci_bridge, :success) } - it 'returns true' do - expect(bridge.retryable?).to eq(true) - end - - context 'without ci_recreate_downstream_pipeline ff' do - before do - stub_feature_flags(ci_recreate_downstream_pipeline: false) - end - - it 'returns false' do - expect(bridge.retryable?).to eq(false) - end + it 'returns false' do + expect(bridge.retryable?).to eq(false) end end @@ -204,6 +196,8 @@ RSpec.describe Ci::Bridge do end describe '#downstream_variables' do + subject(:downstream_variables) { bridge.downstream_variables } + it 'returns variables that are going to be passed downstream' do expect(bridge.downstream_variables) .to include(key: 'BRIDGE', value: 'cross') @@ -309,7 +303,7 @@ RSpec.describe Ci::Bridge do end context 'when the pipeline runs from a pipeline schedule' do - let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) } + let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project) } let(:pipeline) { create(:ci_pipeline, pipeline_schedule: pipeline_schedule) } let(:options) do @@ -328,6 +322,79 @@ RSpec.describe Ci::Bridge do end end end + + context 'when using raw variables' do + let(:options) do + { + trigger: { + project: 'my/project', + branch: 'master', + forward: { yaml_variables: true, + pipeline_variables: true }.compact + } + } + end + + let(:yaml_variables) do + [ + { + key: 'VAR6', + value: 'value6 $VAR1' + }, + { + key: 'VAR7', + value: 'value7 $VAR1', + raw: true + } + ] + end + + let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project) } + let(:pipeline) { create(:ci_pipeline, pipeline_schedule: pipeline_schedule) } + + before do + create(:ci_pipeline_variable, pipeline: pipeline, key: 'VAR1', value: 'value1') + create(:ci_pipeline_variable, pipeline: pipeline, key: 'VAR2', value: 'value2 $VAR1') + create(:ci_pipeline_variable, pipeline: pipeline, key: 'VAR3', value: 'value3 $VAR1', raw: true) + + pipeline_schedule.variables.create!(key: 'VAR4', value: 'value4 $VAR1') + pipeline_schedule.variables.create!(key: 'VAR5', value: 'value5 $VAR1', raw: true) + + bridge.yaml_variables.concat(yaml_variables) + end + + it 'expands variables according to their raw attributes' do + expect(downstream_variables).to contain_exactly( + { key: 'BRIDGE', value: 'cross' }, + { key: 'VAR1', value: 'value1' }, + { key: 'VAR2', value: 'value2 value1' }, + { key: 'VAR3', value: 'value3 $VAR1', raw: true }, + { key: 'VAR4', value: 'value4 value1' }, + { key: 'VAR5', value: 'value5 $VAR1', raw: true }, + { key: 'VAR6', value: 'value6 value1' }, + { key: 'VAR7', value: 'value7 $VAR1', raw: true } + ) + end + + context 'when the FF ci_raw_variables_in_yaml_config is disabled' do + before do + stub_feature_flags(ci_raw_variables_in_yaml_config: false) + end + + it 'ignores the raw attribute' do + expect(downstream_variables).to contain_exactly( + { key: 'BRIDGE', value: 'cross' }, + { key: 'VAR1', value: 'value1' }, + { key: 'VAR2', value: 'value2 value1' }, + { key: 'VAR3', value: 'value3 value1' }, + { key: 'VAR4', value: 'value4 value1' }, + { key: 'VAR5', value: 'value5 value1' }, + { key: 'VAR6', value: 'value6 value1' }, + { key: 'VAR7', value: 'value7 value1' } + ) + end + end + end end describe 'metadata support' do diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index 16cff72db64..e728ce0f474 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -182,4 +182,36 @@ RSpec.describe Ci::BuildMetadata do end end end + + describe 'routing table switch' do + context 'with ff disabled' do + before do + stub_feature_flags(ci_partitioning_use_ci_builds_metadata_routing_table: false) + end + + it 'uses the legacy table' do + expect(described_class.table_name).to eq('ci_builds_metadata') + end + end + + context 'with ff enabled' do + before do + stub_feature_flags(ci_partitioning_use_ci_builds_metadata_routing_table: true) + end + + it 'uses the routing table' do + expect(described_class.table_name).to eq('p_ci_builds_metadata') + end + end + end + + context 'jsonb fields serialization' do + it 'changing other fields does not change config_options' do + expect { metadata.id = metadata.id }.not_to change(metadata, :changes) + end + + it 'accessing config_options does not change it' do + expect { metadata.config_options }.not_to change(metadata, :changes) + end + end end diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb index ed5ed456d7b..9bb8a1bd626 100644 --- a/spec/models/ci/build_runner_session_spec.rb +++ b/spec/models/ci/build_runner_session_spec.rb @@ -72,7 +72,7 @@ RSpec.describe Ci::BuildRunnerSession, model: true do let(:specification) { subject.service_specification(service: service, port: port, path: path, subprotocols: subprotocols) } it 'returns service proxy url' do - expect(specification[:url]).to eq "https://localhost/proxy/#{service}/#{port}/#{path}" + expect(specification[:url]).to eq "https://gitlab.example.com/proxy/#{service}/#{port}/#{path}" end it 'returns default service proxy websocket subprotocol' do @@ -89,7 +89,7 @@ RSpec.describe Ci::BuildRunnerSession, model: true do let(:port) { nil } it 'uses the default port name' do - expect(specification[:url]).to eq "https://localhost/proxy/#{service}/default_port/#{path}" + expect(specification[:url]).to eq "https://gitlab.example.com/proxy/#{service}/default_port/#{path}" end end @@ -97,7 +97,7 @@ RSpec.describe Ci::BuildRunnerSession, model: true do let(:service) { '' } it 'uses the service name "build" as default' do - expect(specification[:url]).to eq "https://localhost/proxy/build/#{port}/#{path}" + expect(specification[:url]).to eq "https://gitlab.example.com/proxy/build/#{port}/#{path}" end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 9713734e97a..813b4b3faa6 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -25,6 +25,7 @@ RSpec.describe Ci::Build do it { is_expected.to have_many(:needs) } it { is_expected.to have_many(:sourced_pipelines) } + it { is_expected.to have_one(:sourced_pipeline) } it { is_expected.to have_many(:job_variables) } it { is_expected.to have_many(:report_results) } it { is_expected.to have_many(:pages_deployments) } @@ -86,6 +87,8 @@ RSpec.describe Ci::Build do it_behaves_like 'has ID tokens', :ci_build + it_behaves_like 'a retryable job' + describe '.manual_actions' do let!(:manual_but_created) { create(:ci_build, :manual, status: :created, pipeline: pipeline) } let!(:manual_but_succeeded) { create(:ci_build, :manual, status: :success, pipeline: pipeline) } @@ -605,8 +608,8 @@ RSpec.describe Ci::Build do end end - describe '#prevent_rollback_deployment?' do - subject { build.prevent_rollback_deployment? } + describe '#outdated_deployment?' do + subject { build.outdated_deployment? } let(:build) { create(:ci_build, :created, :with_deployment, project: project, environment: 'production') } @@ -624,21 +627,33 @@ RSpec.describe Ci::Build do it { expect(subject).to be_falsey } end - context 'when deployment cannot rollback' do + context 'when build is not an outdated deployment' do before do - expect(build.deployment).to receive(:older_than_last_successful_deployment?).and_return(false) + allow(build.deployment).to receive(:older_than_last_successful_deployment?).and_return(false) end it { expect(subject).to be_falsey } end - context 'when build can prevent rollback deployment' do + context 'when build is older than the latest deployment and still pending status' do before do - expect(build.deployment).to receive(:older_than_last_successful_deployment?).and_return(true) + allow(build.deployment).to receive(:older_than_last_successful_deployment?).and_return(true) end it { expect(subject).to be_truthy } end + + context 'when build is older than the latest deployment but succeeded once' do + let(:build) { create(:ci_build, :success, :with_deployment, project: project, environment: 'production') } + + before do + allow(build.deployment).to receive(:older_than_last_successful_deployment?).and_return(true) + end + + it 'returns false for allowing rollback' do + expect(subject).to be_falsey + end + end end describe '#schedulable?' do @@ -1316,6 +1331,8 @@ RSpec.describe Ci::Build do end context 'hide build token' do + let_it_be(:build) { FactoryBot.build(:ci_build, pipeline: pipeline) } + let(:data) { "new #{build.token} data" } it { is_expected.to match(/^new x+ data$/) } @@ -1606,8 +1623,8 @@ RSpec.describe Ci::Build do end describe 'environment' do - describe '#has_environment?' do - subject { build.has_environment? } + describe '#has_environment_keyword?' do + subject { build.has_environment_keyword? } context 'when environment is defined' do before do @@ -1751,7 +1768,7 @@ RSpec.describe Ci::Build do context 'and start action is defined' do before do - build.update!(options: { environment: { action: 'start' } } ) + build.update!(options: { environment: { action: 'start' } }) end it { is_expected.to be_truthy } @@ -1781,7 +1798,7 @@ RSpec.describe Ci::Build do context 'and stop action is defined' do before do - build.update!(options: { environment: { action: 'stop' } } ) + build.update!(options: { environment: { action: 'stop' } }) end it { is_expected.to be_truthy } @@ -2783,16 +2800,6 @@ RSpec.describe Ci::Build do expect(environment_based_variables_collection).to be_empty end - context 'when ci_job_jwt feature flag is disabled' do - before do - stub_feature_flags(ci_job_jwt: false) - end - - it 'CI_JOB_JWT is not included' do - expect(subject.pluck(:key)).not_to include('CI_JOB_JWT') - end - end - context 'when CI_JOB_JWT generation fails' do [ OpenSSL::PKey::RSAError, @@ -3806,6 +3813,26 @@ RSpec.describe Ci::Build do build.enqueue end + + it 'assigns the token' do + expect { build.enqueue }.to change(build, :token).from(nil).to(an_instance_of(String)) + end + + context 'with ci_assign_job_token_on_scheduling disabled' do + before do + stub_feature_flags(ci_assign_job_token_on_scheduling: false) + end + + it 'assigns the token on creation' do + expect(build.token).to be_present + end + + it 'does not change the token when enqueuing' do + expect { build.enqueue }.not_to change(build, :token) + + expect(build).to be_pending + end + end end describe 'state transition: pending: :running' do @@ -5083,6 +5110,60 @@ RSpec.describe Ci::Build do context 'when CI_DEBUG_TRACE is not in variables' do it { is_expected.to eq false } end + + context 'when CI_DEBUG_SERVICES=true is in variables' do + context 'when in instance variables' do + before do + create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: 'true') + end + + it { is_expected.to eq true } + end + + context 'when in group variables' do + before do + create(:ci_group_variable, key: 'CI_DEBUG_SERVICES', value: 'true', group: project.group) + end + + it { is_expected.to eq true } + end + + context 'when in pipeline variables' do + before do + create(:ci_pipeline_variable, key: 'CI_DEBUG_SERVICES', value: 'true', pipeline: pipeline) + end + + it { is_expected.to eq true } + end + + context 'when in project variables' do + before do + create(:ci_variable, key: 'CI_DEBUG_SERVICES', value: 'true', project: project) + end + + it { is_expected.to eq true } + end + + context 'when in job variables' do + before do + create(:ci_job_variable, key: 'CI_DEBUG_SERVICES', value: 'true', job: build) + end + + it { is_expected.to eq true } + end + + context 'when in yaml variables' do + before do + build.update!(yaml_variables: [{ key: :CI_DEBUG_SERVICES, value: 'true' }]) + end + + it { is_expected.to eq true } + end + end + + context 'when CI_DEBUG_SERVICES is not in variables' do + it { is_expected.to eq false } + end end describe '#drop_with_exit_code!' do diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index e08fe196d65..3328ed62f15 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -29,6 +29,11 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git }[data_store] end + describe 'default attributes' do + it { expect(described_class.new.data_store).to eq('redis_trace_chunks') } + it { expect(described_class.new(data_store: :fog).data_store).to eq('fog') } + end + describe 'chunk creation' do let(:metrics) { spy('metrics') } diff --git a/spec/models/ci/build_trace_spec.rb b/spec/models/ci/build_trace_spec.rb index f2df4874b84..907b49dc180 100644 --- a/spec/models/ci/build_trace_spec.rb +++ b/spec/models/ci/build_trace_spec.rb @@ -38,9 +38,9 @@ RSpec.describe Ci::BuildTrace do let(:data) { StringIO.new("UTF-8 dashes here: ───\n🐤🐤🐤🐤\xF0\x9F\x90\n") } it 'returns valid UTF-8 data', :aggregate_failures do - expect(subject.lines[0]).to eq({ offset: 0, content: [{ text: 'UTF-8 dashes here: ───' }] } ) + expect(subject.lines[0]).to eq({ offset: 0, content: [{ text: 'UTF-8 dashes here: ───' }] }) # Each of the dashes is 3 bytes, so we get 19 + 9 + 1 = 29 - expect(subject.lines[1]).to eq({ offset: 29, content: [{ text: '🐤🐤🐤🐤�' }] } ) + expect(subject.lines[1]).to eq({ offset: 29, content: [{ text: '🐤🐤🐤🐤�' }] }) end end end diff --git a/spec/models/ci/pipeline_metadata_spec.rb b/spec/models/ci/pipeline_metadata_spec.rb index 0704cbc8ec1..977c90bcc2a 100644 --- a/spec/models/ci/pipeline_metadata_spec.rb +++ b/spec/models/ci/pipeline_metadata_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Ci::PipelineMetadata do it { is_expected.to belong_to(:pipeline) } describe 'validations' do - it { is_expected.to validate_length_of(:title).is_at_least(1).is_at_most(255) } + it { is_expected.to validate_length_of(:name).is_at_least(1).is_at_most(255) } it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:pipeline) } end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 42b5210a080..2c945898e61 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -50,7 +50,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it { is_expected.to respond_to :git_author_full_text } it { is_expected.to respond_to :short_sha } it { is_expected.to delegate_method(:full_path).to(:project).with_prefix } - it { is_expected.to delegate_method(:title).to(:pipeline_metadata).allow_nil } + it { is_expected.to delegate_method(:name).to(:pipeline_metadata).allow_nil } describe 'validations' do it { is_expected.to validate_presence_of(:sha) } @@ -166,7 +166,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it do pipeline.status = from_status.to_s - if from_status != to_status + if from_status != to_status || success_to_success? expect(pipeline.set_status(to_status.to_s)) .to eq(true) else @@ -174,6 +174,12 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do .to eq(false), "loopback transitions are not allowed" end end + + private + + def success_to_success? + from_status == :success && to_status == :success + end end end @@ -1601,7 +1607,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'track artifact report' do - let(:pipeline) { create(:ci_pipeline, :running, :with_test_reports, status: :running, user: create(:user)) } + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:pipeline) { create(:ci_pipeline, :running, :with_test_reports, :with_coverage_reports, status: :running, user: user) } context 'when transitioning to completed status' do %i[drop! skip! succeed! cancel!].each do |command| @@ -1613,11 +1620,29 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline retried from failed to success', :clean_gitlab_redis_shared_state do - let(:test_event_name) { 'i_testing_test_report_uploaded' } + let(:test_event_name_1) { 'i_testing_test_report_uploaded' } + let(:test_event_name_2) { 'i_testing_coverage_report_uploaded' } let(:start_time) { 1.week.ago } let(:end_time) { 1.week.from_now } - it 'counts only one report' do + it 'counts only one test event report' do + expect(Ci::JobArtifacts::TrackArtifactReportWorker).to receive(:perform_async).with(pipeline.id).twice.and_call_original + + Sidekiq::Testing.inline! do + pipeline.drop! + pipeline.run! + pipeline.succeed! + end + + unique_pipeline_pass = Gitlab::UsageDataCounters::HLLRedisCounter.unique_events( + event_names: test_event_name_1, + start_date: start_time, + end_date: end_time + ) + expect(unique_pipeline_pass).to eq(1) + end + + it 'counts only one coverage event report' do expect(Ci::JobArtifacts::TrackArtifactReportWorker).to receive(:perform_async).with(pipeline.id).twice.and_call_original Sidekiq::Testing.inline! do @@ -1627,7 +1652,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end unique_pipeline_pass = Gitlab::UsageDataCounters::HLLRedisCounter.unique_events( - event_names: test_event_name, + event_names: test_event_name_2, start_date: start_time, end_date: end_time ) @@ -4173,7 +4198,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do let(:pipeline) { create(:ci_pipeline) } let!(:old_job) { create(:ci_build, name: 'rspec', retried: true, pipeline: pipeline) } let!(:job_without_artifacts) { create(:ci_build, name: 'rspec', pipeline: pipeline) } - let!(:expected_job) { create(:ci_build, :artifacts, name: 'rspec', pipeline: pipeline ) } + let!(:expected_job) { create(:ci_build, :artifacts, name: 'rspec', pipeline: pipeline) } let!(:different_job) { create(:ci_build, name: 'deploy', pipeline: pipeline) } subject { pipeline.find_job_with_archive_artifacts('rspec') } diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index a199111b1e3..e62e5f84a6d 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -52,7 +52,11 @@ RSpec.describe Ci::Processable do let_it_be(:internal_job_variable) { create(:ci_job_variable, job: processable) } - let(:clone_accessors) { ::Ci::Build.clone_accessors.without(::Ci::Build.extra_accessors) } + let(:clone_accessors) do + %i[pipeline project ref tag options name allow_failure stage stage_idx trigger_request yaml_variables + when environment coverage_regex description tag_list protected needs_attributes job_variables_attributes + resource_group scheduling_type ci_stage partition_id id_tokens] + end let(:reject_accessors) do %i[id status user token_encrypted coverage runner artifacts_expire_at @@ -77,13 +81,14 @@ RSpec.describe Ci::Processable do commit_id deployment erased_by_id project_id runner_id tag_taggings taggings tags trigger_request_id user_id auto_canceled_by_id retried failure_reason - sourced_pipelines artifacts_file_store artifacts_metadata_store + sourced_pipelines sourced_pipeline artifacts_file_store artifacts_metadata_store metadata runner_session trace_chunks upstream_pipeline_id artifacts_file artifacts_metadata artifacts_size commands resource resource_group_id processed security_scans author pipeline_id report_results pending_state pages_deployments queuing_entry runtime_metadata trace_metadata - dast_site_profile dast_scanner_profile stage_id].freeze + dast_site_profile dast_scanner_profile stage_id dast_site_profiles_build + dast_scanner_profiles_build].freeze end before_all do @@ -177,10 +182,7 @@ RSpec.describe Ci::Processable do Ci::Build.attribute_names.map(&:to_sym) + Ci::Build.attribute_aliases.keys.map(&:to_sym) + Ci::Build.reflect_on_all_associations.map(&:name) + - [:tag_list, :needs_attributes, :job_variables_attributes, :id_tokens] - - # ToDo: Move EE accessors to ee/ - ::Ci::Build.extra_accessors - - [:dast_site_profiles_build, :dast_scanner_profiles_build] + [:tag_list, :needs_attributes, :job_variables_attributes, :id_tokens] current_accessors.uniq! @@ -284,12 +286,6 @@ RSpec.describe Ci::Processable do end end - context 'when the processable is a bridge' do - subject(:processable) { create(:ci_bridge, pipeline: pipeline) } - - it_behaves_like 'retryable processable' - end - context 'when the processable is a build' do subject(:processable) { create(:ci_build, pipeline: pipeline) } diff --git a/spec/models/ci/secure_file_spec.rb b/spec/models/ci/secure_file_spec.rb index 20f64d40865..4413bd8e98b 100644 --- a/spec/models/ci/secure_file_spec.rb +++ b/spec/models/ci/secure_file_spec.rb @@ -21,6 +21,15 @@ RSpec.describe Ci::SecureFile do subject { build(:ci_secure_file, project: create(:project)) } end + describe 'default attributes' do + before do + allow(Ci::SecureFileUploader).to receive(:default_store).and_return(5) + end + + it { expect(described_class.new.file_store).to eq(5) } + it { expect(described_class.new(file_store: 3).file_store).to eq(3) } + end + describe 'validations' do it { is_expected.to validate_presence_of(:checksum) } it { is_expected.to validate_presence_of(:file_store) } @@ -131,7 +140,7 @@ RSpec.describe Ci::SecureFile do describe '#update_metadata!' do it 'assigns the expected metadata when a parsable file is supplied' do file = create(:ci_secure_file, name: 'file1.cer', - file: CarrierWaveStringFile.new(fixture_file('ci_secure_files/sample.cer') )) + file: CarrierWaveStringFile.new(fixture_file('ci_secure_files/sample.cer'))) file.update_metadata! expect(file.expires_at).to eq(DateTime.parse('2022-04-26 19:20:40')) diff --git a/spec/models/ci/sources/pipeline_spec.rb b/spec/models/ci/sources/pipeline_spec.rb index 732dd5c3df3..fdc1c111c40 100644 --- a/spec/models/ci/sources/pipeline_spec.rb +++ b/spec/models/ci/sources/pipeline_spec.rb @@ -20,14 +20,14 @@ RSpec.describe Ci::Sources::Pipeline do context 'loose foreign key on ci_sources_pipelines.source_project_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:project) } + let!(:parent) { create(:project, namespace: create(:group)) } let!(:model) { create(:ci_sources_pipeline, source_project: parent) } end end context 'loose foreign key on ci_sources_pipelines.project_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:project) } + let!(:parent) { create(:project, namespace: create(:group)) } let!(:model) { create(:ci_sources_pipeline, project: parent) } end end diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index dd9af33a562..b392ab4ed11 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -389,7 +389,7 @@ RSpec.describe Ci::Stage, :models do end context 'without pipeline' do - subject(:stage) { build(:ci_stage, pipeline: nil) } + subject(:stage) { build(:ci_stage, pipeline: nil, project: build_stubbed(:project)) } it { is_expected.to validate_presence_of(:partition_id) } diff --git a/spec/models/ci/trigger_request_spec.rb b/spec/models/ci/trigger_request_spec.rb index 0d462741089..a6e8e8496ac 100644 --- a/spec/models/ci/trigger_request_spec.rb +++ b/spec/models/ci/trigger_request_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Ci::TriggerRequest do describe 'validation' do it 'be invalid if saving a variable' do - trigger = build(:ci_trigger_request, variables: { TRIGGER_KEY_1: 'TRIGGER_VALUE_1' } ) + trigger = build(:ci_trigger_request, variables: { TRIGGER_KEY_1: 'TRIGGER_VALUE_1' }) expect(trigger).not_to be_valid end diff --git a/spec/models/ci/unit_test_spec.rb b/spec/models/ci/unit_test_spec.rb index b3180492a36..e35a4ce40da 100644 --- a/spec/models/ci/unit_test_spec.rb +++ b/spec/models/ci/unit_test_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Ci::UnitTest do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:project) } + let!(:parent) { create(:project, namespace: create(:group)) } let!(:model) { create(:ci_unit_test, project: parent) } end diff --git a/spec/models/clusters/applications/cert_manager_spec.rb b/spec/models/clusters/applications/cert_manager_spec.rb index 05ab8c4108e..427a99efadd 100644 --- a/spec/models/clusters/applications/cert_manager_spec.rb +++ b/spec/models/clusters/applications/cert_manager_spec.rb @@ -10,6 +10,11 @@ RSpec.describe Clusters::Applications::CertManager do include_examples 'cluster application version specs', :clusters_applications_cert_manager include_examples 'cluster application initial status specs' + describe 'default values' do + it { expect(cert_manager.version).to eq(described_class::VERSION) } + it { expect(cert_manager.email).to eq("admin@example.com") } + end + describe '#can_uninstall?' do subject { cert_manager.can_uninstall? } diff --git a/spec/models/clusters/applications/crossplane_spec.rb b/spec/models/clusters/applications/crossplane_spec.rb index 7082576028b..d1abaa52c7f 100644 --- a/spec/models/clusters/applications/crossplane_spec.rb +++ b/spec/models/clusters/applications/crossplane_spec.rb @@ -14,6 +14,11 @@ RSpec.describe Clusters::Applications::Crossplane do it { is_expected.to validate_presence_of(:stack) } end + describe 'default values' do + it { expect(subject.version).to eq(described_class::VERSION) } + it { expect(subject.stack).to be_empty } + end + describe '#can_uninstall?' do subject { crossplane.can_uninstall? } diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index 5212e321a55..1b8be92475a 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' RSpec.describe Clusters::Applications::Helm do include_examples 'cluster application core specs', :clusters_applications_helm + describe 'default values' do + it { expect(subject.version).to eq(Gitlab::Kubernetes::Helm::V2::BaseCommand::HELM_VERSION) } + end + describe '.available' do subject { described_class.available } diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index e16d97c42d9..e5caa11452e 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -18,6 +18,11 @@ RSpec.describe Clusters::Applications::Ingress do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) end + describe 'default values' do + it { expect(subject.ingress_type).to eq("nginx") } + it { expect(subject.version).to eq(described_class::VERSION) } + end + describe '#can_uninstall?' do subject { ingress.can_uninstall? } diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index e7de2d24334..9336d2352f8 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -10,6 +10,10 @@ RSpec.describe Clusters::Applications::Jupyter do it { is_expected.to belong_to(:oauth_application) } + describe 'default values' do + it { expect(subject.version).to eq(described_class::VERSION) } + end + describe '#can_uninstall?' do let(:ingress) { create(:clusters_applications_ingress, :installed, external_hostname: 'localhost.localdomain') } let(:jupyter) { create(:clusters_applications_jupyter, cluster: ingress.cluster) } diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index d0e470bfa42..3914450339a 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -21,6 +21,10 @@ RSpec.describe Clusters::Applications::Knative do it { is_expected.to have_one(:serverless_domain_cluster).class_name('::Serverless::DomainCluster').with_foreign_key('clusters_applications_knative_id').inverse_of(:knative) } end + describe 'default values' do + it { expect(subject.version).to eq(described_class::VERSION) } + end + describe 'when cloud run is enabled' do let(:cluster) { create(:cluster, :provided_by_gcp, :cloud_run_enabled) } let(:knative_cloud_run) { create(:clusters_applications_knative, cluster: cluster) } diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 549a273e2d7..15c3162270e 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -12,6 +12,13 @@ RSpec.describe Clusters::Applications::Prometheus do include_examples 'cluster application helm specs', :clusters_applications_prometheus include_examples 'cluster application initial status specs' + describe 'default values' do + subject(:prometheus) { build(:clusters_applications_prometheus) } + + it { expect(prometheus.alert_manager_token).to be_an_instance_of(String) } + it { expect(prometheus.version).to eq(described_class::VERSION) } + end + describe 'after_destroy' do let(:cluster) { create(:cluster, :with_installed_helm) } let(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } @@ -130,7 +137,7 @@ RSpec.describe Clusters::Applications::Prometheus do end context 'with knative installed' do - let(:knative) { create(:clusters_applications_knative, :updated ) } + let(:knative) { create(:clusters_applications_knative, :updated) } let(:prometheus) { create(:clusters_applications_prometheus, cluster: knative.cluster) } subject { prometheus.install_command } @@ -161,7 +168,7 @@ RSpec.describe Clusters::Applications::Prometheus do end describe '#predelete' do - let(:knative) { create(:clusters_applications_knative, :updated ) } + let(:knative) { create(:clusters_applications_knative, :updated) } let(:prometheus) { create(:clusters_applications_prometheus, cluster: knative.cluster) } subject { prometheus.uninstall_command.predelete } diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 8f02161843b..04b5ae9641d 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -13,6 +13,10 @@ RSpec.describe Clusters::Applications::Runner do it { is_expected.to belong_to(:runner) } + describe 'default values' do + it { expect(subject.version).to eq(described_class::VERSION) } + end + describe '#can_uninstall?' do let(:gitlab_runner) { create(:clusters_applications_runner, runner: ci_runner) } diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 73cd7bb9075..be64d72e031 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -49,6 +49,10 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to respond_to :project } it { is_expected.to be_namespace_per_environment } + describe 'default values' do + it { expect(subject.helm_major_version).to eq(3) } + end + it_behaves_like 'it has loose foreign keys' do let(:factory_name) { :cluster } end diff --git a/spec/models/clusters/integrations/prometheus_spec.rb b/spec/models/clusters/integrations/prometheus_spec.rb index 90e99aefdce..d6d1105cdb1 100644 --- a/spec/models/clusters/integrations/prometheus_spec.rb +++ b/spec/models/clusters/integrations/prometheus_spec.rb @@ -15,6 +15,16 @@ RSpec.describe Clusters::Integrations::Prometheus do it { is_expected.not_to allow_value(nil).for(:enabled) } end + describe 'default values' do + subject(:integration) { build(:clusters_integrations_prometheus) } + + before do + allow(SecureRandom).to receive(:hex).and_return('randomtoken') + end + + it { expect(integration.alert_manager_token).to eq('randomtoken') } + end + describe 'after_destroy' do subject(:integration) { create(:clusters_integrations_prometheus, cluster: cluster, enabled: true) } diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index 4ac2fd022ba..b280275c2e5 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -21,6 +21,12 @@ RSpec.describe Clusters::Platforms::Kubernetes do it_behaves_like 'having unique enum values' + describe 'default values' do + let(:kubernetes) { create(:cluster_platform_kubernetes) } + + it { expect(kubernetes.authorization_type).to eq("rbac") } + end + describe 'before_validation' do let(:kubernetes) { create(:cluster_platform_kubernetes, :configured, namespace: namespace) } @@ -427,6 +433,55 @@ RSpec.describe Clusters::Platforms::Kubernetes do let(:cluster) { create(:cluster, :project, platform_kubernetes: service) } include_examples 'successful deployment request' + + context 'when reading ingress raises NoMethodError' do + before do + allow_next_instance_of(Gitlab::Kubernetes::KubeClient) do |kube_client| + allow(kube_client).to receive(:get_pods).with(namespace: namespace).and_return([]) + allow(kube_client).to receive(:get_deployments).with(namespace: namespace).and_return([]) + allow(kube_client).to receive(:get_ingresses).with(namespace: namespace).and_raise(NoMethodError) + end + end + + context 'when version request succeeds' do + before do + stub_server_min_version(min_server_version) + end + + context 'when server min version is < 23' do + let(:min_server_version) { "18" } + + it 'does not raise error', :unlimited_max_formatted_output_length do + expect { subject }.not_to raise_error + end + + it 'returns empty array for the K8s component keys' do + expect(subject).to include({ pods: [], deployments: [], ingresses: [] }) + end + end + + context 'when server min version is >= 23' do + let(:min_server_version) { "23" } + + it 'does raise error' do + expect { subject }.to raise_error(NoMethodError) + end + end + end + + context 'when the version request fails' do + before do + stub_server_min_version_failed_request + end + + it "tracks error and returns empty arrays" do + expect(Gitlab::ErrorTracking) + .to receive(:track_exception).with(kind_of(Clusters::Platforms::Kubernetes::FailedVersionCheckError)) + + expect(subject).to include({ pods: [], deployments: [], ingresses: [] }) + end + end + end end context 'on a group level cluster' do @@ -452,7 +507,7 @@ RSpec.describe Clusters::Platforms::Kubernetes do context 'when there are ignored K8s connections errors' do described_class::IGNORED_CONNECTION_EXCEPTIONS.each do |exception| - context "#{exception}" do + context exception.to_s do before do exception_args = ['arg1'] exception_args.push('arg2', 'arg3') if exception.name == 'Kubeclient::HttpError' diff --git a/spec/models/clusters/providers/aws_spec.rb b/spec/models/clusters/providers/aws_spec.rb index 3b4a48cc5be..2afed663edf 100644 --- a/spec/models/clusters/providers/aws_spec.rb +++ b/spec/models/clusters/providers/aws_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Clusters::Providers::Aws do include_examples 'provider status', :cluster_provider_aws - describe 'default_value_for' do + describe 'default values' do let(:provider) { build(:cluster_provider_aws) } it "sets default values" do diff --git a/spec/models/clusters/providers/gcp_spec.rb b/spec/models/clusters/providers/gcp_spec.rb index ad9ada04875..a1f00069937 100644 --- a/spec/models/clusters/providers/gcp_spec.rb +++ b/spec/models/clusters/providers/gcp_spec.rb @@ -8,13 +8,14 @@ RSpec.describe Clusters::Providers::Gcp do include_examples 'provider status', :cluster_provider_gcp - describe 'default_value_for' do + describe 'default values' do let(:gcp) { build(:cluster_provider_gcp) } it "has default value" do expect(gcp.zone).to eq('us-central1-a') expect(gcp.num_nodes).to eq(3) expect(gcp.machine_type).to eq('n1-standard-2') + expect(gcp.cloud_run).to eq(false) end end diff --git a/spec/models/commit_signatures/gpg_signature_spec.rb b/spec/models/commit_signatures/gpg_signature_spec.rb index 605ad725dd7..1ffaaeba396 100644 --- a/spec/models/commit_signatures/gpg_signature_spec.rb +++ b/spec/models/commit_signatures/gpg_signature_spec.rb @@ -85,4 +85,10 @@ RSpec.describe CommitSignatures::GpgSignature do end end end + + describe '#user' do + it 'retrieves the gpg_key user' do + expect(signature.user).to eq(gpg_key.user) + end + end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index adbd20b6730..704203ed29c 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -32,6 +32,7 @@ RSpec.describe CommitStatus do it { is_expected.to respond_to :failed? } it { is_expected.to respond_to :running? } it { is_expected.to respond_to :pending? } + it { is_expected.not_to be_retried } describe '#author' do subject { commit_status.author } diff --git a/spec/models/concerns/bulk_insert_safe_spec.rb b/spec/models/concerns/bulk_insert_safe_spec.rb index 569dc3a3a3e..577004c2cf6 100644 --- a/spec/models/concerns/bulk_insert_safe_spec.rb +++ b/spec/models/concerns/bulk_insert_safe_spec.rb @@ -73,9 +73,9 @@ RSpec.describe BulkInsertSafe do key: Settings.attr_encrypted_db_key_base_32, insecure_mode: false - default_value_for :enum_value, 'case_1' - default_value_for :sha_value, '2fd4e1c67a2d28fced849ee1bb76e7391b93eb12' - default_value_for :jsonb_value, { "key" => "value" } + attribute :enum_value, default: 'case_1' + attribute :sha_value, default: '2fd4e1c67a2d28fced849ee1bb76e7391b93eb12' + attribute :jsonb_value, default: -> { { "key" => "value" } } def self.name 'BulkInsertItem' diff --git a/spec/models/concerns/ci/has_variable_spec.rb b/spec/models/concerns/ci/has_variable_spec.rb index bf699119a37..861d8f3b974 100644 --- a/spec/models/concerns/ci/has_variable_spec.rb +++ b/spec/models/concerns/ci/has_variable_spec.rb @@ -84,6 +84,7 @@ RSpec.describe Ci::HasVariable do key: subject.key, value: subject.value, public: false, + raw: false, masked: false } end diff --git a/spec/models/concerns/ci/partitionable/switch_spec.rb b/spec/models/concerns/ci/partitionable/switch_spec.rb new file mode 100644 index 00000000000..d955ad223f8 --- /dev/null +++ b/spec/models/concerns/ci/partitionable/switch_spec.rb @@ -0,0 +1,316 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::Partitionable::Switch, :aggregate_failures do + let(:model) do + Class.new(Ci::ApplicationRecord) do + self.primary_key = :id + self.table_name = :_test_ci_jobs_metadata + self.sequence_name = :_test_ci_jobs_metadata_id_seq + + def self.name + 'TestSwitchJobMetadata' + end + end + end + + let(:table_rollout_flag) { :ci_partitioning_use_test_routing_table } + + let(:partitioned_model) { model::Partitioned } + + let(:jobs_model) do + Class.new(Ci::ApplicationRecord) do + self.primary_key = :id + self.table_name = :_test_ci_jobs + + def self.name + 'TestSwitchJob' + end + end + 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, + job_id int, + partition_id int NOT NULL DEFAULT 1, + expanded_environment_name text); + + CREATE TABLE _test_p_ci_jobs_metadata ( + LIKE _test_ci_jobs_metadata INCLUDING DEFAULTS + ) PARTITION BY LIST(partition_id); + + ALTER TABLE _test_p_ci_jobs_metadata + ADD CONSTRAINT _test_p_ci_jobs_metadata_id_partition_id + UNIQUE (id, partition_id); + + ALTER TABLE _test_p_ci_jobs_metadata + ATTACH PARTITION _test_ci_jobs_metadata FOR VALUES IN (1); + + CREATE TABLE _test_ci_jobs(id serial NOT NULL PRIMARY KEY); + SQL + + stub_const('Ci::Partitionable::Testing::PARTITIONABLE_MODELS', [model.name]) + + model.include(Ci::Partitionable) + + model.partitionable scope: ->(r) { 1 }, + through: { table: :_test_p_ci_jobs_metadata, flag: table_rollout_flag } + + model.belongs_to :job, anonymous_class: jobs_model + + jobs_model.has_one :metadata, anonymous_class: model, + foreign_key: :job_id, inverse_of: :job, + dependent: :destroy + + allow(Feature::Definition).to receive(:get).and_call_original + allow(Feature::Definition).to receive(:get).with(table_rollout_flag) + .and_return( + Feature::Definition.new("development/#{table_rollout_flag}.yml", + { type: 'development', name: table_rollout_flag } + ) + ) + end + + it { expect(model).not_to be_routing_class } + + it { expect(partitioned_model).to be_routing_class } + + it { expect(partitioned_model.table_name).to eq('_test_p_ci_jobs_metadata') } + + it { expect(partitioned_model.quoted_table_name).to eq('"_test_p_ci_jobs_metadata"') } + + it { expect(partitioned_model.arel_table.name).to eq('_test_p_ci_jobs_metadata') } + + it { expect(partitioned_model.sequence_name).to eq('_test_ci_jobs_metadata_id_seq') } + + context 'when switching the tables' do + before do + stub_feature_flags(table_rollout_flag => false) + end + + %i[table_name quoted_table_name arel_table predicate_builder].each do |name| + it "switches #{name} to routing table and rollbacks" do + old_value = model.public_send(name) + routing_value = partitioned_model.public_send(name) + + expect(old_value).not_to eq(routing_value) + + expect { stub_feature_flags(table_rollout_flag => true) } + .to change(model, name).from(old_value).to(routing_value) + + expect { stub_feature_flags(table_rollout_flag => false) } + .to change(model, name).from(routing_value).to(old_value) + end + end + + it 'can switch aggregate methods' do + rollout_and_rollback_flag( + -> { expect(sql { model.count }).to all match(/FROM "_test_ci_jobs_metadata"/) }, + -> { expect(sql { model.count }).to all match(/FROM "_test_p_ci_jobs_metadata"/) } + ) + end + + it 'can switch reads' do + rollout_and_rollback_flag( + -> { expect(sql { model.last }).to all match(/FROM "_test_ci_jobs_metadata"/) }, + -> { expect(sql { model.last }).to all match(/FROM "_test_p_ci_jobs_metadata"/) } + ) + end + + it 'can switch inserts' do + rollout_and_rollback_flag( + -> { + expect(sql(filter: /INSERT/) { model.create! }) + .to all match(/INSERT INTO "_test_ci_jobs_metadata"/) + }, + -> { + expect(sql(filter: /INSERT/) { model.create! }) + .to all match(/INSERT INTO "_test_p_ci_jobs_metadata"/) + } + ) + end + + it 'can switch deletes' do + 3.times { model.create! } + + rollout_and_rollback_flag( + -> { + expect(sql(filter: /DELETE/) { model.last.destroy! }) + .to all match(/DELETE FROM "_test_ci_jobs_metadata"/) + }, + -> { + expect(sql(filter: /DELETE/) { model.last.destroy! }) + .to all match(/DELETE FROM "_test_p_ci_jobs_metadata"/) + } + ) + end + + context 'with associations' do + let(:job) { jobs_model.create! } + + it 'reads' do + model.create!(job_id: job.id) + + rollout_and_rollback_flag( + -> { + expect(sql(filter: /jobs_metadata/) { jobs_model.find(job.id).metadata }) + .to all match(/FROM "_test_ci_jobs_metadata"/) + }, + -> { + expect(sql(filter: /jobs_metadata/) { jobs_model.find(job.id).metadata }) + .to all match(/FROM "_test_p_ci_jobs_metadata"/) + } + ) + end + + it 'writes' do + rollout_and_rollback_flag( + -> { + expect(sql(filter: /INSERT .* jobs_metadata/) { jobs_model.find(job.id).create_metadata! }) + .to all match(/INSERT INTO "_test_ci_jobs_metadata"/) + }, + -> { + expect(sql(filter: /INSERT .* jobs_metadata/) { jobs_model.find(job.id).create_metadata! }) + .to all match(/INSERT INTO "_test_p_ci_jobs_metadata"/) + } + ) + end + + it 'deletes' do + 3.times do + job = jobs_model.create! + job.create_metadata! + end + + rollout_and_rollback_flag( + -> { + expect(sql(filter: /DELETE .* jobs_metadata/) { jobs_model.last.destroy! }) + .to all match(/DELETE FROM "_test_ci_jobs_metadata"/) + }, + -> { + expect(sql(filter: /DELETE .* jobs_metadata/) { jobs_model.last.destroy! }) + .to all match(/DELETE FROM "_test_p_ci_jobs_metadata"/) + } + ) + end + + it 'can switch joins from jobs' do + rollout_and_rollback_flag( + -> { + expect(sql { jobs_model.joins(:metadata).last }) + .to all match(/INNER JOIN "_test_ci_jobs_metadata"/) + }, + -> { + expect(sql { jobs_model.joins(:metadata).last }) + .to all match(/INNER JOIN "_test_p_ci_jobs_metadata"/) + } + ) + end + + it 'can switch joins from metadata' do + rollout_and_rollback_flag( + -> { + expect(sql { model.joins(:job).last }) + .to all match(/FROM "_test_ci_jobs_metadata" INNER JOIN "_test_ci_jobs"/) + }, + -> { + expect(sql { model.joins(:job).last }) + .to all match(/FROM "_test_p_ci_jobs_metadata" INNER JOIN "_test_ci_jobs"/) + } + ) + end + + it 'preloads' do + job = jobs_model.create! + job.create_metadata! + + rollout_and_rollback_flag( + -> { + expect(sql(filter: /jobs_metadata/) { jobs_model.preload(:metadata).last }) + .to all match(/FROM "_test_ci_jobs_metadata"/) + }, + -> { + expect(sql(filter: /jobs_metadata/) { jobs_model.preload(:metadata).last }) + .to all match(/FROM "_test_p_ci_jobs_metadata"/) + } + ) + end + + context 'with nested attributes' do + before do + jobs_model.accepts_nested_attributes_for :metadata + end + + it 'writes' do + attrs = { metadata_attributes: { expanded_environment_name: 'test_env_name' } } + + rollout_and_rollback_flag( + -> { + expect(sql(filter: /INSERT .* jobs_metadata/) { jobs_model.create!(attrs) }) + .to all match(/INSERT INTO "_test_ci_jobs_metadata" .* 'test_env_name'/) + }, + -> { + expect(sql(filter: /INSERT .* jobs_metadata/) { jobs_model.create!(attrs) }) + .to all match(/INSERT INTO "_test_p_ci_jobs_metadata" .* 'test_env_name'/) + } + ) + end + end + end + end + + context 'with safe request store', :request_store do + it 'changing the flag to true does not affect the current request' do + stub_feature_flags(table_rollout_flag => false) + + expect(model.table_name).to eq('_test_ci_jobs_metadata') + + stub_feature_flags(table_rollout_flag => true) + + expect(model.table_name).to eq('_test_ci_jobs_metadata') + end + + it 'changing the flag to false does not affect the current request' do + stub_feature_flags(table_rollout_flag => true) + + expect(model.table_name).to eq('_test_p_ci_jobs_metadata') + + stub_feature_flags(table_rollout_flag => false) + + expect(model.table_name).to eq('_test_p_ci_jobs_metadata') + end + end + + def rollout_and_rollback_flag(old, new) + # Load class and SQL statements cache + old.call + + stub_feature_flags(table_rollout_flag => true) + + # Test switch + new.call + + stub_feature_flags(table_rollout_flag => false) + + # Test that it can switch back in the same process + old.call + end + + def create_tables(table_sql) + Ci::ApplicationRecord.connection.execute(table_sql) + end + + def sql(filter: nil, &block) + result = ActiveRecord::QueryRecorder.new(&block) + result = result.log + + return result unless filter + + result.select { |statement| statement.match?(filter) } + end +end diff --git a/spec/models/concerns/ci/partitionable_spec.rb b/spec/models/concerns/ci/partitionable_spec.rb index d53501ccc3d..f3d33c971c7 100644 --- a/spec/models/concerns/ci/partitionable_spec.rb +++ b/spec/models/concerns/ci/partitionable_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Ci::Partitionable do - describe 'partitionable models inclusion' do - let(:ci_model) { Class.new(Ci::ApplicationRecord) } + let(:ci_model) { Class.new(Ci::ApplicationRecord) } + describe 'partitionable models inclusion' do subject { ci_model.include(described_class) } it 'raises an exception' do @@ -23,4 +23,21 @@ RSpec.describe Ci::Partitionable do end end end + + context 'with through options' do + before do + allow(ActiveSupport::DescendantsTracker).to receive(:store_inherited) + stub_const("#{described_class}::Testing::PARTITIONABLE_MODELS", [ci_model.name]) + + ci_model.include(described_class) + ci_model.partitionable scope: ->(r) { 1 }, + through: { table: :_test_table_name, flag: :some_flag } + end + + it { expect(ci_model.routing_table_name).to eq(:_test_table_name) } + + it { expect(ci_model.routing_table_name_flag).to eq(:some_flag) } + + it { expect(ci_model.ancestors).to include(described_class::Switch) } + end end diff --git a/spec/models/concerns/encrypted_user_password_spec.rb b/spec/models/concerns/encrypted_user_password_spec.rb new file mode 100644 index 00000000000..b6447313967 --- /dev/null +++ b/spec/models/concerns/encrypted_user_password_spec.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe User do + describe '#authenticatable_salt' do + let(:user) { build(:user, encrypted_password: encrypted_password) } + + subject(:authenticatable_salt) { user.authenticatable_salt } + + context 'when password is stored in BCrypt format' do + let(:encrypted_password) { '$2a$10$AvwDCyF/8HnlAv./UkAZx.vAlKRS89yNElP38FzdgOmVaSaiDL7xm' } + + it 'returns the first 30 characters of the encrypted_password' do + expect(authenticatable_salt).to eq(user.encrypted_password[0, 29]) + end + end + + context 'when password is stored in PBKDF2 format' do + let(:encrypted_password) { '$pbkdf2-sha512$20000$rKbYsScsDdk$iwWBewXmrkD2fFfaG1SDcMIvl9gvEo3fBWUAfiqyVceTlw/DYgKBByHzf45pF5Qn59R4R.NQHsFpvZB4qlsYmw' } # rubocop:disable Layout/LineLength + + it 'uses the decoded password salt' do + expect(authenticatable_salt).to eq('aca6d8b1272c0dd9') + end + + it 'does not use the first 30 characters of the encrypted_password' do + expect(authenticatable_salt).not_to eq(encrypted_password[0, 29]) + end + end + + context 'when the encrypted_password is an unknown type' do + let(:encrypted_password) { '$argon2i$v=19$m=512,t=4,p=2$eM+ZMyYkpDRGaI3xXmuNcQ$c5DeJg3eb5dskVt1mDdxfw' } + + it 'returns the first 30 characters of the encrypted_password' do + expect(authenticatable_salt).to eq(encrypted_password[0, 29]) + end + end + end + + describe '#valid_password?' do + subject(:validate_password) { user.valid_password?(password) } + + let(:user) { build(:user, encrypted_password: encrypted_password) } + let(:password) { described_class.random_password } + + shared_examples 'password validation fails when the password is encrypted using an unsupported method' do + let(:encrypted_password) { '$argon2i$v=19$m=512,t=4,p=2$eM+ZMyYkpDRGaI3xXmuNcQ$c5DeJg3eb5dskVt1mDdxfw' } + + it { is_expected.to eq(false) } + end + + context 'when the default encryption method is BCrypt' do + it_behaves_like 'password validation fails when the password is encrypted using an unsupported method' + + context 'when the user password PBKDF2+SHA512' do + let(:encrypted_password) do + Devise::Pbkdf2Encryptable::Encryptors::Pbkdf2Sha512.digest( + password, 20_000, Devise.friendly_token[0, 16]) + end + + it { is_expected.to eq(true) } + + it 're-encrypts the password as BCrypt' do + expect(user.encrypted_password).to start_with('$pbkdf2-sha512$') + + validate_password + + expect(user.encrypted_password).to start_with('$2a$') + end + end + end + + context 'when the default encryption method is PBKDF2+SHA512 and the user password is BCrypt', :fips_mode do + it_behaves_like 'password validation fails when the password is encrypted using an unsupported method' + + context 'when the user password BCrypt' do + let(:encrypted_password) { Devise::Encryptor.digest(described_class, password) } + + it { is_expected.to eq(true) } + + it 're-encrypts the password as PBKDF2+SHA512' do + expect(user.encrypted_password).to start_with('$2a$') + + validate_password + + expect(user.reload.encrypted_password).to start_with('$pbkdf2-sha512$') + end + end + end + end + + describe '#password=' do + let(:user) { build(:user) } + let(:password) { described_class.random_password } + + def compare_bcrypt_password(user, password) + Devise::Encryptor.compare(described_class, user.encrypted_password, password) + end + + def compare_pbkdf2_password(user, password) + Devise::Pbkdf2Encryptable::Encryptors::Pbkdf2Sha512.compare(user.encrypted_password, password) + end + + context 'when FIPS mode is enabled', :fips_mode do + it 'calls PBKDF2 digest and not the default Devise encryptor' do + expect(Devise::Pbkdf2Encryptable::Encryptors::Pbkdf2Sha512) + .to receive(:digest).at_least(:once).and_call_original + expect(Devise::Encryptor).not_to receive(:digest) + + user.password = password + end + + it 'saves the password in PBKDF2 format' do + user.password = password + user.save! + + expect(compare_pbkdf2_password(user, password)).to eq(true) + expect { compare_bcrypt_password(user, password) }.to raise_error(::BCrypt::Errors::InvalidHash) + end + end + + it 'calls default Devise encryptor and not the PBKDF2 encryptor' do + expect(Devise::Encryptor).to receive(:digest).at_least(:once).and_call_original + expect(Devise::Pbkdf2Encryptable::Encryptors::Pbkdf2Sha512).not_to receive(:digest) + + user.password = password + end + + it 'saves the password in BCrypt format' do + user.password = password + user.save! + + expect { compare_pbkdf2_password(user, password) } + .to raise_error Devise::Pbkdf2Encryptable::Encryptors::InvalidHash + expect(compare_bcrypt_password(user, password)).to eq(true) + end + end +end diff --git a/spec/models/concerns/file_store_mounter_spec.rb b/spec/models/concerns/file_store_mounter_spec.rb new file mode 100644 index 00000000000..459f3d35668 --- /dev/null +++ b/spec/models/concerns/file_store_mounter_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe FileStoreMounter, :aggregate_failures do + let(:uploader_class) do + Class.new do + def object_store + :object_store + end + end + end + + let(:test_class) { Class.new { include(FileStoreMounter) } } + + let(:uploader_instance) { uploader_class.new } + + describe '.mount_file_store_uploader' do + using RSpec::Parameterized::TableSyntax + + subject(:mount_file_store_uploader) do + test_class.mount_file_store_uploader uploader_class, skip_store_file: skip_store_file, file_field: file_field + end + + where(:skip_store_file, :file_field) do + true | :file + false | :file + false | :signed_file + true | :signed_file + end + + with_them do + it 'defines instance methods and registers a callback' do + expect(test_class).to receive(:mount_uploader).with(file_field, uploader_class) + expect(test_class).to receive(:define_method).with("update_#{file_field}_store") + expect(test_class).to receive(:define_method).with("store_#{file_field}_now!") + + if skip_store_file + expect(test_class).to receive(:skip_callback).with(:save, :after, "store_#{file_field}!".to_sym) + expect(test_class).not_to receive(:after_save) + else + expect(test_class).not_to receive(:skip_callback) + expect(test_class) + .to receive(:after_save) + .with("update_#{file_field}_store".to_sym, if: "saved_change_to_#{file_field}?".to_sym) + end + + mount_file_store_uploader + end + end + + context 'with an unknown file_field' do + let(:skip_store_file) { false } + let(:file_field) { 'unknown' } + + it do + expect { mount_file_store_uploader }.to raise_error(ArgumentError, 'file_field not allowed: unknown') + end + end + end + + context 'with an instance' do + let(:instance) { test_class.new } + + before do + allow(test_class).to receive(:mount_uploader) + allow(test_class).to receive(:after_save) + test_class.mount_file_store_uploader uploader_class + end + + describe '#update_file_store' do + subject(:update_file_store) { instance.update_file_store } + + it 'calls update column' do + expect(instance).to receive(:file).and_return(uploader_instance) + expect(instance).to receive(:update_column).with('file_store', :object_store) + + update_file_store + end + end + + describe '#store_file_now!' do + subject(:store_file_now!) { instance.store_file_now! } + + it 'calls the dynamic functions' do + expect(instance).to receive(:store_file!) + expect(instance).to receive(:update_file_store) + + store_file_now! + end + end + end +end diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index a6a0e074589..b2ea7b22dea 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -88,5 +88,47 @@ RSpec.describe User do end end end + + describe '#redacted_name(viewing_user)' do + let_it_be(:viewing_user) { human } + + subject { observed_user.redacted_name(viewing_user) } + + context 'when user is not a project bot' do + let(:observed_user) { support_bot } + + it { is_expected.to eq(support_bot.name) } + end + + context 'when user is a project_bot' do + let(:observed_user) { project_bot } + + context 'when groups are present and user can :read_group' do + let_it_be(:group) { create(:group) } + + before do + group.add_developer(observed_user) + group.add_developer(viewing_user) + end + + it { is_expected.to eq(observed_user.name) } + end + + context 'when user can :read_project' do + let_it_be(:project) { create(:project) } + + before do + project.add_developer(observed_user) + project.add_developer(viewing_user) + end + + it { is_expected.to eq(observed_user.name) } + end + + context 'when requester does not have permissions to read project_bot name' do + it { is_expected.to eq('****') } + end + end + end end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 8842a36f40a..e553e34ab51 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -337,31 +337,6 @@ RSpec.describe Issuable do it { expect(MergeRequest.to_ability_name).to eq("merge_request") } end - describe "#today?" do - it "returns true when created today" do - # Avoid timezone differences and just return exactly what we want - allow(Date).to receive(:today).and_return(issue.created_at.to_date) - expect(issue.today?).to be_truthy - end - - it "returns false when not created today" do - allow(Date).to receive(:today).and_return(Date.yesterday) - expect(issue.today?).to be_falsey - end - end - - describe "#new?" do - it "returns false when created 30 hours ago" do - allow(issue).to receive(:created_at).and_return(Time.current - 30.hours) - expect(issue.new?).to be_falsey - end - - it "returns true when created 20 hours ago" do - allow(issue).to receive(:created_at).and_return(Time.current - 20.hours) - expect(issue.new?).to be_truthy - end - end - describe "#sort_by_attribute" do let(:project) { create(:project) } @@ -1055,6 +1030,22 @@ RSpec.describe Issuable do end end + describe '#supports_confidentiality?' do + where(:issuable_type, :supports_confidentiality) do + :issue | true + :incident | true + :merge_request | false + end + + with_them do + let(:issuable) { build_stubbed(issuable_type) } + + subject { issuable.supports_confidentiality? } + + it { is_expected.to eq(supports_confidentiality) } + end + end + describe '#severity' do subject { issuable.severity } diff --git a/spec/models/concerns/pg_full_text_searchable_spec.rb b/spec/models/concerns/pg_full_text_searchable_spec.rb index 3e42a3504ac..98b44a2eec2 100644 --- a/spec/models/concerns/pg_full_text_searchable_spec.rb +++ b/spec/models/concerns/pg_full_text_searchable_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe PgFullTextSearchable do - let(:project) { create(:project) } + let(:project) { build(:project) } let(:model_class) do Class.new(ActiveRecord::Base) do @@ -76,7 +76,7 @@ RSpec.describe PgFullTextSearchable do end describe '.pg_full_text_search' do - let(:english) { model_class.create!(project: project, title: 'title', description: 'something english') } + let(:english) { model_class.create!(project: project, title: 'title', description: 'something description english') } let(:with_accent) { model_class.create!(project: project, title: 'Jürgen', description: 'Ærøskøbing') } let(:japanese) { model_class.create!(project: project, title: '日本語 title', description: 'another english description') } @@ -90,8 +90,19 @@ RSpec.describe PgFullTextSearchable do expect(model_class.pg_full_text_search('title english')).to contain_exactly(english, japanese) end + it 'searches specified columns only' do + matching_object = model_class.create!(project: project, 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) + end + + it 'uses prefix matching' do + expect(model_class.pg_full_text_search('tit eng')).to contain_exactly(english, japanese) + end + it 'searches for exact term with quotes' do - expect(model_class.pg_full_text_search('"something english"')).to contain_exactly(english) + expect(model_class.pg_full_text_search('"description english"')).to contain_exactly(english) end it 'ignores accents' do @@ -113,6 +124,27 @@ RSpec.describe PgFullTextSearchable do expect(model_class.pg_full_text_search('gopher://gitlab.com/gitlab-org/gitlab')).to contain_exactly(with_url) end end + + context 'when search term is a path with underscores' do + let(:path) { 'browser_ui/5_package/package_registry/maven/maven_group_level_spec.rb' } + let(:with_underscore) { model_class.create!(project: project, title: 'issue with path', description: "some #{path} other text") } + + it 'allows searching by the path' do + with_underscore.update_search_data! + + expect(model_class.pg_full_text_search(path)).to contain_exactly(with_underscore) + end + end + + context 'when text has numbers preceded by a dash' do + let(:with_dash) { model_class.create!(project: project, title: 'issue with dash', description: 'ABC-123') } + + it 'allows searching by numbers only' do + with_dash.update_search_data! + + expect(model_class.pg_full_text_search('123')).to contain_exactly(with_dash) + end + end end describe '#update_search_data!' do diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb index 89f34834aa4..f168bedc8eb 100644 --- a/spec/models/concerns/project_features_compatibility_spec.rb +++ b/spec/models/concerns/project_features_compatibility_spec.rb @@ -8,7 +8,7 @@ RSpec.describe ProjectFeaturesCompatibility do let(:features) do features_enabled + %w( repository pages operations container_registry package_registry environments feature_flags releases - monitor + monitor infrastructure ) end diff --git a/spec/models/concerns/sha_attribute_spec.rb b/spec/models/concerns/sha_attribute_spec.rb index 790e6936803..fca94b50fee 100644 --- a/spec/models/concerns/sha_attribute_spec.rb +++ b/spec/models/concerns/sha_attribute_spec.rb @@ -72,9 +72,10 @@ RSpec.describe ShaAttribute do end it 'validates column type' do - if expected_error == :no_error + case expected_error + when :no_error expect { load_schema! }.not_to raise_error - elsif expected_error == :sha_mismatch_error + when :sha_mismatch_error expect { load_schema! }.to raise_error( described_class::ShaAttributeTypeMismatchError, /sha_attribute.*#{column_name}.* should be a :binary column/ @@ -89,9 +90,10 @@ RSpec.describe ShaAttribute do end it 'validates column type' do - if expected_error == :no_error + case expected_error + when :no_error expect { load_schema! }.not_to raise_error - elsif expected_error == :sha_mismatch_error + when :sha_mismatch_error expect { load_schema! }.to raise_error( described_class::Sha256AttributeTypeMismatchError, /sha256_attribute.*#{column_name}.* should be a :binary column/ diff --git a/spec/models/concerns/subquery_spec.rb b/spec/models/concerns/subquery_spec.rb new file mode 100644 index 00000000000..95487fd8c2d --- /dev/null +++ b/spec/models/concerns/subquery_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Subquery do + let_it_be(:projects) { create_list :project, 3 } + let_it_be(:project_ids) { projects.map(&:id) } + let(:relation) { Project.where(id: projects) } + + subject { relation.subquery(:id) } + + shared_examples 'subquery as array values' do + specify { is_expected.to match_array project_ids } + specify { expect { subject }.not_to make_queries } + end + + shared_examples 'subquery as relation' do + it { is_expected.to be_a ActiveRecord::Relation } + specify { expect { subject.load }.to make_queries } + end + + shared_context 'when array size exceeds max_limit' do + subject { relation.subquery(:id, max_limit: 1) } + end + + context 'when relation is not loaded' do + it_behaves_like 'subquery as relation' + + context 'when array size exceeds max_limit' do + include_context 'when array size exceeds max_limit' + + it_behaves_like 'subquery as relation' + end + end + + context 'when relation is loaded' do + before do + relation.load + end + + it_behaves_like 'subquery as array values' + + context 'when array size exceeds max_limit' do + include_context 'when array size exceeds max_limit' + + it_behaves_like 'subquery as relation' + end + + context 'with a select' do + let(:relation) { Project.where(id: projects).select(:id) } + + it_behaves_like 'subquery as array values' + + context 'and querying with an unloaded column' do + subject { relation.subquery(:namespace_id) } + + it { expect { subject }.to raise_error(ActiveModel::MissingAttributeError) } + end + end + end +end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index e8db83b7144..e53fdafe3b1 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -214,19 +214,15 @@ end RSpec.describe Ci::Build, 'TokenAuthenticatable' do let(:token_field) { :token } - let(:build) { FactoryBot.build(:ci_build) } + let(:build) { FactoryBot.build(:ci_build, :created) } it_behaves_like 'TokenAuthenticatable' describe 'generating new token' do context 'token is not generated yet' do describe 'token field accessor' do - it 'makes it possible to access token' do - expect(build.token).to be_nil - - build.save! - - expect(build.token).to be_present + it 'does not generate a token when saving a build' do + expect { build.save! }.not_to change(build, :token).from(nil) end end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 0033e9bbd08..9af53bae204 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -871,6 +871,28 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end + describe '#set_delete_ongoing_status', :freeze_time do + let_it_be(:repository) { create(:container_repository) } + + subject { repository.set_delete_ongoing_status } + + it 'updates deletion status attributes' do + expect { subject }.to change(repository, :status).from(nil).to('delete_ongoing') + .and change(repository, :delete_started_at).from(nil).to(Time.zone.now) + end + end + + describe '#set_delete_scheduled_status' do + let_it_be(:repository) { create(:container_repository, :status_delete_ongoing, delete_started_at: 3.minutes.ago) } + + subject { repository.set_delete_scheduled_status } + + it 'updates delete attributes' do + expect { subject }.to change(repository, :status).from('delete_ongoing').to('delete_scheduled') + .and change(repository, :delete_started_at).to(nil) + end + end + context 'registry migration' do before do allow(repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(true) @@ -1274,6 +1296,16 @@ RSpec.describe ContainerRepository, :aggregate_failures do it { is_expected.to contain_exactly(repository1, repository3) } end + describe '.with_stale_delete_at' do + let_it_be(:repository1) { create(:container_repository, delete_started_at: 1.day.ago) } + let_it_be(:repository2) { create(:container_repository, delete_started_at: 25.minutes.ago) } + let_it_be(:repository3) { create(:container_repository, delete_started_at: 1.week.ago) } + + subject { described_class.with_stale_delete_at(27.minutes.ago) } + + it { is_expected.to contain_exactly(repository1, repository3) } + end + describe '.waiting_for_cleanup' do let_it_be(:repository_cleanup_scheduled) { create(:container_repository, :cleanup_scheduled) } let_it_be(:repository_cleanup_unfinished) { create(:container_repository, :cleanup_unfinished) } diff --git a/spec/models/dependency_proxy/group_setting_spec.rb b/spec/models/dependency_proxy/group_setting_spec.rb index c4c4a877d50..4da1fe42ff2 100644 --- a/spec/models/dependency_proxy/group_setting_spec.rb +++ b/spec/models/dependency_proxy/group_setting_spec.rb @@ -7,6 +7,11 @@ RSpec.describe DependencyProxy::GroupSetting, type: :model do it { is_expected.to belong_to(:group) } end + describe 'default values' do + it { is_expected.to be_enabled } + it { expect(described_class.new(enabled: false)).not_to be_enabled } + end + describe 'validations' do it { is_expected.to validate_presence_of(:group) } end diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 635326eeadc..04763accc42 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -427,7 +427,7 @@ RSpec.describe DeployToken do end describe '.gitlab_deploy_token' do - let(:project) { create(:project ) } + let(:project) { create(:project) } subject { project.deploy_tokens.gitlab_deploy_token } diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index b91d836f82f..daa65f528e9 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -388,16 +388,31 @@ RSpec.describe Deployment do end context 'when deployment is behind current deployment' do + let_it_be(:commits) { project.repository.commits('master', limit: 2) } + let!(:deployment) do - create(:deployment, :success, project: project, environment: environment, finished_at: 1.year.ago) + create(:deployment, :success, project: project, environment: environment, + finished_at: 1.year.ago, sha: commits[0].sha) end let!(:last_deployment) do - create(:deployment, :success, project: project, environment: environment) + create(:deployment, :success, project: project, environment: environment, sha: commits[1].sha) end it { is_expected.to be_truthy } end + + context 'when deployment is the same sha as the current deployment' do + let!(:deployment) do + create(:deployment, :success, project: project, environment: environment, finished_at: 1.year.ago) + end + + let!(:last_deployment) do + create(:deployment, :success, project: project, environment: environment, sha: deployment.sha) + end + + it { is_expected.to be_falsey } + end end describe '#success?' do @@ -1323,9 +1338,12 @@ RSpec.describe Deployment do subject { deployment.tags } it 'will return tags related to this deployment' do - expect(project.repository).to receive(:tag_names_contains).with(deployment.sha, limit: 100).and_return(['test']) + expect(project.repository).to receive(:refs_by_oid).with(oid: deployment.sha, + limit: 100, + ref_patterns: [Gitlab::Git::TAG_REF_PREFIX]) + .and_return(["#{Gitlab::Git::TAG_REF_PREFIX}test"]) - is_expected.to match_array(['test']) + is_expected.to match_array(['refs/tags/test']) end end diff --git a/spec/models/diff_discussion_spec.rb b/spec/models/diff_discussion_spec.rb index 7a57f895b8a..fdfc4ec7cc4 100644 --- a/spec/models/diff_discussion_spec.rb +++ b/spec/models/diff_discussion_spec.rb @@ -128,7 +128,7 @@ RSpec.describe DiffDiscussion do end describe '#cache_key' do - let(:notes_sha) { Digest::SHA1.hexdigest("#{diff_note.post_processed_cache_key}") } + let(:notes_sha) { Digest::SHA1.hexdigest(diff_note.post_processed_cache_key.to_s) } let(:position_sha) { Digest::SHA1.hexdigest(diff_note.position.to_json) } it 'returns the cache key with the position sha' do diff --git a/spec/models/diff_viewer/server_side_spec.rb b/spec/models/diff_viewer/server_side_spec.rb index db0814af422..8990aa94b47 100644 --- a/spec/models/diff_viewer/server_side_spec.rb +++ b/spec/models/diff_viewer/server_side_spec.rb @@ -16,34 +16,6 @@ RSpec.describe DiffViewer::ServerSide do subject { viewer_class.new(diff_file) } - describe '#prepare!' do - before do - stub_feature_flags(disable_load_entire_blob_for_diff_viewer: feature_flag_enabled) - end - - context 'when the disable_load_entire_blob_for_diff_viewer flag is disabled' do - let(:feature_flag_enabled) { false } - - it 'loads all diff file data' do - subject - expect(diff_file).to receive_message_chain(:old_blob, :load_all_data!) - expect(diff_file).to receive_message_chain(:new_blob, :load_all_data!) - subject.prepare! - end - end - - context 'when the disable_load_entire_blob_for_diff_viewer flag is enabled' do - let(:feature_flag_enabled) { true } - - it 'does not load file data' do - subject - expect(diff_file).not_to receive(:old_blob) - expect(diff_file).not_to receive(:new_blob) - subject.prepare! - end - end - end - describe '#render_error' do context 'when the diff file is stored externally' do before do diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index a442856d993..8a3d43f58e0 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -884,8 +884,8 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do describe '#actions_for' do let(:deployment) { create(:deployment, :success, environment: environment) } let(:pipeline) { deployment.deployable.pipeline } - let!(:review_action) { create(:ci_build, :manual, name: 'review-apps', pipeline: pipeline, environment: 'review/$CI_COMMIT_REF_NAME' ) } - let!(:production_action) { create(:ci_build, :manual, name: 'production', pipeline: pipeline, environment: 'production' ) } + let!(:review_action) { create(:ci_build, :manual, name: 'review-apps', pipeline: pipeline, environment: 'review/$CI_COMMIT_REF_NAME') } + let!(:production_action) { create(:ci_build, :manual, name: 'production', pipeline: pipeline, environment: 'production') } it 'returns a list of actions with matching environment' do expect(environment.actions_for('review/master')).to contain_exactly(review_action) diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index a9aa5698ebb..2f1edf9ab94 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -37,7 +37,7 @@ RSpec.describe EnvironmentStatus do context 'multiple deployments' do it { - new_deployment = create(:deployment, :succeed, environment: deployment.environment, sha: deployment.sha ) + new_deployment = create(:deployment, :succeed, environment: deployment.environment, sha: deployment.sha) is_expected.to eq(new_deployment) } end diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 30e73d84cfb..d48f6f7f3e4 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -187,39 +187,11 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end - describe '#reactive_cache_limit_enabled?' do - subject { setting.reactive_cache_limit_enabled? } - - it { is_expected.to eq(true) } - - context 'when feature flag disabled' do - before do - stub_feature_flags(error_tracking_sentry_limit: false) - end - - it { is_expected.to eq(false) } - end - end - describe '#sentry_client' do subject { setting.sentry_client } it { is_expected.to be_a(ErrorTracking::SentryClient) } it { is_expected.to have_attributes(url: setting.api_url, token: setting.token) } - - describe '#validate_size_guarded_by_feature_flag?' do - subject { setting.sentry_client.validate_size_guarded_by_feature_flag? } - - it { is_expected.to eq(true) } - - context 'when feature flag disabled' do - before do - stub_feature_flags(error_tracking_sentry_limit: false) - end - - it { is_expected.to eq(false) } - end - end end describe '#list_sentry_issues' do diff --git a/spec/models/event_collection_spec.rb b/spec/models/event_collection_spec.rb index 67b58c7bf6f..40b7930f02b 100644 --- a/spec/models/event_collection_spec.rb +++ b/spec/models/event_collection_spec.rb @@ -5,175 +5,165 @@ require 'spec_helper' RSpec.describe EventCollection do include DesignManagementTestHelpers - shared_examples 'EventCollection examples' do - describe '#to_a' do - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project_empty_repo, group: group) } - let_it_be(:projects) { Project.where(id: project.id) } - let_it_be(:user) { create(:user) } - let_it_be(:merge_request) { create(:merge_request) } - - before do - enable_design_management - end + describe '#to_a' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project_empty_repo, group: group) } + let_it_be(:projects) { Project.where(id: project.id) } + let_it_be(:user) { create(:user) } + let_it_be(:merge_request) { create(:merge_request) } - context 'with project events' do - let_it_be(:push_event_payloads) do - Array.new(9) do - create(:push_event_payload, - event: create(:push_event, project: project, author: user)) - end - end + before do + enable_design_management + end - let_it_be(:merge_request_events) { create_list(:event, 10, :merged, project: project, target: merge_request) } - let_it_be(:closed_issue_event) { create(:closed_issue_event, project: project, author: user) } - let_it_be(:wiki_page_event) { create(:wiki_page_event, project: project) } - let_it_be(:design_event) { create(:design_event, project: project) } - - let(:push_events) { push_event_payloads.map(&:event) } - - it 'returns an Array of all event types when no filter is passed', :aggregate_failures do - most_recent_20_events = [ - wiki_page_event, - design_event, - closed_issue_event, - *push_events, - *merge_request_events - ].sort_by(&:id).reverse.take(20) - events = described_class.new(projects).to_a - - expect(events).to be_an_instance_of(Array) - expect(events).to match_array(most_recent_20_events) + context 'with project events' do + let_it_be(:push_event_payloads) do + Array.new(9) do + create(:push_event_payload, + event: create(:push_event, project: project, author: user)) end + end - it 'includes the wiki page events when using to_a' do - events = described_class.new(projects).to_a + let_it_be(:merge_request_events) { create_list(:event, 10, :merged, project: project, target: merge_request) } + let_it_be(:closed_issue_event) { create(:closed_issue_event, project: project, author: user) } + let_it_be(:wiki_page_event) { create(:wiki_page_event, project: project) } + let_it_be(:design_event) { create(:design_event, project: project) } + + let(:push_events) { push_event_payloads.map(&:event) } + + it 'returns an Array of all event types when no filter is passed', :aggregate_failures do + most_recent_20_events = [ + wiki_page_event, + design_event, + closed_issue_event, + *push_events, + *merge_request_events + ].sort_by(&:id).reverse.take(20) + events = described_class.new(projects).to_a + + expect(events).to be_an_instance_of(Array) + expect(events).to match_array(most_recent_20_events) + end - expect(events).to include(wiki_page_event) - end + it 'includes the wiki page events when using to_a' do + events = described_class.new(projects).to_a - it 'includes the design events' do - collection = described_class.new(projects) + expect(events).to include(wiki_page_event) + end - expect(collection.to_a).to include(design_event) - expect(collection.all_project_events).to include(design_event) - end + it 'includes the design events' do + collection = described_class.new(projects) - it 'includes the wiki page events when using all_project_events' do - events = described_class.new(projects).all_project_events + expect(collection.to_a).to include(design_event) + expect(collection.all_project_events).to include(design_event) + end - expect(events).to include(wiki_page_event) - end + it 'includes the wiki page events when using all_project_events' do + events = described_class.new(projects).all_project_events - it 'applies a limit to the number of events' do - events = described_class.new(projects).to_a + expect(events).to include(wiki_page_event) + end - expect(events.length).to eq(20) - end + it 'applies a limit to the number of events' do + events = described_class.new(projects).to_a - it 'can paginate through events' do - events = described_class.new(projects, limit: 5, offset: 15).to_a + expect(events.length).to eq(20) + end - expect(events.length).to eq(5) - end + it 'can paginate through events' do + events = described_class.new(projects, limit: 5, offset: 15).to_a - it 'returns an empty Array when crossing the maximum page number' do - events = described_class.new(projects, limit: 1, offset: 15).to_a + expect(events.length).to eq(5) + end - expect(events).to be_empty - end + it 'returns an empty Array when crossing the maximum page number' do + events = described_class.new(projects, limit: 1, offset: 15).to_a - it 'allows filtering of events using an EventFilter, returning single item' do - filter = EventFilter.new(EventFilter::ISSUE) - events = described_class.new(projects, filter: filter).to_a + expect(events).to be_empty + end - expect(events).to contain_exactly(closed_issue_event) - end + it 'allows filtering of events using an EventFilter, returning single item' do + filter = EventFilter.new(EventFilter::ISSUE) + events = described_class.new(projects, filter: filter).to_a - it 'allows filtering of events using an EventFilter, returning several items' do - filter = EventFilter.new(EventFilter::MERGED) - events = described_class.new(projects, filter: filter).to_a + expect(events).to contain_exactly(closed_issue_event) + end - expect(events).to match_array(merge_request_events) - end + it 'allows filtering of events using an EventFilter, returning several items' do + filter = EventFilter.new(EventFilter::MERGED) + events = described_class.new(projects, filter: filter).to_a - it 'allows filtering of events using an EventFilter, returning pushes' do - filter = EventFilter.new(EventFilter::PUSH) - events = described_class.new(projects, filter: filter).to_a + expect(events).to match_array(merge_request_events) + end - expect(events).to match_array(push_events) - end + it 'allows filtering of events using an EventFilter, returning pushes' do + filter = EventFilter.new(EventFilter::PUSH) + events = described_class.new(projects, filter: filter).to_a + + expect(events).to match_array(push_events) end + end - context 'with group events' do - let(:groups) { group.self_and_descendants.public_or_visible_to_user(user) } - let(:subject) { described_class.new(projects, groups: groups).to_a } + context 'with group events' do + let(:groups) { group.self_and_descendants.public_or_visible_to_user(user) } + let(:subject) { described_class.new(projects, groups: groups).to_a } - it 'includes also group events' do - subgroup = create(:group, parent: group) - event1 = create(:event, project: project, author: user) - event2 = create(:event, project: nil, group: group, author: user) - event3 = create(:event, project: nil, group: subgroup, author: user) + it 'includes also group events' do + subgroup = create(:group, parent: group) + event1 = create(:event, project: project, author: user) + event2 = create(:event, project: nil, group: group, author: user) + event3 = create(:event, project: nil, group: subgroup, author: user) - expect(subject).to eq([event3, event2, event1]) - end + expect(subject).to eq([event3, event2, event1]) + end - it 'does not include events from inaccessible groups' do - subgroup = create(:group, :private, parent: group) - event1 = create(:event, project: nil, group: group, author: user) - create(:event, project: nil, group: subgroup, author: user) + it 'does not include events from inaccessible groups' do + subgroup = create(:group, :private, parent: group) + event1 = create(:event, project: nil, group: group, author: user) + create(:event, project: nil, group: subgroup, author: user) - expect(subject).to match_array([event1]) - end + expect(subject).to match_array([event1]) + end - context 'pagination through events' do - let_it_be(:project_events) { create_list(:event, 10, project: project) } - let_it_be(:group_events) { create_list(:event, 10, group: group, author: user) } + context 'with pagination through events' do + let_it_be(:project_events) { create_list(:event, 10, project: project) } + let_it_be(:group_events) { create_list(:event, 10, group: group, author: user) } - let(:subject) { described_class.new(projects, limit: 10, offset: 5, groups: groups).to_a } + let(:subject) { described_class.new(projects, limit: 10, offset: 5, groups: groups).to_a } - it 'returns recent groups and projects events' do - recent_events_with_offset = (project_events[5..] + group_events[..4]).reverse + it 'returns recent groups and projects events' do + recent_events_with_offset = (project_events[5..] + group_events[..4]).reverse - expect(subject).to eq(recent_events_with_offset) - end + expect(subject).to eq(recent_events_with_offset) end + end - context 'project exclusive event types' do - using RSpec::Parameterized::TableSyntax + context 'with project exclusive event types' do + using RSpec::Parameterized::TableSyntax - where(:filter, :event) do - EventFilter::PUSH | lazy { create(:push_event, project: project) } - EventFilter::MERGED | lazy { create(:event, :merged, project: project, target: merge_request) } - EventFilter::TEAM | lazy { create(:event, :joined, project: project) } - EventFilter::ISSUE | lazy { create(:closed_issue_event, project: project) } - EventFilter::DESIGNS | lazy { create(:design_event, project: project) } - end + where(:filter, :event) do + EventFilter::PUSH | lazy { create(:push_event, project: project) } + EventFilter::MERGED | lazy { create(:event, :merged, project: project, target: merge_request) } + EventFilter::TEAM | lazy { create(:event, :joined, project: project) } + EventFilter::ISSUE | lazy { create(:closed_issue_event, project: project) } + EventFilter::DESIGNS | lazy { create(:design_event, project: project) } + end - with_them do - let(:subject) do - described_class.new(projects, groups: Group.where(id: group.id), filter: EventFilter.new(filter)) - end + with_them do + let(:subject) do + described_class.new(projects, groups: Group.where(id: group.id), filter: EventFilter.new(filter)) + end - it "queries only project events" do - expected_event = event # Forcing lazy evaluation - expect(subject).to receive(:project_events).with(no_args).and_call_original - expect(subject).not_to receive(:group_events) + it "queries only project events" do + expected_event = event # Forcing lazy evaluation + expect(subject).to receive(:project_events).with(no_args).and_call_original + expect(subject).not_to receive(:group_events) - expect(subject.to_a).to match_array(expected_event) - end + expect(subject.to_a).to match_array(expected_event) end end end end - end - - context 'when the optimized_project_and_group_activity_queries FF is on' do - before do - stub_feature_flags(optimized_project_and_group_activity_queries: true) - end - - it_behaves_like 'EventCollection examples' it 'returns no events if no projects are passed' do events = described_class.new(Project.none).to_a @@ -181,12 +171,4 @@ RSpec.describe EventCollection do expect(events).to be_empty end end - - context 'when the optimized_project_and_group_activity_queries FF is off' do - before do - stub_feature_flags(optimized_project_and_group_activity_queries: false) - end - - it_behaves_like 'EventCollection examples' - end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 9700852e567..9579c4c2d27 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -32,7 +32,7 @@ RSpec.describe Event do describe 'after_create :set_last_repository_updated_at' do context 'with a push event' do it 'updates the project last_repository_updated_at and updated_at' do - project.touch(:last_repository_updated_at, time: 1.year.ago) # rubocop: disable Rails/SkipsModelValidations + project.touch(:last_repository_updated_at, time: 1.year.ago) event = create_push_event(project, project.first_owner) @@ -431,8 +431,6 @@ RSpec.describe Event do include_examples 'visibility examples' do let(:visibility) { visible_to_none_except(:member) } end - - include_examples 'visible to author', true end context 'private project' do @@ -866,7 +864,7 @@ RSpec.describe Event do end it 'updates the project' do - project.touch(:last_activity_at, time: 1.year.ago) # rubocop: disable Rails/SkipsModelValidations + project.touch(:last_activity_at, time: 1.year.ago) event = create_push_event(project, project.first_owner) @@ -882,7 +880,7 @@ RSpec.describe Event do "project:#{project.id}") end - project.touch(:last_activity_at, time: 1.year.ago) # rubocop: disable Rails/SkipsModelValidations + project.touch(:last_activity_at, time: 1.year.ago) create_push_event(project, project.first_owner) end diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb deleted file mode 100644 index de6ce3ba053..00000000000 --- a/spec/models/experiment_spec.rb +++ /dev/null @@ -1,428 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Experiment do - include AfterNextHelpers - - subject { build(:experiment) } - - describe 'associations' do - it { is_expected.to have_many(:experiment_users) } - it { is_expected.to have_many(:experiment_subjects) } - end - - describe 'validations' do - it { is_expected.to validate_presence_of(:name) } - it { is_expected.to validate_uniqueness_of(:name) } - it { is_expected.to validate_length_of(:name).is_at_most(255) } - end - - describe '.add_user' do - let_it_be(:experiment_name) { :experiment_key } - let_it_be(:user) { 'a user' } - let_it_be(:group) { 'a group' } - let_it_be(:context) { { a: 42 } } - - subject(:add_user) { described_class.add_user(experiment_name, group, user, context) } - - context 'when an experiment with the provided name does not exist' do - it 'creates a new experiment record' do - allow_next_instance_of(described_class) do |experiment| - allow(experiment).to receive(:record_user_and_group).with(user, group, context) - end - expect { add_user }.to change(described_class, :count).by(1) - end - - it 'forwards the user, group_type, and context to the instance' do - expect_next_instance_of(described_class) do |experiment| - expect(experiment).to receive(:record_user_and_group).with(user, group, context) - end - add_user - end - end - - context 'when an experiment with the provided name already exists' do - let_it_be(:experiment) { create(:experiment, name: experiment_name) } - - it 'does not create a new experiment record' do - allow_next_found_instance_of(described_class) do |experiment| - allow(experiment).to receive(:record_user_and_group).with(user, group, context) - end - expect { add_user }.not_to change(described_class, :count) - end - - it 'forwards the user, group_type, and context to the instance' do - expect_next_found_instance_of(described_class) do |experiment| - expect(experiment).to receive(:record_user_and_group).with(user, group, context) - end - add_user - end - end - - it 'works without the optional context argument' do - allow_next_instance_of(described_class) do |experiment| - expect(experiment).to receive(:record_user_and_group).with(user, group, {}) - end - - expect { described_class.add_user(experiment_name, group, user) }.not_to raise_error - end - end - - describe '.add_group' do - let_it_be(:experiment_name) { :experiment_key } - let_it_be(:variant) { :control } - let_it_be(:group) { build(:group) } - - subject(:add_group) { described_class.add_group(experiment_name, variant: variant, group: group) } - - context 'when an experiment with the provided name does not exist' do - it 'creates a new experiment record' do - allow_next(described_class, name: :experiment_key) - .to receive(:record_subject_and_variant!).with(group, variant) - - expect { add_group }.to change(described_class, :count).by(1) - end - end - - context 'when an experiment with the provided name already exists' do - before do - create(:experiment, name: experiment_name) - end - - it 'does not create a new experiment record' do - expect { add_group }.not_to change(described_class, :count) - end - end - end - - describe '.record_conversion_event' do - let_it_be(:user) { build(:user) } - let_it_be(:context) { { a: 42 } } - - let(:experiment_key) { :test_experiment } - - subject(:record_conversion_event) { described_class.record_conversion_event(experiment_key, user, context) } - - context 'when no matching experiment exists' do - it 'creates the experiment and uses it' do - expect_next_instance_of(described_class) do |experiment| - expect(experiment).to receive(:record_conversion_event_for_user) - end - expect { record_conversion_event }.to change { described_class.count }.by(1) - end - - context 'but we are unable to successfully create one' do - let(:experiment_key) { nil } - - it 'raises a RecordInvalid error' do - expect { record_conversion_event }.to raise_error(ActiveRecord::RecordInvalid) - end - end - end - - context 'when a matching experiment already exists' do - before do - create(:experiment, name: experiment_key) - end - - it 'sends record_conversion_event_for_user to the experiment instance' do - expect_next_found_instance_of(described_class) do |experiment| - expect(experiment).to receive(:record_conversion_event_for_user).with(user, context) - end - record_conversion_event - end - end - end - - shared_examples 'experiment user with context' do - let_it_be(:context) { { a: 42, 'b' => 34, 'c': { c1: 100, c2: 'c2', e: :e }, d: [1, 3] } } - let_it_be(:initial_expected_context) { { 'a' => 42, 'b' => 34, 'c' => { 'c1' => 100, 'c2' => 'c2', 'e' => 'e' }, 'd' => [1, 3] } } - - before do - subject - experiment.record_user_and_group(user, :experimental, {}) - end - - it 'has an initial context with stringified keys' do - expect(ExperimentUser.last.context).to eq(initial_expected_context) - end - - context 'when updated' do - before do - subject - experiment.record_user_and_group(user, :experimental, new_context) - end - - context 'with an empty context' do - let_it_be(:new_context) { {} } - - it 'keeps the initial context' do - expect(ExperimentUser.last.context).to eq(initial_expected_context) - end - end - - context 'with string keys' do - let_it_be(:new_context) { { f: :some_symbol } } - - it 'adds new symbols stringified' do - expected_context = initial_expected_context.merge('f' => 'some_symbol') - expect(ExperimentUser.last.context).to eq(expected_context) - end - end - - context 'with atomic values or array values' do - let_it_be(:new_context) { { b: 97, d: [99] } } - - it 'overrides the values' do - expected_context = { 'a' => 42, 'b' => 97, 'c' => { 'c1' => 100, 'c2' => 'c2', 'e' => 'e' }, 'd' => [99] } - expect(ExperimentUser.last.context).to eq(expected_context) - end - end - - context 'with nested hashes' do - let_it_be(:new_context) { { c: { g: 107 } } } - - it 'inserts nested additional values in the same keys' do - expected_context = initial_expected_context.deep_merge('c' => { 'g' => 107 }) - expect(ExperimentUser.last.context).to eq(expected_context) - end - end - end - end - - describe '#record_conversion_event_for_user' do - let_it_be(:user) { create(:user) } - let_it_be(:experiment) { create(:experiment) } - let_it_be(:context) { { a: 42 } } - - subject { experiment.record_conversion_event_for_user(user, context) } - - context 'when no existing experiment_user record exists for the given user' do - it 'does not update or create an experiment_user record' do - expect { subject }.not_to change { ExperimentUser.all.to_a } - end - end - - context 'when an existing experiment_user exists for the given user' do - context 'but it has already been converted' do - let!(:experiment_user) { create(:experiment_user, experiment: experiment, user: user, converted_at: 2.days.ago) } - - it 'does not update the converted_at value' do - expect { subject }.not_to change { experiment_user.converted_at } - end - - it_behaves_like 'experiment user with context' do - before do - experiment.record_user_and_group(user, :experimental, context) - end - end - end - - context 'and it has not yet been converted' do - let(:experiment_user) { create(:experiment_user, experiment: experiment, user: user) } - - it 'updates the converted_at value' do - expect { subject }.to change { experiment_user.reload.converted_at } - end - - it_behaves_like 'experiment user with context' do - before do - experiment.record_user_and_group(user, :experimental, context) - end - end - end - end - end - - describe '#record_conversion_event_for_subject' do - let_it_be(:user) { create(:user) } - let_it_be(:experiment) { create(:experiment) } - let_it_be(:context) { { a: 42 } } - - subject(:record_conversion) { experiment.record_conversion_event_for_subject(user, context) } - - context 'when no existing experiment_subject record exists for the given user' do - it 'does not update or create an experiment_subject record' do - expect { record_conversion }.not_to change { ExperimentSubject.all.to_a } - end - end - - context 'when an existing experiment_subject exists for the given user' do - context 'but it has already been converted' do - let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user, converted_at: 2.days.ago) } - - it 'does not update the converted_at value' do - expect { record_conversion }.not_to change { experiment_subject.converted_at } - end - end - - context 'and it has not yet been converted' do - let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user) } - - it 'updates the converted_at value' do - expect { record_conversion }.to change { experiment_subject.reload.converted_at } - end - end - - context 'with no existing context' do - let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user) } - - it 'updates the context' do - expect { record_conversion }.to change { experiment_subject.reload.context }.to('a' => 42) - end - end - - context 'with an existing context' do - let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user, converted_at: 2.days.ago, context: { b: 1 } ) } - - it 'merges the context' do - expect { record_conversion }.to change { experiment_subject.reload.context }.to('a' => 42, 'b' => 1) - end - end - end - end - - describe '#record_subject_and_variant!' do - let_it_be(:subject_to_record) { create(:group) } - let_it_be(:variant) { :control } - let_it_be(:experiment) { create(:experiment) } - - subject(:record_subject_and_variant!) { experiment.record_subject_and_variant!(subject_to_record, variant) } - - context 'when no existing experiment_subject record exists for the given subject' do - it 'creates an experiment_subject record' do - expect { record_subject_and_variant! }.to change(ExperimentSubject, :count).by(1) - expect(ExperimentSubject.last.variant).to eq(variant.to_s) - end - end - - context 'when an existing experiment_subject exists for the given subject' do - let_it_be(:experiment_subject) do - create(:experiment_subject, experiment: experiment, namespace: subject_to_record, user: nil, variant: :experimental) - end - - context 'when it belongs to the same variant' do - let(:variant) { :experimental } - - it 'does not initiate a transaction' do - expect(Experiment.connection).not_to receive(:transaction) - - subject - end - end - - context 'but it belonged to a different variant' do - it 'updates the variant value' do - expect { record_subject_and_variant! }.to change { experiment_subject.reload.variant }.to('control') - end - end - end - - describe 'providing a subject to record' do - context 'when given a group as subject' do - it 'saves the namespace as the experiment subject' do - expect(record_subject_and_variant!.namespace).to eq(subject_to_record) - end - end - - context 'when given a users namespace as subject' do - let_it_be(:subject_to_record) { build(:namespace) } - - it 'saves the namespace as the experiment_subject' do - expect(record_subject_and_variant!.namespace).to eq(subject_to_record) - end - end - - context 'when given a user as subject' do - let_it_be(:subject_to_record) { build(:user) } - - it 'saves the user as experiment_subject user' do - expect(record_subject_and_variant!.user).to eq(subject_to_record) - end - end - - context 'when given a project as subject' do - let_it_be(:subject_to_record) { build(:project) } - - it 'saves the project as experiment_subject user' do - expect(record_subject_and_variant!.project).to eq(subject_to_record) - end - end - - context 'when given no subject' do - let_it_be(:subject_to_record) { nil } - - it 'raises an error' do - expect { record_subject_and_variant! }.to raise_error('Incompatible subject provided!') - end - end - - context 'when given an incompatible subject' do - let_it_be(:subject_to_record) { build(:ci_build) } - - it 'raises an error' do - expect { record_subject_and_variant! }.to raise_error('Incompatible subject provided!') - end - end - end - end - - describe '#record_user_and_group' do - let_it_be(:experiment) { create(:experiment) } - let_it_be(:user) { create(:user) } - let_it_be(:group) { :control } - let_it_be(:context) { { a: 42 } } - - subject { experiment.record_user_and_group(user, group, context) } - - context 'when an experiment_user does not yet exist for the given user' do - it 'creates a new experiment_user record' do - expect { subject }.to change(ExperimentUser, :count).by(1) - end - - it 'assigns the correct group_type to the experiment_user' do - subject - - expect(ExperimentUser.last.group_type).to eq('control') - end - - it 'adds the correct context to the experiment_user' do - subject - - expect(ExperimentUser.last.context).to eq({ 'a' => 42 }) - end - end - - context 'when an experiment_user already exists for the given user' do - before do - # Create an existing experiment_user for this experiment and the :control group - experiment.record_user_and_group(user, :control) - end - - it 'does not create a new experiment_user record' do - expect { subject }.not_to change(ExperimentUser, :count) - end - - context 'when group type or context did not change' do - let(:context) { {} } - - it 'does not initiate a transaction' do - expect(Experiment.connection).not_to receive(:transaction) - - subject - end - end - - context 'but the group_type and context has changed' do - let(:group) { :experimental } - - it 'updates the existing experiment_user record with group_type' do - expect { subject }.to change { ExperimentUser.last.group_type } - end - end - - it_behaves_like 'experiment user with context' - end - end -end diff --git a/spec/models/experiment_subject_spec.rb b/spec/models/experiment_subject_spec.rb deleted file mode 100644 index d86dc3cbf65..00000000000 --- a/spec/models/experiment_subject_spec.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ExperimentSubject, type: :model do - describe 'associations' do - it { is_expected.to belong_to(:experiment) } - it { is_expected.to belong_to(:user) } - it { is_expected.to belong_to(:namespace) } - it { is_expected.to belong_to(:project) } - end - - describe 'validations' do - it { is_expected.to validate_presence_of(:experiment) } - - describe 'must_have_one_subject_present' do - let(:experiment_subject) { build(:experiment_subject, user: nil, namespace: nil, project: nil) } - let(:error_message) { 'Must have exactly one of User, Namespace, or Project.' } - - it 'fails when no subject is present' do - expect(experiment_subject).not_to be_valid - expect(experiment_subject.errors[:base]).to include(error_message) - end - - it 'passes when user subject is present' do - experiment_subject.user = build(:user) - expect(experiment_subject).to be_valid - end - - it 'passes when namespace subject is present' do - experiment_subject.namespace = build(:group) - expect(experiment_subject).to be_valid - end - - it 'passes when project subject is present' do - experiment_subject.project = build(:project) - expect(experiment_subject).to be_valid - end - - it 'fails when more than one subject is present', :aggregate_failures do - # two subjects - experiment_subject.user = build(:user) - experiment_subject.namespace = build(:group) - expect(experiment_subject).not_to be_valid - expect(experiment_subject.errors[:base]).to include(error_message) - - # three subjects - experiment_subject.project = build(:project) - expect(experiment_subject).not_to be_valid - expect(experiment_subject.errors[:base]).to include(error_message) - end - end - end - - describe '.valid_subject?' do - subject(:valid_subject?) { described_class.valid_subject?(subject_class.new) } - - context 'when passing a Group, Namespace, User or Project' do - [Group, Namespace, User, Project].each do |subject_class| - let(:subject_class) { subject_class } - - it { is_expected.to be(true) } - end - end - - context 'when passing another object' do - let(:subject_class) { Issue } - - it { is_expected.to be(false) } - end - end -end diff --git a/spec/models/experiment_user_spec.rb b/spec/models/experiment_user_spec.rb deleted file mode 100644 index 9201529b145..00000000000 --- a/spec/models/experiment_user_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ExperimentUser do - describe 'Associations' do - it { is_expected.to belong_to(:experiment) } - it { is_expected.to belong_to(:user) } - end - - describe 'Validations' do - it { is_expected.to validate_presence_of(:group_type) } - end -end diff --git a/spec/models/exported_protected_branch_spec.rb b/spec/models/exported_protected_branch_spec.rb index 7886a522741..9f862de6ff8 100644 --- a/spec/models/exported_protected_branch_spec.rb +++ b/spec/models/exported_protected_branch_spec.rb @@ -11,7 +11,7 @@ RSpec.describe ExportedProtectedBranch do it 'returns the correct push access levels' do exported_branch = create(:exported_protected_branch, :developers_can_push) deploy_key = create(:deploy_key) - create(:deploy_keys_project, :write_access, project: exported_branch.project, deploy_key: deploy_key ) + create(:deploy_keys_project, :write_access, project: exported_branch.project, deploy_key: deploy_key) create(:protected_branch_push_access_level, protected_branch: exported_branch, deploy_key: deploy_key) dev_push_access_level = exported_branch.push_access_levels.first diff --git a/spec/models/factories_spec.rb b/spec/models/factories_spec.rb index c931c96bafd..65b993cca7f 100644 --- a/spec/models/factories_spec.rb +++ b/spec/models/factories_spec.rb @@ -2,59 +2,100 @@ require 'spec_helper' -RSpec.describe 'factories' do +# `:saas` is used to test `gitlab_subscription` factory. +# It's not available on FOSS but also this very factory is not. +RSpec.describe 'factories', :saas do include Database::DatabaseHelpers + # Used in `skipped` and indicates whether to skip any traits including the + # plain factory. + any = Object.new + # https://gitlab.com/groups/gitlab-org/-/epics/5464 tracks the remaining - # skipped traits. + # skipped factories or traits. # # Consider adding a code comment if a trait cannot produce a valid object. - def skipped_traits - [ - [:audit_event, :unauthenticated], - [:ci_build_trace_chunk, :fog_with_data], - [:ci_job_artifact, :remote_store], - [:ci_job_artifact, :raw], - [:ci_job_artifact, :gzip], - [:ci_job_artifact, :correct_checksum], - [:environment, :non_playable], - [:composer_cache_file, :object_storage], - [:debian_project_component_file, :object_storage], - [:debian_project_distribution, :object_storage], - [:debian_file_metadatum, :unknown], - [:issue_customer_relations_contact, :for_contact], - [:issue_customer_relations_contact, :for_issue], - [:package_file, :object_storage], - [:rpm_repository_file, :object_storage], - [:pages_domain, :without_certificate], - [:pages_domain, :without_key], - [:pages_domain, :with_missing_chain], - [:pages_domain, :with_trusted_chain], - [:pages_domain, :with_trusted_expired_chain], - [:pages_domain, :explicit_ecdsa], - [:project_member, :blocked], - [:remote_mirror, :ssh], - [:user_preference, :only_comments], - [:ci_pipeline_artifact, :remote_store] - ] - end + skipped = [ + [:audit_event, :unauthenticated], + [:ci_build_trace_chunk, :fog_with_data], + [:ci_job_artifact, :remote_store], + [:ci_job_artifact, :raw], + [:ci_job_artifact, :gzip], + [:ci_job_artifact, :correct_checksum], + [:dependency_proxy_blob, :remote_store], + [:environment, :non_playable], + [:composer_cache_file, :object_storage], + [:debian_project_component_file, :object_storage], + [:debian_project_distribution, :object_storage], + [:debian_file_metadatum, :unknown], + [:issue_customer_relations_contact, :for_contact], + [:issue_customer_relations_contact, :for_issue], + [:package_file, :object_storage], + [:rpm_repository_file, :object_storage], + [:pages_domain, :without_certificate], + [:pages_domain, :without_key], + [:pages_domain, :with_missing_chain], + [:pages_domain, :with_trusted_chain], + [:pages_domain, :with_trusted_expired_chain], + [:pages_domain, :explicit_ecdsa], + [:project_member, :blocked], + [:remote_mirror, :ssh], + [:user_preference, :only_comments], + [:ci_pipeline_artifact, :remote_store], + # EE + [:dast_profile, :with_dast_site_validation], + [:ee_ci_build, :dependency_scanning_report], + [:ee_ci_build, :license_scan_v1], + [:ee_ci_job_artifact, :v1], + [:ee_ci_job_artifact, :v1_1], + [:ee_ci_job_artifact, :v2], + [:ee_ci_job_artifact, :v2_1], + [:geo_ci_secure_file_state, any], + [:geo_dependency_proxy_blob_state, any], + [:geo_event_log, :geo_event], + [:geo_job_artifact_state, any], + [:geo_lfs_object_state, any], + [:geo_pages_deployment_state, any], + [:geo_upload_state, any], + [:geo_ci_secure_file_state, any], + [:lfs_object, :checksum_failure], + [:lfs_object, :checksummed], + [:merge_request, :blocked], + [:merge_request_diff, :verification_failed], + [:merge_request_diff, :verification_succeeded], + [:package_file, :verification_failed], + [:package_file, :verification_succeeded], + [:project, :with_vulnerabilities], + [:scan_execution_policy, :with_schedule_and_agent], + [:vulnerability, :with_cluster_image_scanning_finding], + [:vulnerability, :with_findings], + [:vulnerability_export, :finished] + ].freeze shared_examples 'factory' do |factory| + skip_any = skipped.include?([factory.name, any]) + describe "#{factory.name} factory" do it 'does not raise error when built' do + # We use `skip` here because using `build` mostly work even if + # factories break when creating them. + skip 'Factory skipped linting due to legacy error' if skip_any + expect { build(factory.name) }.not_to raise_error end it 'does not raise error when created' do + pending 'Factory skipped linting due to legacy error' if skip_any + expect { create(factory.name) }.not_to raise_error # rubocop:disable Rails/SaveBang end factory.definition.defined_traits.map(&:name).each do |trait_name| + skip_trait = skip_any || skipped.include?([factory.name, trait_name.to_sym]) + describe "linting :#{trait_name} trait" do it 'does not raise error when created' do - if skipped_traits.include?([factory.name, trait_name.to_sym]) - pending("Trait skipped linting due to legacy error") - end + pending 'Trait skipped linting due to legacy error' if skip_trait expect { create(factory.name, trait_name) }.not_to raise_error end @@ -71,6 +112,7 @@ RSpec.describe 'factories' do # is being mutated. skip_factory_defaults = %i[ ci_job_token_project_scope_link + ci_subscriptions_project evidence exported_protected_branch fork_network_member @@ -78,22 +120,27 @@ RSpec.describe 'factories' do import_state issue_customer_relations_contact member_task + merge_request_block milestone_release namespace project_namespace project_repository + project_security_setting prometheus_alert prometheus_alert_event prometheus_metric protected_branch protected_branch_merge_access_level protected_branch_push_access_level + protected_branch_unprotect_access_level protected_tag + protected_tag_create_access_level release release_link self_managed_prometheus_alert_event shard users_star_project + vulnerabilities_finding_identifier wiki_page wiki_page_meta ].to_set.freeze @@ -110,6 +157,27 @@ RSpec.describe 'factories' do without_fd, with_fd = FactoryBot.factories .partition { |factory| skip_factory_defaults.include?(factory.name) } + # Some EE models check licensed features so stub them. + shared_context 'with licensed features' do + licensed_features = %i[ + board_milestone_lists + board_assignee_lists + ].index_with(true) + + if Gitlab.jh? + licensed_features.merge! %i[ + dingtalk_integration + feishu_bot_integration + ].index_with(true) + end + + before do + stub_licensed_features(licensed_features) + end + end + + include_context 'with licensed features' if Gitlab.ee? + context 'with factory defaults', factory_default: :keep do let_it_be(:namespace) { create_default(:namespace).freeze } let_it_be(:project) { create_default(:project, :repository).freeze } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 68c2d1d3995..6ba450b6d57 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -40,6 +40,7 @@ RSpec.describe Group do it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') } it { is_expected.to have_many(:contacts).class_name('CustomerRelations::Contact') } it { is_expected.to have_many(:organizations).class_name('CustomerRelations::Organization') } + it { is_expected.to have_many(:protected_branches) } it { is_expected.to have_one(:crm_settings) } it { is_expected.to have_one(:group_feature) } it { is_expected.to have_one(:harbor_integration) } @@ -98,6 +99,15 @@ RSpec.describe Group do expect(group).to be_valid end + + it 'does not allow a subgroup to have the same name as an existing subgroup' do + sub_group1 = create(:group, parent: group, name: "SG", path: 'api') + sub_group2 = described_class.new(parent: group, name: "SG", path: 'api2') + + expect(sub_group1).to be_valid + expect(sub_group2).not_to be_valid + expect(sub_group2.errors.full_messages.to_sentence).to eq('Name has already been taken') + end end end @@ -1058,28 +1068,45 @@ RSpec.describe Group do end context 'with owners from a parent' do - before do - parent_group = create(:group) - create(:group_member, :owner, group: parent_group) - group.update!(parent: parent_group) + context 'when top-level group' do + it { expect(group.last_owner?(@members[:owner])).to be_truthy } + + context 'with group sharing' do + let!(:subgroup) { create(:group, parent: group) } + + before do + create(:group_group_link, :owner, shared_group: group, shared_with_group: subgroup) + create(:group_member, :owner, group: subgroup) + end + + it { expect(group.last_owner?(@members[:owner])).to be_truthy } + end end - it { expect(group.last_owner?(@members[:owner])).to be_falsy } + context 'when subgroup' do + let!(:subgroup) { create(:group, parent: group) } + + it { expect(subgroup.last_owner?(@members[:owner])).to be_truthy } + + context 'with two owners' do + before do + create(:group_member, :owner, group: group) + end + + it { expect(subgroup.last_owner?(@members[:owner])).to be_falsey } + end + end end end describe '#member_last_blocked_owner?' do - let_it_be(:blocked_user) { create(:user, :blocked) } + let!(:blocked_user) { create(:user, :blocked) } - let(:member) { blocked_user.group_members.last } - - before do - group.add_member(blocked_user, GroupMember::OWNER) - end + let!(:member) { group.add_member(blocked_user, GroupMember::OWNER) } context 'when last_blocked_owner is set' do before do - expect(group).not_to receive(:members_with_parents) + expect(group).not_to receive(:member_owners_excluding_project_bots) end it 'returns true' do @@ -1106,6 +1133,14 @@ RSpec.describe Group do it { expect(group.member_last_blocked_owner?(member)).to be(false) } end + context 'with another active project_bot owner' do + before do + group.add_member(create(:user, :project_bot), GroupMember::OWNER) + end + + it { expect(group.member_last_blocked_owner?(member)).to be(true) } + end + context 'with 2 blocked owners' do before do group.add_member(create(:user, :blocked), GroupMember::OWNER) @@ -1115,13 +1150,36 @@ RSpec.describe Group do end context 'with owners from a parent' do - before do - parent_group = create(:group) - create(:group_member, :owner, group: parent_group) - group.update!(parent: parent_group) + context 'when top-level group' do + it { expect(group.member_last_blocked_owner?(member)).to be(true) } + + context 'with group sharing' do + let!(:subgroup) { create(:group, parent: group) } + + before do + create(:group_group_link, :owner, shared_group: group, shared_with_group: subgroup) + create(:group_member, :owner, group: subgroup) + end + + it { expect(group.member_last_blocked_owner?(member)).to be(true) } + end end - it { expect(group.member_last_blocked_owner?(member)).to be(false) } + context 'when subgroup' do + let!(:subgroup) { create(:group, :nested) } + + let!(:member) { subgroup.add_member(blocked_user, GroupMember::OWNER) } + + it { expect(subgroup.member_last_blocked_owner?(member)).to be(true) } + + context 'with two owners' do + before do + create(:group_member, :owner, group: subgroup.parent) + end + + it { expect(subgroup.member_last_blocked_owner?(member)).to be(false) } + end + end end end end @@ -1174,58 +1232,63 @@ RSpec.describe Group do end end - describe '#all_owners_excluding_project_bots' do + describe '#member_owners_excluding_project_bots' do let_it_be(:user) { create(:user) } - context 'when there is only one owner' do - let!(:owner) do - group.add_member(user, GroupMember::OWNER) - end + let!(:member_owner) do + group.add_member(user, GroupMember::OWNER) + end - it 'returns the owner' do - expect(group.all_owners_excluding_project_bots).to contain_exactly(owner) - end + it 'returns the member-owners' do + expect(group.member_owners_excluding_project_bots).to contain_exactly(member_owner) + end - context 'and there is also a project_bot owner' do - before do - group.add_member(create(:user, :project_bot), GroupMember::OWNER) - end + context 'there is also a project_bot owner' do + before do + group.add_member(create(:user, :project_bot), GroupMember::OWNER) + end - it 'returns only the human owner' do - expect(group.all_owners_excluding_project_bots).to contain_exactly(owner) - end + it 'returns only the human member-owners' do + expect(group.member_owners_excluding_project_bots).to contain_exactly(member_owner) end end - context 'when there are multiple owners' do - let_it_be(:user_2) { create(:user) } + context 'with owners from a parent' do + context 'when top-level group' do + context 'with group sharing' do + let!(:subgroup) { create(:group, parent: group) } - let!(:owner) do - group.add_member(user, GroupMember::OWNER) - end + before do + create(:group_group_link, :owner, shared_group: group, shared_with_group: subgroup) + subgroup.add_member(user, GroupMember::OWNER) + end - let!(:owner2) do - group.add_member(user_2, GroupMember::OWNER) + it 'returns only direct member-owners' do + expect(group.member_owners_excluding_project_bots).to contain_exactly(member_owner) + end + end end - it 'returns both owners' do - expect(group.all_owners_excluding_project_bots).to contain_exactly(owner, owner2) - end + context 'when subgroup' do + let!(:subgroup) { create(:group, parent: group) } - context 'and there is also a project_bot owner' do - before do - group.add_member(create(:user, :project_bot), GroupMember::OWNER) + let_it_be(:user_2) { create(:user) } + + let!(:member_owner_2) do + subgroup.add_member(user_2, GroupMember::OWNER) end - it 'returns only the human owners' do - expect(group.all_owners_excluding_project_bots).to contain_exactly(owner, owner2) + it 'returns member-owners including parents' do + expect(subgroup.member_owners_excluding_project_bots).to contain_exactly(member_owner, member_owner_2) end end end context 'when there are no owners' do - it 'returns false' do - expect(group.all_owners_excluding_project_bots).to be_empty + let_it_be(:empty_group) { create(:group) } + + it 'returns an empty result' do + expect(empty_group.member_owners_excluding_project_bots).to be_empty end end end @@ -2405,23 +2468,6 @@ RSpec.describe Group do end end - describe '.groups_including_descendants_by' do - let_it_be(:parent_group1) { create(:group) } - let_it_be(:parent_group2) { create(:group) } - let_it_be(:extra_group) { create(:group) } - let_it_be(:child_group1) { create(:group, parent: parent_group1) } - let_it_be(:child_group2) { create(:group, parent: parent_group1) } - let_it_be(:child_group3) { create(:group, parent: parent_group2) } - - subject { described_class.groups_including_descendants_by([parent_group2.id, parent_group1.id]) } - - shared_examples 'returns the expected groups for a group and its descendants' do - specify { is_expected.to contain_exactly(parent_group1, parent_group2, child_group1, child_group2, child_group3) } - end - - it_behaves_like 'returns the expected groups for a group and its descendants' - end - describe '.preset_root_ancestor_for' do let_it_be(:rootgroup, reload: true) { create(:group) } let_it_be(:subgroup, reload: true) { create(:group, parent: rootgroup) } @@ -2555,7 +2601,7 @@ RSpec.describe Group do end context 'when parent does not allow' do - let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false ) } + let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false) } let_it_be(:group, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } it 'raises exception' do diff --git a/spec/models/hooks/active_hook_filter_spec.rb b/spec/models/hooks/active_hook_filter_spec.rb index 1f693ce9fde..47c0fbdb106 100644 --- a/spec/models/hooks/active_hook_filter_spec.rb +++ b/spec/models/hooks/active_hook_filter_spec.rb @@ -6,65 +6,102 @@ RSpec.describe ActiveHookFilter do subject(:filter) { described_class.new(hook) } describe '#matches?' do - context 'for push event hooks' do + using RSpec::Parameterized::TableSyntax + + context 'for various types of branch_filter' do let(:hook) do - create(:project_hook, push_events: true, push_events_branch_filter: branch_filter) + build(:project_hook, push_events: true, issues_events: true) end - context 'branch filter is specified' do - let(:branch_filter) { 'master' } + where(:branch_filter_strategy, :branch_filter, :ref, :expected_matches?) do + 'all_branches' | 'master' | 'refs/heads/master' | true + 'all_branches' | '' | 'refs/heads/master' | true + 'all_branches' | nil | 'refs/heads/master' | true + 'all_branches' | '.*' | 'refs/heads/master' | true + 'wildcard' | 'master' | 'refs/heads/master' | true + 'wildcard' | 'master' | 'refs/heads/my_branch' | false + 'wildcard' | 'features/*' | 'refs/heads/features/my-branch' | true + 'wildcard' | 'features/*' | 'refs/heads/features/my-branch/something' | true + 'wildcard' | 'features/*' | 'refs/heads/master' | false + 'wildcard' | nil | 'refs/heads/master' | true + 'wildcard' | '' | 'refs/heads/master' | true + 'regex' | 'master' | 'refs/heads/master' | true + 'regex' | 'master' | 'refs/heads/my_branch' | false + 'regex' | 'features/*' | 'refs/heads/xxxx/features/my-branch' | true + 'regex' | 'features/*' | 'refs/heads/features/' | true + 'regex' | 'features/*' | 'refs/heads/features' | true + 'regex' | 'features/.*' | 'refs/heads/features/my-branch' | true + 'regex' | 'features/.*' | 'refs/heads/features/my-branch/something' | true + 'regex' | 'features/.*' | 'refs/heads/master' | false + 'regex' | '(feature|dev)' | 'refs/heads/feature' | true + 'regex' | '(feature|dev)' | 'refs/heads/dev' | true + 'regex' | '(feature|dev)' | 'refs/heads/master' | false + 'regex' | nil | 'refs/heads/master' | true + 'regex' | '' | 'refs/heads/master' | true + end - it 'returns true if branch matches' do - expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true + with_them do + before do + hook.assign_attributes( + push_events_branch_filter: branch_filter, + branch_filter_strategy: branch_filter_strategy + ) end - it 'returns false if branch does not match' do - expect(filter.matches?(:push_hooks, { ref: 'refs/heads/my_branch' })).to be false - end + it { expect(filter.matches?(:push_hooks, { ref: ref })).to be expected_matches? } + it { expect(filter.matches?(:issues_events, { ref: ref })).to be true } + end - it 'returns false if ref is nil' do - expect(filter.matches?(:push_hooks, {})).to be false + context 'when the branch filter is a invalid regex' do + let(:hook) do + build( + :project_hook, + push_events: true, + push_events_branch_filter: 'master', + branch_filter_strategy: 'regex' + ) end - context 'branch filter contains wildcard' do - let(:branch_filter) { 'features/*' } - - it 'returns true if branch matches' do - expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch' })).to be true - expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch/something' })).to be true - end - - it 'returns false if branch does not match' do - expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false - end + before do + allow(hook).to receive(:push_events_branch_filter).and_return("invalid-regex[") end - end - context 'branch filter is not specified' do - let(:branch_filter) { nil } - - it 'returns true' do - expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true - end + it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false } end - context 'branch filter is empty string' do - let(:branch_filter) { '' } + context 'when the branch filter is not properly set to nil' do + let(:hook) do + build( + :project_hook, + push_events: true, + branch_filter_strategy: 'all_branches' + ) + end - it 'acts like branch is not specified' do - expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true + before do + allow(hook).to receive(:push_events_branch_filter).and_return("master") end + + it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/feature1' })).to be true } end end - context 'for non-push-events hooks' do - let(:hook) do - create(:project_hook, issues_events: true, push_events: false, push_events_branch_filter: '') + context 'when feature flag is disabled' do + before do + stub_feature_flags(enhanced_webhook_support_regex: false) end - it 'returns true as branch filters are not yet supported for these' do - expect(filter.matches?(:issues_events, { ref: 'refs/heads/master' })).to be true + let(:hook) do + build( + :project_hook, + push_events: true, + push_events_branch_filter: '(master)', + branch_filter_strategy: 'regex' + ) end + + it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false } + it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/(master)' })).to be true } end end end diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index 923a6f92424..3d8c377ab21 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -12,14 +12,14 @@ RSpec.describe ProjectHook do end it_behaves_like 'includes Limitable concern' do - subject { build(:project_hook, project: create(:project)) } + subject { build(:project_hook) } end describe '.for_projects' do it 'finds related project hooks' do - hook_a = create(:project_hook) - hook_b = create(:project_hook) - hook_c = create(:project_hook) + hook_a = create(:project_hook, project: build(:project)) + hook_b = create(:project_hook, project: build(:project)) + hook_c = create(:project_hook, project: build(:project)) expect(described_class.for_projects([hook_a.project, hook_b.project])) .to contain_exactly(hook_a, hook_b) @@ -30,16 +30,18 @@ RSpec.describe ProjectHook do describe '.push_hooks' do it 'returns hooks for push events only' do - hook = create(:project_hook, push_events: true) - create(:project_hook, push_events: false) + project = build(:project) + hook = create(:project_hook, project: project, push_events: true) + create(:project_hook, project: project, push_events: false) expect(described_class.push_hooks).to eq([hook]) end end describe '.tag_push_hooks' do it 'returns hooks for tag push events only' do - hook = create(:project_hook, tag_push_events: true) - create(:project_hook, tag_push_events: false) + project = build(:project) + hook = create(:project_hook, project: project, tag_push_events: true) + create(:project_hook, project: project, tag_push_events: false) expect(described_class.tag_push_hooks).to eq([hook]) end end @@ -65,7 +67,7 @@ RSpec.describe ProjectHook do end describe '#update_last_failure', :clean_gitlab_redis_shared_state do - let_it_be(:hook) { create(:project_hook) } + let(:hook) { build(:project_hook) } it 'is a method of this class' do expect { hook.update_last_failure }.not_to raise_error diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index f4786083b75..ba94730b1dd 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -4,7 +4,7 @@ require "spec_helper" RSpec.describe SystemHook do context 'default attributes' do - let(:system_hook) { build(:system_hook) } + let(:system_hook) { described_class.new } it 'sets defined default parameters' do attrs = { @@ -32,10 +32,10 @@ RSpec.describe SystemHook do end describe "execute", :sidekiq_might_not_need_inline do - let(:system_hook) { create(:system_hook) } - let(:user) { create(:user) } - let(:project) { create(:project, namespace: user.namespace) } - let(:group) { create(:group) } + let_it_be(:system_hook) { create(:system_hook) } + let_it_be(:user) { create(:user) } + let(:project) { build(:project, namespace: user.namespace) } + let(:group) { build(:group) } let(:params) do { name: 'John Doe', username: 'jduser', email: 'jg@example.com', password: User.random_password } end @@ -145,6 +145,7 @@ RSpec.describe SystemHook do end it 'group member update hook' do + group = create(:group) group.add_guest(user) group.add_maintainer(user) diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb index 3441dfda7d6..fafca144cae 100644 --- a/spec/models/hooks/web_hook_log_spec.rb +++ b/spec/models/hooks/web_hook_log_spec.rb @@ -12,7 +12,7 @@ RSpec.describe WebHookLog do it { is_expected.to validate_presence_of(:web_hook) } describe '.recent' do - let(:hook) { create(:project_hook) } + let(:hook) { build(:project_hook) } it 'does not return web hook logs that are too old' do create(:web_hook_log, web_hook: hook, created_at: 10.days.ago) @@ -30,8 +30,10 @@ RSpec.describe WebHookLog do end describe '#save' do + let(:hook) { build(:project_hook) } + context 'with basic auth credentials' do - let(:web_hook_log) { build(:web_hook_log, url: 'http://test:123@example.com') } + let(:web_hook_log) { build(:web_hook_log, web_hook: hook, url: 'http://test:123@example.com') } subject { web_hook_log.save! } @@ -45,9 +47,9 @@ RSpec.describe WebHookLog do end context "with users' emails" do - let(:author) { create(:user) } - let(:user) { create(:user) } - let(:web_hook_log) { create(:web_hook_log, request_data: data) } + let(:author) { build(:user) } + let(:user) { build(:user) } + let(:web_hook_log) { create(:web_hook_log, web_hook: hook, request_data: data) } let(:data) do { user: { @@ -93,11 +95,12 @@ RSpec.describe WebHookLog do end describe '.delete_batch_for' do - let(:hook) { create(:project_hook) } + let_it_be(:hook) { build(:project_hook) } + let_it_be(:hook2) { build(:project_hook) } - before do + before_all do create_list(:web_hook_log, 3, web_hook: hook) - create_list(:web_hook_log, 3) + create_list(:web_hook_log, 3, web_hook: hook2) end subject { described_class.delete_batch_for(hook, batch_size: batch_size) } diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index da8c10b67a6..db854670cc3 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -34,6 +34,11 @@ RSpec.describe WebHook do it { is_expected.to allow_value({ 'x' => ('a' * 100) }).for(:url_variables) } it { is_expected.to allow_value({ 'foo' => 'bar', 'bar' => 'baz' }).for(:url_variables) } it { is_expected.to allow_value((1..20).to_h { ["k#{_1}", 'value'] }).for(:url_variables) } + it { is_expected.to allow_value({ 'MY-TOKEN' => 'bar' }).for(:url_variables) } + it { is_expected.to allow_value({ 'my_secr3t-token' => 'bar' }).for(:url_variables) } + it { is_expected.to allow_value({ 'x-y-z' => 'bar' }).for(:url_variables) } + it { is_expected.to allow_value({ 'x_y_z' => 'bar' }).for(:url_variables) } + it { is_expected.to allow_value({ 'f.o.o' => 'bar' }).for(:url_variables) } it { is_expected.not_to allow_value([]).for(:url_variables) } it { is_expected.not_to allow_value({ 'foo' => 1 }).for(:url_variables) } @@ -45,6 +50,10 @@ RSpec.describe WebHook do it { is_expected.not_to allow_value({ '' => 'foo' }).for(:url_variables) } it { is_expected.not_to allow_value({ '1foo' => 'foo' }).for(:url_variables) } it { is_expected.not_to allow_value((1..21).to_h { ["k#{_1}", 'value'] }).for(:url_variables) } + it { is_expected.not_to allow_value({ 'MY--TOKEN' => 'foo' }).for(:url_variables) } + it { is_expected.not_to allow_value({ 'MY__SECRET' => 'foo' }).for(:url_variables) } + it { is_expected.not_to allow_value({ 'x-_y' => 'foo' }).for(:url_variables) } + it { is_expected.not_to allow_value({ 'x..y' => 'foo' }).for(:url_variables) } end describe 'url' do @@ -83,7 +92,7 @@ RSpec.describe WebHook do subject { hook } before do - hook.url_variables = { 'one' => 'a', 'two' => 'b' } + hook.url_variables = { 'one' => 'a', 'two' => 'b', 'url' => 'http://example.com' } end it { is_expected.to allow_value('http://example.com').for(:url) } @@ -92,6 +101,8 @@ RSpec.describe WebHook do it { is_expected.to allow_value('http://example.com/{two}').for(:url) } it { is_expected.to allow_value('http://user:s3cret@example.com/{two}').for(:url) } it { is_expected.to allow_value('http://{one}:{two}@example.com').for(:url) } + it { is_expected.to allow_value('http://{one}').for(:url) } + it { is_expected.to allow_value('{url}').for(:url) } it { is_expected.not_to allow_value('http://example.com/{one}/{two}/{three}').for(:url) } it { is_expected.not_to allow_value('http://example.com/{foo}').for(:url) } @@ -113,24 +124,81 @@ RSpec.describe WebHook do end describe 'push_events_branch_filter' do - it { is_expected.to allow_values("good_branch_name", "another/good-branch_name").for(:push_events_branch_filter) } - it { is_expected.to allow_values("").for(:push_events_branch_filter) } - it { is_expected.not_to allow_values("bad branch name", "bad~branchname").for(:push_events_branch_filter) } - - it 'gets rid of whitespace' do - hook.push_events_branch_filter = ' branch ' - hook.save! + before do + subject.branch_filter_strategy = strategy + end + + context 'with "all branches" strategy' do + let(:strategy) { 'all_branches' } + + it { + is_expected.to allow_values( + "good_branch_name", + "another/good-branch_name", + "good branch name", + "good~branchname", + "good_branchname(", + "good_branchname[", + "" + ).for(:push_events_branch_filter) + } + end + + context 'with "wildcard" strategy' do + let(:strategy) { 'wildcard' } + + it { + is_expected.to allow_values( + "good_branch_name", + "another/good-branch_name", + "good_branch_name(", + "" + ).for(:push_events_branch_filter) + } + + it { + is_expected.not_to allow_values( + "bad branch name", + "bad~branchname", + "bad_branch_name[" + ).for(:push_events_branch_filter) + } + + it 'gets rid of whitespace' do + hook.push_events_branch_filter = ' branch ' + hook.save! + + expect(hook.push_events_branch_filter).to eq('branch') + end - expect(hook.push_events_branch_filter).to eq('branch') + it 'stores whitespace only as empty' do + hook.push_events_branch_filter = ' ' + hook.save! + expect(hook.push_events_branch_filter).to eq('') + end end - it 'stores whitespace only as empty' do - hook.push_events_branch_filter = ' ' - hook.save! + context 'with "regex" strategy' do + let(:strategy) { 'regex' } - expect(hook.push_events_branch_filter).to eq('') + it { + is_expected.to allow_values( + "good_branch_name", + "another/good-branch_name", + "good branch name", + "good~branch~name", + "" + ).for(:push_events_branch_filter) + } + + it { is_expected.not_to allow_values("bad_branch_name(", "bad_branch_name[").for(:push_events_branch_filter) } end end + + it "only consider these branch filter strategies are valid" do + expected_valid_types = %w[all_branches regex wildcard] + expect(described_class.branch_filter_strategies.keys).to contain_exactly(*expected_valid_types) + end end describe 'encrypted attributes' do @@ -141,7 +209,7 @@ RSpec.describe WebHook do describe '.web_hooks_disable_failed?' do it 'returns true when feature is enabled for parent' do - second_hook = build(:project_hook, project: create(:project)) + second_hook = build(:project_hook) stub_feature_flags(web_hooks_disable_failed: [false, second_hook.project]) expect(described_class.web_hooks_disable_failed?(hook)).to eq(false) @@ -242,7 +310,7 @@ RSpec.describe WebHook do end describe '#executable?' do - let(:web_hook) { create(:project_hook, project: project) } + let_it_be_with_reload(:web_hook) { create(:project_hook, project: project) } where(:recent_failures, :not_until, :executable) do [ @@ -389,7 +457,7 @@ RSpec.describe WebHook do end end - describe 'backoff!' do + describe '#backoff!' do context 'when we have not backed off before' do it 'does not disable the hook' do expect { hook.backoff! }.not_to change(hook, :executable?).from(true) @@ -400,6 +468,26 @@ RSpec.describe WebHook do end end + context 'when the recent failure value is the max value of a smallint' do + before do + hook.update!(recent_failures: 32767, disabled_until: 1.hour.ago) + end + + it 'reduces to MAX_FAILURES' do + expect { hook.backoff! }.to change(hook, :recent_failures).to(described_class::MAX_FAILURES) + end + end + + context 'when the recent failure value is MAX_FAILURES' do + before do + hook.update!(recent_failures: described_class::MAX_FAILURES, disabled_until: 1.hour.ago) + end + + it 'does not change recent_failures' do + expect { hook.backoff! }.not_to change(hook, :recent_failures) + end + end + context 'when we have exhausted the grace period' do before do hook.update!(recent_failures: described_class::FAILURE_THRESHOLD) @@ -459,11 +547,21 @@ RSpec.describe WebHook do end end - describe 'failed!' do + describe '#failed!' do it 'increments the failure count' do expect { hook.failed! }.to change(hook, :recent_failures).by(1) end + context 'when the recent failure value is the max value of a smallint' do + before do + hook.update!(recent_failures: 32767) + end + + it 'does not change recent_failures' do + expect { hook.failed! }.not_to change(hook, :recent_failures) + end + end + it 'does not update the hook if the the failure count exceeds the maximum value' do hook.recent_failures = described_class::MAX_FAILURES @@ -670,4 +768,14 @@ RSpec.describe WebHook do expect { described_class.new.update_last_failure }.not_to raise_error end end + + describe '#masked_token' do + it { expect(hook.masked_token).to be_nil } + + context 'with a token' do + let(:hook) { build(:project_hook, :token, project: project) } + + it { expect(hook.masked_token).to eq described_class::SECRET_MASK } + end + end end diff --git a/spec/models/incident_management/timeline_event_spec.rb b/spec/models/incident_management/timeline_event_spec.rb index d288cc1a75d..036f5affb87 100644 --- a/spec/models/incident_management/timeline_event_spec.rb +++ b/spec/models/incident_management/timeline_event_spec.rb @@ -27,6 +27,7 @@ RSpec.describe IncidentManagement::TimelineEvent do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:incident) } it { is_expected.to validate_presence_of(:note) } + it { is_expected.to validate_length_of(:note).is_at_most(280).on(:user_input) } it { is_expected.to validate_length_of(:note).is_at_most(10_000) } it { is_expected.to validate_length_of(:note_html).is_at_most(10_000) } it { is_expected.to validate_presence_of(:occurred_at) } diff --git a/spec/models/incident_management/timeline_event_tag_spec.rb b/spec/models/incident_management/timeline_event_tag_spec.rb index cff8ad8469f..1ec4fa30fb5 100644 --- a/spec/models/incident_management/timeline_event_tag_spec.rb +++ b/spec/models/incident_management/timeline_event_tag_spec.rb @@ -18,11 +18,46 @@ RSpec.describe IncidentManagement::TimelineEventTag do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_length_of(:name).is_at_most(255) } - it { is_expected.to validate_uniqueness_of(:name).scoped_to([:project_id]) } + it { is_expected.to validate_uniqueness_of(:name).scoped_to([:project_id]).ignoring_case_sensitivity } it { is_expected.to allow_value('Test tag 1').for(:name) } it { is_expected.not_to allow_value('Test tag, 1').for(:name) } it { is_expected.not_to allow_value('').for(:name) } it { is_expected.not_to allow_value('s' * 256).for(:name) } end + + describe '.pluck_names' do + it 'returns the names of the tags' do + tag1 = create(:incident_management_timeline_event_tag) + tag2 = create(:incident_management_timeline_event_tag) + + expect(described_class.pluck_names).to contain_exactly(tag1.name, tag2.name) + end + end + + describe 'constants' do + it { expect(described_class::START_TIME_TAG_NAME).to eq('Start time') } + it { expect(described_class::END_TIME_TAG_NAME).to eq('End time') } + end + + describe '#by_names scope' do + let_it_be(:project) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:tag1) { create(:incident_management_timeline_event_tag, name: 'Test tag 1', project: project) } + let_it_be(:tag2) { create(:incident_management_timeline_event_tag, name: 'Test tag 2', project: project) } + let_it_be(:tag3) { create(:incident_management_timeline_event_tag, name: 'Test tag 3', project: project2) } + + it 'returns two matching tags' do + expect(described_class.by_names(['Test tag 1', 'Test tag 2'])).to contain_exactly(tag1, tag2) + end + + it 'returns tags on the project' do + expect(project2.incident_management_timeline_event_tags.by_names(['Test tag 1', + 'Test tag 3'])).to contain_exactly(tag3) + end + + it 'returns one matching tag with case insensitive' do + expect(described_class.by_names(['tESt tAg 2'])).to contain_exactly(tag2) + end + end end diff --git a/spec/models/instance_metadata_spec.rb b/spec/models/instance_metadata_spec.rb index 5fc073c392d..46fd165e065 100644 --- a/spec/models/instance_metadata_spec.rb +++ b/spec/models/instance_metadata_spec.rb @@ -9,7 +9,8 @@ RSpec.describe InstanceMetadata do expect(subject).to have_attributes( version: Gitlab::VERSION, revision: Gitlab.revision, - kas: kind_of(::InstanceMetadata::Kas) + kas: kind_of(::InstanceMetadata::Kas), + enterprise: Gitlab.ee? ) end end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index baa3443b4c5..4938e1797af 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -15,6 +15,23 @@ RSpec.describe Integration do it { is_expected.to have_one(:jira_tracker_data).autosave(true).inverse_of(:integration).with_foreign_key(:integration_id).class_name('Integrations::JiraTrackerData') } end + describe 'default values' do + it { is_expected.to be_alert_events } + it { is_expected.to be_commit_events } + it { is_expected.to be_confidential_issues_events } + it { is_expected.to be_confidential_note_events } + it { is_expected.to be_issues_events } + it { is_expected.to be_job_events } + it { is_expected.to be_merge_requests_events } + it { is_expected.to be_note_events } + it { is_expected.to be_pipeline_events } + it { is_expected.to be_push_events } + it { is_expected.to be_tag_push_events } + it { is_expected.to be_wiki_page_events } + it { is_expected.not_to be_active } + it { expect(subject.category).to eq(:common) } + end + describe 'validations' do it { is_expected.to validate_presence_of(:type) } it { is_expected.to validate_exclusion_of(:type).in_array(described_class::BASE_CLASSES) } @@ -60,10 +77,10 @@ RSpec.describe Integration do describe 'Scopes' do describe '.third_party_wikis' do - let!(:integration1) { create(:jira_integration) } - let!(:integration2) { create(:redmine_integration) } - let!(:integration3) { create(:confluence_integration) } - let!(:integration4) { create(:shimo_integration) } + let!(:integration1) { create(:jira_integration, project: project) } + let!(:integration2) { create(:redmine_integration, project: project) } + let!(:integration3) { create(:confluence_integration, project: project) } + let!(:integration4) { create(:shimo_integration, project: project) } it 'returns the right group integration' do expect(described_class.third_party_wikis).to contain_exactly(integration3, integration4) @@ -89,7 +106,7 @@ RSpec.describe Integration do end describe '.by_type' do - let!(:integration1) { create(:jira_integration) } + let!(:integration1) { create(:jira_integration, project: project) } let!(:integration2) { create(:jira_integration) } let!(:integration3) { create(:redmine_integration) } @@ -110,7 +127,7 @@ RSpec.describe Integration do describe '.for_group' do let!(:integration1) { create(:jira_integration, project_id: nil, group_id: group.id) } - let!(:integration2) { create(:jira_integration) } + let!(:integration2) { create(:jira_integration, project: project) } it 'returns the right group integration' do expect(described_class.for_group(group)).to contain_exactly(integration1) @@ -217,9 +234,19 @@ RSpec.describe Integration do end end + describe '#chat?' do + it 'is true when integration is chat integration' do + expect(build(:mattermost_integration).chat?).to eq(true) + end + + it 'is false when integration is not chat integration' do + expect(build(:integration).chat?).to eq(false) + end + end + describe '.find_or_initialize_non_project_specific_integration' do let!(:integration_1) { create(:jira_integration, project_id: nil, group_id: group.id) } - let!(:integration_2) { create(:jira_integration) } + let!(:integration_2) { create(:jira_integration, project: project) } it 'returns the right integration' do expect(Integration.find_or_initialize_non_project_specific_integration('jira', group_id: group)) @@ -374,7 +401,7 @@ RSpec.describe Integration do context 'when data is stored in properties' do let(:properties) { data_params } let!(:integration) do - create(:jira_integration, :without_properties_callback, properties: properties.merge(additional: 'something')) + create(:jira_integration, :without_properties_callback, project: project, properties: properties.merge(additional: 'something')) end it_behaves_like 'integration creation from an integration' @@ -382,7 +409,7 @@ RSpec.describe Integration do context 'when data are stored in separated fields' do let(:integration) do - create(:jira_integration, data_params.merge(properties: {})) + create(:jira_integration, data_params.merge(properties: {}, project: project)) end it_behaves_like 'integration creation from an integration' @@ -391,7 +418,7 @@ RSpec.describe Integration do context 'when data are stored in both properties and separated fields' do let(:properties) { data_params } let(:integration) do - create(:jira_integration, :without_properties_callback, active: true, properties: properties).tap do |integration| + create(:jira_integration, :without_properties_callback, project: project, active: true, properties: properties).tap do |integration| create(:jira_tracker_data, data_params.merge(integration: integration)) end end @@ -1233,11 +1260,11 @@ RSpec.describe Integration do describe '#attributes' do it 'does not include properties' do - expect(create(:integration).attributes).not_to have_key('properties') + expect(build(:integration, project: project).attributes).not_to have_key('properties') end it 'can be used in assign_attributes without nullifying properties' do - record = create(:integration, :instance, properties: { url: generate(:url) }) + record = build(:integration, :instance, properties: { url: generate(:url) }) attrs = record.attributes @@ -1246,7 +1273,7 @@ RSpec.describe Integration do end describe '#dup' do - let(:original) { create(:integration, properties: { one: 1, two: 2, three: 3 }) } + let(:original) { build(:integration, project: project, properties: { one: 1, two: 2, three: 3 }) } it 'results in distinct ciphertexts, but identical properties' do copy = original.dup @@ -1259,7 +1286,7 @@ RSpec.describe Integration do end context 'when the model supports data-fields' do - let(:original) { create(:jira_integration, username: generate(:username), url: generate(:url)) } + let(:original) { build(:jira_integration, project: project, username: generate(:username), url: generate(:url)) } it 'creates distinct but identical data-fields' do copy = original.dup diff --git a/spec/models/integrations/assembla_spec.rb b/spec/models/integrations/assembla_spec.rb index 960dfea3dc4..e9f4274952d 100644 --- a/spec/models/integrations/assembla_spec.rb +++ b/spec/models/integrations/assembla_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' RSpec.describe Integrations::Assembla do include StubRequests + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { described_class.new } + end + describe "Execute" do let(:user) { create(:user) } let(:project) { create(:project, :repository) } diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index e92226d109f..1d2c90dad51 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -23,6 +23,10 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do ) end + it_behaves_like Integrations::BaseCi + + it_behaves_like Integrations::ResetSecretFields + include_context Integrations::EnableSslVerification describe 'Validations' do @@ -77,48 +81,6 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do end end - describe 'Callbacks' do - describe 'before_validation :reset_password' do - context 'when a password was previously set' do - it 'resets password if url changed' do - integration.bamboo_url = 'http://gitlab1.com' - - expect(integration).not_to be_valid - expect(integration.password).to be_nil - end - - it 'does not reset password if username changed' do - integration.username = 'some_name' - - expect(integration).to be_valid - expect(integration.password).to eq('password') - end - - it "does not reset password if new url is set together with password, even if it's the same password" do - integration.bamboo_url = 'http://gitlab_edited.com' - integration.password = 'password' - - expect(integration).to be_valid - expect(integration.password).to eq('password') - expect(integration.bamboo_url).to eq('http://gitlab_edited.com') - end - end - - it 'saves password if new url is set together with password when no password was previously set' do - integration.password = nil - - integration.bamboo_url = 'http://gitlab_edited.com' - integration.password = 'password' - integration.save! - - expect(integration.reload).to have_attributes( - bamboo_url: 'http://gitlab_edited.com', - password: 'password' - ) - end - end - end - describe '#execute' do it 'runs update and build action' do stub_update_and_build_request diff --git a/spec/models/integrations/base_chat_notification_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index eb503e501d6..b959ead2cae 100644 --- a/spec/models/integrations/base_chat_notification_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -3,11 +3,15 @@ require 'spec_helper' RSpec.describe Integrations::BaseChatNotification do + describe 'default values' do + it { expect(subject.category).to eq(:chat) } + end + describe 'validations' do before do allow(subject).to receive(:activated?).and_return(true) allow(subject).to receive(:default_channel_placeholder).and_return('placeholder') - allow(subject).to receive(:webhook_placeholder).and_return('placeholder') + allow(subject).to receive(:webhook_help).and_return('help') end it { is_expected.to validate_presence_of :webhook } @@ -19,7 +23,7 @@ RSpec.describe Integrations::BaseChatNotification do let_it_be(:project) { create(:project, :repository) } - let(:user) { create(:user) } + let(:user) { build_stubbed(:user) } let(:webhook_url) { 'https://example.gitlab.com/' } let(:data) { Gitlab::DataBuilder::Push.build_sample(subject.project, user) } @@ -44,7 +48,7 @@ RSpec.describe Integrations::BaseChatNotification do context 'with an empty repository' do it 'returns true' do - subject.project = create(:project, :empty_repo) + subject.project = build_stubbed(:project, :empty_repo) expect(chat_integration).to receive(:notify).and_return(true) expect(chat_integration.execute(data)).to be true @@ -61,9 +65,9 @@ RSpec.describe Integrations::BaseChatNotification do end context 'when the data object has a label' do - let_it_be(:label) { create(:label, name: 'Bug') } - let_it_be(:label_2) { create(:label, name: 'Community contribution') } - let_it_be(:label_3) { create(:label, name: 'Backend') } + let_it_be(:label) { create(:label, project: project, name: 'Bug') } + let_it_be(:label_2) { create(:label, project: project, name: 'Community contribution') } + let_it_be(:label_3) { create(:label, project: project, name: 'Backend') } let_it_be(:issue) { create(:labeled_issue, project: project, labels: [label, label_2, label_3]) } let_it_be(:note) { create(:note, noteable: issue, project: project) } @@ -93,7 +97,7 @@ RSpec.describe Integrations::BaseChatNotification do it_behaves_like 'notifies the chat integration' context 'MergeRequest events' do - let(:data) { create(:merge_request, labels: [label]).to_hook_data(user) } + let(:data) { build_stubbed(:merge_request, source_project: project, labels: [label]).to_hook_data(user) } it_behaves_like 'notifies the chat integration' end @@ -280,9 +284,9 @@ RSpec.describe Integrations::BaseChatNotification do end end - describe '#webhook_placeholder' do + describe '#webhook_help' do it 'raises an error' do - expect { subject.webhook_placeholder }.to raise_error(NotImplementedError) + expect { subject.webhook_help }.to raise_error(NotImplementedError) end end diff --git a/spec/models/integrations/base_issue_tracker_spec.rb b/spec/models/integrations/base_issue_tracker_spec.rb index 37f7d99717c..e1a764cd7cb 100644 --- a/spec/models/integrations/base_issue_tracker_spec.rb +++ b/spec/models/integrations/base_issue_tracker_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Integrations::BaseIssueTracker do let_it_be_with_refind(:project) { create :project } + describe 'default values' do + it { expect(subject.category).to eq(:issue_tracker) } + end + describe 'Validations' do describe 'only one issue tracker per project' do before do diff --git a/spec/models/integrations/base_third_party_wiki_spec.rb b/spec/models/integrations/base_third_party_wiki_spec.rb index 11e044c2a18..dbead636cb9 100644 --- a/spec/models/integrations/base_third_party_wiki_spec.rb +++ b/spec/models/integrations/base_third_party_wiki_spec.rb @@ -3,6 +3,10 @@ require 'spec_helper' RSpec.describe Integrations::BaseThirdPartyWiki do + describe 'default values' do + it { expect(subject.category).to eq(:third_party_wiki) } + end + describe 'Validations' do let_it_be_with_reload(:project) { create(:project) } diff --git a/spec/models/integrations/buildkite_spec.rb b/spec/models/integrations/buildkite_spec.rb index c720dc6d418..5f62c68bd2b 100644 --- a/spec/models/integrations/buildkite_spec.rb +++ b/spec/models/integrations/buildkite_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers include StubRequests - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } subject(:integration) do described_class.create!( @@ -18,6 +18,10 @@ RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do ) end + it_behaves_like Integrations::BaseCi + + it_behaves_like Integrations::ResetSecretFields + it_behaves_like Integrations::HasWebHook do let(:hook_url) { 'https://webhook.buildkite.com/deliver/{webhook_token}' } end diff --git a/spec/models/integrations/campfire_spec.rb b/spec/models/integrations/campfire_spec.rb index a6bcd22b6f6..ae923cd38fc 100644 --- a/spec/models/integrations/campfire_spec.rb +++ b/spec/models/integrations/campfire_spec.rb @@ -34,8 +34,8 @@ RSpec.describe Integrations::Campfire do end describe "#execute" do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + let(:user) { build_stubbed(:user) } + let(:project) { build_stubbed(:project, :repository) } before do @campfire_integration = described_class.new diff --git a/spec/models/integrations/chat_message/pipeline_message_spec.rb b/spec/models/integrations/chat_message/pipeline_message_spec.rb index a63cc0b6d83..f3388853b37 100644 --- a/spec/models/integrations/chat_message/pipeline_message_spec.rb +++ b/spec/models/integrations/chat_message/pipeline_message_spec.rb @@ -44,13 +44,18 @@ RSpec.describe Integrations::ChatMessage::PipelineMessage do before do test_commit = double("A test commit", committer: args[:user], title: "A test commit message") - test_project = double("A test project", commit_by: test_commit, name: args[:project][:name], web_url: args[:project][:web_url]) + test_project = build(:project, name: args[:project][:name]) + + allow(test_project).to receive(:commit_by).and_return(test_commit) + allow(test_project).to receive(:web_url).and_return(args[:project][:web_url]) allow(test_project).to receive(:avatar_url).with(no_args).and_return("/avatar") allow(test_project).to receive(:avatar_url).with(only_path: false).and_return(args[:project][:avatar_url]) allow(Project).to receive(:find) { test_project } - test_pipeline = double("A test pipeline", - has_yaml_errors?: has_yaml_errors, yaml_errors: "yaml error description here") + test_pipeline = build(:ci_empty_pipeline, name: 'Build pipeline') + + allow(test_pipeline).to receive(:has_yaml_errors?).and_return(has_yaml_errors) + allow(test_pipeline).to receive(:yaml_errors).and_return("yaml error description here") allow(Ci::Pipeline).to receive(:find) { test_pipeline } allow(Gitlab::UrlBuilder).to receive(:build).with(test_commit).and_return("http://example.com/commit") @@ -69,6 +74,24 @@ RSpec.describe Integrations::ChatMessage::PipelineMessage do ) end + it 'returns pipeline name' do + name_field = subject.attachments.first[:fields].find { |a| a[:title] == 'Pipeline name' } + + expect(name_field[:value]).to eq('Build pipeline') + end + + context 'when pipeline_name feature flag is disabled' do + before do + stub_feature_flags(pipeline_name: false) + end + + it 'does not return pipeline name' do + name_field = subject.attachments.first[:fields].find { |a| a[:title] == 'Pipeline name' } + + expect(name_field).to be nil + end + end + context "when the pipeline failed" do before do args[:object_attributes][:status] = 'failed' @@ -204,8 +227,8 @@ RSpec.describe Integrations::ChatMessage::PipelineMessage do expect(subject.attachments.first[:title_link]).to eq("http://example.gitlab.com/-/pipelines/123") end - it "returns two attachment fields" do - expect(subject.attachments.first[:fields].count).to eq(2) + it "returns three attachment fields" do + expect(subject.attachments.first[:fields].count).to eq(3) end it "returns the commit message as the attachment's second field property" do @@ -232,8 +255,8 @@ RSpec.describe Integrations::ChatMessage::PipelineMessage do ] end - it "returns four attachment fields" do - expect(subject.attachments.first[:fields].count).to eq(4) + it "returns five attachment fields" do + expect(subject.attachments.first[:fields].count).to eq(5) end it "returns the stage name and link to the 'Failed jobs' tab on the pipeline's page as the attachment's third field property" do @@ -337,8 +360,8 @@ RSpec.describe Integrations::ChatMessage::PipelineMessage do context "when the CI config file contains a YAML error" do let(:has_yaml_errors) { true } - it "returns three attachment fields" do - expect(subject.attachments.first[:fields].count).to eq(3) + it "returns four attachment fields" do + expect(subject.attachments.first[:fields].count).to eq(4) end it "returns the YAML error deatils as the attachment's third field property" do diff --git a/spec/models/integrations/confluence_spec.rb b/spec/models/integrations/confluence_spec.rb index e2f9316bc95..999a532527d 100644 --- a/spec/models/integrations/confluence_spec.rb +++ b/spec/models/integrations/confluence_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Integrations::Confluence do + let_it_be(:project) { create(:project) } + describe 'Validations' do before do subject.active = active @@ -40,7 +42,6 @@ RSpec.describe Integrations::Confluence do describe '#help' do it 'can correctly return a link to the project wiki when active' do - project = create(:project) subject.project = project subject.active = true @@ -62,8 +63,6 @@ RSpec.describe Integrations::Confluence do end describe 'Caching has_confluence on project_settings' do - let(:project) { create(:project) } - subject { project.project_setting.has_confluence? } it 'sets the property to true when integration is active' do diff --git a/spec/models/integrations/datadog_spec.rb b/spec/models/integrations/datadog_spec.rb index 71a5bbc4db1..65ecd9bee83 100644 --- a/spec/models/integrations/datadog_spec.rb +++ b/spec/models/integrations/datadog_spec.rb @@ -73,7 +73,15 @@ RSpec.describe Integrations::Datadog do it { is_expected.to validate_presence_of(:datadog_site) } it { is_expected.not_to validate_presence_of(:api_url) } + it { is_expected.to allow_value('data-dog-hq.com').for(:datadog_site) } + it { is_expected.to allow_value('dataDOG.com').for(:datadog_site) } it { is_expected.not_to allow_value('datadog hq.com').for(:datadog_site) } + it { is_expected.not_to allow_value('-datadoghq.com').for(:datadog_site) } + it { is_expected.not_to allow_value('.datadoghq.com').for(:datadog_site) } + it { is_expected.not_to allow_value('datadoghq.com_').for(:datadog_site) } + it { is_expected.not_to allow_value('data-dog').for(:datadog_site) } + it { is_expected.not_to allow_value('datadoghq.com-').for(:datadog_site) } + it { is_expected.not_to allow_value('datadoghq.com.').for(:datadog_site) } end context 'with custom api_url' do diff --git a/spec/models/integrations/discord_spec.rb b/spec/models/integrations/discord_spec.rb index eb90acc73be..138a56d1872 100644 --- a/spec/models/integrations/discord_spec.rb +++ b/spec/models/integrations/discord_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Integrations::Discord do let_it_be(:project) { create(:project, :repository) } - let(:user) { create(:user) } + let(:user) { build_stubbed(:user) } let(:webhook_url) { "https://example.gitlab.com/" } let(:sample_data) do Gitlab::DataBuilder::Push.build_sample(project, user) diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb index f3203a6e69d..6ff6888e0d3 100644 --- a/spec/models/integrations/drone_ci_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do subject(:integration) { described_class.new } + let_it_be(:project) { create(:project, :repository, name: 'project') } + + it_behaves_like Integrations::BaseCi + it_behaves_like Integrations::ResetSecretFields do let(:integration) { subject } end @@ -43,7 +47,6 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do ) end - let(:project) { create(:project, :repository, name: 'project') } let(:path) { project.full_path } let(:drone_url) { 'http://drone.example.com' } let(:sha) { '2ab7834c' } @@ -192,7 +195,7 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do describe "execute" do include_context :drone_ci_integration - let(:user) { create(:user, username: 'username') } + let(:user) { build(:user, username: 'username') } let(:push_sample_data) do Gitlab::DataBuilder::Push.build_sample(project, user) end diff --git a/spec/models/integrations/emails_on_push_spec.rb b/spec/models/integrations/emails_on_push_spec.rb index 15aa105e379..b3fe6bf9506 100644 --- a/spec/models/integrations/emails_on_push_spec.rb +++ b/spec/models/integrations/emails_on_push_spec.rb @@ -87,8 +87,8 @@ RSpec.describe Integrations::EmailsOnPush do end describe '#execute' do + let_it_be(:project) { create(:project, :repository) } let(:push_data) { { object_kind: 'push' } } - let(:project) { create(:project, :repository) } let(:integration) { create(:emails_on_push_integration, project: project) } let(:recipients) { 'test@gitlab.com' } diff --git a/spec/models/integrations/hangouts_chat_spec.rb b/spec/models/integrations/hangouts_chat_spec.rb index 828bcdf5d8f..288478b494e 100644 --- a/spec/models/integrations/hangouts_chat_spec.rb +++ b/spec/models/integrations/hangouts_chat_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Integrations::HangoutsChat do end context 'with issue events' do - let(:issues_sample_data) { create(:issue).to_hook_data(user) } + let(:issues_sample_data) { create(:issue, project: project).to_hook_data(user) } it "adds thread key for issue events" do expect(chat_integration.execute(issues_sample_data)).to be(true) @@ -58,7 +58,7 @@ RSpec.describe Integrations::HangoutsChat do end context 'with merge events' do - let(:merge_sample_data) { create(:merge_request).to_hook_data(user) } + let(:merge_sample_data) { create(:merge_request, source_project: project).to_hook_data(user) } it "adds thread key for merge events" do expect(chat_integration.execute(merge_sample_data)).to be(true) @@ -71,7 +71,7 @@ RSpec.describe Integrations::HangoutsChat do context 'with wiki page events' do let(:wiki_page_sample_data) do - Gitlab::DataBuilder::WikiPage.build(create(:wiki_page, message: 'foo'), user, 'create') + Gitlab::DataBuilder::WikiPage.build(create(:wiki_page, project: project, message: 'foo'), user, 'create') end it "adds thread key for wiki page events" do diff --git a/spec/models/integrations/harbor_spec.rb b/spec/models/integrations/harbor_spec.rb index 9ab37a92e89..b4580028112 100644 --- a/spec/models/integrations/harbor_spec.rb +++ b/spec/models/integrations/harbor_spec.rb @@ -70,7 +70,7 @@ RSpec.describe Integrations::Harbor do end context 'ci variables' do - let(:harbor_integration) { create(:harbor_integration) } + let(:harbor_integration) { build_stubbed(:harbor_integration) } it 'returns vars when harbor_integration is activated' do ci_vars = [ @@ -85,9 +85,12 @@ RSpec.describe Integrations::Harbor do expect(harbor_integration.ci_variables).to match_array(ci_vars) end - it 'returns [] when harbor_integration is inactive' do - harbor_integration.update!(active: false) - expect(harbor_integration.ci_variables).to match_array([]) + context 'when harbor_integration is inactive' do + let(:harbor_integration) { build_stubbed(:harbor_integration, active: false) } + + it 'returns []' do + expect(harbor_integration.ci_variables).to match_array([]) + end end context 'with robot username' do diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb index 200de1305e2..0264982f0dc 100644 --- a/spec/models/integrations/jenkins_spec.rb +++ b/spec/models/integrations/jenkins_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Integrations::Jenkins do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } let(:jenkins_integration) { described_class.new(jenkins_params) } let(:jenkins_url) { 'http://jenkins.example.com/' } let(:jenkins_hook_url) { jenkins_url + 'project/my_project' } @@ -23,6 +23,12 @@ RSpec.describe Integrations::Jenkins do } end + it_behaves_like Integrations::BaseCi + + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { jenkins_integration } + end + include_context Integrations::EnableSslVerification do let(:integration) { jenkins_integration } end @@ -38,7 +44,7 @@ RSpec.describe Integrations::Jenkins do expect(jenkins_integration.tag_push_events).to eq(false) end - describe 'username validation' do + describe 'Validations' do let(:jenkins_integration) do described_class.create!( active: active, @@ -57,28 +63,44 @@ RSpec.describe Integrations::Jenkins do context 'when the integration is active' do let(:active) { true } - context 'when password was not touched' do - before do - allow(subject).to receive(:password_touched?).and_return(false) + describe '#username' do + context 'when password was not touched' do + before do + allow(subject).to receive(:password_touched?).and_return(false) + end + + it { is_expected.not_to validate_presence_of :username } end - it { is_expected.not_to validate_presence_of :username } - end + context 'when password was touched' do + before do + allow(subject).to receive(:password_touched?).and_return(true) + end - context 'when password was touched' do - before do - allow(subject).to receive(:password_touched?).and_return(true) + it { is_expected.to validate_presence_of :username } end - it { is_expected.to validate_presence_of :username } + context 'when password is blank' do + it 'does not validate the username' do + expect(subject).not_to validate_presence_of :username + + subject.password = '' + subject.save! + end + end end - context 'when password is blank' do - it 'does not validate the username' do - expect(subject).not_to validate_presence_of :username + describe '#password' do + it 'does not validate the presence of password if username is nil' do + subject.username = nil + + expect(subject).not_to validate_presence_of(:password) + end + + it 'validates the presence of password if username is present' do + subject.username = 'john' - subject.password = '' - subject.save! + expect(subject).to validate_presence_of(:password) end end end @@ -87,6 +109,7 @@ RSpec.describe Integrations::Jenkins do let(:active) { false } it { is_expected.not_to validate_presence_of :username } + it { is_expected.not_to validate_presence_of :password } end end @@ -144,8 +167,7 @@ RSpec.describe Integrations::Jenkins do describe '#test' do it 'returns the right status' do - user = create(:user, username: 'username') - project = create(:project, name: 'project') + user = build(:user, username: 'username') push_sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) jenkins_integration = described_class.create!(jenkins_params) stub_request(:post, jenkins_hook_url).with(headers: { 'Authorization' => jenkins_authorization }) @@ -157,9 +179,9 @@ RSpec.describe Integrations::Jenkins do end describe '#execute' do - let(:user) { create(:user, username: 'username') } - let(:namespace) { create(:group, :private) } - let(:project) { create(:project, :private, name: 'project', namespace: namespace) } + let(:user) { build(:user, username: 'username') } + let_it_be(:namespace) { create(:group, :private) } + let_it_be(:project) { create(:project, :private, name: 'project', namespace: namespace) } let(:push_sample_data) { Gitlab::DataBuilder::Push.build_sample(project, user) } let(:jenkins_integration) { described_class.create!(jenkins_params) } @@ -191,82 +213,4 @@ RSpec.describe Integrations::Jenkins do ).to have_been_made.once end end - - describe 'Stored password invalidation' do - let(:project) { create(:project) } - - context 'when a password was previously set' do - let(:jenkins_integration) do - described_class.create!( - project: project, - properties: { - jenkins_url: 'http://jenkins.example.com/', - username: 'jenkins', - password: 'password' - } - ) - end - - it 'resets password if url changed' do - jenkins_integration.jenkins_url = 'http://jenkins-edited.example.com/' - jenkins_integration.valid? - - expect(jenkins_integration.password).to be_nil - end - - it 'resets password if username is blank' do - jenkins_integration.username = '' - jenkins_integration.valid? - - expect(jenkins_integration.password).to be_nil - end - - it 'does not reset password if username changed' do - jenkins_integration.username = 'some_name' - jenkins_integration.valid? - - expect(jenkins_integration.password).to eq('password') - end - - it 'does not reset password if new url is set together with password, even if it\'s the same password' do - jenkins_integration.jenkins_url = 'http://jenkins_edited.example.com/' - jenkins_integration.password = 'password' - jenkins_integration.valid? - - expect(jenkins_integration.password).to eq('password') - expect(jenkins_integration.jenkins_url).to eq('http://jenkins_edited.example.com/') - end - - it 'resets password if url changed, even if setter called multiple times' do - jenkins_integration.jenkins_url = 'http://jenkins1.example.com/' - jenkins_integration.jenkins_url = 'http://jenkins1.example.com/' - jenkins_integration.valid? - - expect(jenkins_integration.password).to be_nil - end - end - - context 'when no password was previously set' do - let(:jenkins_integration) do - described_class.create!( - project: create(:project), - properties: { - jenkins_url: 'http://jenkins.example.com/', - username: 'jenkins' - } - ) - end - - it 'saves password if new url is set together with password' do - jenkins_integration.jenkins_url = 'http://jenkins_edited.example.com/' - jenkins_integration.password = 'password' - jenkins_integration.save! - - expect(jenkins_integration.reload).to have_attributes( - jenkins_url: 'http://jenkins_edited.example.com/', - password: 'password' - ) - end - end - end end diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 9f928442b28..819dad9d46d 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -26,7 +26,7 @@ RSpec.describe Integrations::Jira do end before do - WebMock.stub_request(:get, /serverInfo/).to_return(body: server_info_results.to_json ) + WebMock.stub_request(:get, /serverInfo/).to_return(body: server_info_results.to_json) end it_behaves_like Integrations::ResetSecretFields do @@ -162,7 +162,7 @@ RSpec.describe Integrations::Jira do end describe '#fields' do - let(:integration) { create(:jira_integration) } + let(:integration) { jira_integration } subject(:fields) { integration.fields } @@ -172,7 +172,7 @@ RSpec.describe Integrations::Jira do end describe '#sections' do - let(:integration) { create(:jira_integration) } + let(:integration) { jira_integration } subject(:sections) { integration.sections.map { |s| s[:type] } } @@ -332,28 +332,7 @@ RSpec.describe Integrations::Jira do # we need to make sure we are able to read both from properties and jira_tracker_data table # TODO: change this as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 context 'overriding properties' do - let(:access_params) do - { url: url, api_url: api_url, username: username, password: password, - jira_issue_transition_id: transition_id } - end - - let(:data_params) do - { - url: url, api_url: api_url, - username: username, password: password, - jira_issue_transition_id: transition_id - } - end - shared_examples 'handles jira fields' do - let(:data_params) do - { - url: url, api_url: api_url, - username: username, password: password, - jira_issue_transition_id: transition_id - } - end - context 'reading data' do it 'reads data correctly' do expect(integration.url).to eq(url) @@ -449,32 +428,40 @@ RSpec.describe Integrations::Jira do end # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 - context 'when data are stored in properties' do - let(:properties) { data_params } - let!(:integration) do - create(:jira_integration, :without_properties_callback, properties: properties.merge(additional: 'something')) + context 'with properties' do + let(:data_params) do + { + url: url, api_url: api_url, + username: username, password: password, + jira_issue_transition_id: transition_id + } end - it_behaves_like 'handles jira fields' - end + context 'when data are stored in properties' do + let(:integration) do + create(:jira_integration, :without_properties_callback, project: project, properties: data_params.merge(additional: 'something')) + end - context 'when data are stored in separated fields' do - let(:integration) do - create(:jira_integration, data_params.merge(properties: {})) + it_behaves_like 'handles jira fields' end - it_behaves_like 'handles jira fields' - end - - context 'when data are stored in both properties and separated fields' do - let(:properties) { data_params } - let(:integration) do - create(:jira_integration, :without_properties_callback, properties: properties).tap do |integration| - create(:jira_tracker_data, data_params.merge(integration: integration)) + context 'when data are stored in separated fields' do + let(:integration) do + create(:jira_integration, data_params.merge(properties: {}, project: project)) end + + it_behaves_like 'handles jira fields' end - it_behaves_like 'handles jira fields' + context 'when data are stored in both properties and separated fields' do + let(:integration) do + create(:jira_integration, :without_properties_callback, properties: data_params, project: project).tap do |integration| + create(:jira_tracker_data, data_params.merge(integration: integration)) + end + end + + it_behaves_like 'handles jira fields' + end end end @@ -872,7 +859,7 @@ RSpec.describe Integrations::Jira do end context 'when resource is a merge request' do - let(:resource) { create(:merge_request) } + let_it_be(:resource) { create(:merge_request, source_project: project) } let(:commit_id) { resource.diff_head_sha } it_behaves_like 'close_issue' @@ -1084,7 +1071,7 @@ RSpec.describe Integrations::Jira do end it 'removes trailing slashes from url' do - integration = described_class.new(url: 'http://jira.test.com/path/') + integration = described_class.new(url: 'http://jira.test.com/path/', project: project) expect(integration.url).to eq('http://jira.test.com/path') end @@ -1105,7 +1092,7 @@ RSpec.describe Integrations::Jira do end context 'generating external URLs' do - let(:integration) { described_class.new(url: 'http://jira.test.com/path/') } + let(:integration) { described_class.new(url: 'http://jira.test.com/path/', project: project) } describe '#web_url' do it 'handles paths, slashes, and query string' do diff --git a/spec/models/integrations/mattermost_slash_commands_spec.rb b/spec/models/integrations/mattermost_slash_commands_spec.rb index b6abe00469b..070adb9ba93 100644 --- a/spec/models/integrations/mattermost_slash_commands_spec.rb +++ b/spec/models/integrations/mattermost_slash_commands_spec.rb @@ -6,9 +6,9 @@ RSpec.describe Integrations::MattermostSlashCommands do it_behaves_like Integrations::BaseSlashCommands describe 'Mattermost API' do - let(:project) { create(:project) } + let_it_be_with_reload(:project) { create(:project) } let(:integration) { project.build_mattermost_slash_commands_integration } - let(:user) { create(:user) } + let(:user) { build_stubbed(:user) } before do session = ::Mattermost::Session.new(nil) diff --git a/spec/models/integrations/microsoft_teams_spec.rb b/spec/models/integrations/microsoft_teams_spec.rb index b6de2bb7176..c61cc732372 100644 --- a/spec/models/integrations/microsoft_teams_spec.rb +++ b/spec/models/integrations/microsoft_teams_spec.rb @@ -17,35 +17,8 @@ RSpec.describe Integrations::MicrosoftTeams do let(:chat_integration) { described_class.new } let(:webhook_url) { 'https://example.gitlab.com/' } - describe 'Validations' do - context 'when integration is active' do - before do - subject.active = true - end - - it { is_expected.to validate_presence_of(:webhook) } - - it_behaves_like 'issue tracker integration URL attribute', :webhook - end - - context 'when integration is inactive' do - before do - subject.active = false - end - - it { is_expected.not_to validate_presence_of(:webhook) } - end - end - - describe '.supported_events' do - it 'does not support deployment_events' do - expect(described_class.supported_events).not_to include('deployment') - end - end - describe "#execute" do - let(:user) { create(:user) } - + let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :repository, :wiki_repo) } before do @@ -145,8 +118,8 @@ RSpec.describe Integrations::MicrosoftTeams do end describe "Note events" do - let(:user) { create(:user) } - let(:project) { create(:project, :repository, creator: user) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository, creator: user) } before do allow(chat_integration).to receive_messages( @@ -221,8 +194,7 @@ RSpec.describe Integrations::MicrosoftTeams do end describe 'Pipeline events' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + let_it_be_with_reload(:project) { create(:project, :repository) } let(:pipeline) do create(:ci_pipeline, diff --git a/spec/models/integrations/mock_ci_spec.rb b/spec/models/integrations/mock_ci_spec.rb index d29c63b3a97..83954812bfe 100644 --- a/spec/models/integrations/mock_ci_spec.rb +++ b/spec/models/integrations/mock_ci_spec.rb @@ -7,6 +7,8 @@ RSpec.describe Integrations::MockCi do subject(:integration) { described_class.new(project: project, mock_service_url: generate(:url)) } + it_behaves_like Integrations::BaseCi + include_context Integrations::EnableSslVerification describe '#commit_status' do diff --git a/spec/models/integrations/packagist_spec.rb b/spec/models/integrations/packagist_spec.rb index e078debd126..e00de0f7418 100644 --- a/spec/models/integrations/packagist_spec.rb +++ b/spec/models/integrations/packagist_spec.rb @@ -3,50 +3,76 @@ require 'spec_helper' RSpec.describe Integrations::Packagist do - let(:packagist_params) do - { - active: true, - project: project, - properties: { - username: packagist_username, - token: packagist_token, - server: packagist_server - } - } - end - - let(:packagist_hook_url) do - "#{packagist_server}/api/update-package?username=#{packagist_username}&apiToken=#{packagist_token}" - end - - let(:packagist_token) { 'verySecret' } - let(:packagist_username) { 'theUser' } - let(:packagist_server) { 'https://packagist.example.com' } - let(:project) { create(:project) } - it_behaves_like Integrations::HasWebHook do - let(:integration) { described_class.new(packagist_params) } - let(:hook_url) { "#{packagist_server}/api/update-package?username={username}&apiToken={token}" } + let_it_be(:project) { create(:project) } + + let(:integration) { build(:packagist_integration, project: project) } + let(:hook_url) { "#{integration.server}/api/update-package?username={username}&apiToken={token}" } end it_behaves_like Integrations::ResetSecretFields do - let(:integration) { described_class.new(packagist_params) } + let(:integration) { build(:packagist_integration) } end describe '#execute' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:push_sample_data) { Gitlab::DataBuilder::Push.build_sample(project, user) } - let(:packagist_integration) { described_class.create!(packagist_params) } + let(:project) { build(:project) } + let(:integration) { build(:packagist_integration, project: project) } + + let(:packagist_hook_url) do + "#{integration.server}/api/update-package?username=#{integration.username}&apiToken=#{integration.token}" + end before do stub_request(:post, packagist_hook_url) end it 'calls Packagist API' do - packagist_integration.execute(push_sample_data) + user = create(:user) + push_sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) + integration.execute(push_sample_data) expect(a_request(:post, packagist_hook_url)).to have_been_made.once end end + + describe '#test' do + let(:integration) { build(:packagist_integration) } + let(:test_data) { { foo: 'bar' } } + + subject(:result) { integration.test(test_data) } + + context 'when test request executes without errors' do + before do + allow(integration).to receive(:execute).with(test_data).and_return( + ServiceResponse.success(message: 'success message', payload: { http_status: http_status }) + ) + end + + context 'when response is a 200' do + let(:http_status) { 200 } + + it 'return failure result' do + is_expected.to eq(success: false, result: 'success message') + end + end + + context 'when response is a 202' do + let(:http_status) { 202 } + + it 'return success result' do + is_expected.to eq(success: true, result: 'success message') + end + end + end + + context 'when test request executes with errors' do + before do + allow(integration).to receive(:execute).with(test_data).and_raise(StandardError, 'error message') + end + + it 'return failure result' do + is_expected.to eq(success: false, result: 'error message') + end + end + end end diff --git a/spec/models/integrations/pipelines_email_spec.rb b/spec/models/integrations/pipelines_email_spec.rb index d70f104b965..37a3849a768 100644 --- a/spec/models/integrations/pipelines_email_spec.rb +++ b/spec/models/integrations/pipelines_email_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Integrations::PipelinesEmail, :mailer do ) end - let(:project) { create(:project, :repository) } + let_it_be_with_reload(:project) { create(:project, :repository) } let(:recipients) { 'test@gitlab.com' } let(:receivers) { [recipients] } diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb index 3971511872b..3c3850854b3 100644 --- a/spec/models/integrations/prometheus_spec.rb +++ b/spec/models/integrations/prometheus_spec.rb @@ -12,6 +12,8 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, let(:integration) { project.prometheus_integration } + it_behaves_like Integrations::BaseMonitoring + context 'redirects' do it 'does not follow redirects' do redirect_to = 'https://redirected.example.com' @@ -217,7 +219,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, expect(integration.prometheus_client).to be_nil end - context 'with self monitoring project and internal Prometheus URL' do + context 'with self-monitoring project and internal Prometheus URL' do before do stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) stub_application_setting(self_monitoring_project_id: project.id) @@ -308,7 +310,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, end context 'cluster belongs to project' do - let(:cluster) { create(:cluster, projects: [project]) } + let_it_be(:cluster) { create(:cluster, projects: [project]) } it 'returns true' do expect(integration.prometheus_available?).to be(true) @@ -319,7 +321,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, let_it_be(:group) { create(:group) } let(:project) { create(:project, :with_prometheus_integration, group: group) } - let(:cluster) { create(:cluster_for_group, groups: [group]) } + let_it_be(:cluster) { create(:cluster_for_group, groups: [group]) } it 'returns true' do expect(integration.prometheus_available?).to be(true) diff --git a/spec/models/integrations/pushover_spec.rb b/spec/models/integrations/pushover_spec.rb index 716a00c5bcf..8286fd20669 100644 --- a/spec/models/integrations/pushover_spec.rb +++ b/spec/models/integrations/pushover_spec.rb @@ -29,8 +29,8 @@ RSpec.describe Integrations::Pushover do describe 'Execute' do let(:pushover) { described_class.new } - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + let(:user) { build_stubbed(:user) } + let(:project) { build_stubbed(:project, :repository) } let(:sample_data) do Gitlab::DataBuilder::Push.build_sample(project, user) end diff --git a/spec/models/integrations/shimo_spec.rb b/spec/models/integrations/shimo_spec.rb index 41f3f3c0c16..be626012ab2 100644 --- a/spec/models/integrations/shimo_spec.rb +++ b/spec/models/integrations/shimo_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe ::Integrations::Shimo do describe '#fields' do - let(:shimo_integration) { create(:shimo_integration) } + let(:shimo_integration) { build(:shimo_integration) } it 'returns custom fields' do expect(shimo_integration.fields.pluck(:name)).to eq(%w[external_wiki_url]) @@ -12,7 +12,7 @@ RSpec.describe ::Integrations::Shimo do end describe '#create' do - let(:project) { create(:project, :repository) } + let_it_be(:project) { create(:project, :repository) } let(:external_wiki_url) { 'https://shimo.example.com/desktop' } let(:params) { { active: true, project: project, external_wiki_url: external_wiki_url } } @@ -40,7 +40,7 @@ RSpec.describe ::Integrations::Shimo do end describe 'Caching has_shimo on project_settings' do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } subject { project.project_setting.has_shimo? } diff --git a/spec/models/integrations/slack_slash_commands_spec.rb b/spec/models/integrations/slack_slash_commands_spec.rb index ff89d2c6a40..22cbaa777cd 100644 --- a/spec/models/integrations/slack_slash_commands_spec.rb +++ b/spec/models/integrations/slack_slash_commands_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Integrations::SlackSlashCommands do describe '#trigger' do context 'when an auth url is generated' do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } let(:params) do { team_domain: 'http://domain.tld', diff --git a/spec/models/integrations/slack_spec.rb b/spec/models/integrations/slack_spec.rb index ed282f1d39d..a12bc7f4831 100644 --- a/spec/models/integrations/slack_spec.rb +++ b/spec/models/integrations/slack_spec.rb @@ -3,142 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::Slack do - it_behaves_like Integrations::SlackMattermostNotifier, "Slack" - - describe '#execute' do - let(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all', project_id: project.id) } - let(:project) { create_default(:project, :repository, :wiki_repo) } - - before do - stub_request(:post, slack_integration.webhook) - end - - it 'uses only known events', :aggregate_failures do - described_class::SUPPORTED_EVENTS_FOR_USAGE_LOG.each do |action| - expect(Gitlab::UsageDataCounters::HLLRedisCounter.known_event?("i_ecosystem_slack_service_#{action}_notification")).to be true - end - end - - context 'hook data includes a user object' do - let_it_be(:user) { create_default(:user) } - - shared_examples 'increases the usage data counter' do |event_name| - subject(:execute) { slack_integration.execute(data) } - - it 'increases the usage data counter' do - expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(event_name, values: user.id).and_call_original - - execute - end - - it_behaves_like 'Snowplow event tracking' do - let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } - let(:category) { 'Integrations::Slack' } - let(:action) { 'perform_integrations_action' } - let(:namespace) { project.namespace } - let(:label) { 'redis_hll_counters.ecosystem.ecosystem_total_unique_counts_monthly' } - let(:property) { event_name } - end - end - - context 'event is not supported for usage log' do - let_it_be(:pipeline) { create(:ci_pipeline) } - - let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } - - it 'does not increase the usage data counter' do - expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event).with('i_ecosystem_slack_service_pipeline_notification', values: user.id) - - slack_integration.execute(data) - end - end - - context 'issue notification' do - let_it_be(:issue) { create(:issue) } - - let(:data) { issue.to_hook_data(user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_issue_notification' - end - - context 'push notification' do - let(:data) { Gitlab::DataBuilder::Push.build_sample(project, user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_push_notification' - end - - context 'deployment notification' do - let_it_be(:deployment) { create(:deployment, user: user) } - - let(:data) { Gitlab::DataBuilder::Deployment.build(deployment, deployment.status, Time.current) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_deployment_notification' - end - - context 'wiki_page notification' do - let(:wiki_page) { create(:wiki_page, wiki: project.wiki, message: 'user created page: Awesome wiki_page') } - - let(:data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, 'create') } - - before do - # Skip this method that is not relevant to this test to prevent having - # to update project which is frozen - allow(project.wiki).to receive(:after_wiki_activity) - end - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_wiki_page_notification' - end - - context 'merge_request notification' do - let_it_be(:merge_request) { create(:merge_request) } - - let(:data) { merge_request.to_hook_data(user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_merge_request_notification' - end - - context 'note notification' do - let_it_be(:issue_note) { create(:note_on_issue, note: 'issue note') } - - let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_note_notification' - end - - context 'tag_push notification' do - let(:oldrev) { Gitlab::Git::BLANK_SHA } - let(:newrev) { '8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b' } # gitlab-test: git rev-parse refs/tags/v1.1.0 - let(:ref) { 'refs/tags/v1.1.0' } - let(:data) { Git::TagHooksService.new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }).send(:push_data) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_tag_push_notification' - end - - context 'confidential note notification' do - let_it_be(:confidential_issue_note) { create(:note_on_issue, note: 'issue note', confidential: true) } - - let(:data) { Gitlab::DataBuilder::Note.build(confidential_issue_note, user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_confidential_note_notification' - end - - context 'confidential issue notification' do - let_it_be(:issue) { create(:issue, confidential: true) } - - let(:data) { issue.to_hook_data(user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_confidential_issue_notification' - end - end - - context 'hook data does not include a user' do - let(:data) { Gitlab::DataBuilder::Pipeline.build(create(:ci_pipeline)) } - - it 'does not increase the usage data counter' do - expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) - - slack_integration.execute(data) - end - end - end + it_behaves_like Integrations::SlackMattermostNotifier, 'Slack' + it_behaves_like Integrations::BaseSlackNotification, factory: :integrations_slack end diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index da559264c1e..e32088a2f79 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do let(:teamcity_url) { 'https://gitlab.teamcity.com' } let(:teamcity_full_url) { 'https://gitlab.teamcity.com/httpAuth/app/rest/builds/branch:unspecified:any,revision:123' } - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } subject(:integration) do described_class.create!( @@ -22,6 +22,10 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do ) end + it_behaves_like Integrations::BaseCi + + it_behaves_like Integrations::ResetSecretFields + include_context Integrations::EnableSslVerification do describe '#enable_ssl_verification' do before do @@ -120,50 +124,6 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do end end - describe 'Callbacks' do - let(:teamcity_integration) { integration } - - describe 'before_validation :reset_password' do - context 'when a password was previously set' do - it 'resets password if url changed' do - teamcity_integration.teamcity_url = 'http://gitlab1.com' - teamcity_integration.valid? - - expect(teamcity_integration.password).to be_nil - end - - it 'does not reset password if username changed' do - teamcity_integration.username = 'some_name' - teamcity_integration.valid? - - expect(teamcity_integration.password).to eq('password') - end - - it "does not reset password if new url is set together with password, even if it's the same password" do - teamcity_integration.teamcity_url = 'http://gitlab_edited.com' - teamcity_integration.password = 'password' - teamcity_integration.valid? - - expect(teamcity_integration.password).to eq('password') - expect(teamcity_integration.teamcity_url).to eq('http://gitlab_edited.com') - end - end - - it 'saves password if new url is set together with password when no password was previously set' do - teamcity_integration.password = nil - - teamcity_integration.teamcity_url = 'http://gitlab_edited.com' - teamcity_integration.password = 'password' - teamcity_integration.save! - - expect(teamcity_integration.reload).to have_attributes( - teamcity_url: 'http://gitlab_edited.com', - password: 'password' - ) - end - end - end - describe '#build_page' do it 'returns the contents of the reactive cache' do stub_reactive_cache(integration, { build_page: 'foo' }, 'sha', 'ref') diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb index 1a32453819d..2fa4df0e900 100644 --- a/spec/models/integrations/zentao_spec.rb +++ b/spec/models/integrations/zentao_spec.rb @@ -7,15 +7,14 @@ RSpec.describe Integrations::Zentao do let(:api_url) { 'https://jihudemo.zentao.net' } let(:api_token) { 'ZENTAO_TOKEN' } let(:zentao_product_xid) { '3' } - let(:zentao_integration) { create(:zentao_integration) } + let(:zentao_integration) { build(:zentao_integration, project: project) } + let_it_be(:project) { create(:project, :repository) } it_behaves_like Integrations::ResetSecretFields do let(:integration) { zentao_integration } end describe 'set_default_data' do - let(:project) { create(:project, :repository) } - context 'when gitlab.yml was initialized' do it 'is prepopulated with the settings' do settings = { @@ -35,7 +34,6 @@ RSpec.describe Integrations::Zentao do end describe '#create' do - let(:project) { create(:project, :repository) } let(:params) do { project: project, diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index e7b2212ebff..aea8bdaf343 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -25,6 +25,7 @@ RSpec.describe Issue do it { is_expected.to have_many(:design_versions) } it { is_expected.to have_one(:sentry_issue) } it { is_expected.to have_one(:alert_management_alert) } + it { is_expected.to have_many(:alert_management_alerts) } it { is_expected.to have_many(:resource_milestone_events) } it { is_expected.to have_many(:resource_state_events) } it { is_expected.to have_and_belong_to_many(:prometheus_alert_events) } @@ -654,7 +655,7 @@ RSpec.describe Issue do let_it_be(:authorized_issue_a) { create(:issue, project: authorized_project) } let_it_be(:authorized_issue_b) { create(:issue, project: authorized_project) } let_it_be(:authorized_issue_c) { create(:issue, project: authorized_project2) } - let_it_be(:authorized_incident_a) { create(:incident, project: authorized_project ) } + let_it_be(:authorized_incident_a) { create(:incident, project: authorized_project) } let_it_be(:unauthorized_issue) { create(:issue, project: unauthorized_project) } @@ -863,7 +864,7 @@ RSpec.describe Issue do describe '.to_branch_name' do it 'parameterizes arguments and joins with dashes' do - expect(described_class.to_branch_name(123, 'foo bar', '!@#$%', 'f!o@o#b$a%r^')).to eq('123-foo-bar-f-o-o-b-a-r') + expect(described_class.to_branch_name(123, 'foo bar!@#$%f!o@o#b$a%r^')).to eq('123-foo-bar-f-o-o-b-a-r') end it 'preserves the case in the first argument' do @@ -871,7 +872,7 @@ RSpec.describe Issue do end it 'truncates branch name to at most 100 characters' do - expect(described_class.to_branch_name('a' * 101)).to eq('a' * 100) + expect(described_class.to_branch_name('a' * 101, 'a')).to eq('a' * 100) end it 'truncates dangling parts of the branch name' do @@ -883,6 +884,13 @@ RSpec.describe Issue do # 100 characters would've got us "999-lorem...lacus-custom-fri". expect(branch_name).to eq('999-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-mauris-sit-amet-ipsum-id-lacus-custom') end + + it 'takes issue branch template into account' do + project = create(:project) + project.project_setting.update!(issue_branch_template: 'feature-%{id}-%{title}') + + expect(described_class.to_branch_name(123, 'issue title', project: project)).to eq('feature-123-issue-title') + end end describe '#to_branch_name' do @@ -1785,4 +1793,22 @@ RSpec.describe Issue do end end end + + describe '#full_search' do + context 'when searching non-english terms' do + [ + 'abc 中文語', + '中文語cn', + '中文語', + 'Привет' + ].each do |term| + it 'adds extra where clause to match partial index' do + expect(described_class.full_search(term).to_sql).to include( + "AND (issues.title NOT SIMILAR TO '[\\u0000-\\u02FF\\u1E00-\\u1EFF\\u2070-\\u218F]*' " \ + "OR issues.description NOT SIMILAR TO '[\\u0000-\\u02FF\\u1E00-\\u1EFF\\u2070-\\u218F]*')" + ) + end + end + end + end end diff --git a/spec/models/jira_connect_installation_spec.rb b/spec/models/jira_connect_installation_spec.rb index e57d3e78a4e..09a4a7a488c 100644 --- a/spec/models/jira_connect_installation_spec.rb +++ b/spec/models/jira_connect_installation_spec.rb @@ -71,7 +71,7 @@ RSpec.describe JiraConnectInstallation do end describe '#oauth_authorization_url' do - let_it_be(:installation) { create(:jira_connect_installation) } + let(:installation) { build(:jira_connect_installation) } subject { installation.oauth_authorization_url } @@ -82,7 +82,7 @@ RSpec.describe JiraConnectInstallation do it { is_expected.to eq('http://test.host') } context 'with instance_url' do - let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'https://gitlab.example.com') } + let(:installation) { build(:jira_connect_installation, instance_url: 'https://gitlab.example.com') } it { is_expected.to eq('https://gitlab.example.com') } @@ -97,42 +97,42 @@ RSpec.describe JiraConnectInstallation do end describe 'audience_url' do - let_it_be(:installation) { create(:jira_connect_installation) } + let(:installation) { build(:jira_connect_installation) } subject(:audience) { installation.audience_url } it { is_expected.to eq(nil) } context 'when proxy installation' do - let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'https://example.com') } + let(:installation) { build(:jira_connect_installation, instance_url: 'https://example.com') } it { is_expected.to eq('https://example.com/-/jira_connect') } end end describe 'audience_installed_event_url' do - let_it_be(:installation) { create(:jira_connect_installation) } + let(:installation) { build(:jira_connect_installation) } subject(:audience) { installation.audience_installed_event_url } it { is_expected.to eq(nil) } context 'when proxy installation' do - let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'https://example.com') } + let(:installation) { build(:jira_connect_installation, instance_url: 'https://example.com') } it { is_expected.to eq('https://example.com/-/jira_connect/events/installed') } end end describe 'proxy?' do - let_it_be(:installation) { create(:jira_connect_installation) } + let(:installation) { build(:jira_connect_installation) } subject { installation.proxy? } it { is_expected.to eq(false) } context 'when instance_url is present' do - let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'https://example.com') } + let(:installation) { build(:jira_connect_installation, instance_url: 'https://example.com') } it { is_expected.to eq(true) } end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 04df8ecc882..2ecd10cccc6 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -7,6 +7,12 @@ RSpec.describe Member do using RSpec::Parameterized::TableSyntax + describe 'default values' do + subject(:member) { build(:project_member) } + + it { expect(member.notification_level).to eq(NotificationSetting.levels[:global]) } + end + describe 'Associations' do it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:member_namespace) } @@ -213,7 +219,7 @@ RSpec.describe Member do describe 'Scopes & finders' do let_it_be(:project) { create(:project, :public) } let_it_be(:group) { create(:group) } - let_it_be(:blocked_pending_approval_user) { create(:user, :blocked_pending_approval ) } + let_it_be(:blocked_pending_approval_user) { create(:user, :blocked_pending_approval) } let_it_be(:blocked_pending_approval_project_member) { create(:project_member, :invited, :developer, project: project, invite_email: blocked_pending_approval_user.email) } let_it_be(:awaiting_group_member) { create(:group_member, :awaiting, group: group) } let_it_be(:awaiting_project_member) { create(:project_member, :awaiting, project: project) } diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 363830d21dd..77bc6d9753f 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -3,6 +3,12 @@ require 'spec_helper' RSpec.describe GroupMember do + describe 'default values' do + subject(:goup_member) { build(:group_member) } + + it { expect(goup_member.source_type).to eq(described_class::SOURCE_TYPE) } + end + context 'scopes' do let_it_be(:user_1) { create(:user) } let_it_be(:user_2) { create(:user) } diff --git a/spec/models/members/last_group_owner_assigner_spec.rb b/spec/models/members/last_group_owner_assigner_spec.rb index 429cf4190cf..a0a829221de 100644 --- a/spec/models/members/last_group_owner_assigner_spec.rb +++ b/spec/models/members/last_group_owner_assigner_spec.rb @@ -7,14 +7,10 @@ RSpec.describe LastGroupOwnerAssigner do let_it_be(:user, reload: true) { create(:user) } let_it_be(:group) { create(:group) } - let(:group_member) { user.members.last } + let!(:group_member) { group.add_owner(user) } subject(:assigner) { described_class.new(group, [group_member]) } - before do - group.add_owner(user) - end - it "avoids extra database queries utilizing memoization", :aggregate_failures do control = ActiveRecord::QueryRecorder.new { assigner.execute } count_queries = control.occurrences_by_line_method.first[1][:occurrences].find_all { |i| i.include?('SELECT COUNT') } @@ -56,6 +52,40 @@ RSpec.describe LastGroupOwnerAssigner do .from(nil).to(false) end end + + context 'with owners from a parent' do + context 'when top-level group' do + context 'with group sharing' do + let!(:subgroup) { create(:group, parent: group) } + + before do + create(:group_group_link, :owner, shared_group: group, shared_with_group: subgroup) + create(:group_member, :owner, group: subgroup) + end + + specify do + expect { assigner.execute }.to change(group_member, :last_owner) + .from(nil).to(true) + .and change(group_member, :last_blocked_owner) + .from(nil).to(false) + end + end + end + + context 'when subgroup' do + let!(:subgroup) { create(:group, parent: group) } + let!(:group_member_2) { subgroup.add_owner(user) } + + subject(:assigner) { described_class.new(subgroup, [group_member_2]) } + + specify do + expect { assigner.execute }.to change(group_member_2, :last_owner) + .from(nil).to(false) + .and change(group_member_2, :last_blocked_owner) + .from(nil).to(false) + end + end + end end context "when there are blocked owners" do @@ -93,6 +123,54 @@ RSpec.describe LastGroupOwnerAssigner do .from(nil).to(false) end end + + context 'with owners from a parent' do + context 'when top-level group' do + context 'with group sharing' do + let!(:subgroup) { create(:group, parent: group) } + + before do + create(:group_group_link, :owner, shared_group: group, shared_with_group: subgroup) + create(:group_member, :owner, group: subgroup) + end + + specify do + expect { assigner.execute }.to change(group_member, :last_owner) + .from(nil).to(false) + .and change(group_member, :last_blocked_owner) + .from(nil).to(true) + end + end + end + + context 'when subgroup' do + let!(:subgroup) { create(:group, :nested) } + + let!(:group_member) { subgroup.add_owner(user) } + + subject(:assigner) { described_class.new(subgroup, [group_member]) } + + specify do + expect { assigner.execute }.to change(group_member, :last_owner) + .from(nil).to(false) + .and change(group_member, :last_blocked_owner) + .from(nil).to(true) + end + + context 'with two owners' do + before do + create(:group_member, :owner, group: subgroup.parent) + end + + specify do + expect { assigner.execute }.to change(group_member, :last_owner) + .from(nil).to(false) + .and change(group_member, :last_blocked_owner) + .from(nil).to(false) + end + end + end + end end context 'when there are bot members' do diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index ad6f3ca5428..e56c6b38992 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -13,6 +13,10 @@ RSpec.describe ProjectMember do it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } end + describe 'default values' do + it { expect(described_class.new.source_type).to eq('Project') } + end + describe 'delegations' do it { is_expected.to delegate_method(:namespace_id).to(:project) } end diff --git a/spec/models/merge_request_diff_file_spec.rb b/spec/models/merge_request_diff_file_spec.rb index f107a56c1b6..7e127caa649 100644 --- a/spec/models/merge_request_diff_file_spec.rb +++ b/spec/models/merge_request_diff_file_spec.rb @@ -203,16 +203,6 @@ RSpec.describe MergeRequestDiffFile do end end - context 'when externally_stored_diffs_caching_export feature flag is disabled' do - it 'calls #diff' do - stub_feature_flags(externally_stored_diffs_caching_export: false) - - expect(file).to receive(:diff) - - file.utf8_diff - end - end - context 'when diff is not stored externally' do it 'calls #diff' do expect(file).to receive(:diff) diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index e9e8bd9bfea..22fed716897 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -1097,6 +1097,19 @@ RSpec.describe MergeRequestDiff do it 'returns a non-empty CommitCollection' do expect(mr.merge_request_diff.commits.commits.size).to be > 0 end + + context 'with a page' do + it 'returns a limited number of commits for page' do + expect(mr.merge_request_diff.commits(limit: 1, page: 1).map(&:sha)).to eq( + %w[ + b83d6e391c22777fca1ed3012fce84f633d7fed0 + ]) + expect(mr.merge_request_diff.commits(limit: 1, page: 2).map(&:sha)).to eq( + %w[ + 498214de67004b1da3d820901307bed2a68a8ef6 + ]) + end + end end describe '.latest_diff_for_merge_requests' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 32518b867cb..cf4f58f558c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -232,10 +232,6 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'for branch' do - before do - stub_feature_flags(stricter_mr_branch_name: false) - end - where(:branch_name, :valid) do 'foo' | true 'foo:bar' | false @@ -278,6 +274,34 @@ RSpec.describe MergeRequest, factory_default: :keep do end describe 'callbacks' do + describe '#ensure_merge_request_diff' do + let(:merge_request) { build(:merge_request) } + + context 'when async_merge_request_diff_creation is true' do + before do + merge_request.skip_ensure_merge_request_diff = true + end + + it 'does not create a merge_request_diff after create' do + merge_request.save! + + expect(merge_request.merge_request_diff).to be_empty + end + end + + context 'when async_merge_request_diff_creation is false' do + before do + merge_request.skip_ensure_merge_request_diff = false + end + + it 'creates merge_request_diff after create' do + merge_request.save! + + expect(merge_request.merge_request_diff).not_to be_empty + end + end + end + describe '#ensure_merge_request_metrics' do let(:merge_request) { create(:merge_request) } @@ -3228,14 +3252,6 @@ RSpec.describe MergeRequest, factory_default: :keep do describe '#mergeable_state?' do it_behaves_like 'for mergeable_state' - - context 'when merge state caching is off' do - before do - stub_feature_flags(mergeability_caching: false) - end - - it_behaves_like 'for mergeable_state' - end end describe "#public_merge_status" do @@ -4213,14 +4229,6 @@ RSpec.describe MergeRequest, factory_default: :keep do transition! end - - context 'when trigger_mr_subscription_on_merge_status_change is disabled' do - before do - stub_feature_flags(trigger_mr_subscription_on_merge_status_change: false) - end - - it_behaves_like 'transition not triggering mergeRequestMergeStatusUpdated GraphQL subscription' - end end shared_examples 'for an invalid state transition' do @@ -4984,6 +4992,19 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(subject.commits.size).to eq(29) end end + + context 'with a page' do + it 'returns a limited number of commits for page' do + expect(subject.commits(limit: 1, page: 1).map(&:sha)).to eq( + %w[ + b83d6e391c22777fca1ed3012fce84f633d7fed0 + ]) + expect(subject.commits(limit: 1, page: 2).map(&:sha)).to eq( + %w[ + 498214de67004b1da3d820901307bed2a68a8ef6 + ]) + end + end end context 'new merge request' do @@ -5114,17 +5135,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end it 'returns false' do - expect(merge_request.diffable_merge_ref?).to eq(true) - end - - context 'display_merge_conflicts_in_diff is disabled' do - before do - stub_feature_flags(display_merge_conflicts_in_diff: false) - end - - it 'returns false' do - expect(merge_request.diffable_merge_ref?).to eq(false) - end + expect(merge_request.diffable_merge_ref?).to eq(false) end end end diff --git a/spec/models/metrics/dashboard/annotation_spec.rb b/spec/models/metrics/dashboard/annotation_spec.rb index 4b7492016f3..9b8601e4052 100644 --- a/spec/models/metrics/dashboard/annotation_spec.rb +++ b/spec/models/metrics/dashboard/annotation_spec.rb @@ -63,7 +63,7 @@ RSpec.describe Metrics::Dashboard::Annotation do end context 'annotation with shared ownership' do - subject { build(:metrics_dashboard_annotation, :with_cluster, environment: build(:environment) ) } + subject { build(:metrics_dashboard_annotation, :with_cluster, environment: build(:environment)) } it 'reports error about both shared ownership' do subject.valid? diff --git a/spec/models/ml/candidate_metric_spec.rb b/spec/models/ml/candidate_metric_spec.rb index 5ee6030fb8e..9f9a6e8e3ba 100644 --- a/spec/models/ml/candidate_metric_spec.rb +++ b/spec/models/ml/candidate_metric_spec.rb @@ -6,4 +6,17 @@ RSpec.describe Ml::CandidateMetric do describe 'associations' do it { is_expected.to belong_to(:candidate) } end + + describe 'scope :latest' do + let_it_be(:candidate) { create(:ml_candidates) } + let!(:metric1) { create(:ml_candidate_metrics, candidate: candidate) } + let!(:metric2) { create(:ml_candidate_metrics, candidate: candidate ) } + let!(:metric3) { create(:ml_candidate_metrics, name: metric1.name, candidate: candidate) } + + subject { described_class.latest } + + it 'fetches only the last metric for the name' do + expect(subject).to match_array([metric2, metric3] ) + end + end end diff --git a/spec/models/ml/candidate_spec.rb b/spec/models/ml/candidate_spec.rb index 3bf1e80a152..b35496363fe 100644 --- a/spec/models/ml/candidate_spec.rb +++ b/spec/models/ml/candidate_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ml::Candidate, factory_default: :keep do - let_it_be(:candidate) { create_default(:ml_candidates, :with_metrics_and_params) } + let_it_be(:candidate) { create(:ml_candidates, :with_metrics_and_params) } describe 'associations' do it { is_expected.to belong_to(:experiment) } @@ -12,10 +12,14 @@ RSpec.describe Ml::Candidate, factory_default: :keep do it { is_expected.to have_many(:metrics) } end - describe '#new' do - it 'iid is not null' do - expect(candidate.iid).not_to be_nil - end + describe '.artifact_root' do + subject { candidate.artifact_root } + + it { is_expected.to eq("/ml_candidate_#{candidate.iid}/-/") } + end + + describe 'default values' do + it { expect(described_class.new.iid).to be_present } end describe '#by_project_id_and_iid' do @@ -40,4 +44,26 @@ RSpec.describe Ml::Candidate, factory_default: :keep do it { is_expected.to be_nil } end end + + describe "#latest_metrics" do + let_it_be(:candidate2) { create(:ml_candidates, experiment: candidate.experiment) } + let!(:metric1) { create(:ml_candidate_metrics, candidate: candidate2) } + let!(:metric2) { create(:ml_candidate_metrics, candidate: candidate2 ) } + let!(:metric3) { create(:ml_candidate_metrics, name: metric1.name, candidate: candidate2) } + + subject { candidate2.latest_metrics } + + it 'fetches only the last metric for the name' do + expect(subject).to match_array([metric2, metric3] ) + end + end + + describe "#including_metrics_and_params" do + subject { described_class.including_metrics_and_params.find_by(id: candidate.id) } + + it 'loads latest metrics and params', :aggregate_failures do + expect(subject.association_cached?(:latest_metrics)).to be(true) + expect(subject.association_cached?(:params)).to be(true) + end + end end diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index a4446bfedd1..17c49e13c85 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -106,7 +106,7 @@ RSpec.describe NamespaceSetting, type: :model do describe '#prevent_sharing_groups_outside_hierarchy' do let(:settings) { create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true) } - let!(:group) { create(:group, parent: parent, namespace_settings: settings ) } + let!(:group) { create(:group, parent: parent, namespace_settings: settings) } subject(:group_sharing_setting) { settings.prevent_sharing_groups_outside_hierarchy } @@ -133,7 +133,7 @@ RSpec.describe NamespaceSetting, type: :model do context 'when :show_diff_preview_in_email is false' do it 'returns false' do settings = create(:namespace_settings, show_diff_preview_in_email: false) - group = create(:group, namespace_settings: settings ) + group = create(:group, namespace_settings: settings) expect(group.show_diff_preview_in_email?).to be_falsey end @@ -142,7 +142,7 @@ RSpec.describe NamespaceSetting, type: :model do context 'when :show_diff_preview_in_email is true' do it 'returns true' do settings = create(:namespace_settings, show_diff_preview_in_email: true) - group = create(:group, namespace_settings: settings ) + group = create(:group, namespace_settings: settings) expect(group.show_diff_preview_in_email?).to be_truthy end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index c6d028af22d..0516d446945 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1623,7 +1623,7 @@ RSpec.describe Namespace do describe '#share_with_group_lock with subgroups' do context 'when creating a subgroup' do - let(:subgroup) { create(:group, parent: root_group ) } + let(:subgroup) { create(:group, parent: root_group) } context 'under a parent with "Share with group lock" enabled' do let(:root_group) { create(:group, share_with_group_lock: true) } @@ -1644,7 +1644,7 @@ RSpec.describe Namespace do context 'when enabling the parent group "Share with group lock"' do let(:root_group) { create(:group) } - let!(:subgroup) { create(:group, parent: root_group ) } + let!(:subgroup) { create(:group, parent: root_group) } it 'the subgroup "Share with group lock" becomes enabled' do root_group.update!(share_with_group_lock: true) @@ -1657,7 +1657,7 @@ RSpec.describe Namespace do let(:root_group) { create(:group, share_with_group_lock: true) } context 'and the subgroup "Share with group lock" is enabled' do - let(:subgroup) { create(:group, parent: root_group, share_with_group_lock: true ) } + let(:subgroup) { create(:group, parent: root_group, share_with_group_lock: true) } it 'the subgroup "Share with group lock" does not change' do root_group.update!(share_with_group_lock: false) @@ -1667,7 +1667,7 @@ RSpec.describe Namespace do end context 'but the subgroup "Share with group lock" is disabled' do - let(:subgroup) { create(:group, parent: root_group ) } + let(:subgroup) { create(:group, parent: root_group) } it 'the subgroup "Share with group lock" does not change' do root_group.update!(share_with_group_lock: false) @@ -1682,7 +1682,7 @@ RSpec.describe Namespace do let(:root_group) { create(:group, share_with_group_lock: true) } context 'when the subgroup "Share with group lock" is enabled' do - let(:subgroup) { create(:group, share_with_group_lock: true ) } + let(:subgroup) { create(:group, share_with_group_lock: true) } it 'the subgroup "Share with group lock" does not change' do subgroup.parent = root_group @@ -1708,7 +1708,7 @@ RSpec.describe Namespace do let(:root_group) { create(:group) } context 'when the subgroup "Share with group lock" is enabled' do - let(:subgroup) { create(:group, share_with_group_lock: true ) } + let(:subgroup) { create(:group, share_with_group_lock: true) } it 'the subgroup "Share with group lock" does not change' do subgroup.parent = root_group @@ -1826,7 +1826,7 @@ RSpec.describe Namespace do group.update!(parent: parent) - expect(group.full_path_before_last_save).to eq("#{group.path_before_last_save}") + expect(group.full_path_before_last_save).to eq(group.path_before_last_save.to_s) end end @@ -2356,7 +2356,7 @@ RSpec.describe Namespace do end end - describe 'storage_enforcement_date' do + describe 'storage_enforcement_date', :freeze_time do let_it_be(:namespace) { create(:group) } before do @@ -2364,7 +2364,7 @@ RSpec.describe Namespace do end it 'returns correct date' do - expect(namespace.storage_enforcement_date).to eql(Date.new(2022, 10, 19)) + expect(namespace.storage_enforcement_date).to eql(3.months.from_now.to_date) end context 'when :storage_banner_bypass_date_check is enabled' do @@ -2372,7 +2372,7 @@ RSpec.describe Namespace do stub_feature_flags(namespace_storage_limit_bypass_date_check: true) end - it 'returns the current date', :freeze_time do + it 'returns the current date' do expect(namespace.storage_enforcement_date).to eq(Date.current) end end diff --git a/spec/models/network/graph_spec.rb b/spec/models/network/graph_spec.rb index a393aace39c..16894bf28f1 100644 --- a/spec/models/network/graph_spec.rb +++ b/spec/models/network/graph_spec.rb @@ -6,10 +6,24 @@ RSpec.describe Network::Graph do let(:project) { create(:project, :repository) } let!(:note_on_commit) { create(:note_on_commit, project: project) } - it '#initialize' do - graph = described_class.new(project, 'refs/heads/master', project.repository.commit, nil) + describe '#initialize' do + let(:graph) do + described_class.new(project, 'refs/heads/master', project.repository.commit, nil) + end + + it 'has initialized' do + expect(graph).to be_a(described_class) + end - expect(graph.notes).to eq( { note_on_commit.commit_id => 1 } ) + 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 @@ -19,7 +33,7 @@ RSpec.describe Network::Graph do commits = graph.commits expect(commits).not_to be_empty - expect(commits).to all( be_kind_of(Network::Commit) ) + expect(commits).to all(be_kind_of(Network::Commit)) end it 'sorts commits by commit date (descending)' do @@ -42,7 +56,7 @@ RSpec.describe Network::Graph do parent_indexes = commit.parent_ids.map { |parent_id| commit_ids.find_index(parent_id) }.compact # All parents of the current commit should appear after it - expect(parent_indexes).to all( be > index ) + expect(parent_indexes).to all(be > index) end end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 1b44da75c40..7c71080d63e 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -23,6 +23,10 @@ RSpec.describe Note do it { is_expected.to include_module(Sortable) } end + describe 'default values' do + it { expect(described_class.new).not_to be_system } + end + describe 'validation' do it { is_expected.to validate_length_of(:note).is_at_most(1_000_000) } it { is_expected.to validate_presence_of(:note) } @@ -1619,70 +1623,6 @@ RSpec.describe Note do end end - describe '#noteable_assignee_or_author?' do - let(:user) { create(:user) } - let(:noteable) { create(:issue) } - let(:note) { create(:note, project: noteable.project, noteable: noteable) } - - subject { note.noteable_assignee_or_author?(user) } - - shared_examples 'assignee check' do - context 'when the provided user is one of the assignees' do - before do - note.noteable.update!(assignees: [user, create(:user)]) - end - - it 'returns true' do - expect(subject).to be_truthy - end - end - end - - shared_examples 'author check' do - context 'when the provided user is the author' do - before do - note.noteable.update!(author: user) - end - - it 'returns true' do - expect(subject).to be_truthy - end - end - - context 'when the provided user is neither author nor assignee' do - it 'returns true' do - expect(subject).to be_falsey - end - end - end - - context 'when user is nil' do - let(:user) { nil } - - it 'returns false' do - expect(subject).to be_falsey - end - end - - context 'when noteable is an issue' do - it_behaves_like 'author check' - it_behaves_like 'assignee check' - end - - context 'when noteable is a merge request' do - let(:noteable) { create(:merge_request) } - - it_behaves_like 'author check' - it_behaves_like 'assignee check' - end - - context 'when noteable is a snippet' do - let(:noteable) { create(:personal_snippet) } - - it_behaves_like 'author check' - end - end - describe 'banzai_render_context' do let(:project) { build(:project_empty_repo) } diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index cc601fb30c2..730a9045d7f 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -5,6 +5,12 @@ require 'spec_helper' RSpec.describe NotificationSetting do it_behaves_like 'having unique enum values' + describe 'default values' do + subject(:notification_setting) { build(:notification_setting) } + + it { expect(notification_setting.level).to eq('global') } + end + describe "Associations" do it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:source) } diff --git a/spec/models/oauth_access_token_spec.rb b/spec/models/oauth_access_token_spec.rb index a4540ac95bc..92e1ae8ac60 100644 --- a/spec/models/oauth_access_token_spec.rb +++ b/spec/models/oauth_access_token_spec.rb @@ -46,42 +46,11 @@ RSpec.describe OauthAccessToken do expect(described_class.by_token(plaintext_token)).to be_a(OauthAccessToken) end end - - context 'when hash_oauth_secrets is disabled' do - let(:hashed_token) { create(:oauth_access_token, application_id: app_one.id) } - - before do - hashed_token - stub_feature_flags(hash_oauth_tokens: false) - end - - it 'stores the token in plaintext' do - expect(token.token).to eq(token.plaintext_token) - end - - it 'finds a token by plaintext token' do - expect(described_class.by_token(token.plaintext_token)).to be_a(OauthAccessToken) - end - - it 'does not find a token that was previously stored as hashed' do - expect(described_class.by_token(hashed_token.plaintext_token)).to be_nil - end - end end describe '.matching_token_for' do it 'does not find existing tokens' do expect(described_class.matching_token_for(app_one, token.resource_owner, token.scopes)).to be_nil end - - context 'when hash oauth tokens is disabled' do - before do - stub_feature_flags(hash_oauth_tokens: false) - end - - it 'finds an existing token' do - expect(described_class.matching_token_for(app_one, token.resource_owner, token.scopes)).to be_present - end - end end end diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb index 85a475f5c53..dd1ff95a16d 100644 --- a/spec/models/operations/feature_flag_spec.rb +++ b/spec/models/operations/feature_flag_spec.rb @@ -16,6 +16,11 @@ RSpec.describe Operations::FeatureFlag do it { is_expected.to have_many(:strategies) } end + describe 'default values' do + it { expect(described_class.new).to be_active } + it { expect(described_class.new.version).to eq('new_version_flag') } + end + describe '.reference_pattern' do subject { described_class.reference_pattern } diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index 9554fc3bb1b..c665f738ead 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -314,7 +314,7 @@ RSpec.describe Packages::PackageFile, type: :model do # to `1`. expect(package_file) .to receive(:update_column) - .with(:file_store, ::Packages::PackageFileUploader::Store::LOCAL) + .with('file_store', ::Packages::PackageFileUploader::Store::LOCAL) expect { subject }.to change { package_file.size }.from(nil).to(3513) end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 0edb04224a3..241c585099c 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -969,12 +969,12 @@ RSpec.describe Packages::Package, type: :model do end context 'sorting' do - let_it_be(:project) { create(:project, name: 'aaa' ) } - let_it_be(:project2) { create(:project, name: 'bbb' ) } - let_it_be(:package1) { create(:package, project: project ) } - let_it_be(:package2) { create(:package, project: project2 ) } - let_it_be(:package3) { create(:package, project: project2 ) } - let_it_be(:package4) { create(:package, project: project ) } + let_it_be(:project) { create(:project, name: 'aaa') } + let_it_be(:project2) { create(:project, name: 'bbb') } + let_it_be(:package1) { create(:package, project: project) } + let_it_be(:package2) { create(:package, project: project2) } + let_it_be(:package3) { create(:package, project: project2) } + let_it_be(:package4) { create(:package, project: project) } it 'orders packages by their projects name ascending' do expect(Packages::Package.order_project_name).to eq([package1, package4, package2, package3]) diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 463ec904e9a..e5f2e849a0a 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -209,6 +209,10 @@ RSpec.describe PagesDomain do expect(subject.wildcard).to eq(false) end + it 'defaults auto_ssl_enabled to false' do + expect(subject.auto_ssl_enabled).to eq(false) + end + it 'defaults scope to project' do expect(subject.scope).to eq('project') end diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 67e7d444d25..9d4c53f8d55 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -108,7 +108,7 @@ RSpec.describe PersonalAccessToken do describe '.last_used_before' do context 'last_used_*' do let_it_be(:date) { DateTime.new(2022, 01, 01) } - let_it_be(:token) { create(:personal_access_token, last_used_at: date ) } + let_it_be(:token) { create(:personal_access_token, last_used_at: date) } # This token should never occur in the following tests and indicates that filtering was done correctly with it let_it_be(:never_used_token) { create(:personal_access_token) } @@ -194,47 +194,6 @@ RSpec.describe PersonalAccessToken do end end - describe 'Redis storage' do - let(:user_id) { 123 } - let(:token) { 'KS3wegQYXBLYhQsciwsj' } - - context 'reading encrypted data' do - before do - subject.redis_store!(user_id, token) - end - - it 'returns stored data' do - expect(subject.redis_getdel(user_id)).to eq(token) - end - end - - context 'reading unencrypted data' do - before do - Gitlab::Redis::SharedState.with do |redis| - redis.set(described_class.redis_shared_state_key(user_id), - token, - ex: PersonalAccessToken::REDIS_EXPIRY_TIME) - end - end - - it 'returns stored data unmodified' do - expect(subject.redis_getdel(user_id)).to eq(token) - end - end - - context 'after deletion' do - before do - subject.redis_store!(user_id, token) - - expect(subject.redis_getdel(user_id)).to eq(token) - end - - it 'token is removed' do - expect(subject.redis_getdel(user_id)).to be_nil - end - end - end - context "validations" do let(:personal_access_token) { build(:personal_access_token) } @@ -365,7 +324,7 @@ RSpec.describe PersonalAccessToken do describe '.simple_sorts' do it 'includes overridden keys' do - expect(described_class.simple_sorts.keys).to include(*%w(expires_at_asc expires_at_desc expires_at_asc_id_desc)) + expect(described_class.simple_sorts.keys).to include(*%w(expires_at_asc_id_desc)) end end @@ -373,18 +332,6 @@ RSpec.describe PersonalAccessToken do let_it_be(:earlier_token) { create(:personal_access_token, expires_at: 2.days.ago) } let_it_be(:later_token) { create(:personal_access_token, expires_at: 1.day.ago) } - describe '.order_expires_at_asc' do - it 'returns ordered list in asc order of expiry date' do - expect(described_class.order_expires_at_asc).to match [earlier_token, later_token] - end - end - - describe '.order_expires_at_desc' do - it 'returns ordered list in desc order of expiry date' do - expect(described_class.order_expires_at_desc).to match [later_token, earlier_token] - end - end - describe '.order_expires_at_asc_id_desc' do let_it_be(:earlier_token_2) { create(:personal_access_token, expires_at: 2.days.ago) } diff --git a/spec/models/preloaders/labels_preloader_spec.rb b/spec/models/preloaders/labels_preloader_spec.rb index 86e64d114c7..07f148a0a6c 100644 --- a/spec/models/preloaders/labels_preloader_spec.rb +++ b/spec/models/preloaders/labels_preloader_spec.rb @@ -40,10 +40,11 @@ RSpec.describe Preloaders::LabelsPreloader do def access_data(labels) labels.each do |label| - if label.is_a?(ProjectLabel) + case label + when ProjectLabel label.project.project_feature label.lazy_subscription(user, label.project) - elsif label.is_a?(GroupLabel) + when GroupLabel label.group.route label.lazy_subscription(user) end diff --git a/spec/models/preloaders/project_root_ancestor_preloader_spec.rb b/spec/models/preloaders/project_root_ancestor_preloader_spec.rb index bb0de24abe5..2462e305597 100644 --- a/spec/models/preloaders/project_root_ancestor_preloader_spec.rb +++ b/spec/models/preloaders/project_root_ancestor_preloader_spec.rb @@ -63,6 +63,14 @@ RSpec.describe Preloaders::ProjectRootAncestorPreloader do it_behaves_like 'executes N matching DB queries', 0, :full_path end + + context 'when projects are an array and not an ActiveRecord::Relation' do + before do + described_class.new(projects, :namespace, additional_preloads).execute + end + + it_behaves_like 'executes N matching DB queries', 4 + end end context 'when the preloader is not used' do diff --git a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb index 7411bc95147..1cfeeac49cd 100644 --- a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb +++ b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb @@ -28,34 +28,58 @@ RSpec.describe Preloaders::UserMaxAccessLevelInProjectsPreloader do end end - describe '#execute', :request_store do + shared_examples '#execute' do let(:projects_arg) { projects } - before do - Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects_arg, user).execute - end - - it 'avoids N+1 queries' do - expect { query }.not_to make_queries - end - - context 'when projects is an array of IDs' do - let(:projects_arg) { projects.map(&:id) } + context 'when user is present' do + before do + Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects_arg, user).execute + end it 'avoids N+1 queries' do expect { query }.not_to make_queries end + + context 'when projects is an array of IDs' do + let(:projects_arg) { projects.map(&:id) } + + it 'avoids N+1 queries' do + expect { query }.not_to make_queries + end + end + + # Test for handling of SQL table name clashes. + context 'when projects is a relation including project_authorizations' do + let(:projects_arg) do + Project.where(id: ProjectAuthorization.where(project_id: projects).select(:project_id)) + end + + it 'avoids N+1 queries' do + expect { query }.not_to make_queries + end + end end - # Test for handling of SQL table name clashes. - context 'when projects is a relation including project_authorizations' do - let(:projects_arg) do - Project.where(id: ProjectAuthorization.where(project_id: projects).select(:project_id)) + context 'when user is not present' do + before do + Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects_arg, nil).execute end - it 'avoids N+1 queries' do - expect { query }.not_to make_queries + it 'does not avoid N+1 queries' do + expect { query }.to make_queries + end + end + end + + describe '#execute', :request_store do + include_examples '#execute' + + context 'when projects_preloader_fix is disabled' do + before do + stub_feature_flags(projects_preloader_fix: false) end + + include_examples '#execute' end end end diff --git a/spec/models/project_authorization_spec.rb b/spec/models/project_authorization_spec.rb index 55fe28ceb6f..df89e97a41f 100644 --- a/spec/models/project_authorization_spec.rb +++ b/spec/models/project_authorization_spec.rb @@ -86,6 +86,25 @@ RSpec.describe ProjectAuthorization do end end + shared_examples_for 'does not log any detail' do + it 'does not log any detail' do + expect(Gitlab::AppLogger).not_to receive(:info) + + execute + end + end + + shared_examples_for 'logs the detail' do + it 'logs the detail' do + expect(Gitlab::AppLogger).to receive(:info).with( + entire_size: 3, + message: 'Project authorizations refresh performed with delay' + ) + + execute + end + end + describe '.insert_all_in_batches' do let_it_be(:user) { create(:user) } let_it_be(:project_1) { create(:project) } @@ -100,6 +119,8 @@ RSpec.describe ProjectAuthorization do ] end + subject(:execute) { described_class.insert_all_in_batches(attributes, per_batch_size) } + before do # Configure as if a replica database is enabled allow(::Gitlab::Database::LoadBalancing).to receive(:primary_only?).and_return(false) @@ -110,7 +131,7 @@ RSpec.describe ProjectAuthorization do specify do expect(described_class).not_to receive(:sleep) - described_class.insert_all_in_batches(attributes, per_batch_size) + execute expect(user.project_authorizations.pluck(:user_id, :project_id, :access_level)).to match_array(attributes.map(&:values)) end @@ -123,11 +144,13 @@ RSpec.describe ProjectAuthorization do expect(described_class).to receive(:insert_all).twice.and_call_original expect(described_class).to receive(:sleep).twice - described_class.insert_all_in_batches(attributes, per_batch_size) + execute expect(user.project_authorizations.pluck(:user_id, :project_id, :access_level)).to match_array(attributes.map(&:values)) end + it_behaves_like 'logs the detail' + context 'when the GitLab installation does not have a replica database configured' do before do # Configure as if a replica database is not enabled @@ -135,6 +158,7 @@ RSpec.describe ProjectAuthorization do end it_behaves_like 'inserts the rows in batches, as per the `per_batch` size, without a delay between each batch' + it_behaves_like 'does not log any detail' end end @@ -142,6 +166,7 @@ RSpec.describe ProjectAuthorization do let(:per_batch_size) { 5 } it_behaves_like 'inserts the rows in batches, as per the `per_batch` size, without a delay between each batch' + it_behaves_like 'does not log any detail' end end @@ -154,6 +179,14 @@ RSpec.describe ProjectAuthorization do let(:user_ids) { [user_1.id, user_2.id, user_3.id] } + subject(:execute) do + described_class.delete_all_in_batches_for_project( + project: project, + user_ids: user_ids, + per_batch: per_batch_size + ) + end + before do # Configure as if a replica database is enabled allow(::Gitlab::Database::LoadBalancing).to receive(:primary_only?).and_return(false) @@ -171,11 +204,7 @@ RSpec.describe ProjectAuthorization do specify do expect(described_class).not_to receive(:sleep) - described_class.delete_all_in_batches_for_project( - project: project, - user_ids: user_ids, - per_batch: per_batch_size - ) + execute expect(project.project_authorizations.pluck(:user_id)).not_to include(*user_ids) end @@ -187,15 +216,13 @@ RSpec.describe ProjectAuthorization do it 'removes the project authorizations of the specified users in the current project, with a delay between each batch' do expect(described_class).to receive(:sleep).twice - described_class.delete_all_in_batches_for_project( - project: project, - user_ids: user_ids, - per_batch: per_batch_size - ) + execute expect(project.project_authorizations.pluck(:user_id)).not_to include(*user_ids) end + it_behaves_like 'logs the detail' + context 'when the GitLab installation does not have a replica database configured' do before do # Configure as if a replica database is not enabled @@ -203,6 +230,7 @@ RSpec.describe ProjectAuthorization do end it_behaves_like 'removes the project authorizations of the specified users in the current project, without a delay between each batch' + it_behaves_like 'does not log any detail' end end @@ -210,6 +238,7 @@ RSpec.describe ProjectAuthorization do let(:per_batch_size) { 5 } it_behaves_like 'removes the project authorizations of the specified users in the current project, without a delay between each batch' + it_behaves_like 'does not log any detail' end end @@ -222,6 +251,14 @@ RSpec.describe ProjectAuthorization do let(:project_ids) { [project_1.id, project_2.id, project_3.id] } + subject(:execute) do + described_class.delete_all_in_batches_for_user( + user: user, + project_ids: project_ids, + per_batch: per_batch_size + ) + end + before do # Configure as if a replica database is enabled allow(::Gitlab::Database::LoadBalancing).to receive(:primary_only?).and_return(false) @@ -239,11 +276,7 @@ RSpec.describe ProjectAuthorization do specify do expect(described_class).not_to receive(:sleep) - described_class.delete_all_in_batches_for_user( - user: user, - project_ids: project_ids, - per_batch: per_batch_size - ) + execute expect(user.project_authorizations.pluck(:project_id)).not_to include(*project_ids) end @@ -255,15 +288,13 @@ RSpec.describe ProjectAuthorization do it 'removes the project authorizations of the specified projects from the current user, with a delay between each batch' do expect(described_class).to receive(:sleep).twice - described_class.delete_all_in_batches_for_user( - user: user, - project_ids: project_ids, - per_batch: per_batch_size - ) + execute expect(user.project_authorizations.pluck(:project_id)).not_to include(*project_ids) end + it_behaves_like 'logs the detail' + context 'when the GitLab installation does not have a replica database configured' do before do # Configure as if a replica database is not enabled @@ -271,6 +302,7 @@ RSpec.describe ProjectAuthorization do end it_behaves_like 'removes the project authorizations of the specified projects from the current user, without a delay between each batch' + it_behaves_like 'does not log any detail' end end @@ -278,6 +310,7 @@ RSpec.describe ProjectAuthorization do let(:per_batch_size) { 5 } it_behaves_like 'removes the project authorizations of the specified projects from the current user, without a delay between each batch' + it_behaves_like 'does not log any detail' end end end diff --git a/spec/models/project_ci_cd_setting_spec.rb b/spec/models/project_ci_cd_setting_spec.rb index 406485d8cc8..5a32e103e0f 100644 --- a/spec/models/project_ci_cd_setting_spec.rb +++ b/spec/models/project_ci_cd_setting_spec.rb @@ -21,6 +21,12 @@ RSpec.describe ProjectCiCdSetting do end end + describe '#separated_caches' do + it 'is true by default' do + expect(described_class.new.separated_caches).to be_truthy + end + end + describe '#default_git_depth' do let(:default_value) { described_class::DEFAULT_GIT_DEPTH } diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 5730ca58e9e..94a2e2fe3f9 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -6,6 +6,10 @@ RSpec.describe ProjectSetting, type: :model do using RSpec::Parameterized::TableSyntax it { is_expected.to belong_to(:project) } + describe 'default values' do + it { expect(subject.legacy_open_source_license_available).to be_truthy } + end + describe 'scopes' do let_it_be(:project_1) { create(:project) } let_it_be(:project_2) { create(:project) } @@ -20,6 +24,7 @@ RSpec.describe ProjectSetting, type: :model do describe 'validations' do it { is_expected.not_to allow_value(nil).for(:target_platforms) } it { is_expected.to allow_value([]).for(:target_platforms) } + it { is_expected.to validate_length_of(:issue_branch_template).is_at_most(255) } it { is_expected.not_to allow_value(nil).for(:suggested_reviewers_enabled) } it { is_expected.to allow_value(true).for(:suggested_reviewers_enabled) } @@ -70,7 +75,7 @@ RSpec.describe ProjectSetting, type: :model do describe '#show_diff_preview_in_email?' do context 'when a project is a top-level namespace' do - let(:project_settings ) { create(:project_setting, show_diff_preview_in_email: false) } + let(:project_settings) { create(:project_setting, show_diff_preview_in_email: false) } let(:project) { create(:project, project_setting: project_settings) } context 'when show_diff_preview_in_email is disabled' do @@ -80,7 +85,7 @@ RSpec.describe ProjectSetting, type: :model do end context 'when show_diff_preview_in_email is enabled' do - let(:project_settings ) { create(:project_setting, show_diff_preview_in_email: true) } + let(:project_settings) { create(:project_setting, show_diff_preview_in_email: true) } it 'returns true' do settings = create(:project_setting, show_diff_preview_in_email: true) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 75887e49dc9..8cccc9ad83e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -38,6 +38,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:hooks) } it { is_expected.to have_many(:protected_branches) } it { is_expected.to have_many(:exported_protected_branches) } + it { is_expected.to have_one(:wiki_repository).class_name('Projects::WikiRepository').inverse_of(:project) } it { is_expected.to have_one(:slack_integration) } it { is_expected.to have_one(:microsoft_teams_integration) } it { is_expected.to have_one(:mattermost_integration) } @@ -597,7 +598,7 @@ RSpec.describe Project, factory_default: :keep do end it 'contains errors related to the project being deleted' do - expect(new_project.errors.full_messages.first).to eq(_('The project is still being deleted. Please try again later.')) + expect(new_project.errors.full_messages).to include(_('The project is still being deleted. Please try again later.')) end end @@ -862,6 +863,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to delegate_method(:environments_access_level).to(:project_feature) } it { is_expected.to delegate_method(:feature_flags_access_level).to(:project_feature) } it { is_expected.to delegate_method(:releases_access_level).to(:project_feature) } + it { is_expected.to delegate_method(:infrastructure_access_level).to(:project_feature) } it { is_expected.to delegate_method(:maven_package_requests_forwarding).to(:namespace) } it { is_expected.to delegate_method(:pypi_package_requests_forwarding).to(:namespace) } it { is_expected.to delegate_method(:npm_package_requests_forwarding).to(:namespace) } @@ -1352,7 +1354,7 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#open_issues_count', :aggregate_failures do + describe '#open_issues_count' do let(:project) { build(:project) } it 'provides the issue count' do @@ -1657,6 +1659,33 @@ RSpec.describe Project, factory_default: :keep do expect(project.reload.star_count).to eq(0) end + it 'does not count stars from blocked users' do + user1 = create(:user) + user2 = create(:user) + project = create(:project, :public) + + expect(project.star_count).to eq(0) + + user1.toggle_star(project) + expect(project.reload.star_count).to eq(1) + + user2.toggle_star(project) + project.reload + expect(project.reload.star_count).to eq(2) + + user1.block + project.reload + expect(project.reload.star_count).to eq(1) + + user2.block + project.reload + expect(project.reload.star_count).to eq(0) + + user1.activate + project.reload + expect(project.reload.star_count).to eq(1) + end + it 'counts stars on the right project' do user = create(:user) project1 = create(:project, :public) @@ -2981,44 +3010,6 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#uses_external_project_ci_config?' do - subject(:uses_external_project_ci_config) { project.uses_external_project_ci_config? } - - let(:project) { build(:project) } - - context 'when ci_config_path is configured with external project' do - before do - project.ci_config_path = '.gitlab-ci.yml@hello/world' - end - - it { is_expected.to eq(true) } - end - - context 'when ci_config_path is nil' do - before do - project.ci_config_path = nil - end - - it { is_expected.to eq(false) } - end - - context 'when ci_config_path is configured with a file in the project' do - before do - project.ci_config_path = 'hello/world/gitlab-ci.yml' - end - - it { is_expected.to eq(false) } - end - - context 'when ci_config_path is configured with remote file' do - before do - project.ci_config_path = 'https://example.org/file.yml' - end - - it { is_expected.to eq(false) } - end - end - describe '#latest_successful_build_for_ref' do let_it_be(:project) { create(:project, :repository) } let_it_be(:pipeline) { create_pipeline(project) } @@ -4507,7 +4498,7 @@ RSpec.describe Project, factory_default: :keep do project_2 = create(:project, :public, :merge_requests_disabled) project_3 = create(:project, :public, :issues_disabled) project_4 = create(:project, :public) - project_4.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE ) + project_4.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE) project_ids = described_class.ids_with_issuables_available_for(user).pluck(:id) @@ -5798,7 +5789,7 @@ RSpec.describe Project, factory_default: :keep do end describe '#has_active_integrations?' do - let_it_be(:project) { create(:project) } + let_it_be_with_refind(:project) { create(:project) } it { expect(project.has_active_integrations?).to eq(false) } @@ -5808,6 +5799,20 @@ RSpec.describe Project, factory_default: :keep do expect(project.has_active_integrations?(:merge_request_hooks)).to eq(false) expect(project.has_active_integrations?).to eq(true) end + + it 'caches matching integrations' do + create(:custom_issue_tracker_integration, push_events: true, merge_requests_events: false, project: project) + + expect(project.has_active_integrations?(:merge_request_hooks)).to eq(false) + expect(project.has_active_integrations?).to eq(true) + + count = ActiveRecord::QueryRecorder.new do + expect(project.has_active_integrations?(:merge_request_hooks)).to eq(false) + expect(project.has_active_integrations?).to eq(true) + end.count + + expect(count).to eq(0) + end end describe '#badges' do @@ -7074,7 +7079,7 @@ RSpec.describe Project, factory_default: :keep do subject { project.self_monitoring? } - context 'when the project is instance self monitoring' do + context 'when the project is instance self-monitoring' do before do stub_application_setting(self_monitoring_project_id: project.id) end @@ -7082,7 +7087,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to be true } end - context 'when the project is not self monitoring' do + context 'when the project is not self-monitoring' do it { is_expected.to be false } end end @@ -7124,7 +7129,7 @@ RSpec.describe Project, factory_default: :keep do describe '#export_in_progress?' do let(:project) { build(:project) } - let!(:project_export_job ) { create(:project_export_job, project: project) } + let!(:project_export_job) { create(:project_export_job, project: project) } context 'when project export is enqueued' do it { expect(project.export_in_progress?).to be false } @@ -7149,7 +7154,7 @@ RSpec.describe Project, factory_default: :keep do describe '#export_status' do let(:project) { build(:project) } - let!(:project_export_job ) { create(:project_export_job, project: project) } + let!(:project_export_job) { create(:project_export_job, project: project) } context 'when project export is enqueued' do it { expect(project.export_status).to eq :queued } @@ -7173,7 +7178,7 @@ RSpec.describe Project, factory_default: :keep do end context 'when project export is being regenerated' do - let!(:new_project_export_job ) { create(:project_export_job, project: project) } + let!(:new_project_export_job) { create(:project_export_job, project: project) } before do finish_job(project_export_job) @@ -7475,15 +7480,6 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#ci_config_external_project' do - subject(:ci_config_external_project) { project.ci_config_external_project } - - let(:other_project) { create(:project) } - let(:project) { build(:project, ci_config_path: ".gitlab-ci.yml@#{other_project.full_path}") } - - it { is_expected.to eq(other_project) } - end - describe '#enabled_group_deploy_keys' do let_it_be(:project) { create(:project) } diff --git a/spec/models/projects/wiki_repository_spec.rb b/spec/models/projects/wiki_repository_spec.rb new file mode 100644 index 00000000000..6868e1f5fb9 --- /dev/null +++ b/spec/models/projects/wiki_repository_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::WikiRepository do + subject { described_class.new(project: build(:project)) } + + describe 'associations' do + it { is_expected.to belong_to(:project).inverse_of(:wiki_repository) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_uniqueness_of(:project) } + end +end diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index b88367b9ca2..b623d534f29 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -7,11 +7,54 @@ RSpec.describe ProtectedBranch do describe 'Associations' do it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:group) } end describe 'Validation' do - it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:name) } + + describe '#validate_either_project_or_top_group' do + context 'when protected branch does not have project or group association' do + it 'validate failed' do + subject.assign_attributes(project: nil, group: nil) + subject.validate + + expect(subject.errors).to include(:base) + end + end + + context 'when protected branch is associated with both project and group' do + it 'validate failed' do + subject.assign_attributes(project: build(:project), group: build(:group)) + subject.validate + + expect(subject.errors).to include(:base) + end + end + + context 'when protected branch is associated with a subgroup' do + it 'validate failed' do + subject.assign_attributes(project: nil, group: build(:group, :nested)) + subject.validate + + expect(subject.errors).to include(:base) + end + end + end + end + + describe 'set a group' do + context 'when associated with group' do + it 'create successfully' do + expect { subject.group = build(:group) }.not_to raise_error + end + end + + context 'when associated with other namespace' do + it 'create failed with `ActiveRecord::AssociationTypeMismatch`' do + expect { subject.group = build(:namespace) }.to raise_error(ActiveRecord::AssociationTypeMismatch) + end + end end describe "#matches?" do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 6fbf69ec23a..93872bcd827 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' RSpec.describe Repository do include RepoHelpers - include GitHelpers TestBlob = Struct.new(:path) @@ -463,7 +462,7 @@ RSpec.describe Repository do repository.delete_branch(branch) expect(subject).not_to be_empty - expect(subject).to all( be_a(::Commit) ) + expect(subject).to all(be_a(::Commit)) expect(subject.size).to eq(1) end end @@ -482,7 +481,7 @@ RSpec.describe Repository do end it 'returns only Commit instances' do - expect(subject).to all( be_a(Commit) ) + expect(subject).to all(be_a(Commit)) end context 'when some commits are not found ' do @@ -2978,7 +2977,7 @@ RSpec.describe Repository do it 'returns false for invalid commit IDs' do expect(repository.ancestor?(commit.id, Gitlab::Git::BLANK_SHA)).to eq(false) - expect(repository.ancestor?( Gitlab::Git::BLANK_SHA, commit.id)).to eq(false) + expect(repository.ancestor?(Gitlab::Git::BLANK_SHA, commit.id)).to eq(false) end end diff --git a/spec/models/serverless/domain_cluster_spec.rb b/spec/models/serverless/domain_cluster_spec.rb index fdae0483c19..487385c62c1 100644 --- a/spec/models/serverless/domain_cluster_spec.rb +++ b/spec/models/serverless/domain_cluster_spec.rb @@ -5,6 +5,16 @@ require 'spec_helper' RSpec.describe ::Serverless::DomainCluster do subject { create(:serverless_domain_cluster) } + describe 'default values' do + subject(:domain_cluster) { build(:serverless_domain_cluster) } + + before do + allow(::Serverless::Domain).to receive(:generate_uuid).and_return('randomtoken') + end + + it { expect(domain_cluster.uuid).to eq('randomtoken') } + end + describe 'validations' do it { is_expected.to validate_presence_of(:pages_domain) } it { is_expected.to validate_presence_of(:knative) } diff --git a/spec/models/spam_log_spec.rb b/spec/models/spam_log_spec.rb index a40c7c5c892..564710b31d0 100644 --- a/spec/models/spam_log_spec.rb +++ b/spec/models/spam_log_spec.rb @@ -21,37 +21,18 @@ RSpec.describe SpamLog do end context 'when admin mode is enabled', :enable_admin_mode do - context 'when user_destroy_with_limited_execution_time_worker is enabled' do - it 'initiates user removal', :sidekiq_inline do - spam_log = build(:spam_log) - user = spam_log.user - - perform_enqueued_jobs do - spam_log.remove_user(deleted_by: admin) - end - - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin) - ).to be_exists - end - end + it 'initiates user removal', :sidekiq_inline do + spam_log = build(:spam_log) + user = spam_log.user - context 'when user_destroy_with_limited_execution_time_worker is disabled' do - before do - stub_feature_flags(user_destroy_with_limited_execution_time_worker: false) + perform_enqueued_jobs do + spam_log.remove_user(deleted_by: admin) end - it 'removes the user', :sidekiq_inline do - spam_log = build(:spam_log) - user = spam_log.user - - perform_enqueued_jobs do - spam_log.remove_user(deleted_by: admin) - end - - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - end + expect( + Users::GhostUserMigration.where(user: user, + initiator_user: admin) + ).to be_exists end end diff --git a/spec/models/terraform/state_spec.rb b/spec/models/terraform/state_spec.rb index a484952bfe9..533e6e4bd7b 100644 --- a/spec/models/terraform/state_spec.rb +++ b/spec/models/terraform/state_spec.rb @@ -10,6 +10,12 @@ RSpec.describe Terraform::State do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:project_id) } + it { is_expected.to validate_presence_of(:uuid) } + + describe 'default values' do + it { expect(described_class.new.uuid).to be_present } + it { expect(described_class.new(uuid: 'test').uuid).to eq('test') } + end describe 'scopes' do describe '.ordered_by_name' do diff --git a/spec/models/terraform/state_version_spec.rb b/spec/models/terraform/state_version_spec.rb index 22b1397f30a..477041117cb 100644 --- a/spec/models/terraform/state_version_spec.rb +++ b/spec/models/terraform/state_version_spec.rb @@ -10,6 +10,15 @@ RSpec.describe Terraform::StateVersion do it { is_expected.to belong_to(:created_by_user).class_name('User').optional } it { is_expected.to belong_to(:build).class_name('Ci::Build').optional } + describe 'default attributes' do + before do + allow(Terraform::StateUploader).to receive(:default_store).and_return(5) + end + + it { expect(described_class.new.file_store).to eq(5) } + it { expect(described_class.new(file_store: 3).file_store).to eq(3) } + end + describe 'scopes' do describe '.ordered_by_version_desc' do let(:terraform_state) { create(:terraform_state) } diff --git a/spec/models/time_tracking/timelog_category_spec.rb b/spec/models/time_tracking/timelog_category_spec.rb index d8b938e9d68..ac2fb651134 100644 --- a/spec/models/time_tracking/timelog_category_spec.rb +++ b/spec/models/time_tracking/timelog_category_spec.rb @@ -7,6 +7,10 @@ RSpec.describe TimeTracking::TimelogCategory, type: :model do it { is_expected.to belong_to(:namespace).with_foreign_key('namespace_id') } end + describe 'default values' do + it { expect(described_class.new.color).to eq(described_class::DEFAULT_COLOR) } + end + describe 'validations' do subject { create(:timelog_category) } diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 18b0cb36cc6..23ba0be2fbc 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -498,7 +498,7 @@ RSpec.describe Todo do describe '.for_internal_notes' do it 'returns todos created from internal notes' do - internal_note = create(:note, confidential: true ) + internal_note = create(:note, confidential: true) todo = create(:todo, note: internal_note) create(:todo) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8ebf3d70165..7207ee0b172 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -144,6 +144,7 @@ RSpec.describe User do it { is_expected.to have_many(:callouts).class_name('Users::Callout') } it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout') } it { is_expected.to have_many(:project_callouts).class_name('Users::ProjectCallout') } + it { is_expected.to have_many(:created_projects).dependent(:nullify).class_name('Project') } describe '#user_detail' do it 'does not persist `user_detail` by default' do @@ -2481,6 +2482,30 @@ RSpec.describe User do end end + describe 'starred_projects' do + let_it_be(:project) { create(:project) } + + before do + user.toggle_star(project) + end + + context 'when blocking a user' do + let_it_be(:user) { create(:user) } + + it 'decrements star count of project' do + expect { user.block }.to change { project.reload.star_count }.by(-1) + end + end + + context 'when activating a user' do + let_it_be(:user) { create(:user, :blocked) } + + it 'increments star count of project' do + expect { user.activate }.to change { project.reload.star_count }.by(1) + end + end + end + describe '.instance_access_request_approvers_to_be_notified' do let_it_be(:admin_issue_board_list) { create_list(:user, 12, :admin, :with_sign_ins) } @@ -3031,6 +3056,10 @@ RSpec.describe User do it 'returns no matches for nil' do expect(described_class.search(nil)).to be_empty end + + it 'returns no matches for an array' do + expect(described_class.search(%w[the test])).to be_empty + end end describe '.user_search_minimum_char_limit' do @@ -6157,172 +6186,28 @@ RSpec.describe User do end end - describe '#authenticatable_salt' do - let(:user) { create(:user) } - - subject(:authenticatable_salt) { user.authenticatable_salt } - - it 'uses password_salt' do - expect(authenticatable_salt).to eq(user.password_salt) - end - - context 'when the encrypted_password is an unknown type' do - let(:encrypted_password) { '$argon2i$v=19$m=512,t=4,p=2$eM+ZMyYkpDRGaI3xXmuNcQ$c5DeJg3eb5dskVt1mDdxfw' } - - before do - user.update_attribute(:encrypted_password, encrypted_password) - end - - it 'returns the first 30 characters of the encrypted_password' do - expect(authenticatable_salt).to eq(encrypted_password[0, 29]) - end - end - - context 'when pbkdf2_password_encryption is disabled' do - before do - stub_feature_flags(pbkdf2_password_encryption: false) - end - - it 'returns the first 30 characters of the encrypted_password' do - expect(authenticatable_salt).to eq(user.encrypted_password[0, 29]) - end - end - end - - def compare_pbkdf2_password(user, password) - Devise::Pbkdf2Encryptable::Encryptors::Pbkdf2Sha512.compare(user.encrypted_password, password) - end - describe '#valid_password?' do subject(:validate_password) { user.valid_password?(password) } - context 'user with password not in disallowed list' do - let(:user) { create(:user) } - let(:password) { user.password } - - it { is_expected.to be_truthy } - - context 'using a wrong password' do - let(:password) { 'WRONG PASSWORD' } - - it { is_expected.to be_falsey } - end - - context 'when pbkdf2_sha512_encryption is disabled and the user password is pbkdf2+sha512' do - it 'does not validate correctly' do - user # Create the user while the feature is enabled - stub_feature_flags(pbkdf2_password_encryption: false) - - expect(validate_password).to be_falsey - end - end - end - context 'user with disallowed password' do let(:user) { create(:user, :disallowed_password) } let(:password) { user.password } - it { is_expected.to be_falsey } - - context 'using a wrong password' do - let(:password) { 'WRONG PASSWORD' } - - it { is_expected.to be_falsey } - end - end - - context 'user with a bcrypt password hash' do - # Manually set a 'known' encrypted password - let(:password) { User.random_password } - let(:encrypted_password) { Devise::Encryptor.digest(User, password) } - let(:user) { create(:user, encrypted_password: encrypted_password) } - - shared_examples 'not re-encrypting with PBKDF2' do - it 'does not re-encrypt with PBKDF2' do - validate_password - - expect(user.reload.encrypted_password).to eq(encrypted_password) - end - end - - context 'using the wrong password' do - # password 'WRONG PASSWORD' will not match the bcrypt hash - let(:password) { 'WRONG PASSWORD' } - let(:encrypted_password) { Devise::Encryptor.digest(User, User.random_password) } - - it { is_expected.to be_falsey } - - it_behaves_like 'not re-encrypting with PBKDF2' - - context 'when pbkdf2_password_encryption is disabled' do - before do - stub_feature_flags(pbkdf2_password_encryption: false) - end - - it { is_expected.to be_falsey } - - it_behaves_like 'not re-encrypting with PBKDF2' - end - end - - context 'using the correct password' do - it { is_expected.to be_truthy } - - it 'validates the password and re-encrypts with PBKDF2' do - validate_password - - current_encrypted_password = user.reload.encrypted_password - - expect(compare_pbkdf2_password(user, password)).to eq(true) - expect { ::BCrypt::Password.new(current_encrypted_password) } - .to raise_error(::BCrypt::Errors::InvalidHash) - end - - context 'when pbkdf2_password_encryption is disabled' do - before do - stub_feature_flags(pbkdf2_password_encryption: false) - end - - it { is_expected.to be_truthy } - - it_behaves_like 'not re-encrypting with PBKDF2' - end - - context 'when pbkdf2_password_encryption_write is disabled' do - before do - stub_feature_flags(pbkdf2_password_encryption_write: false) - end - - it { is_expected.to be_truthy } - - it_behaves_like 'not re-encrypting with PBKDF2' - end - end + it { is_expected.to eq(false) } end - context 'user with password hash that is neither PBKDF2 nor BCrypt' do - # Manually calculated User.random_password - let(:password) { "gg_w215TmVXGWSt7RJKXwYTVz886f6SDM3zvzztaJf2mX9ttUE8gRkNJSbWyWRLqxz4LFzxBekPe75ydDcGauE9wqg-acKMRT-WpSYjTm1Rdx-tnssE7CQByJcnxwWNH" } - # Created with https://argon2.online/ using 'aaaaaaaa' as the salt - let(:encrypted_password) { "$argon2i$v=19$m=512,t=4,p=2$YWFhYWFhYWE$PvJscKO5XRlevcgRReUg6w" } - let(:user) { create(:user, encrypted_password: encrypted_password) } - - it { is_expected.to be_falsey } - - context 'when pbkdf2_password_encryption is disabled' do - before do - stub_feature_flags(pbkdf2_password_encryption: false) - end + context 'using a wrong password' do + let(:user) { create(:user) } + let(:password) { 'WRONG PASSWORD' } - it { is_expected.to be_falsey } - end + it { is_expected.to eq(false) } end context 'user with autogenerated_password' do let(:user) { build_stubbed(:user, password_automatically_set: true) } let(:password) { user.password } - it { is_expected.to be_falsey } + it { is_expected.to eq(false) } end end @@ -6377,95 +6262,6 @@ RSpec.describe User do end end - # These entire test section can be removed once the :pbkdf2_password_encryption feature flag is removed. - describe '#password=' do - let(:user) { create(:user) } - let(:password) { User.random_password } - - def compare_bcrypt_password(user, password) - Devise::Encryptor.compare(User, user.encrypted_password, password) - end - - context 'when pbkdf2_password_encryption is enabled' do - it 'calls PBKDF2 digest and not the default Devise encryptor' do - expect(Devise::Pbkdf2Encryptable::Encryptors::Pbkdf2Sha512).to receive(:digest).at_least(:once).and_call_original - expect(Devise::Encryptor).not_to receive(:digest) - - user.password = password - end - - it 'saves the password in PBKDF2 format' do - user.password = password - user.save! - - expect(compare_pbkdf2_password(user, password)).to eq(true) - expect { compare_bcrypt_password(user, password) }.to raise_error(::BCrypt::Errors::InvalidHash) - end - - context 'when pbkdf2_password_encryption_write is disabled' do - before do - stub_feature_flags(pbkdf2_password_encryption_write: false) - end - - it 'calls default Devise encryptor and not the PBKDF2 encryptor' do - expect(Devise::Encryptor).to receive(:digest).at_least(:once).and_call_original - expect(Devise::Pbkdf2Encryptable::Encryptors::Pbkdf2Sha512).not_to receive(:digest) - - user.password = password - end - end - end - - context 'when pbkdf2_password_encryption is disabled' do - before do - stub_feature_flags(pbkdf2_password_encryption: false) - end - - it 'calls default Devise encryptor and not the PBKDF2 encryptor' do - expect(Devise::Encryptor).to receive(:digest).at_least(:once).and_call_original - expect(Devise::Pbkdf2Encryptable::Encryptors::Pbkdf2Sha512).not_to receive(:digest) - - user.password = password - end - - it 'saves the password in BCrypt format' do - user.password = password - user.save! - - expect { compare_pbkdf2_password(user, password) }.to raise_error Devise::Pbkdf2Encryptable::Encryptors::InvalidHash - expect(compare_bcrypt_password(user, password)).to eq(true) - end - end - end - - describe '#password_strategy' do - let(:user) { create(:user, encrypted_password: encrypted_password) } - - context 'with a PBKDF2+SHA512 encrypted password' do - let(:encrypted_password) { '$pbkdf2-sha512$20000$boHGAw0hEyI$DBA67J7zNZebyzLtLk2X9wRDbmj1LNKVGnZLYyz6PGrIDGIl45fl/BPH0y1TPZnV90A20i.fD9C3G9Bp8jzzOA' } - - it 'extracts the correct strategy', :aggregate_failures do - expect(user.password_strategy).to eq(:pbkdf2_sha512) - end - end - - context 'with a BCrypt encrypted password' do - let(:encrypted_password) { '$2a$10$xLTxCKOa75IU4RQGqqOrTuZOgZdJEzfSzjG6ZSEi/C31TB/yLZYpi' } - - it 'extracts the correct strategy', :aggregate_failures do - expect(user.password_strategy).to eq(:bcrypt) - end - end - - context 'with an unknown encrypted password' do - let(:encrypted_password) { '$pbkdf2-sha256$6400$.6UI/S.nXIk8jcbdHx3Fhg$98jZicV16ODfEsEZeYPGHU3kbrUrvUEXOPimVSQDD44' } - - it 'returns unknown strategy' do - expect(user.password_strategy).to eq(:unknown) - end - end - end - describe '#password_expired?' do let(:user) { build(:user, password_expires_at: password_expires_at) } diff --git a/spec/models/users/calloutable_spec.rb b/spec/models/users/calloutable_spec.rb index 791fe1c1bc4..7e186445c1b 100644 --- a/spec/models/users/calloutable_spec.rb +++ b/spec/models/users/calloutable_spec.rb @@ -15,8 +15,8 @@ RSpec.describe Users::Calloutable do describe '#dismissed_after?' do let(:some_feature_name) { Users::Callout.feature_names.keys.second } - let(:callout_dismissed_month_ago) { create(:callout, feature_name: some_feature_name, dismissed_at: 1.month.ago ) } - let(:callout_dismissed_day_ago) { create(:callout, feature_name: some_feature_name, dismissed_at: 1.day.ago ) } + let(:callout_dismissed_month_ago) { create(:callout, feature_name: some_feature_name, dismissed_at: 1.month.ago) } + let(:callout_dismissed_day_ago) { create(:callout, feature_name: some_feature_name, dismissed_at: 1.day.ago) } it 'returns whether a callout dismissed after specified date' do expect(callout_dismissed_month_ago.dismissed_after?(15.days.ago)).to eq(false) diff --git a/spec/models/users/ghost_user_migration_spec.rb b/spec/models/users/ghost_user_migration_spec.rb index d4a0657c3be..a0b2af6175a 100644 --- a/spec/models/users/ghost_user_migration_spec.rb +++ b/spec/models/users/ghost_user_migration_spec.rb @@ -8,7 +8,18 @@ RSpec.describe Users::GhostUserMigration do it { is_expected.to belong_to(:initiator_user) } end - describe 'validation' do + describe 'validations' do it { is_expected.to validate_presence_of(:user_id) } end + + describe 'scopes' do + describe '.consume_order' do + let!(:ghost_user_migration_1) { create(:ghost_user_migration, consume_after: Time.current) } + let!(:ghost_user_migration_2) { create(:ghost_user_migration, consume_after: 5.minutes.ago) } + + subject { described_class.consume_order.to_a } + + it { is_expected.to eq([ghost_user_migration_2, ghost_user_migration_1]) } + end + end end diff --git a/spec/models/users/namespace_commit_email_spec.rb b/spec/models/users/namespace_commit_email_spec.rb new file mode 100644 index 00000000000..696dac25f9b --- /dev/null +++ b/spec/models/users/namespace_commit_email_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::NamespaceCommitEmail, type: :model do + subject { build(:namespace_commit_email) } + + describe 'associations' do + it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:namespace) } + it { is_expected.to belong_to(:email) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_presence_of(:namespace) } + it { is_expected.to validate_presence_of(:email) } + end + + it { is_expected.to be_valid } +end diff --git a/spec/models/users_star_project_spec.rb b/spec/models/users_star_project_spec.rb index e41519a2b69..60ec108f77d 100644 --- a/spec/models/users_star_project_spec.rb +++ b/spec/models/users_star_project_spec.rb @@ -3,5 +3,85 @@ require 'spec_helper' RSpec.describe UsersStarProject, type: :model do + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:user_active) { create(:user, state: 'active', name: 'user2', private_profile: true) } + let_it_be(:user_blocked) { create(:user, state: 'blocked', name: 'user1') } + it { is_expected.to belong_to(:project).touch(false) } + + describe 'scopes' do + let_it_be(:users_star_project1) { create(:users_star_project, project: project1, user: user_active) } + let_it_be(:users_star_project2) { create(:users_star_project, project: project2, user: user_blocked) } + + describe '.all' do + it 'returns all records' do + expect(described_class.all).to contain_exactly(users_star_project1, users_star_project2) + end + end + + describe '.with_active_user' do + it 'returns only records of active users' do + expect(described_class.with_active_user).to contain_exactly(users_star_project1) + end + end + + describe '.order_user_name_asc' do + it 'sorts records by ascending user name' do + expect(described_class.order_user_name_asc).to eq([users_star_project2, users_star_project1]) + end + end + + describe '.order_user_name_desc' do + it 'sorts records by descending user name' do + expect(described_class.order_user_name_desc).to eq([users_star_project1, users_star_project2]) + end + end + + describe '.by_project' do + it 'returns only records of given project' do + expect(described_class.by_project(project2)).to contain_exactly(users_star_project2) + end + end + + describe '.with_public_profile' do + it 'returns only records of users with public profile' do + expect(described_class.with_public_profile).to contain_exactly(users_star_project2) + end + end + end + + describe 'star count hooks' do + context 'on after_create' do + context 'if user is active' do + it 'increments star count of project' do + expect { user_active.toggle_star(project1) }.to change { project1.reload.star_count }.by(1) + end + end + + context 'if user is not active' do + it 'does not increment star count of project' do + expect { user_blocked.toggle_star(project1) }.not_to change { project1.reload.star_count } + end + end + end + + context 'on after_destory' do + context 'if user is active' do + let_it_be(:users_star_project) { create(:users_star_project, project: project2, user: user_active) } + + it 'decrements star count of project' do + expect { users_star_project.destroy! }.to change { project2.reload.star_count }.by(-1) + end + end + + context 'if user is not active' do + let_it_be(:users_star_project) { create(:users_star_project, project: project2, user: user_blocked) } + + it 'does not decrement star count of project' do + expect { users_star_project.destroy! }.not_to change { project2.reload.star_count } + end + end + end + end end diff --git a/spec/models/work_items/type_spec.rb b/spec/models/work_items/type_spec.rb index e41df7f0f61..6685720778a 100644 --- a/spec/models/work_items/type_spec.rb +++ b/spec/models/work_items/type_spec.rb @@ -35,10 +35,10 @@ RSpec.describe WorkItems::Type do it 'deletes type but not unrelated issues' do type = create(:work_item_type) - expect(WorkItems::Type.count).to eq(6) + expect(WorkItems::Type.count).to eq(8) expect { type.destroy! }.not_to change(Issue, :count) - expect(WorkItems::Type.count).to eq(5) + expect(WorkItems::Type.count).to eq(7) end end @@ -69,7 +69,8 @@ RSpec.describe WorkItems::Type do ::WorkItems::Widgets::Hierarchy, ::WorkItems::Widgets::Labels, ::WorkItems::Widgets::Assignees, - ::WorkItems::Widgets::StartAndDueDate + ::WorkItems::Widgets::StartAndDueDate, + ::WorkItems::Widgets::Milestone ) end end diff --git a/spec/models/work_items/widgets/hierarchy_spec.rb b/spec/models/work_items/widgets/hierarchy_spec.rb index cd528772710..c847f2694fe 100644 --- a/spec/models/work_items/widgets/hierarchy_spec.rb +++ b/spec/models/work_items/widgets/hierarchy_spec.rb @@ -26,22 +26,6 @@ RSpec.describe WorkItems::Widgets::Hierarchy do subject { described_class.new(parent_link.work_item).parent } it { is_expected.to eq(parent_link.work_item_parent) } - - context 'when work_items flag is disabled' do - before do - stub_feature_flags(work_items: false) - end - - it { is_expected.to be_nil } - end - - context 'when work_items flag is enabled for the parent group' do - before do - stub_feature_flags(work_items: group) - end - - it { is_expected.to eq(parent_link.work_item_parent) } - end end describe '#children' do @@ -51,21 +35,5 @@ RSpec.describe WorkItems::Widgets::Hierarchy do subject { described_class.new(work_item_parent).children } it { is_expected.to contain_exactly(parent_link1.work_item, parent_link2.work_item) } - - context 'when work_items flag is disabled' do - before do - stub_feature_flags(work_items: false) - end - - it { is_expected.to be_empty } - end - - context 'when work_items flag is enabled for the parent group' do - before do - stub_feature_flags(work_items: group) - end - - it { is_expected.to contain_exactly(parent_link1.work_item, parent_link2.work_item) } - end end end diff --git a/spec/models/work_items/widgets/milestone_spec.rb b/spec/models/work_items/widgets/milestone_spec.rb new file mode 100644 index 00000000000..7b2d661df29 --- /dev/null +++ b/spec/models/work_items/widgets/milestone_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::Widgets::Milestone do + let_it_be(:project) { create(:project) } + let_it_be(:milestone) { create(:milestone, project: project) } + let_it_be(:work_item) { create(:work_item, :issue, project: project, milestone: milestone) } + + describe '.type' do + subject { described_class.type } + + it { is_expected.to eq(:milestone) } + end + + describe '#type' do + subject { described_class.new(work_item).type } + + it { is_expected.to eq(:milestone) } + end + + describe '#milestone' do + subject { described_class.new(work_item).milestone } + + it { is_expected.to eq(work_item.milestone) } + end +end |