diff options
Diffstat (limited to 'spec/models')
40 files changed, 992 insertions, 256 deletions
diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 35415030154..ec2e7d672f0 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -26,4 +26,34 @@ describe Appearance do let(:uploader_class) { AttachmentUploader } end end + + shared_examples 'logo paths' do |logo_type| + let(:appearance) { create(:appearance, "with_#{logo_type}".to_sym) } + let(:filename) { 'dk.png' } + let(:expected_path) { "/uploads/-/system/appearance/#{logo_type}/#{appearance.id}/#{filename}" } + + it 'returns nil when there is no upload' do + expect(subject.send("#{logo_type}_path")).to be_nil + end + + it 'returns a local path using the system route' do + expect(appearance.send("#{logo_type}_path")).to eq(expected_path) + end + + describe 'with asset host configured' do + let(:asset_host) { 'https://gitlab-assets.example.com' } + + before do + allow(ActionController::Base).to receive(:asset_host) { asset_host } + end + + it 'returns a full URL with the system path' do + expect(appearance.send("#{logo_type}_path")).to eq("#{asset_host}#{expected_path}") + end + end + end + + %i(logo header_logo favicon).each do |logo_type| + it_behaves_like 'logo paths', logo_type + end end diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index e8c03b587e2..05cf242e84d 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -122,14 +122,14 @@ describe Blob do end end - describe '#raw_binary?' do + describe '#binary?' do context 'if the blob is stored externally' do context 'if the extension has a rich viewer' do context 'if the viewer is binary' do it 'returns true' do blob = fake_blob(path: 'file.pdf', lfs: true) - expect(blob.raw_binary?).to be_truthy + expect(blob.binary?).to be_truthy end end @@ -137,7 +137,7 @@ describe Blob do it 'return false' do blob = fake_blob(path: 'file.md', lfs: true) - expect(blob.raw_binary?).to be_falsey + expect(blob.binary?).to be_falsey end end end @@ -148,7 +148,7 @@ describe Blob do it 'returns false' do blob = fake_blob(path: 'file.txt', lfs: true) - expect(blob.raw_binary?).to be_falsey + expect(blob.binary?).to be_falsey end end @@ -156,7 +156,7 @@ describe Blob do it 'returns false' do blob = fake_blob(path: 'file.ics', lfs: true) - expect(blob.raw_binary?).to be_falsey + expect(blob.binary?).to be_falsey end end end @@ -166,7 +166,7 @@ describe Blob do it 'returns false' do blob = fake_blob(path: 'file.rb', lfs: true) - expect(blob.raw_binary?).to be_falsey + expect(blob.binary?).to be_falsey end end @@ -174,7 +174,7 @@ describe Blob do it 'returns true' do blob = fake_blob(path: 'file.exe', lfs: true) - expect(blob.raw_binary?).to be_truthy + expect(blob.binary?).to be_truthy end end end @@ -184,7 +184,7 @@ describe Blob do it 'returns false' do blob = fake_blob(path: 'file.ini', lfs: true) - expect(blob.raw_binary?).to be_falsey + expect(blob.binary?).to be_falsey end end @@ -192,7 +192,7 @@ describe Blob do it 'returns true' do blob = fake_blob(path: 'file.wtf', lfs: true) - expect(blob.raw_binary?).to be_truthy + expect(blob.binary?).to be_truthy end end end @@ -204,7 +204,7 @@ describe Blob do it 'returns true' do blob = fake_blob(path: 'file.pdf', binary: true) - expect(blob.raw_binary?).to be_truthy + expect(blob.binary?).to be_truthy end end @@ -212,7 +212,7 @@ describe Blob do it 'return false' do blob = fake_blob(path: 'file.md') - expect(blob.raw_binary?).to be_falsey + expect(blob.binary?).to be_falsey end end end diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index d6e5b557870..89839709131 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -49,7 +49,7 @@ describe BroadcastMessage do it 'caches the output of the query' do create(:broadcast_message) - expect(described_class).to receive(:where).and_call_original.once + expect(described_class).to receive(:current_and_future_messages).and_call_original.once described_class.current @@ -93,27 +93,6 @@ describe BroadcastMessage do expect(Rails.cache).to receive(:delete).with(described_class::LEGACY_CACHE_KEY) expect(described_class.current.length).to eq(0) end - - it 'gracefully handles bad cache entry' do - allow(described_class).to receive(:current_and_future_messages).and_return('{') - - expect(described_class.current).to be_empty - end - - it 'gracefully handles an empty hash' do - allow(described_class).to receive(:current_and_future_messages).and_return('{}') - - expect(described_class.current).to be_empty - end - - it 'gracefully handles unknown attributes' do - message = create(:broadcast_message) - - allow(described_class).to receive(:current_and_future_messages) - .and_return([{ bad_attr: 1 }, message]) - - expect(described_class.current).to eq([message]) - end end describe '#active?' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 89f78f629d4..7baf4d93804 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2114,6 +2114,7 @@ describe Ci::Build do { key: 'CI_JOB_NAME', value: 'test', public: true }, { key: 'CI_JOB_STAGE', value: 'test', public: true }, { key: 'CI_COMMIT_SHA', value: build.sha, public: true }, + { key: 'CI_COMMIT_SHORT_SHA', value: build.short_sha, public: true }, { key: 'CI_COMMIT_BEFORE_SHA', value: build.before_sha, public: true }, { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true }, { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true }, @@ -2385,6 +2386,8 @@ describe Ci::Build do end context 'when protected variable is defined' do + let(:ref) { Gitlab::Git::BRANCH_REF_PREFIX + build.ref } + let(:protected_variable) do { key: 'PROTECTED_KEY', value: 'protected_value', public: false } end @@ -2397,7 +2400,7 @@ describe Ci::Build do context 'when the branch is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -2405,7 +2408,7 @@ describe Ci::Build do context 'when the tag is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -2430,6 +2433,8 @@ describe Ci::Build do end context 'when group protected variable is defined' do + let(:ref) { Gitlab::Git::BRANCH_REF_PREFIX + build.ref } + let(:protected_variable) do { key: 'PROTECTED_KEY', value: 'protected_value', public: false } end @@ -2442,7 +2447,7 @@ describe Ci::Build do context 'when the branch is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -2450,7 +2455,7 @@ describe Ci::Build do context 'when the tag is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -2725,6 +2730,7 @@ describe Ci::Build do it 'returns static predefined variables' do keys = %w[CI_JOB_NAME CI_COMMIT_SHA + CI_COMMIT_SHORT_SHA CI_COMMIT_REF_NAME CI_COMMIT_REF_SLUG CI_JOB_STAGE] diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b67c6a4cffa..17f33785fda 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -397,6 +397,10 @@ describe Ci::Pipeline, :mailer do end describe '#protected_ref?' do + before do + pipeline.project = create(:project, :repository) + end + it 'delegates method to project' do expect(pipeline).not_to be_protected_ref end diff --git a/spec/models/clusters/applications/cert_manager_spec.rb b/spec/models/clusters/applications/cert_manager_spec.rb index 170c6001eaf..8e14abe098d 100644 --- a/spec/models/clusters/applications/cert_manager_spec.rb +++ b/spec/models/clusters/applications/cert_manager_spec.rb @@ -14,7 +14,7 @@ describe Clusters::Applications::CertManager do let(:application) { create(:clusters_applications_cert_managers, :scheduled, version: 'v0.4.0') } it 'updates the application version' do - expect(application.reload.version).to eq('v0.5.0') + expect(application.reload.version).to eq('v0.5.2') end end end @@ -28,8 +28,8 @@ describe Clusters::Applications::CertManager do it 'should be initialized with cert_manager arguments' do expect(subject.name).to eq('certmanager') expect(subject.chart).to eq('stable/cert-manager') - expect(subject.version).to eq('v0.5.0') - expect(subject).not_to be_rbac + 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']) end @@ -45,19 +45,19 @@ describe Clusters::Applications::CertManager do end end - context 'on a rbac enabled cluster' do + context 'on a non rbac enabled cluster' do before do - cert_manager.cluster.platform_kubernetes.rbac! + cert_manager.cluster.platform_kubernetes.abac! end - it { is_expected.to be_rbac } + it { is_expected.not_to be_rbac } end context 'application failed to install previously' do let(:cert_manager) { create(:clusters_applications_cert_managers, :errored, version: '0.0.1') } it 'should be initialized with the locked version' do - expect(subject.version).to eq('v0.5.0') + expect(subject.version).to eq('v0.5.2') end end end diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index 2c37cd20ecc..64f6d9c8bb4 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -49,16 +49,16 @@ describe Clusters::Applications::Helm do end describe 'rbac' do - context 'non rbac cluster' do - it { expect(subject).not_to be_rbac } + context 'rbac cluster' do + it { expect(subject).to be_rbac } end - context 'rbac cluster' do + context 'non rbac cluster' do before do - helm.cluster.platform_kubernetes.rbac! + helm.cluster.platform_kubernetes.abac! end - it { expect(subject).to be_rbac } + it { expect(subject).not_to be_rbac } end end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index cd28f1fe9c6..de313a8ca36 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -91,16 +91,16 @@ describe Clusters::Applications::Ingress do expect(subject.name).to eq('ingress') expect(subject.chart).to eq('stable/nginx-ingress') expect(subject.version).to eq('0.23.0') - expect(subject).not_to be_rbac + expect(subject).to be_rbac expect(subject.files).to eq(ingress.files) end - context 'on a rbac enabled cluster' do + context 'on a non rbac enabled cluster' do before do - ingress.cluster.platform_kubernetes.rbac! + ingress.cluster.platform_kubernetes.abac! end - it { is_expected.to be_rbac } + it { is_expected.not_to be_rbac } end context 'application failed to install previously' do diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index a40edbf267b..391e5425384 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -52,17 +52,17 @@ describe Clusters::Applications::Jupyter do expect(subject.name).to eq('jupyter') expect(subject.chart).to eq('jupyter/jupyterhub') expect(subject.version).to eq('v0.6') - expect(subject).not_to be_rbac + expect(subject).to be_rbac expect(subject.repository).to eq('https://jupyterhub.github.io/helm-chart/') expect(subject.files).to eq(jupyter.files) end - context 'on a rbac enabled cluster' do + context 'on a non rbac enabled cluster' do before do - jupyter.cluster.platform_kubernetes.rbac! + jupyter.cluster.platform_kubernetes.abac! end - it { is_expected.to be_rbac } + it { is_expected.not_to be_rbac } end context 'application failed to install previously' do diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index a1579b90436..8fc755d2a26 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -33,10 +33,10 @@ describe Clusters::Applications::Knative do end context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_knative, :scheduled, version: '0.1.3') } + let(:application) { create(:clusters_applications_knative, :scheduled, version: '0.2.2') } it 'updates the application version' do - expect(application.reload.version).to eq('0.1.3') + expect(application.reload.version).to eq('0.2.2') end end end @@ -105,9 +105,26 @@ describe Clusters::Applications::Knative do it 'should be initialized with knative arguments' do expect(subject.name).to eq('knative') expect(subject.chart).to eq('knative/knative') - expect(subject.version).to eq('0.1.3') + expect(subject.version).to eq('0.2.2') expect(subject.files).to eq(knative.files) end + + it 'should not install metrics for prometheus' do + expect(subject.postinstall).to be_nil + end + + context 'with prometheus installed' do + let(:prometheus) { create(:clusters_applications_prometheus, :installed) } + let(:knative) { create(:clusters_applications_knative, cluster: prometheus.cluster) } + + subject { knative.install_command } + + it 'should install metrics' do + expect(subject.postinstall).not_to be_nil + expect(subject.postinstall.length).to be(1) + expect(subject.postinstall[0]).to eql("kubectl apply -f #{Clusters::Applications::Knative::METRICS_CONFIG}") + end + end end describe '#files' do diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 893ed3e3f64..de6b844023a 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -161,16 +161,16 @@ describe Clusters::Applications::Prometheus do expect(subject.name).to eq('prometheus') expect(subject.chart).to eq('stable/prometheus') expect(subject.version).to eq('6.7.3') - expect(subject).not_to be_rbac + expect(subject).to be_rbac expect(subject.files).to eq(prometheus.files) end - context 'on a rbac enabled cluster' do + context 'on a non rbac enabled cluster' do before do - prometheus.cluster.platform_kubernetes.rbac! + prometheus.cluster.platform_kubernetes.abac! end - it { is_expected.to be_rbac } + it { is_expected.not_to be_rbac } end context 'application failed to install previously' do @@ -180,6 +180,21 @@ describe Clusters::Applications::Prometheus do expect(subject.version).to eq('6.7.3') end end + + it 'should not install knative metrics' do + expect(subject.postinstall).to be_nil + end + + context 'with knative installed' do + let(:knative) { create(:clusters_applications_knative, :installed ) } + let(:prometheus) { create(:clusters_applications_prometheus, cluster: knative.cluster) } + + subject { prometheus.install_command } + + it 'should install knative metrics' do + expect(subject.postinstall).to include("kubectl apply -f #{Clusters::Applications::Knative::METRICS_CONFIG}") + end + end end describe '#files' do diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 47daa79873e..3d0735c6d0b 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 let(:application) { create(:clusters_applications_runner, :scheduled, version: '0.1.30') } it 'updates the application version' do - expect(application.reload.version).to eq('0.1.39') + expect(application.reload.version).to eq('0.1.43') end end end @@ -46,25 +46,25 @@ describe Clusters::Applications::Runner do it 'should be initialized with 4 arguments' do expect(subject.name).to eq('runner') expect(subject.chart).to eq('runner/gitlab-runner') - expect(subject.version).to eq('0.1.39') - expect(subject).not_to be_rbac + expect(subject.version).to eq('0.1.43') + expect(subject).to be_rbac expect(subject.repository).to eq('https://charts.gitlab.io') expect(subject.files).to eq(gitlab_runner.files) end - context 'on a rbac enabled cluster' do + context 'on a non rbac enabled cluster' do before do - gitlab_runner.cluster.platform_kubernetes.rbac! + gitlab_runner.cluster.platform_kubernetes.abac! end - it { is_expected.to be_rbac } + it { is_expected.not_to be_rbac } end context 'application failed to install previously' do let(:gitlab_runner) { create(:clusters_applications_runner, :errored, runner: ci_runner, version: '0.1.13') } it 'should be initialized with the locked version' do - expect(subject.version).to eq('0.1.39') + expect(subject.version).to eq('0.1.43') end end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 840f74c9890..f447e64b029 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -29,6 +29,7 @@ describe Clusters::Cluster do it { is_expected.to delegate_method(:available?).to(:application_helm).with_prefix } it { is_expected.to delegate_method(:available?).to(:application_ingress).with_prefix } it { is_expected.to delegate_method(:available?).to(:application_prometheus).with_prefix } + it { is_expected.to delegate_method(:available?).to(:application_knative).with_prefix } it { is_expected.to respond_to :project } diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index 062d2fd0768..6c8a223092e 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -154,19 +154,11 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end describe '#rbac?' do - subject { kubernetes.rbac? } - let(:kubernetes) { build(:cluster_platform_kubernetes, :configured) } - context 'when authorization type is rbac' do - let(:kubernetes) { build(:cluster_platform_kubernetes, :rbac_enabled, :configured) } - - it { is_expected.to be_truthy } - end + subject { kubernetes.rbac? } - context 'when authorization type is nil' do - it { is_expected.to be_falsey } - end + it { is_expected.to be_truthy } end describe '#actual_namespace' do @@ -325,12 +317,13 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching context 'with valid pods' do let(:pod) { kube_pod(app: environment.slug) } + let(:pod_with_no_terminal) { kube_pod(app: environment.slug, status: "Pending") } let(:terminals) { kube_terminals(service, pod) } before do stub_reactive_cache( service, - pods: [pod, pod, kube_pod(app: "should-be-filtered-out")] + pods: [pod, pod, pod_with_no_terminal, kube_pod(app: "should-be-filtered-out")] ) end @@ -394,7 +387,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching context 'when namespace is updated' do it 'should call ConfigureWorker' do - expect(ClusterPlatformConfigureWorker).to receive(:perform_async).with(cluster.id).once + expect(ClusterConfigureWorker).to receive(:perform_async).with(cluster.id).once platform.namespace = 'new-namespace' platform.save @@ -403,7 +396,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching context 'when namespace is not updated' do it 'should not call ConfigureWorker' do - expect(ClusterPlatformConfigureWorker).not_to receive(:perform_async) + expect(ClusterConfigureWorker).not_to receive(:perform_async) platform.username = "new-username" platform.save diff --git a/spec/models/clusters/providers/gcp_spec.rb b/spec/models/clusters/providers/gcp_spec.rb index d134608b538..5012e6f15c6 100644 --- a/spec/models/clusters/providers/gcp_spec.rb +++ b/spec/models/clusters/providers/gcp_spec.rb @@ -79,17 +79,7 @@ describe Clusters::Providers::Gcp do subject { gcp } - it 'should default to true' do - is_expected.to be_legacy_abac - end - - context 'legacy_abac is set to false' do - let(:gcp) { build(:cluster_provider_gcp, legacy_abac: false) } - - it 'is false' do - is_expected.not_to be_legacy_abac - end - end + it { is_expected.not_to be_legacy_abac } end describe '#state_machine' do diff --git a/spec/models/concerns/avatarable_spec.rb b/spec/models/concerns/avatarable_spec.rb index 7d617cb7b19..1ea7f2b9985 100644 --- a/spec/models/concerns/avatarable_spec.rb +++ b/spec/models/concerns/avatarable_spec.rb @@ -33,6 +33,43 @@ describe Avatarable do end describe '#avatar_path' do + context 'with caching enabled', :request_store do + let!(:avatar_path) { [relative_url_root, project.avatar.local_url].join } + let!(:avatar_url) { [gitlab_host, relative_url_root, project.avatar.local_url].join } + + it 'only calls local_url once' do + expect(project.avatar).to receive(:local_url).once.and_call_original + + 2.times do + expect(project.avatar_path).to eq(avatar_path) + end + end + + it 'calls local_url twice for path and URLs' do + expect(project.avatar).to receive(:local_url).exactly(2).times.and_call_original + + expect(project.avatar_path(only_path: true)).to eq(avatar_path) + expect(project.avatar_path(only_path: false)).to eq(avatar_url) + end + + it 'calls local_url twice for different sizes' do + expect(project.avatar).to receive(:local_url).exactly(2).times.and_call_original + + expect(project.avatar_path).to eq(avatar_path) + expect(project.avatar_path(size: 40)).to eq(avatar_path + "?width=40") + end + + it 'handles unpersisted objects' do + new_project = build(:project, :with_avatar) + path = [relative_url_root, new_project.avatar.local_url].join + expect(new_project.avatar).to receive(:local_url).exactly(2).times.and_call_original + + 2.times do + expect(new_project.avatar_path).to eq(path) + end + end + end + using RSpec::Parameterized::TableSyntax where(:has_asset_host, :visibility_level, :only_path, :avatar_path_prefix) do diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb index 827fbc9d7d5..689e7d3058f 100644 --- a/spec/models/concerns/cacheable_attributes_spec.rb +++ b/spec/models/concerns/cacheable_attributes_spec.rb @@ -20,6 +20,10 @@ describe CacheableAttributes do @_last ||= new('foo' => 'a', 'bar' => 'b') end + def self.column_names + %w[foo bar baz] + end + attr_accessor :attributes def initialize(attrs = {}, *) @@ -75,13 +79,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) + expect(minimal_test_class.build_from_defaults.attributes).to eq(minimal_test_class.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(minimal_test_class.build_from_defaults(foo: 'd').attributes['foo']).to eq('d') end end diff --git a/spec/models/concerns/chronic_duration_attribute_spec.rb b/spec/models/concerns/chronic_duration_attribute_spec.rb index b14b773b653..51221e07ca3 100644 --- a/spec/models/concerns/chronic_duration_attribute_spec.rb +++ b/spec/models/concerns/chronic_duration_attribute_spec.rb @@ -43,7 +43,7 @@ shared_examples 'ChronicDurationAttribute writer' do end it "doesn't raise exception" do - expect { subject.send("#{virtual_field}=", '-10m') }.not_to raise_error(ChronicDuration::DurationParseError) + expect { subject.send("#{virtual_field}=", '-10m') }.not_to raise_error end it "doesn't change value" do @@ -87,7 +87,7 @@ shared_examples 'ChronicDurationAttribute writer' do end it "doesn't raise exception" do - expect { subject.send("#{virtual_field}=", nil) }.not_to raise_error(NoMethodError) + expect { subject.send("#{virtual_field}=", nil) }.not_to raise_error end end end diff --git a/spec/models/concerns/discussion_on_diff_spec.rb b/spec/models/concerns/discussion_on_diff_spec.rb index 73eb7a1160d..4b16e6e3902 100644 --- a/spec/models/concerns/discussion_on_diff_spec.rb +++ b/spec/models/concerns/discussion_on_diff_spec.rb @@ -50,11 +50,17 @@ describe DiscussionOnDiff do end context "when the diff line does not exist on a legacy diff note" do + subject { create(:legacy_diff_note_on_merge_request).to_discussion } + it "returns an empty array" do - legacy_note = LegacyDiffNote.new + expect(truncated_lines).to eq([]) + end + end - allow(subject).to receive(:first_note).and_return(legacy_note) + context 'when the discussion is on an image' do + subject { create(:image_diff_note_on_merge_request).to_discussion } + it 'returns an empty array' do expect(truncated_lines).to eq([]) end end diff --git a/spec/models/concerns/has_ref_spec.rb b/spec/models/concerns/has_ref_spec.rb new file mode 100644 index 00000000000..8aed72d77a4 --- /dev/null +++ b/spec/models/concerns/has_ref_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe HasRef do + describe '#branch?' do + let(:build) { create(:ci_build) } + + subject { build.branch? } + + context 'is not a tag' do + before do + build.tag = false + end + + it 'return true when tag is set to false' do + is_expected.to be_truthy + end + end + + context 'is not a tag' do + before do + build.tag = true + end + + it 'return false when tag is set to true' do + is_expected.to be_falsey + end + end + end + + describe '#git_ref' do + subject { build.git_ref } + + context 'when tag is true' do + let(:build) { create(:ci_build, tag: true) } + + it 'returns a tag ref' do + is_expected.to start_with(Gitlab::Git::TAG_REF_PREFIX) + end + end + + context 'when tag is false' do + let(:build) { create(:ci_build, tag: false) } + + it 'returns a branch ref' do + is_expected.to start_with(Gitlab::Git::BRANCH_REF_PREFIX) + end + end + + context 'when tag is nil' do + let(:build) { create(:ci_build, tag: nil) } + + it 'returns a branch ref' do + is_expected.to start_with(Gitlab::Git::BRANCH_REF_PREFIX) + end + end + end +end diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index 8624f0daa4d..fda00a693f0 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -318,25 +318,28 @@ describe DiffNote do end end - describe "image diff notes" do - let(:path) { "files/images/any_image.png" } + describe '#supports_suggestion?' do + context 'when noteable does not support suggestions' do + it 'returns false' do + allow(subject.noteable).to receive(:supports_suggestion?) { false } - let!(:position) do - Gitlab::Diff::Position.new( - old_path: path, - new_path: path, - width: 10, - height: 10, - x: 1, - y: 1, - diff_refs: merge_request.diff_refs, - position_type: "image" - ) + expect(subject.supports_suggestion?).to be(false) + end end - describe "validations" do - subject { build(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) } + context 'when line is not suggestible' do + it 'returns false' do + allow_any_instance_of(Gitlab::Diff::Line).to receive(:suggestible?) { false } + + expect(subject.supports_suggestion?).to be(false) + end + end + end + describe "image diff notes" do + subject { build(:image_diff_note_on_merge_request, project: project, noteable: merge_request) } + + describe "validations" do it { is_expected.not_to validate_presence_of(:line_code) } it "does not validate diff line" do diff --git a/spec/models/diff_viewer/base_spec.rb b/spec/models/diff_viewer/base_spec.rb index c90b32c5d77..f4efe5a7b3a 100644 --- a/spec/models/diff_viewer/base_spec.rb +++ b/spec/models/diff_viewer/base_spec.rb @@ -58,7 +58,7 @@ describe DiffViewer::Base do context 'when the binaryness does not match' do before do - allow_any_instance_of(Blob).to receive(:binary?).and_return(true) + allow_any_instance_of(Blob).to receive(:binary_in_repo?).and_return(true) end it 'returns false' do @@ -141,4 +141,25 @@ describe DiffViewer::Base do end end end + + describe '#render_error_message' do + it 'returns nothing when no render_error' do + expect(viewer.render_error).to be_nil + expect(viewer.render_error_message).to be_nil + end + + context 'when render_error error' do + before do + allow(viewer).to receive(:render_error).and_return(:too_large) + end + + it 'returns an error message' do + expect(viewer.render_error_message).to include('it is too large') + end + + it 'includes a "view the blob" link' do + expect(viewer.render_error_message).to include('view the blob') + end + end + end end diff --git a/spec/models/diff_viewer/server_side_spec.rb b/spec/models/diff_viewer/server_side_spec.rb index 98a8f6d4cc9..86b14b6ebf3 100644 --- a/spec/models/diff_viewer/server_side_spec.rb +++ b/spec/models/diff_viewer/server_side_spec.rb @@ -32,4 +32,24 @@ describe DiffViewer::ServerSide do end end end + + describe '#render_error_reason' do + context 'when the diff file is stored externally' do + before do + allow(diff_file).to receive(:stored_externally?).and_return(true) + end + + it 'returns error message if stored in LFS' do + allow(diff_file).to receive(:external_storage).and_return(:lfs) + + expect(subject.render_error_message).to include('it is stored in LFS') + end + + it 'returns error message if stored externally' do + allow(diff_file).to receive(:external_storage).and_return(:foo) + + expect(subject.render_error_message).to include('it is stored externally') + end + end + end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 81748681528..a64720f1876 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -243,6 +243,20 @@ describe Event do expect(event.visible_to_user?(admin)).to eq true end end + + context 'private project' do + let(:project) { create(:project, :private) } + let(:target) { note_on_issue } + + it do + expect(event.visible_to_user?(non_member)).to eq false + expect(event.visible_to_user?(author)).to eq false + expect(event.visible_to_user?(assignee)).to eq false + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq true + expect(event.visible_to_user?(admin)).to eq true + end + end end context 'merge request diff note event' do @@ -265,8 +279,8 @@ describe Event do it do expect(event.visible_to_user?(non_member)).to eq false - expect(event.visible_to_user?(author)).to eq true - expect(event.visible_to_user?(assignee)).to eq true + expect(event.visible_to_user?(author)).to eq false + expect(event.visible_to_user?(assignee)).to eq false expect(event.visible_to_user?(member)).to eq true expect(event.visible_to_user?(guest)).to eq false expect(event.visible_to_user?(admin)).to eq true diff --git a/spec/models/global_milestone_spec.rb b/spec/models/global_milestone_spec.rb index b6355455c1d..62699df5611 100644 --- a/spec/models/global_milestone_spec.rb +++ b/spec/models/global_milestone_spec.rb @@ -65,56 +65,103 @@ describe GlobalMilestone do ) end - before do - projects = [ + let!(:projects) do + [ project1, project2, project3 ] - - @global_milestones = described_class.build_collection(projects, {}) end - it 'has all project milestones' do - expect(@global_milestones.count).to eq(2) + let!(:global_milestones) { described_class.build_collection(projects, {}) } + + context 'when building a collection of milestones' do + it 'has all project milestones' do + expect(global_milestones.count).to eq(6) + end + + it 'has all project milestones titles' do + expect(global_milestones.map(&:title)).to match_array(['Milestone v1.2', 'Milestone v1.2', 'Milestone v1.2', 'VD-123', 'VD-123', 'VD-123']) + end + + it 'has all project milestones' do + expect(global_milestones.size).to eq(6) + end + + it 'sorts collection by due date' do + expect(global_milestones.map(&:due_date)).to eq [milestone1_due_date, milestone1_due_date, milestone1_due_date, nil, nil, nil] + end end - it 'has all project milestones titles' do - expect(@global_milestones.map(&:title)).to match_array(['Milestone v1.2', 'VD-123']) + context 'when adding new milestones' do + it 'does not add more queries' do + control_count = ActiveRecord::QueryRecorder.new do + described_class.build_collection(projects, {}) + end.count + + create_list(:milestone, 3, project: project3) + + expect do + described_class.build_collection(projects, {}) + end.not_to exceed_all_query_limit(control_count) + end end + end + + describe '.states_count' do + context 'when the projects have milestones' do + before do + create(:closed_milestone, title: 'Active Group Milestone', project: project3) + create(:active_milestone, title: 'Active Group Milestone', project: project1) + create(:active_milestone, title: 'Active Group Milestone', project: project2) + create(:closed_milestone, title: 'Closed Group Milestone', project: project1) + create(:closed_milestone, title: 'Closed Group Milestone', project: project2) + create(:closed_milestone, title: 'Closed Group Milestone', project: project3) + create(:closed_milestone, title: 'Closed Group Milestone 4', group: group) + end + + it 'returns the quantity of global milestones and group milestones in each possible state' do + expected_count = { opened: 2, closed: 5, all: 7 } - it 'has all project milestones' do - expect(@global_milestones.map { |group_milestone| group_milestone.milestones.count }.sum).to eq(6) + count = described_class.states_count(Project.all, group) + + expect(count).to eq(expected_count) + end + + it 'returns the quantity of global milestones in each possible state' do + expected_count = { opened: 2, closed: 4, all: 6 } + + count = described_class.states_count(Project.all) + + expect(count).to eq(expected_count) + end end - it 'sorts collection by due date' do - expect(@global_milestones.map(&:due_date)).to eq [nil, milestone1_due_date] + context 'when the projects do not have milestones' do + before do + project1 + end + + it 'returns 0 as the quantity of global milestones in each state' do + expected_count = { opened: 0, closed: 0, all: 0 } + + count = described_class.states_count(Project.all) + + expect(count).to eq(expected_count) + end end end describe '#initialize' do let(:milestone1_project1) { create(:milestone, title: "Milestone v1.2", project: project1) } - let(:milestone1_project2) { create(:milestone, title: "Milestone v1.2", project: project2) } - let(:milestone1_project3) { create(:milestone, title: "Milestone v1.2", project: project3) } - - before do - milestones = - [ - milestone1_project1, - milestone1_project2, - milestone1_project3 - ] - milestones_relation = Milestone.where(id: milestones.map(&:id)) - - @global_milestone = described_class.new(milestone1_project1.title, milestones_relation) - end + subject(:global_milestone) { described_class.new(milestone1_project1) } it 'has exactly one group milestone' do - expect(@global_milestone.title).to eq('Milestone v1.2') + expect(global_milestone.title).to eq('Milestone v1.2') end it 'has all project milestones with the same title' do - expect(@global_milestone.milestones.count).to eq(3) + expect(global_milestone.milestone).to eq(milestone1_project1) end end @@ -122,7 +169,7 @@ describe GlobalMilestone do let(:milestone) { create(:milestone, title: "git / test", project: project1) } it 'strips out slashes and spaces' do - global_milestone = described_class.new(milestone.title, Milestone.where(id: milestone.id)) + global_milestone = described_class.new(milestone) expect(global_milestone.safe_title).to eq('git-test') end @@ -132,11 +179,8 @@ describe GlobalMilestone do context 'when at least one milestone is active' do it 'returns active' do title = 'Active Group Milestone' - milestones = [ - create(:active_milestone, title: title), - create(:closed_milestone, title: title) - ] - global_milestone = described_class.new(title, milestones) + + global_milestone = described_class.new(create(:active_milestone, title: title)) expect(global_milestone.state).to eq('active') end @@ -145,11 +189,8 @@ describe GlobalMilestone do context 'when all milestones are closed' do it 'returns closed' do title = 'Closed Group Milestone' - milestones = [ - create(:closed_milestone, title: title), - create(:closed_milestone, title: title) - ] - global_milestone = described_class.new(title, milestones) + + global_milestone = described_class.new(create(:closed_milestone, title: title)) expect(global_milestone.state).to eq('closed') end diff --git a/spec/models/group_milestone_spec.rb b/spec/models/group_milestone_spec.rb index b60676afc91..fcc33cd95fe 100644 --- a/spec/models/group_milestone_spec.rb +++ b/spec/models/group_milestone_spec.rb @@ -20,13 +20,36 @@ describe GroupMilestone do end describe '.build_collection' do - before do - project_milestone + let(:group) { create(:group) } + let(:project1) { create(:project, group: group) } + let(:project2) { create(:project, path: 'gitlab-ci', group: group) } + let(:project3) { create(:project, path: 'cookbook-gitlab', group: group) } + + let!(:projects) do + [ + project1, + project2, + project3 + ] end it 'returns array of milestones, each with group assigned' do milestones = described_class.build_collection(group, [project], {}) expect(milestones).to all(have_attributes(group: group)) end + + context 'when adding new milestones' do + it 'does not add more queries' do + control_count = ActiveRecord::QueryRecorder.new do + described_class.build_collection(group, projects, {}) + end.count + + create(:milestone, title: 'This title', project: project1) + + expect do + described_class.build_collection(group, projects, {}) + end.not_to exceed_all_query_limit(control_count) + end + end end end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index cbe60b3a4a5..33e984dc399 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -105,7 +105,7 @@ describe MergeRequestDiff do context 'when the raw diffs are empty' do before do - MergeRequestDiffFile.delete_all(merge_request_diff_id: diff_with_commits.id) + MergeRequestDiffFile.where(merge_request_diff_id: diff_with_commits.id).delete_all end it 'returns an empty DiffCollection' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index bf4117fbcaf..4cc3a6a3644 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -559,6 +559,57 @@ describe MergeRequest do end end + describe '#preload_discussions_diff_highlight' do + let(:merge_request) { create(:merge_request) } + + context 'with commit diff note' do + let(:other_merge_request) { create(:merge_request) } + + let!(:diff_note) do + create(:diff_note_on_commit, project: merge_request.project) + end + + let!(:other_mr_diff_note) do + create(:diff_note_on_commit, project: other_merge_request.project) + end + + it 'preloads diff highlighting' do + expect_next_instance_of(Gitlab::DiscussionsDiff::FileCollection) do |collection| + note_diff_file = diff_note.note_diff_file + + expect(collection) + .to receive(:load_highlight) + .with([note_diff_file.id]).and_call_original + end + + merge_request.preload_discussions_diff_highlight + end + end + + context 'with merge request diff note' do + let!(:unresolved_diff_note) do + create(:diff_note_on_merge_request, project: merge_request.project, noteable: merge_request) + end + + let!(:resolved_diff_note) do + create(:diff_note_on_merge_request, :resolved, project: merge_request.project, noteable: merge_request) + end + + it 'preloads diff highlighting' do + expect_next_instance_of(Gitlab::DiscussionsDiff::FileCollection) do |collection| + note_diff_file = unresolved_diff_note.note_diff_file + + expect(collection) + .to receive(:load_highlight) + .with([note_diff_file.id]) + .and_call_original + end + + merge_request.preload_discussions_diff_highlight + end + end + end + describe '#diff_size' do let(:merge_request) do build(:merge_request, source_branch: 'expand-collapse-files', target_branch: 'master') @@ -1358,7 +1409,7 @@ describe MergeRequest do it 'does not raises a NameError exception' do allow_any_instance_of(service_class_name.constantize).to receive(:execute).and_return(nil) - expect { subject }.not_to raise_error(NameError) + expect { subject }.not_to raise_error end end end @@ -2092,7 +2143,7 @@ describe MergeRequest do head_commit_sha: commit.sha ) - subject.merge_request_diff(true) + subject.reload_merge_request_diff end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index d11eb46159e..b3d31e65c85 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -316,6 +316,15 @@ describe Milestone do end end + describe '#reference_link_text' do + let(:project) { build_stubbed(:project, name: 'sample-project') } + let(:milestone) { build_stubbed(:milestone, iid: 1, project: project, name: 'milestone') } + + it 'returns the title with the reference prefix' do + expect(milestone.reference_link_text).to eq '%milestone' + end + end + describe '#participants' do let(:project) { build(:project, name: 'sample-project') } let(:milestone) { build(:milestone, iid: 1, project: project) } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 18b54cce834..475fbe56e4d 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -337,32 +337,40 @@ describe Namespace do end end - it 'updates project full path in .git/config for each project inside namespace' do - parent = create(:group, name: 'mygroup', path: 'mygroup') - subgroup = create(:group, name: 'mysubgroup', path: 'mysubgroup', parent: parent) - project_in_parent_group = create(:project, :legacy_storage, :repository, namespace: parent, name: 'foo1') - hashed_project_in_subgroup = create(:project, :repository, namespace: subgroup, name: 'foo2') - legacy_project_in_subgroup = create(:project, :legacy_storage, :repository, namespace: subgroup, name: 'foo3') - - parent.update(path: 'mygroup_new') - - # Routes are loaded when creating the projects, so we need to manually - # reload them for the below code to be aware of the above UPDATE. - [ - project_in_parent_group, - hashed_project_in_subgroup, - legacy_project_in_subgroup - ].each do |project| - project.route.reload + context 'for each project inside the namespace' do + let!(:parent) { create(:group, name: 'mygroup', path: 'mygroup') } + let!(:subgroup) { create(:group, name: 'mysubgroup', path: 'mysubgroup', parent: parent) } + let!(:project_in_parent_group) { create(:project, :legacy_storage, :repository, namespace: parent, name: 'foo1') } + let!(:hashed_project_in_subgroup) { create(:project, :repository, namespace: subgroup, name: 'foo2') } + let!(:legacy_project_in_subgroup) { create(:project, :legacy_storage, :repository, namespace: subgroup, name: 'foo3') } + + it 'updates project full path in .git/config' do + parent.update(path: 'mygroup_new') + + expect(project_rugged(project_in_parent_group).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}" + expect(project_rugged(hashed_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}" + expect(project_rugged(legacy_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}" end - expect(project_rugged(project_in_parent_group).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}" - expect(project_rugged(hashed_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}" - expect(project_rugged(legacy_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}" - end + it 'updates the project storage location' do + repository_project_in_parent_group = create(:project_repository, project: project_in_parent_group) + repository_hashed_project_in_subgroup = create(:project_repository, project: hashed_project_in_subgroup) + repository_legacy_project_in_subgroup = create(:project_repository, project: legacy_project_in_subgroup) + + parent.update(path: 'mygroup_moved') + + expect(repository_project_in_parent_group.reload.disk_path).to eq "mygroup_moved/#{project_in_parent_group.path}" + expect(repository_hashed_project_in_subgroup.reload.disk_path).to eq hashed_project_in_subgroup.disk_path + expect(repository_legacy_project_in_subgroup.reload.disk_path).to eq "mygroup_moved/mysubgroup/#{legacy_project_in_subgroup.path}" + end + + def project_rugged(project) + # Routes are loaded when creating the projects, so we need to manually + # reload them for the below code to be aware of the above UPDATE. + project.route.reload - def project_rugged(project) - rugged_repo(project.repository) + rugged_repo(project.repository) + end end end diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index 771d834c4bc..c8ab564e3bc 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -42,12 +42,7 @@ RSpec.describe NotificationSetting do expect(notification_setting.new_issue).to eq(true) expect(notification_setting.close_issue).to eq(true) expect(notification_setting.merge_merge_request).to eq(true) - - # In Rails 5 assigning a value which is not explicitly `true` or `false` ("nil" in this case) - # to a boolean column transforms it to `true`. - # In Rails 4 it transforms the value to `false` with deprecation warning. - # Replace `eq(Gitlab.rails5?)` with `eq(true)` when removing rails5? code. - expect(notification_setting.close_merge_request).to eq(Gitlab.rails5?) + expect(notification_setting.close_merge_request).to eq(true) expect(notification_setting.reopen_merge_request).to eq(false) end end diff --git a/spec/models/pool_repository_spec.rb b/spec/models/pool_repository_spec.rb index 3d3878b8c39..112d4ab56fc 100644 --- a/spec/models/pool_repository_spec.rb +++ b/spec/models/pool_repository_spec.rb @@ -23,4 +23,25 @@ describe PoolRepository do expect(pool.disk_path).to match(%r{\A@pools/\h{2}/\h{2}/\h{64}}) end end + + describe '#unlink_repository' do + let(:pool) { create(:pool_repository, :ready) } + + context 'when the last member leaves' do + it 'schedules pool removal' do + expect(::ObjectPool::DestroyWorker).to receive(:perform_async).with(pool.id).and_call_original + + pool.unlink_repository(pool.source_project.repository) + end + end + + context 'when the second member leaves' do + it 'does not schedule pool removal' do + create(:project, :repository, pool_repository: pool) + expect(::ObjectPool::DestroyWorker).not_to receive(:perform_async).with(pool.id) + + pool.unlink_repository(pool.source_project.repository) + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5e63f14b720..2170ef1f155 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -145,25 +145,10 @@ describe Project do end describe 'ci_pipelines association' do - context 'when feature flag pipeline_ci_sources_only is enabled' do - it 'returns only pipelines from ci_sources' do - stub_feature_flags(pipeline_ci_sources_only: true) + it 'returns only pipelines from ci_sources' do + expect(Ci::Pipeline).to receive(:ci_sources).and_call_original - expect(Ci::Pipeline).to receive(:ci_sources).and_call_original - - subject.ci_pipelines - end - end - - context 'when feature flag pipeline_ci_sources_only is disabled' do - it 'returns all pipelines' do - stub_feature_flags(pipeline_ci_sources_only: false) - - expect(Ci::Pipeline).not_to receive(:ci_sources).and_call_original - expect(Ci::Pipeline).to receive(:all).and_call_original.at_least(:once) - - subject.ci_pipelines - end + subject.ci_pipelines end end end @@ -314,6 +299,13 @@ describe Project do expect(project.errors[:import_url].first).to include('Requests to localhost are not allowed') end + it 'does not allow import_url pointing to the local network' do + project = build(:project, import_url: 'https://192.168.1.1') + + expect(project).to be_invalid + expect(project.errors[:import_url].first).to include('Requests to the local network are not allowed') + end + it "does not allow import_url with invalid ports for new projects" do project = build(:project, import_url: 'http://github.com:25/t.git') @@ -618,16 +610,20 @@ describe Project do end it 'returns the address to create a new issue' do - address = "p+#{project.full_path}+#{user.incoming_email_token}@gl.ab" + address = "p+#{project.full_path_slug}-#{project.project_id}-#{user.incoming_email_token}-issue@gl.ab" expect(project.new_issuable_address(user, 'issue')).to eq(address) end it 'returns the address to create a new merge request' do - address = "p+#{project.full_path}+merge-request+#{user.incoming_email_token}@gl.ab" + address = "p+#{project.full_path_slug}-#{project.project_id}-#{user.incoming_email_token}-merge-request@gl.ab" expect(project.new_issuable_address(user, 'merge_request')).to eq(address) end + + it 'returns nil with invalid address type' do + expect(project.new_issuable_address(user, 'invalid_param')).to be_nil + end end context 'incoming email disabled' do @@ -1666,26 +1662,54 @@ describe Project do end describe '#track_project_repository' do - let(:project) { create(:project, :repository) } + shared_examples 'tracks storage location' do + context 'when a project repository entry does not exist' do + it 'creates a new entry' do + expect { project.track_project_repository }.to change(project, :project_repository) + end - it 'creates a project_repository' do - project.track_project_repository + it 'tracks the project storage location' do + project.track_project_repository - expect(project.reload.project_repository).to be_present - expect(project.project_repository.disk_path).to eq(project.disk_path) - expect(project.project_repository.shard_name).to eq(project.repository_storage) + expect(project.project_repository).to have_attributes( + disk_path: project.disk_path, + shard_name: project.repository_storage + ) + end + end + + context 'when a tracking entry exists' do + let!(:project_repository) { create(:project_repository, project: project) } + let!(:shard) { create(:shard, name: 'foo') } + + it 'does not create a new entry in the database' do + expect { project.track_project_repository }.not_to change(project, :project_repository) + end + + it 'updates the project storage location' do + allow(project).to receive(:disk_path).and_return('fancy/new/path') + allow(project).to receive(:repository_storage).and_return('foo') + + project.track_project_repository + + expect(project.project_repository).to have_attributes( + disk_path: 'fancy/new/path', + shard_name: 'foo' + ) + end + end end - it 'updates the project_repository' do - project.track_project_repository + context 'with projects on legacy storage' do + let(:project) { create(:project, :repository, :legacy_storage) } - allow(project).to receive(:disk_path).and_return('@fancy/new/path') + it_behaves_like 'tracks storage location' + end - expect do - project.track_project_repository - end.not_to change(ProjectRepository, :count) + context 'with projects on hashed storage' do + let(:project) { create(:project, :repository) } - expect(project.reload.project_repository.disk_path).to eq(project.disk_path) + it_behaves_like 'tracks storage location' end end @@ -2530,6 +2554,10 @@ describe Project do end context 'when the ref is not protected' do + before do + allow(project).to receive(:protected_for?).with('ref').and_return(false) + end + it 'contains only the CI variables' do is_expected.to contain_exactly(ci_variable) end @@ -2569,42 +2597,139 @@ describe Project do end describe '#protected_for?' do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } - subject { project.protected_for?('ref') } + subject { project.protected_for?(ref) } - context 'when the ref is not protected' do + shared_examples 'ref is not protected' do before do stub_application_setting( default_branch_protection: Gitlab::Access::PROTECTION_NONE) end it 'returns false' do - is_expected.to be_falsey + is_expected.to be false end end - context 'when the ref is a protected branch' do + shared_examples 'ref is protected branch' do before do - allow(project).to receive(:repository).and_call_original - allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(true) - create(:protected_branch, name: 'ref', project: project) + create(:protected_branch, name: 'master', project: project) end it 'returns true' do - is_expected.to be_truthy + is_expected.to be true end end - context 'when the ref is a protected tag' do + shared_examples 'ref is protected tag' do before do - allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(false) - allow(project).to receive_message_chain(:repository, :tag_exists?).and_return(true) - create(:protected_tag, name: 'ref', project: project) + create(:protected_tag, name: 'v1.0.0', project: project) end it 'returns true' do - is_expected.to be_truthy + is_expected.to be true + end + end + + context 'when ref is nil' do + let(:ref) { nil } + + it 'returns false' do + is_expected.to be false + end + end + + context 'when ref is ref name' do + context 'when ref is ambiguous' do + let(:ref) { 'ref' } + + before do + project.repository.add_branch(project.creator, 'ref', 'master') + project.repository.add_tag(project.creator, 'ref', 'master') + end + + it 'raises an error' do + expect { subject }.to raise_error(Repository::AmbiguousRefError) + end + end + + context 'when the ref is not protected' do + let(:ref) { 'master' } + + it_behaves_like 'ref is not protected' + end + + context 'when the ref is a protected branch' do + let(:ref) { 'master' } + + it_behaves_like 'ref is protected branch' + end + + context 'when the ref is a protected tag' do + let(:ref) { 'v1.0.0' } + + it_behaves_like 'ref is protected tag' + end + + context 'when ref does not exist' do + let(:ref) { 'something' } + + it 'returns false' do + is_expected.to be false + end + end + end + + context 'when ref is full ref' do + context 'when the ref is not protected' do + let(:ref) { 'refs/heads/master' } + + it_behaves_like 'ref is not protected' + end + + context 'when the ref is a protected branch' do + let(:ref) { 'refs/heads/master' } + + it_behaves_like 'ref is protected branch' + end + + context 'when the ref is a protected tag' do + let(:ref) { 'refs/tags/v1.0.0' } + + it_behaves_like 'ref is protected tag' + end + + context 'when branch ref name is a full tag ref' do + let(:ref) { 'refs/tags/something' } + + before do + project.repository.add_branch(project.creator, ref, 'master') + end + + context 'when ref is not protected' do + it 'returns false' do + is_expected.to be false + end + end + + context 'when ref is a protected branch' do + before do + create(:protected_branch, name: 'refs/tags/something', project: project) + end + + it 'returns true' do + is_expected.to be true + end + end + end + + context 'when ref does not exist' do + let(:ref) { 'refs/heads/something' } + + it 'returns false' do + is_expected.to be false + end end end end @@ -2824,7 +2949,7 @@ describe Project do 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' + ' 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) @@ -2838,6 +2963,24 @@ describe Project do end end + describe '#update' do + let(:project) { create(:project) } + + it 'validates the visibility' do + expect(project).to receive(:visibility_level_allowed_as_fork).and_call_original + expect(project).to receive(:visibility_level_allowed_by_group).and_call_original + + project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end + + it 'does not validate the visibility' do + expect(project).not_to receive(:visibility_level_allowed_as_fork).and_call_original + expect(project).not_to receive(:visibility_level_allowed_by_group).and_call_original + + project.update(updated_at: Time.now) + end + end + describe '#last_repository_updated_at' do it 'sets to created_at upon creation' do project = create(:project, created_at: 2.hours.ago) @@ -3064,6 +3207,13 @@ describe Project do expect { project.migrate_to_hashed_storage! }.to change { project.repository_read_only }.to(true) end + it 'does not validate project visibility' do + expect(project).not_to receive(:visibility_level_allowed_as_fork) + expect(project).not_to receive(:visibility_level_allowed_by_group) + + project.migrate_to_hashed_storage! + end + it 'schedules ProjectMigrateHashedStorageWorker with delayed start when the project repo is in use' do Gitlab::ReferenceCounter.new(project.gl_repository(is_wiki: false)).increase @@ -3690,7 +3840,7 @@ describe Project do expect(project.badges.count).to eq 3 end - if Group.supports_nested_groups? + if Group.supports_nested_objects? context 'with nested_groups' do let(:parent_group) { create(:group) } @@ -3710,6 +3860,16 @@ describe Project do let(:user) { create(:user) } let(:target_project) { create(:project, :repository) } let(:project) { fork_project(target_project, nil, repository: true) } + let!(:local_merge_request) do + create( + :merge_request, + target_project: project, + target_branch: 'target-branch', + source_project: project, + source_branch: 'awesome-feature-1', + allow_collaboration: true + ) + end let!(:merge_request) do create( :merge_request, @@ -3754,14 +3914,23 @@ describe Project do end end - describe '#branch_allows_collaboration_push?' do - it 'allows access if the user can merge the merge request' do - expect(project.branch_allows_collaboration?(user, 'awesome-feature-1')) + describe '#any_branch_allows_collaboration?' do + it 'allows access when there are merge requests open allowing collaboration' do + expect(project.any_branch_allows_collaboration?(user)) .to be_truthy end - it 'allows access when there are merge requests open but no branch name is given' do - expect(project.branch_allows_collaboration?(user, nil)) + it 'does not allow access when there are no merge requests open allowing collaboration' do + merge_request.close! + + expect(project.any_branch_allows_collaboration?(user)) + .to be_falsey + end + end + + describe '#branch_allows_collaboration?' do + it 'allows access if the user can merge the merge request' do + expect(project.branch_allows_collaboration?(user, 'awesome-feature-1')) .to be_truthy end @@ -3792,13 +3961,6 @@ describe Project do .to be_falsy end - it 'caches the result' do - control = ActiveRecord::QueryRecorder.new { project.branch_allows_collaboration?(user, 'awesome-feature-1') } - - expect { 3.times { project.branch_allows_collaboration?(user, 'awesome-feature-1') } } - .not_to exceed_query_limit(control) - end - context 'when the requeststore is active', :request_store do it 'only queries per project across instances' do control = ActiveRecord::QueryRecorder.new { project.branch_allows_collaboration?(user, 'awesome-feature-1') } diff --git a/spec/models/prometheus_metric_spec.rb b/spec/models/prometheus_metric_spec.rb index 3692fe9a559..2b978c1c8ff 100644 --- a/spec/models/prometheus_metric_spec.rb +++ b/spec/models/prometheus_metric_spec.rb @@ -59,11 +59,65 @@ describe PrometheusMetric do end end + it_behaves_like 'group_title', :nginx_ingress_vts, 'Response metrics (NGINX Ingress VTS)' + it_behaves_like 'group_title', :nginx_ingress, 'Response metrics (NGINX Ingress)' + it_behaves_like 'group_title', :ha_proxy, 'Response metrics (HA Proxy)' + it_behaves_like 'group_title', :aws_elb, 'Response metrics (AWS ELB)' + it_behaves_like 'group_title', :nginx, 'Response metrics (NGINX)' + it_behaves_like 'group_title', :kubernetes, 'System metrics (Kubernetes)' it_behaves_like 'group_title', :business, 'Business metrics (Custom)' it_behaves_like 'group_title', :response, 'Response metrics (Custom)' it_behaves_like 'group_title', :system, 'System metrics (Custom)' end + describe '#priority' do + using RSpec::Parameterized::TableSyntax + + where(:group, :priority) do + :nginx_ingress_vts | 10 + :nginx_ingress | 10 + :ha_proxy | 10 + :aws_elb | 10 + :nginx | 10 + :kubernetes | 5 + :business | 0 + :response | -5 + :system | -10 + end + + with_them do + before do + subject.group = group + end + + it { expect(subject.priority).to eq(priority) } + end + end + + describe '#required_metrics' do + using RSpec::Parameterized::TableSyntax + + where(:group, :required_metrics) do + :nginx_ingress_vts | %w(nginx_upstream_responses_total nginx_upstream_response_msecs_avg) + :nginx_ingress | %w(nginx_ingress_controller_requests nginx_ingress_controller_ingress_upstream_latency_seconds_sum) + :ha_proxy | %w(haproxy_frontend_http_requests_total haproxy_frontend_http_responses_total) + :aws_elb | %w(aws_elb_request_count_sum aws_elb_latency_average aws_elb_httpcode_backend_5_xx_sum) + :nginx | %w(nginx_server_requests nginx_server_requestMsec) + :kubernetes | %w(container_memory_usage_bytes container_cpu_usage_seconds_total) + :business | %w() + :response | %w() + :system | %w() + end + + with_them do + before do + subject.group = group + end + + it { expect(subject.required_metrics).to eq(required_metrics) } + end + end + describe '#to_query_metric' do it 'converts to queryable metric object' do expect(subject.to_query_metric).to be_instance_of(Gitlab::Prometheus::Metric) diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 3f86347c3ae..92ba2d82f58 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -1,12 +1,15 @@ require 'rails_helper' RSpec.describe Release do - let(:release) { create(:release) } + let(:user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + let(:release) { create(:release, project: project, author: user) } it { expect(release).to be_valid } describe 'associations' do it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:author).class_name('User') } end describe 'validation' do diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 5d3c25062d5..224bc9ed935 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -24,6 +24,20 @@ describe RemoteMirror, :mailer do expect(remote_mirror).to be_invalid expect(remote_mirror.errors[:url].first).to include('Username needs to start with an alphanumeric character') end + + it 'does not allow url pointing to localhost' do + remote_mirror = build(:remote_mirror, url: 'http://127.0.0.2/t.git') + + expect(remote_mirror).to be_invalid + expect(remote_mirror.errors[:url].first).to include('Requests to loopback addresses are not allowed') + end + + it 'does not allow url pointing to the local network' do + remote_mirror = build(:remote_mirror, url: 'https://192.168.1.1') + + expect(remote_mirror).to be_invalid + expect(remote_mirror.errors[:url].first).to include('Requests to the local network are not allowed') + end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index f09b4b67061..2063b4bbe75 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1005,6 +1005,67 @@ describe Repository do end end + describe '#ambiguous_ref?' do + let(:ref) { 'ref' } + + subject { repository.ambiguous_ref?(ref) } + + context 'when ref is ambiguous' do + before do + repository.add_tag(project.creator, ref, 'master') + repository.add_branch(project.creator, ref, 'master') + end + + it 'should be true' do + is_expected.to eq(true) + end + end + + context 'when ref is not ambiguous' do + before do + repository.add_tag(project.creator, ref, 'master') + end + + it 'should be false' do + is_expected.to eq(false) + end + end + end + + describe '#expand_ref' do + let(:ref) { 'ref' } + + subject { repository.expand_ref(ref) } + + context 'when ref is not tag or branch name' do + let(:ref) { 'refs/heads/master' } + + it 'returns nil' do + is_expected.to eq(nil) + end + end + + context 'when ref is tag name' do + before do + repository.add_tag(project.creator, ref, 'master') + end + + it 'returns the tag ref' do + is_expected.to eq("refs/tags/#{ref}") + end + end + + context 'when ref is branch name' do + before do + repository.add_branch(project.creator, ref, 'master') + end + + it 'returns the branch ref' do + is_expected.to eq("refs/heads/#{ref}") + end + end + end + describe '#add_branch' do let(:branch_name) { 'new_feature' } let(:target) { 'master' } diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 7a7272ccb60..664dc3fa145 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -423,4 +423,41 @@ describe Snippet do expect(blob.data).to eq(snippet.content) end end + + describe '#embeddable?' do + context 'project snippet' do + [ + { project: :public, snippet: :public, embeddable: true }, + { project: :internal, snippet: :public, embeddable: false }, + { project: :private, snippet: :public, embeddable: false }, + { project: :public, snippet: :internal, embeddable: false }, + { project: :internal, snippet: :internal, embeddable: false }, + { project: :private, snippet: :internal, embeddable: false }, + { project: :public, snippet: :private, embeddable: false }, + { project: :internal, snippet: :private, embeddable: false }, + { project: :private, snippet: :private, embeddable: false } + ].each do |combination| + it 'only returns true when both project and snippet are public' do + project = create(:project, combination[:project]) + snippet = create(:project_snippet, combination[:snippet], project: project) + + expect(snippet.embeddable?).to eq(combination[:embeddable]) + end + end + end + + context 'personal snippet' do + [ + { snippet: :public, embeddable: true }, + { snippet: :internal, embeddable: false }, + { snippet: :private, embeddable: false } + ].each do |combination| + it 'only returns true when snippet is public' do + snippet = create(:personal_snippet, combination[:snippet]) + + expect(snippet.embeddable?).to eq(combination[:embeddable]) + end + end + end + end end diff --git a/spec/models/suggestion_spec.rb b/spec/models/suggestion_spec.rb new file mode 100644 index 00000000000..cafc725dddb --- /dev/null +++ b/spec/models/suggestion_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Suggestion do + let(:suggestion) { create(:suggestion) } + + describe 'associations' do + it { is_expected.to belong_to(:note) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:note) } + + context 'when suggestion is applied' do + before do + allow(subject).to receive(:applied?).and_return(true) + end + + it { is_expected.to validate_presence_of(:commit_id) } + end + end + + describe '#appliable?' do + context 'when note does not support suggestions' do + it 'returns false' do + expect_next_instance_of(DiffNote) do |note| + allow(note).to receive(:supports_suggestion?) { false } + end + + expect(suggestion).not_to be_appliable + end + end + + context 'when patch is already applied' do + let(:suggestion) { create(:suggestion, :applied) } + + it 'returns false' do + expect(suggestion).not_to be_appliable + end + end + + context 'when merge request is not opened' do + let(:merge_request) { create(:merge_request, :merged) } + let(:note) do + create(:diff_note_on_merge_request, project: merge_request.project, + noteable: merge_request) + end + + let(:suggestion) { create(:suggestion, note: note) } + + it 'returns false' do + expect(suggestion).not_to be_appliable + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ff075e65c76..33842e74b92 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -45,6 +45,7 @@ describe User do it { is_expected.to have_many(:uploads) } it { is_expected.to have_many(:reported_abuse_reports).dependent(:destroy).class_name('AbuseReport') } it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') } + it { is_expected.to have_many(:releases).dependent(:nullify) } describe "#abuse_report" do let(:current_user) { create(:user) } @@ -1965,7 +1966,7 @@ describe User do subject { user.membership_groups } - if Group.supports_nested_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 } @@ -2346,7 +2347,7 @@ describe User do group.add_owner(user) end - if Group.supports_nested_groups? + if Group.supports_nested_objects? it 'returns all groups' do is_expected.to match_array [ group, |