diff options
| author | Achilleas Pipinellis <axil@gitlab.com> | 2019-08-20 20:22:43 +0200 |
|---|---|---|
| committer | Achilleas Pipinellis <axil@gitlab.com> | 2019-08-20 20:22:43 +0200 |
| commit | e61308ce1d82e12e5087371469baea4a452875d1 (patch) | |
| tree | 62551a3ae4eab75e5af7e3b35358c07a51b2132f /spec/models | |
| parent | 4f323bb62fbe71a4352de25cab141f361a3fe1a6 (diff) | |
| parent | 2989ed078c1d45b0959dcecb1bc3c8f4740a3c0d (diff) | |
| download | gitlab-ce-docs-patch-71.tar.gz | |
Merge branch 'master' into docs-patch-71docs-patch-71
Diffstat (limited to 'spec/models')
109 files changed, 3991 insertions, 2074 deletions
diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index 2762eaeccd3..2a689754ee0 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -114,7 +114,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do redis.sadd("session:lookup:user:gitlab:#{user.id}", session_ids) end - expect(ActiveSession.session_ids_for_user(user)).to eq(session_ids) + expect(ActiveSession.session_ids_for_user(user.id)).to eq(session_ids) end end @@ -132,6 +132,19 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do expect(ActiveSession.sessions_from_ids([])).to eq([]) end + + it 'uses redis lookup in batches' do + stub_const('ActiveSession::SESSION_BATCH_SIZE', 1) + + redis = double(:redis) + expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis) + + sessions = ['session-a', 'session-b'] + mget_responses = sessions.map { |session| [Marshal.dump(session)]} + expect(redis).to receive(:mget).twice.and_return(*mget_responses) + + expect(ActiveSession.sessions_from_ids([1, 2])).to eql(sessions) + end end describe '.set' do diff --git a/spec/models/analytics/cycle_analytics/project_stage_spec.rb b/spec/models/analytics/cycle_analytics/project_stage_spec.rb new file mode 100644 index 00000000000..4e3923e82b1 --- /dev/null +++ b/spec/models/analytics/cycle_analytics/project_stage_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Analytics::CycleAnalytics::ProjectStage do + describe 'associations' do + it { is_expected.to belong_to(:project) } + end +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index f8dc1541dd3..db80b85360f 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -37,6 +37,17 @@ describe ApplicationSetting do it { is_expected.not_to allow_value("myemail@example.com").for(:lets_encrypt_notification_email) } it { is_expected.to allow_value("myemail@test.example.com").for(:lets_encrypt_notification_email) } + it { is_expected.to allow_value(['192.168.1.1'] * 1_000).for(:outbound_local_requests_whitelist) } + it { is_expected.not_to allow_value(['192.168.1.1'] * 1_001).for(:outbound_local_requests_whitelist) } + it { is_expected.to allow_value(['1' * 255]).for(:outbound_local_requests_whitelist) } + it { is_expected.not_to allow_value(['1' * 256]).for(:outbound_local_requests_whitelist) } + it { is_expected.not_to allow_value(['ÄŸitlab.com']).for(:outbound_local_requests_whitelist) } + it { is_expected.to allow_value(['xn--itlab-j1a.com']).for(:outbound_local_requests_whitelist) } + it { is_expected.not_to allow_value(['<h1></h1>']).for(:outbound_local_requests_whitelist) } + it { is_expected.to allow_value(['gitlab.com']).for(:outbound_local_requests_whitelist) } + it { is_expected.not_to allow_value(nil).for(:outbound_local_requests_whitelist) } + it { is_expected.to allow_value([]).for(:outbound_local_requests_whitelist) } + context "when user accepted let's encrypt terms of service" do before do setting.update(lets_encrypt_terms_of_service_accepted: true) @@ -354,36 +365,6 @@ describe ApplicationSetting do end end - describe 'setting Sentry DSNs' do - context 'server DSN' do - it 'strips leading and trailing whitespace' do - subject.update(sentry_dsn: ' http://test ') - - expect(subject.sentry_dsn).to eq('http://test') - end - - it 'handles nil values' do - subject.update(sentry_dsn: nil) - - expect(subject.sentry_dsn).to be_nil - end - end - - context 'client-side DSN' do - it 'strips leading and trailing whitespace' do - subject.update(clientside_sentry_dsn: ' http://test ') - - expect(subject.clientside_sentry_dsn).to eq('http://test') - end - - it 'handles nil values' do - subject.update(clientside_sentry_dsn: nil) - - expect(subject.clientside_sentry_dsn).to be_nil - end - end - end - describe '#disabled_oauth_sign_in_sources=' do before do allow(Devise).to receive(:omniauth_providers).and_return([:github]) diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 4d53e4aad8a..020ada3c47a 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -48,14 +48,14 @@ describe BroadcastMessage do expect(described_class.current).to be_empty end - it 'caches the output of the query' do + it 'caches the output of the query for two weeks' do create(:broadcast_message) - expect(described_class).to receive(:current_and_future_messages).and_call_original.once + expect(described_class).to receive(:current_and_future_messages).and_call_original.twice described_class.current - Timecop.travel(1.year) do + Timecop.travel(3.weeks) do described_class.current end end diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index eb32198265b..a871f9b3fe6 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -23,7 +23,7 @@ describe Ci::Bridge do let(:status) { bridge.detailed_status(user) } it 'returns detailed status object' do - expect(status).to be_a Gitlab::Ci::Status::Success + expect(status).to be_a Gitlab::Ci::Status::Created end end diff --git a/spec/models/ci/build_need_spec.rb b/spec/models/ci/build_need_spec.rb new file mode 100644 index 00000000000..450dd550a8f --- /dev/null +++ b/spec/models/ci/build_need_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::BuildNeed, model: true do + let(:build_need) { build(:ci_build_need) } + + it { is_expected.to belong_to(:build) } + + it { is_expected.to validate_presence_of(:build) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_length_of(:name).is_at_most(128) } +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index d98db024f73..4aac4b640f4 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -19,9 +19,11 @@ describe Ci::Build do it { is_expected.to belong_to(:runner) } it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:erased_by) } - it { is_expected.to have_many(:trace_sections)} + it { is_expected.to have_many(:trace_sections) } + it { is_expected.to have_many(:needs) } it { is_expected.to have_one(:deployment) } - it { is_expected.to have_one(:runner_session)} + it { is_expected.to have_one(:runner_session) } + it { is_expected.to have_many(:job_variables) } it { is_expected.to validate_presence_of(:ref) } it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } @@ -181,6 +183,47 @@ describe Ci::Build do end end + describe '.with_needs' do + let!(:build) { create(:ci_build) } + let!(:build_b) { create(:ci_build) } + let!(:build_need_a) { create(:ci_build_need, build: build) } + let!(:build_need_b) { create(:ci_build_need, build: build_b) } + + context 'when passing build name' do + subject { described_class.with_needs(build_need_a.name) } + + it { is_expected.to contain_exactly(build) } + end + + context 'when not passing any build name' do + subject { described_class.with_needs } + + it { is_expected.to contain_exactly(build, build_b) } + end + + context 'when not matching build name' do + subject { described_class.with_needs('undefined') } + + it { is_expected.to be_empty } + end + end + + describe '.without_needs' do + let!(:build) { create(:ci_build) } + + subject { described_class.without_needs } + + context 'when no build_need is created' do + it { is_expected.to contain_exactly(build) } + end + + context 'when a build_need is created' do + let!(:need_a) { create(:ci_build_need, build: build) } + + it { is_expected.to be_empty } + end + end + describe '#enqueue' do let(:build) { create(:ci_build, :created) } @@ -594,6 +637,59 @@ describe Ci::Build do expect(staging.depends_on_builds.map(&:id)) .to contain_exactly(build.id, retried_rspec.id, rubocop_test.id) end + + describe '#dependencies' do + let(:dependencies) { } + let(:needs) { } + + let!(:final) do + create(:ci_build, + pipeline: pipeline, name: 'final', + stage_idx: 3, stage: 'deploy', options: { + dependencies: dependencies + } + ) + end + + before do + needs.to_a.each do |need| + create(:ci_build_need, build: final, name: need) + end + end + + subject { final.dependencies } + + context 'when depedencies are defined' do + let(:dependencies) { %w(rspec staging) } + + it { is_expected.to contain_exactly(rspec_test, staging) } + end + + context 'when needs are defined' do + let(:needs) { %w(build rspec staging) } + + it { is_expected.to contain_exactly(build, rspec_test, staging) } + + context 'when ci_dag_support is disabled' do + before do + stub_feature_flags(ci_dag_support: false) + end + + it { is_expected.to contain_exactly(build, rspec_test, rubocop_test, staging) } + end + end + + context 'when needs and dependencies are defined' do + let(:dependencies) { %w(rspec staging) } + let(:needs) { %w(build rspec staging) } + + it { is_expected.to contain_exactly(rspec_test, staging) } + end + + context 'when nor dependencies or needs are defined' do + it { is_expected.to contain_exactly(build, rspec_test, rubocop_test, staging) } + end + end end describe '#triggered_by?' do @@ -692,6 +788,34 @@ describe Ci::Build do end end + describe '#has_live_trace?' do + subject { build.has_live_trace? } + + let(:build) { create(:ci_build, :trace_live) } + + it { is_expected.to be_truthy } + + context 'when build does not have live trace' do + let(:build) { create(:ci_build) } + + it { is_expected.to be_falsy } + end + end + + describe '#has_archived_trace?' do + subject { build.has_archived_trace? } + + let(:build) { create(:ci_build, :trace_artifact) } + + it { is_expected.to be_truthy } + + context 'when build does not have archived trace' do + let(:build) { create(:ci_build) } + + it { is_expected.to be_falsy } + end + end + describe '#has_job_artifacts?' do subject { build.has_job_artifacts? } @@ -2013,6 +2137,7 @@ describe Ci::Build do { key: 'CI', value: 'true', public: true, masked: false }, { key: 'GITLAB_CI', value: 'true', public: true, masked: false }, { key: 'GITLAB_FEATURES', value: project.licensed_features.join(','), public: true, masked: false }, + { key: 'CI_SERVER_HOST', value: Gitlab.config.gitlab.host, public: true, masked: false }, { key: 'CI_SERVER_NAME', value: 'GitLab', public: true, masked: false }, { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true, masked: false }, { key: 'CI_SERVER_VERSION_MAJOR', value: Gitlab.version_info.major.to_s, public: true, masked: false }, @@ -2215,6 +2340,32 @@ describe Ci::Build do it_behaves_like 'containing environment variables' end end + + context 'when project has an environment specific variable' do + let(:environment_specific_variable) do + { key: 'MY_STAGING_ONLY_VARIABLE', value: 'environment_specific_variable', public: false, masked: false } + end + + before do + create(:ci_variable, environment_specific_variable.slice(:key, :value) + .merge(project: project, environment_scope: 'stag*')) + end + + it_behaves_like 'containing environment variables' + + context 'when environment scope does not match build environment' do + it { is_expected.not_to include(environment_specific_variable) } + end + + context 'when environment scope matches build environment' do + before do + create(:environment, name: 'staging', project: project) + build.update!(environment: 'staging') + end + + it { is_expected.to include(environment_specific_variable) } + end + end end context 'when build started manually' do @@ -2229,6 +2380,16 @@ describe Ci::Build do it { is_expected.to include(manual_variable) } end + context 'when job variable is defined' do + let(:job_variable) { { key: 'first', value: 'first', public: false, masked: false } } + + before do + create(:ci_job_variable, job_variable.slice(:key, :value).merge(job: build)) + end + + it { is_expected.to include(job_variable) } + end + context 'when build is for tag' do let(:tag_variable) do { key: 'CI_COMMIT_TAG', value: 'master', public: true, masked: false } @@ -3574,6 +3735,7 @@ describe Ci::Build do before do build.ensure_metadata + build.needs.create!(name: 'another-job') end it 'drops metadata' do @@ -3581,6 +3743,7 @@ describe Ci::Build do expect(build.reload).to be_degenerated expect(build.metadata).to be_nil + expect(build.needs).to be_empty end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 1ba66565e03..1413da231e0 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -70,6 +70,31 @@ describe Ci::JobArtifact do end end + describe '.archived_trace_exists_for?' do + subject { described_class.archived_trace_exists_for?(job_id) } + + let!(:artifact) { create(:ci_job_artifact, :trace, job: job) } + let(:job) { create(:ci_build) } + + context 'when the specified job_id exists' do + let(:job_id) { job.id } + + it { is_expected.to be_truthy } + + context 'when the job does have archived trace' do + let!(:artifact) { } + + it { is_expected.to be_falsy } + end + end + + context 'when the specified job_id does not exist' do + let(:job_id) { 10000 } + + it { is_expected.to be_falsy } + end + end + describe 'callbacks' do subject { create(:ci_job_artifact, :archive) } diff --git a/spec/models/ci/job_variable_spec.rb b/spec/models/ci/job_variable_spec.rb new file mode 100644 index 00000000000..b94a914c784 --- /dev/null +++ b/spec/models/ci/job_variable_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::JobVariable do + subject { build(:ci_job_variable) } + + it_behaves_like "CI variable" + + it { is_expected.to belong_to(:job) } + it { is_expected.to validate_uniqueness_of(:key).scoped_to(:job_id) } +end diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index 227870eb27f..aee43025288 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -88,23 +88,8 @@ describe Ci::PipelineSchedule do describe '#set_next_run_at' do let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly) } - let(:ideal_next_run_at) { pipeline_schedule.send(:ideal_next_run_at) } - - let(:expected_next_run_at) do - Gitlab::Ci::CronParser.new(Settings.cron_jobs['pipeline_schedule_worker']['cron'], Time.zone.name) - .next_time_from(ideal_next_run_at) - end - - let(:cron_worker_next_run_at) do - Gitlab::Ci::CronParser.new(Settings.cron_jobs['pipeline_schedule_worker']['cron'], Time.zone.name) - .next_time_from(Time.zone.now) - end - - context 'when creates new pipeline schedule' do - it 'updates next_run_at automatically' do - expect(pipeline_schedule.next_run_at).to eq(expected_next_run_at) - end - end + let(:ideal_next_run_at) { pipeline_schedule.send(:ideal_next_run_from, Time.zone.now) } + let(:cron_worker_next_run_at) { pipeline_schedule.send(:cron_worker_next_run_from, Time.zone.now) } context 'when PipelineScheduleWorker runs at a specific interval' do before do @@ -128,8 +113,10 @@ describe Ci::PipelineSchedule do context 'when pipeline schedule runs every minute' do let(:pipeline_schedule) { create(:ci_pipeline_schedule, :every_minute) } - it "updates next_run_at to the sidekiq worker's execution time", :quarantine do - expect(pipeline_schedule.next_run_at).to eq(cron_worker_next_run_at) + it "updates next_run_at to the sidekiq worker's execution time" do + Timecop.freeze(Time.parse("2019-06-01 12:18:00+0000")) do + expect(pipeline_schedule.next_run_at).to eq(cron_worker_next_run_at) + end end end @@ -155,9 +142,8 @@ describe Ci::PipelineSchedule do let(:new_cron) { '0 0 1 1 *' } it 'updates next_run_at automatically' do - pipeline_schedule.update!(cron: new_cron) - - expect(pipeline_schedule.next_run_at).to eq(expected_next_run_at) + expect { pipeline_schedule.update!(cron: new_cron) } + .to change { pipeline_schedule.next_run_at } end end end @@ -165,21 +151,24 @@ describe Ci::PipelineSchedule do describe '#schedule_next_run!' do let!(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly) } - context 'when reschedules after 10 days from now' do - let(:future_time) { 10.days.from_now } - let(:ideal_next_run_at) { pipeline_schedule.send(:ideal_next_run_at) } + before do + pipeline_schedule.update_column(:next_run_at, nil) + end + + it 'updates next_run_at' do + expect { pipeline_schedule.schedule_next_run! } + .to change { pipeline_schedule.next_run_at } + end - let(:expected_next_run_at) do - Gitlab::Ci::CronParser.new(Settings.cron_jobs['pipeline_schedule_worker']['cron'], Time.zone.name) - .next_time_from(ideal_next_run_at) + context 'when record is invalid' do + before do + allow(pipeline_schedule).to receive(:save!) { raise ActiveRecord::RecordInvalid.new(pipeline_schedule) } end - it 'points to proper next_run_at' do - Timecop.freeze(future_time) do - pipeline_schedule.schedule_next_run! + it 'nullifies the next run at' do + pipeline_schedule.schedule_next_run! - expect(pipeline_schedule.next_run_at).to eq(expected_next_run_at) - end + expect(pipeline_schedule.next_run_at).to be_nil end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index a8701f0efa4..78be4a8131a 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -962,7 +962,11 @@ describe Ci::Pipeline, :mailer do end context 'when kubernetes is active' do - shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do + context 'when user configured kubernetes from CI/CD > Clusters' do + let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } + let(:pipeline) { build(:ci_pipeline, project: project, config: config) } + it 'returns seeds for kubernetes dependent job' do seeds = pipeline.stage_seeds @@ -971,21 +975,6 @@ describe Ci::Pipeline, :mailer do expect(seeds.dig(1, 0, :name)).to eq 'production' end end - - context 'when user configured kubernetes from Integration > Kubernetes' do - let(:project) { create(:kubernetes_project) } - let(:pipeline) { build(:ci_pipeline, project: project, config: config) } - - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' - end - - context 'when user configured kubernetes from CI/CD > Clusters' do - let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } - let(:pipeline) { build(:ci_pipeline, project: project, config: config) } - - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' - end end context 'when kubernetes is not active' do @@ -1679,23 +1668,13 @@ describe Ci::Pipeline, :mailer do describe '#has_kubernetes_active?' do context 'when kubernetes is active' do - shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do - it 'returns true' do - expect(pipeline).to have_kubernetes_active - end - end - - context 'when user configured kubernetes from Integration > Kubernetes' do - let(:project) { create(:kubernetes_project) } - - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' - end - context 'when user configured kubernetes from CI/CD > Clusters' do let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } let(:project) { cluster.project } - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' + it 'returns true' do + expect(pipeline).to have_kubernetes_active + end end end @@ -1820,7 +1799,7 @@ describe Ci::Pipeline, :mailer do end end - describe '.latest_successful_for' do + describe '.latest_successful_for_ref' do include_context 'with some outdated pipelines' let!(:latest_successful_pipeline) do @@ -1828,7 +1807,20 @@ describe Ci::Pipeline, :mailer do end it 'returns the latest successful pipeline' do - expect(described_class.latest_successful_for('ref')) + expect(described_class.latest_successful_for_ref('ref')) + .to eq(latest_successful_pipeline) + end + end + + describe '.latest_successful_for_sha' do + include_context 'with some outdated pipelines' + + let!(:latest_successful_pipeline) do + create_pipeline(:success, 'ref', 'awesomesha', project) + end + + it 'returns the latest successful pipeline' do + expect(described_class.latest_successful_for_sha('awesomesha')) .to eq(latest_successful_pipeline) end end @@ -1907,6 +1899,17 @@ describe Ci::Pipeline, :mailer do end end + describe '.latest_for_shas' do + let(:sha) { 'abc' } + + it 'returns latest pipeline for sha' do + create(:ci_pipeline, sha: sha) + pipeline2 = create(:ci_pipeline, sha: sha) + + expect(described_class.latest_for_shas(sha)).to contain_exactly(pipeline2) + end + end + describe '.latest_successful_ids_per_project' do let(:projects) { create_list(:project, 2) } let!(:pipeline1) { create(:ci_pipeline, :success, project: projects[0]) } @@ -1926,6 +1929,13 @@ describe Ci::Pipeline, :mailer do it { is_expected.to be_an(Array) } end + describe '.bridgeable_statuses' do + subject { described_class.bridgeable_statuses } + + it { is_expected.to be_an(Array) } + it { is_expected.not_to include('created', 'preparing', 'pending') } + end + describe '#status' do let(:build) do create(:ci_build, :created, pipeline: pipeline, name: 'test') @@ -2736,7 +2746,7 @@ describe Ci::Pipeline, :mailer do create(:ci_pipeline, project: project, sha: project.commit('master').sha, - user: create(:user)) + user: project.owner) end before do @@ -3008,4 +3018,28 @@ describe Ci::Pipeline, :mailer do end end end + + describe '#error_messages' do + subject { pipeline.error_messages } + + before do + pipeline.valid? + end + + context 'when pipeline has errors' do + let(:pipeline) { build(:ci_pipeline, sha: nil, ref: nil) } + + it 'returns the full error messages' do + is_expected.to eq("Sha can't be blank and Ref can't be blank") + end + end + + context 'when pipeline does not have errors' do + let(:pipeline) { build(:ci_pipeline) } + + it 'returns empty string' do + is_expected.to be_empty + end + end + end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index f735a89f69f..78b151631c1 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -146,7 +146,7 @@ describe Ci::Runner do expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner) end - context 'with a parent group with a runner', :nested_groups do + context 'with a parent group with a runner' do let(:runner) { create(:ci_runner, :group, groups: [parent_group]) } let(:project) { create(:project, group: group) } let(:group) { create(:group, parent: parent_group) } @@ -554,7 +554,7 @@ describe Ci::Runner do end def expect_value_in_queues - Gitlab::Redis::Queues.with do |redis| + Gitlab::Redis::SharedState.with do |redis| runner_queue_key = runner.send(:runner_queue_key) expect(redis.get(runner_queue_key)) end @@ -627,7 +627,7 @@ describe Ci::Runner do end it 'cleans up the queue' do - Gitlab::Redis::Queues.with do |redis| + Gitlab::Redis::SharedState.with do |redis| expect(redis.get(queue_key)).to be_nil end end diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index fde8375f2a5..5b5d6f51b33 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -54,19 +54,31 @@ describe Ci::Trigger do end describe '#can_access_project?' do + let(:owner) { create(:user) } let(:trigger) { create(:ci_trigger, owner: owner, project: project) } context 'when owner is blank' do - let(:owner) { nil } + before do + stub_feature_flags(use_legacy_pipeline_triggers: false) + trigger.update_attribute(:owner, nil) + end subject { trigger.can_access_project? } - it { is_expected.to eq(true) } + it { is_expected.to eq(false) } + + context 'when :use_legacy_pipeline_triggers feature flag is enabled' do + before do + stub_feature_flags(use_legacy_pipeline_triggers: true) + end + + subject { trigger.can_access_project? } + + it { is_expected.to eq(true) } + end end context 'when owner is set' do - let(:owner) { create(:user) } - subject { trigger.can_access_project? } context 'and is member of the project' do diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index a231c7eaed8..3ff547456c6 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -10,6 +10,7 @@ describe Ci::Variable do describe 'validations' do it { is_expected.to include_module(Presentable) } it { is_expected.to include_module(Maskable) } + it { is_expected.to include_module(HasEnvironmentScope) } it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope).with_message(/\(\w+\) has already been taken/) } end diff --git a/spec/models/clusters/applications/cert_manager_spec.rb b/spec/models/clusters/applications/cert_manager_spec.rb index 8d853a04e33..93050e80b07 100644 --- a/spec/models/clusters/applications/cert_manager_spec.rb +++ b/spec/models/clusters/applications/cert_manager_spec.rb @@ -3,17 +3,17 @@ require 'rails_helper' describe Clusters::Applications::CertManager do - let(:cert_manager) { create(:clusters_applications_cert_managers) } + let(:cert_manager) { create(:clusters_applications_cert_manager) } - include_examples 'cluster application core specs', :clusters_applications_cert_managers - include_examples 'cluster application status specs', :clusters_applications_cert_managers - include_examples 'cluster application version specs', :clusters_applications_cert_managers + include_examples 'cluster application core specs', :clusters_applications_cert_manager + include_examples 'cluster application status specs', :clusters_applications_cert_manager + include_examples 'cluster application version specs', :clusters_applications_cert_manager include_examples 'cluster application initial status specs' describe '#can_uninstall?' do subject { cert_manager.can_uninstall? } - it { is_expected.to be_falsey } + it { is_expected.to be_truthy } end describe '#install_command' do @@ -48,7 +48,7 @@ describe Clusters::Applications::CertManager do expect(subject.version).to eq('v0.5.2') expect(subject).to be_rbac expect(subject.files).to eq(cert_manager.files.merge(cluster_issuer_file)) - expect(subject.postinstall).to eq(['/usr/bin/kubectl create -f /data/helm/certmanager/config/cluster_issuer.yaml']) + expect(subject.postinstall).to eq(['kubectl create -f /data/helm/certmanager/config/cluster_issuer.yaml']) end context 'for a specific user' do @@ -72,7 +72,7 @@ describe Clusters::Applications::CertManager do end context 'application failed to install previously' do - let(:cert_manager) { create(:clusters_applications_cert_managers, :errored, version: '0.0.1') } + let(:cert_manager) { create(:clusters_applications_cert_manager, :errored, version: '0.0.1') } it 'is initialized with the locked version' do expect(subject.version).to eq('v0.5.2') @@ -80,6 +80,44 @@ describe Clusters::Applications::CertManager do end end + describe '#uninstall_command' do + subject { cert_manager.uninstall_command } + + it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::Helm::DeleteCommand) } + + it 'is initialized with cert_manager arguments' do + expect(subject.name).to eq('certmanager') + expect(subject).to be_rbac + expect(subject.files).to eq(cert_manager.files) + end + + it 'specifies a post delete command to remove custom resource definitions' do + expect(subject.postdelete).to eq([ + "kubectl delete secret -n gitlab-managed-apps letsencrypt-prod --ignore-not-found", + 'kubectl delete crd certificates.certmanager.k8s.io --ignore-not-found', + 'kubectl delete crd clusterissuers.certmanager.k8s.io --ignore-not-found', + 'kubectl delete crd issuers.certmanager.k8s.io --ignore-not-found' + ]) + end + + context 'secret key name is not found' do + before do + allow(File).to receive(:read).and_call_original + expect(File).to receive(:read) + .with(Rails.root.join('vendor', 'cert_manager', 'cluster_issuer.yaml')) + .and_return('key: value') + end + + it 'does not try and delete the secret' do + expect(subject.postdelete).to eq([ + 'kubectl delete crd certificates.certmanager.k8s.io --ignore-not-found', + 'kubectl delete crd clusterissuers.certmanager.k8s.io --ignore-not-found', + 'kubectl delete crd issuers.certmanager.k8s.io --ignore-not-found' + ]) + end + end + end + describe '#files' do let(:application) { cert_manager } let(:values) { subject[:'values.yaml'] } diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index 6ea6c110d62..00b5c72a3d3 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -19,11 +19,35 @@ describe Clusters::Applications::Helm do end describe '#can_uninstall?' do - let(:helm) { create(:clusters_applications_helm) } + context "with other existing applications" do + Clusters::Cluster::APPLICATIONS.keys.each do |application_name| + next if application_name == 'helm' + + it "is false when #{application_name} is installed" do + cluster_application = create("clusters_applications_#{application_name}".to_sym) + + helm = cluster_application.cluster.application_helm + + expect(helm.allowed_to_uninstall?).to be_falsy + end + end - subject { helm.can_uninstall? } + it 'executes a single query only' do + cluster_application = create(:clusters_applications_ingress) + helm = cluster_application.cluster.application_helm - it { is_expected.to be_falsey } + query_count = ActiveRecord::QueryRecorder.new { helm.allowed_to_uninstall? }.count + expect(query_count).to eq(1) + end + end + + context "without other existing applications" do + subject { helm.can_uninstall? } + + let(:helm) { create(:clusters_applications_helm) } + + it { is_expected.to be_truthy } + end end describe '#issue_client_cert' do @@ -73,4 +97,41 @@ describe Clusters::Applications::Helm do end end end + + describe '#uninstall_command' do + let(:helm) { create(:clusters_applications_helm) } + + subject { helm.uninstall_command } + + it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::Helm::ResetCommand) } + + it 'has name' do + expect(subject.name).to eq('helm') + end + + it 'has cert files' do + expect(subject.files[:'ca.pem']).to be_present + expect(subject.files[:'ca.pem']).to eq(helm.ca_cert) + + expect(subject.files[:'cert.pem']).to be_present + expect(subject.files[:'key.pem']).to be_present + + cert = OpenSSL::X509::Certificate.new(subject.files[:'cert.pem']) + expect(cert.not_after).to be > 999.years.from_now + end + + describe 'rbac' do + context 'rbac cluster' do + it { expect(subject).to be_rbac } + end + + context 'non rbac cluster' do + before do + helm.cluster.platform_kubernetes.abac! + end + + it { expect(subject).not_to be_rbac } + end + end + end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index 292ddabd2d8..057517d3820 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -21,7 +21,21 @@ describe Clusters::Applications::Ingress do describe '#can_uninstall?' do subject { ingress.can_uninstall? } - it { is_expected.to be_falsey } + it 'returns true if application_jupyter_nil_or_installable? AND external_ip_or_hostname? are true' do + ingress.external_ip = 'IP' + + is_expected.to be_truthy + end + + it 'returns false if application_jupyter_nil_or_installable? is false' do + create(:clusters_applications_jupyter, :installed, cluster: ingress.cluster) + + is_expected.to be_falsey + end + + it 'returns false if external_ip_or_hostname? is false' do + is_expected.to be_falsey + end end describe '#make_installed!' do diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index 43fa1010b2b..3ff66a074e4 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -16,7 +16,7 @@ describe Clusters::Applications::Jupyter do subject { jupyter.can_uninstall? } - it { is_expected.to be_falsey } + it { is_expected.to be_truthy } end describe '#set_initial_status' do diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index b38cf96de7e..334f10526cb 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -39,7 +39,7 @@ describe Clusters::Applications::Knative do describe '#can_uninstall?' do subject { knative.can_uninstall? } - it { is_expected.to be_falsey } + it { is_expected.to be_truthy } end describe '#schedule_status_update with external_ip' do @@ -91,7 +91,7 @@ describe Clusters::Applications::Knative do end it 'does not install metrics for prometheus' do - expect(subject.postinstall).to be_nil + expect(subject.postinstall).to be_empty end context 'with prometheus installed' do @@ -101,7 +101,7 @@ describe Clusters::Applications::Knative do subject { knative.install_command } it 'installs metrics' do - expect(subject.postinstall).not_to be_nil + expect(subject.postinstall).not_to be_empty expect(subject.postinstall.length).to be(1) expect(subject.postinstall[0]).to eql("kubectl apply -f #{Clusters::Applications::Knative::METRICS_CONFIG}") end @@ -112,7 +112,7 @@ describe Clusters::Applications::Knative do subject { knative.install_command } it 'is initialized with latest version' do - expect(subject.version).to eq('0.5.0') + expect(subject.version).to eq('0.6.0') end it_behaves_like 'a command' @@ -129,6 +129,46 @@ describe Clusters::Applications::Knative do it_behaves_like 'a command' end + describe '#uninstall_command' do + subject { knative.uninstall_command } + + it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::Helm::DeleteCommand) } + + it "removes knative deployed services before uninstallation" do + 2.times do |i| + cluster_project = create(:cluster_project, cluster: knative.cluster) + + create(:cluster_kubernetes_namespace, + cluster: cluster_project.cluster, + cluster_project: cluster_project, + project: cluster_project.project, + namespace: "namespace_#{i}") + end + + remove_namespaced_services_script = [ + "kubectl delete ksvc --all -n #{knative.cluster.kubernetes_namespaces.first.namespace}", + "kubectl delete ksvc --all -n #{knative.cluster.kubernetes_namespaces.second.namespace}" + ] + + expect(subject.predelete).to match_array(remove_namespaced_services_script) + end + + it "initializes command with all necessary postdelete script" do + api_resources = YAML.safe_load(File.read(Rails.root.join(Clusters::Applications::Knative::API_RESOURCES_PATH))) + + remove_knative_istio_leftovers_script = [ + "kubectl delete --ignore-not-found ns knative-serving", + "kubectl delete --ignore-not-found ns knative-build" + ] + + full_delete_commands_size = api_resources.size + remove_knative_istio_leftovers_script.size + + expect(subject.postdelete).to include(*remove_knative_istio_leftovers_script) + expect(subject.postdelete.size).to eq(full_delete_commands_size) + expect(subject.postdelete[2]).to eq("kubectl delete --ignore-not-found crd #{api_resources[0]}") + end + end + describe '#files' do let(:application) { knative } let(:values) { subject[:'values.yaml'] } diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 26267c64112..eb6ccba5584 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -86,16 +86,15 @@ describe Clusters::Applications::Prometheus do project: cluster.cluster_project.project) end - it 'creates proxy prometheus rest client' do - expect(subject.prometheus_client).to be_instance_of(RestClient::Resource) + it 'creates proxy prometheus_client' do + expect(subject.prometheus_client).to be_instance_of(Gitlab::PrometheusClient) end - it 'creates proper url' do - expect(subject.prometheus_client.url).to eq("#{kubernetes_url}/api/v1/namespaces/gitlab-managed-apps/services/prometheus-prometheus-server:80/proxy") - end - - it 'copies options and headers from kube client to proxy client' do - expect(subject.prometheus_client.options).to eq(kube_client.rest_client.options.merge(headers: kube_client.headers)) + it 'copies proxy_url, options and headers from kube client to prometheus_client' do + expect(Gitlab::PrometheusClient) + .to(receive(:new)) + .with(a_valid_url, kube_client.rest_client.options.merge(headers: kube_client.headers)) + subject.prometheus_client end context 'when cluster is not reachable' do @@ -142,7 +141,7 @@ describe Clusters::Applications::Prometheus do end it 'does not install knative metrics' do - expect(subject.postinstall).to be_nil + expect(subject.postinstall).to be_empty end context 'with knative installed' do diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 4f0cd0efe9c..4abe45a2152 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -18,7 +18,7 @@ describe Clusters::Applications::Runner do subject { gitlab_runner.can_uninstall? } - it { is_expected.to be_falsey } + it { is_expected.to be_truthy } end describe '#install_command' do @@ -156,4 +156,35 @@ describe Clusters::Applications::Runner do end end end + + describe '#prepare_uninstall' do + it 'pauses associated runner' do + active_runner = create(:ci_runner, contacted_at: 1.second.ago) + + expect(active_runner.status).to eq(:online) + + application_runner = create(:clusters_applications_runner, :scheduled, runner: active_runner) + application_runner.prepare_uninstall + + expect(active_runner.status).to eq(:paused) + end + end + + describe '#make_uninstalling!' do + subject { create(:clusters_applications_runner, :scheduled, runner: ci_runner) } + + it 'calls prepare_uninstall' do + expect_any_instance_of(described_class).to receive(:prepare_uninstall).and_call_original + + subject.make_uninstalling! + end + end + + describe '#post_uninstall' do + it 'destroys its runner' do + application_runner = create(:clusters_applications_runner, :scheduled, runner: ci_runner) + + expect { application_runner.post_uninstall }.to change { Ci::Runner.count }.by(-1) + end + end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index f206bb41f45..9afbe6328ca 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -38,11 +38,6 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to respond_to :project } - it do - expect(subject.knative_services_finder(subject.project)) - .to be_instance_of(Clusters::KnativeServicesFinder) - end - describe '.enabled' do subject { described_class.enabled } @@ -121,26 +116,6 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end - describe '.missing_kubernetes_namespace' do - let!(:cluster) { create(:cluster, :provided_by_gcp, :project) } - let(:project) { cluster.project } - let(:kubernetes_namespaces) { project.kubernetes_namespaces } - - subject do - described_class.joins(:projects).where(projects: { id: project.id }).missing_kubernetes_namespace(kubernetes_namespaces) - end - - it { is_expected.to contain_exactly(cluster) } - - context 'kubernetes namespace exists' do - before do - create(:cluster_kubernetes_namespace, project: project, cluster: cluster) - end - - it { is_expected.to be_empty } - end - end - describe 'validations' do subject { cluster.valid? } @@ -342,7 +317,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end - context 'when sub-group has configured kubernetes cluster', :nested_groups do + context 'when sub-group has configured kubernetes cluster' do let(:sub_group_cluster) { create(:cluster, :provided_by_gcp, :group) } let(:sub_group) { sub_group_cluster.group } let(:project) { create(:project, group: sub_group) } @@ -423,31 +398,6 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end - describe '#all_projects' do - let(:project) { create(:project) } - let(:cluster) { create(:cluster, projects: [project]) } - - subject { cluster.all_projects } - - context 'project cluster' do - it 'returns project' do - is_expected.to eq([project]) - end - end - - context 'group cluster' do - let(:cluster) { create(:cluster, :group) } - let(:group) { cluster.group } - let(:project) { create(:project, group: group) } - let(:subgroup) { create(:group, parent: group) } - let(:subproject) { create(:project, group: subgroup) } - - it 'returns all projects for group' do - is_expected.to contain_exactly(project, subproject) - end - end - end - describe '#first_project' do subject { cluster.first_project } @@ -496,7 +446,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do context 'when applications are created' do let!(:helm) { create(:clusters_applications_helm, cluster: cluster) } let!(:ingress) { create(:clusters_applications_ingress, cluster: cluster) } - let!(:cert_manager) { create(:clusters_applications_cert_managers, cluster: cluster) } + let!(:cert_manager) { create(:clusters_applications_cert_manager, cluster: cluster) } let!(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) } let!(:runner) { create(:clusters_applications_runner, cluster: cluster) } let!(:jupyter) { create(:clusters_applications_jupyter, cluster: cluster) } @@ -514,19 +464,43 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do subject { cluster.allow_user_defined_namespace? } context 'project type cluster' do - it { is_expected.to be_truthy } + context 'gitlab managed' do + it { is_expected.to be_truthy } + end + + context 'not managed' do + let(:cluster) { create(:cluster, :provided_by_gcp, managed: false) } + + it { is_expected.to be_truthy } + end end context 'group type cluster' do - let(:cluster) { create(:cluster, :provided_by_gcp, :group) } + context 'gitlab managed' do + let(:cluster) { create(:cluster, :provided_by_gcp, :group) } + + it { is_expected.to be_falsey } + end + + context 'not managed' do + let(:cluster) { create(:cluster, :provided_by_gcp, :group, managed: false) } - it { is_expected.to be_falsey } + it { is_expected.to be_truthy } + end end context 'instance type cluster' do - let(:cluster) { create(:cluster, :provided_by_gcp, :instance) } + context 'gitlab managed' do + let(:cluster) { create(:cluster, :provided_by_gcp, :instance) } + + it { is_expected.to be_falsey } + end + + context 'not managed' do + let(:cluster) { create(:cluster, :provided_by_gcp, :instance, managed: false) } - it { is_expected.to be_falsey } + it { is_expected.to be_truthy } + end end end @@ -555,6 +529,42 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end + describe '#kubernetes_namespace_for' do + let(:cluster) { create(:cluster, :group) } + let(:environment) { create(:environment) } + + subject { cluster.kubernetes_namespace_for(environment) } + + before do + expect(Clusters::KubernetesNamespaceFinder).to receive(:new) + .with(cluster, project: environment.project, environment_slug: environment.slug) + .and_return(double(execute: persisted_namespace)) + end + + context 'a persisted namespace exists' do + let(:persisted_namespace) { create(:cluster_kubernetes_namespace) } + + it { is_expected.to eq persisted_namespace.namespace } + end + + context 'no persisted namespace exists' do + let(:persisted_namespace) { nil } + let(:namespace_generator) { double } + let(:default_namespace) { 'a-default-namespace' } + + before do + expect(Gitlab::Kubernetes::DefaultNamespace).to receive(:new) + .with(cluster, project: environment.project) + .and_return(namespace_generator) + expect(namespace_generator).to receive(:from_environment_slug) + .with(environment.slug) + .and_return(default_namespace) + end + + it { is_expected.to eq default_namespace } + end + end + describe '#predefined_variables' do subject { cluster.predefined_variables } diff --git a/spec/models/clusters/clusters_hierarchy_spec.rb b/spec/models/clusters/clusters_hierarchy_spec.rb new file mode 100644 index 00000000000..0470ebe17ea --- /dev/null +++ b/spec/models/clusters/clusters_hierarchy_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Clusters::ClustersHierarchy do + describe '#base_and_ancestors' do + def base_and_ancestors(clusterable) + described_class.new(clusterable).base_and_ancestors + end + + context 'project in nested group with clusters at every level' do + let!(:cluster) { create(:cluster, :project, projects: [project]) } + let!(:child) { create(:cluster, :group, groups: [child_group]) } + let!(:parent) { create(:cluster, :group, groups: [parent_group]) } + let!(:ancestor) { create(:cluster, :group, groups: [ancestor_group]) } + + let(:ancestor_group) { create(:group) } + let(:parent_group) { create(:group, parent: ancestor_group) } + let(:child_group) { create(:group, parent: parent_group) } + let(:project) { create(:project, group: child_group) } + + it 'returns clusters for project' do + expect(base_and_ancestors(project)).to eq([cluster, child, parent, ancestor]) + end + + it 'returns clusters for child_group' do + expect(base_and_ancestors(child_group)).to eq([child, parent, ancestor]) + end + + it 'returns clusters for parent_group' do + expect(base_and_ancestors(parent_group)).to eq([parent, ancestor]) + end + + it 'returns clusters for ancestor_group' do + expect(base_and_ancestors(ancestor_group)).to eq([ancestor]) + end + end + + context 'project in a namespace' do + let!(:cluster) { create(:cluster, :project) } + + it 'returns clusters for project' do + expect(base_and_ancestors(cluster.project)).to eq([cluster]) + end + end + + context 'project in nested group with clusters at some levels' do + let!(:child) { create(:cluster, :group, groups: [child_group]) } + let!(:ancestor) { create(:cluster, :group, groups: [ancestor_group]) } + + let(:ancestor_group) { create(:group) } + let(:parent_group) { create(:group, parent: ancestor_group) } + let(:child_group) { create(:group, parent: parent_group) } + let(:project) { create(:project, group: child_group) } + + it 'returns clusters for project' do + expect(base_and_ancestors(project)).to eq([child, ancestor]) + end + + it 'returns clusters for child_group' do + expect(base_and_ancestors(child_group)).to eq([child, ancestor]) + end + + it 'returns clusters for parent_group' do + expect(base_and_ancestors(parent_group)).to eq([ancestor]) + end + + it 'returns clusters for ancestor_group' do + expect(base_and_ancestors(ancestor_group)).to eq([ancestor]) + end + end + end +end diff --git a/spec/models/clusters/kubernetes_namespace_spec.rb b/spec/models/clusters/kubernetes_namespace_spec.rb index b5cba80b806..d4e3a0ac84d 100644 --- a/spec/models/clusters/kubernetes_namespace_spec.rb +++ b/spec/models/clusters/kubernetes_namespace_spec.rb @@ -24,70 +24,60 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do end end - describe 'namespace uniqueness validation' do - let(:cluster_project) { create(:cluster_project) } - let(:kubernetes_namespace) { build(:cluster_kubernetes_namespace, namespace: 'my-namespace') } + describe '.with_environment_slug' do + let(:cluster) { create(:cluster, :group) } + let(:environment) { create(:environment, slug: slug) } - subject { kubernetes_namespace } + let(:slug) { 'production' } - context 'when cluster is using the namespace' do - before do - create(:cluster_kubernetes_namespace, - cluster: kubernetes_namespace.cluster, - namespace: 'my-namespace') - end + subject { described_class.with_environment_slug(slug) } - it { is_expected.not_to be_valid } - end + context 'there is no associated environment' do + let!(:namespace) { create(:cluster_kubernetes_namespace, cluster: cluster, project: environment.project) } - context 'when cluster is not using the namespace' do - it { is_expected.to be_valid } + it { is_expected.to be_empty } end - end - describe '#set_defaults' do - let(:kubernetes_namespace) { build(:cluster_kubernetes_namespace) } - let(:cluster) { kubernetes_namespace.cluster } - let(:platform) { kubernetes_namespace.platform_kubernetes } - - subject { kubernetes_namespace.set_defaults } - - describe '#namespace' do - before do - platform.update_column(:namespace, namespace) + context 'there is an assicated environment' do + let!(:namespace) do + create( + :cluster_kubernetes_namespace, + cluster: cluster, + project: environment.project, + environment: environment + ) end - context 'when platform has a namespace assigned' do - let(:namespace) { 'platform-namespace' } - - it 'copies the namespace' do - subject - - expect(kubernetes_namespace.namespace).to eq('platform-namespace') - end + context 'with a matching slug' do + it { is_expected.to eq [namespace] } end - context 'when platform does not have namespace assigned' do - let(:project) { kubernetes_namespace.project } - let(:namespace) { nil } - let(:project_slug) { "#{project.path}-#{project.id}" } - - it 'fallbacks to project namespace' do - subject + context 'without a matching slug' do + let(:environment) { create(:environment, slug: 'staging') } - expect(kubernetes_namespace.namespace).to eq(project_slug) - end + it { is_expected.to be_empty } end end + end - describe '#service_account_name' do - let(:service_account_name) { "#{kubernetes_namespace.namespace}-service-account" } + describe 'namespace uniqueness validation' do + let(:kubernetes_namespace) { build(:cluster_kubernetes_namespace, namespace: 'my-namespace') } - it 'sets a service account name based on namespace' do - subject + subject { kubernetes_namespace } - expect(kubernetes_namespace.service_account_name).to eq(service_account_name) + context 'when cluster is using the namespace' do + before do + create(:cluster_kubernetes_namespace, + cluster: kubernetes_namespace.cluster, + environment: kubernetes_namespace.environment, + namespace: 'my-namespace') end + + it { is_expected.not_to be_valid } + end + + context 'when cluster is not using the namespace' do + it { is_expected.to be_valid } end end diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index c485850c16e..0c4cf291d20 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -2,13 +2,11 @@ require 'spec_helper' -describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching do +describe Clusters::Platforms::Kubernetes do include KubernetesHelpers - include ReactiveCachingHelpers it { is_expected.to belong_to(:cluster) } it { is_expected.to be_kind_of(Gitlab::Kubernetes) } - it { is_expected.to be_kind_of(ReactiveCaching) } it { is_expected.to respond_to :ca_pem } it { is_expected.to validate_exclusion_of(:namespace).in_array(%w(gitlab-managed-apps)) } @@ -108,7 +106,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching before do allow(ApplicationSetting) .to receive(:current) - .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true)) + .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: true)) end it { expect(kubernetes.save).to be_truthy } @@ -207,194 +205,100 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it { is_expected.to be_truthy } end - describe '#kubernetes_namespace_for' do - let(:cluster) { create(:cluster, :project) } - let(:project) { cluster.project } - - let(:platform) do - create(:cluster_platform_kubernetes, - cluster: cluster, - namespace: namespace) - end - - subject { platform.kubernetes_namespace_for(project) } - - context 'with a namespace assigned' do - let(:namespace) { 'namespace-123' } - - it { is_expected.to eq(namespace) } - end - - context 'with no namespace assigned' do - let(:namespace) { nil } - - context 'when kubernetes namespace is present' do - let(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster) } - - before do - kubernetes_namespace - end - - it { is_expected.to eq(kubernetes_namespace.namespace) } - end - - context 'when kubernetes namespace is not present' do - it { is_expected.to eq("#{project.path}-#{project.id}") } - end - end - end - describe '#predefined_variables' do - let!(:cluster) { create(:cluster, :project, platform_kubernetes: kubernetes) } - let(:kubernetes) { create(:cluster_platform_kubernetes, api_url: api_url, ca_cert: ca_pem) } - let(:api_url) { 'https://kube.domain.com' } - let(:ca_pem) { File.read(Rails.root.join('spec/fixtures/clusters/sample_cert.pem')) } - - subject { kubernetes.predefined_variables(project: cluster.project) } - - shared_examples 'setting variables' do - it 'sets the variables' do - expect(subject).to include( - { key: 'KUBE_URL', value: api_url, public: true }, - { key: 'KUBE_CA_PEM', value: ca_pem, public: true }, - { key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true } - ) - end - end + let(:project) { create(:project) } + let(:cluster) { create(:cluster, :group, platform_kubernetes: platform) } + let(:platform) { create(:cluster_platform_kubernetes) } + let(:persisted_namespace) { create(:cluster_kubernetes_namespace, project: project, cluster: cluster) } - context 'kubernetes namespace is created with no service account token' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster) } + let(:environment_name) { 'env/production' } + let(:environment_slug) { Gitlab::Slug::Environment.new(environment_name).generate } - it_behaves_like 'setting variables' + subject { platform.predefined_variables(project: project, environment_name: environment_name) } - it 'sets KUBE_TOKEN' do - expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes.token, public: false, masked: true } - ) - end + before do + allow(Clusters::KubernetesNamespaceFinder).to receive(:new) + .with(cluster, project: project, environment_slug: environment_slug) + .and_return(double(execute: persisted_namespace)) end - context 'kubernetes namespace is created with no service account token' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster) } + it { is_expected.to include(key: 'KUBE_URL', value: platform.api_url, public: true) } - it_behaves_like 'setting variables' + context 'platform has a CA certificate' do + let(:ca_pem) { File.read(Rails.root.join('spec/fixtures/clusters/sample_cert.pem')) } + let(:platform) { create(:cluster_platform_kubernetes, ca_cert: ca_pem) } - it 'sets KUBE_TOKEN' do - expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false, masked: true } - ) - end + it { is_expected.to include(key: 'KUBE_CA_PEM', value: ca_pem, public: true) } + it { is_expected.to include(key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true) } end - context 'namespace is provided' do - let(:namespace) { 'my-project' } + context 'kubernetes namespace exists' do + let(:variable) { Hash(key: :fake_key, value: 'fake_value') } + let(:namespace_variables) { Gitlab::Ci::Variables::Collection.new([variable]) } before do - kubernetes.namespace = namespace + expect(persisted_namespace).to receive(:predefined_variables).and_return(namespace_variables) end - it_behaves_like 'setting variables' - - it 'sets KUBE_TOKEN' do - expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes.token, public: false, masked: true } - ) - end - end - - context 'no namespace provided' do - it_behaves_like 'setting variables' - - it 'sets KUBE_TOKEN' do - expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes.token, public: false, masked: true } - ) - end + it { is_expected.to include(variable) } end - context 'group level cluster' do - let!(:cluster) { create(:cluster, :group, platform_kubernetes: kubernetes) } - - let(:project) { create(:project, group: cluster.group) } - - subject { kubernetes.predefined_variables(project: project) } - - context 'no kubernetes namespace for the project' do - it_behaves_like 'setting variables' + context 'kubernetes namespace does not exist' do + let(:persisted_namespace) { nil } + let(:namespace) { 'kubernetes-namespace' } + let(:kubeconfig) { 'kubeconfig' } - it 'does not return KUBE_TOKEN' do - expect(subject).not_to include( - { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } - ) - end - - context 'the cluster is not managed' do - let!(:cluster) { create(:cluster, :group, :not_managed, platform_kubernetes: kubernetes) } - - it_behaves_like 'setting variables' - - it 'sets KUBE_TOKEN' do - expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes.token, public: false, masked: true } - ) - end - end + before do + allow(Gitlab::Kubernetes::DefaultNamespace).to receive(:new) + .with(cluster, project: project).and_return(double(from_environment_name: namespace)) + allow(platform).to receive(:kubeconfig).with(namespace).and_return(kubeconfig) end - context 'kubernetes namespace exists for the project' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster, project: project) } + it { is_expected.not_to include(key: 'KUBE_TOKEN', value: platform.token, public: false, masked: true) } + it { is_expected.not_to include(key: 'KUBE_NAMESPACE', value: namespace) } + it { is_expected.not_to include(key: 'KUBECONFIG', value: kubeconfig, public: false, file: true) } - it_behaves_like 'setting variables' + context 'cluster is unmanaged' do + let(:cluster) { create(:cluster, :group, :not_managed, platform_kubernetes: platform) } - it 'sets KUBE_TOKEN' do - expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false, masked: true } - ) - end + it { is_expected.to include(key: 'KUBE_TOKEN', value: platform.token, public: false, masked: true) } + it { is_expected.to include(key: 'KUBE_NAMESPACE', value: namespace) } + it { is_expected.to include(key: 'KUBECONFIG', value: kubeconfig, public: false, file: true) } end end - context 'with a domain' do - let!(:cluster) do - create(:cluster, :provided_by_gcp, :with_domain, - platform_kubernetes: kubernetes) - end + context 'cluster variables' do + let(:variable) { Hash(key: :fake_key, value: 'fake_value') } + let(:cluster_variables) { Gitlab::Ci::Variables::Collection.new([variable]) } - it 'sets KUBE_INGRESS_BASE_DOMAIN' do - expect(subject).to include( - { key: 'KUBE_INGRESS_BASE_DOMAIN', value: cluster.domain, public: true } - ) + before do + expect(cluster).to receive(:predefined_variables).and_return(cluster_variables) end + + it { is_expected.to include(variable) } end end describe '#terminals' do - subject { service.terminals(environment) } + subject { service.terminals(environment, pods: pods) } let!(:cluster) { create(:cluster, :project, platform_kubernetes: service) } let(:project) { cluster.project } let(:service) { create(:cluster_platform_kubernetes, :configured) } let(:environment) { build(:environment, project: project, name: "env", slug: "env-000000") } + let(:pods) { [{ "bad" => "pod" }] } context 'with invalid pods' do it 'returns no terminals' do - stub_reactive_cache(service, pods: [{ "bad" => "pod" }]) - is_expected.to be_empty end end context 'with valid pods' do - let(:pod) { kube_pod(environment_slug: environment.slug, namespace: cluster.kubernetes_namespace_for(project), project_slug: project.full_path_slug) } + let(:pod) { kube_pod(environment_slug: environment.slug, namespace: cluster.kubernetes_namespace_for(environment), project_slug: project.full_path_slug) } let(:pod_with_no_terminal) { kube_pod(environment_slug: environment.slug, project_slug: project.full_path_slug, status: "Pending") } let(:terminals) { kube_terminals(service, pod) } - - before do - stub_reactive_cache( - service, - pods: [pod, pod, pod_with_no_terminal, kube_pod(environment_slug: "should-be-filtered-out")] - ) - end + let(:pods) { [pod, pod, pod_with_no_terminal, kube_pod(environment_slug: "should-be-filtered-out")] } it 'returns terminals' do is_expected.to eq(terminals + terminals) @@ -409,16 +313,18 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end end - describe '#calculate_reactive_cache' do - subject { service.calculate_reactive_cache } - - let!(:cluster) { create(:cluster, :project, enabled: enabled, platform_kubernetes: service) } + describe '#calculate_reactive_cache_for' do let(:service) { create(:cluster_platform_kubernetes, :configured) } - let(:enabled) { true } - let(:namespace) { cluster.kubernetes_namespace_for(cluster.project) } + let(:pod) { kube_pod } + let(:namespace) { pod["metadata"]["namespace"] } + let(:environment) { instance_double(Environment, deployment_namespace: namespace) } - context 'when cluster is disabled' do - let(:enabled) { false } + subject { service.calculate_reactive_cache_for(environment) } + + context 'when the kubernetes integration is disabled' do + before do + allow(service).to receive(:enabled?).and_return(false) + end it { is_expected.to be_nil } end @@ -429,7 +335,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching stub_kubeclient_deployments(namespace) end - it { is_expected.to include(pods: [kube_pod]) } + it { is_expected.to include(pods: [pod]) } end context 'when kubernetes responds with 500s' do @@ -449,34 +355,5 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it { is_expected.to include(pods: []) } end - - context 'when the cluster is not project level' do - let(:cluster) { create(:cluster, :group, platform_kubernetes: service) } - - it { is_expected.to include(pods: []) } - end - end - - describe '#update_kubernetes_namespace' do - let(:cluster) { create(:cluster, :provided_by_gcp) } - let(:platform) { cluster.platform } - - context 'when namespace is updated' do - it 'calls ConfigureWorker' do - expect(ClusterConfigureWorker).to receive(:perform_async).with(cluster.id).once - - platform.namespace = 'new-namespace' - platform.save - end - end - - context 'when namespace is not updated' do - it 'does not call ConfigureWorker' do - expect(ClusterConfigureWorker).not_to receive(:perform_async) - - platform.username = "new-username" - platform.save - end - end end end diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb index b96ca89c893..4a524b585e1 100644 --- a/spec/models/commit_range_spec.rb +++ b/spec/models/commit_range_spec.rb @@ -139,8 +139,8 @@ describe CommitRange do end describe '#has_been_reverted?' do - let(:issue) { create(:issue) } - let(:user) { issue.author } + let(:user) { create(:user) } + let(:issue) { create(:issue, author: user, project: project) } it 'returns true if the commit has been reverted' do create(:note_on_issue, @@ -149,9 +149,11 @@ describe CommitRange do note: commit1.revert_description(user), project: issue.project) - expect_any_instance_of(Commit).to receive(:reverts_commit?) - .with(commit1, user) - .and_return(true) + expect_next_instance_of(Commit) do |commit| + expect(commit).to receive(:reverts_commit?) + .with(commit1, user) + .and_return(true) + end expect(commit1.has_been_reverted?(user, issue.notes_with_associations)).to eq(true) end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index e76186fb280..7b35c2ffd36 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -556,6 +556,7 @@ eos it 'returns the URI type at the given path' do expect(commit.uri_type('files/html')).to be(:tree) expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) + expect(commit.uri_type('files/images/wm.svg')).to be(:raw) expect(project.commit('video').uri_type('files/videos/intro.mp4')).to be(:raw) expect(commit.uri_type('files/js/application.js')).to be(:blob) end diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 0e5fb2b5153..9a12c3d6965 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -198,6 +198,36 @@ describe CacheMarkdownField, :clean_gitlab_redis_cache do end end end + + describe '#updated_cached_html_for' do + let(:thing) { klass.new(description: markdown, description_html: html, cached_markdown_version: cache_version) } + + context 'when the markdown cache is outdated' do + before do + thing.cached_markdown_version += 1 + end + + it 'calls #refresh_markdown_cache' do + expect(thing).to receive(:refresh_markdown_cache) + + expect(thing.updated_cached_html_for(:description)).to eq(html) + end + end + + context 'when the markdown field does not exist' do + it 'returns nil' do + expect(thing.updated_cached_html_for(:something)).to eq(nil) + end + end + + context 'when the markdown cache is up to date' do + it 'does not call #refresh_markdown_cache' do + expect(thing).not_to receive(:refresh_markdown_cache) + + expect(thing.updated_cached_html_for(:description)).to eq(html) + end + end + end end context 'for Active record classes' do diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb index 394fac52aa7..da46effe411 100644 --- a/spec/models/concerns/cacheable_attributes_spec.rb +++ b/spec/models/concerns/cacheable_attributes_spec.rb @@ -7,6 +7,7 @@ describe CacheableAttributes do Class.new do include ActiveModel::Model extend ActiveModel::Callbacks + include ActiveModel::AttributeMethods define_model_callbacks :commit include CacheableAttributes @@ -34,44 +35,60 @@ describe CacheableAttributes do end end + before do + stub_const("MinimalTestClass", minimal_test_class) + end + shared_context 'with defaults' do before do - minimal_test_class.define_singleton_method(:defaults) do + MinimalTestClass.define_singleton_method(:defaults) do { foo: 'a', bar: 'b', baz: 'c' } end end end + describe '.expire', :use_clean_rails_memory_store_caching, :request_store do + it 'wipes the cache' do + obj = MinimalTestClass.new + obj.cache! + expect(MinimalTestClass.cached).not_to eq(nil) + + MinimalTestClass.expire + + expect(MinimalTestClass.cached).to eq(nil) + end + end + describe '.current_without_cache' do it 'defaults to last' do - expect(minimal_test_class.current_without_cache).to eq(minimal_test_class.last) + expect(MinimalTestClass.current_without_cache).to eq(MinimalTestClass.last) end it 'can be overridden' do - minimal_test_class.define_singleton_method(:current_without_cache) do + MinimalTestClass.define_singleton_method(:current_without_cache) do first end - expect(minimal_test_class.current_without_cache).to eq(minimal_test_class.first) + expect(MinimalTestClass.current_without_cache).to eq(MinimalTestClass.first) end end describe '.cache_key' do it 'excludes cache attributes' do - expect(minimal_test_class.cache_key).to eq("TestClass:#{Gitlab::VERSION}:#{Rails.version}") + expect(MinimalTestClass.cache_key).to eq("TestClass:#{Gitlab::VERSION}:#{Rails.version}") end end describe '.defaults' do it 'defaults to {}' do - expect(minimal_test_class.defaults).to eq({}) + expect(MinimalTestClass.defaults).to eq({}) end context 'with defaults defined' do include_context 'with defaults' it 'can be overridden' do - expect(minimal_test_class.defaults).to eq({ foo: 'a', bar: 'b', baz: 'c' }) + expect(MinimalTestClass.defaults).to eq({ foo: 'a', bar: 'b', baz: 'c' }) end end end @@ -81,13 +98,13 @@ describe CacheableAttributes do context 'without any attributes given' do it 'intializes a new object with the defaults' do - expect(minimal_test_class.build_from_defaults.attributes).to eq(minimal_test_class.defaults.stringify_keys) + expect(MinimalTestClass.build_from_defaults.attributes).to eq(MinimalTestClass.defaults.stringify_keys) end end context 'with attributes given' do it 'intializes a new object with the given attributes merged into the defaults' do - expect(minimal_test_class.build_from_defaults(foo: 'd').attributes['foo']).to eq('d') + expect(MinimalTestClass.build_from_defaults(foo: 'd').attributes['foo']).to eq('d') end end @@ -108,8 +125,8 @@ describe CacheableAttributes do describe '.current', :use_clean_rails_memory_store_caching do context 'redis unavailable' do before do - allow(minimal_test_class).to receive(:last).and_return(:last) - expect(Rails.cache).to receive(:read).with(minimal_test_class.cache_key).and_raise(Redis::BaseError) + allow(MinimalTestClass).to receive(:last).and_return(:last) + expect(Rails.cache).to receive(:read).with(MinimalTestClass.cache_key).and_raise(Redis::BaseError) end context 'in production environment' do @@ -120,7 +137,7 @@ describe CacheableAttributes do it 'returns an uncached record and logs a warning' do expect(Rails.logger).to receive(:warn).with("Cached record for TestClass couldn't be loaded, falling back to uncached record: Redis::BaseError") - expect(minimal_test_class.current).to eq(:last) + expect(MinimalTestClass.current).to eq(:last) end end @@ -132,7 +149,7 @@ describe CacheableAttributes do it 'returns an uncached record and logs a warning' do expect(Rails.logger).not_to receive(:warn) - expect { minimal_test_class.current }.to raise_error(Redis::BaseError) + expect { MinimalTestClass.current }.to raise_error(Redis::BaseError) end end end @@ -143,14 +160,14 @@ describe CacheableAttributes do allow(ApplicationSetting).to receive(:current_without_cache).twice.and_return(nil) expect(ApplicationSetting.current).to be_nil - expect(Rails.cache.exist?(ApplicationSetting.cache_key)).to be(false) + expect(ApplicationSetting.cache_backend.exist?(ApplicationSetting.cache_key)).to be(false) end it 'caches non-nil object' do create(:application_setting) expect(ApplicationSetting.current).to eq(ApplicationSetting.last) - expect(Rails.cache.exist?(ApplicationSetting.cache_key)).to be(true) + expect(ApplicationSetting.cache_backend.exist?(ApplicationSetting.cache_key)).to be(true) # subsequent calls retrieve the record from the cache last_record = ApplicationSetting.last @@ -188,11 +205,12 @@ describe CacheableAttributes do end end - it 'uses RequestStore in addition to Rails.cache', :request_store do + it 'uses RequestStore in addition to Thread memory cache', :request_store do # Warm up the cache create(:application_setting).cache! - expect(Rails.cache).to receive(:read).with(ApplicationSetting.cache_key).once.and_call_original + expect(ApplicationSetting.cache_backend).to eq(Gitlab::ThreadMemoryCache.cache_backend) + expect(ApplicationSetting.cache_backend).to receive(:read).with(ApplicationSetting.cache_key).once.and_call_original 2.times { ApplicationSetting.current } end @@ -201,7 +219,7 @@ describe CacheableAttributes do describe '.cached', :use_clean_rails_memory_store_caching do context 'when cache is cold' do it 'returns nil' do - expect(minimal_test_class.cached).to be_nil + expect(MinimalTestClass.cached).to be_nil end end diff --git a/spec/models/concerns/case_sensitivity_spec.rb b/spec/models/concerns/case_sensitivity_spec.rb index d6d41a25eac..9819f656f0d 100644 --- a/spec/models/concerns/case_sensitivity_spec.rb +++ b/spec/models/concerns/case_sensitivity_spec.rb @@ -28,28 +28,13 @@ describe CaseSensitivity do .to contain_exactly(model_1) end - # Using `mysql` & `postgresql` metadata-tags here because both adapters build - # the query slightly differently - context 'for MySQL', :mysql do - it 'builds a simple query' do - query = model.iwhere(path: %w(MODEL-1 model-2), name: 'model 1').to_sql - expected_query = <<~QRY.strip - SELECT `namespaces`.* FROM `namespaces` WHERE (`namespaces`.`path` IN ('MODEL-1', 'model-2')) AND (`namespaces`.`name` = 'model 1') - QRY - - expect(query).to eq(expected_query) - end - end + it 'builds a query using LOWER' do + query = model.iwhere(path: %w(MODEL-1 model-2), name: 'model 1').to_sql + expected_query = <<~QRY.strip + SELECT \"namespaces\".* FROM \"namespaces\" WHERE (LOWER(\"namespaces\".\"path\") IN (LOWER('MODEL-1'), LOWER('model-2'))) AND (LOWER(\"namespaces\".\"name\") = LOWER('model 1')) + QRY - context 'for PostgreSQL', :postgresql do - it 'builds a query using LOWER' do - query = model.iwhere(path: %w(MODEL-1 model-2), name: 'model 1').to_sql - expected_query = <<~QRY.strip - SELECT \"namespaces\".* FROM \"namespaces\" WHERE (LOWER(\"namespaces\".\"path\") IN (LOWER('MODEL-1'), LOWER('model-2'))) AND (LOWER(\"namespaces\".\"name\") = LOWER('model 1')) - QRY - - expect(query).to eq(expected_query) - end + expect(query).to eq(expected_query) end end end diff --git a/spec/models/concerns/deployable_spec.rb b/spec/models/concerns/deployable_spec.rb index 42bed9434f5..bb73dd8ade0 100644 --- a/spec/models/concerns/deployable_spec.rb +++ b/spec/models/concerns/deployable_spec.rb @@ -7,10 +7,6 @@ describe Deployable do let(:deployment) { job.deployment } let(:environment) { deployment&.environment } - before do - job.reload - end - context 'when the deployable object will deploy to production' do let!(:job) { create(:ci_build, :start_review_app) } @@ -26,6 +22,16 @@ describe Deployable do end end + context 'when the deployable object will deploy to a cluster' do + let(:project) { create(:project) } + let!(:cluster) { create(:cluster, :provided_by_user, projects: [project]) } + let!(:job) { create(:ci_build, :start_review_app, project: project) } + + it 'creates a deployment with cluster association' do + expect(deployment.cluster).to eq(cluster) + end + end + context 'when the deployable object will stop an environment' do let!(:job) { create(:ci_build, :stop_review_app) } diff --git a/spec/models/concerns/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb index 0e34d8fccf3..27f535487c8 100644 --- a/spec/models/concerns/deployment_platform_spec.rb +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -8,40 +8,7 @@ describe DeploymentPlatform do describe '#deployment_platform' do subject { project.deployment_platform } - context 'with no Kubernetes configuration on CI/CD, no Kubernetes Service and a Kubernetes template configured' do - let!(:kubernetes_service) { create(:kubernetes_service, template: true) } - - it 'returns a platform kubernetes' do - expect(subject).to be_a_kind_of(Clusters::Platforms::Kubernetes) - end - - it 'creates a cluster and a platform kubernetes' do - expect { subject } - .to change { Clusters::Cluster.count }.by(1) - .and change { Clusters::Platforms::Kubernetes.count }.by(1) - end - - it 'includes appropriate attributes for Cluster' do - cluster = subject.cluster - expect(cluster.name).to eq('kubernetes-template') - expect(cluster.project).to eq(project) - expect(cluster.provider_type).to eq('user') - expect(cluster.platform_type).to eq('kubernetes') - end - - it 'creates a platform kubernetes' do - expect { subject }.to change { Clusters::Platforms::Kubernetes.count }.by(1) - end - - it 'copies attributes from Clusters::Platform::Kubernetes template into the new Cluster::Platforms::Kubernetes' do - expect(subject.api_url).to eq(kubernetes_service.api_url) - expect(subject.ca_pem).to eq(kubernetes_service.ca_pem) - expect(subject.token).to eq(kubernetes_service.token) - expect(subject.namespace).to eq(kubernetes_service.namespace) - end - end - - context 'with no Kubernetes configuration on CI/CD, no Kubernetes Service and no Kubernetes template configured' do + context 'with no Kubernetes configuration on CI/CD, no Kubernetes Service' do it { is_expected.to be_nil } end @@ -78,13 +45,12 @@ describe DeploymentPlatform do is_expected.to eq(group_cluster.platform_kubernetes) end - context 'when child group has configured kubernetes cluster', :nested_groups do - let!(:child_group1_cluster) { create(:cluster, :provided_by_gcp, :group) } - let(:child_group1) { child_group1_cluster.group } + context 'when child group has configured kubernetes cluster' do + let(:child_group1) { create(:group, parent: group) } + let!(:child_group1_cluster) { create(:cluster_for_group, groups: [child_group1]) } before do project.update!(group: child_group1) - child_group1.update!(parent: group) end it 'returns the Kubernetes platform for the child group' do @@ -92,11 +58,10 @@ describe DeploymentPlatform do end context 'deeply nested group' do - let!(:child_group2_cluster) { create(:cluster, :provided_by_gcp, :group) } - let(:child_group2) { child_group2_cluster.group } + let(:child_group2) { create(:group, parent: child_group1) } + let!(:child_group2_cluster) { create(:cluster_for_group, groups: [child_group2]) } before do - child_group2.update!(parent: child_group1) project.update!(group: child_group2) end @@ -115,34 +80,6 @@ describe DeploymentPlatform do end end end - - context 'feature flag disabled' do - before do - stub_feature_flags(group_clusters: false) - end - - it 'returns nil' do - is_expected.to be_nil - end - end - end - - context 'when user configured kubernetes integration from project services' do - let!(:kubernetes_service) { create(:kubernetes_service, project: project) } - - it 'returns the Kubernetes service' do - expect(subject).to eq(kubernetes_service) - end - end - - context 'when the cluster creation fails' do - let!(:kubernetes_service) { create(:kubernetes_service, template: true) } - - before do - allow_any_instance_of(Clusters::Cluster).to receive(:persisted?).and_return(false) - end - - it { is_expected.to be_nil } end end end diff --git a/spec/models/concerns/group_descendant_spec.rb b/spec/models/concerns/group_descendant_spec.rb index 194caac3fce..192e884f3e8 100644 --- a/spec/models/concerns/group_descendant_spec.rb +++ b/spec/models/concerns/group_descendant_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe GroupDescendant, :nested_groups do +describe GroupDescendant do let(:parent) { create(:group) } let(:subgroup) { create(:group, parent: parent) } let(:subsub_group) { create(:group, parent: subgroup) } @@ -84,7 +84,7 @@ describe GroupDescendant, :nested_groups do it 'tracks the exception when a parent was not preloaded' do expect(Gitlab::Sentry).to receive(:track_exception).and_call_original - expect { GroupDescendant.build_hierarchy([subsub_group]) }.to raise_error(ArgumentError) + expect { described_class.build_hierarchy([subsub_group]) }.to raise_error(ArgumentError) end it 'recovers if a parent was not reloaded by querying for the parent' do @@ -93,7 +93,7 @@ describe GroupDescendant, :nested_groups do # this does not raise in production, so stubbing it here. allow(Gitlab::Sentry).to receive(:track_exception) - expect(GroupDescendant.build_hierarchy([subsub_group])).to eq(expected_hierarchy) + expect(described_class.build_hierarchy([subsub_group])).to eq(expected_hierarchy) end it 'raises an error if not all elements were preloaded' do diff --git a/spec/models/concerns/has_environment_scope_spec.rb b/spec/models/concerns/has_environment_scope_spec.rb new file mode 100644 index 00000000000..a6e1ba59263 --- /dev/null +++ b/spec/models/concerns/has_environment_scope_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe HasEnvironmentScope do + subject { build(:ci_variable) } + + it { is_expected.to allow_value('*').for(:environment_scope) } + it { is_expected.to allow_value('review/*').for(:environment_scope) } + it { is_expected.not_to allow_value('').for(:environment_scope) } + it { is_expected.not_to allow_value('!!()()').for(:environment_scope) } + + it do + is_expected.to validate_uniqueness_of(:key) + .scoped_to(:project_id, :environment_scope) + .with_message(/\(\w+\) has already been taken/) + end + + describe '.on_environment' do + let(:project) { create(:project) } + + it 'returns scoped objects' do + variable1 = create(:ci_variable, project: project, environment_scope: '*') + variable2 = create(:ci_variable, project: project, environment_scope: 'product/*') + create(:ci_variable, project: project, environment_scope: 'staging/*') + + expect(project.variables.on_environment('product/canary-1')).to eq([variable1, variable2]) + end + + it 'returns only the most relevant object if relevant_only is true' do + create(:ci_variable, project: project, environment_scope: '*') + variable2 = create(:ci_variable, project: project, environment_scope: 'product/*') + create(:ci_variable, project: project, environment_scope: 'staging/*') + + expect(project.variables.on_environment('product/canary-1', relevant_only: true)).to eq([variable2]) + end + + it 'returns scopes ordered by lowest precedence first' do + create(:ci_variable, project: project, environment_scope: '*') + create(:ci_variable, project: project, environment_scope: 'production*') + create(:ci_variable, project: project, environment_scope: 'production') + + result = project.variables.on_environment('production').map(&:environment_scope) + + expect(result).to eq(['*', 'production*', 'production']) + end + end + + describe '#environment_scope=' do + context 'when the new environment_scope is nil' do + it 'strips leading and trailing whitespaces' do + subject.environment_scope = nil + + expect(subject.environment_scope).to eq('') + end + end + + context 'when the new environment_scope has leadind and trailing whitespaces' do + it 'strips leading and trailing whitespaces' do + subject.environment_scope = ' * ' + + expect(subject.environment_scope).to eq('*') + end + end + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 64f02978d79..39680c0e51a 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -128,7 +128,7 @@ describe Issuable do expect(build_issuable(milestone.id).milestone_available?).to be_truthy end - it 'returns true with a milestone from the the parent of the issue project group', :nested_groups do + it 'returns true with a milestone from the the parent of the issue project group' do parent = create(:group) group.update(parent: parent) milestone = create(:milestone, group: parent) @@ -223,6 +223,16 @@ describe Issuable do expect(issuable_class.full_search(searchable_issue2.description.downcase)).to eq([searchable_issue2]) end + it 'returns issues with a fuzzy matching description for a query shorter than 3 chars if told to do so' do + search = searchable_issue2.description.downcase.scan(/\w+/).sample[-1] + + expect(issuable_class.full_search(search, use_minimum_char_limit: false)).to include(searchable_issue2) + end + + it 'returns issues with a fuzzy matching title for a query shorter than 3 chars if told to do so' do + expect(issuable_class.full_search('i', use_minimum_char_limit: false)).to include(searchable_issue) + end + context 'when matching columns is "title"' do it 'returns issues with a matching title' do expect(issuable_class.full_search(searchable_issue.title, matched_columns: 'title')) @@ -764,4 +774,25 @@ describe Issuable do end end end + + describe '#supports_milestone?' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + + context "for issues" do + let(:issue) { build(:issue, project: project) } + + it 'returns true' do + expect(issue.supports_milestone?).to be_truthy + end + end + + context "for merge requests" do + let(:merge_request) { build(:merge_request, target_project: project, source_project: project) } + + it 'returns true' do + expect(merge_request.supports_milestone?).to be_truthy + end + end + end end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index f31e3e8821d..6034344d034 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -18,7 +18,7 @@ describe Mentionable do let(:project) { create(:project) } let(:mentionable) { Example.new } - it 'excludes JIRA references' do + it 'excludes Jira references' do allow(project).to receive_messages(jira_tracker?: true) mentionable.project = project diff --git a/spec/models/concerns/project_api_compatibility_spec.rb b/spec/models/concerns/project_api_compatibility_spec.rb new file mode 100644 index 00000000000..f5722f88aac --- /dev/null +++ b/spec/models/concerns/project_api_compatibility_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ProjectAPICompatibility do + let(:project) { create(:project) } + + # git_strategy + it "converts build_git_strategy=fetch to build_allow_git_fetch=true" do + project.update!(build_git_strategy: 'fetch') + expect(project.build_allow_git_fetch).to eq(true) + end + + it "converts build_git_strategy=clone to build_allow_git_fetch=false" do + project.update!(build_git_strategy: 'clone') + expect(project.build_allow_git_fetch).to eq(false) + end + + describe '#auto_devops_enabled' do + where( + initial: [:missing, nil, false, true], + final: [nil, false, true] + ) + + with_them do + before do + project.build_auto_devops(enabled: initial) unless initial == :missing + end + + # Implicit auto devops when enabled is nil + let(:expected) { final.nil? ? true : final } + + it 'sets the correct value' do + project.update!(auto_devops_enabled: final) + + expect(project.auto_devops_enabled?).to eq(expected) + end + end + end + + describe '#auto_devops_deploy_strategy' do + where( + initial: [:missing, *ProjectAutoDevops.deploy_strategies.keys], + final: ProjectAutoDevops.deploy_strategies.keys + ) + + with_them do + before do + project.build_auto_devops(deploy_strategy: initial) unless initial == :missing + end + + it 'sets the correct value' do + project.update!(auto_devops_deploy_strategy: final) + + expect(project.auto_devops.deploy_strategy).to eq(final) + end + end + end +end diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb index 5aa43b58217..1fe176ab5af 100644 --- a/spec/models/concerns/project_features_compatibility_spec.rb +++ b/spec/models/concerns/project_features_compatibility_spec.rb @@ -4,7 +4,8 @@ require 'spec_helper' describe ProjectFeaturesCompatibility do let(:project) { create(:project) } - let(:features) { %w(issues wiki builds merge_requests snippets) } + let(:features_except_repository) { %w(issues wiki builds merge_requests snippets) } + let(:features) { features_except_repository + ['repository'] } # We had issues_enabled, snippets_enabled, builds_enabled, merge_requests_enabled and issues_enabled fields on projects table # All those fields got moved to a new table called project_feature and are now integers instead of booleans @@ -12,30 +13,37 @@ describe ProjectFeaturesCompatibility do # So we can keep it compatible it "converts fields from 'true' to ProjectFeature::ENABLED" do - features.each do |feature| + features_except_repository.each do |feature| project.update_attribute("#{feature}_enabled".to_sym, "true") expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::ENABLED) end end it "converts fields from 'false' to ProjectFeature::DISABLED" do - features.each do |feature| + features_except_repository.each do |feature| project.update_attribute("#{feature}_enabled".to_sym, "false") expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::DISABLED) end end it "converts fields from true to ProjectFeature::ENABLED" do - features.each do |feature| + features_except_repository.each do |feature| project.update_attribute("#{feature}_enabled".to_sym, true) expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::ENABLED) end end it "converts fields from false to ProjectFeature::DISABLED" do - features.each do |feature| + features_except_repository.each do |feature| project.update_attribute("#{feature}_enabled".to_sym, false) expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::DISABLED) end end + + it "accepts private as ProjectFeature::PRIVATE" do + features.each do |feature| + project.update!("#{feature}_access_level".to_sym => 'private') + expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::PRIVATE) + end + end end diff --git a/spec/models/concerns/prometheus_adapter_spec.rb b/spec/models/concerns/prometheus_adapter_spec.rb index 25a2d290f76..3d26ba95192 100644 --- a/spec/models/concerns/prometheus_adapter_spec.rb +++ b/spec/models/concerns/prometheus_adapter_spec.rb @@ -40,13 +40,13 @@ describe PrometheusAdapter, :use_clean_rails_memory_store_caching do describe 'matched_metrics' do let(:matched_metrics_query) { Gitlab::Prometheus::Queries::MatchedMetricQuery } - let(:prometheus_client_wrapper) { double(:prometheus_client_wrapper, label_values: nil) } + let(:prometheus_client) { double(:prometheus_client, label_values: nil) } context 'with valid data' do subject { service.query(:matched_metrics) } before do - allow(service).to receive(:prometheus_client_wrapper).and_return(prometheus_client_wrapper) + allow(service).to receive(:prometheus_client).and_return(prometheus_client) synchronous_reactive_cache(service) end diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 7faa196623f..3d026932f59 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -47,30 +47,12 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do subject(:go!) { instance.result } - context 'when cache is empty' do - it { is_expected.to be_nil } - - it 'enqueues a background worker to bootstrap the cache' do - expect(ReactiveCachingWorker).to receive(:perform_async).with(CacheTest, 666) - - go! - end - - it 'updates the cache lifespan' do - expect(reactive_cache_alive?(instance)).to be_falsy - - go! - - expect(reactive_cache_alive?(instance)).to be_truthy - end - end - - context 'when the cache is full' do + shared_examples 'a cacheable value' do |cached_value| before do - stub_reactive_cache(instance, 4) + stub_reactive_cache(instance, cached_value) end - it { is_expected.to eq(4) } + it { is_expected.to eq(cached_value) } it 'does not enqueue a background worker' do expect(ReactiveCachingWorker).not_to receive(:perform_async) @@ -90,9 +72,7 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do end it { is_expected.to be_nil } - end - context 'when cache was invalidated' do it 'refreshes cache' do expect(ReactiveCachingWorker).to receive(:perform_async).with(CacheTest, 666) @@ -101,12 +81,34 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do end end - context 'when cache contains non-nil but blank value' do - before do - stub_reactive_cache(instance, false) + context 'when cache is empty' do + it { is_expected.to be_nil } + + it 'enqueues a background worker to bootstrap the cache' do + expect(ReactiveCachingWorker).to receive(:perform_async).with(CacheTest, 666) + + go! end - it { is_expected.to eq(false) } + it 'updates the cache lifespan' do + expect(reactive_cache_alive?(instance)).to be_falsy + + go! + + expect(reactive_cache_alive?(instance)).to be_truthy + end + end + + context 'when the cache is full' do + it_behaves_like 'a cacheable value', 4 + end + + context 'when the cache contains non-nil but blank value' do + it_behaves_like 'a cacheable value', false + end + + context 'when the cache contains nil value' do + it_behaves_like 'a cacheable value', nil end end @@ -206,8 +208,9 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do expect(read_reactive_cache(instance)).to eq("preexisting") end - it 'enqueues a repeat worker' do - expect_reactive_cache_update_queued(instance) + it 'does not enqueue a repeat worker' do + expect(ReactiveCachingWorker) + .not_to receive(:perform_in) expect { go! }.to raise_error("foo") end diff --git a/spec/models/concerns/relative_positioning_spec.rb b/spec/models/concerns/relative_positioning_spec.rb deleted file mode 100644 index d0ae45f7871..00000000000 --- a/spec/models/concerns/relative_positioning_spec.rb +++ /dev/null @@ -1,242 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe RelativePositioning do - let(:project) { create(:project) } - let(:issue) { create(:issue, project: project) } - let(:issue1) { create(:issue, project: project) } - let(:new_issue) { create(:issue, project: project) } - - describe '.move_to_end' do - it 'moves the object to the end' do - Issue.move_to_end([issue, issue1]) - - expect(issue1.prev_relative_position).to eq issue.relative_position - expect(issue.prev_relative_position).to eq nil - expect(issue1.next_relative_position).to eq nil - end - - it 'does not perform any moves if all issues have their relative_position set' do - issue.update!(relative_position: 1) - - expect(issue).not_to receive(:save) - - Issue.move_to_end([issue]) - end - end - - describe '#max_relative_position' do - it 'returns maximum position' do - expect(issue.max_relative_position).to eq issue1.relative_position - end - end - - describe '#prev_relative_position' do - it 'returns previous position if there is an issue above' do - expect(issue1.prev_relative_position).to eq issue.relative_position - end - - it 'returns nil if there is no issue above' do - expect(issue.prev_relative_position).to eq nil - end - end - - describe '#next_relative_position' do - it 'returns next position if there is an issue below' do - expect(issue.next_relative_position).to eq issue1.relative_position - end - - it 'returns nil if there is no issue below' do - expect(issue1.next_relative_position).to eq nil - end - end - - describe '#move_before' do - it 'moves issue before' do - [issue1, issue].each(&:move_to_end) - - issue.move_before(issue1) - - expect(issue.relative_position).to be < issue1.relative_position - end - end - - describe '#move_after' do - it 'moves issue after' do - [issue, issue1].each(&:move_to_end) - - issue.move_after(issue1) - - expect(issue.relative_position).to be > issue1.relative_position - end - end - - describe '#move_to_end' do - before do - [issue, issue1].each do |issue| - issue.move_to_end && issue.save - end - end - - it 'moves issue to the end' do - new_issue.move_to_end - - expect(new_issue.relative_position).to be > issue1.relative_position - end - end - - describe '#shift_after?' do - before do - [issue, issue1].each do |issue| - issue.move_to_end && issue.save - end - end - - it 'returns true' do - issue.update(relative_position: issue1.relative_position - 1) - - expect(issue.shift_after?).to be_truthy - end - - it 'returns false' do - issue.update(relative_position: issue1.relative_position - 2) - - expect(issue.shift_after?).to be_falsey - end - end - - describe '#shift_before?' do - before do - [issue, issue1].each do |issue| - issue.move_to_end && issue.save - end - end - - it 'returns true' do - issue.update(relative_position: issue1.relative_position + 1) - - expect(issue.shift_before?).to be_truthy - end - - it 'returns false' do - issue.update(relative_position: issue1.relative_position + 2) - - expect(issue.shift_before?).to be_falsey - end - end - - describe '#move_between' do - before do - [issue, issue1].each do |issue| - issue.move_to_end && issue.save - end - end - - it 'positions issue between two other' do - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to be > issue.relative_position - expect(new_issue.relative_position).to be < issue1.relative_position - end - - it 'positions issue between on top' do - new_issue.move_between(nil, issue) - - expect(new_issue.relative_position).to be < issue.relative_position - end - - it 'positions issue between to end' do - new_issue.move_between(issue1, nil) - - expect(new_issue.relative_position).to be > issue1.relative_position - end - - it 'positions issues even when after and before positions are the same' do - issue1.update relative_position: issue.relative_position - - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to be > issue.relative_position - expect(issue.relative_position).to be < issue1.relative_position - end - - it 'positions issues between other two if distance is 1' do - issue1.update relative_position: issue.relative_position + 1 - - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to be > issue.relative_position - expect(issue.relative_position).to be < issue1.relative_position - end - - it 'positions issue in the middle of other two if distance is big enough' do - issue.update relative_position: 6000 - issue1.update relative_position: 10000 - - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to eq(8000) - end - - it 'positions issue closer to the middle if we are at the very top' do - issue1.update relative_position: 6000 - - new_issue.move_between(nil, issue1) - - expect(new_issue.relative_position).to eq(6000 - RelativePositioning::IDEAL_DISTANCE) - end - - it 'positions issue closer to the middle if we are at the very bottom' do - issue.update relative_position: 6000 - issue1.update relative_position: nil - - new_issue.move_between(issue, nil) - - expect(new_issue.relative_position).to eq(6000 + RelativePositioning::IDEAL_DISTANCE) - end - - it 'positions issue in the middle of other two if distance is not big enough' do - issue.update relative_position: 100 - issue1.update relative_position: 400 - - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to eq(250) - end - - it 'positions issue in the middle of other two is there is no place' do - issue.update relative_position: 100 - issue1.update relative_position: 101 - - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to be_between(issue.relative_position, issue1.relative_position) - end - - it 'uses rebalancing if there is no place' do - issue.update relative_position: 100 - issue1.update relative_position: 101 - issue2 = create(:issue, relative_position: 102, project: project) - new_issue.update relative_position: 103 - - new_issue.move_between(issue1, issue2) - new_issue.save! - - expect(new_issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position) - expect(issue.reload.relative_position).not_to eq(100) - end - - it 'positions issue right if we pass none-sequential parameters' do - issue.update relative_position: 99 - issue1.update relative_position: 101 - issue2 = create(:issue, relative_position: 102, project: project) - new_issue.update relative_position: 103 - - new_issue.move_between(issue, issue2) - new_issue.save! - - expect(new_issue.relative_position).to be(100) - end - end -end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 1fb0dd5030c..31163a5bb5c 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -15,23 +15,46 @@ describe Group, 'Routable' do end describe 'Callbacks' do - it 'creates route record on create' do - expect(group.route.path).to eq(group.path) - expect(group.route.name).to eq(group.name) - end + context 'for a group' do + it 'creates route record on create' do + expect(group.route.path).to eq(group.path) + expect(group.route.name).to eq(group.name) + end + + it 'updates route record on path change' do + group.update(path: 'wow', name: 'much') - it 'updates route record on path change' do - group.update(path: 'wow', name: 'much') + expect(group.route.path).to eq('wow') + expect(group.route.name).to eq('much') + end + + it 'ensure route path uniqueness across different objects' do + create(:group, parent: group, path: 'xyz') + duplicate = build(:project, namespace: group, path: 'xyz') - expect(group.route.path).to eq('wow') - expect(group.route.name).to eq('much') + expect { duplicate.save! }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Path has already been taken') + end end - it 'ensure route path uniqueness across different objects' do - create(:group, parent: group, path: 'xyz') - duplicate = build(:project, namespace: group, path: 'xyz') + context 'for a user' do + let(:user) { create(:user, username: 'jane', name: "Jane Doe") } - expect { duplicate.save! }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Path has already been taken') + it 'creates the route for a record on create' do + expect(user.namespace.name).to eq('Jane Doe') + expect(user.namespace.path).to eq('jane') + end + + it 'updates routes and nested routes on name change' do + project = create(:project, path: 'work-stuff', name: 'Work stuff', namespace: user.namespace) + + user.update!(username: 'jaen', name: 'Jaen Did') + project.reload + + expect(user.namespace.name).to eq('Jaen Did') + expect(user.namespace.path).to eq('jaen') + expect(project.full_name).to eq('Jaen Did / Work stuff') + expect(project.full_path).to eq('jaen/work-stuff') + end end end diff --git a/spec/models/concerns/stepable_spec.rb b/spec/models/concerns/stepable_spec.rb new file mode 100644 index 00000000000..5685de6a9bf --- /dev/null +++ b/spec/models/concerns/stepable_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Stepable do + let(:described_class) do + Class.new do + include Stepable + + steps :method1, :method2, :method3 + + def execute + execute_steps + end + + private + + def method1 + { status: :success } + end + + def method2 + return { status: :error } unless @pass + + { status: :success, variable1: 'var1' } + end + + def method3 + { status: :success, variable2: 'var2' } + end + end + end + + let(:prepended_module) do + Module.new do + extend ActiveSupport::Concern + + prepended do + steps :appended_method1 + end + + private + + def appended_method1 + { status: :success } + end + end + end + + before do + described_class.prepend(prepended_module) + end + + it 'stops after the first error' do + expect(subject).not_to receive(:method3) + expect(subject).not_to receive(:appended_method1) + + expect(subject.execute).to eq( + status: :error, + failed_step: :method2 + ) + end + + context 'when all methods return success' do + before do + subject.instance_variable_set(:@pass, true) + end + + it 'calls all methods in order' do + expect(subject).to receive(:method1).and_call_original.ordered + expect(subject).to receive(:method2).and_call_original.ordered + expect(subject).to receive(:method3).and_call_original.ordered + expect(subject).to receive(:appended_method1).and_call_original.ordered + + subject.execute + end + + it 'merges variables returned by all steps' do + expect(subject.execute).to eq( + status: :success, + variable1: 'var1', + variable2: 'var2' + ) + end + end + + context 'with multiple stepable classes' do + let(:other_class) do + Class.new do + include Stepable + + steps :other_method1, :other_method2 + + private + + def other_method1 + { status: :success } + end + + def other_method2 + { status: :success } + end + end + end + + it 'does not leak steps' do + expect(other_class.new.steps).to contain_exactly(:other_method1, :other_method2) + expect(subject.steps).to contain_exactly(:method1, :method2, :method3, :appended_method1) + end + end +end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 013112d1d51..935838ce294 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -16,7 +16,7 @@ describe ContainerRepository do host_port: 'registry.gitlab') stub_request(:get, 'http://registry.gitlab/v2/group/test/my_image/tags/list') - .with(headers: { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' }) + .with(headers: { 'Accept' => ContainerRegistry::Client::ACCEPTED_TYPES.join(', ') }) .to_return( status: 200, body: JSON.dump(tags: ['test_tag']), diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb index b22a0340015..808659552ff 100644 --- a/spec/models/cycle_analytics/code_spec.rb +++ b/spec/models/cycle_analytics/code_spec.rb @@ -8,7 +8,8 @@ describe 'CycleAnalytics#code' do let(:project) { create(:project, :repository) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + + subject { CycleAnalytics::ProjectLevel.new(project, options: { from: from_date }) } context 'with deployment' do generate_cycle_analytics_spec( @@ -37,7 +38,7 @@ describe 'CycleAnalytics#code' do merge_merge_requests_closing_issue(user, project, issue) deploy_master(user, project) - expect(subject[:code].median).to be_nil + expect(subject[:code].project_median).to be_nil end end end @@ -67,7 +68,7 @@ describe 'CycleAnalytics#code' do merge_merge_requests_closing_issue(user, project, issue) - expect(subject[:code].median).to be_nil + expect(subject[:code].project_median).to be_nil end end end diff --git a/spec/models/cycle_analytics/group_level_spec.rb b/spec/models/cycle_analytics/group_level_spec.rb new file mode 100644 index 00000000000..154c1b9c0f8 --- /dev/null +++ b/spec/models/cycle_analytics/group_level_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe CycleAnalytics::GroupLevel do + let(:group) { create(:group)} + let(:project) { create(:project, :repository, namespace: group) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + let(:issue) { create(:issue, project: project, created_at: 2.days.ago) } + let(:milestone) { create(:milestone, project: project) } + let(:mr) { create_merge_request_closing_issue(user, project, issue, commit_message: "References #{issue.to_reference}") } + let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: project, ref: mr.source_branch, sha: mr.source_branch_sha, head_pipeline_of: mr) } + + subject { described_class.new(group: group, options: { from: from_date, current_user: user }) } + + describe '#permissions' do + it 'returns true for all stages' do + expect(subject.permissions.values.uniq).to eq([true]) + end + end + + describe '#stats' do + before do + allow_any_instance_of(Gitlab::ReferenceExtractor).to receive(:issues).and_return([issue]) + + create_cycle(user, project, issue, mr, milestone, pipeline) + deploy_master(user, project) + end + + it 'returns medians for each stage for a specific group' do + expect(subject.no_stats?).to eq(false) + end + end + + describe '#summary' do + before do + create_cycle(user, project, issue, mr, milestone, pipeline) + deploy_master(user, project) + end + + it 'returns medians for each stage for a specific group' do + expect(subject.summary.map { |summary| summary[:value] }).to contain_exactly(1, 1) + end + end +end diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb index 07d60be091a..8cdf83b1292 100644 --- a/spec/models/cycle_analytics/issue_spec.rb +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -8,7 +8,8 @@ describe 'CycleAnalytics#issue' do let(:project) { create(:project, :repository) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + + subject { CycleAnalytics::ProjectLevel.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :issue, @@ -23,7 +24,7 @@ describe 'CycleAnalytics#issue' do ["list label added to issue", -> (context, data) do if data[:issue].persisted? - data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) + data[:issue].update(label_ids: [context.create(:list).label_id]) end end]], post_fn: -> (context, data) do @@ -42,7 +43,7 @@ describe 'CycleAnalytics#issue' do create_merge_request_closing_issue(user, project, issue) merge_merge_requests_closing_issue(user, project, issue) - expect(subject[:issue].median).to be_nil + expect(subject[:issue].project_median).to be_nil end end end diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb index 3d22a284264..28ad9bd194d 100644 --- a/spec/models/cycle_analytics/plan_spec.rb +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -8,7 +8,8 @@ describe 'CycleAnalytics#plan' do let(:project) { create(:project, :repository) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + + subject { CycleAnalytics::ProjectLevel.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :plan, @@ -24,7 +25,7 @@ describe 'CycleAnalytics#plan' do end], ["list label added to issue", -> (context, data) do - data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) + data[:issue].update(label_ids: [context.create(:list).label_id]) end]], end_time_conditions: [["issue mentioned in a commit", -> (context, data) do @@ -46,7 +47,7 @@ describe 'CycleAnalytics#plan' do create_merge_request_closing_issue(user, project, issue, source_branch: branch_name) merge_merge_requests_closing_issue(user, project, issue) - expect(subject[:issue].median).to be_nil + expect(subject[:issue].project_median).to be_nil end end end diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb index 383727cd8f7..613c1786540 100644 --- a/spec/models/cycle_analytics/production_spec.rb +++ b/spec/models/cycle_analytics/production_spec.rb @@ -8,7 +8,8 @@ describe 'CycleAnalytics#production' do let(:project) { create(:project, :repository) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + + subject { CycleAnalytics::ProjectLevel.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :production, @@ -40,7 +41,7 @@ describe 'CycleAnalytics#production' do MergeRequests::MergeService.new(project, user).execute(merge_request) deploy_master(user, project) - expect(subject[:production].median).to be_nil + expect(subject[:production].project_median).to be_nil end end @@ -51,7 +52,7 @@ describe 'CycleAnalytics#production' do MergeRequests::MergeService.new(project, user).execute(merge_request) deploy_master(user, project, environment: 'staging') - expect(subject[:production].median).to be_nil + expect(subject[:production].project_median).to be_nil end end end diff --git a/spec/models/cycle_analytics_spec.rb b/spec/models/cycle_analytics/project_level_spec.rb index 5d8b5b573cf..4de01b1c679 100644 --- a/spec/models/cycle_analytics_spec.rb +++ b/spec/models/cycle_analytics/project_level_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe CycleAnalytics do +describe CycleAnalytics::ProjectLevel do let(:project) { create(:project, :repository) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } @@ -11,9 +11,9 @@ describe CycleAnalytics do let(:mr) { create_merge_request_closing_issue(user, project, issue, commit_message: "References #{issue.to_reference}") } let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: project, ref: mr.source_branch, sha: mr.source_branch_sha, head_pipeline_of: mr) } - subject { described_class.new(project, from: from_date) } + subject { described_class.new(project, options: { from: from_date }) } - describe '#all_medians_per_stage' do + describe '#all_medians_by_stage' do before do allow_any_instance_of(Gitlab::ReferenceExtractor).to receive(:issues).and_return([issue]) @@ -23,10 +23,10 @@ describe CycleAnalytics do it 'returns every median for each stage for a specific project' do values = described_class::STAGES.each_with_object({}) do |stage_name, hsh| - hsh[stage_name] = subject[stage_name].median.presence + hsh[stage_name] = subject[stage_name].project_median.presence end - expect(subject.all_medians_per_stage).to eq(values) + expect(subject.all_medians_by_stage).to eq(values) end end end diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb index 1af5f9cc1f4..ef88fd86340 100644 --- a/spec/models/cycle_analytics/review_spec.rb +++ b/spec/models/cycle_analytics/review_spec.rb @@ -8,7 +8,8 @@ describe 'CycleAnalytics#review' do let(:project) { create(:project, :repository) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + + subject { CycleAnalytics::ProjectLevel.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :review, @@ -27,7 +28,7 @@ describe 'CycleAnalytics#review' do it "returns nil" do MergeRequests::MergeService.new(project, user).execute(create(:merge_request)) - expect(subject[:review].median).to be_nil + expect(subject[:review].project_median).to be_nil end end end diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb index 8375944f03c..571792559d8 100644 --- a/spec/models/cycle_analytics/staging_spec.rb +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -9,7 +9,7 @@ describe 'CycleAnalytics#staging' do let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + subject { CycleAnalytics::ProjectLevel.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :staging, @@ -45,7 +45,7 @@ describe 'CycleAnalytics#staging' do MergeRequests::MergeService.new(project, user).execute(merge_request) deploy_master(user, project) - expect(subject[:staging].median).to be_nil + expect(subject[:staging].project_median).to be_nil end end @@ -56,7 +56,7 @@ describe 'CycleAnalytics#staging' do MergeRequests::MergeService.new(project, user).execute(merge_request) deploy_master(user, project, environment: 'staging') - expect(subject[:staging].median).to be_nil + expect(subject[:staging].project_median).to be_nil end end end diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb index b78258df564..7b3001d2bd8 100644 --- a/spec/models/cycle_analytics/test_spec.rb +++ b/spec/models/cycle_analytics/test_spec.rb @@ -8,7 +8,8 @@ describe 'CycleAnalytics#test' do let(:project) { create(:project, :repository) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + + subject { CycleAnalytics::ProjectLevel.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :test, @@ -35,7 +36,7 @@ describe 'CycleAnalytics#test' do merge_merge_requests_closing_issue(user, project, issue) - expect(subject[:test].median).to be_nil + expect(subject[:test].project_median).to be_nil end end @@ -46,7 +47,7 @@ describe 'CycleAnalytics#test' do pipeline.run! pipeline.succeed! - expect(subject[:test].median).to be_nil + expect(subject[:test].project_median).to be_nil end end @@ -61,7 +62,7 @@ describe 'CycleAnalytics#test' do merge_merge_requests_closing_issue(user, project, issue) - expect(subject[:test].median).to be_nil + expect(subject[:test].project_median).to be_nil end end @@ -76,7 +77,7 @@ describe 'CycleAnalytics#test' do merge_merge_requests_closing_issue(user, project, issue) - expect(subject[:test].median).to be_nil + expect(subject[:test].project_median).to be_nil end end end diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 2fe82eaa778..8d951ab6f0f 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -8,6 +8,15 @@ describe DeployToken do it { is_expected.to have_many :project_deploy_tokens } it { is_expected.to have_many(:projects).through(:project_deploy_tokens) } + describe 'validations' do + let(:username_format_message) { "can contain only letters, digits, '_', '-', '+', and '.'" } + + it { is_expected.to validate_length_of(:username).is_at_most(255) } + it { is_expected.to allow_value('GitLab+deploy_token-3.14').for(:username) } + it { is_expected.not_to allow_value('<script>').for(:username).with_message(username_format_message) } + it { is_expected.not_to allow_value('').for(:username).with_message(username_format_message) } + end + describe '#ensure_token' do it 'ensures a token' do deploy_token.token = nil @@ -87,8 +96,30 @@ describe DeployToken do end describe '#username' do - it 'returns a harcoded username' do - expect(deploy_token.username).to eq("gitlab+deploy-token-#{deploy_token.id}") + context 'persisted records' do + it 'returns a default username if none is set' do + expect(deploy_token.username).to eq("gitlab+deploy-token-#{deploy_token.id}") + end + + it 'returns the username provided if one is set' do + deploy_token = create(:deploy_token, username: 'deployer') + + expect(deploy_token.username).to eq('deployer') + end + end + + context 'new records' do + it 'returns nil if no username is set' do + deploy_token = build(:deploy_token) + + expect(deploy_token.username).to be_nil + end + + it 'returns the username provided if one is set' do + deploy_token = build(:deploy_token, username: 'deployer') + + expect(deploy_token.username).to eq('deployer') + end end end diff --git a/spec/models/deployment_metrics_spec.rb b/spec/models/deployment_metrics_spec.rb new file mode 100644 index 00000000000..7c574a8b6c8 --- /dev/null +++ b/spec/models/deployment_metrics_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe DeploymentMetrics do + describe '#has_metrics?' do + subject { described_class.new(deployment.project, deployment).has_metrics? } + + context 'when deployment is failed' do + let(:deployment) { create(:deployment, :failed) } + + it { is_expected.to be_falsy } + end + + context 'when deployment is success' do + let(:deployment) { create(:deployment, :success) } + + context 'without a monitoring service' do + it { is_expected.to be_falsy } + end + + context 'with a Prometheus Service' do + let(:prometheus_service) { instance_double(PrometheusService, can_query?: true) } + + before do + allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service + end + + it { is_expected.to be_truthy } + end + + context 'with a Prometheus Service that cannot query' do + let(:prometheus_service) { instance_double(PrometheusService, can_query?: false) } + + before do + allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service + end + + it { is_expected.to be_falsy } + end + + context 'with a cluster Prometheus' do + let(:deployment) { create(:deployment, :success, :on_cluster) } + let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: deployment.cluster) } + + before do + expect(deployment.cluster.application_prometheus).to receive(:can_query?).and_return(true) + end + + it { is_expected.to be_truthy } + end + end + end + + describe '#metrics' do + let(:deployment) { create(:deployment, :success) } + let(:prometheus_adapter) { instance_double(PrometheusService, can_query?: true) } + let(:deployment_metrics) { described_class.new(deployment.project, deployment) } + + subject { deployment_metrics.metrics } + + context 'metrics are disabled' do + it { is_expected.to eq({}) } + end + + context 'metrics are enabled' do + let(:simple_metrics) do + { + success: true, + metrics: {}, + last_update: 42 + } + end + + before do + allow(deployment_metrics).to receive(:prometheus_adapter).and_return(prometheus_adapter) + expect(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics) + end + + it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } + end + end + + describe '#additional_metrics' do + let(:project) { create(:project, :repository) } + let(:deployment) { create(:deployment, :succeed, project: project) } + let(:deployment_metrics) { described_class.new(deployment.project, deployment) } + + subject { deployment_metrics.additional_metrics } + + context 'metrics are disabled' do + it { is_expected.to eq({}) } + end + + context 'metrics are enabled' do + let(:simple_metrics) do + { + success: true, + metrics: {}, + last_update: 42 + } + end + + let(:prometheus_adapter) { instance_double('prometheus_adapter', can_query?: true) } + + before do + allow(deployment_metrics).to receive(:prometheus_adapter).and_return(prometheus_adapter) + expect(prometheus_adapter).to receive(:query).with(:additional_metrics_deployment, deployment).and_return(simple_metrics) + end + + it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } + end + end +end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 1dceef3fc00..d4e631f109b 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -7,6 +7,7 @@ describe Deployment do it { is_expected.to belong_to(:project).required } it { is_expected.to belong_to(:environment).required } + it { is_expected.to belong_to(:cluster).class_name('Clusters::Cluster') } it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:deployable) } @@ -294,64 +295,6 @@ describe Deployment do end end - describe '#metrics' do - let(:deployment) { create(:deployment, :success) } - let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } - - subject { deployment.metrics } - - context 'metrics are disabled' do - it { is_expected.to eq({}) } - end - - context 'metrics are enabled' do - let(:simple_metrics) do - { - success: true, - metrics: {}, - last_update: 42 - } - end - - before do - allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) - allow(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics) - end - - it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } - end - end - - describe '#additional_metrics' do - let(:project) { create(:project, :repository) } - let(:deployment) { create(:deployment, :succeed, project: project) } - - subject { deployment.additional_metrics } - - context 'metrics are disabled' do - it { is_expected.to eq({}) } - end - - context 'metrics are enabled' do - let(:simple_metrics) do - { - success: true, - metrics: {}, - last_update: 42 - } - end - - let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } - - before do - allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) - allow(prometheus_adapter).to receive(:query).with(:additional_metrics_deployment, deployment).and_return(simple_metrics) - end - - it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } - end - end - describe '#stop_action' do let(:build) { create(:ci_build) } @@ -379,38 +322,4 @@ describe Deployment do end end end - - describe '#cluster' do - let(:deployment) { create(:deployment) } - let(:project) { deployment.project } - let(:environment) { deployment.environment } - - subject { deployment.cluster } - - before do - expect(project).to receive(:deployment_platform) - .with(environment: environment.name).and_call_original - end - - context 'project has no deployment platform' do - before do - expect(project.clusters).to be_empty - end - - it { is_expected.to be_nil } - end - - context 'project uses the kubernetes service for deployments' do - let!(:service) { create(:kubernetes_service, project: project) } - - it { is_expected.to be_nil } - end - - context 'project has a deployment platform' do - let!(:cluster) { create(:cluster, projects: [project]) } - let!(:platform) { create(:cluster_platform_kubernetes, cluster: cluster) } - - it { is_expected.to eq cluster } - end - end end diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index d9e1fe4b165..8d7dafc523d 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -34,6 +34,10 @@ describe DiffNote do subject { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) } + describe 'validations' do + it_behaves_like 'a valid diff positionable note', :diff_note_on_commit + end + describe "#position=" do context "when provided a string" do it "sets the position" do diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb index 0d02165787a..950bdec4d00 100644 --- a/spec/models/discussion_spec.rb +++ b/spec/models/discussion_spec.rb @@ -10,6 +10,20 @@ describe Discussion do let(:second_note) { create(:diff_note_on_merge_request, in_reply_to: first_note) } let(:third_note) { create(:diff_note_on_merge_request) } + describe '.lazy_find' do + let!(:note1) { create(:discussion_note_on_merge_request).to_discussion } + let!(:note2) { create(:discussion_note_on_merge_request, in_reply_to: note1).to_discussion } + + subject { [note1, note2].map { |note| described_class.lazy_find(note.discussion_id) } } + + it 'batches requests' do + expect do + [described_class.lazy_find(note1.id), + described_class.lazy_find(note2.id)].map(&:__sync) + end.not_to exceed_query_limit(1) + end + end + describe '.build' do it 'returns a discussion of the right type' do discussion = described_class.build([first_note, second_note], merge_request) @@ -29,4 +43,12 @@ describe Discussion do ]) end end + + describe 'authorization' do + it 'delegates to the first note' do + policy = DeclarativePolicy.policy_for(instance_double(User, id: 1), subject) + + expect(policy).to be_a(NotePolicy) + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 7233d2454c6..521c4704c87 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -2,10 +2,15 @@ require 'spec_helper' -describe Environment do +describe Environment, :use_clean_rails_memory_store_caching do + include ReactiveCachingHelpers + using RSpec::Parameterized::TableSyntax + let(:project) { create(:project, :stubbed_repository) } subject(:environment) { create(:environment, project: project) } + it { is_expected.to be_kind_of(ReactiveCaching) } + it { is_expected.to belong_to(:project).required } it { is_expected.to have_many(:deployments) } @@ -515,29 +520,19 @@ describe Environment do context 'when the environment is available' do context 'with a deployment service' do - shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do - context 'and a deployment' do + context 'when user configured kubernetes from CI/CD > Clusters' do + let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } + + context 'with deployment' do let!(:deployment) { create(:deployment, :success, environment: environment) } it { is_expected.to be_truthy } end - context 'but no deployments' do + context 'without deployments' do it { is_expected.to be_falsy } end end - - context 'when user configured kubernetes from Integration > Kubernetes' do - let(:project) { create(:kubernetes_project) } - - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' - end - - context 'when user configured kubernetes from CI/CD > Clusters' do - let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } - - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' - end end context 'without a deployment service' do @@ -546,8 +541,6 @@ describe Environment do end context 'when the environment is unavailable' do - let(:project) { create(:kubernetes_project) } - before do environment.stop end @@ -582,45 +575,96 @@ describe Environment do end end + describe '#deployment_namespace' do + let(:environment) { create(:environment) } + + subject { environment.deployment_namespace } + + before do + allow(environment).to receive(:deployment_platform).and_return(deployment_platform) + end + + context 'no deployment platform available' do + let(:deployment_platform) { nil } + + it { is_expected.to be_nil } + end + + context 'deployment platform is available' do + let(:cluster) { create(:cluster, :provided_by_user, :project, projects: [environment.project]) } + let(:deployment_platform) { cluster.platform } + + it 'retrieves a namespace from the cluster' do + expect(cluster).to receive(:kubernetes_namespace_for) + .with(environment).and_return('mock-namespace') + + expect(subject).to eq 'mock-namespace' + end + end + end + describe '#terminals' do subject { environment.terminals } - context 'when the environment has terminals' do + before do + allow(environment).to receive(:deployment_platform).and_return(double) + end + + context 'reactive cache is empty' do before do - allow(environment).to receive(:has_terminals?).and_return(true) + stub_reactive_cache(environment, nil) end - shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do - it 'returns the terminals from the deployment service' do - expect(environment.deployment_platform) - .to receive(:terminals).with(environment) - .and_return(:fake_terminals) + it { is_expected.to be_nil } + end + + context 'reactive cache has pod data' do + let(:cache_data) { Hash(pods: %w(pod1 pod2)) } - is_expected.to eq(:fake_terminals) - end + before do + stub_reactive_cache(environment, cache_data) end - context 'when user configured kubernetes from Integration > Kubernetes' do - let(:project) { create(:kubernetes_project) } + it 'retrieves terminals from the deployment platform' do + expect(environment.deployment_platform) + .to receive(:terminals).with(environment, cache_data) + .and_return(:fake_terminals) - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' + is_expected.to eq(:fake_terminals) end + end + end - context 'when user configured kubernetes from CI/CD > Clusters' do - let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } + describe '#calculate_reactive_cache' do + let(:cluster) { create(:cluster, :project, :provided_by_user) } + let(:project) { cluster.project } + let(:environment) { create(:environment, project: project) } + let!(:deployment) { create(:deployment, :success, environment: environment) } - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' - end + subject { environment.calculate_reactive_cache } + + it 'returns cache data from the deployment platform' do + expect(environment.deployment_platform).to receive(:calculate_reactive_cache_for) + .with(environment).and_return(pods: %w(pod1 pod2)) + + is_expected.to eq(pods: %w(pod1 pod2)) end - context 'when the environment does not have terminals' do + context 'environment does not have terminals available' do before do allow(environment).to receive(:has_terminals?).and_return(false) end it { is_expected.to be_nil } end + + context 'project is pending deletion' do + before do + allow(environment.project).to receive(:pending_delete?).and_return(true) + end + + it { is_expected.to be_nil } + end end describe '#has_metrics?' do @@ -747,32 +791,6 @@ describe Environment do end end - describe '#generate_slug' do - SUFFIX = "-[a-z0-9]{6}".freeze - { - "staging-12345678901234567" => "staging-123456789" + SUFFIX, - "9-staging-123456789012345" => "env-9-staging-123" + SUFFIX, - "staging-1234567890123456" => "staging-1234567890123456", - "production" => "production", - "PRODUCTION" => "production" + SUFFIX, - "review/1-foo" => "review-1-foo" + SUFFIX, - "1-foo" => "env-1-foo" + SUFFIX, - "1/foo" => "env-1-foo" + SUFFIX, - "foo-" => "foo" + SUFFIX, - "foo--bar" => "foo-bar" + SUFFIX, - "foo**bar" => "foo-bar" + SUFFIX, - "*-foo" => "env-foo" + SUFFIX, - "staging-12345678-" => "staging-12345678" + SUFFIX, - "staging-12345678-01234567" => "staging-12345678" + SUFFIX - }.each do |name, matcher| - it "returns a slug matching #{matcher}, given #{name}" do - slug = described_class.new(name: name).generate_slug - - expect(slug).to match(/\A#{matcher}\z/) - end - end - end - describe '#ref_path' do subject(:environment) do create(:environment, name: 'staging / review-1') @@ -793,12 +811,9 @@ describe Environment do let(:source_path) { 'source/file.html' } let(:sha) { RepoHelpers.sample_commit.id } - before do - environment.external_url = 'http://example.com' - end - context 'when the public path is not known' do before do + environment.external_url = 'http://example.com' allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return(nil) end @@ -808,12 +823,23 @@ describe Environment do end context 'when the public path is known' do - before do - allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return('file.html') - end - - it 'returns the full external URL' do - expect(environment.external_url_for(source_path, sha)).to eq('http://example.com/file.html') + where(:external_url, :public_path, :full_url) do + 'http://example.com' | 'file.html' | 'http://example.com/file.html' + 'http://example.com/' | 'file.html' | 'http://example.com/file.html' + 'http://example.com' | '/file.html' | 'http://example.com/file.html' + 'http://example.com/' | '/file.html' | 'http://example.com/file.html' + 'http://example.com/subpath' | 'public/file.html' | 'http://example.com/subpath/public/file.html' + 'http://example.com/subpath/' | 'public/file.html' | 'http://example.com/subpath/public/file.html' + 'http://example.com/subpath' | '/public/file.html' | 'http://example.com/subpath/public/file.html' + 'http://example.com/subpath/' | '/public/file.html' | 'http://example.com/subpath/public/file.html' + end + with_them do + it 'returns the full external URL' do + environment.external_url = external_url + allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return(public_path) + + expect(environment.external_url_for(source_path, sha)).to eq(full_url) + end end end end @@ -825,4 +851,35 @@ describe Environment do subject.prometheus_adapter end end + + describe '#knative_services_finder' do + let(:environment) { create(:environment) } + + subject { environment.knative_services_finder } + + context 'environment has no deployments' do + it { is_expected.to be_nil } + end + + context 'environment has a deployment' do + let!(:deployment) { create(:deployment, :success, environment: environment, cluster: cluster) } + + context 'with no cluster associated' do + let(:cluster) { nil } + + it { is_expected.to be_nil } + end + + context 'with a cluster associated' do + let(:cluster) { create(:cluster) } + + it 'calls the service finder' do + expect(Clusters::KnativeServicesFinder).to receive(:new) + .with(cluster, environment).and_return(:finder) + + is_expected.to eq :finder + end + end + end + end end diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index c503c35305f..e2836420df9 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -11,11 +11,10 @@ describe EnvironmentStatus do let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) } let(:sha) { deployment.sha } - subject(:environment_status) { described_class.new(environment, merge_request, sha) } + subject(:environment_status) { described_class.new(project, environment, merge_request, sha) } it { is_expected.to delegate_method(:id).to(:environment) } it { is_expected.to delegate_method(:name).to(:environment) } - it { is_expected.to delegate_method(:project).to(:environment) } it { is_expected.to delegate_method(:deployed_at).to(:deployment) } it { is_expected.to delegate_method(:status).to(:deployment) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d7accbef6bd..1c41ceb7deb 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -23,6 +23,7 @@ describe Group do it { is_expected.to have_many(:badges).class_name('GroupBadge') } it { is_expected.to have_many(:cluster_groups).class_name('Clusters::Group') } it { is_expected.to have_many(:clusters).class_name('Clusters::Cluster') } + it { is_expected.to have_many(:container_repositories) } describe '#members & #requesters' do let(:requester) { create(:user) } @@ -71,7 +72,7 @@ describe Group do end end - describe '#notification_settings', :nested_groups do + describe '#notification_settings' do let(:user) { create(:user) } let(:group) { create(:group) } let(:sub_group) { create(:group, parent_id: group.id) } @@ -95,6 +96,43 @@ describe Group do end end + describe '#notification_email_for' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:subgroup) { create(:group, parent: group) } + + let(:group_notification_email) { 'user+group@example.com' } + let(:subgroup_notification_email) { 'user+subgroup@example.com' } + + subject { subgroup.notification_email_for(user) } + + context 'when both group notification emails are set' do + it 'returns subgroup notification email' do + create(:notification_setting, user: user, source: group, notification_email: group_notification_email) + create(:notification_setting, user: user, source: subgroup, notification_email: subgroup_notification_email) + + is_expected.to eq(subgroup_notification_email) + end + end + + context 'when subgroup notification email is blank' do + it 'returns parent group notification email' do + create(:notification_setting, user: user, source: group, notification_email: group_notification_email) + create(:notification_setting, user: user, source: subgroup, notification_email: '') + + is_expected.to eq(group_notification_email) + end + end + + context 'when only the parent group notification email is set' do + it 'returns parent group notification email' do + create(:notification_setting, user: user, source: group, notification_email: group_notification_email) + + is_expected.to eq(group_notification_email) + end + end + end + describe '#visibility_level_allowed_by_parent' do let(:parent) { create(:group, :internal) } let(:sub_group) { build(:group, parent_id: parent.id) } @@ -200,7 +238,7 @@ describe Group do it { is_expected.to match_array([private_group, internal_group, group]) } end - context 'when user is a member of private subgroup', :postgresql do + context 'when user is a member of private subgroup' do let!(:private_subgroup) { create(:group, :private, parent: private_group) } before do @@ -379,7 +417,7 @@ describe Group do it { expect(group.last_owner?(@members[:owner])).to be_falsy } end - context 'with owners from a parent', :postgresql do + context 'with owners from a parent' do before do parent_group = create(:group) create(:group_member, :owner, group: parent_group) @@ -487,7 +525,7 @@ describe Group do it { expect(subject.parent).to be_kind_of(described_class) } end - describe '#members_with_parents', :nested_groups do + describe '#members_with_parents' do let!(:group) { create(:group, :nested) } let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } @@ -498,7 +536,7 @@ describe Group do end end - describe '#direct_and_indirect_members', :nested_groups do + describe '#direct_and_indirect_members' do let!(:group) { create(:group, :nested) } let!(:sub_group) { create(:group, parent: group) } let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } @@ -515,7 +553,7 @@ describe Group do end end - describe '#users_with_descendants', :nested_groups do + describe '#users_with_descendants' do let(:user_a) { create(:user) } let(:user_b) { create(:user) } @@ -534,7 +572,7 @@ describe Group do end end - describe '#direct_and_indirect_users', :nested_groups do + describe '#direct_and_indirect_users' do let(:user_a) { create(:user) } let(:user_b) { create(:user) } let(:user_c) { create(:user) } @@ -564,7 +602,7 @@ describe Group do end end - describe '#project_users_with_descendants', :nested_groups do + describe '#project_users_with_descendants' do let(:user_a) { create(:user) } let(:user_b) { create(:user) } let(:user_c) { create(:user) } @@ -641,7 +679,7 @@ describe Group do end end - context 'sub groups and projects', :nested_groups do + context 'sub groups and projects' do it 'enables two_factor_requirement for group member' do group.add_user(user, GroupMember::OWNER) @@ -650,7 +688,7 @@ describe Group do expect(user.reload.require_two_factor_authentication_from_group).to be_truthy end - context 'expanded group members', :nested_groups do + context 'expanded group members' do let(:indirect_user) { create(:user) } it 'enables two_factor_requirement for subgroup member' do @@ -683,7 +721,7 @@ describe Group do expect(user.reload.require_two_factor_authentication_from_group).to be_falsey end - it 'does not enable two_factor_requirement for subgroup child project member', :nested_groups do + it 'does not enable two_factor_requirement for subgroup child project member' do subgroup = create(:group, :nested, parent: group) project = create(:project, group: subgroup) project.add_maintainer(user) @@ -783,7 +821,7 @@ describe Group do it_behaves_like 'ref is protected' end - context 'when group has children', :postgresql do + context 'when group has children' do let(:group_child) { create(:group, parent: group) } let(:group_child_2) { create(:group, parent: group_child) } let(:group_child_3) { create(:group, parent: group_child_2) } @@ -806,7 +844,7 @@ describe Group do end end - describe '#highest_group_member', :nested_groups do + describe '#highest_group_member' do let(:nested_group) { create(:group, parent: group) } let(:nested_group_2) { create(:group, parent: nested_group) } let(:user) { create(:user) } @@ -866,35 +904,6 @@ describe Group do end end - describe '#group_clusters_enabled?' do - before do - # Override global stub in spec/spec_helper.rb - expect(Feature).to receive(:enabled?).and_call_original - end - - subject { group.group_clusters_enabled? } - - it { is_expected.to be_truthy } - - context 'explicitly disabled for root ancestor' do - before do - feature = Feature.get(:group_clusters) - feature.disable(group.root_ancestor) - end - - it { is_expected.to be_falsey } - end - - context 'explicitly disabled for root ancestor' do - before do - feature = Feature.get(:group_clusters) - feature.enable(group.root_ancestor) - end - - it { is_expected.to be_truthy } - end - end - describe '#first_auto_devops_config' do using RSpec::Parameterized::TableSyntax @@ -924,7 +933,7 @@ describe Group do it { is_expected.to eq(config) } end - context 'with parent groups', :nested_groups do + context 'with parent groups' do where(:instance_value, :parent_value, :group_value, :config) do # Instance level enabled true | nil | nil | { status: true, scope: :instance } @@ -1023,4 +1032,11 @@ describe Group do expect(group.project_creation_level).to eq(Gitlab::CurrentSettings.default_project_creation) end end + + describe 'subgroup_creation_level' do + it 'defaults to maintainers' do + expect(group.subgroup_creation_level) + .to eq(Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS) + end + end end diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb index 806b4f61bd8..28630f7d3fe 100644 --- a/spec/models/internal_id_spec.rb +++ b/spec/models/internal_id_spec.rb @@ -158,7 +158,7 @@ describe InternalId do before do described_class.reset_column_information # Project factory will also call the current_version - expect(ActiveRecord::Migrator).to receive(:current_version).twice.and_return(InternalId::REQUIRED_SCHEMA_VERSION - 1) + expect(ActiveRecord::Migrator).to receive(:current_version).at_least(:once).and_return(InternalId::REQUIRED_SCHEMA_VERSION - 1) end it 'does not reset any of the iids' do diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb index 07858fe8a70..7aa0d97b194 100644 --- a/spec/models/issue/metrics_spec.rb +++ b/spec/models/issue/metrics_spec.rb @@ -32,7 +32,7 @@ describe Issue::Metrics do context "list labels" do it "records the first time an issue is associated with a list label" do - list_label = create(:label, lists: [create(:list)]) + list_label = create(:list).label time = Time.now Timecop.freeze(time) { subject.update(label_ids: [list_label.id]) } metrics = subject.metrics @@ -43,9 +43,9 @@ describe Issue::Metrics do it "does not record the second time an issue is associated with a list label" do time = Time.now - first_list_label = create(:label, lists: [create(:list)]) + first_list_label = create(:list).label Timecop.freeze(time) { subject.update(label_ids: [first_list_label.id]) } - second_list_label = create(:label, lists: [create(:list)]) + second_list_label = create(:list).label Timecop.freeze(time + 5.hours) { subject.update(label_ids: [second_list_label.id]) } metrics = subject.metrics diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index a5c7e9db2a1..2e7d78d77a8 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -862,4 +862,21 @@ describe Issue do end end end + + describe "#labels_hook_attrs" do + let(:label) { create(:label) } + let(:issue) { create(:labeled_issue, labels: [label]) } + + it "returns a list of label hook attributes" do + expect(issue.labels_hook_attrs).to eq([label.hook_attrs]) + end + end + + context "relative positioning" do + it_behaves_like "a class that supports relative positioning" do + let(:project) { create(:project) } + let(:factory) { :issue } + let(:default_params) { { project: project } } + end + end end diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 5174c590a10..c2e2298823e 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -193,4 +193,17 @@ describe Label do expect(described_class.optionally_subscribed_by(nil)).to match_array([label, label2]) end end + + describe '#templates' do + context 'with invalid template labels' do + it 'returns only valid template labels' do + create(:label) + # Project labels should not have template set to true + create(:label, template: true) + valid_template_label = described_class.create!(title: 'test', template: true, type: nil) + + expect(described_class.templates).to eq([valid_template_label]) + end + end + end end diff --git a/spec/models/lfs_download_object_spec.rb b/spec/models/lfs_download_object_spec.rb index effd8b08124..8b53effe98f 100644 --- a/spec/models/lfs_download_object_spec.rb +++ b/spec/models/lfs_download_object_spec.rb @@ -50,7 +50,7 @@ describe LfsDownloadObject do before do allow(ApplicationSetting) .to receive(:current) - .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: setting)) + .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: setting)) end context 'are allowed' do diff --git a/spec/models/lfs_object_spec.rb b/spec/models/lfs_object_spec.rb index 3d4d4b7d795..85bfc3f1387 100644 --- a/spec/models/lfs_object_spec.rb +++ b/spec/models/lfs_object_spec.rb @@ -3,6 +3,20 @@ require 'spec_helper' describe LfsObject do + it 'has a distinct has_many :projects relation through lfs_objects_projects' do + lfs_object = create(:lfs_object) + project = create(:project) + [:project, :design].each do |repository_type| + create(:lfs_objects_project, project: project, + lfs_object: lfs_object, + repository_type: repository_type) + end + + expect(lfs_object.lfs_objects_projects.size).to eq(2) + expect(lfs_object.projects.size).to eq(1) + expect(lfs_object.projects.to_a).to eql([project]) + end + describe '#local_store?' do it 'returns true when file_store is equal to LfsObjectUploader::Store::LOCAL' do subject.file_store = LfsObjectUploader::Store::LOCAL diff --git a/spec/models/lfs_objects_project_spec.rb b/spec/models/lfs_objects_project_spec.rb index 3e86ee38566..e320f873989 100644 --- a/spec/models/lfs_objects_project_spec.rb +++ b/spec/models/lfs_objects_project_spec.rb @@ -20,8 +20,8 @@ describe LfsObjectsProject do it 'validates object id' do is_expected.to validate_uniqueness_of(:lfs_object_id) - .scoped_to(:project_id) - .with_message("already exists in project") + .scoped_to(:project_id, :repository_type) + .with_message("already exists in repository") end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 782a84f922b..2cb4f222ea4 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -172,6 +172,19 @@ describe Member do it { expect(described_class.non_request).to include @accepted_request_member } end + describe '.search_invite_email' do + it 'returns only members the matching e-mail' do + create(:group_member, :invited) + + invited = described_class.search_invite_email(@invited_member.invite_email) + + expect(invited.count).to eq(1) + expect(invited.first).to eq(@invited_member) + + expect(described_class.search_invite_email('bad-email@example.com').count).to eq(0) + end + end + describe '.developers' do subject { described_class.developers.to_a } diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index f227abd3dae..ad7dfac87af 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -3,19 +3,29 @@ require 'spec_helper' describe GroupMember do - describe '.count_users_by_group_id' do - it 'counts users by group ID' do - user_1 = create(:user) - user_2 = create(:user) - group_1 = create(:group) - group_2 = create(:group) - - group_1.add_owner(user_1) - group_1.add_owner(user_2) - group_2.add_owner(user_1) - - expect(described_class.count_users_by_group_id).to eq(group_1.id => 2, - group_2.id => 1) + context 'scopes' do + describe '.count_users_by_group_id' do + it 'counts users by group ID' do + user_1 = create(:user) + user_2 = create(:user) + group_1 = create(:group) + group_2 = create(:group) + + group_1.add_owner(user_1) + group_1.add_owner(user_2) + group_2.add_owner(user_1) + + expect(described_class.count_users_by_group_id).to eq(group_1.id => 2, + group_2.id => 1) + end + end + + describe '.of_ldap_type' do + it 'returns ldap type users' do + group_member = create(:group_member, :ldap) + + expect(described_class.of_ldap_type).to eq([group_member]) + end end end @@ -69,7 +79,7 @@ describe GroupMember do end end - context 'access levels', :nested_groups do + context 'access levels' do context 'with parent group' do it_behaves_like 'inherited access level as a member of entity' do let(:entity) { create(:group, parent: parent_entity) } diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 497764b6825..79c39b81196 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -130,7 +130,7 @@ describe ProjectMember do end end - context 'with parent group and a subgroup', :nested_groups do + context 'with parent group and a subgroup' do it_behaves_like 'inherited access level as a member of entity' do let(:subgroup) { create(:group, parent: parent_entity) } let(:entity) { create(:project, group: subgroup) } diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index a53add67066..e7dd7287a75 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -484,4 +484,12 @@ describe MergeRequestDiff do end end end + + describe '#lines_count' do + subject { diff_with_commits } + + it 'returns sum of all changed lines count in diff files' do + expect(subject.lines_count).to eq 109 + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c6251326c22..d344a6d0f0d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -7,6 +7,8 @@ describe MergeRequest do include ProjectForksHelper include ReactiveCachingHelpers + using RSpec::Parameterized::TableSyntax + subject { create(:merge_request) } describe 'associations' do @@ -1986,6 +1988,7 @@ describe MergeRequest do params = {} merge_jid = 'hash-123' + expect(merge_request).to receive(:expire_etag_cache) expect(MergeWorker).to receive(:perform_async).with(merge_request.id, user_id, params) do merge_jid end @@ -1996,54 +1999,45 @@ describe MergeRequest do end end - describe '#check_if_can_be_merged' do - let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } - - shared_examples 'checking if can be merged' do - context 'when it is not broken and has no conflicts' do - before do - allow(subject).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?).and_return(true) - end + describe '#rebase_async' do + let(:merge_request) { create(:merge_request) } + let(:user_id) { double(:user_id) } + let(:rebase_jid) { 'rebase-jid' } - it 'is marked as mergeable' do - expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged') - end - end + subject(:execute) { merge_request.rebase_async(user_id) } - context 'when broken' do - before do - allow(subject).to receive(:broken?) { true } - allow(project.repository).to receive(:can_be_merged?).and_return(false) - end + it 'atomically enqueues a RebaseWorker job and updates rebase_jid' do + expect(RebaseWorker) + .to receive(:perform_async) + .with(merge_request.id, user_id) + .and_return(rebase_jid) - it 'becomes unmergeable' do - expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') - end - end + expect(merge_request).to receive(:expire_etag_cache) + expect(merge_request).to receive(:lock!).and_call_original - context 'when it has conflicts' do - before do - allow(subject).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?).and_return(false) - end + execute - it 'becomes unmergeable' do - expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') - end - end + expect(merge_request.rebase_jid).to eq(rebase_jid) end - context 'when merge_status is unchecked' do - subject { create(:merge_request, source_project: project, merge_status: :unchecked) } + it 'refuses to enqueue a job if a rebase is in progress' do + merge_request.update_column(:rebase_jid, rebase_jid) + + expect(RebaseWorker).not_to receive(:perform_async) + expect(Gitlab::SidekiqStatus) + .to receive(:running?) + .with(rebase_jid) + .and_return(true) - it_behaves_like 'checking if can be merged' + expect { execute }.to raise_error(ActiveRecord::StaleObjectError) end - context 'when merge_status is unchecked' do - subject { create(:merge_request, source_project: project, merge_status: :cannot_be_merged_recheck) } + it 'refuses to enqueue a job if the MR is not open' do + merge_request.update_column(:state, 'foo') + + expect(RebaseWorker).not_to receive(:perform_async) - it_behaves_like 'checking if can be merged' + expect { execute }.to raise_error(ActiveRecord::StaleObjectError) end end @@ -2060,7 +2054,7 @@ describe MergeRequest do it 'return true if #mergeable_state? is true and the MR #can_be_merged? is true' do allow(subject).to receive(:mergeable_state?) { true } - expect(subject).to receive(:check_if_can_be_merged) + expect(subject).to receive(:check_mergeability) expect(subject).to receive(:can_be_merged?) { true } expect(subject.mergeable?).to be_truthy @@ -2074,7 +2068,7 @@ describe MergeRequest do it 'checks if merge request can be merged' do allow(subject).to receive(:mergeable_ci_state?) { true } - expect(subject).to receive(:check_if_can_be_merged) + expect(subject).to receive(:check_mergeability) subject.mergeable? end @@ -2175,7 +2169,7 @@ describe MergeRequest do allow(subject).to receive(:head_pipeline) { nil } end - it { expect(subject.mergeable_ci_state?).to be_truthy } + it { expect(subject.mergeable_ci_state?).to be_falsey } end end @@ -2462,6 +2456,13 @@ describe MergeRequest do describe "#diff_refs" do context "with diffs" do subject { create(:merge_request, :with_diffs) } + let(:expected_diff_refs) do + Gitlab::Diff::DiffRefs.new( + base_sha: subject.merge_request_diff.base_commit_sha, + start_sha: subject.merge_request_diff.start_commit_sha, + head_sha: subject.merge_request_diff.head_commit_sha + ) + end it "does not touch the repository" do subject # Instantiate the object @@ -2472,14 +2473,18 @@ describe MergeRequest do end it "returns expected diff_refs" do - expected_diff_refs = Gitlab::Diff::DiffRefs.new( - base_sha: subject.merge_request_diff.base_commit_sha, - start_sha: subject.merge_request_diff.start_commit_sha, - head_sha: subject.merge_request_diff.head_commit_sha - ) - expect(subject.diff_refs).to eq(expected_diff_refs) end + + context 'when importing' do + before do + subject.importing = true + end + + it "returns MR diff_refs" do + expect(subject.diff_refs).to eq(expected_diff_refs) + end + end end end @@ -2997,39 +3002,24 @@ describe MergeRequest do end describe '#rebase_in_progress?' do - shared_examples 'checking whether a rebase is in progress' do - let(:repo_path) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - subject.source_project.repository.path - end - end - let(:rebase_path) { File.join(repo_path, "gitlab-worktree", "rebase-#{subject.id}") } - - before do - system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{rebase_path} master)) - end - - it 'returns true when there is a current rebase directory' do - expect(subject.rebase_in_progress?).to be_truthy - end - - it 'returns false when there is no rebase directory' do - FileUtils.rm_rf(rebase_path) + where(:rebase_jid, :jid_valid, :result) do + 'foo' | true | true + 'foo' | false | false + '' | true | false + nil | true | false + end - expect(subject.rebase_in_progress?).to be_falsey - end + with_them do + let(:merge_request) { create(:merge_request) } - it 'returns false when the rebase directory has expired' do - time = 20.minutes.ago.to_time - File.utime(time, time, rebase_path) + subject { merge_request.rebase_in_progress? } - expect(subject.rebase_in_progress?).to be_falsey - end + it do + allow(Gitlab::SidekiqStatus).to receive(:running?).with(rebase_jid) { jid_valid } - it 'returns false when the source project has been removed' do - allow(subject).to receive(:source_project).and_return(nil) + merge_request.rebase_jid = rebase_jid - expect(subject.rebase_in_progress?).to be_falsey + is_expected.to eq(result) end end end @@ -3142,38 +3132,6 @@ describe MergeRequest do end end - describe '#mergeable_to_ref?' do - it 'returns true when merge request is mergeable' do - subject = create(:merge_request) - - expect(subject.mergeable_to_ref?).to be(true) - end - - it 'returns false when merge request is already merged' do - subject = create(:merge_request, :merged) - - expect(subject.mergeable_to_ref?).to be(false) - end - - it 'returns false when merge request is closed' do - subject = create(:merge_request, :closed) - - expect(subject.mergeable_to_ref?).to be(false) - end - - it 'returns false when merge request is work in progress' do - subject = create(:merge_request, title: 'WIP: The feature') - - expect(subject.mergeable_to_ref?).to be(false) - end - - it 'returns false when merge request has no commits' do - subject = create(:merge_request, source_branch: 'empty-branch', target_branch: 'master') - - expect(subject.mergeable_to_ref?).to be(false) - end - end - describe '#merge_participants' do it 'contains author' do expect(subject.merge_participants).to eq([subject.author]) @@ -3236,4 +3194,34 @@ describe MergeRequest do it { is_expected.to be_truthy } end end + + describe '#cleanup_refs' do + subject { merge_request.cleanup_refs(only: only) } + + let(:merge_request) { build(:merge_request) } + + context 'when removing all refs' do + let(:only) { :all } + + it 'deletes all refs from the target project' do + expect(merge_request.target_project.repository) + .to receive(:delete_refs) + .with(merge_request.ref_path, merge_request.merge_ref_path, merge_request.train_ref_path) + + subject + end + end + + context 'when removing only train ref' do + let(:only) { :train } + + it 'deletes train ref from the target project' do + expect(merge_request.target_project.repository) + .to receive(:delete_refs) + .with(merge_request.train_ref_path) + + subject + end + end + end end diff --git a/spec/models/namespace/aggregation_schedule_spec.rb b/spec/models/namespace/aggregation_schedule_spec.rb new file mode 100644 index 00000000000..38bf8089411 --- /dev/null +++ b/spec/models/namespace/aggregation_schedule_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespace::AggregationSchedule, :clean_gitlab_redis_shared_state, type: :model do + include ExclusiveLeaseHelpers + + it { is_expected.to belong_to :namespace } + + describe '#schedule_root_storage_statistics' do + let(:namespace) { create(:namespace) } + let(:aggregation_schedule) { namespace.build_aggregation_schedule } + let(:lease_key) { "namespace:namespaces_root_statistics:#{namespace.id}" } + + context "when we can't obtain the lease" do + it 'does not schedule the workers' do + stub_exclusive_lease_taken(lease_key, timeout: described_class::DEFAULT_LEASE_TIMEOUT) + + expect(Namespaces::RootStatisticsWorker) + .not_to receive(:perform_async) + + expect(Namespaces::RootStatisticsWorker) + .not_to receive(:perform_in) + + aggregation_schedule.save! + end + end + + context 'when we can obtain the lease' do + it 'schedules a root storage statistics after create' do + stub_exclusive_lease(lease_key, timeout: described_class::DEFAULT_LEASE_TIMEOUT) + + expect(Namespaces::RootStatisticsWorker) + .to receive(:perform_async).once + + expect(Namespaces::RootStatisticsWorker) + .to receive(:perform_in).once + .with(described_class::DEFAULT_LEASE_TIMEOUT, aggregation_schedule.namespace_id) + + aggregation_schedule.save! + end + + it 'does not release the lease' do + stub_exclusive_lease(lease_key, timeout: described_class::DEFAULT_LEASE_TIMEOUT) + + aggregation_schedule.save! + + exclusive_lease = aggregation_schedule.exclusive_lease + expect(exclusive_lease.exists?).to be_truthy + end + + it 'only executes the workers once' do + # Avoid automatic deletion of Namespace::AggregationSchedule + # for testing purposes. + expect(Namespaces::RootStatisticsWorker) + .to receive(:perform_async).once + .and_return(nil) + + expect(Namespaces::RootStatisticsWorker) + .to receive(:perform_in).once + .with(described_class::DEFAULT_LEASE_TIMEOUT, aggregation_schedule.namespace_id) + .and_return(nil) + + # Scheduling workers for the first time + aggregation_schedule.schedule_root_storage_statistics + + # Executing again, this time workers should not be scheduled + # due to the lease not been released. + aggregation_schedule.schedule_root_storage_statistics + end + end + end +end diff --git a/spec/models/namespace/root_storage_statistics_spec.rb b/spec/models/namespace/root_storage_statistics_spec.rb new file mode 100644 index 00000000000..5341278db7c --- /dev/null +++ b/spec/models/namespace/root_storage_statistics_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespace::RootStorageStatistics, type: :model do + it { is_expected.to belong_to :namespace } + it { is_expected.to have_one(:route).through(:namespace) } + + it { is_expected.to delegate_method(:all_projects).to(:namespace) } + + describe '#recalculate!' do + let(:namespace) { create(:group) } + let(:root_storage_statistics) { create(:namespace_root_storage_statistics, namespace: namespace) } + + let(:project1) { create(:project, namespace: namespace) } + let(:project2) { create(:project, namespace: namespace) } + + let!(:stat1) { create(:project_statistics, project: project1, with_data: true, size_multiplier: 100) } + let!(:stat2) { create(:project_statistics, project: project2, with_data: true, size_multiplier: 200) } + + shared_examples 'data refresh' do + it 'aggregates project statistics' do + root_storage_statistics.recalculate! + + root_storage_statistics.reload + + total_repository_size = stat1.repository_size + stat2.repository_size + total_wiki_size = stat1.wiki_size + stat2.wiki_size + total_lfs_objects_size = stat1.lfs_objects_size + stat2.lfs_objects_size + total_build_artifacts_size = stat1.build_artifacts_size + stat2.build_artifacts_size + total_packages_size = stat1.packages_size + stat2.packages_size + total_storage_size = stat1.storage_size + stat2.storage_size + + expect(root_storage_statistics.repository_size).to eq(total_repository_size) + expect(root_storage_statistics.wiki_size).to eq(total_wiki_size) + expect(root_storage_statistics.lfs_objects_size).to eq(total_lfs_objects_size) + expect(root_storage_statistics.build_artifacts_size).to eq(total_build_artifacts_size) + expect(root_storage_statistics.packages_size).to eq(total_packages_size) + expect(root_storage_statistics.storage_size).to eq(total_storage_size) + end + + it 'works when there are no projects' do + Project.delete_all + + root_storage_statistics.recalculate! + + root_storage_statistics.reload + expect(root_storage_statistics.repository_size).to eq(0) + expect(root_storage_statistics.wiki_size).to eq(0) + expect(root_storage_statistics.lfs_objects_size).to eq(0) + expect(root_storage_statistics.build_artifacts_size).to eq(0) + expect(root_storage_statistics.packages_size).to eq(0) + expect(root_storage_statistics.storage_size).to eq(0) + end + end + + it_behaves_like 'data refresh' + + context 'with subgroups' do + let(:subgroup1) { create(:group, parent: namespace)} + let(:subgroup2) { create(:group, parent: subgroup1)} + + let(:project1) { create(:project, namespace: subgroup1) } + let(:project2) { create(:project, namespace: subgroup2) } + + it_behaves_like 'data refresh' + end + + context 'with a personal namespace' do + let(:namespace) { create(:user).namespace } + + it_behaves_like 'data refresh' + end + end +end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index d80183af33e..972f26ac745 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -15,6 +15,8 @@ describe Namespace do it { is_expected.to have_many :project_statistics } it { is_expected.to belong_to :parent } it { is_expected.to have_many :children } + it { is_expected.to have_one :root_storage_statistics } + it { is_expected.to have_one :aggregation_schedule } end describe 'validations' do @@ -189,7 +191,7 @@ describe Namespace do end end - describe '#ancestors_upto', :nested_groups do + describe '#ancestors_upto' do let(:parent) { create(:group) } let(:child) { create(:group, parent: parent) } let(:child2) { create(:group, parent: child) } @@ -269,7 +271,7 @@ describe Namespace do end end - context 'with subgroups', :nested_groups do + context 'with subgroups' do let(:parent) { create(:group, name: 'parent', path: 'parent') } let(:new_parent) { create(:group, name: 'new_parent', path: 'new_parent') } let(:child) { create(:group, name: 'child', path: 'child', parent: parent) } @@ -473,7 +475,7 @@ describe Namespace do end end - describe '#self_and_hierarchy', :nested_groups do + describe '#self_and_hierarchy' do let!(:group) { create(:group, path: 'git_lab') } let!(:nested_group) { create(:group, parent: group) } let!(:deep_nested_group) { create(:group, parent: nested_group) } @@ -488,7 +490,7 @@ describe Namespace do end end - describe '#ancestors', :nested_groups do + describe '#ancestors' do let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } let(:deep_nested_group) { create(:group, parent: nested_group) } @@ -502,7 +504,7 @@ describe Namespace do end end - describe '#self_and_ancestors', :nested_groups do + describe '#self_and_ancestors' do let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } let(:deep_nested_group) { create(:group, parent: nested_group) } @@ -516,7 +518,7 @@ describe Namespace do end end - describe '#descendants', :nested_groups do + describe '#descendants' do let!(:group) { create(:group, path: 'git_lab') } let!(:nested_group) { create(:group, parent: group) } let!(:deep_nested_group) { create(:group, parent: nested_group) } @@ -532,7 +534,7 @@ describe Namespace do end end - describe '#self_and_descendants', :nested_groups do + describe '#self_and_descendants' do let!(:group) { create(:group, path: 'git_lab') } let!(:nested_group) { create(:group, parent: group) } let!(:deep_nested_group) { create(:group, parent: nested_group) } @@ -548,7 +550,7 @@ describe Namespace do end end - describe '#users_with_descendants', :nested_groups do + describe '#users_with_descendants' do let(:user_a) { create(:user) } let(:user_b) { create(:user) } @@ -595,7 +597,7 @@ describe Namespace do it { expect(group.all_pipelines.to_a).to match_array([pipeline1, pipeline2]) } end - describe '#share_with_group_lock with subgroups', :nested_groups do + describe '#share_with_group_lock with subgroups' do context 'when creating a subgroup' do let(:subgroup) { create(:group, parent: root_group )} @@ -736,7 +738,7 @@ describe Namespace do end describe '#root_ancestor' do - it 'returns the top most ancestor', :nested_groups do + it 'returns the top most ancestor' do root_group = create(:group) nested_group = create(:group, parent: root_group) deep_nested_group = create(:group, parent: nested_group) @@ -835,4 +837,80 @@ describe Namespace do it { is_expected.to be_falsy } end end + + describe '#aggregation_scheduled?' do + let(:namespace) { create(:namespace) } + + subject { namespace.aggregation_scheduled? } + + context 'with an aggregation scheduled association' do + let(:namespace) { create(:namespace, :with_aggregation_schedule) } + + it { is_expected.to be_truthy } + end + + context 'without an aggregation scheduled association' do + it { is_expected.to be_falsy } + end + end + + describe '#emails_disabled?' do + context 'when not a subgroup' do + it 'returns false' do + group = create(:group, emails_disabled: false) + + expect(group.emails_disabled?).to be_falsey + end + + it 'returns true' do + group = create(:group, emails_disabled: true) + + expect(group.emails_disabled?).to be_truthy + end + end + + context 'when a subgroup' do + let(:grandparent) { create(:group) } + let(:parent) { create(:group, parent: grandparent) } + let(:group) { create(:group, parent: parent) } + + it 'returns false' do + expect(group.emails_disabled?).to be_falsey + end + + context 'when ancestor emails are disabled' do + it 'returns true' do + grandparent.update_attribute(:emails_disabled, true) + + expect(group.emails_disabled?).to be_truthy + end + end + end + + context 'when :emails_disabled feature flag is off' do + before do + stub_feature_flags(emails_disabled: false) + end + + context 'when not a subgroup' do + it 'returns false' do + group = create(:group, emails_disabled: true) + + expect(group.emails_disabled?).to be_falsey + end + end + + context 'when a subgroup and ancestor emails are disabled' do + let(:grandparent) { create(:group) } + let(:parent) { create(:group, parent: grandparent) } + let(:group) { create(:group, parent: parent) } + + it 'returns false' do + grandparent.update_attribute(:emails_disabled, true) + + expect(group.emails_disabled?).to be_falsey + end + end + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 7a1ab20186a..bfd0e5f0558 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -177,6 +177,7 @@ describe Note do pipeline: :note, cache_key: [note1, "note"], project: note1.project, + rendered: note1.note_html, author: note1.author } }]).and_call_original @@ -189,6 +190,7 @@ describe Note do pipeline: :note, cache_key: [note2, "note"], project: note2.project, + rendered: note2.note_html, author: note2.author } }]).and_call_original @@ -911,6 +913,22 @@ describe Note do end end + describe '#special_role=' do + let(:role) { Note::SpecialRole::FIRST_TIME_CONTRIBUTOR } + + it 'assigns role' do + subject.special_role = role + + expect(subject.special_role).to eq(role) + end + + it 'does not assign unknown role' do + expect { subject.special_role = :bogus }.to raise_error(/Role is undefined/) + + expect(subject.special_role).to be_nil + end + end + describe '#parent' do it 'returns project for project notes' do project = create(:project) diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb index 20278d81f6d..2ba53818e54 100644 --- a/spec/models/notification_recipient_spec.rb +++ b/spec/models/notification_recipient_spec.rb @@ -9,6 +9,38 @@ describe NotificationRecipient do subject(:recipient) { described_class.new(user, :watch, target: target, project: project) } + describe '#notifiable?' do + let(:recipient) { described_class.new(user, :mention, target: target, project: project) } + + context 'when emails are disabled' do + it 'returns false if group disabled' do + expect(project.namespace).to receive(:emails_disabled?).and_return(true) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq false + end + + it 'returns false if project disabled' do + expect(project).to receive(:emails_disabled?).and_return(true) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq false + end + end + + context 'when emails are enabled' do + it 'returns true if group enabled' do + expect(project.namespace).to receive(:emails_disabled?).and_return(false) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq true + end + + it 'returns true if project enabled' do + expect(project).to receive(:emails_disabled?).and_return(false) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq true + end + end + end + describe '#has_access?' do before do allow(user).to receive(:can?).and_call_original @@ -49,7 +81,7 @@ describe NotificationRecipient do end context '#notification_setting' do - context 'for child groups', :nested_groups do + context 'for child groups' do let!(:moved_group) { create(:group) } let(:group) { create(:group) } let(:sub_group_1) { create(:group, parent: group) } diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index fdc81359d34..519c519fbcf 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -53,24 +53,33 @@ describe PagesDomain do end let(:pages_domain) do - build(:pages_domain, certificate: certificate, key: key).tap do |pd| + build(:pages_domain, certificate: certificate, key: key, + auto_ssl_enabled: auto_ssl_enabled).tap do |pd| allow(pd).to receive(:project).and_return(project) pd.valid? end end - where(:pages_https_only, :certificate, :key, :errors_on) do + where(:pages_https_only, :certificate, :key, :auto_ssl_enabled, :errors_on) do attributes = attributes_for(:pages_domain) cert, key = attributes.fetch_values(:certificate, :key) - true | nil | nil | %i(certificate key) - true | cert | nil | %i(key) - true | nil | key | %i(certificate key) - true | cert | key | [] - false | nil | nil | [] - false | cert | nil | %i(key) - false | nil | key | %i(key) - false | cert | key | [] + true | nil | nil | false | %i(certificate key) + true | nil | nil | true | [] + true | cert | nil | false | %i(key) + true | cert | nil | true | %i(key) + true | nil | key | false | %i(certificate key) + true | nil | key | true | %i(key) + true | cert | key | false | [] + true | cert | key | true | [] + false | nil | nil | false | [] + false | nil | nil | true | [] + false | cert | nil | false | %i(key) + false | cert | nil | true | %i(key) + false | nil | key | false | %i(key) + false | nil | key | true | %i(key) + false | cert | key | false | [] + false | cert | key | true | [] end with_them do @@ -118,6 +127,30 @@ describe PagesDomain do it { is_expected.not_to be_valid } end + + context 'when certificate is expired' do + let(:domain) do + build(:pages_domain, :with_trusted_expired_chain) + end + + context 'when certificate is being changed' do + it "adds error to certificate" do + domain.valid? + + expect(domain.errors.keys).to contain_exactly(:key, :certificate) + end + end + + context 'when certificate is already saved' do + it "doesn't add error to certificate" do + domain.save(validate: false) + + domain.valid? + + expect(domain.errors.keys).to contain_exactly(:key) + end + end + end end describe 'validations' do @@ -356,6 +389,102 @@ describe PagesDomain do end end + describe '#user_provided_key' do + subject { domain.user_provided_key } + + context 'when certificate is provided by user' do + let(:domain) { create(:pages_domain) } + + it 'returns key' do + is_expected.to eq(domain.key) + end + end + + context 'when certificate is provided by gitlab' do + let(:domain) { create(:pages_domain, :letsencrypt) } + + it 'returns nil' do + is_expected.to be_nil + end + end + end + + describe '#user_provided_certificate' do + subject { domain.user_provided_certificate } + + context 'when certificate is provided by user' do + let(:domain) { create(:pages_domain) } + + it 'returns key' do + is_expected.to eq(domain.certificate) + end + end + + context 'when certificate is provided by gitlab' do + let(:domain) { create(:pages_domain, :letsencrypt) } + + it 'returns nil' do + is_expected.to be_nil + end + end + end + + shared_examples 'certificate setter' do |attribute, setter_name, old_certificate_source, new_certificate_source| + let(:domain) do + create(:pages_domain, certificate_source: old_certificate_source) + end + + let(:old_value) { domain.public_send(attribute) } + + subject { domain.public_send(setter_name, new_value) } + + context 'when value has been changed' do + let(:new_value) { 'new_value' } + + it "assignes new value to #{attribute}" do + expect do + subject + end.to change { domain.public_send(attribute) }.from(old_value).to('new_value') + end + + it 'changes certificate source' do + expect do + subject + end.to change { domain.certificate_source }.from(old_certificate_source).to(new_certificate_source) + end + end + + context 'when value has not been not changed' do + let(:new_value) { old_value } + + it 'does not change certificate source' do + expect do + subject + end.not_to change { domain.certificate_source }.from(old_certificate_source) + end + end + end + + describe '#user_provided_key=' do + include_examples('certificate setter', 'key', 'user_provided_key=', + 'gitlab_provided', 'user_provided') + end + + describe '#gitlab_provided_key=' do + include_examples('certificate setter', 'key', 'gitlab_provided_key=', + 'user_provided', 'gitlab_provided') + end + + describe '#user_provided_certificate=' do + include_examples('certificate setter', 'certificate', 'user_provided_certificate=', + 'gitlab_provided', 'user_provided') + end + + describe '#gitlab_provided_certificate=' do + include_examples('certificate setter', 'certificate', 'gitlab_provided_certificate=', + 'user_provided', 'gitlab_provided') + end + describe '.for_removal' do subject { described_class.for_removal } @@ -383,4 +512,30 @@ describe PagesDomain do end end end + + describe '.need_auto_ssl_renewal' do + subject { described_class.need_auto_ssl_renewal } + + let!(:domain_with_user_provided_certificate) { create(:pages_domain) } + let!(:domain_with_expired_user_provided_certificate) do + create(:pages_domain, :with_expired_certificate) + end + let!(:domain_with_user_provided_certificate_and_auto_ssl) do + create(:pages_domain, auto_ssl_enabled: true) + end + + let!(:domain_with_gitlab_provided_certificate) { create(:pages_domain, :letsencrypt) } + let!(:domain_with_expired_gitlab_provided_certificate) do + create(:pages_domain, :letsencrypt, :with_expired_certificate) + end + + it 'contains only domains needing verification' do + is_expected.to( + contain_exactly( + domain_with_user_provided_certificate_and_auto_ssl, + domain_with_expired_gitlab_provided_certificate + ) + ) + end + end end diff --git a/spec/models/postgresql/replication_slot_spec.rb b/spec/models/postgresql/replication_slot_spec.rb index e100af7ddc7..d435fccc09a 100644 --- a/spec/models/postgresql/replication_slot_spec.rb +++ b/spec/models/postgresql/replication_slot_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Postgresql::ReplicationSlot, :postgresql do +describe Postgresql::ReplicationSlot do describe '.in_use?' do it 'returns true when replication slots are present' do expect(described_class).to receive(:exists?).and_return(true) @@ -47,5 +47,13 @@ describe Postgresql::ReplicationSlot, :postgresql do expect(described_class.lag_too_great?).to eq(false) end + + it 'returns false when there is a nil replication lag' do + expect(described_class) + .to receive(:pluck) + .and_return([0.megabytes, nil]) + + expect(described_class.lag_too_great?).to eq(false) + end end end diff --git a/spec/models/project_auto_devops_spec.rb b/spec/models/project_auto_devops_spec.rb index 7bdd2367a68..da9e56ef897 100644 --- a/spec/models/project_auto_devops_spec.rb +++ b/spec/models/project_auto_devops_spec.rb @@ -15,7 +15,7 @@ describe ProjectAutoDevops do it { is_expected.to respond_to(:updated_at) } describe '#predefined_variables' do - let(:auto_devops) { build_stubbed(:project_auto_devops, project: project, domain: domain) } + let(:auto_devops) { build_stubbed(:project_auto_devops, project: project) } context 'when deploy_strategy is manual' do let(:auto_devops) { build_stubbed(:project_auto_devops, :manual_deployment, project: project) } diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index 50c9d5968ac..dc7a8433064 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -8,11 +8,7 @@ describe ProjectFeature do describe '.quoted_access_level_column' do it 'returns the table name and quoted column name for a feature' do - expected = if Gitlab::Database.postgresql? - '"project_features"."issues_access_level"' - else - '`project_features`.`issues_access_level`' - end + expected = '"project_features"."issues_access_level"' expect(described_class.quoted_access_level_column(:issues)).to eq(expected) end @@ -150,4 +146,32 @@ describe ProjectFeature do end end end + + describe 'default pages access level' do + subject { project.project_feature.pages_access_level } + + before do + # project factory overrides all values in project_feature after creation + project.project_feature.destroy! + project.build_project_feature.save! + end + + context 'when new project is private' do + let(:project) { create(:project, :private) } + + it { is_expected.to eq(ProjectFeature::PRIVATE) } + end + + context 'when new project is internal' do + let(:project) { create(:project, :internal) } + + it { is_expected.to eq(ProjectFeature::PRIVATE) } + end + + context 'when new project is public' do + let(:project) { create(:project, :public) } + + it { is_expected.to eq(ProjectFeature::ENABLED) } + end + end end diff --git a/spec/models/project_group_link_spec.rb b/spec/models/project_group_link_spec.rb index dad5506900b..cd997224122 100644 --- a/spec/models/project_group_link_spec.rb +++ b/spec/models/project_group_link_spec.rb @@ -25,7 +25,7 @@ describe ProjectGroupLink do expect(project_group_link).not_to be_valid end - it "doesn't allow a project to be shared with an ancestor of the group it is in", :nested_groups do + it "doesn't allow a project to be shared with an ancestor of the group it is in" do project_group_link.group = parent_group expect(project_group_link).not_to be_valid diff --git a/spec/models/project_services/bugzilla_service_spec.rb b/spec/models/project_services/bugzilla_service_spec.rb index 6818db48fee..74c85a13c88 100644 --- a/spec/models/project_services/bugzilla_service_spec.rb +++ b/spec/models/project_services/bugzilla_service_spec.rb @@ -32,4 +32,51 @@ describe BugzillaService do it { is_expected.not_to validate_presence_of(:new_issue_url) } end end + + context 'overriding properties' do + let(:default_title) { 'JIRA' } + let(:default_description) { 'JiraService|Jira issue tracker' } + let(:url) { 'http://bugzilla.example.com' } + let(:access_params) do + { project_url: url, issues_url: url, new_issue_url: url } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) do + create(:bugzilla_service, :without_properties_callback, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:bugzilla_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:bugzilla_service, :without_properties_callback, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:bugzilla_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('Bugzilla') + expect(service.description).to eq('Bugzilla issue tracker') + end + end + end end diff --git a/spec/models/project_services/chat_message/pipeline_message_spec.rb b/spec/models/project_services/chat_message/pipeline_message_spec.rb index 8f9fa310ad4..cf7c7bf7e61 100644 --- a/spec/models/project_services/chat_message/pipeline_message_spec.rb +++ b/spec/models/project_services/chat_message/pipeline_message_spec.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true - require 'spec_helper' describe ChatMessage::PipelineMessage do subject { described_class.new(args) } - let(:user) { { name: "The Hacker", username: 'hacker' } } - let(:duration) { 7210 } let(:args) do { object_attributes: { @@ -14,122 +11,437 @@ describe ChatMessage::PipelineMessage do sha: '97de212e80737a608d939f648d959671fb0a0142', tag: false, ref: 'develop', - status: status, - duration: duration + status: 'success', + detailed_status: nil, + duration: 7210, + finished_at: "2019-05-27 11:56:36 -0300" }, project: { - path_with_namespace: 'project_name', - web_url: 'http://example.gitlab.com' + id: 234, + name: "project_name", + path_with_namespace: 'group/project_name', + web_url: 'http://example.gitlab.com', + avatar_url: 'http://example.com/project_avatar' + }, + user: { + id: 345, + name: "The Hacker", + username: "hacker", + email: "hacker@example.gitlab.com", + avatar_url: "http://example.com/avatar" + }, + commit: { + id: "abcdef" }, - user: user + builds: nil, + markdown: false } end - let(:combined_name) { "The Hacker (hacker)" } - context 'without markdown' do - context 'pipeline succeeded' do - let(:status) { 'success' } - let(:color) { 'good' } - let(:message) { build_message('passed', combined_name) } + let(:has_yaml_errors) { false } + + 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]) + 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") + allow(Ci::Pipeline).to receive(:find) { test_pipeline } + + allow(Gitlab::UrlBuilder).to receive(:build).with(test_commit).and_return("http://example.com/commit") + allow(Gitlab::UrlBuilder).to receive(:build).with(args[:user]).and_return("http://example.gitlab.com/hacker") + end + + context 'when the fancy_pipeline_slack_notifications feature flag is disabled' do + before do + stub_feature_flags(fancy_pipeline_slack_notifications: false) + end + + it 'returns an empty pretext' do + expect(subject.pretext).to be_empty + end + + it "returns the pipeline summary in the activity's title" do + expect(subject.activity[:title]).to eq( + "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/commits/develop)" \ + " by The Hacker (hacker) passed" + ) + end - it 'returns a message with information about succeeded build' do - expect(subject.pretext).to be_empty - expect(subject.fallback).to eq(message) - expect(subject.attachments).to eq([text: message, color: color]) + context "when the pipeline failed" do + before do + args[:object_attributes][:status] = 'failed' + end + + it "returns the summary with a 'failed' status" do + expect(subject.activity[:title]).to eq( + "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/commits/develop)" \ + " by The Hacker (hacker) failed" + ) end end - context 'pipeline failed' do - let(:status) { 'failed' } - let(:color) { 'danger' } - let(:message) { build_message(status, combined_name) } + context 'when no user is provided because the pipeline was triggered by the API' do + before do + args[:user] = nil + end - it 'returns a message with information about failed build' do - expect(subject.pretext).to be_empty - expect(subject.fallback).to eq(message) - expect(subject.attachments).to eq([text: message, color: color]) + it "returns the summary with 'API' as the username" do + expect(subject.activity[:title]).to eq( + "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/commits/develop)" \ + " by API passed" + ) end + end - context 'when triggered by API therefore lacking user' do - let(:user) { nil } - let(:message) { build_message(status, 'API') } + it "returns a link to the project in the activity's subtitle" do + expect(subject.activity[:subtitle]).to eq("in [project_name](http://example.gitlab.com)") + end - it 'returns a message stating it is by API' do - expect(subject.pretext).to be_empty - expect(subject.fallback).to eq(message) - expect(subject.attachments).to eq([text: message, color: color]) - end + it "returns the build duration in the activity's text property" do + expect(subject.activity[:text]).to eq("in 02:00:10") + end + + it "returns the user's avatar image URL in the activity's image property" do + expect(subject.activity[:image]).to eq("http://example.com/avatar") + end + + context 'when the user does not have an avatar' do + before do + args[:user][:avatar_url] = nil + end + + it "returns an empty string in the activity's image property" do + expect(subject.activity[:image]).to be_empty + end + end + + it "returns the pipeline summary as the attachment's text property" do + expect(subject.attachments.first[:text]).to eq( + "<http://example.gitlab.com|project_name>:" \ + " Pipeline <http://example.gitlab.com/pipelines/123|#123>" \ + " of branch <http://example.gitlab.com/commits/develop|develop>" \ + " by The Hacker (hacker) passed in 02:00:10" + ) + end + + it "returns 'good' as the attachment's color property" do + expect(subject.attachments.first[:color]).to eq('good') + end + + context "when the pipeline failed" do + before do + args[:object_attributes][:status] = 'failed' + end + + it "returns 'danger' as the attachment's color property" do + expect(subject.attachments.first[:color]).to eq('danger') end end - def build_message(status_text = status, name = user[:name]) - "<http://example.gitlab.com|project_name>:" \ - " Pipeline <http://example.gitlab.com/pipelines/123|#123>" \ - " of branch <http://example.gitlab.com/commits/develop|develop>" \ - " by #{name} #{status_text} in 02:00:10" + context 'when rendering markdown' do + before do + args[:markdown] = true + end + + it 'returns the pipeline summary as the attachments in markdown format' do + expect(subject.attachments).to eq( + "[project_name](http://example.gitlab.com):" \ + " Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/commits/develop)" \ + " by The Hacker (hacker) passed in 02:00:10" + ) + end end end - context 'with markdown' do + context 'when the fancy_pipeline_slack_notifications feature flag is enabled' do before do - args[:markdown] = true - end - - context 'pipeline succeeded' do - let(:status) { 'success' } - let(:color) { 'good' } - let(:message) { build_markdown_message('passed', combined_name) } - - it 'returns a message with information about succeeded build' do - expect(subject.pretext).to be_empty - expect(subject.attachments).to eq(message) - expect(subject.activity).to eq({ - title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of branch [develop](http://example.gitlab.com/commits/develop) by The Hacker (hacker) passed', - subtitle: 'in [project_name](http://example.gitlab.com)', - text: 'in 02:00:10', - image: '' - }) + stub_feature_flags(fancy_pipeline_slack_notifications: true) + end + + it 'returns an empty pretext' do + expect(subject.pretext).to be_empty + end + + it "returns the pipeline summary in the activity's title" do + expect(subject.activity[:title]).to eq( + "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/commits/develop)" \ + " by The Hacker (hacker) has passed" + ) + end + + context "when the pipeline failed" do + before do + args[:object_attributes][:status] = 'failed' + end + + it "returns the summary with a 'failed' status" do + expect(subject.activity[:title]).to eq( + "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/commits/develop)" \ + " by The Hacker (hacker) has failed" + ) + end + end + + context "when the pipeline passed with warnings" do + before do + args[:object_attributes][:detailed_status] = 'passed with warnings' + end + + it "returns the summary with a 'passed with warnings' status" do + expect(subject.activity[:title]).to eq( + "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/commits/develop)" \ + " by The Hacker (hacker) has passed with warnings" + ) + end + end + + context 'when no user is provided because the pipeline was triggered by the API' do + before do + args[:user] = nil + end + + it "returns the summary with 'API' as the username" do + expect(subject.activity[:title]).to eq( + "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/commits/develop)" \ + " by API has passed" + ) + end + end + + it "returns a link to the project in the activity's subtitle" do + expect(subject.activity[:subtitle]).to eq("in [project_name](http://example.gitlab.com)") + end + + it "returns the build duration in the activity's text property" do + expect(subject.activity[:text]).to eq("in 02:00:10") + end + + it "returns the user's avatar image URL in the activity's image property" do + expect(subject.activity[:image]).to eq("http://example.com/avatar") + end + + context 'when the user does not have an avatar' do + before do + args[:user][:avatar_url] = nil + end + + it "returns an empty string in the activity's image property" do + expect(subject.activity[:image]).to be_empty + end + end + + it "returns the pipeline summary as the attachment's fallback property" do + expect(subject.attachments.first[:fallback]).to eq( + "<http://example.gitlab.com|project_name>:" \ + " Pipeline <http://example.gitlab.com/pipelines/123|#123>" \ + " of branch <http://example.gitlab.com/commits/develop|develop>" \ + " by The Hacker (hacker) has passed in 02:00:10" + ) + end + + it "returns 'good' as the attachment's color property" do + expect(subject.attachments.first[:color]).to eq('good') + end + + context "when the pipeline failed" do + before do + args[:object_attributes][:status] = 'failed' + end + + it "returns 'danger' as the attachment's color property" do + expect(subject.attachments.first[:color]).to eq('danger') + end + end + + context "when the pipeline passed with warnings" do + before do + args[:object_attributes][:detailed_status] = 'passed with warnings' + end + + it "returns 'warning' as the attachment's color property" do + expect(subject.attachments.first[:color]).to eq('warning') end end - context 'pipeline failed' do - let(:status) { 'failed' } - let(:color) { 'danger' } - let(:message) { build_markdown_message(status, combined_name) } + it "returns the committer's name and username as the attachment's author_name property" do + expect(subject.attachments.first[:author_name]).to eq('The Hacker (hacker)') + end + + it "returns the committer's avatar URL as the attachment's author_icon property" do + expect(subject.attachments.first[:author_icon]).to eq('http://example.com/avatar') + end + + it "returns the committer's GitLab profile URL as the attachment's author_link property" do + expect(subject.attachments.first[:author_link]).to eq('http://example.gitlab.com/hacker') + end + + context 'when no user is provided because the pipeline was triggered by the API' do + before do + args[:user] = nil + end + + it "returns the committer's name and username as the attachment's author_name property" do + expect(subject.attachments.first[:author_name]).to eq('API') + end + + it "returns nil as the attachment's author_icon property" do + expect(subject.attachments.first[:author_icon]).to be_nil + end + + it "returns nil as the attachment's author_link property" do + expect(subject.attachments.first[:author_link]).to be_nil + end + end + + it "returns the pipeline ID, status, and duration as the attachment's title property" do + expect(subject.attachments.first[:title]).to eq("Pipeline #123 has passed in 02:00:10") + end + + it "returns the pipeline URL as the attachment's title_link property" 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) + end + + it "returns the commit message as the attachment's second field property" do + expect(subject.attachments.first[:fields][0]).to eq({ + title: "Branch", + value: "<http://example.gitlab.com/commits/develop|develop>", + short: true + }) + end + + it "returns the ref name and link as the attachment's second field property" do + expect(subject.attachments.first[:fields][1]).to eq({ + title: "Commit", + value: "<http://example.com/commit|A test commit message>", + short: true + }) + end + + context "when a job in the pipeline fails" do + before do + args[:builds] = [ + { id: 1, name: "rspec", status: "failed", stage: "test" }, + { id: 2, name: "karma", status: "success", stage: "test" } + ] + end + + it "returns four attachment fields" do + expect(subject.attachments.first[:fields].count).to eq(4) + end - it 'returns a message with information about failed build' do - expect(subject.pretext).to be_empty - expect(subject.attachments).to eq(message) - expect(subject.activity).to eq({ - title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of branch [develop](http://example.gitlab.com/commits/develop) by The Hacker (hacker) failed', - subtitle: 'in [project_name](http://example.gitlab.com)', - text: 'in 02:00:10', - image: '' + 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 + expect(subject.attachments.first[:fields][2]).to eq({ + title: "Failed stage", + value: "<http://example.gitlab.com/pipelines/123/failures|test>", + short: true }) end - context 'when triggered by API therefore lacking user' do - let(:user) { nil } - let(:message) { build_markdown_message(status, 'API') } + it "returns the job name and link as the attachment's fourth field property" do + expect(subject.attachments.first[:fields][3]).to eq({ + title: "Failed job", + value: "<http://example.gitlab.com/-/jobs/1|rspec>", + short: true + }) + end + end + + context "when lots of jobs across multiple stages fail" do + before do + args[:builds] = (1..25).map do |i| + { id: i, name: "job-#{i}", status: "failed", stage: "stage-" + ((i % 3) + 1).to_s } + end + end + + it "returns the stage names and links to the 'Failed jobs' tab on the pipeline's page as the attachment's third field property" do + expect(subject.attachments.first[:fields][2]).to eq({ + title: "Failed stages", + value: "<http://example.gitlab.com/pipelines/123/failures|stage-2>, <http://example.gitlab.com/pipelines/123/failures|stage-1>, <http://example.gitlab.com/pipelines/123/failures|stage-3>", + short: true + }) + end - it 'returns a message stating it is by API' do - expect(subject.pretext).to be_empty - expect(subject.attachments).to eq(message) - expect(subject.activity).to eq({ - title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of branch [develop](http://example.gitlab.com/commits/develop) by API failed', - subtitle: 'in [project_name](http://example.gitlab.com)', - text: 'in 02:00:10', - image: '' - }) + it "returns the job names and links as the attachment's fourth field property" do + expected_jobs = 25.downto(16).map do |i| + "<http://example.gitlab.com/-/jobs/#{i}|job-#{i}>" end + + expected_jobs << "and <http://example.gitlab.com/pipelines/123/failures|15 more>" + + expect(subject.attachments.first[:fields][3]).to eq({ + title: "Failed jobs", + value: expected_jobs.join(", "), + short: true + }) end end - def build_markdown_message(status_text = status, name = user[:name]) - "[project_name](http://example.gitlab.com):" \ - " Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ - " of branch [develop](http://example.gitlab.com/commits/develop)" \ - " by #{name} #{status_text} in 02:00:10" + 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) + end + + it "returns the YAML error deatils as the attachment's third field property" do + expect(subject.attachments.first[:fields][2]).to eq({ + title: "Invalid CI config YAML file", + value: "yaml error description here", + short: false + }) + end + end + + it "returns the stage name and link as the attachment's second field property" do + expect(subject.attachments.first[:fields][1]).to eq({ + title: "Commit", + value: "<http://example.com/commit|A test commit message>", + short: true + }) + end + + it "returns the project's name as the attachment's footer property" do + expect(subject.attachments.first[:footer]).to eq("project_name") + end + + it "returns the project's avatar URL as the attachment's footer_icon property" do + expect(subject.attachments.first[:footer_icon]).to eq("http://example.com/project_avatar") + end + + it "returns the pipeline's timestamp as the attachment's ts property" do + expected_ts = Time.parse(args[:object_attributes][:finished_at]).to_i + expect(subject.attachments.first[:ts]).to eq(expected_ts) + end + + context 'when rendering markdown' do + before do + args[:markdown] = true + end + + it 'returns the pipeline summary as the attachments in markdown format' do + expect(subject.attachments).to eq( + "[project_name](http://example.gitlab.com):" \ + " Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/commits/develop)" \ + " by The Hacker (hacker) has passed in 02:00:10" + ) + end end end end diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/project_services/custom_issue_tracker_service_spec.rb index f0e7551693d..5259357a254 100644 --- a/spec/models/project_services/custom_issue_tracker_service_spec.rb +++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb @@ -48,4 +48,49 @@ describe CustomIssueTrackerService do end end end + + context 'overriding properties' do + let(:url) { 'http://custom.example.com' } + let(:access_params) do + { project_url: url, issues_url: url, new_issue_url: url } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) do + create(:custom_issue_tracker_service, :without_properties_callback, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:custom_issue_tracker_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:custom_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:custom_issue_tracker_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('Custom Issue Tracker') + expect(service.description).to eq('Custom issue tracker') + end + end + end end diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/project_services/drone_ci_service_spec.rb index 22df19d943f..a771d1bf27f 100644 --- a/spec/models/project_services/drone_ci_service_spec.rb +++ b/spec/models/project_services/drone_ci_service_spec.rb @@ -101,6 +101,15 @@ describe DroneCiService, :use_clean_rails_memory_store_caching do is_expected.to eq(:error) end + Gitlab::HTTP::HTTP_ERRORS.each do |http_error| + it "sets commit status to :error with a #{http_error.name} error" do + WebMock.stub_request(:get, commit_status_path) + .to_raise(http_error) + + is_expected.to eq(:error) + end + end + { "killed" => :canceled, "failure" => :failed, diff --git a/spec/models/project_services/emails_on_push_service_spec.rb b/spec/models/project_services/emails_on_push_service_spec.rb index 0a58eb367e3..ffe241aa880 100644 --- a/spec/models/project_services/emails_on_push_service_spec.rb +++ b/spec/models/project_services/emails_on_push_service_spec.rb @@ -20,4 +20,24 @@ describe EmailsOnPushService do it { is_expected.not_to validate_presence_of(:recipients) } end end + + context 'project emails' do + let(:push_data) { { object_kind: 'push' } } + let(:project) { create(:project, :repository) } + let(:service) { create(:emails_on_push_service, project: project) } + + it 'does not send emails when disabled' do + expect(project).to receive(:emails_disabled?).and_return(true) + expect(EmailsOnPushWorker).not_to receive(:perform_async) + + service.execute(push_data) + end + + it 'does send emails when enabled' do + expect(project).to receive(:emails_disabled?).and_return(false) + expect(EmailsOnPushWorker).to receive(:perform_async) + + service.execute(push_data) + end + end end diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb index 11f96c03d46..0c4fc290a13 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -51,4 +51,49 @@ describe GitlabIssueTrackerService do end end end + + context 'overriding properties' do + let(:url) { 'http://gitlab.example.com' } + let(:access_params) do + { project_url: url, issues_url: url, new_issue_url: url } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) do + create(:gitlab_issue_tracker_service, :without_properties_callback, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:gitlab_issue_tracker_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:gitlab_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:gitlab_issue_tracker_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('GitLab') + expect(service.description).to eq('GitLab issue tracker') + end + end + end end diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index a04b984c1f6..a1bd0855708 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -301,7 +301,7 @@ describe HipchatService do end context 'pipeline events' do - let(:pipeline) { create(:ci_empty_pipeline, user: create(:user)) } + let(:pipeline) { create(:ci_empty_pipeline, user: project.owner) } let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } context 'for failed' do diff --git a/spec/models/project_services/issue_tracker_data_spec.rb b/spec/models/project_services/issue_tracker_data_spec.rb new file mode 100644 index 00000000000..aaab654f874 --- /dev/null +++ b/spec/models/project_services/issue_tracker_data_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe IssueTrackerData do + let(:service) { create(:custom_issue_tracker_service, active: false, properties: {}) } + + describe 'Associations' do + it { is_expected.to belong_to :service } + end + + describe 'Validations' do + subject { described_class.new(service: service) } + + context 'url validations' do + context 'when service is inactive' do + it { is_expected.not_to validate_presence_of(:project_url) } + it { is_expected.not_to validate_presence_of(:issues_url) } + end + + context 'when service is active' do + before do + service.update(active: true) + end + + it_behaves_like 'issue tracker service URL attribute', :project_url + it_behaves_like 'issue tracker service URL attribute', :issues_url + it_behaves_like 'issue tracker service URL attribute', :new_issue_url + + it { is_expected.to validate_presence_of(:project_url) } + it { is_expected.to validate_presence_of(:issues_url) } + end + end + end +end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 04ae9390436..02060699e9a 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -115,6 +115,72 @@ describe JiraService do end end + describe '#create' do + let(:params) do + { + project: create(:project), title: 'custom title', description: 'custom description' + } + end + + subject { described_class.create(params) } + + it 'does not store title & description into properties' do + expect(subject.properties.keys).not_to include('title', 'description') + end + + it 'sets title & description correctly' do + service = subject + + expect(service.title).to eq('custom title') + expect(service.description).to eq('custom description') + end + end + + context 'overriding properties' do + let(:url) { 'http://issue_tracker.example.com' } + let(:access_params) do + { url: url, username: 'username', password: 'password' } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) do + create(:jira_service, :without_properties_callback, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:jira_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:jira_service, :without_properties_callback, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:jira_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('Jira') + expect(service.description).to eq('Jira issue tracker') + end + end + end + describe '#close_issue' do let(:custom_base_url) { 'http://custom_url' } let(:user) { create(:user) } @@ -158,7 +224,7 @@ describe JiraService do WebMock.stub_request(:post, @remote_link_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)) end - it 'calls JIRA API' do + it 'calls Jira API' do @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) expect(WebMock).to have_requested(:post, @comment_url).with( @@ -170,19 +236,19 @@ describe JiraService do allow(JIRA::Resource::Remotelink).to receive(:all).and_return(nil) expect { @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) } - .not_to raise_error(NoMethodError) + .not_to raise_error end # Check https://developer.atlassian.com/jiradev/jira-platform/guides/other/guide-jira-remote-issue-links/fields-in-remote-issue-links # for more information - it 'creates Remote Link reference in JIRA for comment' do + it 'creates Remote Link reference in Jira for comment' do @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) favicon_path = "http://localhost/assets/#{find_asset('favicon.png').digest_path}" # Creates comment expect(WebMock).to have_requested(:post, @comment_url) - # Creates Remote Link in JIRA issue fields + # Creates Remote Link in Jira issue fields expect(WebMock).to have_requested(:post, @remote_link_url).with( body: hash_including( GlobalID: 'GitLab', @@ -319,7 +385,7 @@ describe JiraService do end context 'when the test succeeds' do - it 'tries to get JIRA project with URL when API URL not set' do + it 'tries to get Jira project with URL when API URL not set' do test_settings('jira.example.com') end @@ -327,7 +393,7 @@ describe JiraService do expect(test_settings).to eq( { success: true, result: { 'url' => 'http://url' } }) end - it 'tries to get JIRA project with API URL if set' do + it 'tries to get Jira project with API URL if set' do jira_service.update(api_url: 'http://jira.api.com') test_settings('jira.api.com') end @@ -450,36 +516,54 @@ describe JiraService do end describe 'description and title' do - let(:project) { create(:project) } + let(:title) { 'Jira One' } + let(:description) { 'Jira One issue tracker' } + let(:properties) do + { + url: 'http://jira.example.com/web', + username: 'mic', + password: 'password', + title: title, + description: description + } + end context 'when it is not set' do - before do - @service = project.create_jira_service(active: true) - end + it 'default values are returned' do + service = create(:jira_service) - after do - @service.destroy! + expect(service.title).to eq('Jira') + expect(service.description).to eq('Jira issue tracker') end + end - it 'is initialized' do - expect(@service.title).to eq('JIRA') - expect(@service.description).to eq('Jira issue tracker') + context 'when it is set in properties' do + it 'values from properties are returned' do + service = create(:jira_service, properties: properties) + + expect(service.title).to eq(title) + expect(service.description).to eq(description) end end - context 'when it is set' do - before do - properties = { 'title' => 'Jira One', 'description' => 'Jira One issue tracker' } - @service = project.create_jira_service(active: true, properties: properties) - end + context 'when it is in title & description fields' do + it 'values from title and description fields are returned' do + service = create(:jira_service, title: title, description: description) - after do - @service.destroy! + expect(service.title).to eq(title) + expect(service.description).to eq(description) end + end + + context 'when it is in both properites & title & description fields' do + it 'values from title and description fields are returned' do + title2 = 'Jira 2' + description2 = 'Jira description 2' - it 'is correct' do - expect(@service.title).to eq('Jira One') - expect(@service.description).to eq('Jira One issue tracker') + service = create(:jira_service, title: title2, description: description2, properties: properties) + + expect(service.title).to eq(title2) + expect(service.description).to eq(description2) end end end @@ -505,31 +589,29 @@ describe JiraService do end describe 'project and issue urls' do - let(:project) { create(:project) } - context 'when gitlab.yml was initialized' do - before do + it 'is prepopulated with the settings' do settings = { 'jira' => { - 'title' => 'Jira', 'url' => 'http://jira.sample/projects/project_a', 'api_url' => 'http://jira.sample/api' } } allow(Gitlab.config).to receive(:issues_tracker).and_return(settings) - @service = project.create_jira_service(active: true) - end - after do - @service.destroy! - end + project = create(:project) + service = project.create_jira_service(active: true) - it 'is prepopulated with the settings' do - expect(@service.properties['title']).to eq('Jira') - expect(@service.properties['url']).to eq('http://jira.sample/projects/project_a') - expect(@service.properties['api_url']).to eq('http://jira.sample/api') + expect(service.properties['url']).to eq('http://jira.sample/projects/project_a') + expect(service.properties['api_url']).to eq('http://jira.sample/api') end end + + it 'removes trailing slashes from url' do + service = described_class.new(url: 'http://jira.test.com/path/') + + expect(service.url).to eq('http://jira.test.com/path') + end end describe 'favicon urls', :request_store do @@ -545,4 +627,20 @@ describe JiraService do expect(props[:object][:icon][:url16x16]).to match %r{^http://localhost/uploads/-/system/appearance/favicon/\d+/dk.png$} end end + + context 'generating external URLs' do + let(:service) { described_class.new(url: 'http://jira.test.com/path/') } + + describe '#issues_url' do + it 'handles trailing slashes' do + expect(service.issues_url).to eq('http://jira.test.com/path/browse/:id') + end + end + + describe '#new_issue_url' do + it 'handles trailing slashes' do + expect(service.new_issue_url).to eq('http://jira.test.com/path/secure/CreateIssue.jspa') + end + end + end end diff --git a/spec/models/project_services/jira_tracker_data_spec.rb b/spec/models/project_services/jira_tracker_data_spec.rb new file mode 100644 index 00000000000..1b6ece8531b --- /dev/null +++ b/spec/models/project_services/jira_tracker_data_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe JiraTrackerData do + let(:service) { create(:jira_service, active: false, properties: {}) } + + describe 'Associations' do + it { is_expected.to belong_to(:service) } + end + + describe 'Validations' do + subject { described_class.new(service: service) } + + context 'jira_issue_transition_id' do + it { is_expected.to allow_value(nil).for(:jira_issue_transition_id) } + it { is_expected.to allow_value('1,2,3').for(:jira_issue_transition_id) } + it { is_expected.to allow_value('1;2;3').for(:jira_issue_transition_id) } + it { is_expected.not_to allow_value('a,b,cd').for(:jira_issue_transition_id) } + end + + context 'url validations' do + context 'when service is inactive' do + it { is_expected.not_to validate_presence_of(:url) } + it { is_expected.not_to validate_presence_of(:username) } + it { is_expected.not_to validate_presence_of(:password) } + end + + context 'when service is active' do + before do + service.update(active: true) + end + + it_behaves_like 'issue tracker service URL attribute', :url + + it { is_expected.to validate_presence_of(:url) } + it { is_expected.to validate_presence_of(:username) } + it { is_expected.to validate_presence_of(:password) } + end + end + end +end diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb deleted file mode 100644 index 2fce120381b..00000000000 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ /dev/null @@ -1,430 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe KubernetesService, :use_clean_rails_memory_store_caching do - include KubernetesHelpers - include ReactiveCachingHelpers - - let(:project) { create(:kubernetes_project) } - let(:service) { project.deployment_platform } - - describe 'Associations' do - it { is_expected.to belong_to :project } - end - - describe 'Validations' do - context 'when service is active' do - before do - subject.active = true - end - - it { is_expected.not_to validate_presence_of(:namespace) } - it { is_expected.to validate_presence_of(:api_url) } - it { is_expected.to validate_presence_of(:token) } - - context 'namespace format' do - before do - subject.project = project - subject.api_url = "http://example.com" - subject.token = "test" - end - - { - 'foo' => true, - '1foo' => true, - 'foo1' => true, - 'foo-bar' => true, - '-foo' => false, - 'foo-' => false, - 'a' * 63 => true, - 'a' * 64 => false, - 'a.b' => false, - 'a*b' => false, - 'FOO' => true - }.each do |namespace, validity| - it "validates #{namespace} as #{validity ? 'valid' : 'invalid'}" do - subject.namespace = namespace - - expect(subject.valid?).to eq(validity) - end - end - end - end - - context 'when service is inactive' do - before do - subject.project = project - subject.active = false - end - - it { is_expected.not_to validate_presence_of(:api_url) } - it { is_expected.not_to validate_presence_of(:token) } - end - - context 'with a deprecated service' do - let(:kubernetes_service) { create(:kubernetes_service) } - - before do - kubernetes_service.update_attribute(:active, false) - kubernetes_service.properties['namespace'] = "foo" - end - - it 'does not update attributes' do - expect(kubernetes_service.save).to be_falsy - end - - it 'includes an error with a deprecation message' do - kubernetes_service.valid? - expect(kubernetes_service.errors[:base].first).to match(/Kubernetes service integration has been deprecated/) - end - end - - context 'with a non-deprecated service' do - let(:kubernetes_service) { create(:kubernetes_service) } - - it 'updates attributes' do - kubernetes_service.properties['namespace'] = 'foo' - expect(kubernetes_service.save).to be_truthy - end - end - - context 'with an active and deprecated service' do - let(:kubernetes_service) { create(:kubernetes_service) } - - before do - kubernetes_service.active = false - kubernetes_service.properties['namespace'] = 'foo' - kubernetes_service.save - end - - it 'deactivates the service' do - expect(kubernetes_service.active?).to be_falsy - end - - it 'does not include a deprecation message as error' do - expect(kubernetes_service.errors.messages.count).to eq(0) - end - - it 'updates attributes' do - expect(kubernetes_service.properties['namespace']).to eq("foo") - end - end - - context 'with a template service' do - let(:kubernetes_service) { create(:kubernetes_service, template: true, active: false) } - - before do - kubernetes_service.properties['namespace'] = 'foo' - end - - it 'updates attributes' do - expect(kubernetes_service.save).to be_truthy - expect(kubernetes_service.properties['namespace']).to eq('foo') - end - end - end - - describe '#initialize_properties' do - context 'without a project' do - it 'leaves the namespace unset' do - expect(described_class.new.namespace).to be_nil - end - end - end - - describe '#fields' do - let(:kube_namespace) do - subject.fields.find { |h| h[:name] == 'namespace' } - end - - context 'as template' do - before do - subject.template = true - end - - it 'sets the namespace to the default' do - expect(kube_namespace).not_to be_nil - expect(kube_namespace[:placeholder]).to eq(subject.class::TEMPLATE_PLACEHOLDER) - end - end - - context 'with associated project' do - before do - subject.project = project - end - - it 'sets the namespace to the default' do - expect(kube_namespace).not_to be_nil - expect(kube_namespace[:placeholder]).to match(/\A#{Gitlab::PathRegex::PATH_REGEX_STR}-\d+\z/) - end - end - end - - describe '#kubernetes_namespace_for' do - subject { service.kubernetes_namespace_for(project) } - - shared_examples 'a correctly formatted namespace' do - it 'returns a valid Kubernetes namespace name' do - expect(subject).to match(Gitlab::Regex.kubernetes_namespace_regex) - expect(subject).to eq(expected_namespace) - end - end - - it_behaves_like 'a correctly formatted namespace' do - let(:expected_namespace) { service.send(:default_namespace) } - end - - context 'when the project path contains forbidden characters' do - before do - project.path = '-a_Strange.Path--forSure' - end - - it_behaves_like 'a correctly formatted namespace' do - let(:expected_namespace) { "a-strange-path--forsure-#{project.id}" } - end - end - - context 'when namespace is specified' do - before do - service.namespace = 'my-namespace' - end - - it_behaves_like 'a correctly formatted namespace' do - let(:expected_namespace) { 'my-namespace' } - end - end - - context 'when service is not assigned to project' do - before do - service.project = nil - end - - it 'does not return namespace' do - is_expected.to be_nil - end - end - end - - describe '#test' do - let(:discovery_url) { 'https://kubernetes.example.com/api/v1' } - - before do - stub_kubeclient_discover(service.api_url) - end - - context 'with path prefix in api_url' do - let(:discovery_url) { 'https://kubernetes.example.com/prefix/api/v1' } - - it 'tests with the prefix' do - service.api_url = 'https://kubernetes.example.com/prefix' - stub_kubeclient_discover(service.api_url) - - expect(service.test[:success]).to be_truthy - expect(WebMock).to have_requested(:get, discovery_url).once - end - end - - context 'with custom CA certificate' do - it 'is added to the certificate store' do - service.ca_pem = "CA PEM DATA" - - cert = double("certificate") - expect(OpenSSL::X509::Certificate).to receive(:new).with(service.ca_pem).and_return(cert) - expect_any_instance_of(OpenSSL::X509::Store).to receive(:add_cert).with(cert) - - expect(service.test[:success]).to be_truthy - expect(WebMock).to have_requested(:get, discovery_url).once - end - end - - context 'success' do - it 'reads the discovery endpoint' do - expect(service.test[:success]).to be_truthy - expect(WebMock).to have_requested(:get, discovery_url).once - end - end - - context 'failure' do - it 'fails to read the discovery endpoint' do - WebMock.stub_request(:get, service.api_url + '/api/v1').to_return(status: 404) - - expect(service.test[:success]).to be_falsy - expect(WebMock).to have_requested(:get, discovery_url).once - end - end - end - - describe '#predefined_variable' do - let(:kubeconfig) do - config_file = expand_fixture_path('config/kubeconfig.yml') - config = YAML.load(File.read(config_file)) - config.dig('users', 0, 'user')['token'] = 'token' - config.dig('contexts', 0, 'context')['namespace'] = namespace - config.dig('clusters', 0, 'cluster')['certificate-authority-data'] = - Base64.strict_encode64('CA PEM DATA') - - YAML.dump(config) - end - - before do - subject.api_url = 'https://kube.domain.com' - subject.token = 'token' - subject.ca_pem = 'CA PEM DATA' - subject.project = project - end - - shared_examples 'setting variables' do - it 'sets the variables' do - expect(subject.predefined_variables(project: project)).to include( - { key: 'KUBE_URL', value: 'https://kube.domain.com', public: true }, - { key: 'KUBE_TOKEN', value: 'token', public: false, masked: true }, - { key: 'KUBE_NAMESPACE', value: namespace, public: true }, - { key: 'KUBECONFIG', value: kubeconfig, public: false, file: true }, - { key: 'KUBE_CA_PEM', value: 'CA PEM DATA', public: true }, - { key: 'KUBE_CA_PEM_FILE', value: 'CA PEM DATA', public: true, file: true } - ) - end - end - - context 'namespace is provided' do - let(:namespace) { 'my-project' } - - before do - subject.namespace = namespace - end - - it_behaves_like 'setting variables' - end - - context 'no namespace provided' do - let(:namespace) { subject.kubernetes_namespace_for(project) } - - it_behaves_like 'setting variables' - - it 'sets the KUBE_NAMESPACE' do - kube_namespace = subject.predefined_variables(project: project).find { |h| h[:key] == 'KUBE_NAMESPACE' } - - expect(kube_namespace).not_to be_nil - expect(kube_namespace[:value]).to match(/\A#{Gitlab::PathRegex::PATH_REGEX_STR}-\d+\z/) - end - end - end - - describe '#terminals' do - let(:environment) { build(:environment, project: project, name: "env", slug: "env-000000") } - - subject { service.terminals(environment) } - - context 'with invalid pods' do - it 'returns no terminals' do - stub_reactive_cache(service, pods: [{ "bad" => "pod" }]) - - is_expected.to be_empty - end - end - - context 'with valid pods' do - let(:pod) { kube_pod(environment_slug: environment.slug, namespace: service.kubernetes_namespace_for(project), project_slug: project.full_path_slug) } - let(:pod_with_no_terminal) { kube_pod(environment_slug: environment.slug, project_slug: project.full_path_slug, status: "Pending") } - let(:terminals) { kube_terminals(service, pod) } - - before do - stub_reactive_cache( - service, - pods: [pod, pod, pod_with_no_terminal, kube_pod(environment_slug: "should-be-filtered-out")] - ) - end - - it 'returns terminals' do - is_expected.to eq(terminals + terminals) - end - - it 'uses max session time from settings' do - stub_application_setting(terminal_max_session_time: 600) - - times = subject.map { |terminal| terminal[:max_session_time] } - expect(times).to eq [600, 600, 600, 600] - end - end - end - - describe '#calculate_reactive_cache' do - subject { service.calculate_reactive_cache } - - let(:namespace) { service.kubernetes_namespace_for(project) } - - context 'when service is inactive' do - before do - service.active = false - end - - it { is_expected.to be_nil } - end - - context 'when kubernetes responds with valid pods' do - before do - stub_kubeclient_pods(namespace) - stub_kubeclient_deployments(namespace) # Used by EE - end - - it { is_expected.to include(pods: [kube_pod]) } - end - - context 'when kubernetes responds with 500s' do - before do - stub_kubeclient_pods(namespace, status: 500) - stub_kubeclient_deployments(namespace, status: 500) # Used by EE - end - - it { expect { subject }.to raise_error(Kubeclient::HttpError) } - end - - context 'when kubernetes responds with 404s' do - before do - stub_kubeclient_pods(namespace, status: 404) - stub_kubeclient_deployments(namespace, status: 404) # Used by EE - end - - it { is_expected.to include(pods: []) } - end - end - - describe "#deprecated?" do - let(:kubernetes_service) { create(:kubernetes_service) } - - context 'with an active kubernetes service' do - it 'returns false' do - expect(kubernetes_service.deprecated?).to be_falsy - end - end - - context 'with a inactive kubernetes service' do - it 'returns true' do - kubernetes_service.update_attribute(:active, false) - expect(kubernetes_service.deprecated?).to be_truthy - end - end - end - - describe "#deprecation_message" do - let(:kubernetes_service) { create(:kubernetes_service) } - - it 'indicates the service is deprecated' do - expect(kubernetes_service.deprecation_message).to match(/Kubernetes service integration has been deprecated/) - end - - context 'if the services is active' do - it 'returns a message' do - expect(kubernetes_service.deprecation_message).to match(/Your Kubernetes cluster information on this page is still editable/) - end - end - - context 'if the service is not active' do - it 'returns a message' do - kubernetes_service.update_attribute(:active, false) - expect(kubernetes_service.deprecation_message).to match(/Fields on this page are now uneditable/) - end - end - end -end diff --git a/spec/models/project_services/microsoft_teams_service_spec.rb b/spec/models/project_services/microsoft_teams_service_spec.rb index c025d7c882e..73c20359091 100644 --- a/spec/models/project_services/microsoft_teams_service_spec.rb +++ b/spec/models/project_services/microsoft_teams_service_spec.rb @@ -289,6 +289,19 @@ describe MicrosoftTeamsService do expect(result).to be_falsy end end + + context 'when disabled' do + let(:pipeline) do + create(:ci_pipeline, :failed, project: project, + sha: project.commit.sha, ref: 'not-the-default-branch') + end + + before do + chat_service.notify_only_default_branch = false + end + + it_behaves_like 'call Microsoft Teams API' + end end end end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index e9c7c94ad70..e5ac6ca65d6 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -105,10 +105,6 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do context 'manual configuration is enabled' do let(:manual_configuration) { true } - it 'returns rest client from api_url' do - expect(service.prometheus_client.url).to eq(api_url) - end - it 'calls valid?' do allow(service).to receive(:valid?).and_call_original diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb index ac570ac27e1..c1ee6546b12 100644 --- a/spec/models/project_services/redmine_service_spec.rb +++ b/spec/models/project_services/redmine_service_spec.rb @@ -40,4 +40,49 @@ describe RedmineService do expect(described_class.reference_pattern.match('#123')[:issue]).to eq('123') end end + + context 'overriding properties' do + let(:url) { 'http://redmine.example.com' } + let(:access_params) do + { project_url: url, issues_url: url, new_issue_url: url } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) do + create(:redmine_service, :without_properties_callback, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:redmine_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:redmine_service, :without_properties_callback, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:redmine_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('Redmine') + expect(service.description).to eq('Redmine issue tracker') + end + end + end end diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb index 1c434b25205..3d875bc49e7 100644 --- a/spec/models/project_services/teamcity_service_spec.rb +++ b/spec/models/project_services/teamcity_service_spec.rb @@ -7,10 +7,11 @@ describe TeamcityService, :use_clean_rails_memory_store_caching do include StubRequests let(:teamcity_url) { 'http://gitlab.com/teamcity' } + let(:project) { create(:project) } subject(:service) do described_class.create( - project: create(:project), + project: project, properties: { teamcity_url: teamcity_url, username: 'mic', @@ -207,6 +208,97 @@ describe TeamcityService, :use_clean_rails_memory_store_caching do end end + describe '#execute' do + context 'when push' do + let(:data) do + { + object_kind: 'push', + ref: 'refs/heads/dev-123_branch', + after: '0220c11b9a3e6c69dc8fd35321254ca9a7b98f7e', + total_commits_count: 1 + } + end + + it 'handles push request correctly' do + stub_post_to_build_queue(branch: 'dev-123_branch') + + expect(service.execute(data)).to include('Ok') + end + + it 'returns nil when ref is blank' do + data[:after] = Gitlab::Git::BLANK_SHA + + expect(service.execute(data)).to be_nil + end + + it 'returns nil when there is no content' do + data[:total_commits_count] = 0 + + expect(service.execute(data)).to be_nil + end + + it 'returns nil when a merge request is opened for the same ref' do + create(:merge_request, source_project: project, source_branch: 'dev-123_branch') + + expect(service.execute(data)).to be_nil + end + end + + context 'when merge_request' do + let(:data) do + { + object_kind: 'merge_request', + ref: 'refs/heads/dev-123_branch', + after: '0220c11b9a3e6c69dc8fd35321254ca9a7b98f7e', + total_commits_count: 1, + object_attributes: { + state: 'opened', + source_branch: 'dev-123_branch', + merge_status: 'unchecked' + } + } + end + + it 'handles merge request correctly' do + stub_post_to_build_queue(branch: 'dev-123_branch') + + expect(service.execute(data)).to include('Ok') + end + + it 'returns nil when merge request is not opened' do + data[:object_attributes][:state] = 'closed' + + expect(service.execute(data)).to be_nil + end + + it 'returns nil unless merge request is marked as unchecked' do + data[:object_attributes][:merge_status] = 'can_be_merged' + + expect(service.execute(data)).to be_nil + end + end + + it 'returns nil when event is not supported' do + data = { object_kind: 'foo' } + + expect(service.execute(data)).to be_nil + end + end + + def stub_post_to_build_queue(branch:) + teamcity_full_url = 'http://gitlab.com/teamcity/httpAuth/app/rest/buildQueue' + body ||= %Q(<build branchName=\"#{branch}\"><buildType id=\"foo\"/></build>) + auth = %w(mic password) + + stub_full_request(teamcity_full_url, method: :post).with( + basic_auth: auth, + body: body, + headers: { + 'Content-Type' => 'application/xml' + } + ).to_return(status: 200, body: 'Ok', headers: {}) + end + def stub_request(status: 200, body: nil, build_status: 'success') teamcity_full_url = 'http://gitlab.com/teamcity/httpAuth/app/rest/builds/branch:unspecified:any,revision:123' auth = %w(mic password) diff --git a/spec/models/project_services/youtrack_service_spec.rb b/spec/models/project_services/youtrack_service_spec.rb index bf9d892f66c..c48bf487af0 100644 --- a/spec/models/project_services/youtrack_service_spec.rb +++ b/spec/models/project_services/youtrack_service_spec.rb @@ -37,4 +37,49 @@ describe YoutrackService do expect(described_class.reference_pattern.match('YT-123')[:issue]).to eq('YT-123') end end + + context 'overriding properties' do + let(:url) { 'http://youtrack.example.com' } + let(:access_params) do + { project_url: url, issues_url: url, new_issue_url: url } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) do + create(:youtrack_service, :without_properties_callback, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:youtrack_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:youtrack_service, :without_properties_callback, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:youtrack_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('YouTrack') + expect(service.description).to eq('YouTrack issue tracker') + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index aad08b9d4aa..ff9e94afc12 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -98,11 +98,26 @@ describe Project do it { is_expected.to have_many(:lfs_file_locks) } it { is_expected.to have_many(:project_deploy_tokens) } it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) } + it { is_expected.to have_many(:cycle_analytics_stages) } it 'has an inverse relationship with merge requests' do expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project) end + it 'has a distinct has_many :lfs_objects relation through lfs_objects_projects' do + project = create(:project) + lfs_object = create(:lfs_object) + [:project, :design].each do |repository_type| + create(:lfs_objects_project, project: project, + lfs_object: lfs_object, + repository_type: repository_type) + end + + expect(project.lfs_objects_projects.size).to eq(2) + expect(project.lfs_objects.size).to eq(1) + expect(project.lfs_objects.to_a).to eql([lfs_object]) + end + context 'after initialized' do it "has a project_feature" do expect(described_class.new.project_feature).to be_present @@ -159,24 +174,6 @@ describe Project do it { is_expected.to include_module(Sortable) } end - describe '.missing_kubernetes_namespace' do - let!(:project) { create(:project) } - let!(:cluster) { create(:cluster, :provided_by_user, :group) } - let(:kubernetes_namespaces) { project.kubernetes_namespaces } - - subject { described_class.missing_kubernetes_namespace(kubernetes_namespaces) } - - it { is_expected.to contain_exactly(project) } - - context 'kubernetes namespace exists' do - before do - create(:cluster_kubernetes_namespace, project: project, cluster: cluster) - end - - it { is_expected.to be_empty } - end - end - describe 'validation' do let!(:project) { create(:project) } @@ -199,7 +196,7 @@ describe Project do .only_integer .is_greater_than_or_equal_to(10.minutes) .is_less_than(1.month) - .with_message('needs to be beetween 10 minutes and 1 month') + .with_message('needs to be between 10 minutes and 1 month') end it 'does not allow new projects beyond user limits' do @@ -483,7 +480,6 @@ describe Project do it { is_expected.to delegate_method(:members).to(:team).with_prefix(true) } it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).with_arguments(allow_nil: true) } - it { is_expected.to delegate_method(:group_clusters_enabled?).to(:group).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:root_ancestor).to(:namespace).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:last_pipeline).to(:commit).with_arguments(allow_nil: true) } end @@ -1161,7 +1157,6 @@ describe Project do describe '#pipeline_for' do let(:project) { create(:project, :repository) } - let!(:pipeline) { create_pipeline(project) } shared_examples 'giving the correct pipeline' do it { is_expected.to eq(pipeline) } @@ -1173,19 +1168,47 @@ describe Project do end end - context 'with explicit sha' do - subject { project.pipeline_for('master', pipeline.sha) } + context 'with a matching pipeline' do + let!(:pipeline) { create_pipeline(project) } + + context 'with explicit sha' do + subject { project.pipeline_for('master', pipeline.sha) } + + it_behaves_like 'giving the correct pipeline' + + context 'with supplied id' do + let!(:other_pipeline) { create_pipeline(project) } + + subject { project.pipeline_for('master', pipeline.sha, other_pipeline.id) } - it_behaves_like 'giving the correct pipeline' + it { is_expected.to eq(other_pipeline) } + end + end + + context 'with implicit sha' do + subject { project.pipeline_for('master') } + + it_behaves_like 'giving the correct pipeline' + end end - context 'with implicit sha' do + context 'when there is no matching pipeline' do subject { project.pipeline_for('master') } - it_behaves_like 'giving the correct pipeline' + it { is_expected.to be_nil } end end + describe '#pipelines_for' do + let(:project) { create(:project, :repository) } + let!(:pipeline) { create_pipeline(project) } + let!(:other_pipeline) { create_pipeline(project) } + + subject { project.pipelines_for(project.default_branch, project.commit.sha) } + + it { is_expected.to contain_exactly(pipeline, other_pipeline) } + end + describe '#builds_enabled' do let(:project) { create(:project) } @@ -1479,11 +1502,28 @@ describe Project do end context 'when set to INTERNAL in application settings' do + using RSpec::Parameterized::TableSyntax + before do stub_application_setting(default_project_visibility: Gitlab::VisibilityLevel::INTERNAL) end it { is_expected.to eq(Gitlab::VisibilityLevel::INTERNAL) } + + where(:attribute_name, :value) do + :visibility | 'public' + :visibility_level | Gitlab::VisibilityLevel::PUBLIC + 'visibility' | 'public' + 'visibility_level' | Gitlab::VisibilityLevel::PUBLIC + end + + with_them do + it 'sets the visibility level' do + proj = described_class.new(attribute_name => value, name: 'test', path: 'test') + + expect(proj.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) + end + end end end @@ -1645,26 +1685,6 @@ describe Project do end end - describe '.paginate_in_descending_order_using_id' do - let!(:project1) { create(:project) } - let!(:project2) { create(:project) } - - it 'orders the relation in descending order' do - expect(described_class.paginate_in_descending_order_using_id) - .to eq([project2, project1]) - end - - it 'applies a limit to the relation' do - expect(described_class.paginate_in_descending_order_using_id(limit: 1)) - .to eq([project2]) - end - - it 'limits projects by and ID when given' do - expect(described_class.paginate_in_descending_order_using_id(before: project2.id)) - .to eq([project1]) - end - end - describe '.including_namespace_and_owner' do it 'eager loads the namespace and namespace owner' do create(:project) @@ -1989,62 +2009,33 @@ describe Project do end end - describe '#latest_successful_build_for' do + describe '#latest_successful_build_for_ref' do let(:project) { create(:project, :repository) } let(:pipeline) { create_pipeline(project) } - context 'with many builds' do - it 'gives the latest builds from latest pipeline' do - pipeline1 = create_pipeline(project) - pipeline2 = create_pipeline(project) - create_build(pipeline1, 'test') - create_build(pipeline1, 'test2') - build1_p2 = create_build(pipeline2, 'test') - create_build(pipeline2, 'test2') + it_behaves_like 'latest successful build for sha or ref' - expect(project.latest_successful_build_for(build1_p2.name)) - .to eq(build1_p2) - end - end - - context 'with succeeded pipeline' do - let!(:build) { create_build } + subject { project.latest_successful_build_for_ref(build_name) } - context 'standalone pipeline' do - it 'returns builds for ref for default_branch' do - expect(project.latest_successful_build_for(build.name)) - .to eq(build) - end + context 'with a specified ref' do + let(:build) { create_build } - it 'returns empty relation if the build cannot be found' do - expect(project.latest_successful_build_for('TAIL')) - .to be_nil - end - end + subject { project.latest_successful_build_for_ref(build.name, project.default_branch) } - context 'with some pending pipeline' do - before do - create_build(create_pipeline(project, 'pending')) - end - - it 'gives the latest build from latest pipeline' do - expect(project.latest_successful_build_for(build.name)) - .to eq(build) - end - end + it { is_expected.to eq(build) } end + end - context 'with pending pipeline' do - it 'returns empty relation' do - pipeline.update(status: 'pending') - pending_build = create_build(pipeline) + describe '#latest_successful_build_for_sha' do + let(:project) { create(:project, :repository) } + let(:pipeline) { create_pipeline(project) } - expect(project.latest_successful_build_for(pending_build.name)).to be_nil - end - end + it_behaves_like 'latest successful build for sha or ref' + + subject { project.latest_successful_build_for_sha(build_name, project.commit.sha) } end - describe '#latest_successful_build_for!' do + describe '#latest_successful_build_for_ref!' do let(:project) { create(:project, :repository) } let(:pipeline) { create_pipeline(project) } @@ -2057,7 +2048,7 @@ describe Project do build1_p2 = create_build(pipeline2, 'test') create_build(pipeline2, 'test2') - expect(project.latest_successful_build_for(build1_p2.name)) + expect(project.latest_successful_build_for_ref!(build1_p2.name)) .to eq(build1_p2) end end @@ -2067,12 +2058,12 @@ describe Project do context 'standalone pipeline' do it 'returns builds for ref for default_branch' do - expect(project.latest_successful_build_for!(build.name)) + expect(project.latest_successful_build_for_ref!(build.name)) .to eq(build) end it 'returns exception if the build cannot be found' do - expect { project.latest_successful_build_for!(build.name, 'TAIL') } + expect { project.latest_successful_build_for_ref!(build.name, 'TAIL') } .to raise_error(ActiveRecord::RecordNotFound) end end @@ -2083,7 +2074,7 @@ describe Project do end it 'gives the latest build from latest pipeline' do - expect(project.latest_successful_build_for!(build.name)) + expect(project.latest_successful_build_for_ref!(build.name)) .to eq(build) end end @@ -2094,7 +2085,7 @@ describe Project do pipeline.update(status: 'pending') pending_build = create_build(pipeline) - expect { project.latest_successful_build_for!(pending_build.name) } + expect { project.latest_successful_build_for_ref!(pending_build.name) } .to raise_error(ActiveRecord::RecordNotFound) end end @@ -2262,7 +2253,22 @@ describe Project do end end - describe '#ancestors_upto', :nested_groups do + describe '#mark_stuck_remote_mirrors_as_failed!' do + it 'fails stuck remote mirrors' do + project = create(:project, :repository, :remote_mirror) + + project.remote_mirrors.first.update( + update_status: :started, + last_update_started_at: 2.days.ago + ) + + expect do + project.mark_stuck_remote_mirrors_as_failed! + end.to change { project.remote_mirrors.stuck.count }.from(1).to(0) + end + end + + describe '#ancestors_upto' do let(:parent) { create(:group) } let(:child) { create(:group, parent: parent) } let(:child2) { create(:group, parent: child) } @@ -2301,7 +2307,7 @@ describe Project do it { is_expected.to eq(group) } end - context 'in a nested group', :nested_groups do + context 'in a nested group' do let(:root) { create(:group) } let(:child) { create(:group, parent: root) } let(:project) { create(:project, group: child) } @@ -2310,6 +2316,57 @@ describe Project do end end + describe '#emails_disabled?' do + let(:project) { create(:project, emails_disabled: false) } + + context 'emails disabled in group' do + it 'returns true' do + allow(project.namespace).to receive(:emails_disabled?) { true } + + expect(project.emails_disabled?).to be_truthy + end + end + + context 'emails enabled in group' do + before do + allow(project.namespace).to receive(:emails_disabled?) { false } + end + + it 'returns false' do + expect(project.emails_disabled?).to be_falsey + end + + it 'returns true' do + project.update_attribute(:emails_disabled, true) + + expect(project.emails_disabled?).to be_truthy + end + end + + context 'when :emails_disabled feature flag is off' do + before do + stub_feature_flags(emails_disabled: false) + end + + context 'emails disabled in group' do + it 'returns false' do + allow(project.namespace).to receive(:emails_disabled?) { true } + + expect(project.emails_disabled?).to be_falsey + end + end + + context 'emails enabled in group' do + it 'returns false' do + allow(project.namespace).to receive(:emails_disabled?) { false } + project.update_attribute(:emails_disabled, true) + + expect(project.emails_disabled?).to be_falsey + end + end + end + end + describe '#lfs_enabled?' do let(:project) { create(:project) } @@ -2449,7 +2506,7 @@ describe Project do expect(forked_project.in_fork_network_of?(project)).to be_truthy end - it 'is true for a fork of a fork', :postgresql do + it 'is true for a fork of a fork' do other_fork = fork_project(forked_project) expect(other_fork.in_fork_network_of?(project)).to be_truthy @@ -2604,55 +2661,33 @@ describe Project do end describe '#deployment_variables' do - context 'when project has no deployment service' do - let(:project) { create(:project) } - - it 'returns an empty array' do - expect(project.deployment_variables).to eq [] - end - end + let(:project) { create(:project) } + let(:environment) { 'production' } - context 'when project uses mock deployment service' do - let(:project) { create(:mock_deployment_project) } + subject { project.deployment_variables(environment: environment) } - it 'returns an empty array' do - expect(project.deployment_variables).to eq [] - end + before do + expect(project).to receive(:deployment_platform).with(environment: environment) + .and_return(deployment_platform) end - context 'when project has a deployment service' do - shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do - it 'returns variables from this service' do - expect(project.deployment_variables).to include( - { key: 'KUBE_TOKEN', value: project.deployment_platform.token, public: false, masked: true } - ) - end - end + context 'when project has no deployment platform' do + let(:deployment_platform) { nil } - context 'when user configured kubernetes from Integration > Kubernetes' do - let(:project) { create(:kubernetes_project) } - - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' - end + it { is_expected.to eq [] } + end - context 'when user configured kubernetes from CI/CD > Clusters and KubernetesNamespace migration has not been executed' do - let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } + context 'when project has a deployment platform' do + let(:platform_variables) { %w(platform variables) } + let(:deployment_platform) { double } - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' + before do + expect(deployment_platform).to receive(:predefined_variables) + .with(project: project, environment_name: environment) + .and_return(platform_variables) end - context 'when user configured kubernetes from CI/CD > Clusters and KubernetesNamespace migration has been executed' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token) } - let!(:cluster) { kubernetes_namespace.cluster } - let(:project) { kubernetes_namespace.project } - - it 'returns token from kubernetes namespace' do - expect(project.deployment_variables).to include( - { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false, masked: true } - ) - end - end + it { is_expected.to eq platform_variables } end end @@ -2680,9 +2715,10 @@ describe Project do describe '#ci_variables_for' do let(:project) { create(:project) } + let(:environment_scope) { '*' } let!(:ci_variable) do - create(:ci_variable, value: 'secret', project: project) + create(:ci_variable, value: 'secret', project: project, environment_scope: environment_scope) end let!(:protected_variable) do @@ -2727,6 +2763,96 @@ describe Project do it_behaves_like 'ref is protected' end + + context 'when environment name is specified' do + let(:environment) { 'review/name' } + + subject do + project.ci_variables_for(ref: 'ref', environment: environment) + end + + context 'when environment scope is exactly matched' do + let(:environment_scope) { 'review/name' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope is matched by wildcard' do + let(:environment_scope) { 'review/*' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope does not match' do + let(:environment_scope) { 'review/*/special' } + + it { is_expected.not_to contain_exactly(ci_variable) } + end + + context 'when environment scope has _' do + let(:environment_scope) { '*_*' } + + it 'does not treat it as wildcard' do + is_expected.not_to contain_exactly(ci_variable) + end + + context 'when environment name contains underscore' do + let(:environment) { 'foo_bar/test' } + let(:environment_scope) { 'foo_bar/*' } + + it 'matches literally for _' do + is_expected.to contain_exactly(ci_variable) + end + end + end + + # The environment name and scope cannot have % at the moment, + # but we're considering relaxing it and we should also make sure + # it doesn't break in case some data sneaked in somehow as we're + # not checking this integrity in database level. + context 'when environment scope has %' do + it 'does not treat it as wildcard' do + ci_variable.update_attribute(:environment_scope, '*%*') + + is_expected.not_to contain_exactly(ci_variable) + end + + context 'when environment name contains a percent' do + let(:environment) { 'foo%bar/test' } + + it 'matches literally for _' do + ci_variable.update(environment_scope: 'foo%bar/*') + + is_expected.to contain_exactly(ci_variable) + end + end + end + + context 'when variables with the same name have different environment scopes' do + let!(:partially_matched_variable) do + create(:ci_variable, + key: ci_variable.key, + value: 'partial', + environment_scope: 'review/*', + project: project) + end + + let!(:perfectly_matched_variable) do + create(:ci_variable, + key: ci_variable.key, + value: 'prefect', + environment_scope: 'review/name', + project: project) + end + + it 'puts variables matching environment scope more in the end' do + is_expected.to eq( + [ci_variable, + partially_matched_variable, + perfectly_matched_variable]) + end + end + end end describe '#any_lfs_file_locks?', :request_store do @@ -2977,6 +3103,16 @@ describe Project do expect(project.public_path_for_source_path('file.html', sha)).to be_nil end end + + it 'returns a public path with a leading slash unmodified' do + route_map = Gitlab::RouteMap.new(<<-MAP.strip_heredoc) + - source: 'source/file.html' + public: '/public/file' + MAP + allow(project).to receive(:route_map_for).with(sha).and_return(route_map) + + expect(project.public_path_for_source_path('source/file.html', sha)).to eq('/public/file') + end end context 'when there is no route map' do @@ -3097,11 +3233,8 @@ describe Project do let(:project) { create(:project) } it 'shows full error updating an invalid MR' do - error_message = 'Failed to replace merge_requests because one or more of the new records could not be saved.'\ - ' Validate fork Source project is not a fork of the target project' - expect { project.append_or_update_attribute(:merge_requests, [create(:merge_request)]) } - .to raise_error(ActiveRecord::RecordNotSaved, error_message) + .to raise_error(ActiveRecord::RecordInvalid, /Failed to set merge_requests:/) end it 'updates the project successfully' do @@ -3447,6 +3580,7 @@ describe Project do before do allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + stub_application_setting(hashed_storage_enabled: false) end describe '#base_dir' do @@ -3553,10 +3687,6 @@ describe Project do let(:hashed_prefix) { File.join('@hashed', hash[0..1], hash[2..3]) } let(:hashed_path) { File.join(hashed_prefix, hash) } - before do - stub_application_setting(hashed_storage_enabled: true) - end - describe '#legacy_storage?' do it 'returns false' do expect(project.legacy_storage?).to be_falsey @@ -3787,7 +3917,7 @@ describe Project do end end - context 'when enabled on root parent', :nested_groups do + context 'when enabled on root parent' do let(:parent_group) { create(:group, parent: create(:group, :auto_devops_enabled)) } context 'when auto devops instance enabled' do @@ -3807,7 +3937,7 @@ describe Project do end end - context 'when disabled on root parent', :nested_groups do + context 'when disabled on root parent' do let(:parent_group) { create(:group, parent: create(:group, :auto_devops_disabled)) } context 'when auto devops instance enabled' do @@ -4019,7 +4149,7 @@ describe Project do context 'with a ref that is not the default branch' do it 'returns the latest successful pipeline for the given ref' do - expect(project.ci_pipelines).to receive(:latest_successful_for).with('foo') + expect(project.ci_pipelines).to receive(:latest_successful_for_ref).with('foo') project.latest_successful_pipeline_for('foo') end @@ -4047,7 +4177,7 @@ describe Project do it 'memoizes and returns the latest successful pipeline for the default branch' do pipeline = double(:pipeline) - expect(project.ci_pipelines).to receive(:latest_successful_for) + expect(project.ci_pipelines).to receive(:latest_successful_for_ref) .with(project.default_branch) .and_return(pipeline) .once @@ -4234,6 +4364,39 @@ describe Project do end end + describe '#has_active_hooks?' do + set(:project) { create(:project) } + + it { expect(project.has_active_hooks?).to be_falsey } + + it 'returns true when a matching push hook exists' do + create(:project_hook, push_events: true, project: project) + + expect(project.has_active_hooks?(:merge_request_events)).to be_falsey + expect(project.has_active_hooks?).to be_truthy + end + + it 'returns true when a matching system hook exists' do + create(:system_hook, push_events: true) + + expect(project.has_active_hooks?(:merge_request_events)).to be_falsey + expect(project.has_active_hooks?).to be_truthy + end + end + + describe '#has_active_services?' do + set(:project) { create(:project) } + + it { expect(project.has_active_services?).to be_falsey } + + it 'returns true when a matching service exists' do + create(:custom_issue_tracker_service, push_events: true, merge_requests_events: false, project: project) + + expect(project.has_active_services?(:merge_request_hooks)).to be_falsey + expect(project.has_active_services?).to be_truthy + end + end + describe '#badges' do let(:project_group) { create(:group) } let(:project) { create(:project, path: 'avatar', namespace: project_group) } @@ -4250,18 +4413,16 @@ describe Project do expect(project.badges.count).to eq 3 end - if Group.supports_nested_objects? - context 'with nested_groups' do - let(:parent_group) { create(:group) } + context 'with nested_groups' do + let(:parent_group) { create(:group) } - before do - create_list(:group_badge, 2, group: project_group) - project_group.update(parent: parent_group) - end + before do + create_list(:group_badge, 2, group: project_group) + project_group.update(parent: parent_group) + end - it 'returns the project and the project nested groups badges' do - expect(project.badges.count).to eq 5 - end + it 'returns the project and the project nested groups badges' do + expect(project.badges.count).to eq 5 end end end @@ -4698,10 +4859,6 @@ describe Project do subject { project.object_pool_params } - before do - stub_application_setting(hashed_storage_enabled: true) - end - context 'when the objects cannot be pooled' do let(:project) { create(:project, :repository, :private) } @@ -4747,10 +4904,6 @@ describe Project do context 'when objects are poolable' do let(:project) { create(:project, :repository, :public) } - before do - stub_application_setting(hashed_storage_enabled: true) - end - it { is_expected.to be_git_objects_poolable } end end diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index 1cb49d83ffa..a164ed9bbea 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -135,6 +135,38 @@ describe ProjectStatistics do expect(statistics.wiki_size).to eq(0) end end + + context 'when the column is namespace relatable' do + let(:namespace) { create(:group) } + let(:project) { create(:project, namespace: namespace) } + + context 'when arguments are passed' do + it 'schedules the aggregation worker' do + expect(Namespaces::ScheduleAggregationWorker) + .to receive(:perform_async) + + statistics.refresh!(only: [:lfs_objects_size]) + end + end + + context 'when no argument is passed' do + it 'schedules the aggregation worker' do + expect(Namespaces::ScheduleAggregationWorker) + .to receive(:perform_async) + + statistics.refresh! + end + end + end + + context 'when the column is not namespace relatable' do + it 'does not schedules an aggregation worker' do + expect(Namespaces::ScheduleAggregationWorker) + .not_to receive(:perform_async) + + statistics.refresh!(only: [:commit_count]) + end + end end describe '#update_commit_count' do diff --git a/spec/models/prometheus_metric_spec.rb b/spec/models/prometheus_metric_spec.rb index 3610408c138..a123ff5a2a6 100644 --- a/spec/models/prometheus_metric_spec.rb +++ b/spec/models/prometheus_metric_spec.rb @@ -150,4 +150,17 @@ describe PrometheusMetric do expect(subject.to_query_metric.queries).to eq(queries) end end + + describe '#to_metric_hash' do + it 'returns a hash suitable for inclusion on a metrics dashboard' do + expected_output = { + query_range: subject.query, + unit: subject.unit, + label: subject.legend, + metric_id: subject.id + } + + expect(subject.to_metric_hash).to eq(expected_output) + end + end end diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 7c106ce6b85..e9d846e7291 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -64,4 +64,14 @@ RSpec.describe Release do is_expected.to all(be_a(Releases::Source)) end end + + describe '#upcoming_release?' do + context 'during the backfill migration when released_at could be nil' do + it 'handles a nil released_at value and returns false' do + allow(release).to receive(:released_at).and_return nil + + expect(release.upcoming_release?).to eq(false) + end + end + end end diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index e14b19db915..7edeb56efe2 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -113,7 +113,7 @@ describe RemoteMirror, :mailer do remote_mirror = create(:remote_mirror) - expect(remote_mirror.remote_name).to eq("remote_mirror_secret") + expect(remote_mirror.remote_name).to eq('remote_mirror_secret') end end @@ -153,14 +153,14 @@ describe RemoteMirror, :mailer do end end - describe '#mark_as_failed' do + describe '#mark_as_failed!' do let(:remote_mirror) { create(:remote_mirror) } let(:error_message) { 'http://user:pass@test.com/root/repoC.git/' } let(:sanitized_error_message) { 'http://*****:*****@test.com/root/repoC.git/' } subject do remote_mirror.update_start - remote_mirror.mark_as_failed(error_message) + remote_mirror.mark_as_failed!(error_message) end it 'sets the update_status to failed' do @@ -201,11 +201,20 @@ describe RemoteMirror, :mailer do end context 'stuck mirrors' do - it 'includes mirrors stuck in started with no last_update_at set' do + it 'includes mirrors that were started over an hour ago' do + mirror = create_mirror(url: 'http://cantbeblank', + update_status: 'started', + last_update_started_at: 3.hours.ago, + last_update_at: 2.hours.ago) + + expect(described_class.stuck.last).to eq(mirror) + end + + it 'includes mirrors started over 3 hours ago for their first sync' do mirror = create_mirror(url: 'http://cantbeblank', update_status: 'started', last_update_at: nil, - updated_at: 25.hours.ago) + last_update_started_at: 4.hours.ago) expect(described_class.stuck.last).to eq(mirror) end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index c5ab7e57272..79395fcc994 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -844,6 +844,19 @@ describe Repository do end end + describe '#get_raw_changes' do + context `with non-UTF8 bytes in paths` do + let(:old_rev) { 'd0888d297eadcd7a345427915c309413b1231e65' } + let(:new_rev) { '19950f03c765f7ac8723a73a0599764095f52fc0' } + let(:changes) { repository.raw_changes_between(old_rev, new_rev) } + + it 'returns the changes' do + expect { changes }.not_to raise_error + expect(changes.first.new_path.bytes).to eq("hello\x80world".bytes) + end + end + end + describe '#create_ref' do it 'redirects the call to write_ref' do ref, ref_path = '1', '2' @@ -1210,36 +1223,66 @@ describe Repository do end describe '#branch_exists?' do - it 'uses branch_names' do - allow(repository).to receive(:branch_names).and_return(['foobar']) + let(:branch) { repository.root_ref } - expect(repository.branch_exists?('foobar')).to eq(true) - expect(repository.branch_exists?('master')).to eq(false) + subject { repository.branch_exists?(branch) } + + it 'delegates to branch_names when the cache is empty' do + repository.expire_branches_cache + + expect(repository).to receive(:branch_names).and_call_original + is_expected.to eq(true) + end + + it 'uses redis set caching when the cache is filled' do + repository.branch_names # ensure the branch name cache is filled + + expect(repository) + .to receive(:branch_names_include?) + .with(branch) + .and_call_original + + is_expected.to eq(true) end end describe '#tag_exists?' do - it 'uses tag_names' do - allow(repository).to receive(:tag_names).and_return(['foobar']) + let(:tag) { repository.tags.first.name } - expect(repository.tag_exists?('foobar')).to eq(true) - expect(repository.tag_exists?('master')).to eq(false) + subject { repository.tag_exists?(tag) } + + it 'delegates to tag_names when the cache is empty' do + repository.expire_tags_cache + + expect(repository).to receive(:tag_names).and_call_original + is_expected.to eq(true) + end + + it 'uses redis set caching when the cache is filled' do + repository.tag_names # ensure the tag name cache is filled + + expect(repository) + .to receive(:tag_names_include?) + .with(tag) + .and_call_original + + is_expected.to eq(true) end end - describe '#branch_names', :use_clean_rails_memory_store_caching do + describe '#branch_names', :clean_gitlab_redis_cache do let(:fake_branch_names) { ['foobar'] } it 'gets cached across Repository instances' do allow(repository.raw_repository).to receive(:branch_names).once.and_return(fake_branch_names) - expect(repository.branch_names).to eq(fake_branch_names) + expect(repository.branch_names).to match_array(fake_branch_names) fresh_repository = Project.find(project.id).repository expect(fresh_repository.object_id).not_to eq(repository.object_id) expect(fresh_repository.raw_repository).not_to receive(:branch_names) - expect(fresh_repository.branch_names).to eq(fake_branch_names) + expect(fresh_repository.branch_names).to match_array(fake_branch_names) end end @@ -1407,12 +1450,13 @@ describe Repository do source_project: project) end - it 'writes merge of source and target to MR merge_ref_path' do + it 'writes merge of source SHA and first parent ref to MR merge_ref_path' do merge_commit_id = repository.merge_to_ref(user, merge_request.diff_head_sha, merge_request, merge_request.merge_ref_path, - 'Custom message') + 'Custom message', + merge_request.target_branch_ref) merge_commit = repository.commit(merge_commit_id) @@ -1730,12 +1774,23 @@ describe Repository do end end - describe '#before_push_tag' do + describe '#expires_caches_for_tags' do it 'flushes the cache' do expect(repository).to receive(:expire_statistics_caches) expect(repository).to receive(:expire_emptiness_caches) expect(repository).to receive(:expire_tags_cache) + repository.expire_caches_for_tags + end + end + + describe '#before_push_tag' do + it 'logs an event' do + expect(repository).not_to receive(:expire_statistics_caches) + expect(repository).not_to receive(:expire_emptiness_caches) + expect(repository).not_to receive(:expire_tags_cache) + expect(repository).to receive(:repository_event).with(:push_tag) + repository.before_push_tag end end @@ -1767,6 +1822,12 @@ describe Repository do repository.after_create_branch end + + it 'does not expire the branch caches when specified' do + expect(repository).not_to receive(:expire_branches_cache) + + repository.after_create_branch(expire_cache: false) + end end describe '#after_remove_branch' do @@ -1775,25 +1836,45 @@ describe Repository do repository.after_remove_branch end + + it 'does not expire the branch caches when specified' do + expect(repository).not_to receive(:expire_branches_cache) + + repository.after_remove_branch(expire_cache: false) + end end describe '#after_create' do + it 'calls expire_status_cache' do + expect(repository).to receive(:expire_status_cache) + + repository.after_create + end + + it 'logs an event' do + expect(repository).to receive(:repository_event).with(:create_repository) + + repository.after_create + end + end + + describe '#expire_status_cache' do it 'flushes the exists cache' do expect(repository).to receive(:expire_exists_cache) - repository.after_create + repository.expire_status_cache end it 'flushes the root ref cache' do expect(repository).to receive(:expire_root_ref_cache) - repository.after_create + repository.expire_status_cache end it 'flushes the emptiness caches' do expect(repository).to receive(:expire_emptiness_caches) - repository.after_create + repository.expire_status_cache end end @@ -2285,48 +2366,6 @@ describe Repository do end end - describe '#diverging_commit_counts' do - let(:diverged_branch) { repository.find_branch('fix') } - let(:root_ref_sha) { repository.raw_repository.commit(repository.root_ref).id } - let(:diverged_branch_sha) { diverged_branch.dereferenced_target.sha } - - it 'returns the commit counts behind and ahead of default branch' do - result = repository.diverging_commit_counts(diverged_branch) - - expect(result).to eq(behind: 29, ahead: 2) - end - - context 'when gitaly_count_diverging_commits_no_max is enabled' do - before do - stub_feature_flags(gitaly_count_diverging_commits_no_max: true) - end - - it 'calls diverging_commit_count without max count' do - expect(repository.raw_repository) - .to receive(:diverging_commit_count) - .with(root_ref_sha, diverged_branch_sha) - .and_return([29, 2]) - - repository.diverging_commit_counts(diverged_branch) - end - end - - context 'when gitaly_count_diverging_commits_no_max is disabled' do - before do - stub_feature_flags(gitaly_count_diverging_commits_no_max: false) - end - - it 'calls diverging_commit_count with max count' do - expect(repository.raw_repository) - .to receive(:diverging_commit_count) - .with(root_ref_sha, diverged_branch_sha, max_count: Repository::MAX_DIVERGING_COUNT) - .and_return([29, 2]) - - repository.diverging_commit_counts(diverged_branch) - end - end - end - describe '#refresh_method_caches' do it 'refreshes the caches of the given types' do expect(repository).to receive(:expire_method_caches) @@ -2454,16 +2493,15 @@ describe Repository do # Gets the commit oid, and warms the cache oid = project.commit.id - expect(Gitlab::Git::Commit).not_to receive(:find).once + expect(Gitlab::Git::Commit).to receive(:find).once - project.commit_by(oid: oid) + 2.times { project.commit_by(oid: oid) } end it 'caches nil values' do expect(Gitlab::Git::Commit).to receive(:find).once - project.commit_by(oid: '1' * 40) - project.commit_by(oid: '1' * 40) + 2.times { project.commit_by(oid: '1' * 40) } end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 64db32781fe..0797b9a9d83 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -6,6 +6,8 @@ describe Service do describe "Associations" do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } + it { is_expected.to have_one :jira_tracker_data } + it { is_expected.to have_one :issue_tracker_data } end describe 'Validations' do @@ -80,7 +82,7 @@ describe Service do context 'when template is invalid' do it 'sets service template to inactive when template is invalid' do project = create(:project) - template = KubernetesService.new(template: true, active: true) + template = build(:prometheus_service, template: true, active: true, properties: {}) template.save(validate: false) service = described_class.build_from_template(project.id, template) @@ -242,7 +244,8 @@ describe Service do let(:service) do GitlabIssueTrackerService.create( project: create(:project), - title: 'random title' + title: 'random title', + project_url: 'http://gitlab.example.com' ) end @@ -250,8 +253,12 @@ describe Service do expect { service }.not_to raise_error end + it 'sets title correctly' do + expect(service.title).to eq('random title') + end + it 'creates the properties' do - expect(service.properties).to eq({ "title" => "random title" }) + expect(service.properties).to eq({ "project_url" => "http://gitlab.example.com" }) end end @@ -307,10 +314,10 @@ describe Service do end describe '.find_by_template' do - let!(:kubernetes_service) { create(:kubernetes_service, template: true) } + let!(:service) { create(:service, template: true) } it 'returns service template' do - expect(KubernetesService.find_by_template).to eq(kubernetes_service) + expect(described_class.find_by_template).to eq(service) end end diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index b5bf294790a..9aeef7c3b4b 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -262,11 +262,7 @@ describe Todo do todo2 = create(:todo, group: child_group) todos = described_class.for_group_and_descendants(parent_group) - expect(todos).to include(todo1) - - # Nested groups only work on PostgreSQL, so on MySQL todo2 won't be - # present. - expect(todos).to include(todo2) if Gitlab::Database.postgresql? + expect(todos).to contain_exactly(todo1, todo2) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c95bbb0b3f5..46b86e8393d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -530,6 +530,17 @@ describe User do end describe 'before save hook' do + context '#default_private_profile_to_false' do + let(:user) { create(:user, private_profile: true) } + + it 'converts nil to false' do + user.private_profile = nil + user.save! + + expect(user.private_profile).to eq false + end + end + context 'when saving an external user' do let(:user) { create(:user) } let(:external_user) { create(:user, external: true) } @@ -783,6 +794,24 @@ describe User do end end + describe '#accessible_deploy_keys' do + let(:user) { create(:user) } + let(:project) { create(:project) } + let!(:private_deploy_keys_project) { create(:deploy_keys_project) } + let!(:public_deploy_keys_project) { create(:deploy_keys_project) } + let!(:accessible_deploy_keys_project) { create(:deploy_keys_project, project: project) } + + before do + public_deploy_keys_project.deploy_key.update(public: true) + project.add_developer(user) + end + + it 'user can only see deploy keys accessible to right projects' do + expect(user.accessible_deploy_keys).to match_array([public_deploy_keys_project.deploy_key, + accessible_deploy_keys_project.deploy_key]) + end + end + describe '#deploy_keys' do include_context 'user keys' @@ -974,7 +1003,7 @@ describe User do it { expect(user.namespaces).to contain_exactly(user.namespace, group) } it { expect(user.manageable_namespaces).to contain_exactly(user.namespace, group) } - context 'with child groups', :nested_groups do + context 'with child groups' do let!(:subgroup) { create(:group, parent: group) } describe '#manageable_namespaces' do @@ -1127,6 +1156,7 @@ describe User do expect(user.can_create_group).to eq(Gitlab.config.gitlab.default_can_create_group) expect(user.theme_id).to eq(Gitlab.config.gitlab.default_theme) expect(user.external).to be_falsey + expect(user.private_profile).to eq false end end @@ -1769,6 +1799,26 @@ describe User do end end + describe '#ultraauth_user?' do + it 'is true if provider is ultraauth' do + user = create(:omniauth_user, provider: 'ultraauth') + + expect(user.ultraauth_user?).to be_truthy + end + + it 'is false with othe provider' do + user = create(:omniauth_user, provider: 'not-ultraauth') + + expect(user.ultraauth_user?).to be_falsey + end + + it 'is false if no extern_uid is provided' do + user = create(:omniauth_user, extern_uid: nil) + + expect(user.ldap_user?).to be_falsey + end + end + describe '#full_website_url' do let(:user) { create(:user) } @@ -2050,11 +2100,7 @@ describe User do subject { user.membership_groups } - if Group.supports_nested_objects? - it { is_expected.to contain_exactly parent_group, child_group } - else - it { is_expected.to contain_exactly parent_group } - end + it { is_expected.to contain_exactly parent_group, child_group } end describe '#authorizations_for_projects' do @@ -2354,7 +2400,7 @@ describe User do it_behaves_like :member end - context 'with subgroup with different owner for project runner', :nested_groups do + context 'with subgroup with different owner for project runner' do let(:group) { create(:group) } let(:another_user) { create(:user) } let(:subgroup) { create(:group, parent: group) } @@ -2458,22 +2504,16 @@ describe User do group.add_owner(user) end - if Group.supports_nested_objects? - it 'returns all groups' do - is_expected.to match_array [ - group, - nested_group_1, nested_group_1_1, - nested_group_2, nested_group_2_1 - ] - end - else - it 'returns the top-level groups' do - is_expected.to match_array [group] - end + it 'returns all groups' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1, + nested_group_2, nested_group_2_1 + ] end end - context 'user is member of the first child (internal node), branch 1', :nested_groups do + context 'user is member of the first child (internal node), branch 1' do before do nested_group_1.add_owner(user) end @@ -2486,7 +2526,7 @@ describe User do end end - context 'user is member of the first child (internal node), branch 2', :nested_groups do + context 'user is member of the first child (internal node), branch 2' do before do nested_group_2.add_owner(user) end @@ -2499,7 +2539,7 @@ describe User do end end - context 'user is member of the last child (leaf node)', :nested_groups do + context 'user is member of the last child (leaf node)' do before do nested_group_1_1.add_owner(user) end @@ -2655,7 +2695,7 @@ describe User do end end - context 'with 2FA requirement from expanded groups', :nested_groups do + context 'with 2FA requirement from expanded groups' do let!(:group1) { create :group, require_two_factor_authentication: true } let!(:group1a) { create :group, parent: group1 } @@ -2670,7 +2710,7 @@ describe User do end end - context 'with 2FA requirement on nested child group', :nested_groups do + context 'with 2FA requirement on nested child group' do let!(:group1) { create :group, require_two_factor_authentication: false } let!(:group1a) { create :group, require_two_factor_authentication: true, parent: group1 } @@ -2807,6 +2847,12 @@ describe User do expect(user.allow_password_authentication_for_web?).to be_falsey end + + it 'returns false for ultraauth user' do + user = create(:omniauth_user, provider: 'ultraauth') + + expect(user.allow_password_authentication_for_web?).to be_falsey + end end describe '#allow_password_authentication_for_git?' do @@ -2829,6 +2875,12 @@ describe User do expect(user.allow_password_authentication_for_git?).to be_falsey end + + it 'returns false for ultraauth user' do + user = create(:omniauth_user, provider: 'ultraauth') + + expect(user.allow_password_authentication_for_git?).to be_falsey + end end describe '#assigned_open_merge_requests_count' do @@ -2899,7 +2951,7 @@ describe User do let(:user) { create(:user, username: username) } context 'when the user is updated' do - context 'when the username is changed' do + context 'when the username or name is changed' do let(:new_username) { 'bar' } it 'changes the namespace (just to compare to when username is not changed)' do @@ -2910,16 +2962,24 @@ describe User do end.to change { user.namespace.updated_at } end - it 'updates the namespace name' do + it 'updates the namespace path when the username was changed' do user.update!(username: new_username) - expect(user.namespace.name).to eq(new_username) + expect(user.namespace.path).to eq(new_username) end - it 'updates the namespace path' do - user.update!(username: new_username) + it 'updates the namespace name if the name was changed' do + user.update!(name: 'New name') - expect(user.namespace.path).to eq(new_username) + expect(user.namespace.name).to eq('New name') + end + + it 'updates nested routes for the namespace if the name was changed' do + project = create(:project, namespace: user.namespace) + + user.update!(name: 'New name') + + expect(project.route.reload.name).to include('New name') end context 'when there is a validation error (namespace name taken) while updating namespace' do @@ -3322,7 +3382,7 @@ describe User do end describe '#requires_usage_stats_consent?' do - let(:user) { create(:user, created_at: 8.days.ago) } + let(:user) { create(:user, :admin, created_at: 8.days.ago) } before do allow(user).to receive(:has_current_license?).and_return false @@ -3346,7 +3406,7 @@ describe User do end it 'does not require consent if usage stats were set by this user' do - allow(Gitlab::CurrentSettings).to receive(:usage_stats_set_by_user_id).and_return(user.id) + create(:application_setting, usage_stats_set_by_user_id: user.id) expect(user.requires_usage_stats_consent?).to be false end @@ -3452,4 +3512,37 @@ describe User do expect(described_class.reorder_by_name).to eq([user1, user2]) end end + + describe '#notification_email_for' do + let(:user) { create(:user) } + let(:group) { create(:group) } + + subject { user.notification_email_for(group) } + + context 'when group is nil' do + let(:group) { nil } + + it 'returns global notification email' do + is_expected.to eq(user.notification_email) + end + end + + context 'when group has no notification email set' do + it 'returns global notification email' do + create(:notification_setting, user: user, source: group, notification_email: '') + + is_expected.to eq(user.notification_email) + end + end + + context 'when group has notification email set' do + it 'returns group notification email' do + group_notification_email = 'user+group@example.com' + + create(:notification_setting, user: user, source: group, notification_email: group_notification_email) + + is_expected.to eq(group_notification_email) + end + end + end end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 520a06e138e..18c62c917dc 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -81,10 +81,9 @@ describe WikiPage do grouped_entries = described_class.group_by_directory(wiki.list_pages) actual_order = - grouped_entries.map do |page_or_dir| + grouped_entries.flat_map do |page_or_dir| get_slugs(page_or_dir) end - .flatten expect(actual_order).to eq(expected_order) end end |
