From db0b7fb39e921728385b3287d206aabbeb88690e Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 17 Jul 2017 19:36:29 +0100 Subject: Expire ETag cache on note when award emoji changes --- spec/models/award_emoji_spec.rb | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'spec/models') diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index 2a9a27752c1..2925fc1271c 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -41,4 +41,40 @@ describe AwardEmoji, models: true do end end end + + describe 'expiring ETag cache' do + context 'on a note' do + let(:note) { create(:note_on_issue) } + let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: note) } + + it 'calls expire_etag_cache on the note when saved' do + expect(note).to receive(:expire_etag_cache) + + award_emoji.save! + end + + it 'calls expire_etag_cache on the note when destroyed' do + expect(note).to receive(:expire_etag_cache) + + award_emoji.destroy! + end + end + + context 'on another awardable' do + let(:issue) { create(:issue) } + let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: issue) } + + it 'does not call expire_etag_cache on the issue when saved' do + expect(issue).not_to receive(:expire_etag_cache) + + award_emoji.save! + end + + it 'does not call expire_etag_cache on the issue when destroyed' do + expect(issue).not_to receive(:expire_etag_cache) + + award_emoji.destroy! + end + end + end end -- cgit v1.2.1 From 919c0c7ba7369970bf57e9388333af120b74203d Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Wed, 16 Aug 2017 13:18:07 +0200 Subject: Extended UTs for the new cherry-pick message format --- spec/models/commit_spec.rb | 24 ++++++++++++++++++++++++ spec/models/repository_spec.rb | 5 ++++- 2 files changed, 28 insertions(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index c18c635d811..9711222b3a1 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -195,6 +195,30 @@ eos it { expect(data[:removed]).to eq([]) } end + describe '#cherry_pick_message' do + let(:regular_commit) { project.commit('video') } + let(:merge_commit) { project.commit('wip') } + + context 'of a regular commit' do + it { expect(regular_commit.cherry_pick_message(project, 'master')).to include("\n\n(cherry picked from commit 88790590ed1337ab189bccaa355f068481c90bec)") } + end + + context 'of a merge commit' do + it do + expected_appended_text = <<~STR.rstrip + + + (cherry picked from commit b9238ee5bf1d7359dd3b8c89fd76c1c7f8b75aba) + + 6d664995 This commit will be fixupped against + 64117577 fixup! This commit will be fixupped against + STR + + expect(merge_commit.cherry_pick_message(project, 'master')).to include(expected_appended_text) + end + end + end + describe '#reverts_commit?' do let(:another_commit) { double(:commit, revert_description: "This reverts commit #{commit.sha}") } let(:user) { commit.author } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 4926d5d6c49..c407799e47e 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1344,8 +1344,11 @@ describe Repository, models: true do it 'cherry-picks the changes' do expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).to be_nil - repository.cherry_pick(user, pickable_merge, 'improve/awesome') + cherry_pick_commit_sha = repository.cherry_pick(user, pickable_merge, 'improve/awesome') + cherry_pick_commit_message = project.commit(cherry_pick_commit_sha).message + expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).not_to be_nil + expect(cherry_pick_commit_message).to include('cherry picked from') end end end -- cgit v1.2.1 From c0f921606c91e3e5c601497f57becbf4c6a8e3ac Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Thu, 17 Aug 2017 20:26:06 +0200 Subject: Correct the cherry-pick message for merge commits The list of commits must be generated from the merge request, not from a diff of the branches. --- spec/models/commit_spec.rb | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 9711222b3a1..98054b91c29 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -196,25 +196,52 @@ eos end describe '#cherry_pick_message' do - let(:regular_commit) { project.commit('video') } - let(:merge_commit) { project.commit('wip') } + let(:user) { create(:user) } context 'of a regular commit' do - it { expect(regular_commit.cherry_pick_message(project, 'master')).to include("\n\n(cherry picked from commit 88790590ed1337ab189bccaa355f068481c90bec)") } + let(:commit) { project.commit('video') } + + it { expect(commit.cherry_pick_message(user)).to include("\n\n(cherry picked from commit 88790590ed1337ab189bccaa355f068481c90bec)") } end context 'of a merge commit' do + let(:repository) { project.repository } + + let(:commit_options) do + author = repository.user_to_committer(user) + commit_options = { message: 'Test message', committer: author, author: author } + end + + let(:merge_commit) do + merge_request = create(:merge_request, + source_branch: 'feature', + target_branch: 'master', + source_project: project, + author: user) + + merge_commit_id = repository.merge(user, + merge_request.diff_head_sha, + merge_request, + commit_options) + + merge_commit = repository.commit(merge_commit_id) + + # Manually mark as completed. + # + merge_request.update(merge_commit_sha: merge_commit_id) + + merge_commit + end + it do expected_appended_text = <<~STR.rstrip + (cherry picked from commit #{merge_commit.sha}) - (cherry picked from commit b9238ee5bf1d7359dd3b8c89fd76c1c7f8b75aba) - - 6d664995 This commit will be fixupped against - 64117577 fixup! This commit will be fixupped against + 0b4bc9a4 Feature added STR - expect(merge_commit.cherry_pick_message(project, 'master')).to include(expected_appended_text) + expect(merge_commit.cherry_pick_message(user)).to include(expected_appended_text) end end end -- cgit v1.2.1 From b4622c772ed1fb6974b0e5f39890978d48ad77da Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Fri, 18 Aug 2017 08:30:14 +0200 Subject: Remove redundant statement part in a test suite --- spec/models/commit_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 98054b91c29..8fc2a30e3ad 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -209,7 +209,7 @@ eos let(:commit_options) do author = repository.user_to_committer(user) - commit_options = { message: 'Test message', committer: author, author: author } + { message: 'Test message', committer: author, author: author } end let(:merge_commit) do -- cgit v1.2.1 From 9ec358af7bfe95c90d1c3c3fdd1db8a3a69c11ed Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Mon, 21 Aug 2017 20:47:18 +0200 Subject: Reverse order of commits in MR cherry-pick message Also improved the UT for better documenting this change. --- spec/models/commit_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 8fc2a30e3ad..25de444da11 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -214,7 +214,7 @@ eos let(:merge_commit) do merge_request = create(:merge_request, - source_branch: 'feature', + source_branch: 'video', target_branch: 'master', source_project: project, author: user) @@ -238,7 +238,8 @@ eos (cherry picked from commit #{merge_commit.sha}) - 0b4bc9a4 Feature added + 467dc98f Add new 'videos' directory + 88790590 Upload new video file STR expect(merge_commit.cherry_pick_message(user)).to include(expected_appended_text) -- cgit v1.2.1 From e6220c018eec5a9fdf2491651b00863c5b8a54c0 Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Mon, 21 Aug 2017 21:05:11 +0200 Subject: Add UT for cherry-pick of an existing merge that is not found --- spec/models/commit_spec.rb | 49 ++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 19 deletions(-) (limited to 'spec/models') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 25de444da11..d35eeae00f3 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -212,39 +212,50 @@ eos { message: 'Test message', committer: author, author: author } end - let(:merge_commit) do - merge_request = create(:merge_request, - source_branch: 'video', - target_branch: 'master', - source_project: project, - author: user) + let(:merge_request) do + create(:merge_request, + source_branch: 'video', + target_branch: 'master', + source_project: project, + author: user) + end + let(:merge_commit) do merge_commit_id = repository.merge(user, merge_request.diff_head_sha, merge_request, commit_options) - merge_commit = repository.commit(merge_commit_id) + repository.commit(merge_commit_id) + end - # Manually mark as completed. - # - merge_request.update(merge_commit_sha: merge_commit_id) + context 'that is found' do + before do + # Artificially mark as completed. + merge_request.update(merge_commit_sha: merge_commit.id) + end - merge_commit - end + it do + expected_appended_text = <<~STR.rstrip - it do - expected_appended_text = <<~STR.rstrip + (cherry picked from commit #{merge_commit.sha}) - (cherry picked from commit #{merge_commit.sha}) + 467dc98f Add new 'videos' directory + 88790590 Upload new video file + STR - 467dc98f Add new 'videos' directory - 88790590 Upload new video file - STR + expect(merge_commit.cherry_pick_message(user)).to include(expected_appended_text) + end + end - expect(merge_commit.cherry_pick_message(user)).to include(expected_appended_text) + context "that is existing but not found (eg. it's in progress)" do + it do + expect(merge_commit.cherry_pick_message(user)).to end_with("(cherry picked from commit #{merge_commit.sha})") + end end + end + end describe '#reverts_commit?' do -- cgit v1.2.1 From 7a8a8e07648091f778cafe9fcbc290316ae82e9d Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Mon, 21 Aug 2017 22:13:41 +0200 Subject: Fix rubocop errors in spec/models/commit_spec.rb --- spec/models/commit_spec.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index d35eeae00f3..6fbd58f1d84 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -253,9 +253,7 @@ eos expect(merge_commit.cherry_pick_message(user)).to end_with("(cherry picked from commit #{merge_commit.sha})") end end - end - end describe '#reverts_commit?' do -- cgit v1.2.1 From 934eef72902d6e753f47f7f8dbc42959c399ad09 Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Tue, 22 Aug 2017 22:20:22 +0200 Subject: Improve description of a Commit#cherry_pick_message UT --- spec/models/commit_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 6fbd58f1d84..11e64a0f877 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -248,8 +248,8 @@ eos end end - context "that is existing but not found (eg. it's in progress)" do - it do + context "that is existing but not found" do + it 'does not include details of the merged commits' do expect(merge_commit.cherry_pick_message(user)).to end_with("(cherry picked from commit #{merge_commit.sha})") end end -- cgit v1.2.1 From 8f178c4222d917b5f2878beb97642bff0ee5345e Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 23 Aug 2017 01:19:35 +0200 Subject: Prevent new / renamed project from using a repository path that already exists on disk There are some redundancies in the validation steps, and that is to preserve current error messages behavior Also few specs have to be changed in order to fix madness in validation logic. --- spec/models/container_repository_spec.rb | 2 +- spec/models/project_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index bae88cb1d24..e46945e301e 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe ContainerRepository do let(:group) { create(:group, name: 'group') } - let(:project) { create(:project, :repository, path: 'test', group: group) } + let(:project) { create(:project, path: 'test', group: group) } let(:repository) do create(:container_repository, name: 'my_image', project: project) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2e613c44357..eb590f7dae5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -181,7 +181,7 @@ describe Project do end end - context 'repository storages inclussion' do + context 'repository storages inclusion' do let(:project2) { build(:project, repository_storage: 'missing') } before do -- cgit v1.2.1 From d413f8e4e426e2cb2dc61d5a72d84a7dc67a28c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Tue, 15 Aug 2017 20:25:47 -0500 Subject: Add validation for visibility level of sub groups Sub groups should not have a visibility level higher than its parent. --- spec/models/group_spec.rb | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'spec/models') diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index c5bfae47606..a3310cf1dce 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -84,6 +84,39 @@ describe Group do expect(group).not_to be_valid end end + + describe '#visibility_level_allowed_by_parent' do + let(:parent) { create(:group, :internal) } + let(:sub_group) { build(:group, parent_id: parent.id) } + + context 'without a parent' do + it 'is valid' do + sub_group.parent_id = nil + + expect(sub_group).to be_valid + end + end + + context 'with a parent' do + context 'when visibility of sub group is greater than the parent' do + it 'is invalid' do + sub_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(sub_group).to be_invalid + end + end + + context 'when visibility of sub group is lower or equal to the parent' do + [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PRIVATE].each do |level| + it 'is valid' do + sub_group.visibility_level = level + + expect(sub_group).to be_valid + end + end + end + end + end end describe '.visible_to_user' do -- cgit v1.2.1 From b2b9d63f9b7301cbef9d1e1b8d4ad3cefeacf35d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 23 Aug 2017 11:51:11 -0500 Subject: Add validation to check visibility level of sub groups. --- spec/models/group_spec.rb | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) (limited to 'spec/models') diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index a3310cf1dce..c3342afb7fd 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -117,6 +117,50 @@ describe Group do end end end + + describe '#visibility_level_allowed_by_projects' do + let!(:internal_group) { create(:group, :internal) } + let!(:internal_project) { create(:project, :internal, group: internal_group) } + + context 'when group has a lower visibility' do + it 'is invalid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE + + expect(internal_group).to be_invalid + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are projects with higher visibility.') + end + end + + context 'when group has a higher visibility' do + it 'is valid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(internal_group).to be_valid + end + end + end + + describe '#visibility_level_allowed_by_sub_groups' do + let!(:internal_group) { create(:group, :internal) } + let!(:internal_sub_group) { create(:group, :internal, parent: internal_group) } + + context 'when parent group has a lower visibility' do + it 'is invalid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE + + expect(internal_group).to be_invalid + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are sub groups with higher visibility.') + end + end + + context 'when parent group has a higher visibility' do + it 'is valid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(internal_group).to be_valid + end + end + end end describe '.visible_to_user' do -- cgit v1.2.1 From d6d713f6bac9cd1840e45ed43911b99adba9c07b Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 11:57:16 -0500 Subject: fix failing tests due to new group visibility restrictions --- spec/models/group_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index c3342afb7fd..f9cd12c0ff3 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -127,7 +127,7 @@ describe Group do internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE expect(internal_group).to be_invalid - expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are projects with higher visibility.') + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since this group contains projects with higher visibility.') end end @@ -149,7 +149,7 @@ describe Group do internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE expect(internal_group).to be_invalid - expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are sub groups with higher visibility.') + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are sub-groups with higher visibility.') end end -- cgit v1.2.1 From c158a22fd3cc5c3d60b5e0e1bf44f8aa75617d70 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 25 Aug 2017 15:26:22 +0200 Subject: Only update the sidebar count caches when needed This ensures the issues/MR cache of the sidebar is only updated when the state or confidential flags changes, instead of changing this for every update. --- spec/models/issue_spec.rb | 18 ++++++++++++++++++ spec/models/merge_request_spec.rb | 12 ++++++++++++ 2 files changed, 30 insertions(+) (limited to 'spec/models') diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index de86788d142..e547da0cfbe 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -769,4 +769,22 @@ describe Issue do expect(described_class.public_only).to eq([public_issue]) end end + + describe '#update_project_counter_caches?' do + it 'returns true when the state changes' do + subject.state = 'closed' + + expect(subject.update_project_counter_caches?).to eq(true) + end + + it 'returns true when the confidential flag changes' do + subject.confidential = true + + expect(subject.update_project_counter_caches?).to eq(true) + end + + it 'returns false when the state or confidential flag did not change' do + expect(subject.update_project_counter_caches?).to eq(false) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 92cf15a5a51..09f3b97ec58 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1700,4 +1700,16 @@ describe MergeRequest do .to change { project.open_merge_requests_count }.from(1).to(0) end end + + describe '#update_project_counter_caches?' do + it 'returns true when the state changes' do + subject.state = 'closed' + + expect(subject.update_project_counter_caches?).to eq(true) + end + + it 'returns false when the state did not change' do + expect(subject.update_project_counter_caches?).to eq(false) + end + end end -- cgit v1.2.1 From b0f982fbdf69c292ab4530c0aaaf1ab42f4e7a01 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 21 Aug 2017 11:30:03 +0100 Subject: Add settings for minimum key strength and allowed key type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is an amalgamation of: * Cory Hinshaw: Initial implementation !5552 * Rémy Coutable: Updates !9350 * Nick Thomas: Resolve conflicts and add ED25519 support !13712 --- spec/models/application_setting_spec.rb | 33 +++++++++++++ spec/models/key_spec.rb | 85 ++++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 359753b600e..44d473db07d 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -72,6 +72,27 @@ describe ApplicationSetting do .is_greater_than(0) end + it { is_expected.to validate_presence_of(:minimum_rsa_bits) } + it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('rsa')).for(:minimum_rsa_bits) } + it { is_expected.not_to allow_value(128).for(:minimum_rsa_bits) } + + it { is_expected.to validate_presence_of(:minimum_dsa_bits) } + it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('dsa')).for(:minimum_dsa_bits) } + it { is_expected.not_to allow_value(128).for(:minimum_dsa_bits) } + + it { is_expected.to validate_presence_of(:minimum_ecdsa_bits) } + it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('ecdsa')).for(:minimum_ecdsa_bits) } + it { is_expected.not_to allow_value(128).for(:minimum_ecdsa_bits) } + + it { is_expected.to validate_presence_of(:minimum_ed25519_bits) } + it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('ed25519')).for(:minimum_ed25519_bits) } + it { is_expected.not_to allow_value(128).for(:minimum_ed25519_bits) } + + describe 'allowed_key_types validations' do + it { is_expected.to allow_value(Gitlab::SSHPublicKey.technology_names).for(:allowed_key_types) } + it { is_expected.not_to allow_value(['foo']).for(:allowed_key_types) } + end + it_behaves_like 'an object with email-formated attributes', :admin_notification_email do subject { setting } end @@ -441,4 +462,16 @@ describe ApplicationSetting do end end end + + context 'allowed key types attribute' do + it 'set value with array of symbols' do + setting.allowed_key_types = [:rsa] + expect(setting.allowed_key_types).to contain_exactly(:rsa) + end + + it 'get value as array of symbols' do + setting.allowed_key_types = ['rsa'] + expect(setting.allowed_key_types).to eq(['rsa']) + end + end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 3508391c721..83b11baa371 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -1,6 +1,13 @@ require 'spec_helper' describe Key, :mailer do + include Gitlab::CurrentSettings + + describe 'modules' do + subject { described_class } + it { is_expected.to include_module(Gitlab::CurrentSettings) } + end + describe "Associations" do it { is_expected.to belong_to(:user) } end @@ -11,8 +18,10 @@ describe Key, :mailer do it { is_expected.to validate_presence_of(:key) } it { is_expected.to validate_length_of(:key).is_at_most(5000) } - it { is_expected.to allow_value('ssh-foo').for(:key) } - it { is_expected.to allow_value('ecdsa-foo').for(:key) } + it { is_expected.to allow_value(attributes_for(:rsa_key_2048)[:key]).for(:key) } + it { is_expected.to allow_value(attributes_for(:dsa_key_2048)[:key]).for(:key) } + it { is_expected.to allow_value(attributes_for(:ecdsa_key_256)[:key]).for(:key) } + it { is_expected.to allow_value(attributes_for(:ed25519_key_256)[:key]).for(:key) } it { is_expected.not_to allow_value('foo-bar').for(:key) } end @@ -95,6 +104,78 @@ describe Key, :mailer do end end + context 'validate it meets minimum bit length' do + where(:factory, :minimum, :result) do + [ + [:rsa_key_2048, 1024, true], + [:rsa_key_2048, 2048, true], + [:rsa_key_2048, 4096, false], + [:dsa_key_2048, 1024, true], + [:dsa_key_2048, 2048, true], + [:dsa_key_2048, 4096, false], + [:ecdsa_key_256, 256, true], + [:ecdsa_key_256, 384, false], + [:ed25519_key_256, 256, true], + [:ed25519_key_256, 384, false] + ] + end + + with_them do + subject(:key) { build(factory) } + + before do + stub_application_setting("minimum_#{key.public_key.type}_bits" => minimum) + end + + it { expect(key.valid?).to eq(result) } + end + end + + context 'validate the key type is allowed' do + it 'accepts RSA, DSA, ECDSA and ED25519 keys by default' do + expect(build(:rsa_key_2048)).to be_valid + expect(build(:dsa_key_2048)).to be_valid + expect(build(:ecdsa_key_256)).to be_valid + expect(build(:ed25519_key_256)).to be_valid + end + + it 'rejects RSA, ECDSA and ED25519 keys if DSA is the only allowed type' do + stub_application_setting(allowed_key_types: ['dsa']) + + expect(build(:rsa_key_2048)).not_to be_valid + expect(build(:dsa_key_2048)).to be_valid + expect(build(:ecdsa_key_256)).not_to be_valid + expect(build(:ed25519_key_256)).not_to be_valid + end + + it 'rejects RSA, DSA and ED25519 keys if ECDSA is the only allowed type' do + stub_application_setting(allowed_key_types: ['ecdsa']) + + expect(build(:rsa_key_2048)).not_to be_valid + expect(build(:dsa_key_2048)).not_to be_valid + expect(build(:ecdsa_key_256)).to be_valid + expect(build(:ed25519_key_256)).not_to be_valid + end + + it 'rejects DSA, ECDSA and ED25519 keys if RSA is the only allowed type' do + stub_application_setting(allowed_key_types: ['rsa']) + + expect(build(:rsa_key_2048)).to be_valid + expect(build(:dsa_key_2048)).not_to be_valid + expect(build(:ecdsa_key_256)).not_to be_valid + expect(build(:ed25519_key_256)).not_to be_valid + end + + it 'rejects RSA, DSA and ECDSA keys if ED25519 is the only allowed type' do + stub_application_setting(allowed_key_types: ['ed25519']) + + expect(build(:rsa_key_2048)).not_to be_valid + expect(build(:dsa_key_2048)).not_to be_valid + expect(build(:ecdsa_key_256)).not_to be_valid + expect(build(:ed25519_key_256)).to be_valid + end + end + context 'callbacks' do it 'adds new key to authorized_file' do key = build(:personal_key, id: 7) -- cgit v1.2.1 From 6847060266792471c9c14518a5106e0f622cd6c5 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 25 Aug 2017 14:08:48 +0100 Subject: Rework the permissions model for SSH key restrictions `allowed_key_types` is removed and the `minimum__bits` fields are renamed to `_key_restriction`. A special sentinel value (`-1`) signifies that the key type is disabled. This also feeds through to the UI - checkboxes per key type are out, inline selection of "forbidden" and "allowed" (i.e., no restrictions) are in. As with the previous model, unknown key types are disallowed, even if the underlying ssh daemon happens to support them. The defaults have also been changed from the lowest known bit size to "no restriction". So if someone does happen to have a 768-bit RSA key, it will continue to work on upgrade, at least until the administrator restricts them. --- spec/models/application_setting_spec.rb | 63 +++++++++++++++++++------------ spec/models/key_spec.rb | 66 +++++++++------------------------ 2 files changed, 58 insertions(+), 71 deletions(-) (limited to 'spec/models') diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 44d473db07d..0435aa9dfe1 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -72,25 +72,22 @@ describe ApplicationSetting do .is_greater_than(0) end - it { is_expected.to validate_presence_of(:minimum_rsa_bits) } - it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('rsa')).for(:minimum_rsa_bits) } - it { is_expected.not_to allow_value(128).for(:minimum_rsa_bits) } - - it { is_expected.to validate_presence_of(:minimum_dsa_bits) } - it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('dsa')).for(:minimum_dsa_bits) } - it { is_expected.not_to allow_value(128).for(:minimum_dsa_bits) } + context 'key restrictions' do + it 'supports all key types' do + expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519) + end - it { is_expected.to validate_presence_of(:minimum_ecdsa_bits) } - it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('ecdsa')).for(:minimum_ecdsa_bits) } - it { is_expected.not_to allow_value(128).for(:minimum_ecdsa_bits) } + where(:type) do + described_class::SUPPORTED_KEY_TYPES + end - it { is_expected.to validate_presence_of(:minimum_ed25519_bits) } - it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('ed25519')).for(:minimum_ed25519_bits) } - it { is_expected.not_to allow_value(128).for(:minimum_ed25519_bits) } + with_them do + let(:field) { :"#{type}_key_restriction" } - describe 'allowed_key_types validations' do - it { is_expected.to allow_value(Gitlab::SSHPublicKey.technology_names).for(:allowed_key_types) } - it { is_expected.not_to allow_value(['foo']).for(:allowed_key_types) } + it { is_expected.to validate_presence_of(field) } + it { is_expected.to allow_value(*described_class.supported_key_restrictions(type)).for(field) } + it { is_expected.not_to allow_value(128).for(field) } + end end it_behaves_like 'an object with email-formated attributes', :admin_notification_email do @@ -463,15 +460,35 @@ describe ApplicationSetting do end end - context 'allowed key types attribute' do - it 'set value with array of symbols' do - setting.allowed_key_types = [:rsa] - expect(setting.allowed_key_types).to contain_exactly(:rsa) + describe '#allowed_key_types' do + it 'includes all key types by default' do + expect(setting.allowed_key_types).to contain_exactly(*described_class::SUPPORTED_KEY_TYPES) + end + + it 'excludes disabled key types' do + expect(setting.allowed_key_types).to include(:ed25519) + + setting.ed25519_key_restriction = described_class::FORBIDDEN_KEY_VALUE + + expect(setting.allowed_key_types).not_to include(:ed25519) + end + end + + describe '#key_restriction_for' do + it 'returns the restriction value for recognised types' do + setting.rsa_key_restriction = 1024 + + expect(setting.key_restriction_for(:rsa)).to eq(1024) + end + + it 'allows types to be passed as a string' do + setting.rsa_key_restriction = 1024 + + expect(setting.key_restriction_for('rsa')).to eq(1024) end - it 'get value as array of symbols' do - setting.allowed_key_types = ['rsa'] - expect(setting.allowed_key_types).to eq(['rsa']) + it 'returns forbidden for unrecognised type' do + expect(setting.key_restriction_for(:foo)).to eq(described_class::FORBIDDEN_KEY_VALUE) end end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 83b11baa371..96baeaff0a4 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -104,19 +104,34 @@ describe Key, :mailer do end end - context 'validate it meets minimum bit length' do + context 'validate it meets key restrictions' do where(:factory, :minimum, :result) do + forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE + [ + [:rsa_key_2048, 0, true], + [:dsa_key_2048, 0, true], + [:ecdsa_key_256, 0, true], + [:ed25519_key_256, 0, true], + [:rsa_key_2048, 1024, true], [:rsa_key_2048, 2048, true], [:rsa_key_2048, 4096, false], + [:dsa_key_2048, 1024, true], [:dsa_key_2048, 2048, true], [:dsa_key_2048, 4096, false], + [:ecdsa_key_256, 256, true], [:ecdsa_key_256, 384, false], + [:ed25519_key_256, 256, true], - [:ed25519_key_256, 384, false] + [:ed25519_key_256, 384, false], + + [:rsa_key_2048, forbidden, false], + [:dsa_key_2048, forbidden, false], + [:ecdsa_key_256, forbidden, false], + [:ed25519_key_256, forbidden, false] ] end @@ -124,58 +139,13 @@ describe Key, :mailer do subject(:key) { build(factory) } before do - stub_application_setting("minimum_#{key.public_key.type}_bits" => minimum) + stub_application_setting("#{key.public_key.type}_key_restriction" => minimum) end it { expect(key.valid?).to eq(result) } end end - context 'validate the key type is allowed' do - it 'accepts RSA, DSA, ECDSA and ED25519 keys by default' do - expect(build(:rsa_key_2048)).to be_valid - expect(build(:dsa_key_2048)).to be_valid - expect(build(:ecdsa_key_256)).to be_valid - expect(build(:ed25519_key_256)).to be_valid - end - - it 'rejects RSA, ECDSA and ED25519 keys if DSA is the only allowed type' do - stub_application_setting(allowed_key_types: ['dsa']) - - expect(build(:rsa_key_2048)).not_to be_valid - expect(build(:dsa_key_2048)).to be_valid - expect(build(:ecdsa_key_256)).not_to be_valid - expect(build(:ed25519_key_256)).not_to be_valid - end - - it 'rejects RSA, DSA and ED25519 keys if ECDSA is the only allowed type' do - stub_application_setting(allowed_key_types: ['ecdsa']) - - expect(build(:rsa_key_2048)).not_to be_valid - expect(build(:dsa_key_2048)).not_to be_valid - expect(build(:ecdsa_key_256)).to be_valid - expect(build(:ed25519_key_256)).not_to be_valid - end - - it 'rejects DSA, ECDSA and ED25519 keys if RSA is the only allowed type' do - stub_application_setting(allowed_key_types: ['rsa']) - - expect(build(:rsa_key_2048)).to be_valid - expect(build(:dsa_key_2048)).not_to be_valid - expect(build(:ecdsa_key_256)).not_to be_valid - expect(build(:ed25519_key_256)).not_to be_valid - end - - it 'rejects RSA, DSA and ECDSA keys if ED25519 is the only allowed type' do - stub_application_setting(allowed_key_types: ['ed25519']) - - expect(build(:rsa_key_2048)).not_to be_valid - expect(build(:dsa_key_2048)).not_to be_valid - expect(build(:ecdsa_key_256)).not_to be_valid - expect(build(:ed25519_key_256)).to be_valid - end - end - context 'callbacks' do it 'adds new key to authorized_file' do key = build(:personal_key, id: 7) -- cgit v1.2.1 From eb05bdc6f589f6f0713df12582eb9f18fc4022b3 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 28 Aug 2017 21:58:36 +0100 Subject: Move the key restriction validation to its own class --- spec/models/application_setting_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 0435aa9dfe1..a3b6baca0a2 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -85,7 +85,7 @@ describe ApplicationSetting do let(:field) { :"#{type}_key_restriction" } it { is_expected.to validate_presence_of(field) } - it { is_expected.to allow_value(*described_class.supported_key_restrictions(type)).for(field) } + it { is_expected.to allow_value(*KeyRestrictionValidator.supported_key_restrictions(type)).for(field) } it { is_expected.not_to allow_value(128).for(field) } end end -- cgit v1.2.1 From 378ee1dac262a490e48334a3dd3300be5f1c7299 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Wed, 30 Aug 2017 17:16:08 +0200 Subject: Unescape HTML characters in Wiki title The special characters of a wiki title are now escaped correctly. --- spec/models/wiki_page_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec/models') diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 40a222be24d..9ef8d117123 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -281,6 +281,12 @@ describe WikiPage do @page.title = "Import-existing-repositories-into-GitLab" expect(@page.title).to eq("Import existing repositories into GitLab") end + + it 'unescapes html' do + @page.title = 'foo & bar' + + expect(@page.title).to eq('foo & bar') + end end describe '#directory' do -- cgit v1.2.1 From 29b40db58944a32db6cf1ae9906653a2e5f4be9d Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 30 Aug 2017 21:20:00 +0100 Subject: More review comments --- spec/models/application_setting_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'spec/models') diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index a3b6baca0a2..f921545668d 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -77,6 +77,15 @@ describe ApplicationSetting do expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519) end + it 'does not allow all key types to be disabled' do + described_class::SUPPORTED_KEY_TYPES.each do |type| + setting["#{type}_key_restriction"] = described_class::FORBIDDEN_KEY_VALUE + end + + expect(setting).not_to be_valid + expect(setting.errors.messages).to have_key(:allowed_key_types) + end + where(:type) do described_class::SUPPORTED_KEY_TYPES end -- cgit v1.2.1 From 04cd47dd5a08ca5cc84c44346b2893111da9594c Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Tue, 29 Aug 2017 12:15:19 +0200 Subject: Don't show references to Pages when not available In this instance its subgroups, and given we can't deploy it, we shouldn't allow it to be shown. Fixes gitlab-org/gitlab-ce#34864 --- spec/models/project_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2e613c44357..867f629264c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2225,6 +2225,28 @@ describe Project do end end + describe '#pages_available?' do + let(:project) { create(:project, group: group) } + + subject { project.pages_available? } + + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + end + + context 'when the project is in a top level namespace' do + let(:group) { create(:group) } + + it { is_expected.to be(true) } + end + + context 'when the project is in a subgroup' do + let(:group) { create(:group, :nested) } + + it { is_expected.to be(false) } + end + end + describe '#remove_private_deploy_keys' do let!(:project) { create(:project) } -- cgit v1.2.1 From 3e4013f21760be5522ac080669b6da9fb9118999 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Tue, 29 Aug 2017 15:36:35 -0300 Subject: Remove closing external issues by reference error --- spec/models/merge_request_spec.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'spec/models') diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 92cf15a5a51..05cf9308127 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -159,6 +159,7 @@ describe MergeRequest do before do subject.project.has_external_issue_tracker = true subject.project.save! + create(:jira_service, project: subject.project) end it 'does not cache issues from external trackers' do @@ -166,6 +167,7 @@ describe MergeRequest do commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") allow(subject).to receive(:commits).and_return([commit]) + expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to raise_error expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count) end -- cgit v1.2.1 From eda34b1a1846a5d5b55cc127a32b0c7628580f25 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 21 Aug 2017 17:24:44 +0900 Subject: Add spec. Fix runner setting page. It worked. --- spec/models/ci/runner_spec.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 48f878bbee6..45a64b6f76f 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -456,4 +456,28 @@ describe Ci::Runner do expect(described_class.search(runner.description.upcase)).to eq([runner]) end end + + describe '.access_level' do + context 'when access_level of a runner is protected' do + before do + create(:ci_runner, :protected) + end + + it 'a protected runner exists' do + expect(Ci::Runner.count).to eq(1) + expect(Ci::Runner.last.protected_?).to eq(true) + end + end + + context 'when access_level of a runner is unprotected' do + before do + create(:ci_runner, :unprotected) + end + + it 'an unprotected runner exists' do + expect(Ci::Runner.count).to eq(1) + expect(Ci::Runner.last.unprotected?).to eq(true) + end + end + end end -- cgit v1.2.1 From bbe967abeba7be1db79e34439e74cd113c240b52 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 22 Aug 2017 17:01:11 +0900 Subject: Add the rest of specs --- spec/models/ci/build_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 0c35ad3c9d8..1673d873b57 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -43,6 +43,16 @@ describe Ci::Build do it { is_expected.not_to include(manual_but_created) } end + describe '.protected_' do + let!(:protected_job) { create(:ci_build, :protected) } + let!(:unprotected_job) { create(:ci_build, :unprotected) } + + subject { described_class.protected_ } + + it { is_expected.to include(protected_job) } + it { is_expected.not_to include(unprotected_job) } + end + describe '#actionize' do context 'when build is a created' do before do -- cgit v1.2.1 From 1925bfd2a5b8fab48de8b1f2a4ae7db6a09736e1 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 22 Aug 2017 22:59:08 +0900 Subject: Fix spec --- spec/models/ci/runner_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 45a64b6f76f..a04d4615b6b 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -464,8 +464,8 @@ describe Ci::Runner do end it 'a protected runner exists' do - expect(Ci::Runner.count).to eq(1) - expect(Ci::Runner.last.protected_?).to eq(true) + expect(described_class.count).to eq(1) + expect(described_class.last.protected_?).to eq(true) end end @@ -475,8 +475,8 @@ describe Ci::Runner do end it 'an unprotected runner exists' do - expect(Ci::Runner.count).to eq(1) - expect(Ci::Runner.last.unprotected?).to eq(true) + expect(described_class.count).to eq(1) + expect(described_class.last.unprotected?).to eq(true) end end end -- cgit v1.2.1 From 4b0e31ba64118011ffc29c31bc771fa2568cd270 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 28 Aug 2017 21:10:01 +0900 Subject: Extend can_pick? --- spec/models/ci/runner_spec.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index a04d4615b6b..566b9b48879 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -95,6 +95,8 @@ describe Ci::Runner do let(:build) { create(:ci_build, pipeline: pipeline) } let(:runner) { create(:ci_runner) } + subject { runner.can_pick?(build) } + before do build.project.runners << runner end @@ -222,6 +224,28 @@ describe Ci::Runner do end end end + + context 'when runner is protected' do + before do + runner.protected_! + end + + context 'when build is protected' do + before do + build.protected = true + end + + it { is_expected.to be_truthy } + end + + context 'when build is unprotected' do + before do + build.protected = false + end + + it { is_expected.to be_falsey } + end + end end describe '#status' do -- cgit v1.2.1 From 1024718e9fddbb0d61d3f64f44303964641fcdd8 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 29 Aug 2017 15:56:03 +0900 Subject: Refactor access_level to not_protected and ref_protected --- spec/models/ci/build_spec.rb | 4 ++-- spec/models/ci/runner_spec.rb | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 1673d873b57..ef6c91d11f1 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -43,11 +43,11 @@ describe Ci::Build do it { is_expected.not_to include(manual_but_created) } end - describe '.protected_' do + describe '.ref_protected' do let!(:protected_job) { create(:ci_build, :protected) } let!(:unprotected_job) { create(:ci_build, :unprotected) } - subject { described_class.protected_ } + subject { described_class.ref_protected } it { is_expected.to include(protected_job) } it { is_expected.not_to include(unprotected_job) } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 566b9b48879..54f68542d41 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -227,7 +227,7 @@ describe Ci::Runner do context 'when runner is protected' do before do - runner.protected_! + runner.ref_protected! end context 'when build is protected' do @@ -489,18 +489,18 @@ describe Ci::Runner do it 'a protected runner exists' do expect(described_class.count).to eq(1) - expect(described_class.last.protected_?).to eq(true) + expect(described_class.last.ref_protected?).to eq(true) end end - context 'when access_level of a runner is unprotected' do + context 'when access_level of a runner is not_protected' do before do - create(:ci_runner, :unprotected) + create(:ci_runner, :not_protected) end - it 'an unprotected runner exists' do + it 'an not_protected runner exists' do expect(described_class.count).to eq(1) - expect(described_class.last.unprotected?).to eq(true) + expect(described_class.last.not_protected?).to eq(true) end end end -- cgit v1.2.1 From 2170eec662e05b24148dc1c959dbe5fe9723b4fb Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 29 Aug 2017 17:31:59 +0900 Subject: Fix spec --- spec/models/ci/runner_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 54f68542d41..8c157d4369b 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -482,9 +482,9 @@ describe Ci::Runner do end describe '.access_level' do - context 'when access_level of a runner is protected' do + context 'when access_level of a runner is ref_protected' do before do - create(:ci_runner, :protected) + create(:ci_runner, :ref_protected) end it 'a protected runner exists' do -- cgit v1.2.1 From 07f7a01b4da02533c417fa636f004fedf3c4c661 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 31 Aug 2017 18:28:29 +0900 Subject: Improve spec. Add validation for accel_level on runner. --- spec/models/ci/build_spec.rb | 22 ++++++++++++++---- spec/models/ci/runner_spec.rb | 54 ++++++++++++++++++++++++------------------- 2 files changed, 47 insertions(+), 29 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index ef6c91d11f1..a94fdbf8434 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -44,13 +44,25 @@ describe Ci::Build do end describe '.ref_protected' do - let!(:protected_job) { create(:ci_build, :protected) } - let!(:unprotected_job) { create(:ci_build, :unprotected) } - subject { described_class.ref_protected } - it { is_expected.to include(protected_job) } - it { is_expected.not_to include(unprotected_job) } + context 'when protected is true' do + let!(:job) { create(:ci_build, :protected) } + + it { is_expected.to include(job) } + end + + context 'when protected is false' do + let!(:job) { create(:ci_build, :unprotected) } + + it { is_expected.not_to include(job) } + end + + context 'when protected is false' do + let!(:job) { create(:ci_build, protected: nil) } + + it { is_expected.not_to include(job) } + end end describe '#actionize' do diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 8c157d4369b..ca48372c5e2 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' describe Ci::Runner do describe 'validation' do + it { is_expected.to validate_presence_of(:access_level) } + context 'when runner is not allowed to pick untagged jobs' do context 'when runner does not have tags' do it 'is not valid' do @@ -19,6 +21,34 @@ describe Ci::Runner do end end + describe '#access_level' do + context 'when creating new runner and access_level is nil' do + let(:runner) do + build(:ci_runner, access_level: nil) + end + + it "object is invalid" do + expect(runner).not_to be_valid + end + end + + context 'when creating new runner and access_level is defined in enum' do + let(:runner) do + build(:ci_runner, access_level: :not_protected) + end + + it "object is valid" do + expect(runner).to be_valid + end + end + + context 'when creating new runner and access_level is not defined in enum' do + it "raises an error" do + expect { build(:ci_runner, access_level: :this_is_not_defined) }.to raise_error(ArgumentError) + end + end + end + describe '#display_name' do it 'returns the description if it has a value' do runner = FactoryGirl.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') @@ -480,28 +510,4 @@ describe Ci::Runner do expect(described_class.search(runner.description.upcase)).to eq([runner]) end end - - describe '.access_level' do - context 'when access_level of a runner is ref_protected' do - before do - create(:ci_runner, :ref_protected) - end - - it 'a protected runner exists' do - expect(described_class.count).to eq(1) - expect(described_class.last.ref_protected?).to eq(true) - end - end - - context 'when access_level of a runner is not_protected' do - before do - create(:ci_runner, :not_protected) - end - - it 'an not_protected runner exists' do - expect(described_class.count).to eq(1) - expect(described_class.last.not_protected?).to eq(true) - end - end - end end -- cgit v1.2.1 From ce7c0ac3dbdc3f2f2f34b39bf5ef3755e79e9d42 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 31 Aug 2017 21:13:40 +0900 Subject: Add validation for protected attributes --- spec/models/ci/build_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index a94fdbf8434..cab59db3580 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -59,7 +59,11 @@ describe Ci::Build do end context 'when protected is false' do - let!(:job) { create(:ci_build, protected: nil) } + let!(:job) { create(:ci_build) } + + before do + job.update_attribute(:protected, nil) + end it { is_expected.not_to include(job) } end -- cgit v1.2.1 From eab938d50519a4da272818c1364ceeca82e32982 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 1 Sep 2017 20:30:42 +0900 Subject: Fix typo --- 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 cab59db3580..1f965cfbede 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -58,7 +58,7 @@ describe Ci::Build do it { is_expected.not_to include(job) } end - context 'when protected is false' do + context 'when protected is nil' do let!(:job) { create(:ci_build) } before do -- cgit v1.2.1 From 1b481342a08caea34e0d605780de13d47d8dd7e2 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 2 Sep 2017 16:31:14 +0900 Subject: Fix spec --- spec/models/ci/build_spec.rb | 2 +- spec/models/ci/runner_spec.rb | 24 +++++++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 1f965cfbede..3fe3ec17d36 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -53,7 +53,7 @@ describe Ci::Build do end context 'when protected is false' do - let!(:job) { create(:ci_build, :unprotected) } + let!(:job) { create(:ci_build) } it { is_expected.not_to include(job) } end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ca48372c5e2..2e686e515c5 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -255,7 +255,29 @@ describe Ci::Runner do end end - context 'when runner is protected' do + context 'when access_level of runner is not_protected' do + before do + runner.not_protected! + end + + context 'when build is protected' do + before do + build.protected = true + end + + it { is_expected.to be_truthy } + end + + context 'when build is unprotected' do + before do + build.protected = false + end + + it { is_expected.to be_truthy } + end + end + + context 'when access_level of runner is ref_protected' do before do runner.ref_protected! end -- cgit v1.2.1