diff options
Diffstat (limited to 'spec/models')
89 files changed, 2589 insertions, 1426 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..83d6ff754c5 --- /dev/null +++ b/spec/models/analytics/cycle_analytics/project_stage_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Analytics::CycleAnalytics::ProjectStage do + describe 'associations' do + it { is_expected.to belong_to(:project) } + end + + it 'default stages must be valid' do + project = create(:project) + + Gitlab::Analytics::CycleAnalytics::DefaultStages.all.each do |params| + stage = described_class.new(params.merge(project: project)) + expect(stage).to be_valid + end + end + + it_behaves_like "cycle analytics stage" do + let(:parent) { create(:project) } + let(:parent_name) { :project } + end +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index ab6f6dfe720..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) diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index 8452ac69734..b15b26b1630 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -44,6 +44,29 @@ describe AwardEmoji do end end + describe 'scopes' do + set(:thumbsup) { create(:award_emoji, name: 'thumbsup') } + set(:thumbsdown) { create(:award_emoji, name: 'thumbsdown') } + + describe '.upvotes' do + it { expect(described_class.upvotes).to contain_exactly(thumbsup) } + end + + describe '.downvotes' do + it { expect(described_class.downvotes).to contain_exactly(thumbsdown) } + end + + describe '.named' do + it { expect(described_class.named('thumbsup')).to contain_exactly(thumbsup) } + it { expect(described_class.named(%w[thumbsup thumbsdown])).to contain_exactly(thumbsup, thumbsdown) } + end + + describe '.awarded_by' do + it { expect(described_class.awarded_by(thumbsup.user)).to contain_exactly(thumbsup) } + it { expect(described_class.awarded_by([thumbsup.user, thumbsdown.user])).to contain_exactly(thumbsup, thumbsdown) } + end + end + describe 'expiring ETag cache' do context 'on a note' do let(:note) { create(:note_on_issue) } 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..bc853d45085 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) } @@ -147,6 +149,56 @@ describe Ci::Build do end end + describe '.with_stale_live_trace' do + subject { described_class.with_stale_live_trace } + + context 'when build has a stale live trace' do + let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 1.day.ago) } + + it 'selects the build' do + is_expected.to eq([build]) + end + end + + context 'when build does not have a stale live trace' do + let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 1.hour.ago) } + + it 'does not select the build' do + is_expected.to be_empty + end + end + end + + describe '.finished_before' do + subject { described_class.finished_before(date) } + + let(:date) { 1.hour.ago } + + context 'when build has finished one day ago' do + let!(:build) { create(:ci_build, :success, finished_at: 1.day.ago) } + + it 'selects the build' do + is_expected.to eq([build]) + end + end + + context 'when build has finished 30 minutes ago' do + let!(:build) { create(:ci_build, :success, finished_at: 30.minutes.ago) } + + it 'returns an empty array' do + is_expected.to be_empty + end + end + + context 'when build is still running' do + let!(:build) { create(:ci_build, :running) } + + it 'returns an empty array' do + is_expected.to be_empty + end + end + end + describe '.with_reports' do subject { described_class.with_reports(Ci::JobArtifact.test_reports) } @@ -181,6 +233,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 +687,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 +838,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 +2187,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 +2390,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 +2430,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 +3785,7 @@ describe Ci::Build do before do build.ensure_metadata + build.needs.create!(name: 'another-job') end it 'drops metadata' do @@ -3581,6 +3793,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_spec.rb b/spec/models/ci/pipeline_spec.rb index e24bbc39761..7d84d094bdf 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe Ci::Pipeline, :mailer do include ProjectForksHelper + include StubRequests let(:user) { create(:user) } set(:project) { create(:project) } @@ -1799,7 +1800,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 @@ -1807,7 +1808,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 @@ -1916,6 +1930,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') @@ -2484,7 +2505,7 @@ describe Ci::Pipeline, :mailer do let(:enabled) { true } before do - WebMock.stub_request(:post, hook.url) + stub_full_request(hook.url, method: :post) end context 'with multiple builds' do @@ -2538,7 +2559,7 @@ describe Ci::Pipeline, :mailer do end def have_requested_pipeline_hook(status) - have_requested(:post, hook.url).with do |req| + have_requested(:post, stubbed_hostname(hook.url)).with do |req| json_body = JSON.parse(req.body) json_body['object_attributes']['status'] == status && json_body['builds'].length == 2 diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index f735a89f69f..70ff3cf5dc4 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -80,6 +80,13 @@ describe Ci::Runner do end end + describe 'constraints' do + it '.UPDATE_CONTACT_COLUMN_EVERY' do + expect(described_class::UPDATE_CONTACT_COLUMN_EVERY.max) + .to be <= described_class::ONLINE_CONTACT_TIMEOUT + end + end + describe '#access_level' do context 'when creating new runner and access_level is nil' do let(:runner) do @@ -146,7 +153,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 +561,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 +634,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/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index 7f4819cbb9a..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 @@ -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 52661178d76..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) } @@ -579,60 +529,39 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end - describe '#find_or_initialize_kubernetes_namespace_for_project' do - let(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.projects.first } - - subject { cluster.find_or_initialize_kubernetes_namespace_for_project(project) } - - context 'kubernetes namespace exists' do - context 'with no service account token' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, project: project, cluster: cluster) } + describe '#kubernetes_namespace_for' do + let(:cluster) { create(:cluster, :group) } + let(:environment) { create(:environment) } - it { is_expected.to eq kubernetes_namespace } - end - - context 'with a service account token' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, project: project, cluster: cluster) } - - it { is_expected.to eq kubernetes_namespace } - end - end + subject { cluster.kubernetes_namespace_for(environment) } - context 'kubernetes namespace does not exist' do - it 'initializes a new namespace and sets default values' do - expect(subject).to be_new_record - expect(subject.project).to eq project - expect(subject.cluster).to eq cluster - expect(subject.namespace).to be_present - expect(subject.service_account_name).to be_present - end + 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 custom scope is provided' do - let(:scope) { cluster.kubernetes_namespaces.has_service_account_token } - - subject { cluster.find_or_initialize_kubernetes_namespace_for_project(project, scope: scope) } + context 'a persisted namespace exists' do + let(:persisted_namespace) { create(:cluster_kubernetes_namespace) } - context 'kubernetes namespace exists' do - context 'with no service account token' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, project: project, cluster: cluster) } - - it 'initializes a new namespace and sets default values' do - expect(subject).to be_new_record - expect(subject.project).to eq project - expect(subject.cluster).to eq cluster - expect(subject.namespace).to be_present - expect(subject.service_account_name).to be_present - end - end + it { is_expected.to eq persisted_namespace.namespace } + end - context 'with a service account token' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, project: project, cluster: cluster) } + context 'no persisted namespace exists' do + let(:persisted_namespace) { nil } + let(:namespace_generator) { double } + let(:default_namespace) { 'a-default-namespace' } - it { is_expected.to eq kubernetes_namespace } - end + 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 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 471769e4aab..0c4cf291d20 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -106,7 +106,7 @@ describe Clusters::Platforms::Kubernetes do 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 } @@ -205,192 +205,77 @@ describe Clusters::Platforms::Kubernetes do 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) } - - context 'kubernetes namespace is present but has no service account token' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster) } - - it { is_expected.to eq(namespace) } - end - end - - context 'with no namespace assigned' do - let(:namespace) { nil } - - context 'when kubernetes namespace is present' do - let(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster) } - - before do - kubernetes_namespace - end - - it { is_expected.to eq(kubernetes_namespace.namespace) } - - context 'kubernetes namespace has no service account token' do - before do - kubernetes_namespace.update!(namespace: 'old-namespace', service_account_token: nil) - end + describe '#predefined_variables' do + 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) } - it { is_expected.to eq("#{project.path}-#{project.id}") } - end - end + let(:environment_name) { 'env/production' } + let(:environment_slug) { Gitlab::Slug::Environment.new(environment_name).generate } - context 'when kubernetes namespace is not present' do - it { is_expected.to eq("#{project.path}-#{project.id}") } - end - end - end + subject { platform.predefined_variables(project: project, environment_name: environment_name) } - 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 + 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, 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 'does not set KUBE_TOKEN' do - expect(subject).not_to include( - { key: 'KUBE_TOKEN', value: kubernetes.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 'kubernetes namespace is created with service account token' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster) } - - it_behaves_like 'setting variables' + context 'kubernetes namespace exists' do + let(:variable) { Hash(key: :fake_key, value: 'fake_value') } + let(:namespace_variables) { Gitlab::Ci::Variables::Collection.new([variable]) } - it 'sets KUBE_TOKEN' do - expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false, masked: true } - ) + before do + expect(persisted_namespace).to receive(:predefined_variables).and_return(namespace_variables) end - context 'the cluster has been set to unmanaged after the namespace was created' do - before do - cluster.update!(managed: false) - end - - it_behaves_like 'setting variables' - - it 'sets KUBE_TOKEN from the platform' do - expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes.token, public: false, masked: true } - ) - end - - context 'the platform has a custom namespace set' do - before do - kubernetes.update!(namespace: 'custom-namespace') - end - - it 'sets KUBE_NAMESPACE from the platform' do - expect(subject).to include( - { key: 'KUBE_NAMESPACE', value: kubernetes.namespace, public: true, masked: false } - ) - end - end - - context 'there is no namespace specified on the platform' do - let(:project) { cluster.project } - - before do - kubernetes.update!(namespace: nil) - end - - it 'sets KUBE_NAMESPACE to a default for the project' do - expect(subject).to include( - { key: 'KUBE_NAMESPACE', value: "#{project.path}-#{project.id}", public: true, masked: false } - ) - end - end - 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' - - 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) } + context 'kubernetes namespace does not exist' do + let(:persisted_namespace) { nil } + let(:namespace) { 'kubernetes-namespace' } + let(:kubeconfig) { 'kubeconfig' } - 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 @@ -410,7 +295,7 @@ describe Clusters::Platforms::Kubernetes do 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) } let(:pods) { [pod, pod, pod_with_no_terminal, kube_pod(environment_slug: "should-be-filtered-out")] } 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/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index 9e7106281ee..76da42cf243 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -82,16 +82,6 @@ describe Awardable do end end - describe "#toggle_award_emoji" do - it "adds an emoji if it isn't awarded yet" do - expect { issue.toggle_award_emoji("thumbsup", award_emoji.user) }.to change { AwardEmoji.count }.by(1) - end - - it "toggles already awarded emoji" do - expect { issue.toggle_award_emoji("thumbsdown", award_emoji.user) }.to change { AwardEmoji.count }.by(-1) - end - end - describe 'querying award_emoji on an Awardable' do let(:issue) { create(:issue) } 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 eeacdadab9c..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 @@ -202,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/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb index e2fc8a5d127..27f535487c8 100644 --- a/spec/models/concerns/deployment_platform_spec.rb +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -5,7 +5,7 @@ require 'rails_helper' describe DeploymentPlatform do let(:project) { create(:project) } - shared_examples '#deployment_platform' do + describe '#deployment_platform' do subject { project.deployment_platform } context 'with no Kubernetes configuration on CI/CD, no Kubernetes Service' do @@ -45,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 @@ -59,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 @@ -84,20 +82,4 @@ describe DeploymentPlatform do end end end - - context 'legacy implementation' do - before do - stub_feature_flags(clusters_cte: false) - end - - include_examples '#deployment_platform' - end - - context 'CTE implementation' do - before do - stub_feature_flags(clusters_cte: true) - end - - include_examples '#deployment_platform' - 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 68224a56515..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) @@ -774,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/project_api_compatibility_spec.rb b/spec/models/concerns/project_api_compatibility_spec.rb index 8cecd4fe7bc..f5722f88aac 100644 --- a/spec/models/concerns/project_api_compatibility_spec.rb +++ b/spec/models/concerns/project_api_compatibility_spec.rb @@ -16,23 +16,44 @@ describe ProjectAPICompatibility do expect(project.build_allow_git_fetch).to eq(false) end - # auto_devops_enabled - it "converts auto_devops_enabled=false to auto_devops_enabled?=false" do - expect(project.auto_devops_enabled?).to eq(true) - project.update!(auto_devops_enabled: false) - expect(project.auto_devops_enabled?).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) - it "converts auto_devops_enabled=true to auto_devops_enabled?=true" do - expect(project.auto_devops_enabled?).to eq(true) - project.update!(auto_devops_enabled: true) - expect(project.auto_devops_enabled?).to eq(true) + expect(project.auto_devops_enabled?).to eq(expected) + end + end end - # auto_devops_deploy_strategy - it "converts auto_devops_deploy_strategy=timed_incremental to auto_devops.deploy_strategy=timed_incremental" do - expect(project.auto_devops).to be_nil - project.update!(auto_devops_deploy_strategy: 'timed_incremental') - expect(project.auto_devops.deploy_strategy).to eq('timed_incremental') + 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/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/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/deployment_metrics_spec.rb b/spec/models/deployment_metrics_spec.rb index 0aadb1f3a5e..7c574a8b6c8 100644 --- a/spec/models/deployment_metrics_spec.rb +++ b/spec/models/deployment_metrics_spec.rb @@ -49,18 +49,6 @@ describe DeploymentMetrics do it { is_expected.to be_truthy } end - - context 'fallback deployment platform' do - let(:cluster) { create(:cluster, :provided_by_user, environment_scope: '*', projects: [deployment.project]) } - let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } - - before do - expect(deployment.project).to receive(:deployment_platform).and_return(cluster.platform) - expect(cluster.application_prometheus).to receive(:can_query?).and_return(true) - end - - it { is_expected.to be_truthy } - end end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index d4e631f109b..51ed8e9421b 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -322,4 +322,30 @@ describe Deployment do end end end + + describe '#deployed_by' do + it 'returns the deployment user if there is no deployable' do + deployment_user = create(:user) + deployment = create(:deployment, deployable: nil, user: deployment_user) + + expect(deployment.deployed_by).to eq(deployment_user) + end + + it 'returns the deployment user if the deployable have no user' do + deployment_user = create(:user) + build = create(:ci_build, user: nil) + deployment = create(:deployment, deployable: build, user: deployment_user) + + expect(deployment.deployed_by).to eq(deployment_user) + end + + it 'returns the deployable user if there is one' do + build_user = create(:user) + deployment_user = create(:user) + build = create(:ci_build, user: build_user) + deployment = create(:deployment, deployable: build, user: deployment_user) + + expect(deployment.deployed_by).to eq(build_user) + 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/environment_spec.rb b/spec/models/environment_spec.rb index fe4d64818b4..521c4704c87 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' 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) } @@ -574,6 +575,34 @@ describe Environment, :use_clean_rails_memory_store_caching 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 } @@ -762,32 +791,6 @@ describe Environment, :use_clean_rails_memory_store_caching 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') @@ -808,12 +811,9 @@ describe Environment, :use_clean_rails_memory_store_caching 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 @@ -823,12 +823,23 @@ describe Environment, :use_clean_rails_memory_store_caching 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 @@ -840,4 +851,35 @@ describe Environment, :use_clean_rails_memory_store_caching 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/group_spec.rb b/spec/models/group_spec.rb index 470ce65707d..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) } @@ -895,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 } @@ -994,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/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 d5b016dc8f6..2e7d78d77a8 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -871,4 +871,12 @@ describe Issue 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/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 9b0c232f370..d344a6d0f0d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1988,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 @@ -2011,6 +2012,7 @@ describe MergeRequest do .with(merge_request.id, user_id) .and_return(rebase_jid) + expect(merge_request).to receive(:expire_etag_cache) expect(merge_request).to receive(:lock!).and_call_original execute @@ -2454,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 @@ -2464,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 @@ -3002,9 +3015,6 @@ describe MergeRequest do subject { merge_request.rebase_in_progress? } it do - # Stub out the legacy gitaly implementation - allow(merge_request).to receive(:gitaly_rebase_in_progress?) { false } - allow(Gitlab::SidekiqStatus).to receive(:running?).with(rebase_jid) { jid_valid } merge_request.rebase_jid = rebase_jid @@ -3014,42 +3024,6 @@ describe MergeRequest do end end - describe '#gitaly_rebase_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) - - expect(subject.rebase_in_progress?).to be_falsey - end - - it 'returns false when the rebase directory has expired' do - time = 20.minutes.ago.to_time - File.utime(time, time, rebase_path) - - expect(subject.rebase_in_progress?).to be_falsey - end - - it 'returns false when the source project has been removed' do - allow(subject).to receive(:source_project).and_return(nil) - - expect(subject.rebase_in_progress?).to be_falsey - end - end - describe '#allow_collaboration' do let(:merge_request) do build(:merge_request, source_branch: 'fixes', allow_collaboration: true) diff --git a/spec/models/namespace/aggregation_schedule_spec.rb b/spec/models/namespace/aggregation_schedule_spec.rb index 0f1283717e0..38bf8089411 100644 --- a/spec/models/namespace/aggregation_schedule_spec.rb +++ b/spec/models/namespace/aggregation_schedule_spec.rb @@ -7,24 +7,6 @@ RSpec.describe Namespace::AggregationSchedule, :clean_gitlab_redis_shared_state, it { is_expected.to belong_to :namespace } - describe '.delay_timeout' do - context 'when timeout is set on redis' do - it 'uses personalized timeout' do - Gitlab::Redis::SharedState.with do |redis| - redis.set(described_class::REDIS_SHARED_KEY, 1.hour) - end - - expect(described_class.delay_timeout).to eq(1.hour) - end - end - - context 'when timeout is not set on redis' do - it 'uses default timeout' do - expect(described_class.delay_timeout).to eq(3.hours) - end - end - end - describe '#schedule_root_storage_statistics' do let(:namespace) { create(:namespace) } let(:aggregation_schedule) { namespace.build_aggregation_schedule } @@ -87,21 +69,5 @@ RSpec.describe Namespace::AggregationSchedule, :clean_gitlab_redis_shared_state, aggregation_schedule.schedule_root_storage_statistics end end - - context 'with a personalized lease timeout' do - before do - Gitlab::Redis::SharedState.with do |redis| - redis.set(described_class::REDIS_SHARED_KEY, 1.hour) - end - end - - it 'uses a personalized time' do - expect(Namespaces::RootStatisticsWorker) - .to receive(:perform_in) - .with(1.hour, aggregation_schedule.namespace_id) - - aggregation_schedule.save! - end - end end end diff --git a/spec/models/namespace/root_storage_statistics_spec.rb b/spec/models/namespace/root_storage_statistics_spec.rb index 3229a32234e..9e12831a704 100644 --- a/spec/models/namespace/root_storage_statistics_spec.rb +++ b/spec/models/namespace/root_storage_statistics_spec.rb @@ -8,6 +8,19 @@ RSpec.describe Namespace::RootStorageStatistics, type: :model do it { is_expected.to delegate_method(:all_projects).to(:namespace) } + context 'scopes' do + describe '.for_namespace_ids' do + it 'returns only requested namespaces' do + stats = create_list(:namespace_root_storage_statistics, 3) + namespace_ids = stats[0..1].map { |s| s.namespace_id } + + requested_stats = described_class.for_namespace_ids(namespace_ids).pluck(:namespace_id) + + expect(requested_stats).to eq(namespace_ids) + end + end + end + describe '#recalculate!' do let(:namespace) { create(:group) } let(:root_storage_statistics) { create(:namespace_root_storage_statistics, namespace: namespace) } @@ -56,7 +69,7 @@ RSpec.describe Namespace::RootStorageStatistics, type: :model do it_behaves_like 'data refresh' - context 'with subgroups', :nested_groups do + context 'with subgroups' do let(:subgroup1) { create(:group, parent: namespace)} let(:subgroup2) { create(:group, parent: subgroup1)} diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index f908f3504e0..972f26ac745 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -191,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) } @@ -271,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) } @@ -475,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) } @@ -490,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) } @@ -504,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) } @@ -518,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) } @@ -534,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) } @@ -550,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) } @@ -597,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 )} @@ -738,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) @@ -853,4 +853,64 @@ describe Namespace 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 661957cf08b..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 diff --git a/spec/models/postgresql/replication_slot_spec.rb b/spec/models/postgresql/replication_slot_spec.rb index 95ae204a8a8..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) 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 d5b0f94f461..74c85a13c88 100644 --- a/spec/models/project_services/bugzilla_service_spec.rb +++ b/spec/models/project_services/bugzilla_service_spec.rb @@ -44,7 +44,9 @@ describe BugzillaService do # 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) { create(:bugzilla_service, properties: properties) } + let(:service) do + create(:bugzilla_service, :without_properties_callback, properties: properties) + end include_examples 'issue tracker fields' end @@ -60,7 +62,7 @@ describe BugzillaService do 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, title: title, description: description, properties: properties) + create(:bugzilla_service, :without_properties_callback, title: title, description: description, properties: properties) end include_examples 'issue tracker fields' 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 56b0bda6626..5259357a254 100644 --- a/spec/models/project_services/custom_issue_tracker_service_spec.rb +++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb @@ -58,7 +58,9 @@ describe CustomIssueTrackerService do # 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) { create(:custom_issue_tracker_service, properties: properties) } + let(:service) do + create(:custom_issue_tracker_service, :without_properties_callback, properties: properties) + end include_examples 'issue tracker fields' end @@ -74,7 +76,7 @@ describe CustomIssueTrackerService do 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, title: title, description: description, properties: properties) + create(:custom_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties) end include_examples 'issue tracker fields' 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 a3726f09dc5..0c4fc290a13 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -61,7 +61,9 @@ describe GitlabIssueTrackerService do # 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) { create(:gitlab_issue_tracker_service, properties: properties) } + let(:service) do + create(:gitlab_issue_tracker_service, :without_properties_callback, properties: properties) + end include_examples 'issue tracker fields' end @@ -77,7 +79,7 @@ describe GitlabIssueTrackerService do 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, title: title, description: description, properties: properties) + create(:gitlab_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties) end include_examples 'issue tracker fields' diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 9b122d85293..02060699e9a 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -145,7 +145,9 @@ describe JiraService do # 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) { create(:jira_service, properties: properties) } + let(:service) do + create(:jira_service, :without_properties_callback, properties: properties) + end include_examples 'issue tracker fields' end @@ -161,7 +163,7 @@ describe JiraService do 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, title: title, description: description, properties: properties) + create(:jira_service, :without_properties_callback, title: title, description: description, properties: properties) end include_examples 'issue tracker fields' @@ -234,7 +236,7 @@ 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 @@ -604,6 +606,12 @@ describe JiraService do 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 @@ -619,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/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb deleted file mode 100644 index d33bbb0470f..00000000000 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ /dev/null @@ -1,167 +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) { create(:kubernetes_service, project: project) } - - 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 - subject.skip_deprecation_validation = 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.skip_deprecation_validation = 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 disabled/) - end - end - - context 'with an active and deprecated service' do - let(:kubernetes_service) { create(:kubernetes_service) } - - before do - kubernetes_service.skip_deprecation_validation = false - 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 - 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 "#deprecated?" do - let(:kubernetes_service) { create(:kubernetes_service) } - - it 'returns true' do - expect(kubernetes_service.deprecated?).to be_truthy - 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 disabled/) - 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 not used by GitLab/) - 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 3ffe633868f..73c20359091 100644 --- a/spec/models/project_services/microsoft_teams_service_spec.rb +++ b/spec/models/project_services/microsoft_teams_service_spec.rb @@ -292,7 +292,8 @@ describe MicrosoftTeamsService do context 'when disabled' do let(:pipeline) do - create(:ci_pipeline, :failed, project: project, ref: 'not-the-default-branch') + create(:ci_pipeline, :failed, project: project, + sha: project.commit.sha, ref: 'not-the-default-branch') end before do 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 806e3695962..c1ee6546b12 100644 --- a/spec/models/project_services/redmine_service_spec.rb +++ b/spec/models/project_services/redmine_service_spec.rb @@ -50,7 +50,9 @@ describe RedmineService do # 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) { create(:redmine_service, properties: properties) } + let(:service) do + create(:redmine_service, :without_properties_callback, properties: properties) + end include_examples 'issue tracker fields' end @@ -66,7 +68,7 @@ describe RedmineService do 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, title: title, description: description, properties: properties) + create(:redmine_service, :without_properties_callback, title: title, description: description, properties: properties) end include_examples 'issue tracker fields' diff --git a/spec/models/project_services/youtrack_service_spec.rb b/spec/models/project_services/youtrack_service_spec.rb index b47ef6702b4..c48bf487af0 100644 --- a/spec/models/project_services/youtrack_service_spec.rb +++ b/spec/models/project_services/youtrack_service_spec.rb @@ -47,7 +47,9 @@ describe YoutrackService do # 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) { create(:youtrack_service, properties: properties) } + let(:service) do + create(:youtrack_service, :without_properties_callback, properties: properties) + end include_examples 'issue tracker fields' end @@ -63,7 +65,7 @@ describe YoutrackService do 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, title: title, description: description, properties: properties) + create(:youtrack_service, :without_properties_callback, title: title, description: description, properties: properties) end include_examples 'issue tracker fields' diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1bc092fa41a..bd352db2236 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -98,6 +98,7 @@ 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) @@ -173,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) } @@ -213,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 @@ -1174,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) } @@ -1186,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' + 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 { 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) } @@ -1675,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) @@ -2019,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 + subject { project.latest_successful_build_for_ref(build_name) } - context 'with succeeded pipeline' do - let!(:build) { create_build } + context 'with a specified ref' do + let(:build) { create_build } - 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 + subject { project.latest_successful_build_for_ref(build.name, project.default_branch) } - it 'returns empty relation if the build cannot be found' do - expect(project.latest_successful_build_for('TAIL')) - .to be_nil - end - end - - 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) } @@ -2087,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 @@ -2097,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 @@ -2113,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 @@ -2124,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 @@ -2292,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) } @@ -2331,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) } @@ -2340,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) } @@ -2479,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 @@ -2634,45 +2661,33 @@ describe Project do end describe '#deployment_variables' do - context 'when project has no deployment service' do - let(:project) { create(:project) } + let(:project) { create(:project) } + let(:environment) { 'production' } - it 'returns an empty array' do - expect(project.deployment_variables).to eq [] - end + subject { project.deployment_variables(environment: environment) } + + before do + expect(project).to receive(:deployment_platform).with(environment: environment) + .and_return(deployment_platform) end - context 'when project uses mock deployment service' do - let(:project) { create(:mock_deployment_project) } + context 'when project has no deployment platform' do + let(:deployment_platform) { nil } - it 'returns an empty array' do - expect(project.deployment_variables).to eq [] - end + it { is_expected.to eq [] } end - context 'when project has a deployment service' do - 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 'does not return variables from this service' do - expect(project.deployment_variables).not_to include( - { key: 'KUBE_TOKEN', value: project.deployment_platform.token, public: false, masked: true } - ) - end + 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 @@ -2700,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 @@ -2747,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 @@ -2997,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 @@ -3117,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 @@ -3804,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 @@ -3824,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 @@ -4036,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 @@ -4064,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 @@ -4251,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) } @@ -4267,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 @@ -4733,35 +4877,22 @@ describe Project do describe '#git_objects_poolable?' do subject { project } - - context 'when the feature flag is turned off' do - before do - stub_feature_flags(object_pools: false) - end - - let(:project) { create(:project, :repository, :public) } + context 'when not using hashed storage' do + let(:project) { create(:project, :legacy_storage, :public, :repository) } it { is_expected.not_to be_git_objects_poolable } end - context 'when the feature flag is enabled' do - context 'when not using hashed storage' do - let(:project) { create(:project, :legacy_storage, :public, :repository) } - - it { is_expected.not_to be_git_objects_poolable } - end - - context 'when the project is not public' do - let(:project) { create(:project, :private) } + context 'when the project is not public' do + let(:project) { create(:project, :private) } - it { is_expected.not_to be_git_objects_poolable } - end + it { is_expected.not_to be_git_objects_poolable } + end - context 'when objects are poolable' do - let(:project) { create(:project, :repository, :public) } + context 'when objects are poolable' do + let(:project) { create(:project, :repository, :public) } - it { is_expected.to be_git_objects_poolable } - 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 db3e4902c64..a164ed9bbea 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -140,18 +140,7 @@ describe ProjectStatistics do let(:namespace) { create(:group) } let(:project) { create(:project, namespace: namespace) } - context 'when the feature flag is off' do - it 'does not schedule the aggregation worker' do - stub_feature_flags(update_statistics_namespace: false, namespace: namespace) - - expect(Namespaces::ScheduleAggregationWorker) - .not_to receive(:perform_async) - - statistics.refresh!(only: [:lfs_objects_size]) - end - end - - context 'when the feature flag is on' do + context 'when arguments are passed' do it 'schedules the aggregation worker' do expect(Namespaces::ScheduleAggregationWorker) .to receive(:perform_async) 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/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 3d967aa4ab8..79395fcc994 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1223,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 } + + subject { repository.tag_exists?(tag) } + + it 'delegates to tag_names when the cache is empty' do + repository.expire_tags_cache - expect(repository.tag_exists?('foobar')).to eq(true) - expect(repository.tag_exists?('master')).to eq(false) + 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 @@ -1744,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 @@ -1781,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 @@ -1789,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 @@ -2426,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/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 a4d177da0be..8338d2b5b39 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -103,6 +103,14 @@ describe User do it { is_expected.to validate_length_of(:name).is_at_most(128) } end + describe 'first name' do + it { is_expected.to validate_length_of(:first_name).is_at_most(255) } + end + + describe 'last name' do + it { is_expected.to validate_length_of(:last_name).is_at_most(255) } + end + describe 'username' do it 'validates presence' do expect(subject).to validate_presence_of(:username) @@ -530,6 +538,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) } @@ -667,6 +686,18 @@ describe User do end end + describe 'name getters' do + let(:user) { create(:user, name: 'Kane Martin William') } + + it 'derives first name from full name, if not present' do + expect(user.first_name).to eq('Kane') + end + + it 'derives last name from full name, if not present' do + expect(user.last_name).to eq('Martin William') + end + end + describe '#highest_role' do let(:user) { create(:user) } @@ -783,6 +814,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 +1023,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 +1176,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 @@ -2070,11 +2120,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 @@ -2374,7 +2420,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) } @@ -2478,22 +2524,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 @@ -2506,7 +2546,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 @@ -2519,7 +2559,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 @@ -2675,7 +2715,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 } @@ -2690,7 +2730,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 } @@ -2931,7 +2971,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 @@ -2942,16 +2982,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 @@ -3484,4 +3532,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 |