diff options
author | Pawel Chojnacki <pawel@chojnacki.ws> | 2017-06-07 11:27:52 +0300 |
---|---|---|
committer | Pawel Chojnacki <pawel@chojnacki.ws> | 2017-06-07 11:27:52 +0300 |
commit | dbb3c28088b63c8cf40a90c599b6eedb4dfbbb66 (patch) | |
tree | 9e6c4d495e45355ef62c56c0c3e102f95220f89f /spec/models | |
parent | a924152219c1367bf494f3f387d050ac3ff2d7d3 (diff) | |
parent | dddc54aa0aea4088e5a233d18a62cb2435590fe9 (diff) |
Merge remote-tracking branch 'upstream/master' into 28717-additional-metrics-review-branch
# Conflicts:
# app/models/project_services/prometheus_service.rb
# app/views/projects/services/_form.html.haml
Diffstat (limited to 'spec/models')
-rw-r--r-- | spec/models/abuse_report_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/blob_viewer/base_spec.rb | 70 | ||||
-rw-r--r-- | spec/models/ci/build_spec.rb | 165 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 26 | ||||
-rw-r--r-- | spec/models/ci/variable_spec.rb | 33 | ||||
-rw-r--r-- | spec/models/concerns/editable_spec.rb | 11 | ||||
-rw-r--r-- | spec/models/deployment_spec.rb | 13 | ||||
-rw-r--r-- | spec/models/diff_note_spec.rb | 27 | ||||
-rw-r--r-- | spec/models/environment_spec.rb | 27 | ||||
-rw-r--r-- | spec/models/forked_project_link_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/key_spec.rb | 10 | ||||
-rw-r--r-- | spec/models/merge_request_diff_spec.rb | 11 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 14 | ||||
-rw-r--r-- | spec/models/pages_domain_spec.rb | 51 | ||||
-rw-r--r-- | spec/models/project_services/jira_service_spec.rb | 36 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 130 | ||||
-rw-r--r-- | spec/models/project_team_spec.rb | 151 | ||||
-rw-r--r-- | spec/models/project_wiki_spec.rb | 18 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 25 |
19 files changed, 540 insertions, 286 deletions
diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index ced93c8f762..90aec2b45e6 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -28,9 +28,7 @@ RSpec.describe AbuseReport, type: :model do end it 'lets a worker delete the user' do - expect(DeleteUserWorker).to receive(:perform_async).with(user.id, subject.user.id, - delete_solo_owned_groups: true, - hard_delete: true) + expect(DeleteUserWorker).to receive(:perform_async).with(user.id, subject.user.id, hard_delete: true) subject.remove_user(deleted_by: user) end diff --git a/spec/models/blob_viewer/base_spec.rb b/spec/models/blob_viewer/base_spec.rb index 92fbf64a6b7..d56379eb59d 100644 --- a/spec/models/blob_viewer/base_spec.rb +++ b/spec/models/blob_viewer/base_spec.rb @@ -11,8 +11,8 @@ describe BlobViewer::Base, model: true do self.extensions = %w(pdf) self.binary = true - self.overridable_max_size = 1.megabyte - self.max_size = 5.megabytes + self.collapse_limit = 1.megabyte + self.size_limit = 5.megabytes end end @@ -69,77 +69,49 @@ describe BlobViewer::Base, model: true do end end - describe '#exceeds_overridable_max_size?' do - context 'when the blob size is larger than the overridable max size' do + describe '#collapsed?' do + context 'when the blob size is larger than the collapse limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } it 'returns true' do - expect(viewer.exceeds_overridable_max_size?).to be_truthy + expect(viewer.collapsed?).to be_truthy end end - context 'when the blob size is smaller than the overridable max size' do + context 'when the blob size is smaller than the collapse limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 10.kilobytes) } it 'returns false' do - expect(viewer.exceeds_overridable_max_size?).to be_falsey + expect(viewer.collapsed?).to be_falsey end end end - describe '#exceeds_max_size?' do - context 'when the blob size is larger than the max size' do + describe '#too_large?' do + context 'when the blob size is larger than the size limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 10.megabytes) } it 'returns true' do - expect(viewer.exceeds_max_size?).to be_truthy + expect(viewer.too_large?).to be_truthy end end - context 'when the blob size is smaller than the max size' do + context 'when the blob size is smaller than the size limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } it 'returns false' do - expect(viewer.exceeds_max_size?).to be_falsey - end - end - end - - describe '#can_override_max_size?' do - context 'when the blob size is larger than the overridable max size' do - context 'when the blob size is larger than the max size' do - let(:blob) { fake_blob(path: 'file.pdf', size: 10.megabytes) } - - it 'returns false' do - expect(viewer.can_override_max_size?).to be_falsey - end - end - - context 'when the blob size is smaller than the max size' do - let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } - - it 'returns true' do - expect(viewer.can_override_max_size?).to be_truthy - end - end - end - - context 'when the blob size is smaller than the overridable max size' do - let(:blob) { fake_blob(path: 'file.pdf', size: 10.kilobytes) } - - it 'returns false' do - expect(viewer.can_override_max_size?).to be_falsey + expect(viewer.too_large?).to be_falsey end end end describe '#render_error' do - context 'when the max size is overridden' do + context 'when expanded' do before do - viewer.override_max_size = true + viewer.expanded = true end - context 'when the blob size is larger than the max size' do + context 'when the blob size is larger than the size limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 10.megabytes) } it 'returns :too_large' do @@ -147,7 +119,7 @@ describe BlobViewer::Base, model: true do end end - context 'when the blob size is smaller than the max size' do + context 'when the blob size is smaller than the size limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } it 'returns nil' do @@ -156,16 +128,16 @@ describe BlobViewer::Base, model: true do end end - context 'when the max size is not overridden' do - context 'when the blob size is larger than the overridable max size' do + context 'when not expanded' do + context 'when the blob size is larger than the collapse limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } - it 'returns :too_large' do - expect(viewer.render_error).to eq(:too_large) + it 'returns :collapsed' do + expect(viewer.render_error).to eq(:collapsed) end end - context 'when the blob size is smaller than the overridable max size' do + context 'when the blob size is smaller than the collapse limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 10.kilobytes) } it 'returns nil' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index e971b4bc3f9..b0716e04d3d 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -427,6 +427,42 @@ describe Ci::Build, :models do end end + describe '#environment_url' do + subject { job.environment_url } + + context 'when yaml environment uses $CI_COMMIT_REF_NAME' do + let(:job) do + create(:ci_build, + ref: 'master', + options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } }) + end + + it { is_expected.to eq('http://review/master') } + end + + context 'when yaml environment uses yaml_variables containing symbol keys' do + let(:job) do + create(:ci_build, + yaml_variables: [{ key: :APP_HOST, value: 'host' }], + options: { environment: { url: 'http://review/$APP_HOST' } }) + end + + it { is_expected.to eq('http://review/host') } + end + + context 'when yaml environment does not have url' do + let(:job) { create(:ci_build, environment: 'staging') } + + let!(:environment) do + create(:environment, project: job.project, name: job.environment) + end + + it 'returns the external_url from persisted environment' do + is_expected.to eq(environment.external_url) + end + end + end + describe '#starts_environment?' do subject { build.starts_environment? } @@ -918,6 +954,10 @@ describe Ci::Build, :models do it { is_expected.to eq(environment) } end + + context 'when there is no environment' do + it { is_expected.to be_nil } + end end describe '#play' do @@ -1139,6 +1179,7 @@ describe Ci::Build, :models do { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true }, { key: 'CI_PROJECT_NAME', value: project.path, public: true }, { key: 'CI_PROJECT_PATH', value: project.full_path, public: true }, + { key: 'CI_PROJECT_PATH_SLUG', value: project.full_path.parameterize, public: true }, { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true }, { key: 'CI_PROJECT_URL', value: project.web_url, public: true }, { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true }, @@ -1176,11 +1217,6 @@ describe Ci::Build, :models do end context 'when build has an environment' do - before do - build.update(environment: 'production') - create(:environment, project: build.project, name: 'production', slug: 'prod-slug') - end - let(:environment_variables) do [ { key: 'CI_ENVIRONMENT_NAME', value: 'production', public: true }, @@ -1188,7 +1224,56 @@ describe Ci::Build, :models do ] end - it { environment_variables.each { |v| is_expected.to include(v) } } + let!(:environment) do + create(:environment, + project: build.project, + name: 'production', + slug: 'prod-slug', + external_url: '') + end + + before do + build.update(environment: 'production') + end + + shared_examples 'containing environment variables' do + it { environment_variables.each { |v| is_expected.to include(v) } } + end + + context 'when no URL was set' do + it_behaves_like 'containing environment variables' + + it 'does not have CI_ENVIRONMENT_URL' do + keys = subject.map { |var| var[:key] } + + expect(keys).not_to include('CI_ENVIRONMENT_URL') + end + end + + context 'when an URL was set' do + let(:url) { 'http://host/test' } + + before do + environment_variables << + { key: 'CI_ENVIRONMENT_URL', value: url, public: true } + end + + context 'when the URL was set from the job' do + before do + build.update(options: { environment: { url: 'http://host/$CI_JOB_NAME' } }) + end + + it_behaves_like 'containing environment variables' + end + + context 'when the URL was not set from the job, but environment' do + before do + environment.update(external_url: url) + end + + it_behaves_like 'containing environment variables' + end + end end context 'when build started manually' do @@ -1215,16 +1300,49 @@ describe Ci::Build, :models do it { is_expected.to include(tag_variable) } end - context 'when secure variable is defined' do - let(:secure_variable) do + context 'when secret variable is defined' do + let(:secret_variable) do { key: 'SECRET_KEY', value: 'secret_value', public: false } end before do - build.project.variables << Ci::Variable.new(key: 'SECRET_KEY', value: 'secret_value') + create(:ci_variable, + secret_variable.slice(:key, :value).merge(project: project)) + end + + it { is_expected.to include(secret_variable) } + end + + context 'when protected variable is defined' do + let(:protected_variable) do + { key: 'PROTECTED_KEY', value: 'protected_value', public: false } + end + + before do + create(:ci_variable, + :protected, + protected_variable.slice(:key, :value).merge(project: project)) + end + + context 'when the branch is protected' do + before do + create(:protected_branch, project: build.project, name: build.ref) + end + + it { is_expected.to include(protected_variable) } end - it { is_expected.to include(secure_variable) } + context 'when the tag is protected' do + before do + create(:protected_tag, project: build.project, name: build.ref) + end + + it { is_expected.to include(protected_variable) } + end + + context 'when the ref is not protected' do + it { is_expected.not_to include(protected_variable) } + end end context 'when build is for triggers' do @@ -1346,15 +1464,30 @@ describe Ci::Build, :models do end context 'returns variables in valid order' do + let(:build_pre_var) { { key: 'build', value: 'value' } } + let(:project_pre_var) { { key: 'project', value: 'value' } } + let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } } + let(:build_yaml_var) { { key: 'yaml', value: 'value' } } + before do - allow(build).to receive(:predefined_variables) { ['predefined'] } - allow(project).to receive(:predefined_variables) { ['project'] } - allow(pipeline).to receive(:predefined_variables) { ['pipeline'] } - allow(build).to receive(:yaml_variables) { ['yaml'] } - allow(project).to receive(:secret_variables) { ['secret'] } + allow(build).to receive(:predefined_variables) { [build_pre_var] } + allow(project).to receive(:predefined_variables) { [project_pre_var] } + allow(pipeline).to receive(:predefined_variables) { [pipeline_pre_var] } + allow(build).to receive(:yaml_variables) { [build_yaml_var] } + + allow(project).to receive(:secret_variables_for).with(build.ref) do + [create(:ci_variable, key: 'secret', value: 'value')] + end end - it { is_expected.to eq(%w[predefined project pipeline yaml secret]) } + it do + is_expected.to eq( + [build_pre_var, + project_pre_var, + pipeline_pre_var, + build_yaml_var, + { key: 'secret', value: 'value', public: false }]) + end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index c8023dc13b1..ae1b01b76ab 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -21,13 +21,35 @@ describe Ci::Pipeline, models: true do it { is_expected.to have_many(:auto_canceled_pipelines) } it { is_expected.to have_many(:auto_canceled_jobs) } - it { is_expected.to validate_presence_of :sha } - it { is_expected.to validate_presence_of :status } + it { is_expected.to validate_presence_of(:sha) } + it { is_expected.to validate_presence_of(:status) } it { is_expected.to respond_to :git_author_name } it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :short_sha } + describe '#source' do + context 'when creating new pipeline' do + let(:pipeline) do + build(:ci_empty_pipeline, status: :created, project: project, source: nil) + end + + it "prevents from creating an object" do + expect(pipeline).not_to be_valid + end + end + + context 'when updating existing pipeline' do + before do + pipeline.update_attribute(:source, nil) + end + + it "object is valid" do + expect(pipeline).to be_valid + end + end + end + describe '#block' do it 'changes pipeline status to manual' do expect(pipeline.block).to be true diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index fe8c52d5353..077b10227d7 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -12,11 +12,33 @@ describe Ci::Variable, models: true do it { is_expected.not_to allow_value('foo bar').for(:key) } it { is_expected.not_to allow_value('foo/bar').for(:key) } - before :each do - subject.value = secret_value + describe '.unprotected' do + subject { described_class.unprotected } + + context 'when variable is protected' do + before do + create(:ci_variable, :protected) + end + + it 'returns nothing' do + is_expected.to be_empty + end + end + + context 'when variable is not protected' do + let(:variable) { create(:ci_variable, protected: false) } + + it 'returns the variable' do + is_expected.to contain_exactly(variable) + end + end end describe '#value' do + before do + subject.value = secret_value + end + it 'stores the encrypted value' do expect(subject.encrypted_value).not_to be_nil end @@ -36,4 +58,11 @@ describe Ci::Variable, models: true do to raise_error(OpenSSL::Cipher::CipherError, 'bad decrypt') end end + + describe '#to_runner_variable' do + it 'returns a hash for the runner' do + expect(subject.to_runner_variable) + .to eq(key: subject.key, value: subject.value, public: false) + end + end end diff --git a/spec/models/concerns/editable_spec.rb b/spec/models/concerns/editable_spec.rb new file mode 100644 index 00000000000..cd73af3b480 --- /dev/null +++ b/spec/models/concerns/editable_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe Editable do + describe '#is_edited?' do + let(:issue) { create(:issue, last_edited_at: nil) } + let(:edited_issue) { create(:issue, created_at: 3.days.ago, last_edited_at: 2.days.ago) } + + it { expect(issue.is_edited?).to eq(false) } + it { expect(edited_issue.is_edited?).to eq(true) } + end +end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index bbb7dbf0922..d15ff9e5ffd 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -16,6 +16,19 @@ describe Deployment, models: true do it { is_expected.to validate_presence_of(:ref) } it { is_expected.to validate_presence_of(:sha) } + describe 'after_create callbacks' do + let(:environment) { create(:environment) } + let(:store) { Gitlab::EtagCaching::Store.new } + + it 'invalidates the environment etag cache' do + old_value = store.get(environment.etag_cache_key) + + create(:deployment, environment: environment) + + expect(store.get(environment.etag_cache_key)).not_to eq(old_value) + end + end + describe '#includes_commit?' do let(:project) { create(:project, :repository) } let(:environment) { create(:environment, project: project) } diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index 96f075d4f7d..297c2108dc2 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -160,12 +160,6 @@ describe DiffNote, models: true do context "when noteable is a commit" do let(:diff_note) { create(:diff_note_on_commit, project: project, position: position) } - it "doesn't use the DiffPositionUpdateService" do - expect(Notes::DiffPositionUpdateService).not_to receive(:new) - - diff_note - end - it "doesn't update the position" do diff_note @@ -178,12 +172,6 @@ describe DiffNote, models: true do let(:diff_note) { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) } context "when the note is active" do - it "doesn't use the DiffPositionUpdateService" do - expect(Notes::DiffPositionUpdateService).not_to receive(:new) - - diff_note - end - it "doesn't update the position" do diff_note @@ -197,18 +185,11 @@ describe DiffNote, models: true do allow(merge_request).to receive(:diff_refs).and_return(commit.diff_refs) end - it "uses the DiffPositionUpdateService" do - service = instance_double("Notes::DiffPositionUpdateService") - expect(Notes::DiffPositionUpdateService).to receive(:new).with( - project, - nil, - old_diff_refs: position.diff_refs, - new_diff_refs: commit.diff_refs, - paths: [path] - ).and_return(service) - expect(service).to receive(:execute) - + it "updates the position" do diff_note + + expect(diff_note.original_position).to eq(position) + expect(diff_note.position).not_to eq(position) end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index c8709409cea..f711065a0ab 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Environment, models: true do - let(:project) { create(:empty_project) } + set(:project) { create(:empty_project) } subject(:environment) { create(:environment, project: project) } it { is_expected.to belong_to(:project) } @@ -34,6 +34,26 @@ describe Environment, models: true do end end + describe 'state machine' do + it 'invalidates the cache after a change' do + expect(environment).to receive(:expire_etag_cache) + + environment.stop + end + end + + describe '#expire_etag_cache' do + let(:store) { Gitlab::EtagCaching::Store.new } + + it 'changes the cached value' do + old_value = store.get(environment.etag_cache_key) + + environment.stop + + expect(store.get(environment.etag_cache_key)).not_to eq(old_value) + end + end + describe '#nullify_external_url' do it 'replaces a blank url with nil' do env = build(:environment, external_url: "") @@ -227,7 +247,10 @@ describe Environment, models: true do context 'when user is allowed to stop environment' do before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) end context 'when action did not yet finish' do diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb index 454550c9710..6e8d43f988c 100644 --- a/spec/models/forked_project_link_spec.rb +++ b/spec/models/forked_project_link_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe ForkedProjectLink, "add link on fork" do let(:project_from) { create(:project, :repository) } - let(:namespace) { create(:namespace) } - let(:user) { create(:user, namespace: namespace) } + let(:user) { create(:user) } + let(:namespace) { user.namespace } before do create(:project_member, :reporter, user: user, project: project_from) diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 7c40cfd8253..f1e2a2cc518 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -66,14 +66,16 @@ describe Key, models: true do end it "does not accept the exact same key twice" do - create(:key, user: user) - expect(build(:key, user: user)).not_to be_valid + first_key = create(:key, user: user) + + expect(build(:key, user: user, key: first_key.key)).not_to be_valid end it "does not accept a duplicate key with a different comment" do - create(:key, user: user) - duplicate = build(:key, user: user) + first_key = create(:key, user: user) + duplicate = build(:key, user: user, key: first_key.key) duplicate.key << ' extra comment' + expect(duplicate).not_to be_valid end end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 0a10ee01506..ed9fde57bf7 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -139,4 +139,15 @@ describe MergeRequestDiff, models: true do expect(subject.commits_count).to eq 2 end end + + describe '#utf8_st_diffs' do + it 'does not raise error when a hash value is in binary' do + subject.st_diffs = [ + { diff: "\0" }, + { diff: "\x05\x00\x68\x65\x6c\x6c\x6f" } + ] + + expect { subject.utf8_st_diffs }.not_to raise_error + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 712470d6bf5..060754fab63 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -238,10 +238,10 @@ describe MergeRequest, models: true do end context 'when there are no MR diffs' do - it 'delegates to the compare object, setting no_collapse: true' do + it 'delegates to the compare object, setting expanded: true' do merge_request.compare = double(:compare) - expect(merge_request.compare).to receive(:diffs).with(options.merge(no_collapse: true)) + expect(merge_request.compare).to receive(:diffs).with(options.merge(expanded: true)) merge_request.diffs(options) end @@ -1178,7 +1178,7 @@ describe MergeRequest, models: true do end describe "#reload_diff" do - let(:note) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject) } + let(:discussion) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject).to_discussion } let(:commit) { subject.project.commit(sample_commit.id) } @@ -1197,7 +1197,7 @@ describe MergeRequest, models: true do subject.reload_diff end - it "updates diff note positions" do + it "updates diff discussion positions" do old_diff_refs = subject.diff_refs # Update merge_request_diff so that #diff_refs will return commit.diff_refs @@ -1211,15 +1211,15 @@ describe MergeRequest, models: true do subject.merge_request_diff(true) end - expect(Notes::DiffPositionUpdateService).to receive(:new).with( + expect(Discussions::UpdateDiffPositionService).to receive(:new).with( subject.project, subject.author, old_diff_refs: old_diff_refs, new_diff_refs: commit.diff_refs, - paths: note.position.paths + paths: discussion.position.paths ).and_call_original - expect_any_instance_of(Notes::DiffPositionUpdateService).to receive(:execute).with(note) + expect_any_instance_of(Discussions::UpdateDiffPositionService).to receive(:execute).with(discussion).and_call_original expect_any_instance_of(DiffNote).to receive(:save).once subject.reload_diff(subject.author) diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index c6c45d78990..f9d060d4e0e 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -6,7 +6,7 @@ describe PagesDomain, models: true do end describe 'validate domain' do - subject { build(:pages_domain, domain: domain) } + subject(:pages_domain) { build(:pages_domain, domain: domain) } context 'is unique' do let(:domain) { 'my.domain.com' } @@ -14,36 +14,25 @@ describe PagesDomain, models: true do it { is_expected.to validate_uniqueness_of(:domain) } end - context 'valid domain' do - let(:domain) { 'my.domain.com' } - - it { is_expected.to be_valid } - end - - context 'valid hexadecimal-looking domain' do - let(:domain) { '0x12345.com'} - - it { is_expected.to be_valid } - end - - context 'no domain' do - let(:domain) { nil } - - it { is_expected.not_to be_valid } - end - - context 'invalid domain' do - let(:domain) { '0123123' } - - it { is_expected.not_to be_valid } - end - - context 'domain from .example.com' do - let(:domain) { 'my.domain.com' } - - before { allow(Settings.pages).to receive(:host).and_return('domain.com') } - - it { is_expected.not_to be_valid } + { + 'my.domain.com' => true, + '123.456.789' => true, + '0x12345.com' => true, + '0123123' => true, + '_foo.com' => false, + 'reserved.com' => false, + 'a.reserved.com' => false, + nil => false + }.each do |value, validity| + context "domain #{value.inspect} validity" do + before do + allow(Settings.pages).to receive(:host).and_return('reserved.com') + end + + let(:domain) { value } + + it { expect(pages_domain.valid?).to eq(validity) } + end end end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 349067e73ab..0ee050196e4 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -69,41 +69,6 @@ describe JiraService, models: true do end end - describe '#can_test?' do - let(:jira_service) { described_class.new } - - it 'returns false if username is blank' do - allow(jira_service).to receive_messages( - url: 'http://jira.example.com', - username: '', - password: '12345678' - ) - - expect(jira_service.can_test?).to be_falsy - end - - it 'returns false if password is blank' do - allow(jira_service).to receive_messages( - url: 'http://jira.example.com', - username: 'tester', - password: '' - ) - - expect(jira_service.can_test?).to be_falsy - end - - it 'returns true if password and username are present' do - jira_service = described_class.new - allow(jira_service).to receive_messages( - url: 'http://jira.example.com', - username: 'tester', - password: '12345678' - ) - - expect(jira_service.can_test?).to be_truthy - end - end - describe '#close_issue' do let(:custom_base_url) { 'http://custom_url' } let(:user) { create(:user) } @@ -133,6 +98,7 @@ describe JiraService, models: true do allow(JIRA::Resource::Issue).to receive(:find).and_return(open_issue, closed_issue) allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return("JIRA-123") + allow(JIRA::Resource::Remotelink).to receive(:all).and_return([]) @jira_service.save diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 36575acf671..3ed52d42f86 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -50,7 +50,7 @@ describe Project, models: true do it { is_expected.to have_one(:external_wiki_service).dependent(:destroy) } it { is_expected.to have_one(:project_feature).dependent(:destroy) } it { is_expected.to have_one(:statistics).class_name('ProjectStatistics').dependent(:delete) } - it { is_expected.to have_one(:import_data).class_name('ProjectImportData').dependent(:destroy) } + it { is_expected.to have_one(:import_data).class_name('ProjectImportData').dependent(:delete) } it { is_expected.to have_one(:last_event).class_name('Event') } it { is_expected.to have_one(:forked_from_project).through(:forked_project_link) } it { is_expected.to have_many(:commit_statuses) } @@ -948,6 +948,20 @@ describe Project, models: true do end end + describe '.starred_by' do + it 'returns only projects starred by the given user' do + user1 = create(:user) + user2 = create(:user) + project1 = create(:empty_project) + project2 = create(:empty_project) + create(:empty_project) + user1.toggle_star(project1) + user2.toggle_star(project2) + + expect(Project.starred_by(user1)).to contain_exactly(project1) + end + end + describe '.visible_to_user' do let!(:project) { create(:empty_project, :private) } let!(:user) { create(:user) } @@ -1432,16 +1446,13 @@ describe Project, models: true do end describe 'Project import job' do - let(:project) { create(:empty_project) } - let(:mirror) { false } + let(:project) { create(:empty_project, import_url: generate(:url)) } before do allow_any_instance_of(Gitlab::Shell).to receive(:import_repository) .with(project.repository_storage_path, project.path_with_namespace, project.import_url) .and_return(true) - allow(project).to receive(:repository_exists?).and_return(true) - expect_any_instance_of(Repository).to receive(:after_import) .and_call_original end @@ -1449,8 +1460,7 @@ describe Project, models: true do it 'imports a project' do expect_any_instance_of(RepositoryImportWorker).to receive(:perform).and_call_original - project.import_start - project.add_import_job + project.import_schedule expect(project.reload.import_status).to eq('finished') end @@ -1537,7 +1547,7 @@ describe Project, models: true do describe '#add_import_job' do context 'forked' do - let(:forked_project_link) { create(:forked_project_link) } + let(:forked_project_link) { create(:forked_project_link, :forked_to_empty_project) } let(:forked_from_project) { forked_project_link.forked_from_project } let(:project) { forked_project_link.forked_to_project } @@ -1551,9 +1561,9 @@ describe Project, models: true do end context 'not forked' do - let(:project) { create(:empty_project) } - it 'schedules a RepositoryImportWorker job' do + project = create(:empty_project, import_url: generate(:url)) + expect(RepositoryImportWorker).to receive(:perform_async).with(project.id) project.add_import_job @@ -1735,6 +1745,90 @@ describe Project, models: true do end end + describe '#secret_variables_for' do + let(:project) { create(:empty_project) } + + let!(:secret_variable) do + create(:ci_variable, value: 'secret', project: project) + end + + let!(:protected_variable) do + create(:ci_variable, :protected, value: 'protected', project: project) + end + + subject { project.secret_variables_for('ref') } + + shared_examples 'ref is protected' do + it 'contains all the variables' do + is_expected.to contain_exactly(secret_variable, protected_variable) + end + end + + context 'when the ref is not protected' do + before do + stub_application_setting( + default_branch_protection: Gitlab::Access::PROTECTION_NONE) + end + + it 'contains only the secret variables' do + is_expected.to contain_exactly(secret_variable) + end + end + + context 'when the ref is a protected branch' do + before do + create(:protected_branch, name: 'ref', project: project) + end + + it_behaves_like 'ref is protected' + end + + context 'when the ref is a protected tag' do + before do + create(:protected_tag, name: 'ref', project: project) + end + + it_behaves_like 'ref is protected' + end + end + + describe '#protected_for?' do + let(:project) { create(:empty_project) } + + subject { project.protected_for?('ref') } + + context 'when the 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 + end + end + + context 'when the ref is a protected branch' do + before do + create(:protected_branch, name: 'ref', project: project) + end + + it 'returns true' do + is_expected.to be_truthy + end + end + + context 'when the ref is a protected tag' do + before do + create(:protected_tag, name: 'ref', project: project) + end + + it 'returns true' do + is_expected.to be_truthy + end + end + end + describe '#update_project_statistics' do let(:project) { create(:empty_project) } @@ -1909,19 +2003,9 @@ describe Project, models: true do describe '#http_url_to_repo' do let(:project) { create :empty_project } - context 'when no user is given' do - it 'returns the url to the repo without a username' do - expect(project.http_url_to_repo).to eq("#{project.web_url}.git") - expect(project.http_url_to_repo).not_to include('@') - end - end - - context 'when user is given' do - it 'returns the url to the repo with the username' do - user = build_stubbed(:user) - - expect(project.http_url_to_repo(user)).to start_with("http://#{user.username}@") - end + it 'returns the url to the repo without a username' do + expect(project.http_url_to_repo).to eq("#{project.web_url}.git") + expect(project.http_url_to_repo).not_to include('@') end end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index fb2d5f60009..362565506e5 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -327,69 +327,114 @@ describe ProjectTeam, models: true do end end - shared_examples_for "#max_member_access_for_users" do |enable_request_store| - describe "#max_member_access_for_users" do + shared_examples 'max member access for users' do + let(:project) { create(:project) } + let(:group) { create(:group) } + let(:second_group) { create(:group) } + + let(:master) { create(:user) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } + + let(:promoted_guest) { create(:user) } + + let(:group_developer) { create(:user) } + let(:second_developer) { create(:user) } + + let(:user_without_access) { create(:user) } + let(:second_user_without_access) { create(:user) } + + let(:users) do + [master, reporter, promoted_guest, guest, group_developer, second_developer, user_without_access].map(&:id) + end + + let(:expected) do + { + master.id => Gitlab::Access::MASTER, + reporter.id => Gitlab::Access::REPORTER, + promoted_guest.id => Gitlab::Access::DEVELOPER, + guest.id => Gitlab::Access::GUEST, + group_developer.id => Gitlab::Access::DEVELOPER, + second_developer.id => Gitlab::Access::MASTER, + user_without_access.id => Gitlab::Access::NO_ACCESS + } + end + + before do + project.add_master(master) + project.add_reporter(reporter) + project.add_guest(promoted_guest) + project.add_guest(guest) + + project.project_group_links.create( + group: group, + group_access: Gitlab::Access::DEVELOPER + ) + + group.add_master(promoted_guest) + group.add_developer(group_developer) + group.add_developer(second_developer) + + project.project_group_links.create( + group: second_group, + group_access: Gitlab::Access::MASTER + ) + + second_group.add_master(second_developer) + end + + it 'returns correct roles for different users' do + expect(project.team.max_member_access_for_user_ids(users)).to eq(expected) + end + end + + describe '#max_member_access_for_user_ids' do + context 'with RequestStore enabled' do before do - RequestStore.begin! if enable_request_store + RequestStore.begin! end after do - if enable_request_store - RequestStore.end! - RequestStore.clear! - end + RequestStore.end! + RequestStore.clear! end - it 'returns correct roles for different users' do - master = create(:user) - reporter = create(:user) - promoted_guest = create(:user) - guest = create(:user) - project = create(:empty_project) + include_examples 'max member access for users' - project.add_master(master) - project.add_reporter(reporter) - project.add_guest(promoted_guest) - project.add_guest(guest) + def access_levels(users) + project.team.max_member_access_for_user_ids(users) + end + + it 'does not perform extra queries when asked for users who have already been found' do + access_levels(users) + + expect { access_levels(users) }.not_to exceed_query_limit(0) - group = create(:group) - group_developer = create(:user) - second_developer = create(:user) - project.project_group_links.create( - group: group, - group_access: Gitlab::Access::DEVELOPER) - - group.add_master(promoted_guest) - group.add_developer(group_developer) - group.add_developer(second_developer) - - second_group = create(:group) - project.project_group_links.create( - group: second_group, - group_access: Gitlab::Access::MASTER) - second_group.add_master(second_developer) - - users = [master, reporter, promoted_guest, guest, group_developer, second_developer].map(&:id) - - expected = { - master.id => Gitlab::Access::MASTER, - reporter.id => Gitlab::Access::REPORTER, - promoted_guest.id => Gitlab::Access::DEVELOPER, - guest.id => Gitlab::Access::GUEST, - group_developer.id => Gitlab::Access::DEVELOPER, - second_developer.id => Gitlab::Access::MASTER - } - - expect(project.team.max_member_access_for_user_ids(users)).to eq(expected) + expect(access_levels(users)).to eq(expected) end - end - end - describe '#max_member_access_for_users with RequestStore' do - it_behaves_like "#max_member_access_for_users", true - end + it 'only requests the extra users when uncached users are passed' do + new_user = create(:user) + second_new_user = create(:user) + all_users = users + [new_user.id, second_new_user.id] + + expected_all = expected.merge(new_user.id => Gitlab::Access::NO_ACCESS, + second_new_user.id => Gitlab::Access::NO_ACCESS) + + access_levels(users) - describe '#max_member_access_for_users without RequestStore' do - it_behaves_like "#max_member_access_for_users", false + queries = ActiveRecord::QueryRecorder.new { access_levels(all_users) } + + expect(queries.count).to eq(1) + expect(queries.log_message).to match(/\W#{new_user.id}\W/) + expect(queries.log_message).to match(/\W#{second_new_user.id}\W/) + expect(queries.log_message).not_to match(/\W#{promoted_guest.id}\W/) + expect(access_levels(all_users)).to eq(expected_all) + end + end + + context 'with RequestStore disabled' do + include_examples 'max member access for users' + end end end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 969e9f7a130..224067f58dd 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -37,21 +37,11 @@ describe ProjectWiki, models: true do describe "#http_url_to_repo" do let(:project) { create :empty_project } - context 'when no user is given' do - it 'returns the url to the repo without a username' do - expected_url = "#{Gitlab.config.gitlab.url}/#{subject.path_with_namespace}.git" + it 'returns the full http url to the repo' do + expected_url = "#{Gitlab.config.gitlab.url}/#{subject.path_with_namespace}.git" - expect(project_wiki.http_url_to_repo).to eq(expected_url) - expect(project_wiki.http_url_to_repo).not_to include('@') - end - end - - context 'when user is given' do - it 'returns the url to the repo with the username' do - user = build_stubbed(:user) - - expect(project_wiki.http_url_to_repo(user)).to start_with("http://#{user.username}@") - end + expect(project_wiki.http_url_to_repo).to eq(expected_url) + expect(project_wiki.http_url_to_repo).not_to include('@') end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9edf34b05ad..a83726b48a0 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -13,6 +13,10 @@ describe User, models: true do it { is_expected.to include_module(TokenAuthenticatable) } end + describe 'delegations' do + it { is_expected.to delegate_method(:path).to(:namespace).with_prefix } + end + describe 'associations' do it { is_expected.to have_one(:namespace) } it { is_expected.to have_many(:snippets).dependent(:destroy) } @@ -22,7 +26,7 @@ describe User, models: true do it { is_expected.to have_many(:deploy_keys).dependent(:destroy) } it { is_expected.to have_many(:events).dependent(:destroy) } it { is_expected.to have_many(:recent_events).class_name('Event') } - it { is_expected.to have_many(:issues).dependent(:restrict_with_exception) } + it { is_expected.to have_many(:issues).dependent(:destroy) } it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:merge_requests).dependent(:destroy) } it { is_expected.to have_many(:identities).dependent(:destroy) } @@ -1496,25 +1500,6 @@ describe User, models: true do end end - describe '#viewable_starred_projects' do - let(:user) { create(:user) } - let(:public_project) { create(:empty_project, :public) } - let(:private_project) { create(:empty_project, :private) } - let(:private_viewable_project) { create(:empty_project, :private) } - - before do - private_viewable_project.team << [user, Gitlab::Access::MASTER] - - [public_project, private_project, private_viewable_project].each do |project| - user.toggle_star(project) - end - end - - it 'returns only starred projects the user can view' do - expect(user.viewable_starred_projects).not_to include(private_project) - end - end - describe '#projects_with_reporter_access_limited_to' do let(:project1) { create(:empty_project) } let(:project2) { create(:empty_project) } |