From ebede2b3ff1c2b089529c4f9d6268641580e280b Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 12 May 2017 15:19:27 +0200 Subject: Use etag caching for environments JSON For the index view, the environments can now be requested every 15 seconds. Any transition state of a projects environments will trigger a cache invalidation action. Fixes gitlab-org/gitlab-ce#31701 --- spec/models/environment_spec.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 12519de8636..768f557bb9f 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,14 @@ 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 '#nullify_external_url' do it 'replaces a blank url with nil' do env = build(:environment, external_url: "") -- cgit v1.2.1 From 3be9820da63d77ab8b4469dbbb5385292f928057 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Tue, 23 May 2017 10:43:55 +0200 Subject: Test etag caching router and incorporate review --- spec/models/deployment_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'spec/models') diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 4bda7d4314a..9e8acb3812b 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -16,6 +16,14 @@ 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 + it 'invalidates the cache for the environment' do + expect(subject).to receive(:invalidate_cache) + + subject.save! + end + end + describe '#includes_commit?' do let(:project) { create(:project, :repository) } let(:environment) { create(:environment, project: project) } -- cgit v1.2.1 From 9f6dc8b2b5fe3d4790d67f13660eb8bfabd4dbca Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 25 May 2017 18:05:27 +0800 Subject: Add tests and also pass protected vars to protected tags --- spec/models/ci/build_spec.rb | 41 +++++++++++++++++++++++++++++++++++++---- spec/models/project_spec.rb | 30 ++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index e971b4bc3f9..0cc1fc2b360 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1215,16 +1215,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(secure_variable) } + 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 + + 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 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f2b4e9070b4..8478df059bc 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1710,6 +1710,36 @@ describe Project, models: true do end end + describe 'variables' 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 + + describe '#secret_variables' do + it 'contains only the secret variables' do + expect(project.secret_variables).to eq( + [{ key: secret_variable.key, + value: secret_variable.value, + public: false } ]) + end + end + + describe '#protected_variables' do + it 'contains only the protected variables' do + expect(project.protected_variables).to eq( + [{ key: protected_variable.key, + value: protected_variable.value, + public: false } ]) + end + end + end + describe '#update_project_statistics' do let(:project) { create(:empty_project) } -- cgit v1.2.1 From efebdba21db9e11b876ecb54db48358e13b08ad3 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 25 May 2017 19:31:21 +0800 Subject: Frontend implementation, tests, and changelog --- spec/models/project_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8478df059bc..b9094387865 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1726,7 +1726,7 @@ describe Project, models: true do expect(project.secret_variables).to eq( [{ key: secret_variable.key, value: secret_variable.value, - public: false } ]) + public: false }]) end end @@ -1735,7 +1735,7 @@ describe Project, models: true do expect(project.protected_variables).to eq( [{ key: protected_variable.key, value: protected_variable.value, - public: false } ]) + public: false }]) end end end -- cgit v1.2.1 From b74b7309a5121be50fe6c07c85b61e258c113ce6 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 25 May 2017 22:14:22 +0800 Subject: Add tests for CI_ENVIRONMENT_URL --- spec/models/ci/build_spec.rb | 59 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 6 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index e971b4bc3f9..5b2c1472974 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -20,6 +20,7 @@ describe Ci::Build, :models do it { is_expected.to validate_presence_of(:ref) } it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } + it { is_expected.to validate_length_of(:external_url).is_at_most(255) } describe '#actionize' do context 'when build is a created' do @@ -427,6 +428,30 @@ describe Ci::Build, :models do end end + describe '#expanded_environment_url' do + subject { build.expanded_environment_url } + + context 'when environment uses $CI_COMMIT_REF_NAME' do + let(:build) do + create(:ci_build, + ref: 'master', + environment_url: 'http://review/$CI_COMMIT_REF_NAME') + end + + it { is_expected.to eq('http://review/master') } + end + + context 'when environment uses yaml_variables containing symbol keys' do + let(:build) do + create(:ci_build, + yaml_variables: [{ key: :APP_HOST, value: 'host' }], + environment_url: 'http://review/$APP_HOST') + end + + it { is_expected.to eq('http://review/host') } + end + end + describe '#starts_environment?' do subject { build.starts_environment? } @@ -1176,11 +1201,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 +1208,34 @@ describe Ci::Build, :models do ] end - it { environment_variables.each { |v| is_expected.to include(v) } } + before do + build.update(environment: 'production') + create(:environment, project: build.project, name: 'production', slug: 'prod-slug') + end + + context 'when no URL was set' do + it { environment_variables.each { |v| is_expected.to include(v) } } + + it 'does not have CI_ENVIRONMENT_URL' do + keys = subject.map { |var| var[:key] } + + expect(keys).to include('CI_ENVIRONMENT_NAME', 'CI_ENVIRONMENT_SLUG') + expect(keys).not_to include('CI_ENVIRONMENT_URL') + end + end + + context 'when an URL was set' do + before do + build.update(environment_url: 'http://host/$CI_JOB_NAME') + + environment_variables << + { key: 'CI_ENVIRONMENT_URL', + value: 'http://host/test', + public: true } + end + + it { environment_variables.each { |v| is_expected.to include(v) } } + end end context 'when build started manually' do -- cgit v1.2.1 From 88ba7a034be6c2a93c495edd1d1db08ec6d98bb3 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 25 May 2017 22:58:51 +0800 Subject: Pass external_url from environment if job doesn't have one Also update the document and add a changelog entry --- spec/models/ci/build_spec.rb | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 5b2c1472974..2df439819ef 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -20,7 +20,7 @@ describe Ci::Build, :models do it { is_expected.to validate_presence_of(:ref) } it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } - it { is_expected.to validate_length_of(:external_url).is_at_most(255) } + it { is_expected.to validate_length_of(:environment_url).is_at_most(255) } describe '#actionize' do context 'when build is a created' do @@ -1208,33 +1208,55 @@ describe Ci::Build, :models do ] end + let!(:environment) do + create(:environment, + project: build.project, + name: 'production', + slug: 'prod-slug', + external_url: '') + end + before do build.update(environment: 'production') - create(:environment, project: build.project, name: 'production', slug: 'prod-slug') end - context 'when no URL was set' do + 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).to include('CI_ENVIRONMENT_NAME', 'CI_ENVIRONMENT_SLUG') expect(keys).not_to include('CI_ENVIRONMENT_URL') end end context 'when an URL was set' do - before do - build.update(environment_url: 'http://host/$CI_JOB_NAME') + let(:url) { 'http://host/test' } + before do environment_variables << - { key: 'CI_ENVIRONMENT_URL', - value: 'http://host/test', - public: true } + { key: 'CI_ENVIRONMENT_URL', value: url, public: true } end - it { environment_variables.each { |v| is_expected.to include(v) } } + context 'when the URL was set from the job' do + before do + build.update(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 -- cgit v1.2.1 From 2785bc4faa0eb5c46b0c8b46b77aa4cf1d4ee4b4 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Sat, 27 May 2017 01:46:57 +0800 Subject: Merge secret and protected vars to variables_for(ref) Also introduce Ci::Variable#to_runner_variable to build up the hash for runner. --- spec/models/ci/build_spec.rb | 2 +- spec/models/ci/variable_spec.rb | 7 +++++++ spec/models/project_spec.rb | 42 +++++++++++++++++++++++++++++------------ 3 files changed, 38 insertions(+), 13 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 0cc1fc2b360..6e7aa3d5841 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1384,7 +1384,7 @@ describe Ci::Build, :models do 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(project).to receive(:variables_for).with(build.ref) { ['secret'] } end it { is_expected.to eq(%w[predefined project pipeline yaml secret]) } diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index fe8c52d5353..38b869f59ae 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -36,4 +36,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/project_spec.rb b/spec/models/project_spec.rb index b9094387865..7e5e6e899e2 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1710,7 +1710,7 @@ describe Project, models: true do end end - describe 'variables' do + describe '#variables_for' do let(:project) { create(:empty_project) } let!(:secret_variable) do @@ -1721,22 +1721,40 @@ describe Project, models: true do create(:ci_variable, :protected, value: 'protected', project: project) end - describe '#secret_variables' do + subject { project.variables_for('ref') } + + shared_examples 'ref is protected' do + it 'contains all the variables' do + is_expected.to contain_exactly( + *[secret_variable, protected_variable].map(&:to_runner_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 - expect(project.secret_variables).to eq( - [{ key: secret_variable.key, - value: secret_variable.value, - public: false }]) + is_expected.to contain_exactly(secret_variable.to_runner_variable) end end - describe '#protected_variables' do - it 'contains only the protected variables' do - expect(project.protected_variables).to eq( - [{ key: protected_variable.key, - value: protected_variable.value, - public: false }]) + 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 -- cgit v1.2.1 From aed0387f97ec62b184da90bdca0ce40f9dc58b45 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 26 May 2017 18:27:30 -0500 Subject: Consistent diff and blob size limit names --- spec/models/blob_viewer/base_spec.rb | 54 ++++++++++++++++++------------------ spec/models/merge_request_spec.rb | 4 +-- 2 files changed, 29 insertions(+), 29 deletions(-) (limited to 'spec/models') diff --git a/spec/models/blob_viewer/base_spec.rb b/spec/models/blob_viewer/base_spec.rb index 92fbf64a6b7..0749ff78777 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,77 @@ 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 + expect(viewer.too_large?).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 + describe '#can_expanded?' do + context 'when the blob size is larger than the collapse limit' 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 false' do - expect(viewer.can_override_max_size?).to be_falsey + expect(viewer.can_expanded?).to be_falsey 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 true' do - expect(viewer.can_override_max_size?).to be_truthy + expect(viewer.can_expanded?).to be_truthy end 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.can_override_max_size?).to be_falsey + expect(viewer.can_expanded?).to be_falsey end end end describe '#render_error' do - context 'when the max size is overridden' do + context 'when the size limit is overridden' 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 +147,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,8 +156,8 @@ 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 the size limit is not overridden' 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 @@ -165,7 +165,7 @@ describe BlobViewer::Base, model: true do 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/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 712470d6bf5..58f273ade28 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 -- cgit v1.2.1 From fd376b3ed4de310f8e599cbbeb5c18e5a31c188c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 30 May 2017 14:11:58 +0200 Subject: Revert "Merge branch '1937-https-clone-url-username' into 'master' " MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit c425f366bfa84efab92b5d5e1d0721f16a2890bc, reversing changes made to 82f6c0f5ac4ed29390ed90592d2c431f3494d93f. Signed-off-by: Rémy Coutable --- spec/models/project_spec.rb | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 36575acf671..2ef3d5654d5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1909,19 +1909,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 -- cgit v1.2.1 From bf4cc9e1f3ca6e4d828eb2bc1ca22d4f350c9dc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 30 May 2017 14:18:58 +0200 Subject: Don't allow to pass a user to ProjectWiki#http_url_to_repo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This partially reverts be25bbc4d2c7e3d5cf3da6f51cb7f7355295ef52. Signed-off-by: Rémy Coutable --- spec/models/project_wiki_spec.rb | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) (limited to 'spec/models') 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 -- cgit v1.2.1 From ab8d54b26befb4b70988a514759c7c4d9fd7630d Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Thu, 18 May 2017 08:07:48 +0200 Subject: =?UTF-8?q?Don=E2=80=99t=20create=20comment=20on=20JIRA=20if=20lin?= =?UTF-8?q?k=20already=20exists?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/models/project_services/jira_service_spec.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'spec/models') diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 349067e73ab..1920b5bf42b 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -133,6 +133,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 -- cgit v1.2.1 From 8e72ad70bd2479ae5a465eac1df74f99f03ea731 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Tue, 23 May 2017 22:40:07 +0200 Subject: Add starred_by scope to Project Add a scope to search for the projects that are starred by a certain user. --- spec/models/project_spec.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2ef3d5654d5..da1b29a2bda 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -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) } -- cgit v1.2.1 From 1e5506d01619780da68fc51ada58188a9070255b Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Tue, 30 May 2017 23:24:17 +0200 Subject: Remove some deprecated methods To avoid the use of slow queries, remove some deprecated methods and encourage the use of ProjectFinder to find projects. --- spec/models/user_spec.rb | 19 ------------------- 1 file changed, 19 deletions(-) (limited to 'spec/models') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9edf34b05ad..fe9df3360ff 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1496,25 +1496,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) } -- cgit v1.2.1 From ce869e3964a40b4cf04a803f5201d940ad61b13c Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 30 May 2017 21:48:30 -0500 Subject: Fix Diff#too_large? and specs --- spec/models/blob_viewer/base_spec.rb | 36 ++++-------------------------------- 1 file changed, 4 insertions(+), 32 deletions(-) (limited to 'spec/models') diff --git a/spec/models/blob_viewer/base_spec.rb b/spec/models/blob_viewer/base_spec.rb index 0749ff78777..d56379eb59d 100644 --- a/spec/models/blob_viewer/base_spec.rb +++ b/spec/models/blob_viewer/base_spec.rb @@ -105,36 +105,8 @@ describe BlobViewer::Base, model: true do end end - describe '#can_expanded?' do - context 'when the blob size is larger than the collapse limit' 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 false' do - expect(viewer.can_expanded?).to be_falsey - end - end - - context 'when the blob size is smaller than the size limit' do - let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } - - it 'returns true' do - expect(viewer.can_expanded?).to be_truthy - end - end - end - - 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.can_expanded?).to be_falsey - end - end - end - describe '#render_error' do - context 'when the size limit is overridden' do + context 'when expanded' do before do viewer.expanded = true end @@ -156,12 +128,12 @@ describe BlobViewer::Base, model: true do end end - context 'when the size limit is not overridden' 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 -- cgit v1.2.1 From fb1b7b00f3f76e0c0ace91b5dfea63408c24de08 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 11:20:36 +0200 Subject: Fix environment model specs related to protected actions --- spec/models/environment_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 12519de8636..9fbe19b04d5 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -227,7 +227,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 -- cgit v1.2.1 From 07c984d81cd7985d4ab7597cbb21b5f623b438e9 Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Thu, 18 May 2017 13:24:34 +0100 Subject: Port fix-realtime-edited-text-for-issues 9-2-stable fix to master. --- spec/models/concerns/editable_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 spec/models/concerns/editable_spec.rb (limited to 'spec/models') 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 -- cgit v1.2.1 From 161af17c1b69e7e00aefcd4f540a55755259ceda Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 24 May 2017 15:13:51 +0200 Subject: Introduce source to pipeline entity --- spec/models/ci/pipeline_spec.rb | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) (limited to 'spec/models') 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 -- cgit v1.2.1 From c4dded593a9df770dd08051fc645f713ca295f13 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 31 May 2017 22:45:51 +0800 Subject: Update docs and use protected secret variable as the name --- spec/models/ci/build_spec.rb | 27 +++++++++++++++++++++------ spec/models/project_spec.rb | 4 ++-- 2 files changed, 23 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 6e7aa3d5841..e2406290c6c 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1379,15 +1379,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(:variables_for).with(build.ref) { ['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/project_spec.rb b/spec/models/project_spec.rb index 38964f278f3..36140b519d6 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1735,7 +1735,7 @@ describe Project, models: true do end end - describe '#variables_for' do + describe '#secret_variables_for' do let(:project) { create(:empty_project) } let!(:secret_variable) do @@ -1746,7 +1746,7 @@ describe Project, models: true do create(:ci_variable, :protected, value: 'protected', project: project) end - subject { project.variables_for('ref') } + subject { project.secret_variables_for('ref') } shared_examples 'ref is protected' do it 'contains all the variables' do -- cgit v1.2.1 From 65563c7b1b64d3ab7133896ba291a3efc7afba86 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 1 Jun 2017 00:12:42 +0800 Subject: Now secret_variables_for would return the variables --- spec/models/project_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 36140b519d6..a953faaaedf 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1750,8 +1750,7 @@ describe Project, models: true do shared_examples 'ref is protected' do it 'contains all the variables' do - is_expected.to contain_exactly( - *[secret_variable, protected_variable].map(&:to_runner_variable)) + is_expected.to contain_exactly(secret_variable, protected_variable) end end @@ -1762,7 +1761,7 @@ describe Project, models: true do end it 'contains only the secret variables' do - is_expected.to contain_exactly(secret_variable.to_runner_variable) + is_expected.to contain_exactly(secret_variable) end end -- cgit v1.2.1 From f4aa01053ed39a265ddfd9ee6e9618bd3406e59d Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Tue, 30 May 2017 23:56:26 +0200 Subject: Test etag cache key changing value --- spec/models/environment_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'spec/models') diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 768f557bb9f..b6162908e12 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -42,6 +42,18 @@ describe Environment, models: true do 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: "") -- cgit v1.2.1 From 09838ac626df739f0ab9852e3c7d862c7fd707e5 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 31 May 2017 14:31:33 -0500 Subject: Update diff discussion position per discussion instead of per note --- spec/models/diff_note_spec.rb | 27 ++++----------------------- spec/models/merge_request_spec.rb | 10 +++++----- 2 files changed, 9 insertions(+), 28 deletions(-) (limited to 'spec/models') 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/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 712470d6bf5..6383f112543 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -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) -- cgit v1.2.1 From 17c926313a242c6ad988a7ffd55aa6330c8aacfd Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 1 Jun 2017 03:49:10 +0800 Subject: Add test for Project#protected_for? --- spec/models/project_spec.rb | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 360fcae29a5..86ab2550bfb 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1796,6 +1796,43 @@ describe Project, models: true do 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) } -- cgit v1.2.1 From 6fad5640e73837ba148b0ed005205adff91d8a4e Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Mon, 29 May 2017 11:29:43 +0200 Subject: fix failing specs --- spec/models/project_services/jira_service_spec.rb | 35 ----------------------- 1 file changed, 35 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 1920b5bf42b..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) } -- cgit v1.2.1 From 7f73f440f9a4a8761c083a6781d3c4015228d9b9 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Wed, 31 May 2017 16:45:14 +0100 Subject: Fix N+1 queries for non-members in comment threads When getting the max member access for a group of users, we stored the results in RequestStore. However, this will only return results for project members, so anyone who wasn't a member of the project would be checked once at the start, and then once for each comment they made. These queries are generally quite fast, but no query is faster! --- spec/models/project_team_spec.rb | 151 +++++++++++++++++++++++++-------------- 1 file changed, 98 insertions(+), 53 deletions(-) (limited to 'spec/models') 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 -- cgit v1.2.1 From 4f8a1c0d4cc9372823cb28a16936c49dd4f09402 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 1 Jun 2017 18:29:11 +0800 Subject: Just use the url from options, not saving it as a column --- spec/models/ci/build_spec.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2df439819ef..141a2741ced 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -20,7 +20,6 @@ describe Ci::Build, :models do it { is_expected.to validate_presence_of(:ref) } it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } - it { is_expected.to validate_length_of(:environment_url).is_at_most(255) } describe '#actionize' do context 'when build is a created' do @@ -435,7 +434,7 @@ describe Ci::Build, :models do let(:build) do create(:ci_build, ref: 'master', - environment_url: 'http://review/$CI_COMMIT_REF_NAME') + options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } }) end it { is_expected.to eq('http://review/master') } @@ -445,7 +444,7 @@ describe Ci::Build, :models do let(:build) do create(:ci_build, yaml_variables: [{ key: :APP_HOST, value: 'host' }], - environment_url: 'http://review/$APP_HOST') + options: { environment: { url: 'http://review/$APP_HOST' } }) end it { is_expected.to eq('http://review/host') } @@ -1244,7 +1243,7 @@ describe Ci::Build, :models do context 'when the URL was set from the job' do before do - build.update(environment_url: 'http://host/$CI_JOB_NAME') + build.update(options: { environment: { url: 'http://host/$CI_JOB_NAME' } }) end it_behaves_like 'containing environment variables' -- cgit v1.2.1 From 66edbc5e5cfe49984069512a9e550df9498497d8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 1 Jun 2017 19:07:57 +0800 Subject: Introduce ci_environment_url which would fallback to the external_url from persisted environment, and make the other utility methods private as we don't really need to use them outside of the job. --- spec/models/ci/build_spec.rb | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 141a2741ced..0c68ac20c7d 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -427,11 +427,11 @@ describe Ci::Build, :models do end end - describe '#expanded_environment_url' do - subject { build.expanded_environment_url } + describe '#ci_environment_url' do + subject { job.ci_environment_url } - context 'when environment uses $CI_COMMIT_REF_NAME' do - let(:build) do + 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' } }) @@ -440,8 +440,8 @@ describe Ci::Build, :models do it { is_expected.to eq('http://review/master') } end - context 'when environment uses yaml_variables containing symbol keys' do - let(:build) do + 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' } }) @@ -449,6 +449,18 @@ describe Ci::Build, :models do 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 -- cgit v1.2.1 From db17e6ad8fe4e91f13fe9028a162236faad7f214 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 1 Jun 2017 21:09:36 +0800 Subject: split tests for expanded_environment_url --- spec/models/ci/build_spec.rb | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 0c68ac20c7d..5a5702e5e8d 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -427,8 +427,8 @@ describe Ci::Build, :models do end end - describe '#ci_environment_url' do - subject { job.ci_environment_url } + describe '#expanded_environment_url' do + subject { job.expanded_environment_url } context 'when yaml environment uses $CI_COMMIT_REF_NAME' do let(:job) do @@ -453,10 +453,31 @@ describe Ci::Build, :models do 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) + it { is_expected.to be_nil } + end + end + + describe '#ci_environment_url' do + subject { job.ci_environment_url } + + let!(:environment) do + create(:environment, project: job.project, name: job.environment) + end + + context 'when yaml environment has url' do + let(:job) do + create(:ci_build, + ref: 'master', + environment: 'staging', + options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } }) end + it { is_expected.to eq('http://review/master') } + end + + context 'when yaml environment does not have url' do + let(:job) { create(:ci_build, environment: 'staging') } + it 'returns the external_url from persisted environment' do is_expected.to eq(environment.external_url) end -- cgit v1.2.1 From 7da88278c33d20d97cf0eabb76b3117219479d12 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 1 Jun 2017 21:48:31 +0800 Subject: Make sure protected can't be null; Test protected! --- spec/models/ci/variable_spec.rb | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index 38b869f59ae..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 -- cgit v1.2.1 From 7193108c9314a50287184f4d2b21d0f8ec12f65f Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 1 Jun 2017 22:52:01 +0800 Subject: Merge all environment url methods, introduce ensure_persisted_environment To make sure that we're accessing the same instance, so ending up with `env.external` = `env.external_url` shall be fine. --- spec/models/ci/build_spec.rb | 63 ++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 25 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 5a5702e5e8d..586acfacff8 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -427,8 +427,8 @@ describe Ci::Build, :models do end end - describe '#expanded_environment_url' do - subject { job.expanded_environment_url } + describe '#environment_url' do + subject { job.environment_url } context 'when yaml environment uses $CI_COMMIT_REF_NAME' do let(:job) do @@ -453,31 +453,10 @@ describe Ci::Build, :models do context 'when yaml environment does not have url' do let(:job) { create(:ci_build, environment: 'staging') } - it { is_expected.to be_nil } - end - end - - describe '#ci_environment_url' do - subject { job.ci_environment_url } - - let!(:environment) do - create(:environment, project: job.project, name: job.environment) - end - - context 'when yaml environment has url' do - let(:job) do - create(:ci_build, - ref: 'master', - environment: 'staging', - options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } }) + let!(:environment) do + create(:environment, project: job.project, name: job.environment) end - it { is_expected.to eq('http://review/master') } - end - - context 'when yaml environment does not have url' do - let(:job) { create(:ci_build, environment: 'staging') } - it 'returns the external_url from persisted environment' do is_expected.to eq(environment.external_url) end @@ -975,6 +954,40 @@ describe Ci::Build, :models do it { is_expected.to eq(environment) } end + + context 'when there is not environment' do + it { is_expected.to be_nil } + end + end + + describe '#ensure_persisted_environment' do + subject { job.ensure_persisted_environment } + + let(:job) do + create(:ci_build, + ref: 'master', + environment: 'staging/$CI_COMMIT_REF_NAME') + end + + context 'when there is no environment' do + it 'creates one by the expanded name' do + expect do + expect(subject.name).to eq('staging/master') + end.to change { Environment.count }.by(1) + end + end + + context 'when there is already an environment' do + let!(:environment) do + create(:environment, project: job.project, name: 'staging/master') + end + + it 'returns the existing environment' do + expect do + expect(subject).to eq(environment) + end.to change { Environment.count }.by(0) + end + end end describe '#play' do -- cgit v1.2.1 From 696b0395116f9f973c919b281a1a5df023f22084 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Thu, 1 Jun 2017 16:59:18 +0200 Subject: Explicitly test etag cache invalidation When a new deployment is created, this will trigger the invalidation on the etag cache on the environment. This flow is now explicitly tested. --- spec/models/deployment_spec.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'spec/models') diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 9e8acb3812b..6f0d2db23c7 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -17,10 +17,15 @@ describe Deployment, models: true do it { is_expected.to validate_presence_of(:sha) } describe 'after_create callbacks' do - it 'invalidates the cache for the environment' do - expect(subject).to receive(:invalidate_cache) + let(:environment) { create(:environment) } + let(:store) { Gitlab::EtagCaching::Store.new } - subject.save! + 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 -- cgit v1.2.1 From e9a98d3d2dbf8d7d9c181a5505c5d3a3200d711b Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 1 Jun 2017 23:00:08 +0800 Subject: Fix a typo: not -> no --- spec/models/ci/build_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 586acfacff8..9a9c5f5a742 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -955,7 +955,7 @@ describe Ci::Build, :models do it { is_expected.to eq(environment) } end - context 'when there is not environment' do + context 'when there is no environment' do it { is_expected.to be_nil } end end -- cgit v1.2.1 From 7db09c63cc4532acea2d736f667b36c96b22007d Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Thu, 1 Jun 2017 17:30:01 +0100 Subject: Fix hard-deleting users when they have authored issues --- spec/models/user_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index fe9df3360ff..1c3541da44f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -22,7 +22,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) } -- cgit v1.2.1 From 3731ae092cac523a0092c64000d9e838662a85df Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 2 Jun 2017 15:19:34 +0800 Subject: CreatePipelineBuildsService would have created env So we don't have to do it in CreateDeploymentService Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11695#note_31322649 --- spec/models/ci/build_spec.rb | 30 ------------------------------ 1 file changed, 30 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2ad7f27ffac..22ee469dd86 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -960,36 +960,6 @@ describe Ci::Build, :models do end end - describe '#ensure_persisted_environment' do - subject { job.ensure_persisted_environment } - - let(:job) do - create(:ci_build, - ref: 'master', - environment: 'staging/$CI_COMMIT_REF_NAME') - end - - context 'when there is no environment' do - it 'creates one by the expanded name' do - expect do - expect(subject.name).to eq('staging/master') - end.to change { Environment.count }.by(1) - end - end - - context 'when there is already an environment' do - let!(:environment) do - create(:environment, project: job.project, name: 'staging/master') - end - - it 'returns the existing environment' do - expect do - expect(subject).to eq(environment) - end.to change { Environment.count }.by(0) - end - end - end - describe '#play' do let(:build) { create(:ci_build, :manual, pipeline: pipeline) } -- cgit v1.2.1 From 857d039145bccaa81da1e7654e51eee9e4b4823a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 31 May 2017 15:43:19 +0200 Subject: Lint our factories creation in addition to their build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/models/key_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'spec/models') 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 -- cgit v1.2.1 From 3f80281d9c07e47cb5cf921add9f5933763ad3df Mon Sep 17 00:00:00 2001 From: vanadium23 Date: Thu, 1 Jun 2017 08:04:16 +0300 Subject: Add slugify project path to CI enviroment variables --- spec/models/ci/build_spec.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index e2406290c6c..55bfeb6eb41 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1139,6 +1139,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 }, -- cgit v1.2.1 From 158581a447bb4976161eca26ddcb2fccd25888ab Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 2 Jun 2017 14:18:24 +0100 Subject: Refactor the DeleteUserWorker --- spec/models/abuse_report_spec.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'spec/models') 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 -- cgit v1.2.1 From 223d07b38315a68fe39df90a7675f043d8b7acaf Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Jun 2017 18:38:09 +0200 Subject: Environments#additional_metrics tests --- spec/models/environment_spec.rb | 93 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) (limited to 'spec/models') diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 12519de8636..12de9c9111b 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -409,6 +409,99 @@ describe Environment, models: true do end end + describe '#has_metrics?' do + subject { environment.has_metrics? } + + context 'when the enviroment is available' do + context 'with a deployment service' do + let(:project) { create(:prometheus_project) } + + context 'and a deployment' do + let!(:deployment) { create(:deployment, environment: environment) } + it { is_expected.to be_truthy } + end + + context 'but no deployments' do + it { is_expected.to be_falsy } + end + end + + context 'without a monitoring service' do + it { is_expected.to be_falsy } + end + end + + context 'when the environment is unavailable' do + let(:project) { create(:prometheus_project) } + + before do + environment.stop + end + + it { is_expected.to be_falsy } + end + end + + describe '#additional_metrics' do + let(:project) { create(:prometheus_project) } + subject { environment.additional_metrics } + + context 'when the environment has additional metrics' do + before do + allow(environment).to receive(:has_additional_metrics?).and_return(true) + end + + it 'returns the additional metrics from the deployment service' do + expect(project.monitoring_service).to receive(:reactive_query) + .with(Gitlab::Prometheus::Queries::AdditionalMetricsQuery.name, environment.id) + .and_return(:fake_metrics) + + is_expected.to eq(:fake_metrics) + end + end + + context 'when the environment does not have metrics' do + before do + allow(environment).to receive(:has_additional_metrics?).and_return(false) + end + + it { is_expected.to be_nil } + end + end + + describe '#has_additional_metrics??' do + subject { environment.has_metrics? } + + context 'when the enviroment is available' do + context 'with a deployment service' do + let(:project) { create(:prometheus_project) } + + context 'and a deployment' do + let!(:deployment) { create(:deployment, environment: environment) } + it { is_expected.to be_truthy } + end + + context 'but no deployments' do + it { is_expected.to be_falsy } + end + end + + context 'without a monitoring service' do + it { is_expected.to be_falsy } + end + end + + context 'when the environment is unavailable' do + let(:project) { create(:prometheus_project) } + + before do + environment.stop + end + + it { is_expected.to be_falsy } + end + end + describe '#slug' do it "is automatically generated" do expect(environment.slug).not_to be_nil -- cgit v1.2.1 From 810866ecb6c7be4fdac88dc3b2a6cd9ad49ac7bf Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Thu, 1 Jun 2017 15:27:35 +0100 Subject: backports changed import logic from pull mirroring feature into CE --- spec/models/project_spec.rb | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index da1b29a2bda..947cb36dcd8 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) } @@ -1446,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 @@ -1463,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 @@ -1551,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 } @@ -1565,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 -- cgit v1.2.1 From c0a66dbd2dd8b716e809938d20e7655d84595176 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Jun 2017 11:16:15 +0200 Subject: Fix prometheus service tests --- spec/models/project_services/prometheus_service_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 1f9d3c07b51..b761567ad76 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -61,7 +61,7 @@ describe PrometheusService, models: true, caching: true do end it 'returns reactive data' do - is_expected.to eq(prometheus_data) + is_expected.to eq(prometheus_metrics_data) end end end @@ -82,7 +82,7 @@ describe PrometheusService, models: true, caching: true do end it 'returns reactive data' do - is_expected.to eq(prometheus_data.merge(deployment_time: deployment.created_at.to_i)) + is_expected.to eq(prometheus_metrics_data.merge(deployment_time: deployment.created_at.to_i)) end end end @@ -112,6 +112,7 @@ describe PrometheusService, models: true, caching: true do end it { expect(subject.to_json).to eq(prometheus_data.to_json) } + it { expect(subject.to_json).to eq(prometheus_data.to_json) } end [404, 500].each do |status| -- cgit v1.2.1 From 8df7bcf532c9f0407fcefa12d205cb9d160fe5f4 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Fri, 19 May 2017 09:07:38 -0500 Subject: Allow numeric pages domain Previously, `PagesDomain` would not allow a domain such as 123.example.com. With this change, this is now allowed, because it is a perfectly valid domain. --- spec/models/pages_domain_spec.rb | 51 ++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 31 deletions(-) (limited to 'spec/models') 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 -- cgit v1.2.1 From d919f924bf32220237c389dc913093efead8928c Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 6 Jun 2017 21:42:45 +0800 Subject: Backport https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1942 --- spec/models/user_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'spec/models') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 1c3541da44f..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) } -- cgit v1.2.1 From ccf89acc7145bb129f5666108854daa71a022827 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Jun 2017 16:07:33 +0200 Subject: expand Namespaces:: and refactoring yaml parsing out of MetricGroup class --- spec/models/environment_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 12de9c9111b..e25d2bb6955 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -453,7 +453,7 @@ describe Environment, models: true do it 'returns the additional metrics from the deployment service' do expect(project.monitoring_service).to receive(:reactive_query) - .with(Gitlab::Prometheus::Queries::AdditionalMetricsQuery.name, environment.id) + .with(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, environment.id) .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) -- cgit v1.2.1 From bf6961cad85ab9a60f37f9b2974fb7bdd3975810 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 6 Jun 2017 23:36:41 +0800 Subject: Just let the user to create the namespace --- spec/models/forked_project_link_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') 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) -- cgit v1.2.1 From 6b53add3f93caffa830e3bdbb3d8fd8d674ee3aa Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 6 Jun 2017 16:40:07 +0000 Subject: Fix binary encoding error on MR diffs --- spec/models/merge_request_diff_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'spec/models') 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 -- cgit v1.2.1 From a7e1205219387a6d24c8579994f73a33b3028010 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 7 Jun 2017 02:36:59 +0200 Subject: Use explicit instance of prometheus service and add access methods to it --- spec/models/deployment_spec.rb | 29 +++++++++++++++++++++++++++++ spec/models/environment_spec.rb | 10 +++++++--- 2 files changed, 36 insertions(+), 3 deletions(-) (limited to 'spec/models') diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 4bda7d4314a..bbb7dbf0922 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -77,6 +77,35 @@ describe Deployment, models: true do end end + describe '#additional_metrics' do + let(:deployment) { create(:deployment) } + + subject { deployment.additional_metrics } + + context 'metrics are disabled' do + it { is_expected.to eq({}) } + end + + context 'metrics are enabled' do + let(:simple_metrics) do + { + success: true, + metrics: {}, + last_update: 42 + } + end + + let(:prometheus_service) { double('prometheus_service') } + + before do + allow(deployment).to receive(:prometheus_service).and_return(prometheus_service) + allow(prometheus_service).to receive(:additional_deployment_metrics).and_return(simple_metrics) + end + + it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } + end + end + describe '#stop_action' do let(:build) { create(:ci_build) } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index e25d2bb6955..8c9d093fc48 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -452,8 +452,8 @@ describe Environment, models: true do end it 'returns the additional metrics from the deployment service' do - expect(project.monitoring_service).to receive(:reactive_query) - .with(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, environment.id) + expect(environment.prometheus_service).to receive(:additional_environment_metrics) + .with(environment) .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) @@ -470,7 +470,11 @@ describe Environment, models: true do end describe '#has_additional_metrics??' do - subject { environment.has_metrics? } + subject { environment.has_additional_metrics? } + + before do + + end context 'when the enviroment is available' do context 'with a deployment service' do -- cgit v1.2.1 From a924152219c1367bf494f3f387d050ac3ff2d7d3 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 7 Jun 2017 04:07:47 +0200 Subject: Remove unecessary before block --- spec/models/environment_spec.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 8c9d093fc48..c8709409cea 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -472,10 +472,6 @@ describe Environment, models: true do describe '#has_additional_metrics??' do subject { environment.has_additional_metrics? } - before do - - end - context 'when the enviroment is available' do context 'with a deployment service' do let(:project) { create(:prometheus_project) } -- cgit v1.2.1