From 918646f8e9daa954d25f122f866bd675c1278c42 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 23 Jun 2016 18:36:44 +0800 Subject: Add Project#new_issue_address(author): Which would return the email address for creating a new issue by sending an email to that particular address. --- spec/models/project_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 30aa2b70c8d..407efc9f966 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -129,6 +129,22 @@ describe Project, models: true do end end + describe "#new_issue_address" do + before do + stub_incoming_email_setting(address: "p+%{key}@gl.ab") + end + + let(:project) { create(:empty_project, path: "somewhere") } + let(:user) { create(:user) } + + it 'returns the address to create a new issue' do + token = user.authentication_token + address = "p+#{project.namespace.path}/#{project.path}+#{token}@gl.ab" + + expect(project.new_issue_address(user)).to eq(address) + end + end + describe 'last_activity methods' do let(:project) { create(:project) } let(:last_event) { double(created_at: Time.now) } -- cgit v1.2.1 From e616fbfd425f838ca83fe01c3adf10e6b8e5a934 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 24 Jun 2016 16:05:05 +0800 Subject: Only return the address if incoming emails is enabled: Feedback from: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3363#note_12669123 --- spec/models/project_spec.rb | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 407efc9f966..5d41bc2d38c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -130,18 +130,30 @@ describe Project, models: true do end describe "#new_issue_address" do - before do - stub_incoming_email_setting(address: "p+%{key}@gl.ab") - end - let(:project) { create(:empty_project, path: "somewhere") } let(:user) { create(:user) } - it 'returns the address to create a new issue' do - token = user.authentication_token - address = "p+#{project.namespace.path}/#{project.path}+#{token}@gl.ab" + context 'incoming email enabled' do + before do + stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab") + end + + it 'returns the address to create a new issue' do + token = user.authentication_token + address = "p+#{project.namespace.path}/#{project.path}+#{token}@gl.ab" - expect(project.new_issue_address(user)).to eq(address) + expect(project.new_issue_address(user)).to eq(address) + end + end + + context 'incoming email disabled' do + before do + stub_incoming_email_setting(enabled: false) + end + + it 'returns nil' do + expect(project.new_issue_address(user)).to be_nil + end end end -- cgit v1.2.1 From 939436480c8c6bd524d3c4c78674e81f90861d06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 25 Jul 2016 17:08:36 +0200 Subject: Ensure relative paths for video are rewritten as we do for images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/models/blob_spec.rb | 16 ++++++++++++++++ spec/models/commit_spec.rb | 1 + 2 files changed, 17 insertions(+) (limited to 'spec/models') diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 78e95c8fac5..1e5d6a34f83 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -33,6 +33,22 @@ describe Blob do end end + describe '#video?' do + it 'is falsey with image extension' do + git_blob = Gitlab::Git::Blob.new(name: 'image.png') + + expect(described_class.decorate(git_blob)).not_to be_video + end + + UploaderHelper::VIDEO_EXT.each do |ext| + it "is truthy when extension is .#{ext}" do + git_blob = Gitlab::Git::Blob.new(name: "video.#{ext}") + + expect(described_class.decorate(git_blob)).to be_video + end + end + end + describe '#to_partial_path' do def stubbed_blob(overrides = {}) overrides.reverse_merge!( diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ba02d5fe977..ec1544bf815 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -212,6 +212,7 @@ eos it 'returns the URI type at the given path' do expect(commit.uri_type('files/html')).to be(:tree) expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) + expect(project.commit('video').uri_type('files/videos/intro.mp4')).to be(:raw) expect(commit.uri_type('files/js/application.js')).to be(:blob) end -- cgit v1.2.1 From 940382793384547ad87baf8b38a16bc501243a88 Mon Sep 17 00:00:00 2001 From: Katarzyna Kobierska Date: Mon, 25 Jul 2016 11:21:20 +0200 Subject: Fix tests --- spec/models/build_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 978ad9c52d5..dc88697199b 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -259,7 +259,7 @@ describe Ci::Build, models: true do let(:trigger) { create(:ci_trigger, project: project) } let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) } let(:user_trigger_variable) do - { key: :TRIGGER_KEY, value: 'TRIGGER_VALUE', public: false } + { key: :TRIGGER_KEY_1, value: 'TRIGGER_VALUE_1', public: false } end let(:predefined_trigger_variable) do { key: 'CI_BUILD_TRIGGERED', value: 'true', public: true } -- cgit v1.2.1 From 255162e194918d1d9c3cd7c0c6d3927e3f2006bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 26 Jul 2016 11:55:00 +0200 Subject: Little refactor, add specs, and a CHANGELOG entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .../project_services/hipchat_service_spec.rb | 30 +++++++++++++++++----- 1 file changed, 24 insertions(+), 6 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 5f618322aab..62ae5f6cf74 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -340,18 +340,36 @@ describe HipchatService, models: true do end context "#message_options" do - it "should be set to the defaults" do - expect(hipchat.send(:message_options)).to eq({ notify: false, color: 'yellow' }) + it "is set to the defaults" do + expect(hipchat.__send__(:message_options)).to eq({ notify: false, color: 'yellow' }) end - it "should set notfiy to true" do + it "sets notify to true" do allow(hipchat).to receive(:notify).and_return('1') - expect(hipchat.send(:message_options)).to eq({ notify: true, color: 'yellow' }) + + expect(hipchat.__send__(:message_options)).to eq({ notify: true, color: 'yellow' }) end - it "should set the color" do + it "sets the color" do allow(hipchat).to receive(:color).and_return('red') - expect(hipchat.send(:message_options)).to eq({ notify: false, color: 'red' }) + + expect(hipchat.__send__(:message_options)).to eq({ notify: false, color: 'red' }) + end + + context 'with a successful build' do + it 'uses the green color' do + build_data = { object_kind: 'build', commit: { status: 'success' } } + + expect(hipchat.__send__(:message_options, build_data)).to eq({ notify: false, color: 'green' }) + end + end + + context 'with a failed build' do + it 'uses the red color' do + build_data = { object_kind: 'build', commit: { status: 'failed' } } + + expect(hipchat.__send__(:message_options, build_data)).to eq({ notify: false, color: 'red' }) + end end end end -- cgit v1.2.1 From 1dcfb1d3a7b6b8002b1f25e8cf463617acbc1299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 26 Jul 2016 17:22:13 -0400 Subject: Fix a bug where forking a project from a repository storage to another would fail --- spec/models/project_spec.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9b017288488..c884bb31199 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1244,6 +1244,32 @@ describe Project, models: true do end end + describe '#add_import_job' do + context 'forked' do + let(:forked_project_link) { create(:forked_project_link) } + let(:forked_from_project) { forked_project_link.forked_from_project } + let(:project) { forked_project_link.forked_to_project } + + it 'schedules a RepositoryForkWorker job' do + expect(RepositoryForkWorker).to receive(:perform_async). + with(project.id, forked_from_project.repository_storage_path, + forked_from_project.path_with_namespace, project.namespace.path) + + project.add_import_job + end + end + + context 'not forked' do + let(:project) { create(:project) } + + it 'schedules a RepositoryImportWorker job' do + expect(RepositoryImportWorker).to receive(:perform_async).with(project.id) + + project.add_import_job + end + end + end + describe '.where_paths_in' do context 'without any paths' do it 'returns an empty relation' do -- cgit v1.2.1 From d1ea2bca61dff21948024d897e1d4475123a10e8 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 19 Jul 2016 21:52:31 -0700 Subject: Optimize maximum user access level lookup in loading of notes NotesHelper#note_editable? and ProjectTeam#human_max_access currently take about 16% of the load time of an issue page. This MR preloads the maximum access level of users for all notes in issues and merge requests with several queries instead of one per user and caches the result in RequestStore. --- spec/models/ability_spec.rb | 56 ++++++++++++++++++++++++++++++++++++++++ spec/models/project_team_spec.rb | 51 ++++++++++++++++++++++++++++++------ 2 files changed, 99 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 1acb5846fcf..cd5f40fe3d2 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -1,6 +1,62 @@ require 'spec_helper' describe Ability, lib: true do + describe '.can_edit_note?' do + let(:project) { create(:empty_project) } + let!(:note) { create(:note_on_issue, project: project) } + + context 'using an anonymous user' do + it 'returns false' do + expect(described_class.can_edit_note?(nil, note)).to be_falsy + end + end + + context 'using a system note' do + it 'returns false' do + system_note = create(:note, system: true) + user = create(:user) + + expect(described_class.can_edit_note?(user, system_note)).to be_falsy + end + end + + context 'using users with different access levels' do + let(:user) { create(:user) } + + it 'returns true for the author' do + expect(described_class.can_edit_note?(note.author, note)).to be_truthy + end + + it 'returns false for a guest user' do + project.team << [user, :guest] + + expect(described_class.can_edit_note?(user, note)).to be_falsy + end + + it 'returns false for a developer' do + project.team << [user, :developer] + + expect(described_class.can_edit_note?(user, note)).to be_falsy + end + + it 'returns true for a master' do + project.team << [user, :master] + + expect(described_class.can_edit_note?(user, note)).to be_truthy + end + + it 'returns true for a group owner' do + group = create(:group) + project.project_group_links.create( + group: group, + group_access: Gitlab::Access::MASTER) + group.add_owner(user) + + expect(described_class.can_edit_note?(user, note)).to be_truthy + end + end + end + describe '.users_that_can_read_project' do context 'using a public project' do it 'returns all the users' do diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 9262aeb6ed8..115fffd82d9 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -151,8 +151,8 @@ describe ProjectTeam, models: true do it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::MASTER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } it { expect(project.team.max_member_access(guest.id)).to eq(Gitlab::Access::GUEST) } - it { expect(project.team.max_member_access(nonmember.id)).to be_nil } - it { expect(project.team.max_member_access(requester.id)).to be_nil } + it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } end context 'when project is shared with group' do @@ -168,14 +168,14 @@ describe ProjectTeam, models: true do it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::DEVELOPER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } - it { expect(project.team.max_member_access(nonmember.id)).to be_nil } - it { expect(project.team.max_member_access(requester.id)).to be_nil } + it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } context 'but share_with_group_lock is true' do before { project.namespace.update(share_with_group_lock: true) } - it { expect(project.team.max_member_access(master.id)).to be_nil } - it { expect(project.team.max_member_access(reporter.id)).to be_nil } + it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::NO_ACCESS) } end end end @@ -194,8 +194,43 @@ describe ProjectTeam, models: true do it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::MASTER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } it { expect(project.team.max_member_access(guest.id)).to eq(Gitlab::Access::GUEST) } - it { expect(project.team.max_member_access(nonmember.id)).to be_nil } - it { expect(project.team.max_member_access(requester.id)).to be_nil } + it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } + end + end + + describe "#max_member_access_for_users" do + it 'returns correct roles for different users' do + master = create(:user) + reporter = create(:user) + promoted_guest = create(:user) + guest = create(:user) + project = create(:project) + + project.team << [master, :master] + project.team << [reporter, :reporter] + project.team << [promoted_guest, :guest] + project.team << [guest, :guest] + + group = create(:group) + group_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) + users = [master, reporter, promoted_guest, guest, group_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 + } + + expect(project.team.max_member_access_for_user_ids(users)).to eq(expected) end end end -- cgit v1.2.1 From 1fba07109f44dfb69664e528d3aca409a2dbaff1 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 26 Jul 2016 16:56:06 -0700 Subject: Optimize the invited group link access level check --- spec/models/project_team_spec.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 115fffd82d9..f9b3e65745e 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -214,20 +214,30 @@ describe ProjectTeam, models: true do 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) - users = [master, reporter, promoted_guest, guest, group_developer].map(&:id) + 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 + 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) -- cgit v1.2.1 From ba192c73cdaeec7cec9079a7b5dec4787b15ebdf Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 26 Jul 2016 17:05:52 -0700 Subject: Rubocop offenses --- spec/models/project_team_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index f9b3e65745e..1f42fbd3385 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -225,19 +225,19 @@ describe ProjectTeam, models: true do second_group = create(:group) project.project_group_links.create( - group: second_group, - group_access: Gitlab::Access::MASTER) + 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 + 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) -- cgit v1.2.1 From 277e9e4ee84d6b72152ce55854b579c992f9db9c Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 26 Jul 2016 17:20:19 -0700 Subject: Add a spec for access_for_user_ids --- spec/models/member_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'spec/models') diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 40181a8b906..44cd3c08718 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -79,6 +79,18 @@ describe Member, models: true do @accepted_request_member = project.requesters.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request } end + describe '.access_for_user_ids' do + it 'returns the right access levels' do + users = [@owner_user.id, @master_user.id] + expected = { + @owner_user.id => Gitlab::Access::OWNER, + @master_user.id => Gitlab::Access::MASTER + } + + expect(described_class.access_for_user_ids(users)).to eq(expected) + end + end + describe '.invite' do it { expect(described_class.invite).not_to include @master } it { expect(described_class.invite).to include @invited_member } -- cgit v1.2.1 From 2c0f8eb1cc844561c294a3d4fe9388c6c7ed2acf Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Wed, 27 Jul 2016 10:51:37 +0100 Subject: Respective cache is now expired when creating a new branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Project and branch cache is expired when project is still empty or new branch is created. develops tests accordingly Signed-off-by: Rémy Coutable --- spec/models/repository_spec.rb | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) (limited to 'spec/models') diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 881ab5ff8dc..5bc1bd9a930 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -446,6 +446,43 @@ describe Repository, models: true do end.to raise_error(GitHooksService::PreReceiveError) end end + + context 'when target branch is different from source branch' do + before do + allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, '']) + end + + it 'expires branch cache' do + expect(repository).not_to receive(:expire_exists_cache) + expect(repository).not_to receive(:expire_root_ref_cache) + expect(repository).not_to receive(:expire_emptiness_caches) + expect(repository).to receive(:expire_branches_cache) + expect(repository).to receive(:expire_has_visible_content_cache) + expect(repository).to receive(:expire_branch_count_cache) + + repository.commit_with_hooks(user, 'new-feature') { sample_commit.id } + end + end + + context 'when repository is empty' do + before do + allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, '']) + end + + it 'expires creation and branch cache' do + empty_repository = create(:empty_project, :empty_repo).repository + + expect(empty_repository).to receive(:expire_exists_cache) + expect(empty_repository).to receive(:expire_root_ref_cache) + expect(empty_repository).to receive(:expire_emptiness_caches) + expect(empty_repository).to receive(:expire_branches_cache) + expect(empty_repository).to receive(:expire_has_visible_content_cache) + expect(empty_repository).to receive(:expire_branch_count_cache) + + empty_repository.commit_file(user, 'CHANGELOG', 'Changelog!', + 'Updates file content', 'master', false) + end + end end describe '#exists?' do -- cgit v1.2.1 From ea4d6c87d73301ade3e49f8f865eb448ecc8a569 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 26 Jul 2016 15:27:42 -0300 Subject: Fix Project#to_param to keep invalid project suitable for use in URLs --- spec/models/project_spec.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c884bb31199..be5c53c44be 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -372,6 +372,24 @@ describe Project, models: true do it { expect(@project.to_param).to eq('gitlabhq') } end + + context 'with invalid path' do + it 'returns previous path to keep project suitable for use in URLs when persisted' do + project = create(:empty_project, path: 'gitlab') + project.path = 'foo&bar' + + expect(project).not_to be_valid + expect(project.to_param).to eq 'gitlab' + end + + it 'returns current path when new record' do + project = build(:empty_project, path: 'gitlab') + project.path = 'foo&bar' + + expect(project).not_to be_valid + expect(project.to_param).to eq 'foo&bar' + end + end end describe '#repository' do -- cgit v1.2.1 From d27e36f35af0c2850c5370a3348d30ce4bcf1a68 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 27 Jul 2016 16:42:38 -0700 Subject: Add specs for caching commit author --- spec/models/commit_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'spec/models') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ec1544bf815..c3392ee7440 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -13,6 +13,26 @@ describe Commit, models: true do it { is_expected.to include_module(StaticModel) } end + describe '#author' do + it 'looks up the author in a case-insensitive way' do + user = create(:user, email: commit.author_email.upcase) + expect(commit.author).to eq(user) + end + + it 'caches the author' do + user = create(:user, email: commit.author_email) + expect(RequestStore).to receive(:active?).twice.and_return(true) + expect_any_instance_of(Commit).to receive(:find_author_by_any_email).and_call_original + + expect(commit.author).to eq(user) + key = "commit_author:#{commit.author_email}" + expect(RequestStore.store[key]).to eq(user) + + expect(commit.author).to eq(user) + RequestStore.store.clear + end + end + describe '#to_reference' do it 'returns a String reference to the object' do expect(commit.to_reference).to eq commit.id -- cgit v1.2.1 From d00679d54f2bbaab9d6963c3b871a03ab8a3d7d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Wed, 27 Jul 2016 11:26:29 -0400 Subject: Update to gitlab_git 10.4.1 and take advantage of preserved Ref objects --- spec/models/repository_spec.rb | 45 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 22 deletions(-) (limited to 'spec/models') diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 5bc1bd9a930..cce15538b93 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -50,8 +50,9 @@ describe Repository, models: true do double_first = double(committed_date: Time.now) double_last = double(committed_date: Time.now - 1.second) - allow(repository).to receive(:commit).with(tag_a.target).and_return(double_first) - allow(repository).to receive(:commit).with(tag_b.target).and_return(double_last) + allow(tag_a).to receive(:target).and_return(double_first) + allow(tag_b).to receive(:target).and_return(double_last) + allow(repository).to receive(:tags).and_return([tag_a, tag_b]) end it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } @@ -64,8 +65,9 @@ describe Repository, models: true do double_first = double(committed_date: Time.now - 1.second) double_last = double(committed_date: Time.now) - allow(repository).to receive(:commit).with(tag_a.target).and_return(double_last) - allow(repository).to receive(:commit).with(tag_b.target).and_return(double_first) + allow(tag_a).to receive(:target).and_return(double_last) + allow(tag_b).to receive(:target).and_return(double_first) + allow(repository).to receive(:tags).and_return([tag_a, tag_b]) end it { is_expected.to eq(['v1.1.0', 'v1.0.0']) } @@ -381,9 +383,13 @@ describe Repository, models: true do end describe '#rm_branch' do + let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature + let(:blank_sha) { '0000000000000000000000000000000000000000' } + context 'when pre hooks were successful' do it 'should run without errors' do - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) + expect_any_instance_of(GitHooksService).to receive(:execute). + with(user, project.repository.path_to_repo, old_rev, blank_sha, 'refs/heads/feature') expect { repository.rm_branch(user, 'feature') }.not_to raise_error end @@ -418,10 +424,13 @@ describe Repository, models: true do end describe '#commit_with_hooks' do + let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature + context 'when pre hooks were successful' do before do expect_any_instance_of(GitHooksService).to receive(:execute). - and_return(true) + with(user, repository.path_to_repo, old_rev, sample_commit.id, 'refs/heads/feature'). + and_yield.and_return(true) end it 'should run without errors' do @@ -435,6 +444,14 @@ describe Repository, models: true do repository.commit_with_hooks(user, 'feature') { sample_commit.id } end + + context "when the branch wasn't empty" do + it 'updates the head' do + expect(repository.find_branch('feature').target.id).to eq(old_rev) + repository.commit_with_hooks(user, 'feature') { sample_commit.id } + expect(repository.find_branch('feature').target.id).to eq(sample_commit.id) + end + end end context 'when pre hooks failed' do @@ -1198,17 +1215,6 @@ describe Repository, models: true do end end - describe '#local_branches' do - it 'returns the local branches' do - masterrev = repository.find_branch('master').target - create_remote_branch('joe', 'remote_branch', masterrev) - repository.add_branch(user, 'local_branch', masterrev) - - expect(repository.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false) - expect(repository.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true) - end - end - describe "#keep_around" do it "does not fail if we attempt to reference bad commit" do expect(repository.kept_around?('abc1234')).to be_falsey @@ -1236,9 +1242,4 @@ describe Repository, models: true do File.delete(path) end end - - def create_remote_branch(remote_name, branch_name, target) - rugged = repository.rugged - rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target) - end end -- cgit v1.2.1 From c647540c1010fd1e51bced1db90947aa00c83fa8 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 8 Jul 2016 14:46:13 +0530 Subject: Fix all specs related to changes in !5081. 1. Remove `Project#developers_can_push_to_protected_branch?` since it isn't used anymore. 2. Remove `Project#developers_can_merge_to_protected_branch?` since it isn't used anymore. --- spec/models/project_spec.rb | 40 ---------------------------------------- 1 file changed, 40 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 72b8a4e25bd..e365e4e98b2 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1095,46 +1095,6 @@ describe Project, models: true do end end - describe "#developers_can_push_to_protected_branch?" do - let(:project) { create(:empty_project) } - - context "when the branch matches a protected branch via direct match" do - it "returns true if 'Developers can Push' is turned on" do - create(:protected_branch, name: "production", project: project, developers_can_push: true) - - expect(project.developers_can_push_to_protected_branch?('production')).to be true - end - - it "returns false if 'Developers can Push' is turned off" do - create(:protected_branch, name: "production", project: project, developers_can_push: false) - - expect(project.developers_can_push_to_protected_branch?('production')).to be false - end - end - - context "when the branch matches a protected branch via wilcard match" do - it "returns true if 'Developers can Push' is turned on" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: true) - - expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be true - end - - it "returns false if 'Developers can Push' is turned off" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: false) - - expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be false - end - end - - context "when the branch does not match a protected branch" do - it "returns false" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: true) - - expect(project.developers_can_push_to_protected_branch?('staging/some-branch')).to be false - end - end - end - describe '#container_registry_path_with_namespace' do let(:project) { create(:empty_project, path: 'PROJECT') } -- cgit v1.2.1 From 002ad215818450d2cbbc5fa065850a953dc7ada8 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 20 Jul 2016 20:13:02 +0200 Subject: Method for returning issues readable by a user The method Ability.issues_readable_by_user takes a list of users and an optional user and returns an Array of issues readable by said user. This method in turn is used by Banzai::ReferenceParser::IssueParser#nodes_visible_to_user so this method no longer needs to get all the available abilities just to check if a user has the "read_issue" ability. To test this I benchmarked an issue with 222 comments on my development environment. Using these changes the time spent in nodes_visible_to_user was reduced from around 120 ms to around 40 ms. --- spec/models/ability_spec.rb | 48 ++++++ spec/models/concerns/mentionable_spec.rb | 6 +- spec/models/issue_spec.rb | 253 +++++++++++++++++++++++++++++++ 3 files changed, 306 insertions(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index cd5f40fe3d2..853f6943cef 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -170,4 +170,52 @@ describe Ability, lib: true do end end end + + describe '.issues_readable_by_user' do + context 'with an admin user' do + it 'returns all given issues' do + user = build(:user, admin: true) + issue = build(:issue) + + expect(described_class.issues_readable_by_user([issue], user)). + to eq([issue]) + end + end + + context 'with a regular user' do + it 'returns the issues readable by the user' do + user = build(:user) + issue = build(:issue) + + expect(issue).to receive(:readable_by?).with(user).and_return(true) + + expect(described_class.issues_readable_by_user([issue], user)). + to eq([issue]) + end + + it 'returns an empty Array when no issues are readable' do + user = build(:user) + issue = build(:issue) + + expect(issue).to receive(:readable_by?).with(user).and_return(false) + + expect(described_class.issues_readable_by_user([issue], user)).to eq([]) + end + end + + context 'without a regular user' do + it 'returns issues that are publicly visible' do + hidden_issue = build(:issue) + visible_issue = build(:issue) + + expect(hidden_issue).to receive(:publicly_visible?).and_return(false) + expect(visible_issue).to receive(:publicly_visible?).and_return(true) + + issues = described_class. + issues_readable_by_user([hidden_issue, visible_issue]) + + expect(issues).to eq([visible_issue]) + end + end + end end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 5e652660e2c..549b0042038 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -68,7 +68,7 @@ describe Issue, "Mentionable" do describe '#create_cross_references!' do let(:project) { create(:project) } - let(:author) { double('author') } + let(:author) { build(:user) } let(:commit) { project.commit } let(:commit2) { project.commit } @@ -88,6 +88,10 @@ describe Issue, "Mentionable" do let(:author) { create(:author) } let(:issues) { create_list(:issue, 2, project: project, author: author) } + before do + project.team << [author, Gitlab::Access::DEVELOPER] + end + context 'before changes are persisted' do it 'ignores pre-existing references' do issue = create_issue(description: issues[0].to_reference) diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 6a897c96690..3259f795296 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -306,4 +306,257 @@ describe Issue, models: true do expect(user2.assigned_open_issues_count).to eq(1) end end + + describe '#visible_to_user?' do + context 'with a user' do + let(:user) { build(:user) } + let(:issue) { build(:issue) } + + it 'returns true when the issue is readable' do + expect(issue).to receive(:readable_by?).with(user).and_return(true) + + expect(issue.visible_to_user?(user)).to eq(true) + end + + it 'returns false when the issue is not readable' do + expect(issue).to receive(:readable_by?).with(user).and_return(false) + + expect(issue.visible_to_user?(user)).to eq(false) + end + end + + context 'without a user' do + let(:issue) { build(:issue) } + + it 'returns true when the issue is publicly visible' do + expect(issue).to receive(:publicly_visible?).and_return(true) + + expect(issue.visible_to_user?).to eq(true) + end + + it 'returns false when the issue is not publicly visible' do + expect(issue).to receive(:publicly_visible?).and_return(false) + + expect(issue.visible_to_user?).to eq(false) + end + end + end + + describe '#readable_by?' do + describe 'with a regular user that is not a team member' do + let(:user) { create(:user) } + + context 'using a public project' do + let(:project) { create(:empty_project, :public) } + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, project: project, confidential: true) + + expect(issue).not_to be_readable_by(user) + end + end + + context 'using an internal project' do + let(:project) { create(:empty_project, :internal) } + + context 'using an internal user' do + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + end + + context 'using an external user' do + before do + allow(user).to receive(:external?).and_return(true) + end + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + end + end + + context 'using a private project' do + let(:project) { create(:empty_project, :private) } + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + + context 'when the user is the project owner' do + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + end + end + end + + context 'with a regular user that is a team member' do + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public) } + + context 'using a public project' do + before do + project.team << [user, Gitlab::Access::DEVELOPER] + end + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + + context 'using an internal project' do + let(:project) { create(:empty_project, :internal) } + + before do + project.team << [user, Gitlab::Access::DEVELOPER] + end + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + + context 'using a private project' do + let(:project) { create(:empty_project, :private) } + + before do + project.team << [user, Gitlab::Access::DEVELOPER] + end + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + end + + context 'with an admin user' do + let(:project) { create(:empty_project) } + let(:user) { create(:user, admin: true) } + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + end + + describe '#publicly_visible?' do + context 'using a public project' do + let(:project) { create(:empty_project, :public) } + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_publicly_visible + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_publicly_visible + end + end + + context 'using an internal project' do + let(:project) { create(:empty_project, :internal) } + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_publicly_visible + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_publicly_visible + end + end + + context 'using a private project' do + let(:project) { create(:empty_project, :private) } + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_publicly_visible + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_publicly_visible + end + end + end end -- cgit v1.2.1 From be9aa7f19474424991923f128053e2523fa166d8 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Tue, 26 Jul 2016 09:35:47 +0200 Subject: Add an URL field to Environments This MR adds a string (thus max 255 chars) field to the enviroments table to expose it later in other features. --- spec/models/environment_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'spec/models') diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 7629af6a570..6c11cfc4c9b 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -11,4 +11,14 @@ describe Environment, models: true do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) } it { is_expected.to validate_length_of(:name).is_within(0..255) } + + it { is_expected.to validate_length_of(:external_url).is_within(0..255) } + + # To circumvent a not null violation of the name column: + # https://github.com/thoughtbot/shoulda-matchers/issues/336 + it 'validates uniqueness of :external_url' do + create(:environment) + + is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id) + end end -- cgit v1.2.1 From 76e9b68439510af5c783a81b93944f1c8d96d150 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Tue, 26 Jul 2016 14:19:37 +0200 Subject: Incorporate feedback --- spec/models/environment_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'spec/models') diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 6c11cfc4c9b..ef2148be1bd 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -21,4 +21,12 @@ describe Environment, models: true do is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id) end + + describe '#nullify_external_url' do + it 'replaces a blank url with nil' do + env = build(:environment, external_url: "") + + expect(env.save).to be true + end + end end -- cgit v1.2.1 From 1b72256fa14e65256d78347f81b289d43c44e991 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 29 Jul 2016 12:14:36 +0200 Subject: Use Grape DSL for environment endpoints Also a couple of minor edits for this branch are included --- spec/models/environment_spec.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'spec/models') diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index ef2148be1bd..8a84ac0a7c7 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -27,6 +27,7 @@ describe Environment, models: true do env = build(:environment, external_url: "") expect(env.save).to be true + expect(env.external_url).to be_nil end end end -- cgit v1.2.1 From 5b4ceeed6317cc8039642981ba356565e11d991e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Fri, 29 Jul 2016 18:24:11 -0300 Subject: Fix attr reader to force the intended values for source and target shas When importing a pull request from GitHub, the old and new branches may no longer actually exist by those names, but we need to recreate the merge request diff with the right source and target shas. We use these `target_branch_sha` and `source_branch_sha` attributes to force these to the intended values. But the reader methods were always looking up to the target/source branch head instead of check if these values was previously set. --- spec/models/merge_request_spec.rb | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'spec/models') diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c8ad7ab3e7f..a0e3c26e542 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -65,11 +65,11 @@ describe MergeRequest, models: true do end describe '#target_branch_sha' do - context 'when the target branch does not exist anymore' do - let(:project) { create(:project) } + let(:project) { create(:project) } - subject { create(:merge_request, source_project: project, target_project: project) } + subject { create(:merge_request, source_project: project, target_project: project) } + context 'when the target branch does not exist' do before do project.repository.raw_repository.delete_branch(subject.target_branch) end @@ -78,6 +78,12 @@ describe MergeRequest, models: true do expect(subject.target_branch_sha).to be_nil end end + + it 'returns memoized value' do + subject.target_branch_sha = '8ffb3c15a5475e59ae909384297fede4badcb4c7' + + expect(subject.target_branch_sha).to eq '8ffb3c15a5475e59ae909384297fede4badcb4c7' + end end describe '#source_branch_sha' do @@ -103,6 +109,12 @@ describe MergeRequest, models: true do expect(subject.source_branch_sha).to be_nil end end + + it 'returns memoized value' do + subject.source_branch_sha = '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' + + expect(subject.source_branch_sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' + end end describe '#to_reference' do -- cgit v1.2.1 From 99c02ed53c994fbd71442410c78daf220c6d1ced Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 1 Aug 2016 13:11:45 -0700 Subject: Only use RequestStore in ProjectTeam#max_member_access_for_user if it is active --- spec/models/project_team_spec.rb | 101 +++++++++++++++++++++++---------------- 1 file changed, 61 insertions(+), 40 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 1f42fbd3385..5eaf0d3b7a6 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -199,48 +199,69 @@ describe ProjectTeam, models: true do end end - describe "#max_member_access_for_users" do - it 'returns correct roles for different users' do - master = create(:user) - reporter = create(:user) - promoted_guest = create(:user) - guest = create(:user) - project = create(:project) + shared_examples_for "#max_member_access_for_users" do |enable_request_store| + describe "#max_member_access_for_users" do + before do + RequestStore.begin! if enable_request_store + end - project.team << [master, :master] - project.team << [reporter, :reporter] - project.team << [promoted_guest, :guest] - project.team << [guest, :guest] + after do + if enable_request_store + RequestStore.end! + RequestStore.clear! + end + end - 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) + it 'returns correct roles for different users' do + master = create(:user) + reporter = create(:user) + promoted_guest = create(:user) + guest = create(:user) + project = create(:project) + + project.team << [master, :master] + project.team << [reporter, :reporter] + project.team << [promoted_guest, :guest] + project.team << [guest, :guest] + + 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) + end end end + + describe '#max_member_access_for_users with RequestStore' do + it_behaves_like "#max_member_access_for_users", true + end + + describe '#max_member_access_for_users without RequestStore' do + it_behaves_like "#max_member_access_for_users", false + end end -- cgit v1.2.1 From edc5f4018e45327421e112de18d53bfbdabd38f9 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Mon, 1 Aug 2016 10:06:57 +0100 Subject: developer cannot push to protected branch when project is empty or he has not been granted permission to do so --- spec/models/project_spec.rb | 119 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 108 insertions(+), 11 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e365e4e98b2..1e6f735699b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -69,6 +69,7 @@ describe Project, models: true do it { is_expected.to include_module(Gitlab::ConfigHelper) } it { is_expected.to include_module(Gitlab::ShellAdapter) } it { is_expected.to include_module(Gitlab::VisibilityLevel) } + it { is_expected.to include_module(Gitlab::CurrentSettings) } it { is_expected.to include_module(Referable) } it { is_expected.to include_module(Sortable) } end @@ -1070,28 +1071,124 @@ describe Project, models: true do end describe '#protected_branch?' do + context 'existing project' do + let(:project) { create(:project) } + + it 'returns true when the branch matches a protected branch via direct match' do + project.protected_branches.create!(name: 'foo') + + expect(project.protected_branch?('foo')).to eq(true) + end + + it 'returns true when the branch matches a protected branch via wildcard match' do + project.protected_branches.create!(name: 'production/*') + + expect(project.protected_branch?('production/some-branch')).to eq(true) + end + + it 'returns false when the branch does not match a protected branch via direct match' do + expect(project.protected_branch?('foo')).to eq(false) + end + + it 'returns false when the branch does not match a protected branch via wildcard match' do + project.protected_branches.create!(name: 'production/*') + + expect(project.protected_branch?('staging/some-branch')).to eq(false) + end + end + + context "new project" do + let(:project) { create(:empty_project) } + + it 'returns false when default_protected_branch is unprotected' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + + expect(project.protected_branch?('master')).to be false + end + + it 'returns false when default_protected_branch lets developers push' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + + expect(project.protected_branch?('master')).to be false + end + + it 'returns true when default_branch_protection does not let developers push but let developer merge branches' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + + expect(project.protected_branch?('master')).to be true + end + + it 'returns true when default_branch_protection is in full protection' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) + + expect(project.protected_branch?('master')).to be true + end + end + end + + describe "#developers_can_push_to_protected_branch?" do let(:project) { create(:empty_project) } - it 'returns true when the branch matches a protected branch via direct match' do - project.protected_branches.create!(name: 'foo') + context "when the branch matches a protected branch via direct match" do + it "returns true if 'Developers can Push' is turned on" do + create(:protected_branch, name: "production", project: project, developers_can_push: true) + + expect(project.developers_can_push_to_protected_branch?('production')).to be true + end - expect(project.protected_branch?('foo')).to eq(true) + it "returns false if 'Developers can Push' is turned off" do + create(:protected_branch, name: "production", project: project, developers_can_push: false) + + expect(project.developers_can_push_to_protected_branch?('production')).to be false + end end - it 'returns true when the branch matches a protected branch via wildcard match' do - project.protected_branches.create!(name: 'production/*') + context "when project is new" do + it "returns true if project is unprotected" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + + expect(project.developers_can_push_to_protected_branch?('master')).to be true + end + + it "returns true if project allows developers to push to protected branch" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - expect(project.protected_branch?('production/some-branch')).to eq(true) + expect(project.developers_can_push_to_protected_branch?('master')).to be true + end + + it "returns false if project does not let developer push to protected branch but let them merge branches" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + + expect(project.developers_can_push_to_protected_branch?('master')).to be false + end + + it "returns false if project is on full protection mode" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) + + expect(project.developers_can_push_to_protected_branch?('master')).to be false + end end - it 'returns false when the branch does not match a protected branch via direct match' do - expect(project.protected_branch?('foo')).to eq(false) + context "when the branch matches a protected branch via wilcard match" do + it "returns true if 'Developers can Push' is turned on" do + create(:protected_branch, name: "production/*", project: project, developers_can_push: true) + + expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be true + end + + it "returns false if 'Developers can Push' is turned off" do + create(:protected_branch, name: "production/*", project: project, developers_can_push: false) + + expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be false + end end - it 'returns false when the branch does not match a protected branch via wildcard match' do - project.protected_branches.create!(name: 'production/*') + context "when the branch does not match a protected branch" do + it "returns false" do + create(:protected_branch, name: "production/*", project: project, developers_can_push: true) - expect(project.protected_branch?('staging/some-branch')).to eq(false) + expect(project.developers_can_push_to_protected_branch?('staging/some-branch')).to be false + end end end -- cgit v1.2.1 From 4768afbdbf85abbb5e2281c8855e7d27c07a581e Mon Sep 17 00:00:00 2001 From: Keith Pope Date: Tue, 2 Aug 2016 06:56:23 +0100 Subject: Add simple identifier to public SSH keys --- spec/models/key_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 49cf3d8633a..a4d46ca84de 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -16,12 +16,13 @@ describe Key, models: true do end describe "Methods" do + let(:user) { create(:user) } it { is_expected.to respond_to :projects } it { is_expected.to respond_to :publishable_key } describe "#publishable_keys" do - it 'strips all personal information' do - expect(build(:key).publishable_key).not_to match(/dummy@gitlab/) + it 'replaces SSH key comment with simple identifier of username + hostname' do + expect(build(:key, user: user).publishable_key).to match(/#{Regexp.escape(user.name)} \(localhost\)/) end end end -- cgit v1.2.1 From b371d751287fd8a01126c7aa5f156f868d177ef2 Mon Sep 17 00:00:00 2001 From: Keith Pope Date: Tue, 2 Aug 2016 12:49:59 +0100 Subject: Tidy the key spec and fix failing user spec --- spec/models/key_spec.rb | 2 +- spec/models/user_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index a4d46ca84de..6d68e52a822 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -22,7 +22,7 @@ describe Key, models: true do describe "#publishable_keys" do it 'replaces SSH key comment with simple identifier of username + hostname' do - expect(build(:key, user: user).publishable_key).to match(/#{Regexp.escape(user.name)} \(localhost\)/) + expect(build(:key, user: user).publishable_key).to include("#{user.name} (localhost)") end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2a5a7fb2fc6..9f432501c59 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -643,7 +643,7 @@ describe User, models: true do user = create :user key = create :key, key: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD33bWLBxu48Sev9Fert1yzEO4WGcWglWF7K/AwblIUFselOt/QdOL9DSjpQGxLagO1s9wl53STIO8qGS4Ms0EJZyIXOEFMjFJ5xmjSy+S37By4sG7SsltQEHMxtbtFOaW5LV2wCrX+rUsRNqLMamZjgjcPO0/EgGCXIGMAYW4O7cwGZdXWYIhQ1Vwy+CsVMDdPkPgBXqK7nR/ey8KMs8ho5fMNgB5hBw/AL9fNGhRw3QTD6Q12Nkhl4VZES2EsZqlpNnJttnPdp847DUsT6yuLRlfiQfz5Cn9ysHFdXObMN5VYIiPFwHeYCZp1X2S4fDZooRE8uOLTfxWHPXwrhqSH", user_id: user.id - expect(user.all_ssh_keys).to include(key.key) + expect(user.all_ssh_keys).to include(a_string_starting_with(key.key)) end end -- cgit v1.2.1 From 8716ff7f63fff0b056e110bef930c32a98e86c63 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Mon, 1 Aug 2016 16:55:51 +0200 Subject: Speedup DiffNote#active? on discussions, preloading noteables and avoid touching git repository to return diff_refs when possible - Preloading noteable we share the same noteable instance when more than one discussion refers to the same noteable. - Any other call to that object that is cached in that object will be for any discussion. - In those cases where merge_request_diff has all the sha stored to build a diff_refs get that diff_refs using directly those sha instead accessing to the git repository to first get the commits and later the sha. --- spec/models/diff_note_spec.rb | 4 ++-- spec/models/merge_request_spec.rb | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index af8e890ca95..1fa96eb1f15 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -119,7 +119,7 @@ describe DiffNote, models: true do context "when the merge request's diff refs don't match that of the diff note" do before do - allow(subject.noteable).to receive(:diff_refs).and_return(commit.diff_refs) + allow(subject.noteable).to receive(:diff_sha_refs).and_return(commit.diff_refs) end it "returns false" do @@ -168,7 +168,7 @@ describe DiffNote, models: true do context "when the note is outdated" do before do - allow(merge_request).to receive(:diff_refs).and_return(commit.diff_refs) + allow(merge_request).to receive(:diff_sha_refs).and_return(commit.diff_refs) end it "uses the DiffPositionUpdateService" do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a0e3c26e542..21d22c776e9 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -686,4 +686,28 @@ describe MergeRequest, models: true do subject.reload_diff end end + + describe "#diff_sha_refs" do + context "with diffs" do + subject { create(:merge_request, :with_diffs) } + + it "does not touch the repository" do + subject # Instantiate the object + + expect_any_instance_of(Repository).not_to receive(:commit) + + subject.diff_sha_refs + end + + it "returns expected diff_refs" do + expected_diff_refs = Gitlab::Diff::DiffRefs.new( + base_sha: subject.merge_request_diff.base_commit_sha, + start_sha: subject.merge_request_diff.start_commit_sha, + head_sha: subject.merge_request_diff.head_commit_sha + ) + + expect(subject.diff_sha_refs).to eq(expected_diff_refs) + end + end + end end -- cgit v1.2.1 From e4c517a635b6a45a9afc65b37682cc4b4951e922 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Mon, 25 Jul 2016 23:49:06 -0500 Subject: Expand commit message width in repo view --- spec/models/commit_spec.rb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'spec/models') diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index c3392ee7440..d3e6a6648cc 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -86,6 +86,27 @@ eos end end + describe '#full_title' do + it "returns no_commit_message when safe_message is blank" do + allow(commit).to receive(:safe_message).and_return('') + expect(commit.full_title).to eq("--no commit message") + end + + it "returns entire message if there is no newline" do + message = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id felis id blandit. Vivamus egestas lacinia lacus, sed rutrum mauris.' + + allow(commit).to receive(:safe_message).and_return(message) + expect(commit.full_title).to eq(message) + end + + it "returns first line of message if there is a newLine" do + message = commit.safe_message.split(" ").first + + allow(commit).to receive(:safe_message).and_return(message + "\n" + message) + expect(commit.full_title).to eq(message) + end + end + describe "delegation" do subject { commit } -- cgit v1.2.1 From 8f359ea9170b984ad43d126e17628c31ac3a1f14 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Tue, 26 Jul 2016 09:21:42 +0200 Subject: Move to Gitlab::Diff::FileCollection Instead calling diff_collection.count use diff_collection.size which is cache on the diff_collection --- spec/models/merge_request_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec/models') diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 21d22c776e9..fa1f7edae8e 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -660,6 +660,12 @@ describe MergeRequest, models: true do subject.reload_diff end + it "executs diff cache service" do + expect_any_instance_of(MergeRequests::MergeRequestDiffCacheService).to receive(:execute).with(subject) + + subject.reload_diff + end + it "updates diff note positions" do old_diff_refs = subject.diff_refs -- cgit v1.2.1 From c86c1905b5574cac234315598d8d715fcaee3ea7 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Wed, 27 Jul 2016 19:00:34 +0200 Subject: switch from diff_file_collection to diffs So we have raw_diffs too --- spec/models/compare_spec.rb | 77 ++++++++++++++++++++++++++++++++++++ spec/models/legacy_diff_note_spec.rb | 2 +- spec/models/merge_request_spec.rb | 29 +++++++++++++- spec/models/repository_spec.rb | 4 +- 4 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 spec/models/compare_spec.rb (limited to 'spec/models') diff --git a/spec/models/compare_spec.rb b/spec/models/compare_spec.rb new file mode 100644 index 00000000000..49ab3c4b6e9 --- /dev/null +++ b/spec/models/compare_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Compare, models: true do + include RepoHelpers + + let(:project) { create(:project, :public) } + let(:commit) { project.commit } + + let(:start_commit) { sample_image_commit } + let(:head_commit) { sample_commit } + + let(:raw_compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, start_commit.id, head_commit.id) } + + subject { described_class.new(raw_compare, project) } + + describe '#start_commit' do + it 'returns raw compare base commit' do + expect(subject.start_commit.id).to eq(start_commit.id) + end + + it 'returns nil if compare base commit is nil' do + expect(raw_compare).to receive(:base).and_return(nil) + + expect(subject.start_commit).to eq(nil) + end + end + + describe '#commit' do + it 'returns raw compare head commit' do + expect(subject.commit.id).to eq(head_commit.id) + end + + it 'returns nil if compare head commit is nil' do + expect(raw_compare).to receive(:head).and_return(nil) + + expect(subject.commit).to eq(nil) + end + end + + describe '#base_commit' do + let(:base_commit) { Commit.new(another_sample_commit, project) } + + it 'returns project merge base commit' do + expect(project).to receive(:merge_base_commit).with(start_commit.id, head_commit.id).and_return(base_commit) + + expect(subject.base_commit).to eq(base_commit) + end + + it 'returns nil if there is no start_commit' do + expect(subject).to receive(:start_commit).and_return(nil) + + expect(subject.base_commit).to eq(nil) + end + + it 'returns nil if there is no head commit' do + expect(subject).to receive(:head_commit).and_return(nil) + + expect(subject.base_commit).to eq(nil) + end + end + + describe '#diff_refs' do + it 'uses base_commit sha as base_sha' do + expect(subject).to receive(:base_commit).at_least(:once).and_call_original + + expect(subject.diff_refs.base_sha).to eq(subject.base_commit.id) + end + + it 'uses start_commit sha as start_sha' do + expect(subject.diff_refs.start_sha).to eq(start_commit.id) + end + + it 'uses commit sha as head sha' do + expect(subject.diff_refs.head_sha).to eq(head_commit.id) + end + end +end diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb index d23fc06c3ad..c8ee656fe3b 100644 --- a/spec/models/legacy_diff_note_spec.rb +++ b/spec/models/legacy_diff_note_spec.rb @@ -58,7 +58,7 @@ describe LegacyDiffNote, models: true do # Generate a real line_code value so we know it will match. We use a # random line from a random diff just for funsies. - diff = merge.diffs.to_a.sample + diff = merge.raw_diffs.to_a.sample line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index fa1f7edae8e..152e0cce5ad 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -128,7 +128,7 @@ describe MergeRequest, models: true do end end - describe '#diffs' do + describe '#raw_diffs' do let(:merge_request) { build(:merge_request) } let(:options) { { paths: ['a/b', 'b/a', 'c/*'] } } @@ -138,6 +138,31 @@ describe MergeRequest, models: true do expect(merge_request.merge_request_diff).to receive(:diffs).with(options) + merge_request.raw_diffs(options) + end + end + + context 'when there are no MR diffs' do + it 'delegates to the compare object' do + merge_request.compare = double(:compare) + + expect(merge_request.compare).to receive(:raw_diffs).with(options) + + merge_request.raw_diffs(options) + end + end + end + + describe '#diffs' do + let(:merge_request) { build(:merge_request) } + let(:options) { { paths: ['a/b', 'b/a', 'c/*'] } } + + context 'when there are MR diffs' do + it 'delegates to the MR diffs' do + merge_request.merge_request_diff = MergeRequestDiff.new + + expect(merge_request.merge_request_diff).to receive(:diffs).with(hash_including(options)) + merge_request.diffs(options) end end @@ -146,7 +171,7 @@ describe MergeRequest, models: true do it 'delegates to the compare object' do merge_request.compare = double(:compare) - expect(merge_request.compare).to receive(:diffs).with(options) + expect(merge_request.compare).to receive(:diffs).with(diff_options: options) merge_request.diffs(options) end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index cce15538b93..2a053b1804f 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1154,7 +1154,7 @@ describe Repository, models: true do it 'does not flush the cache if the commit does not change any logos' do diff = double(:diff, new_path: 'test.txt') - expect(commit).to receive(:diffs).and_return([diff]) + expect(commit).to receive(:raw_diffs).and_return([diff]) expect(cache).not_to receive(:expire) repository.expire_avatar_cache(repository.root_ref, '123') @@ -1163,7 +1163,7 @@ describe Repository, models: true do it 'flushes the cache if the commit changes any of the logos' do diff = double(:diff, new_path: Repository::AVATAR_FILES[0]) - expect(commit).to receive(:diffs).and_return([diff]) + expect(commit).to receive(:raw_diffs).and_return([diff]) expect(cache).to receive(:expire).with(:avatar) repository.expire_avatar_cache(repository.root_ref, '123') -- cgit v1.2.1 From c008a1a9674f7c01b4504e22ed414b07eff05385 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 3 Aug 2016 09:32:01 -0700 Subject: Make Compare#diffs diff_options a regular argument --- spec/models/merge_request_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 152e0cce5ad..e43008c1a4d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -171,7 +171,7 @@ describe MergeRequest, models: true do it 'delegates to the compare object' do merge_request.compare = double(:compare) - expect(merge_request.compare).to receive(:diffs).with(diff_options: options) + expect(merge_request.compare).to receive(:diffs).with(options) merge_request.diffs(options) end -- cgit v1.2.1 From 631f59d4e7f449c59735cd7eab25cf407e06d5d8 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Wed, 3 Aug 2016 23:32:12 +0200 Subject: change the API on the merge_request_diff model from diffs -> raw_diffs --- spec/models/merge_request_diff_spec.rb | 12 ++++++------ spec/models/merge_request_spec.rb | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 9a637c94fbe..29f7396f862 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -10,7 +10,7 @@ describe MergeRequestDiff, models: true do expect(mr_diff).not_to receive(:load_diffs) expect(Gitlab::Git::Compare).to receive(:new).and_call_original - mr_diff.diffs(ignore_whitespace_change: true) + mr_diff.raw_diffs(ignore_whitespace_change: true) end end @@ -18,19 +18,19 @@ describe MergeRequestDiff, models: true do before { mr_diff.update_attributes(st_diffs: '') } it 'returns an empty DiffCollection' do - expect(mr_diff.diffs).to be_a(Gitlab::Git::DiffCollection) - expect(mr_diff.diffs).to be_empty + expect(mr_diff.raw_diffs).to be_a(Gitlab::Git::DiffCollection) + expect(mr_diff.raw_diffs).to be_empty end end context 'when the raw diffs exist' do it 'returns the diffs' do - expect(mr_diff.diffs).to be_a(Gitlab::Git::DiffCollection) - expect(mr_diff.diffs).not_to be_empty + expect(mr_diff.raw_diffs).to be_a(Gitlab::Git::DiffCollection) + expect(mr_diff.raw_diffs).not_to be_empty end context 'when the :paths option is set' do - let(:diffs) { mr_diff.diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) } + let(:diffs) { mr_diff.raw_diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) } it 'only returns diffs that match the (old path, new path) given' do expect(diffs.map(&:new_path)).to contain_exactly('files/ruby/popen.rb') diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index e43008c1a4d..d793cfd0bde 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -136,7 +136,7 @@ describe MergeRequest, models: true do it 'delegates to the MR diffs' do merge_request.merge_request_diff = MergeRequestDiff.new - expect(merge_request.merge_request_diff).to receive(:diffs).with(options) + expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(options) merge_request.raw_diffs(options) end @@ -161,7 +161,7 @@ describe MergeRequest, models: true do it 'delegates to the MR diffs' do merge_request.merge_request_diff = MergeRequestDiff.new - expect(merge_request.merge_request_diff).to receive(:diffs).with(hash_including(options)) + expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options)) merge_request.diffs(options) end -- cgit v1.2.1 From f15ed5f0a5c298a2f0eb5aaa6d848364133532a5 Mon Sep 17 00:00:00 2001 From: Herminio Torres Date: Thu, 4 Aug 2016 01:35:17 -0300 Subject: Fix Rename `add_users_into_project` and `projects_ids` We never add things `into` projects, we just add them `to` projects. So how about we rename this to `add_users_to_project`. Rename `projects_ids` to `project_ids` by following the convention of rails. --- spec/models/members/project_member_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index ba622dfb9be..e7c0c506463 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -101,7 +101,7 @@ describe ProjectMember, models: true do end end - describe '.add_users_into_projects' do + describe '.add_users_to_projects' do before do @project_1 = create :project @project_2 = create :project @@ -109,7 +109,7 @@ describe ProjectMember, models: true do @user_1 = create :user @user_2 = create :user - ProjectMember.add_users_into_projects( + ProjectMember.add_users_to_projects( [@project_1.id, @project_2.id], [@user_1.id, @user_2.id], ProjectMember::MASTER -- cgit v1.2.1 From 482d7802cc71280595cad71882bf1b438461e435 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Mon, 1 Aug 2016 16:48:15 +0100 Subject: changes default_branch_protection to allow devs_can_merge protection option aswell --- spec/models/project_spec.rb | 71 ++++++++++++++------------------------------- 1 file changed, 22 insertions(+), 49 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1e6f735699b..567f87b9970 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1126,69 +1126,42 @@ describe Project, models: true do end end - describe "#developers_can_push_to_protected_branch?" do + describe '#user_can_push_to_empty_repo?' do let(:project) { create(:empty_project) } + let(:user) { create(:user) } - context "when the branch matches a protected branch via direct match" do - it "returns true if 'Developers can Push' is turned on" do - create(:protected_branch, name: "production", project: project, developers_can_push: true) + it 'returns false when default_branch_protection is in full protection and user is developer' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) - expect(project.developers_can_push_to_protected_branch?('production')).to be true - end - - it "returns false if 'Developers can Push' is turned off" do - create(:protected_branch, name: "production", project: project, developers_can_push: false) - - expect(project.developers_can_push_to_protected_branch?('production')).to be false - end + expect(project.user_can_push_to_empty_repo?(user)).to be_falsey end - context "when project is new" do - it "returns true if project is unprotected" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) - - expect(project.developers_can_push_to_protected_branch?('master')).to be true - end - - it "returns true if project allows developers to push to protected branch" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - - expect(project.developers_can_push_to_protected_branch?('master')).to be true - end - - it "returns false if project does not let developer push to protected branch but let them merge branches" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - expect(project.developers_can_push_to_protected_branch?('master')).to be false - end - - it "returns false if project is on full protection mode" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) + it 'returns false when default_branch_protection only lets devs merge and user is dev' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - expect(project.developers_can_push_to_protected_branch?('master')).to be false - end + expect(project.user_can_push_to_empty_repo?(user)).to be_falsey end - context "when the branch matches a protected branch via wilcard match" do - it "returns true if 'Developers can Push' is turned on" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: true) + it 'returns true when default_branch_protection lets devs push and user is developer' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be true - end + expect(project.user_can_push_to_empty_repo?(user)).to be_truthy + end - it "returns false if 'Developers can Push' is turned off" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: false) + it 'returns true when default_branch_protection is unprotected and user is developer' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) - expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be false - end + expect(project.user_can_push_to_empty_repo?(user)).to be_truthy end - context "when the branch does not match a protected branch" do - it "returns false" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: true) + it 'returns true when user is master' do + project.team << [user, :master] - expect(project.developers_can_push_to_protected_branch?('staging/some-branch')).to be false - end + expect(project.user_can_push_to_empty_repo?(user)).to be_truthy end end -- cgit v1.2.1 From 77c8520e2ecd70520757aed0fbdf434643b60234 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 8 Aug 2016 16:18:13 +0200 Subject: Added concern for a faster "cache_key" method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This concern provides an optimized/simplified version of the "cache_key" method. This method is about 9 times faster than the default "cache_key" method. The produced cache keys _are_ different from the previous ones but this is worth the performance improvement. To showcase this I set up a benchmark (using benchmark-ips) that compares FasterCacheKeys#cache_key with the regular cache_key. The output of this benchmark was: Calculating ------------------------------------- cache_key 4.825k i/100ms cache_key_fast 21.723k i/100ms ------------------------------------------------- cache_key 59.422k (± 7.2%) i/s - 299.150k cache_key_fast 543.243k (± 9.2%) i/s - 2.694M Comparison: cache_key_fast: 543243.4 i/s cache_key: 59422.0 i/s - 9.14x slower To see the impact on real code I applied these changes and benchmarked Issue#referenced_merge_requests. For an issue referencing 10 merge requests these changes shaved off between 40 and 60 milliseconds. --- spec/models/concerns/faster_cache_keys_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 spec/models/concerns/faster_cache_keys_spec.rb (limited to 'spec/models') diff --git a/spec/models/concerns/faster_cache_keys_spec.rb b/spec/models/concerns/faster_cache_keys_spec.rb new file mode 100644 index 00000000000..8d3f94267fa --- /dev/null +++ b/spec/models/concerns/faster_cache_keys_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe FasterCacheKeys do + describe '#cache_key' do + it 'returns a String' do + # We're using a fixed string here so it's easier to set an expectation for + # the resulting cache key. + time = '2016-08-08 16:39:00+02' + issue = build(:issue, updated_at: time) + issue.extend(described_class) + + expect(issue).to receive(:id).and_return(1) + + expect(issue.cache_key).to eq("issues/1-#{time}") + end + end +end -- cgit v1.2.1 From 1d268a89deef10854193db48d65cf5d519a0363d Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Mon, 1 Aug 2016 16:00:44 +0100 Subject: adds second batch of tests changed to active tense --- spec/models/application_setting_spec.rb | 22 ++++++------ spec/models/broadcast_message_spec.rb | 6 ++-- spec/models/build_spec.rb | 30 ++++++++-------- spec/models/ci/pipeline_spec.rb | 8 ++--- spec/models/ci/trigger_spec.rb | 4 +-- spec/models/commit_status_spec.rb | 12 +++---- spec/models/concerns/milestoneish_spec.rb | 36 +++++++++---------- spec/models/concerns/token_authenticatable_spec.rb | 2 +- spec/models/forked_project_link_spec.rb | 10 +++--- spec/models/global_milestone_spec.rb | 12 +++---- spec/models/group_spec.rb | 6 ++-- spec/models/hooks/project_hook_spec.rb | 4 +-- spec/models/key_spec.rb | 4 +-- spec/models/label_spec.rb | 4 +-- spec/models/legacy_diff_note_spec.rb | 4 +-- spec/models/members/group_member_spec.rb | 4 +-- spec/models/members/project_member_spec.rb | 2 +- spec/models/merge_request_spec.rb | 6 ++-- spec/models/milestone_spec.rb | 20 +++++------ spec/models/namespace_spec.rb | 6 ++-- spec/models/note_spec.rb | 6 ++-- spec/models/project_security_spec.rb | 18 +++++----- spec/models/project_services/asana_service_spec.rb | 8 ++--- .../project_services/assembla_service_spec.rb | 2 +- .../project_services/external_wiki_service_spec.rb | 2 +- .../project_services/flowdock_service_spec.rb | 2 +- .../project_services/gemnasium_service_spec.rb | 2 +- .../gitlab_issue_tracker_service_spec.rb | 4 +-- .../project_services/hipchat_service_spec.rb | 38 ++++++++++---------- spec/models/project_services/irker_service_spec.rb | 2 +- spec/models/project_services/jira_service_spec.rb | 10 +++--- .../project_services/pushover_service_spec.rb | 2 +- .../slack_service/note_message_spec.rb | 1 + .../slack_service/wiki_page_message_spec.rb | 4 +-- spec/models/project_services/slack_service_spec.rb | 20 +++++------ spec/models/project_spec.rb | 36 +++++++++---------- spec/models/repository_spec.rb | 40 +++++++++++----------- spec/models/service_spec.rb | 4 +-- spec/models/user_spec.rb | 26 +++++++------- spec/models/wiki_page_spec.rb | 6 ++-- 40 files changed, 218 insertions(+), 217 deletions(-) (limited to 'spec/models') diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index fb040ba82bc..cc215d252f9 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -53,59 +53,59 @@ describe ApplicationSetting, models: true do end context 'restricted signup domains' do - it 'set single domain' do + it 'sets single domain' do setting.domain_whitelist_raw = 'example.com' expect(setting.domain_whitelist).to eq(['example.com']) end - it 'set multiple domains with spaces' do + it 'sets multiple domains with spaces' do setting.domain_whitelist_raw = 'example.com *.example.com' expect(setting.domain_whitelist).to eq(['example.com', '*.example.com']) end - it 'set multiple domains with newlines and a space' do + it 'sets multiple domains with newlines and a space' do setting.domain_whitelist_raw = "example.com\n *.example.com" expect(setting.domain_whitelist).to eq(['example.com', '*.example.com']) end - it 'set multiple domains with commas' do + it 'sets multiple domains with commas' do setting.domain_whitelist_raw = "example.com, *.example.com" expect(setting.domain_whitelist).to eq(['example.com', '*.example.com']) end end context 'blacklisted signup domains' do - it 'set single domain' do + it 'sets single domain' do setting.domain_blacklist_raw = 'example.com' expect(setting.domain_blacklist).to contain_exactly('example.com') end - it 'set multiple domains with spaces' do + it 'sets multiple domains with spaces' do setting.domain_blacklist_raw = 'example.com *.example.com' expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com') end - it 'set multiple domains with newlines and a space' do + it 'sets multiple domains with newlines and a space' do setting.domain_blacklist_raw = "example.com\n *.example.com" expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com') end - it 'set multiple domains with commas' do + it 'sets multiple domains with commas' do setting.domain_blacklist_raw = "example.com, *.example.com" expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com') end - it 'set multiple domains with semicolon' do + it 'sets multiple domains with semicolon' do setting.domain_blacklist_raw = "example.com; *.example.com" expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com') end - it 'set multiple domains with mixture of everything' do + it 'sets multiple domains with mixture of everything' do setting.domain_blacklist_raw = "example.com; *.example.com\n test.com\sblock.com yes.com" expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com', 'test.com', 'block.com', 'yes.com') end - it 'set multiple domain with file' do + it 'sets multiple domain with file' do setting.domain_blacklist_file = File.open(Rails.root.join('spec/fixtures/', 'domain_blacklist.txt')) expect(setting.domain_blacklist).to contain_exactly('example.com', 'test.com', 'foo.bar') end diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 6ad8bfef4f2..72688137f08 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -23,19 +23,19 @@ describe BroadcastMessage, models: true do end describe '.current' do - it "should return last message if time match" do + it "returns last message if time match" do message = create(:broadcast_message) expect(BroadcastMessage.current).to eq message end - it "should return nil if time not come" do + it "returns nil if time not come" do create(:broadcast_message, :future) expect(BroadcastMessage.current).to be_nil end - it "should return nil if time has passed" do + it "returns nil if time has passed" do create(:broadcast_message, :expired) expect(BroadcastMessage.current).to be_nil diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index dc88697199b..9ecc9aac84b 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -32,7 +32,7 @@ describe Ci::Build, models: true do end let(:create_from_build) { Ci::Build.create_from build } - it 'there should be a pending task' do + it 'exists a pending task' do expect(Ci::Build.pending.count(:all)).to eq 0 create_from_build expect(Ci::Build.pending.count(:all)).to be > 0 @@ -573,19 +573,19 @@ describe Ci::Build, models: true do let!(:rubocop_test) { create(:ci_build, pipeline: pipeline, name: 'rubocop', stage_idx: 1, stage: 'test') } let!(:staging) { create(:ci_build, pipeline: pipeline, name: 'staging', stage_idx: 2, stage: 'deploy') } - it 'to have no dependents if this is first build' do + it 'expects to have no dependents if this is first build' do expect(build.depends_on_builds).to be_empty end - it 'to have one dependent if this is test' do + it 'expects to have one dependent if this is test' do expect(rspec_test.depends_on_builds.map(&:id)).to contain_exactly(build.id) end - it 'to have all builds from build and test stage if this is last' do + it 'expects to have all builds from build and test stage if this is last' do expect(staging.depends_on_builds.map(&:id)).to contain_exactly(build.id, rspec_test.id, rubocop_test.id) end - it 'to have retried builds instead the original ones' do + it 'expects to have retried builds instead the original ones' do retried_rspec = Ci::Build.retry(rspec_test) expect(staging.depends_on_builds.map(&:id)).to contain_exactly(build.id, retried_rspec.id, rubocop_test.id) end @@ -655,23 +655,23 @@ describe Ci::Build, models: true do describe 'build erasable' do shared_examples 'erasable' do - it 'should remove artifact file' do + it 'removes artifact file' do expect(build.artifacts_file.exists?).to be_falsy end - it 'should remove artifact metadata file' do + it 'removes artifact metadata file' do expect(build.artifacts_metadata.exists?).to be_falsy end - it 'should erase build trace in trace file' do + it 'erases build trace in trace file' do expect(build.trace).to be_empty end - it 'should set erased to true' do + it 'sets erased to true' do expect(build.erased?).to be true end - it 'should set erase date' do + it 'sets erase date' do expect(build.erased_at).not_to be_falsy end end @@ -704,7 +704,7 @@ describe Ci::Build, models: true do include_examples 'erasable' - it 'should record user who erased a build' do + it 'records user who erased a build' do expect(build.erased_by).to eq user end end @@ -714,7 +714,7 @@ describe Ci::Build, models: true do include_examples 'erasable' - it 'should not set user who erased a build' do + it 'does not set user who erased a build' do expect(build.erased_by).to be_nil end end @@ -750,7 +750,7 @@ describe Ci::Build, models: true do end describe '#erase' do - it 'should not raise error' do + it 'does not raise error' do expect { build.erase }.not_to raise_error end end @@ -900,7 +900,7 @@ describe Ci::Build, models: true do context 'when build is running' do before { build.run! } - it 'should return false' do + it 'returns false' do expect(build.retryable?).to be false end end @@ -908,7 +908,7 @@ describe Ci::Build, models: true do context 'when build is finished' do before { build.success! } - it 'should return true' do + it 'returns true' do expect(build.retryable?).to be true end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 0d4c86955ce..ccee591cf7a 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -427,7 +427,7 @@ describe Ci::Pipeline, models: true do end describe '#update_state' do - it 'execute update_state after touching object' do + it 'executes update_state after touching object' do expect(pipeline).to receive(:update_state).and_return(true) pipeline.touch end @@ -435,7 +435,7 @@ describe Ci::Pipeline, models: true do context 'dependent objects' do let(:commit_status) { build :commit_status, pipeline: pipeline } - it 'execute update_state after saving dependent object' do + it 'executes update_state after saving dependent object' do expect(pipeline).to receive(:update_state).and_return(true) commit_status.save end @@ -513,7 +513,7 @@ describe Ci::Pipeline, models: true do create :ci_build, :success, pipeline: pipeline, name: 'rspec' create :ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop' end - + it 'returns true' do is_expected.to be_truthy end @@ -524,7 +524,7 @@ describe Ci::Pipeline, models: true do create :ci_build, :success, pipeline: pipeline, name: 'rspec' create :ci_build, :allowed_to_fail, :success, pipeline: pipeline, name: 'rubocop' end - + it 'returns false' do is_expected.to be_falsey end diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 474b0b1621d..3ca9231f58e 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -4,12 +4,12 @@ describe Ci::Trigger, models: true do let(:project) { FactoryGirl.create :empty_project } describe 'before_validation' do - it 'should set an random token if none provided' do + it 'sets an random token if none provided' do trigger = FactoryGirl.create :ci_trigger_without_token, project: project expect(trigger.token).not_to be_nil end - it 'should not set an random token if one provided' do + it 'does not set an random token if one provided' do trigger = FactoryGirl.create :ci_trigger, project: project expect(trigger.token).to eq('token') end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index ff6371ad685..fcfa3138ce5 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -133,7 +133,7 @@ describe CommitStatus, models: true do @commit5 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'success' end - it 'return unique statuses' do + it 'returns unique statuses' do is_expected.to eq([@commit4, @commit5]) end end @@ -149,7 +149,7 @@ describe CommitStatus, models: true do @commit5 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'ee', ref: nil, status: 'canceled' end - it 'return statuses that are running or pending' do + it 'returns statuses that are running or pending' do is_expected.to eq([@commit1, @commit2]) end end @@ -160,7 +160,7 @@ describe CommitStatus, models: true do context 'when no before_sha is set for pipeline' do before { pipeline.before_sha = nil } - it 'return blank sha' do + it 'returns blank sha' do is_expected.to eq(Gitlab::Git::BLANK_SHA) end end @@ -169,7 +169,7 @@ describe CommitStatus, models: true do let(:value) { '1234' } before { pipeline.before_sha = value } - it 'return the set value' do + it 'returns the set value' do is_expected.to eq(value) end end @@ -186,7 +186,7 @@ describe CommitStatus, models: true do context 'stages list' do subject { CommitStatus.where(pipeline: pipeline).stages } - it 'return ordered list of stages' do + it 'returns ordered list of stages' do is_expected.to eq(%w(build test deploy)) end end @@ -194,7 +194,7 @@ describe CommitStatus, models: true do context 'stages with statuses' do subject { CommitStatus.where(pipeline: pipeline).latest.stages_status } - it 'return list of stages with statuses' do + it 'returns list of stages with statuses' do is_expected.to eq({ 'build' => 'failed', 'test' => 'success', diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 7e9ab8940cf..b7e973798a3 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -26,53 +26,53 @@ describe Milestone, 'Milestoneish' do end describe '#closed_items_count' do - it 'should not count confidential issues for non project members' do + it 'does not count confidential issues for non project members' do expect(milestone.closed_items_count(non_member)).to eq 2 end - it 'should not count confidential issues for project members with guest role' do + it 'does not count confidential issues for project members with guest role' do expect(milestone.closed_items_count(guest)).to eq 2 end - it 'should count confidential issues for author' do + it 'counts confidential issues for author' do expect(milestone.closed_items_count(author)).to eq 4 end - it 'should count confidential issues for assignee' do + it 'counts confidential issues for assignee' do expect(milestone.closed_items_count(assignee)).to eq 4 end - it 'should count confidential issues for project members' do + it 'counts confidential issues for project members' do expect(milestone.closed_items_count(member)).to eq 6 end - it 'should count all issues for admin' do + it 'counts all issues for admin' do expect(milestone.closed_items_count(admin)).to eq 6 end end describe '#total_items_count' do - it 'should not count confidential issues for non project members' do + it 'does not count confidential issues for non project members' do expect(milestone.total_items_count(non_member)).to eq 4 end - it 'should not count confidential issues for project members with guest role' do + it 'does not count confidential issues for project members with guest role' do expect(milestone.total_items_count(guest)).to eq 4 end - it 'should count confidential issues for author' do + it 'counts confidential issues for author' do expect(milestone.total_items_count(author)).to eq 7 end - it 'should count confidential issues for assignee' do + it 'counts confidential issues for assignee' do expect(milestone.total_items_count(assignee)).to eq 7 end - it 'should count confidential issues for project members' do + it 'counts confidential issues for project members' do expect(milestone.total_items_count(member)).to eq 10 end - it 'should count all issues for admin' do + it 'counts all issues for admin' do expect(milestone.total_items_count(admin)).to eq 10 end end @@ -91,27 +91,27 @@ describe Milestone, 'Milestoneish' do end describe '#percent_complete' do - it 'should not count confidential issues for non project members' do + it 'does not count confidential issues for non project members' do expect(milestone.percent_complete(non_member)).to eq 50 end - it 'should not count confidential issues for project members with guest role' do + it 'does not count confidential issues for project members with guest role' do expect(milestone.percent_complete(guest)).to eq 50 end - it 'should count confidential issues for author' do + it 'counts confidential issues for author' do expect(milestone.percent_complete(author)).to eq 57 end - it 'should count confidential issues for assignee' do + it 'counts confidential issues for assignee' do expect(milestone.percent_complete(assignee)).to eq 57 end - it 'should count confidential issues for project members' do + it 'counts confidential issues for project members' do expect(milestone.percent_complete(member)).to eq 60 end - it 'should count confidential issues for admin' do + it 'counts confidential issues for admin' do expect(milestone.percent_complete(admin)).to eq 60 end end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 9e8ebc56a31..eb64f3d0c83 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -41,7 +41,7 @@ describe ApplicationSetting, 'TokenAuthenticatable' do describe 'ensured! token' do subject { described_class.new.send("ensure_#{token_field}!") } - it 'should persist new token' do + it 'persists new token' do expect(subject).to eq described_class.current[token_field] end end diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb index f94987dcaff..9c81d159cdf 100644 --- a/spec/models/forked_project_link_spec.rb +++ b/spec/models/forked_project_link_spec.rb @@ -9,11 +9,11 @@ describe ForkedProjectLink, "add link on fork" do @project_to = fork_project(project_from, user) end - it "project_to should know it is forked" do + it "project_to knows it is forked" do expect(@project_to.forked?).to be_truthy end - it "project should know who it is forked from" do + it "project knows who it is forked from" do expect(@project_to.forked_from_project).to eq(project_from) end end @@ -29,15 +29,15 @@ describe '#forked?' do forked_project_link.save! end - it "project_to should know it is forked" do + it "project_to knows it is forked" do expect(project_to.forked?).to be_truthy end - it "project_from should not be forked" do + it "project_from is not forked" do expect(project_from.forked?).to be_falsey end - it "project_to.destroy should destroy fork_link" do + it "project_to.destroy destroys fork_link" do expect(forked_project_link).to receive(:destroy) project_to.destroy end diff --git a/spec/models/global_milestone_spec.rb b/spec/models/global_milestone_spec.rb index ae77ec5b348..92e0f7f27ce 100644 --- a/spec/models/global_milestone_spec.rb +++ b/spec/models/global_milestone_spec.rb @@ -29,15 +29,15 @@ describe GlobalMilestone, models: true do @global_milestones = GlobalMilestone.build_collection(milestones) end - it 'should have all project milestones' do + it 'has all project milestones' do expect(@global_milestones.count).to eq(2) end - it 'should have all project milestones titles' do + it 'has all project milestones titles' do expect(@global_milestones.map(&:title)).to match_array(['Milestone v1.2', 'VD-123']) end - it 'should have all project milestones' do + it 'has all project milestones' do expect(@global_milestones.map { |group_milestone| group_milestone.milestones.count }.sum).to eq(6) end end @@ -54,11 +54,11 @@ describe GlobalMilestone, models: true do @global_milestone = GlobalMilestone.new(milestone1_project1.title, milestones) end - it 'should have exactly one group milestone' do + it 'has exactly one group milestone' do expect(@global_milestone.title).to eq('Milestone v1.2') end - it 'should have all project milestones with the same title' do + it 'has all project milestones with the same title' do expect(@global_milestone.milestones.count).to eq(3) end end @@ -66,7 +66,7 @@ describe GlobalMilestone, models: true do describe '#safe_title' do let(:milestone) { create(:milestone, title: "git / test", project: project1) } - it 'should strip out slashes and spaces' do + it 'strips out slashes and spaces' do global_milestone = GlobalMilestone.new(milestone.title, [milestone]) expect(global_milestone.safe_title).to eq('git-test') diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 266c46213a6..ea4b59c26b1 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -116,7 +116,7 @@ describe Group, models: true do let(:user) { create(:user) } before { group.add_users([user.id], GroupMember::GUEST) } - it "should update the group permission" do + it "updates the group permission" do expect(group.group_members.guests.map(&:user)).to include(user) group.add_users([user.id], GroupMember::DEVELOPER) expect(group.group_members.developers.map(&:user)).to include(user) @@ -128,12 +128,12 @@ describe Group, models: true do let(:user) { create(:user) } before { group.add_user(user, GroupMember::MASTER) } - it "should be true if avatar is image" do + it "is true if avatar is image" do group.update_attribute(:avatar, 'uploads/avatar.png') expect(group.avatar_type).to be_truthy end - it "should be false if avatar is html page" do + it "is false if avatar is html page" do group.update_attribute(:avatar, 'uploads/avatar.html') expect(group.avatar_type).to eq(["only images allowed"]) end diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index 983848392b7..4a457997a4f 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -24,7 +24,7 @@ describe ProjectHook, models: true do end describe '.push_hooks' do - it 'should return hooks for push events only' do + it 'returns hooks for push events only' do hook = create(:project_hook, push_events: true) create(:project_hook, push_events: false) expect(ProjectHook.push_hooks).to eq([hook]) @@ -32,7 +32,7 @@ describe ProjectHook, models: true do end describe '.tag_push_hooks' do - it 'should return hooks for tag push events only' do + it 'returns hooks for tag push events only' do hook = create(:project_hook, tag_push_events: true) create(:project_hook, tag_push_events: false) expect(ProjectHook.tag_push_hooks).to eq([hook]) diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 6d68e52a822..fd4a2beff58 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -73,13 +73,13 @@ describe Key, models: true do end context 'callbacks' do - it 'should add new key to authorized_file' do + it 'adds new key to authorized_file' do @key = build(:personal_key, id: 7) expect(GitlabShellWorker).to receive(:perform_async).with(:add_key, @key.shell_id, @key.key) @key.save end - it 'should remove key from authorized_file' do + it 'removes key from authorized_file' do @key = create(:personal_key) expect(GitlabShellWorker).to receive(:perform_async).with(:remove_key, @key.shell_id, @key.key) @key.destroy diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index f37f44a608e..2a09063f857 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -18,7 +18,7 @@ describe Label, models: true do describe 'validation' do it { is_expected.to validate_presence_of(:project) } - it 'should validate color code' do + it 'validates color code' do expect(label).not_to allow_value('G-ITLAB').for(:color) expect(label).not_to allow_value('AABBCC').for(:color) expect(label).not_to allow_value('#AABBCCEE').for(:color) @@ -30,7 +30,7 @@ describe Label, models: true do expect(label).to allow_value('#abcdef').for(:color) end - it 'should validate title' do + it 'validates title' do expect(label).not_to allow_value('G,ITLAB').for(:title) expect(label).not_to allow_value('').for(:title) diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb index c8ee656fe3b..2cfd26419ca 100644 --- a/spec/models/legacy_diff_note_spec.rb +++ b/spec/models/legacy_diff_note_spec.rb @@ -5,12 +5,12 @@ describe LegacyDiffNote, models: true do let!(:note) { create(:legacy_diff_note_on_commit, note: "+1 from me") } let!(:commit) { note.noteable } - it "should save a valid note" do + it "saves a valid note" do expect(note.commit_id).to eq(commit.id) expect(note.noteable.id).to eq(commit.id) end - it "should be recognized by #legacy_diff_note?" do + it "is recognized by #legacy_diff_note?" do expect(note).to be_legacy_diff_note end end diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 18439cac2a4..4f875fd257a 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -22,7 +22,7 @@ require 'spec_helper' describe GroupMember, models: true do describe 'notifications' do describe "#after_create" do - it "should send email to user" do + it "sends email to user" do membership = build(:group_member) allow(membership).to receive(:notification_service). @@ -40,7 +40,7 @@ describe GroupMember, models: true do and_return(double('NotificationService').as_null_object) end - it "should send email to user" do + it "sends email to user" do expect(@group_member).to receive(:notification_service) @group_member.update_attribute(:access_level, GroupMember::MASTER) end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index e7c0c506463..28673de3189 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -52,7 +52,7 @@ describe ProjectMember, models: true do master_todos end - it "destroy itself and delete associated todos" do + it "destroys itself and delete associated todos" do expect(owner.user.todos.size).to eq(2) expect(master.user.todos.size).to eq(3) expect(Todo.count).to eq(5) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d793cfd0bde..3270b877c1a 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -188,12 +188,12 @@ describe MergeRequest, models: true do create(:note, noteable: merge_request, project: merge_request.project) end - it "should include notes for commits" do + it "includes notes for commits" do expect(merge_request.commits).not_to be_empty expect(merge_request.mr_and_commit_notes.count).to eq(2) end - it "should include notes for commits from target project as well" do + it "includes notes for commits from target project as well" do create(:note_on_commit, commit_id: merge_request.commits.first.id, project: merge_request.target_project) @@ -304,7 +304,7 @@ describe MergeRequest, models: true do expect(subject.can_remove_source_branch?(user)).to be_falsey end - it "cant remove a root ref" do + it "can't remove a root ref" do subject.source_branch = "master" subject.target_branch = "feature" diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index d661dc0e59a..d64d6cde2b5 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -28,12 +28,12 @@ describe Milestone, models: true do end describe "unique milestone title per project" do - it "shouldn't accept the same title in a project twice" do + it "does not accept the same title in a project twice" do new_milestone = Milestone.new(project: milestone.project, title: milestone.title) expect(new_milestone).not_to be_valid end - it "should accept the same title in another project" do + it "accepts the same title in another project" do project = build(:project) new_milestone = Milestone.new(project: project, title: milestone.title) @@ -42,29 +42,29 @@ describe Milestone, models: true do end describe "#percent_complete" do - it "should not count open issues" do + it "does not count open issues" do milestone.issues << issue expect(milestone.percent_complete(user)).to eq(0) end - it "should count closed issues" do + it "counts closed issues" do issue.close milestone.issues << issue expect(milestone.percent_complete(user)).to eq(100) end - it "should recover from dividing by zero" do + it "recovers from dividing by zero" do expect(milestone.percent_complete(user)).to eq(0) end end describe "#expires_at" do - it "should be nil when due_date is unset" do + it "is nil when due_date is unset" do milestone.update_attributes(due_date: nil) expect(milestone.expires_at).to be_nil end - it "should not be nil when due_date is set" do + it "is not nil when due_date is set" do milestone.update_attributes(due_date: Date.tomorrow) expect(milestone.expires_at).to be_present end @@ -121,7 +121,7 @@ describe Milestone, models: true do create :merge_request, milestone: milestone end - it 'Should return total count of issues and merge requests assigned to milestone' do + it 'returns total count of issues and merge requests assigned to milestone' do expect(milestone.total_items_count(user)).to eq 2 end end @@ -134,11 +134,11 @@ describe Milestone, models: true do create :issue end - it 'should be true if milestone active and all nested issues closed' do + it 'returns true if milestone active and all nested issues closed' do expect(milestone.can_be_closed?).to be_truthy end - it 'should be false if milestone active and not all nested issues closed' do + it 'returns false if milestone active and not all nested issues closed' do issue.milestone = milestone issue.save diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index a162da0208e..544920d1824 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -61,11 +61,11 @@ describe Namespace, models: true do allow(@namespace).to receive(:path_changed?).and_return(true) end - it "should raise error when directory exists" do + it "raises error when directory exists" do expect { @namespace.move_dir }.to raise_error("namespace directory cannot be moved") end - it "should move dir if path changed" do + it "moves dir if path changed" do new_path = @namespace.path + "_new" allow(@namespace).to receive(:path_was).and_return(@namespace.path) allow(@namespace).to receive(:path).and_return(new_path) @@ -93,7 +93,7 @@ describe Namespace, models: true do before { namespace.destroy } - it "should remove its dirs when deleted" do + it "removes its dirs when deleted" do expect(File.exist?(path)).to be(false) end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 1243f5420a7..53733d253f7 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -56,18 +56,18 @@ describe Note, models: true do let!(:note) { create(:note_on_commit, note: "+1 from me") } let!(:commit) { note.noteable } - it "should be accessible through #noteable" do + it "is accessible through #noteable" do expect(note.commit_id).to eq(commit.id) expect(note.noteable).to be_a(Commit) expect(note.noteable).to eq(commit) end - it "should save a valid note" do + it "saves a valid note" do expect(note.commit_id).to eq(commit.id) note.noteable == commit end - it "should be recognized by #for_commit?" do + it "is recognized by #for_commit?" do expect(note).to be_for_commit end diff --git a/spec/models/project_security_spec.rb b/spec/models/project_security_spec.rb index 2142c7c13ef..36379074ea0 100644 --- a/spec/models/project_security_spec.rb +++ b/spec/models/project_security_spec.rb @@ -21,7 +21,7 @@ describe Project, models: true do let(:owner_actions) { Ability.project_owner_rules } describe "Non member rules" do - it "should deny for non-project users any actions" do + it "denies for non-project users any actions" do owner_actions.each do |action| expect(@abilities.allowed?(@u1, action, @p1)).to be_falsey end @@ -33,7 +33,7 @@ describe Project, models: true do @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::GUEST) end - it "should allow for project user any guest actions" do + it "allows for project user any guest actions" do guest_actions.each do |action| expect(@abilities.allowed?(@u2, action, @p1)).to be_truthy end @@ -45,7 +45,7 @@ describe Project, models: true do @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::REPORTER) end - it "should allow for project user any report actions" do + it "allows for project user any report actions" do report_actions.each do |action| expect(@abilities.allowed?(@u2, action, @p1)).to be_truthy end @@ -58,13 +58,13 @@ describe Project, models: true do @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::DEVELOPER) end - it "should deny for developer master-specific actions" do + it "denies for developer master-specific actions" do [dev_actions - report_actions].each do |action| expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey end end - it "should allow for project user any dev actions" do + it "allows for project user any dev actions" do dev_actions.each do |action| expect(@abilities.allowed?(@u3, action, @p1)).to be_truthy end @@ -77,13 +77,13 @@ describe Project, models: true do @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::MASTER) end - it "should deny for developer master-specific actions" do + it "denies for developer master-specific actions" do [master_actions - dev_actions].each do |action| expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey end end - it "should allow for project user any master actions" do + it "allows for project user any master actions" do master_actions.each do |action| expect(@abilities.allowed?(@u3, action, @p1)).to be_truthy end @@ -96,13 +96,13 @@ describe Project, models: true do @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::MASTER) end - it "should deny for masters admin-specific actions" do + it "denies for masters admin-specific actions" do [owner_actions - master_actions].each do |action| expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey end end - it "should allow for project owner any admin actions" do + it "allows for project owner any admin actions" do owner_actions.each do |action| expect(@abilities.allowed?(@u4, action, @p1)).to be_truthy end diff --git a/spec/models/project_services/asana_service_spec.rb b/spec/models/project_services/asana_service_spec.rb index f3d15f3c1ea..dc702cfc42c 100644 --- a/spec/models/project_services/asana_service_spec.rb +++ b/spec/models/project_services/asana_service_spec.rb @@ -65,7 +65,7 @@ describe AsanaService, models: true do ) end - it 'should call Asana service to create a story' do + it 'calls Asana service to create a story' do data = create_data_for_commits('Message from commit. related to #123456') expected_message = "#{data[:user_name]} pushed to branch #{data[:ref]} of #{project.name_with_namespace} ( #{data[:commits][0][:url]} ): #{data[:commits][0][:message]}" @@ -76,7 +76,7 @@ describe AsanaService, models: true do @asana.execute(data) end - it 'should call Asana service to create a story and close a task' do + it 'calls Asana service to create a story and close a task' do data = create_data_for_commits('fix #456789') d1 = double('Asana::Task') expect(d1).to receive(:add_comment) @@ -86,7 +86,7 @@ describe AsanaService, models: true do @asana.execute(data) end - it 'should be able to close via url' do + it 'is able to close via url' do data = create_data_for_commits('closes https://app.asana.com/19292/956299/42') d1 = double('Asana::Task') expect(d1).to receive(:add_comment) @@ -96,7 +96,7 @@ describe AsanaService, models: true do @asana.execute(data) end - it 'should allow multiple matches per line' do + it 'allows multiple matches per line' do message = <<-EOF minor bigfix, refactoring, fixed #123 and Closes #456 work on #789 ref https://app.asana.com/19292/956299/42 and closing https://app.asana.com/19292/956299/12 diff --git a/spec/models/project_services/assembla_service_spec.rb b/spec/models/project_services/assembla_service_spec.rb index 17e9361dd5c..00c4e0fb64c 100644 --- a/spec/models/project_services/assembla_service_spec.rb +++ b/spec/models/project_services/assembla_service_spec.rb @@ -44,7 +44,7 @@ describe AssemblaService, models: true do WebMock.stub_request(:post, @api_url) end - it "should call Assembla API" do + it "calls Assembla API" do @assembla_service.execute(@sample_data) expect(WebMock).to have_requested(:post, @api_url).with( body: /#{@sample_data[:before]}.*#{@sample_data[:after]}.*#{project.path}/ diff --git a/spec/models/project_services/external_wiki_service_spec.rb b/spec/models/project_services/external_wiki_service_spec.rb index 5fe5ea7d2df..d7c5ea95d71 100644 --- a/spec/models/project_services/external_wiki_service_spec.rb +++ b/spec/models/project_services/external_wiki_service_spec.rb @@ -56,7 +56,7 @@ describe ExternalWikiService, models: true do @service.destroy! end - it 'should replace the wiki url' do + it 'replaces the wiki url' do wiki_path = get_project_wiki_path(project) expect(wiki_path).to match('https://gitlab.com') end diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb index b7e627e6518..6518098ceea 100644 --- a/spec/models/project_services/flowdock_service_spec.rb +++ b/spec/models/project_services/flowdock_service_spec.rb @@ -57,7 +57,7 @@ describe FlowdockService, models: true do WebMock.stub_request(:post, @api_url) end - it "should call FlowDock API" do + it "calls FlowDock API" do @flowdock_service.execute(@sample_data) @sample_data[:commits].each do |commit| # One request to Flowdock per new commit diff --git a/spec/models/project_services/gemnasium_service_spec.rb b/spec/models/project_services/gemnasium_service_spec.rb index a08f1ac229f..2c5583bdaa2 100644 --- a/spec/models/project_services/gemnasium_service_spec.rb +++ b/spec/models/project_services/gemnasium_service_spec.rb @@ -57,7 +57,7 @@ describe GemnasiumService, models: true do ) @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) end - it "should call Gemnasium service" do + it "calls Gemnasium service" do expect(Gemnasium::GitlabService).to receive(:execute).with(an_instance_of(Hash)).once @gemnasium_service.execute(@sample_data) end diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb index 7a1f106d6e3..8ef79a17d50 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -54,7 +54,7 @@ describe GitlabIssueTrackerService, models: true do @service.destroy! end - it 'should give the correct path' do + it 'gives the correct path' do expect(@service.project_url).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues") expect(@service.new_issue_url).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues/new") expect(@service.issue_url(432)).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues/432") @@ -71,7 +71,7 @@ describe GitlabIssueTrackerService, models: true do @service.destroy! end - it 'should give the correct path' do + it 'gives the correct path' do expect(@service.project_path).to eq("/gitlab/root/#{project.path_with_namespace}/issues") expect(@service.new_issue_path).to eq("/gitlab/root/#{project.path_with_namespace}/issues/new") expect(@service.issue_path(432)).to eq("/gitlab/root/#{project.path_with_namespace}/issues/432") diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 62ae5f6cf74..bf438b26690 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -61,7 +61,7 @@ describe HipchatService, models: true do WebMock.stub_request(:post, api_url) end - it 'should test and return errors' do + it 'tests and return errors' do allow(hipchat).to receive(:execute).and_raise(StandardError, 'no such room') result = hipchat.test(push_sample_data) @@ -69,7 +69,7 @@ describe HipchatService, models: true do expect(result[:result].to_s).to eq('no such room') end - it 'should use v1 if version is provided' do + it 'uses v1 if version is provided' do allow(hipchat).to receive(:api_version).and_return('v1') expect(HipChat::Client).to receive(:new).with( token, @@ -79,7 +79,7 @@ describe HipchatService, models: true do hipchat.execute(push_sample_data) end - it 'should use v2 as the version when nothing is provided' do + it 'uses v2 as the version when nothing is provided' do allow(hipchat).to receive(:api_version).and_return('') expect(HipChat::Client).to receive(:new).with( token, @@ -90,13 +90,13 @@ describe HipchatService, models: true do end context 'push events' do - it "should call Hipchat API for push events" do + it "calls Hipchat API for push events" do hipchat.execute(push_sample_data) expect(WebMock).to have_requested(:post, api_url).once end - it "should create a push message" do + it "creates a push message" do message = hipchat.send(:create_push_message, push_sample_data) push_sample_data[:object_attributes] @@ -110,13 +110,13 @@ describe HipchatService, models: true do context 'tag_push events' do let(:push_sample_data) { Gitlab::PushDataBuilder.build(project, user, Gitlab::Git::BLANK_SHA, '1' * 40, 'refs/tags/test', []) } - it "should call Hipchat API for tag push events" do + it "calls Hipchat API for tag push events" do hipchat.execute(push_sample_data) expect(WebMock).to have_requested(:post, api_url).once end - it "should create a tag push message" do + it "creates a tag push message" do message = hipchat.send(:create_push_message, push_sample_data) push_sample_data[:object_attributes] @@ -131,13 +131,13 @@ describe HipchatService, models: true do let(:issue_service) { Issues::CreateService.new(project, user) } let(:issues_sample_data) { issue_service.hook_data(issue, 'open') } - it "should call Hipchat API for issue events" do + it "calls Hipchat API for issue events" do hipchat.execute(issues_sample_data) expect(WebMock).to have_requested(:post, api_url).once end - it "should create an issue message" do + it "creates an issue message" do message = hipchat.send(:create_issue_message, issues_sample_data) obj_attr = issues_sample_data[:object_attributes] @@ -154,13 +154,13 @@ describe HipchatService, models: true do let(:merge_service) { MergeRequests::CreateService.new(project, user) } let(:merge_sample_data) { merge_service.hook_data(merge_request, 'open') } - it "should call Hipchat API for merge requests events" do + it "calls Hipchat API for merge requests events" do hipchat.execute(merge_sample_data) expect(WebMock).to have_requested(:post, api_url).once end - it "should create a merge request message" do + it "creates a merge request message" do message = hipchat.send(:create_merge_request_message, merge_sample_data) @@ -184,7 +184,7 @@ describe HipchatService, models: true do note: 'a comment on a commit') end - it "should call Hipchat API for commit comment events" do + it "calls Hipchat API for commit comment events" do data = Gitlab::NoteDataBuilder.build(commit_note, user) hipchat.execute(data) @@ -216,7 +216,7 @@ describe HipchatService, models: true do note: "merge request note") end - it "should call Hipchat API for merge request comment events" do + it "calls Hipchat API for merge request comment events" do data = Gitlab::NoteDataBuilder.build(merge_request_note, user) hipchat.execute(data) @@ -243,7 +243,7 @@ describe HipchatService, models: true do note: "issue note") end - it "should call Hipchat API for issue comment events" do + it "calls Hipchat API for issue comment events" do data = Gitlab::NoteDataBuilder.build(issue_note, user) hipchat.execute(data) @@ -269,7 +269,7 @@ describe HipchatService, models: true do note: "snippet note") end - it "should call Hipchat API for snippet comment events" do + it "calls Hipchat API for snippet comment events" do data = Gitlab::NoteDataBuilder.build(snippet_note, user) hipchat.execute(data) @@ -297,13 +297,13 @@ describe HipchatService, models: true do context 'for failed' do before { build.drop } - it "should call Hipchat API" do + it "calls Hipchat API" do hipchat.execute(data) expect(WebMock).to have_requested(:post, api_url).once end - it "should create a build message" do + it "creates a build message" do message = hipchat.send(:create_build_message, data) project_url = project.web_url @@ -325,13 +325,13 @@ describe HipchatService, models: true do build.success end - it "should call Hipchat API" do + it "calls Hipchat API" do hipchat.notify_only_broken_builds = false hipchat.execute(data) expect(WebMock).to have_requested(:post, api_url).once end - it "should notify only broken" do + it "notifies only broken" do hipchat.notify_only_broken_builds = true hipchat.execute(data) expect(WebMock).not_to have_requested(:post, api_url).once diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index 4ee022a5171..b528baaf15c 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -71,7 +71,7 @@ describe IrkerService, models: true do @irker_server.close end - it 'should send valid JSON messages to an Irker listener' do + it 'sends valid JSON messages to an Irker listener' do irker.execute(sample_data) conn = @irker_server.accept diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 5a97cf370da..342403f6354 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -75,7 +75,7 @@ describe JiraService, models: true do WebMock.stub_request(:post, @comment_url) end - it "should call JIRA API" do + it "calls JIRA API" do @jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project)) expect(WebMock).to have_requested(:post, @comment_url).with( @@ -128,7 +128,7 @@ describe JiraService, models: true do expect(@jira_service.api_url).to eq("http://jira_edited.example.com/rest/api/2") end - it "should reset password if url changed, even if setter called multiple times" do + it "resets password if url changed, even if setter called multiple times" do @jira_service.api_url = 'http://jira1.example.com/rest/api/2' @jira_service.api_url = 'http://jira1.example.com/rest/api/2' @jira_service.save @@ -181,7 +181,7 @@ describe JiraService, models: true do @service.destroy! end - it 'should be initialized' do + it 'is initialized' do expect(@service.title).to eq('JIRA') expect(@service.description).to eq("Jira issue tracker") end @@ -197,7 +197,7 @@ describe JiraService, models: true do @service.destroy! end - it "should be correct" do + it "is correct" do expect(@service.title).to eq('Jira One') expect(@service.description).to eq('Jira One issue tracker') end @@ -225,7 +225,7 @@ describe JiraService, models: true do @service.destroy! end - it 'should be prepopulated with the settings' do + it 'is prepopulated with the settings' do expect(@service.properties["project_url"]).to eq('http://jira.sample/projects/project_a') expect(@service.properties["issues_url"]).to eq("http://jira.sample/issues/:id") expect(@service.properties["new_issue_url"]).to eq("http://jira.sample/projects/project_a/issues/new") diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb index 555d9757b47..19c0270a493 100644 --- a/spec/models/project_services/pushover_service_spec.rb +++ b/spec/models/project_services/pushover_service_spec.rb @@ -72,7 +72,7 @@ describe PushoverService, models: true do WebMock.stub_request(:post, api_url) end - it 'should call Pushover API' do + it 'calls Pushover API' do pushover.execute(sample_data) expect(WebMock).to have_requested(:post, api_url).once diff --git a/spec/models/project_services/slack_service/note_message_spec.rb b/spec/models/project_services/slack_service/note_message_spec.rb index 379c3e1219c..41b93f08050 100644 --- a/spec/models/project_services/slack_service/note_message_spec.rb +++ b/spec/models/project_services/slack_service/note_message_spec.rb @@ -60,6 +60,7 @@ describe SlackService::NoteMessage, models: true do title: "merge request title\ndetails\n" } end + it 'returns a message regarding notes on a merge request' do message = SlackService::NoteMessage.new(@args) expect(message.pretext).to eq("Test User commented on " \ diff --git a/spec/models/project_services/slack_service/wiki_page_message_spec.rb b/spec/models/project_services/slack_service/wiki_page_message_spec.rb index 46dedb66c7c..13aea0b0600 100644 --- a/spec/models/project_services/slack_service/wiki_page_message_spec.rb +++ b/spec/models/project_services/slack_service/wiki_page_message_spec.rb @@ -47,7 +47,7 @@ describe SlackService::WikiPageMessage, models: true do context 'when :action == "create"' do before { args[:object_attributes][:action] = 'create' } - it 'it returns the attachment for a new wiki page' do + it 'returns the attachment for a new wiki page' do expect(subject.attachments).to eq([ { text: "Wiki page description", @@ -60,7 +60,7 @@ describe SlackService::WikiPageMessage, models: true do context 'when :action == "update"' do before { args[:object_attributes][:action] = 'update' } - it 'it returns the attachment for an updated wiki page' do + it 'returns the attachment for an updated wiki page' do expect(subject.attachments).to eq([ { text: "Wiki page description", diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index df511b1bc4c..45a5f4ef12a 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -93,31 +93,31 @@ describe SlackService, models: true do @wiki_page_sample_data = wiki_page_service.hook_data(@wiki_page, 'create') end - it "should call Slack API for push events" do + it "calls Slack API for push events" do slack.execute(push_sample_data) expect(WebMock).to have_requested(:post, webhook_url).once end - it "should call Slack API for issue events" do + it "calls Slack API for issue events" do slack.execute(@issues_sample_data) expect(WebMock).to have_requested(:post, webhook_url).once end - it "should call Slack API for merge requests events" do + it "calls Slack API for merge requests events" do slack.execute(@merge_sample_data) expect(WebMock).to have_requested(:post, webhook_url).once end - it "should call Slack API for wiki page events" do + it "calls Slack API for wiki page events" do slack.execute(@wiki_page_sample_data) expect(WebMock).to have_requested(:post, webhook_url).once end - it 'should use the username as an option for slack when configured' do + it 'uses the username as an option for slack when configured' do allow(slack).to receive(:username).and_return(username) expect(Slack::Notifier).to receive(:new). with(webhook_url, username: username). @@ -128,7 +128,7 @@ describe SlackService, models: true do slack.execute(push_sample_data) end - it 'should use the channel as an option when it is configured' do + it 'uses the channel as an option when it is configured' do allow(slack).to receive(:channel).and_return(channel) expect(Slack::Notifier).to receive(:new). with(webhook_url, channel: channel). @@ -234,7 +234,7 @@ describe SlackService, models: true do note: 'a comment on a commit') end - it "should call Slack API for commit comment events" do + it "calls Slack API for commit comment events" do data = Gitlab::NoteDataBuilder.build(commit_note, user) slack.execute(data) @@ -248,7 +248,7 @@ describe SlackService, models: true do note: "merge request note") end - it "should call Slack API for merge request comment events" do + it "calls Slack API for merge request comment events" do data = Gitlab::NoteDataBuilder.build(merge_request_note, user) slack.execute(data) @@ -261,7 +261,7 @@ describe SlackService, models: true do create(:note_on_issue, project: project, note: "issue note") end - it "should call Slack API for issue comment events" do + it "calls Slack API for issue comment events" do data = Gitlab::NoteDataBuilder.build(issue_note, user) slack.execute(data) @@ -275,7 +275,7 @@ describe SlackService, models: true do note: "snippet note") end - it "should call Slack API for snippet comment events" do + it "calls Slack API for snippet comment events" do data = Gitlab::NoteDataBuilder.build(snippet_note, user) slack.execute(data) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 567f87b9970..1c3d694075a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -89,7 +89,7 @@ describe Project, models: true do it { is_expected.to validate_presence_of(:namespace) } it { is_expected.to validate_presence_of(:repository_storage) } - it 'should not allow new projects beyond user limits' do + it 'does not allow new projects beyond user limits' do project2 = build(:project) allow(project2).to receive(:creator).and_return(double(can_create_project?: false, projects_limit: 0).as_null_object) expect(project2).not_to be_valid @@ -98,7 +98,7 @@ describe Project, models: true do describe 'wiki path conflict' do context "when the new path has been used by the wiki of other Project" do - it 'should have an error on the name attribute' do + it 'has an error on the name attribute' do new_project = build_stubbed(:project, namespace_id: project.namespace_id, path: "#{project.path}.wiki") expect(new_project).not_to be_valid @@ -107,7 +107,7 @@ describe Project, models: true do end context "when the new wiki path has been used by the path of other Project" do - it 'should have an error on the name attribute' do + it 'has an error on the name attribute' do project_with_wiki_suffix = create(:project, path: 'foo.wiki') new_project = build_stubbed(:project, namespace_id: project_with_wiki_suffix.namespace_id, path: 'foo') @@ -125,7 +125,7 @@ describe Project, models: true do allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end - it "should not allow repository storages that don't match a label in the configuration" do + it "does not allow repository storages that don't match a label in the configuration" do expect(project2).not_to be_valid expect(project2.errors[:repository_storage].first).to match(/is not included in the list/) end @@ -172,12 +172,12 @@ describe Project, models: true do end describe 'project token' do - it 'should set an random token if none provided' do + it 'sets an random token if none provided' do project = FactoryGirl.create :empty_project, runners_token: '' expect(project.runners_token).not_to eq('') end - it 'should not set an random toke if one provided' do + it 'does not set an random toke if one provided' do project = FactoryGirl.create :empty_project, runners_token: 'my-token' expect(project.runners_token).to eq('my-token') end @@ -225,7 +225,7 @@ describe Project, models: true do end end - it 'should return valid url to repo' do + it 'returns valid url to repo' do project = Project.new(path: 'somewhere') expect(project.url_to_repo).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + 'somewhere.git') end @@ -279,7 +279,7 @@ describe Project, models: true do let(:last_event) { double(created_at: Time.now) } describe 'last_activity' do - it 'should alias last_activity to last_event' do + it 'alias last_activity to last_event' do allow(project).to receive(:last_event).and_return(last_event) expect(project.last_activity).to eq(last_event) end @@ -350,13 +350,13 @@ describe Project, models: true do let(:prev_commit_id) { merge_request.commits.last.id } let(:commit_id) { merge_request.commits.first.id } - it 'should close merge request if last commit from source branch was pushed to target branch' do + it 'closes merge request if last commit from source branch was pushed to target branch' do project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.target_branch}", key.user) merge_request.reload expect(merge_request.merged?).to be_truthy end - it 'should update merge request commits with new one if pushed to source branch' do + it 'updates merge request commits with new one if pushed to source branch' do project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.source_branch}", key.user) merge_request.reload expect(merge_request.diff_head_sha).to eq(commit_id) @@ -433,11 +433,11 @@ describe Project, models: true do let(:project) { create(:project) } let(:ext_project) { create(:redmine_project) } - it "should be true if used internal tracker" do + it "is true if used internal tracker" do expect(project.default_issues_tracker?).to be_truthy end - it "should be false if used other tracker" do + it "is false if used other tracker" do expect(ext_project.default_issues_tracker?).to be_falsey end end @@ -636,12 +636,12 @@ describe Project, models: true do describe '#avatar_type' do let(:project) { create(:project) } - it 'should be true if avatar is image' do + it 'is true if avatar is image' do project.update_attribute(:avatar, 'uploads/avatar.png') expect(project.avatar_type).to be_truthy end - it 'should be false if avatar is html page' do + it 'is false if avatar is html page' do project.update_attribute(:avatar, 'uploads/avatar.html') expect(project.avatar_type).to eq(['only images allowed']) end @@ -814,16 +814,16 @@ describe Project, models: true do context 'for shared runners disabled' do let(:shared_runners_enabled) { false } - it 'there are no runners available' do + it 'has no runners available' do expect(project.any_runners?).to be_falsey end - it 'there is a specific runner' do + it 'has a specific runner' do project.runners << specific_runner expect(project.any_runners?).to be_truthy end - it 'there is a shared runner, but they are prohibited to use' do + it 'has a shared runner, but they are prohibited to use' do shared_runner expect(project.any_runners?).to be_falsey end @@ -837,7 +837,7 @@ describe Project, models: true do context 'for shared runners enabled' do let(:shared_runners_enabled) { true } - it 'there is a shared runner' do + it 'has a shared runner' do shared_runner expect(project.any_runners?).to be_truthy end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 2a053b1804f..f7dbfd712cc 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -340,14 +340,14 @@ describe Repository, models: true do describe '#add_branch' do context 'when pre hooks were successful' do - it 'should run without errors' do + it 'runs without errors' do hook = double(trigger: [true, nil]) expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) expect { repository.add_branch(user, 'new_feature', 'master') }.not_to raise_error end - it 'should create the branch' do + it 'creates the branch' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) branch = repository.add_branch(user, 'new_feature', 'master') @@ -363,7 +363,7 @@ describe Repository, models: true do end context 'when pre hooks failed' do - it 'should get an error' do + it 'gets an error' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do @@ -371,7 +371,7 @@ describe Repository, models: true do end.to raise_error(GitHooksService::PreReceiveError) end - it 'should not create the branch' do + it 'does not create the branch' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do @@ -387,14 +387,14 @@ describe Repository, models: true do let(:blank_sha) { '0000000000000000000000000000000000000000' } context 'when pre hooks were successful' do - it 'should run without errors' do + it 'runs without errors' do expect_any_instance_of(GitHooksService).to receive(:execute). with(user, project.repository.path_to_repo, old_rev, blank_sha, 'refs/heads/feature') expect { repository.rm_branch(user, 'feature') }.not_to raise_error end - it 'should delete the branch' do + it 'deletes the branch' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) expect { repository.rm_branch(user, 'feature') }.not_to raise_error @@ -404,7 +404,7 @@ describe Repository, models: true do end context 'when pre hooks failed' do - it 'should get an error' do + it 'gets an error' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do @@ -412,7 +412,7 @@ describe Repository, models: true do end.to raise_error(GitHooksService::PreReceiveError) end - it 'should not delete the branch' do + it 'does not delete the branch' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do @@ -433,13 +433,13 @@ describe Repository, models: true do and_yield.and_return(true) end - it 'should run without errors' do + it 'runs without errors' do expect do repository.commit_with_hooks(user, 'feature') { sample_commit.id } end.not_to raise_error end - it 'should ensure the autocrlf Git option is set to :input' do + it 'ensures the autocrlf Git option is set to :input' do expect(repository).to receive(:update_autocrlf_option) repository.commit_with_hooks(user, 'feature') { sample_commit.id } @@ -455,7 +455,7 @@ describe Repository, models: true do end context 'when pre hooks failed' do - it 'should get an error' do + it 'gets an error' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do @@ -715,7 +715,7 @@ describe Repository, models: true do end describe '#merge' do - it 'should merge the code and return the commit id' do + it 'merges the code and return the commit id' do expect(merge_commit).to be_present expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present end @@ -726,13 +726,13 @@ describe Repository, models: true do let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } context 'when there is a conflict' do - it 'should abort the operation' do + it 'aborts the operation' do expect(repository.revert(user, new_image_commit, 'master')).to eq(false) end end context 'when commit was already reverted' do - it 'should abort the operation' do + it 'aborts the operation' do repository.revert(user, update_image_commit, 'master') expect(repository.revert(user, update_image_commit, 'master')).to eq(false) @@ -740,13 +740,13 @@ describe Repository, models: true do end context 'when commit can be reverted' do - it 'should revert the changes' do + it 'reverts the changes' do expect(repository.revert(user, update_image_commit, 'master')).to be_truthy end end context 'reverting a merge commit' do - it 'should revert the changes' do + it 'reverts the changes' do merge_commit expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).to be_present @@ -762,13 +762,13 @@ describe Repository, models: true do let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') } context 'when there is a conflict' do - it 'should abort the operation' do + it 'aborts the operation' do expect(repository.cherry_pick(user, conflict_commit, 'master')).to eq(false) end end context 'when commit was already cherry-picked' do - it 'should abort the operation' do + it 'aborts the operation' do repository.cherry_pick(user, pickable_commit, 'master') expect(repository.cherry_pick(user, pickable_commit, 'master')).to eq(false) @@ -776,13 +776,13 @@ describe Repository, models: true do end context 'when commit can be cherry-picked' do - it 'should cherry-pick the changes' do + it 'cherry-picks the changes' do expect(repository.cherry_pick(user, pickable_commit, 'master')).to be_truthy end end context 'cherry-picking a merge commit' do - it 'should cherry-pick the changes' do + it 'cherry-picks the changes' do expect(repository.blob_at_branch('master', 'foo/bar/.gitkeep')).to be_nil repository.cherry_pick(user, pickable_merge, 'master') diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 67b3783d514..05056a4bb47 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -65,13 +65,13 @@ describe Service, models: true do end let(:project) { create(:project) } - describe 'should be prefilled for projects pushover service' do + describe 'is prefilled for projects pushover service' do before do service_template project.build_missing_services end - it "should have all fields prefilled" do + it "has all fields prefilled" do service = project.pushover_service expect(service.template).to eq(false) expect(service.device).to eq('MyDevice') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9f432501c59..f67acbbef37 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -166,7 +166,7 @@ describe User, models: true do allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return(['*.example.com']) end - it 'should give priority to whitelist and allow info@test.example.com' do + it 'gives priority to whitelist and allow info@test.example.com' do user = build(:user, email: 'info@test.example.com') expect(user).to be_valid end @@ -304,18 +304,18 @@ describe User, models: true do end describe '#generate_password' do - it "should execute callback when force_random_password specified" do + it "executes callback when force_random_password specified" do user = build(:user, force_random_password: true) expect(user).to receive(:generate_password) user.save end - it "should not generate password by default" do + it "does not generate password by default" do user = create(:user, password: 'abcdefghe') expect(user.password).to eq('abcdefghe') end - it "should generate password when forcing random password" do + it "generates password when forcing random password" do allow(Devise).to receive(:friendly_token).and_return('123456789') user = create(:user, password: 'abcdefg', force_random_password: true) expect(user.password).to eq('12345678') @@ -323,7 +323,7 @@ describe User, models: true do end describe 'authentication token' do - it "should have authentication token" do + it "has authentication token" do user = create(:user) expect(user.authentication_token).not_to be_blank end @@ -430,7 +430,7 @@ describe User, models: true do describe 'blocking user' do let(:user) { create(:user, name: 'John Smith') } - it "should block user" do + it "blocks user" do user.block expect(user.blocked?).to be_truthy end @@ -501,7 +501,7 @@ describe User, models: true do describe 'with defaults' do let(:user) { User.new } - it "should apply defaults to user" do + it "applies defaults to user" do expect(user.projects_limit).to eq(Gitlab.config.gitlab.default_projects_limit) expect(user.can_create_group).to eq(Gitlab.config.gitlab.default_can_create_group) expect(user.theme_id).to eq(Gitlab.config.gitlab.default_theme) @@ -512,7 +512,7 @@ describe User, models: true do describe 'with default overrides' do let(:user) { User.new(projects_limit: 123, can_create_group: false, can_create_team: true, theme_id: 1) } - it "should apply defaults to user" do + it "applies defaults to user" do expect(user.projects_limit).to eq(123) expect(user.can_create_group).to be_falsey expect(user.theme_id).to eq(1) @@ -602,7 +602,7 @@ describe User, models: true do describe 'by_username_or_id' do let(:user1) { create(:user, username: 'foo') } - it "should get the correct user" do + it "gets the correct user" do expect(User.by_username_or_id(user1.id)).to eq(user1) expect(User.by_username_or_id('foo')).to eq(user1) expect(User.by_username_or_id(-1)).to be_nil @@ -614,7 +614,7 @@ describe User, models: true do let(:username) { 'John' } let!(:user) { create(:user, username: username) } - it 'should get the correct user' do + it 'gets the correct user' do expect(User.by_login(user.email.upcase)).to eq user expect(User.by_login(user.email)).to eq user expect(User.by_login(username.downcase)).to eq user @@ -639,7 +639,7 @@ describe User, models: true do describe 'all_ssh_keys' do it { is_expected.to have_many(:keys).dependent(:destroy) } - it "should have all ssh keys" do + it "has all ssh keys" do user = create :user key = create :key, key: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD33bWLBxu48Sev9Fert1yzEO4WGcWglWF7K/AwblIUFselOt/QdOL9DSjpQGxLagO1s9wl53STIO8qGS4Ms0EJZyIXOEFMjFJ5xmjSy+S37By4sG7SsltQEHMxtbtFOaW5LV2wCrX+rUsRNqLMamZjgjcPO0/EgGCXIGMAYW4O7cwGZdXWYIhQ1Vwy+CsVMDdPkPgBXqK7nR/ey8KMs8ho5fMNgB5hBw/AL9fNGhRw3QTD6Q12Nkhl4VZES2EsZqlpNnJttnPdp847DUsT6yuLRlfiQfz5Cn9ysHFdXObMN5VYIiPFwHeYCZp1X2S4fDZooRE8uOLTfxWHPXwrhqSH", user_id: user.id @@ -650,12 +650,12 @@ describe User, models: true do describe '#avatar_type' do let(:user) { create(:user) } - it "should be true if avatar is image" do + it "is true if avatar is image" do user.update_attribute(:avatar, 'uploads/avatar.png') expect(user.avatar_type).to be_truthy end - it "should be false if avatar is html page" do + it "is false if avatar is html page" do user.update_attribute(:avatar, 'uploads/avatar.html') expect(user.avatar_type).to eq(["only images allowed"]) end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index ddc49495eda..5c34b1b0a30 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -147,12 +147,12 @@ describe WikiPage, models: true do @page = wiki.find_page("Delete Page") end - it "should delete the page" do + it "deletes the page" do @page.delete expect(wiki.pages).to be_empty end - it "should return true" do + it "returns true" do expect(@page.delete).to eq(true) end end @@ -183,7 +183,7 @@ describe WikiPage, models: true do destroy_page("Title") end - it "should be replace a hyphen to a space" do + it "replaces a hyphen to a space" do @page.title = "Import-existing-repositories-into-GitLab" expect(@page.title).to eq("Import existing repositories into GitLab") end -- cgit v1.2.1 From 1e6316172b913b622379675d3d48e6837dfc1843 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 9 Aug 2016 14:08:33 -0700 Subject: Add a method in Project to return a cached value of total count of projects This is in preparation to address the DB load caused by the counting in gitlab-com/infrastructure#303. --- 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 1c3d694075a..9c3b4712cab 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -714,6 +714,20 @@ describe Project, models: true do it { expect(project.builds_enabled?).to be_truthy } end + describe '.cached_count', caching: true do + let(:group) { create(:group, :public) } + let!(:project1) { create(:empty_project, :public, group: group) } + let!(:project2) { create(:empty_project, :public, group: group) } + + it 'returns total project count' do + expect(Project).to receive(:count).once.and_call_original + + 3.times do + expect(Project.cached_count).to eq(2) + end + end + end + describe '.trending' do let(:group) { create(:group, :public) } let(:project1) { create(:empty_project, :public, group: group) } -- cgit v1.2.1 From 4955a47cb1c52168114364e45a2fccf6bc105452 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 6 Aug 2016 07:25:51 -0700 Subject: Clean up project destruction Instead of redirecting from the project service to the service and back to the model, put all destruction code in the service. Also removes a possible source of failure where run_after_commit may not destroy the project. --- spec/models/hooks/system_hook_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 4078b9e4ff5..cbdf7eec082 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -38,7 +38,7 @@ describe SystemHook, models: true do end it "project_destroy hook" do - Projects::DestroyService.new(project, user, {}).pending_delete! + Projects::DestroyService.new(project, user, {}).async_execute expect(WebMock).to have_requested(:post, system_hook.url).with( body: /project_destroy/, -- cgit v1.2.1 From 29850364eccccc3ce7305f6706cea1d5d073de2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 23 Jun 2016 17:14:31 +0200 Subject: New AccessRequests API endpoints for Group & Project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also, mutualize AccessRequests and Members endpoints for Group & Project. New API documentation for the AccessRequests endpoints. Signed-off-by: Rémy Coutable --- spec/models/member_spec.rb | 2 +- spec/models/members/project_member_spec.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 44cd3c08718..2277f4e13bf 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -10,7 +10,7 @@ describe Member, models: true do it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:source) } - it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } + it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.all_values) } it_behaves_like 'an object with email-formated attributes', :invite_email do subject { build(:project_member) } diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 28673de3189..913d74645a7 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -27,6 +27,7 @@ describe ProjectMember, models: true do describe 'validations' do it { is_expected.to allow_value('Project').for(:source_type) } it { is_expected.not_to allow_value('project').for(:source_type) } + it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } end describe 'modules' do @@ -40,7 +41,7 @@ describe ProjectMember, models: true do end describe "#destroy" do - let(:owner) { create(:project_member, access_level: ProjectMember::OWNER) } + let(:owner) { create(:project_member, access_level: ProjectMember::MASTER) } let(:project) { owner.project } let(:master) { create(:project_member, project: project) } -- cgit v1.2.1 From fb748daf538e43efcf8884f017391bcbfccf2ea2 Mon Sep 17 00:00:00 2001 From: Thomas Balthazar Date: Wed, 10 Aug 2016 12:25:01 +0200 Subject: Replace the tinder gem by bare HTTP requests --- .../project_services/campfire_service_spec.rb | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb index 3e6da42803b..1adf93258f3 100644 --- a/spec/models/project_services/campfire_service_spec.rb +++ b/spec/models/project_services/campfire_service_spec.rb @@ -39,4 +39,62 @@ describe CampfireService, models: true do it { is_expected.not_to validate_presence_of(:token) } end end + + describe "#execute" do + let(:user) { create(:user) } + let(:project) { create(:project) } + + before do + @campfire_service = CampfireService.new + allow(@campfire_service).to receive_messages( + project_id: project.id, + project: project, + service_hook: true, + token: 'verySecret', + subdomain: 'project-name', + room: 'test-room' + ) + @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) + @rooms_url = 'https://verySecret:X@project-name.campfirenow.com/rooms.json' + @headers = { 'Content-Type' => 'application/json; charset=utf-8' } + end + + it "calls Campfire API to get a list of rooms and speak in a room" do + # make sure a valid list of rooms is returned + body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms.json') + WebMock.stub_request(:get, @rooms_url).to_return( + body: body, + status: 200, + headers: @headers + ) + # stub the speak request with the room id found in the previous request's response + speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/123/speak.json' + WebMock.stub_request(:post, speak_url) + + @campfire_service.execute(@sample_data) + + expect(WebMock).to have_requested(:get, @rooms_url).once + expect(WebMock).to have_requested(:post, speak_url).with( + body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/ + ).once + end + + it "calls Campfire API to get a list of rooms but shouldn't speak in a room" do + # return a list of rooms that do not contain a room named 'test-room' + body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms2.json') + WebMock.stub_request(:get, @rooms_url).to_return( + body: body, + status: 200, + headers: @headers + ) + # we want to make sure no request is sent to the /speak endpoint, here is a basic + # regexp that matches this endpoint + speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/.*/speak.json' + + @campfire_service.execute(@sample_data) + + expect(WebMock).to have_requested(:get, @rooms_url).once + expect(WebMock).not_to have_requested(:post, /#{speak_url}/) + end + end end -- cgit v1.2.1 From 39203f1adfc6fee3eca50f0cab99ffc597865200 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 11 Aug 2016 15:22:35 +0200 Subject: Pre-create all builds for Pipeline when a trigger is received This change simplifies a Pipeline processing by introducing a special new status: created. This status is used for all builds that are created for a pipeline. We are then processing next stages and queueing some of the builds (created -> pending) or skipping them (created -> skipped). This makes it possible to simplify and solve a few ordering problems with how previously builds were scheduled. This also allows us to visualise a full pipeline (with created builds). This also removes an after_touch used for updating a pipeline state parameters. Right now in various places we explicitly call a reload_status! on pipeline to force it to be updated and saved. --- spec/models/build_spec.rb | 47 +++ spec/models/ci/pipeline_spec.rb | 316 +-------------------- .../project_services/hipchat_service_spec.rb | 3 +- 3 files changed, 63 insertions(+), 303 deletions(-) (limited to 'spec/models') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 9ecc9aac84b..60a221eba50 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -764,6 +764,53 @@ describe Ci::Build, models: true do end end + describe '#when' do + subject { build.when } + + context 'if is undefined' do + before do + build.when = nil + end + + context 'use from gitlab-ci.yml' do + before do + stub_ci_pipeline_yaml_file(config) + end + + context 'if config is not found' do + let(:config) { nil } + + it { is_expected.to eq('on_success') } + end + + context 'if config does not have a questioned job' do + let(:config) do + YAML.dump({ + test_other: { + script: 'Hello World' + } + }) + end + + it { is_expected.to eq('on_success') } + end + + context 'if config has when' do + let(:config) do + YAML.dump({ + test: { + script: 'Hello World', + when: 'always' + } + }) + end + + it { is_expected.to eq('always') } + end + end + end + end + describe '#retryable?' do context 'when build is running' do before do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index ccee591cf7a..fdb579ab45c 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -38,9 +38,6 @@ describe Ci::Pipeline, models: true do it { expect(pipeline.sha).to start_with(subject) } end - describe '#create_next_builds' do - end - describe '#retried' do subject { pipeline.retried } @@ -54,304 +51,20 @@ describe Ci::Pipeline, models: true do end end - describe '#create_builds' do - let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project, ref: 'master', tag: false } - - def create_builds(trigger_request = nil) - pipeline.create_builds(nil, trigger_request) - end - - def create_next_builds - pipeline.create_next_builds(pipeline.builds.order(:id).last) - end - - it 'creates builds' do - expect(create_builds).to be_truthy - pipeline.builds.update_all(status: "success") - expect(pipeline.builds.count(:all)).to eq(2) - - expect(create_next_builds).to be_truthy - pipeline.builds.update_all(status: "success") - expect(pipeline.builds.count(:all)).to eq(4) - - expect(create_next_builds).to be_truthy - pipeline.builds.update_all(status: "success") - expect(pipeline.builds.count(:all)).to eq(5) - - expect(create_next_builds).to be_falsey - end - - context 'custom stage with first job allowed to fail' do - let(:yaml) do - { - stages: ['clean', 'test'], - clean_job: { - stage: 'clean', - allow_failure: true, - script: 'BUILD', - }, - test_job: { - stage: 'test', - script: 'TEST', - }, - } - end - - before do - stub_ci_pipeline_yaml_file(YAML.dump(yaml)) - create_builds - end - - it 'properly schedules builds' do - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:drop) - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending', 'failed') - end - end - - context 'properly creates builds when "when" is defined' do - let(:yaml) do - { - stages: ["build", "test", "test_failure", "deploy", "cleanup"], - build: { - stage: "build", - script: "BUILD", - }, - test: { - stage: "test", - script: "TEST", - }, - test_failure: { - stage: "test_failure", - script: "ON test failure", - when: "on_failure", - }, - deploy: { - stage: "deploy", - script: "PUBLISH", - }, - cleanup: { - stage: "cleanup", - script: "TIDY UP", - when: "always", - } - } - end - - before do - stub_ci_pipeline_yaml_file(YAML.dump(yaml)) - end - - context 'when builds are successful' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success') - pipeline.reload - expect(pipeline.status).to eq('success') - end - end - - context 'when test job fails' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success') - pipeline.reload - expect(pipeline.status).to eq('failed') - end - end - - context 'when test and test_failure jobs fail' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success') - pipeline.reload - expect(pipeline.status).to eq('failed') - end - end - - context 'when deploy job fails' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success') - pipeline.reload - expect(pipeline.status).to eq('failed') - end - end - - context 'when build is canceled in the second stage' do - it 'does not schedule builds after build has been canceled' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.running_or_pending).not_to be_empty - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:cancel) - - expect(pipeline.builds.running_or_pending).to be_empty - expect(pipeline.reload.status).to eq('canceled') - end - end - - context 'when listing manual actions' do - let(:yaml) do - { - stages: ["build", "test", "staging", "production", "cleanup"], - build: { - stage: "build", - script: "BUILD", - }, - test: { - stage: "test", - script: "TEST", - }, - staging: { - stage: "staging", - script: "PUBLISH", - }, - production: { - stage: "production", - script: "PUBLISH", - when: "manual", - }, - cleanup: { - stage: "cleanup", - script: "TIDY UP", - when: "always", - }, - clear_cache: { - stage: "cleanup", - script: "CLEAR CACHE", - when: "manual", - } - } - end - - it 'returns only for skipped builds' do - # currently all builds are created - expect(create_builds).to be_truthy - expect(manual_actions).to be_empty - - # succeed stage build - pipeline.builds.running_or_pending.each(&:success) - expect(manual_actions).to be_empty - - # succeed stage test - pipeline.builds.running_or_pending.each(&:success) - expect(manual_actions).to be_empty - - # succeed stage staging and skip stage production - pipeline.builds.running_or_pending.each(&:success) - expect(manual_actions).to be_many # production and clear cache - - # succeed stage cleanup - pipeline.builds.running_or_pending.each(&:success) - - # after processing a pipeline we should have 6 builds, 5 succeeded - expect(pipeline.builds.count).to eq(6) - expect(pipeline.builds.success.count).to eq(4) - end - - def manual_actions - pipeline.manual_actions - end - end - end - - context 'when no builds created' do - let(:pipeline) { build(:ci_pipeline) } - - before do - stub_ci_pipeline_yaml_file(YAML.dump(before_script: ['ls'])) - end - - it 'returns false' do - expect(pipeline.create_builds(nil)).to be_falsey - expect(pipeline).not_to be_persisted - end - end - end - describe "#finished_at" do let(:pipeline) { FactoryGirl.create :ci_pipeline } it "returns finished_at of latest build" do build = FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 60 FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 120 + pipeline.reload_status! expect(pipeline.finished_at.to_i).to eq(build.finished_at.to_i) end it "returns nil if there is no finished build" do FactoryGirl.create :ci_not_started_build, pipeline: pipeline + pipeline.reload_status! expect(pipeline.finished_at).to be_nil end @@ -359,7 +72,7 @@ describe Ci::Pipeline, models: true do describe "coverage" do let(:project) { FactoryGirl.create :empty_project, build_coverage_regex: "/.*/" } - let(:pipeline) { FactoryGirl.create :ci_pipeline, project: project } + let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project } it "calculates average when there are two builds with coverage" do FactoryGirl.create :ci_build, name: "rspec", coverage: 30, pipeline: pipeline @@ -426,31 +139,30 @@ describe Ci::Pipeline, models: true do end end - describe '#update_state' do - it 'executes update_state after touching object' do - expect(pipeline).to receive(:update_state).and_return(true) - pipeline.touch - end + describe '#reload_status!' do + let(:pipeline) { create :ci_empty_pipeline, project: project } context 'dependent objects' do - let(:commit_status) { build :commit_status, pipeline: pipeline } + let(:commit_status) { create :commit_status, :pending, pipeline: pipeline } + + it 'executes reload_status! after succeeding dependent object' do + expect(pipeline).to receive(:reload_status!).and_return(true) - it 'executes update_state after saving dependent object' do - expect(pipeline).to receive(:update_state).and_return(true) - commit_status.save + commit_status.success end end - context 'update state' do + context 'updates' do let(:current) { Time.now.change(usec: 0) } - let(:build) { FactoryGirl.create :ci_build, :success, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 } + let(:build) { FactoryGirl.create :ci_build, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 } before do build + pipeline.reload_status! end [:status, :started_at, :finished_at, :duration].each do |param| - it "update #{param}" do + it "#{param}" do expect(pipeline.send(param)).to eq(build.send(param)) end end diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index bf438b26690..1b383219eb9 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -291,7 +291,8 @@ describe HipchatService, models: true do end context 'build events' do - let(:build) { create(:ci_build) } + let(:pipeline) { create(:ci_empty_pipeline) } + let(:build) { create(:ci_build, pipeline: pipeline) } let(:data) { Gitlab::BuildDataBuilder.build(build) } context 'for failed' do -- cgit v1.2.1 From d983c5bd4671d989edf3741d0db0a54dcef9c3b6 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 11 Aug 2016 18:37:36 +0200 Subject: Verify the pipeline status after executing events on builds --- spec/models/ci/pipeline_spec.rb | 45 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index fdb579ab45c..9a9720cfbfc 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -254,4 +254,49 @@ describe Ci::Pipeline, models: true do end end end + + describe '#status' do + let!(:build) { create(:ci_build, :created, pipeline: pipeline, name: 'test') } + + subject { pipeline.reload.status } + + context 'on queuing' do + before { build.queue } + + it { is_expected.to eq('pending') } + end + + context 'on run' do + before do + build.queue + build.run + end + + it { is_expected.to eq('running') } + end + + context 'on drop' do + before do + build.drop + end + + it { is_expected.to eq('failed') } + end + + context 'on success' do + before do + build.success + end + + it { is_expected.to eq('success') } + end + + context 'on cancel' do + before do + build.cancel + end + + it { is_expected.to eq('canceled') } + end + end end -- cgit v1.2.1 From b497b0ce3fc3c1882639f9c7d55f7991ce41f15d Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Wed, 3 Aug 2016 13:37:39 +0200 Subject: Incorporate feedback --- spec/models/deployment_spec.rb | 20 ++++++++++++++++++++ spec/models/environment_spec.rb | 10 ++++++++++ spec/models/merge_request_spec.rb | 15 +++++++++++++++ 3 files changed, 45 insertions(+) (limited to 'spec/models') diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 7df3df4bb9e..107f8b38acf 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -15,4 +15,24 @@ describe Deployment, models: true do it { is_expected.to validate_presence_of(:ref) } it { is_expected.to validate_presence_of(:sha) } + + describe '#deployed_to?' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + let(:deployment) do + create(:deployment, environment: environment, sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') + end + + context 'when there is no project commit' do + it 'returns false' do + expect(deployment.deployed_to?('random-branch')).to be false + end + end + + context 'when they share the same tree branch' do + it 'returns true' do + expect(deployment.deployed_to?('HEAD')).to be true + end + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 8a84ac0a7c7..e65b4f82eff 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -30,4 +30,14 @@ describe Environment, models: true do expect(env.external_url).to be_nil end end + + describe '#deployed_from?' do + let(:environment) { create(:environment) } + + context 'without a last deployment' do + it "returns false" do + expect(environment.deployed_from?('HEAD')).to be false + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 3270b877c1a..0727dd29951 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -674,6 +674,21 @@ describe MergeRequest, models: true do end end + describe "#environments" do + let(:project) { create(:project) } + + let!(:deployment) { create(:deployment, environment: environment, sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') } + + let!(:environment) { create(:environment, project: project) } + let!(:environment1) { create(:environment, project: project) } + + let(:merge_request) { create(:merge_request, source_project: project) } + + it 'selects deployed environments' do + expect(merge_request.environments).to eq [environment] + end + end + describe "#reload_diff" do let(:note) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject) } -- cgit v1.2.1 From 03ea01946524a74773b24430c81804c2724b84b6 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Thu, 4 Aug 2016 11:29:41 +0200 Subject: CI build status not per environment --- spec/models/deployment_spec.rb | 3 ++- spec/models/merge_request_spec.rb | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 107f8b38acf..d7cb142dd32 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -20,7 +20,8 @@ describe Deployment, models: true do let(:project) { create(:project) } let(:environment) { create(:environment, project: project) } let(:deployment) do - create(:deployment, environment: environment, sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') + create(:deployment, environment: environment, + sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') end context 'when there is no project commit' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 0727dd29951..e605720a2dd 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -677,7 +677,10 @@ describe MergeRequest, models: true do describe "#environments" do let(:project) { create(:project) } - let!(:deployment) { create(:deployment, environment: environment, sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') } + let!(:deployment) do + create(:deployment, environment: environment, + sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') + end let!(:environment) { create(:environment, project: project) } let!(:environment1) { create(:environment, project: project) } -- cgit v1.2.1 From 6a6a69f4afbe0107a75df018b662f02b5ec0166a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 11 Aug 2016 20:54:02 +0200 Subject: Use state machine for pipeline event processing --- spec/models/ci/pipeline_spec.rb | 38 ++++---------------------------------- 1 file changed, 4 insertions(+), 34 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 9a9720cfbfc..eb762276cbe 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Ci::Pipeline, models: true do let(:project) { FactoryGirl.create :empty_project } - let(:pipeline) { FactoryGirl.create :ci_pipeline, project: project } + let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project } it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:user) } @@ -51,25 +51,6 @@ describe Ci::Pipeline, models: true do end end - describe "#finished_at" do - let(:pipeline) { FactoryGirl.create :ci_pipeline } - - it "returns finished_at of latest build" do - build = FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 60 - FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 120 - pipeline.reload_status! - - expect(pipeline.finished_at.to_i).to eq(build.finished_at.to_i) - end - - it "returns nil if there is no finished build" do - FactoryGirl.create :ci_not_started_build, pipeline: pipeline - pipeline.reload_status! - - expect(pipeline.finished_at).to be_nil - end - end - describe "coverage" do let(:project) { FactoryGirl.create :empty_project, build_coverage_regex: "/.*/" } let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project } @@ -139,31 +120,20 @@ describe Ci::Pipeline, models: true do end end - describe '#reload_status!' do + describe '#update_counters' do let(:pipeline) { create :ci_empty_pipeline, project: project } - context 'dependent objects' do - let(:commit_status) { create :commit_status, :pending, pipeline: pipeline } - - it 'executes reload_status! after succeeding dependent object' do - expect(pipeline).to receive(:reload_status!).and_return(true) - - commit_status.success - end - end - context 'updates' do let(:current) { Time.now.change(usec: 0) } let(:build) { FactoryGirl.create :ci_build, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 } before do - build - pipeline.reload_status! + build.skip end [:status, :started_at, :finished_at, :duration].each do |param| it "#{param}" do - expect(pipeline.send(param)).to eq(build.send(param)) + expect(pipeline.reload.send(param)).to eq(build.send(param)) end end end -- cgit v1.2.1 From 4ccf39cde7356bf98bef5aae694257fb2c001e75 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 11 Aug 2016 22:54:25 +0200 Subject: Fix test failures, that did occur because of missing previously used `reload_status!` call --- spec/models/ci/pipeline_spec.rb | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index eb762276cbe..adfe4bdd0c8 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -120,22 +120,18 @@ describe Ci::Pipeline, models: true do end end - describe '#update_counters' do - let(:pipeline) { create :ci_empty_pipeline, project: project } + describe '#duration' do + let(:current) { Time.now.change(usec: 0) } + let!(:build) { create :ci_build, name: 'build1', pipeline: pipeline, started_at: current - 60, finished_at: current } + let!(:build2) { create :ci_build, name: 'build2', pipeline: pipeline, started_at: current - 60, finished_at: current } - context 'updates' do - let(:current) { Time.now.change(usec: 0) } - let(:build) { FactoryGirl.create :ci_build, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 } - - before do - build.skip - end + before do + build.skip + build2.skip + end - [:status, :started_at, :finished_at, :duration].each do |param| - it "#{param}" do - expect(pipeline.reload.send(param)).to eq(build.send(param)) - end - end + it 'matches sum of builds duration' do + expect(pipeline.reload.duration).to eq(build.duration + build2.duration) end end -- cgit v1.2.1 From 07fc2f852a0b4136b6d97c1d9773819c47e7e8e7 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Tue, 9 Aug 2016 15:11:14 +0200 Subject: Method names changed to #includes_commit? --- spec/models/deployment_spec.rb | 13 ++++++++----- spec/models/environment_spec.rb | 31 +++++++++++++++++++++++++++---- spec/models/merge_request_spec.rb | 11 ++++------- 3 files changed, 39 insertions(+), 16 deletions(-) (limited to 'spec/models') diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index d7cb142dd32..bfff639ad78 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -16,23 +16,26 @@ describe Deployment, models: true do it { is_expected.to validate_presence_of(:ref) } it { is_expected.to validate_presence_of(:sha) } - describe '#deployed_to?' do + describe '#includes_commit?' do let(:project) { create(:project) } let(:environment) { create(:environment, project: project) } let(:deployment) do - create(:deployment, environment: environment, - sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') + create(:deployment, environment: environment, sha: project.commit.id) end context 'when there is no project commit' do it 'returns false' do - expect(deployment.deployed_to?('random-branch')).to be false + commit = project.commit('feature') + + expect(deployment.includes_commit?(commit)).to be false end end context 'when they share the same tree branch' do it 'returns true' do - expect(deployment.deployed_to?('HEAD')).to be true + commit = project.commit + + expect(deployment.includes_commit?(commit)).to be true end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index e65b4f82eff..c881897926e 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -31,12 +31,35 @@ describe Environment, models: true do end end - describe '#deployed_from?' do - let(:environment) { create(:environment) } - + describe '#includes_commit?' do context 'without a last deployment' do it "returns false" do - expect(environment.deployed_from?('HEAD')).to be false + expect(environment.includes_commit?('HEAD')).to be false + end + end + + context 'with a last deployment' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + + let!(:deployment) do + create(:deployment, environment: environment, sha: project.commit('master').id) + end + + context 'in the same branch' do + it 'returns true' do + expect(environment.includes_commit?(RepoHelpers.sample_commit)).to be true + end + end + + context 'not in the same branch' do + before do + deployment.update(sha: project.commit('feature').id) + end + + it 'returns false' do + expect(environment.includes_commit?(RepoHelpers.sample_commit)).to be false + end end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index e605720a2dd..35a4418ebb3 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -676,18 +676,15 @@ describe MergeRequest, models: true do describe "#environments" do let(:project) { create(:project) } - - let!(:deployment) do - create(:deployment, environment: environment, - sha: '5f923865dde3436854e9ceb9cdb7815618d4e849') - end - let!(:environment) { create(:environment, project: project) } let!(:environment1) { create(:environment, project: project) } - + let!(:environment2) { create(:environment, project: project) } let(:merge_request) { create(:merge_request, source_project: project) } it 'selects deployed environments' do + create(:deployment, environment: environment, sha: project.commit('master').id) + create(:deployment, environment: environment1, sha: project.commit('feature').id) + expect(merge_request.environments).to eq [environment] end end -- cgit v1.2.1 From ad3e1edcfce1e24fb9889d5d73852680cf4facf9 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 11:53:27 +0200 Subject: Added specs for started_at and finished_at --- spec/models/ci/pipeline_spec.rb | 46 ++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index adfe4bdd0c8..28d07f67b26 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -120,18 +120,48 @@ describe Ci::Pipeline, models: true do end end - describe '#duration' do + describe 'state machine' do let(:current) { Time.now.change(usec: 0) } - let!(:build) { create :ci_build, name: 'build1', pipeline: pipeline, started_at: current - 60, finished_at: current } - let!(:build2) { create :ci_build, name: 'build2', pipeline: pipeline, started_at: current - 60, finished_at: current } + let(:build) { create :ci_build, name: 'build1', pipeline: pipeline, started_at: current - 60, finished_at: current } + let(:build2) { create :ci_build, name: 'build2', pipeline: pipeline, started_at: current - 60, finished_at: current } - before do - build.skip - build2.skip + describe '#duration' do + before do + build.skip + build2.skip + end + + it 'matches sum of builds duration' do + expect(pipeline.reload.duration).to eq(build.duration + build2.duration) + end end - it 'matches sum of builds duration' do - expect(pipeline.reload.duration).to eq(build.duration + build2.duration) + describe '#started_at' do + it 'updates on transitioning to running' do + build.run + + expect(pipeline.reload.started_at).not_to be_nil + end + + it 'do not update on transitioning to success' do + build.success + + expect(pipeline.reload.started_at).to be_nil + end + end + + describe '#finished_at' do + it 'updates on transitioning to success' do + build.success + + expect(pipeline.reload.finished_at).not_to be_nil + end + + it 'do not update on transitioning to running' do + build.run + + expect(pipeline.reload.finished_at).to be_nil + end end end -- cgit v1.2.1 From ea4ac578534d3a233c3525bf8351eb2045f6e632 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 13:57:58 +0200 Subject: Use event `enqueue` instead of `queue` --- spec/models/build_spec.rb | 6 ++++-- spec/models/ci/pipeline_spec.rb | 23 +++++++++++++++++++---- 2 files changed, 23 insertions(+), 6 deletions(-) (limited to 'spec/models') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 60a221eba50..04fb074cfd8 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -886,8 +886,10 @@ describe Ci::Build, models: true do is_expected.to eq(build) end - context 'for success build' do - before { build.queue } + context 'for successful build' do + before do + build.success + end it 'creates a new build' do is_expected.to be_pending diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 28d07f67b26..950833cb219 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -143,7 +143,7 @@ describe Ci::Pipeline, models: true do expect(pipeline.reload.started_at).not_to be_nil end - it 'do not update on transitioning to success' do + it 'does not update on transitioning to success' do build.success expect(pipeline.reload.started_at).to be_nil @@ -157,7 +157,7 @@ describe Ci::Pipeline, models: true do expect(pipeline.reload.finished_at).not_to be_nil end - it 'do not update on transitioning to running' do + it 'does not update on transitioning to running' do build.run expect(pipeline.reload.finished_at).to be_nil @@ -257,14 +257,16 @@ describe Ci::Pipeline, models: true do subject { pipeline.reload.status } context 'on queuing' do - before { build.queue } + before do + build.enqueue + end it { is_expected.to eq('pending') } end context 'on run' do before do - build.queue + build.enqueue build.run end @@ -294,5 +296,18 @@ describe Ci::Pipeline, models: true do it { is_expected.to eq('canceled') } end + + context 'on failure and build retry' do + before do + build.drop + Ci::Build.retry(build) + end + + # We are changing a state: created > failed > running + # Instead of: created > failed > pending + # Since the pipeline already run, so it should not be pending anymore + + it { is_expected.to eq('running') } + end end end -- cgit v1.2.1 From 7cfc47432170be14f9449a77f893c4662634019d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 15:09:35 +0200 Subject: Fix build play failure --- spec/models/build_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 04fb074cfd8..5980f6ddc32 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -888,7 +888,7 @@ describe Ci::Build, models: true do context 'for successful build' do before do - build.success + build.update(status: 'success') end it 'creates a new build' do -- cgit v1.2.1 From 4e4ca27ab148f16d9252634e3e2f58447acdeeef Mon Sep 17 00:00:00 2001 From: Egor Lynko Date: Wed, 10 Aug 2016 17:15:27 +0300 Subject: Ability to specify branches for pivotal tracker integration --- .../pivotaltracker_service_spec.rb | 71 ++++++++++++++++++++++ 1 file changed, 71 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_services/pivotaltracker_service_spec.rb b/spec/models/project_services/pivotaltracker_service_spec.rb index f37edd4d970..d098d988521 100644 --- a/spec/models/project_services/pivotaltracker_service_spec.rb +++ b/spec/models/project_services/pivotaltracker_service_spec.rb @@ -39,4 +39,75 @@ describe PivotaltrackerService, models: true do it { is_expected.not_to validate_presence_of(:token) } end end + + describe 'Execute' do + let(:service) do + PivotaltrackerService.new.tap do |service| + service.token = 'secret_api_token' + end + end + + let(:url) { PivotaltrackerService::API_ENDPOINT } + + def push_data(branch: 'master') + { + object_kind: 'push', + ref: "refs/heads/#{branch}", + commits: [ + { + id: '21c12ea', + author: { + name: 'Some User' + }, + url: 'https://example.com/commit', + message: 'commit message', + } + ] + } + end + + before do + WebMock.stub_request(:post, url) + end + + it 'should post correct message' do + service.execute(push_data) + expect(WebMock).to have_requested(:post, url).with( + body: { + 'source_commit' => { + 'commit_id' => '21c12ea', + 'author' => 'Some User', + 'url' => 'https://example.com/commit', + 'message' => 'commit message' + } + }, + headers: { + 'Content-Type' => 'application/json', + 'X-TrackerToken' => 'secret_api_token' + } + ).once + end + + context 'when allowed branches is specified' do + let(:service) do + super().tap do |service| + service.restrict_to_branch = 'master,v10' + end + end + + it 'should post message if branch is in the list' do + service.execute(push_data(branch: 'master')) + service.execute(push_data(branch: 'v10')) + + expect(WebMock).to have_requested(:post, url).twice + end + + it 'should not post message if branch is not in the list' do + service.execute(push_data(branch: 'mas')) + service.execute(push_data(branch: 'v11')) + + expect(WebMock).not_to have_requested(:post, url) + end + end + end end -- cgit v1.2.1 From 8171544b3d44df6ce810aa436bf87d137bc9b28f Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 12 Aug 2016 17:19:17 +0200 Subject: Limit the size of SVGs when viewing them as blobs This ensures that SVGs greater than 2 megabytes are not scrubbed and rendered. This in turn prevents requests from timing out due to reading/scrubbing large SVGs potentially taking a lot of time (and memory). The use of 2 megabytes is completely arbitrary. Fixes gitlab-org/gitlab-ce#1435 --- spec/models/blob_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'spec/models') diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 1e5d6a34f83..cee20234e1f 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -94,4 +94,26 @@ describe Blob do expect(blob.to_partial_path).to eq 'download' end end + + describe '#size_within_svg_limits?' do + let(:blob) { described_class.decorate(double(:blob)) } + + it 'returns true when the blob size is smaller than the SVG limit' do + expect(blob).to receive(:size).and_return(42) + + expect(blob.size_within_svg_limits?).to eq(true) + end + + it 'returns true when the blob size is equal to the SVG limit' do + expect(blob).to receive(:size).and_return(Blob::MAXIMUM_SVG_SIZE) + + expect(blob.size_within_svg_limits?).to eq(true) + end + + it 'returns false when the blob size is larger than the SVG limit' do + expect(blob).to receive(:size).and_return(1.terabyte) + + expect(blob.size_within_svg_limits?).to eq(false) + end + end end -- cgit v1.2.1 From 95419679f23f0628d1885dd9656cc159e9d55ea9 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Wed, 27 Jul 2016 19:03:06 -0500 Subject: Lay the ground works to submit information to Akismet - New concern `AkismetSubmittable` to allow issues and other `Spammable` models to be submitted to Akismet. - New model `UserAgentDetail` to store information needed for Akismet. - Services needed for their creation and tests. --- spec/models/user_agent_detail_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 spec/models/user_agent_detail_spec.rb (limited to 'spec/models') diff --git a/spec/models/user_agent_detail_spec.rb b/spec/models/user_agent_detail_spec.rb new file mode 100644 index 00000000000..8debcbbeba6 --- /dev/null +++ b/spec/models/user_agent_detail_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +describe UserAgentDetail, type: :model do + describe '.submittable?' do + it 'should be submittable' do + detail = create(:user_agent_detail, :on_issue) + expect(detail.submittable?).to be_truthy + end + end + + describe '.valid?' do + it 'should be valid with a subject' do + detail = create(:user_agent_detail, :on_issue) + expect(detail).to be_valid + end + + it 'should not be valid without a subject' do + detail = build(:user_agent_detail) + expect(detail).not_to be_valid + end + end +end -- cgit v1.2.1 From 722fc84e3d4785fb3a9db5f1c7d2aabad22e8e01 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Thu, 28 Jul 2016 19:02:56 -0500 Subject: Complete refactor of the `Spammable` concern and tests: - Merged `AkismetSubmittable` into `Spammable` - Clean up `SpamCheckService` - Added tests for `Spammable` - Added submit (ham or spam) options to `AkismetHelper` --- spec/models/concerns/spammable_spec.rb | 41 ++++++++++++++++++++++++++++++++++ spec/models/user_agent_detail_spec.rb | 5 ----- 2 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 spec/models/concerns/spammable_spec.rb (limited to 'spec/models') diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb new file mode 100644 index 00000000000..e61a6dcb69d --- /dev/null +++ b/spec/models/concerns/spammable_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe Issue, 'Spammable' do + let(:issue) { create(:issue, description: 'Test Desc.') } + + describe 'Associations' do + it { is_expected.to have_one(:user_agent_detail).dependent(:destroy) } + end + + describe 'ClassMethods' do + it 'should return correct attr_spammable' do + expect(issue.send(:spammable_text)).to eq("#{issue.title}\n#{issue.description}") + end + end + + describe 'InstanceMethods' do + it 'should return the correct creator' do + expect(issue.send(:creator).id).to eq(issue.author_id) + end + + it 'should be invalid if spam' do + issue.spam = true + expect(issue.valid?).to be_truthy + end + + it 'should be submittable' do + create(:user_agent_detail, subject_id: issue.id, subject_type: issue.class.to_s) + expect(issue.can_be_submitted?).to be_truthy + end + end + + describe 'AkismetMethods' do + before do + allow_any_instance_of(Gitlab::AkismetHelper).to receive_messages(check_for_spam?: true, is_spam?: true, ham!: nil, spam!: nil) + end + + it { expect(issue.spam?(:mock_env, :mock_user)).to be_truthy } + it { expect(issue.submit_spam).to be_nil } + it { expect(issue.submit_ham).to be_nil } + end +end diff --git a/spec/models/user_agent_detail_spec.rb b/spec/models/user_agent_detail_spec.rb index 8debcbbeba6..ba21161fc7f 100644 --- a/spec/models/user_agent_detail_spec.rb +++ b/spec/models/user_agent_detail_spec.rb @@ -13,10 +13,5 @@ describe UserAgentDetail, type: :model do detail = create(:user_agent_detail, :on_issue) expect(detail).to be_valid end - - it 'should not be valid without a subject' do - detail = build(:user_agent_detail) - expect(detail).not_to be_valid - end end end -- cgit v1.2.1 From 64ab2b3d9f10366249c03a6bcf5e8b1d20010d8f Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Fri, 29 Jul 2016 23:18:32 -0500 Subject: Refactored spam related code even further - Removed unnecessary column from `SpamLog` - Moved creation of SpamLogs out of its own service and into SpamCheckService - Simplified code in SpamCheckService. - Moved move spam related code into Spammable concern --- spec/models/concerns/spammable_spec.rb | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index e61a6dcb69d..3f7d2721d22 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -15,7 +15,7 @@ describe Issue, 'Spammable' do describe 'InstanceMethods' do it 'should return the correct creator' do - expect(issue.send(:creator).id).to eq(issue.author_id) + expect(issue.send(:owner).id).to eq(issue.author_id) end it 'should be invalid if spam' do @@ -27,15 +27,28 @@ describe Issue, 'Spammable' do create(:user_agent_detail, subject_id: issue.id, subject_type: issue.class.to_s) expect(issue.can_be_submitted?).to be_truthy end + + describe '#check_for_spam?' do + before do + allow_any_instance_of(Gitlab::AkismetHelper).to receive(:akismet_enabled?).and_return(true) + end + it 'returns true for public project' do + issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) + expect(issue.check_for_spam?).to eq(true) + end + + it 'returns false for other visibility levels' do + expect(issue.check_for_spam?).to eq(false) + end + end end describe 'AkismetMethods' do before do - allow_any_instance_of(Gitlab::AkismetHelper).to receive_messages(check_for_spam?: true, is_spam?: true, ham!: nil, spam!: nil) + allow_any_instance_of(Gitlab::AkismetHelper).to receive_messages(is_spam?: true, spam!: nil) end - it { expect(issue.spam?(:mock_env, :mock_user)).to be_truthy } + it { expect(issue.spam_detected?(:mock_env)).to be_truthy } it { expect(issue.submit_spam).to be_nil } - it { expect(issue.submit_ham).to be_nil } end end -- cgit v1.2.1 From abf2dcd25c4a176801314872733ede91297d1ab0 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Mon, 1 Aug 2016 12:14:03 -0500 Subject: Allow `SpamLog` to be submitted as ham - Added `submitted_as_ham` to `SpamLog` to mark which logs have been submitted to Akismet. - Added routes and controller action. --- spec/models/concerns/spammable_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index 3f7d2721d22..7492c42f71e 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -45,10 +45,11 @@ describe Issue, 'Spammable' do describe 'AkismetMethods' do before do - allow_any_instance_of(Gitlab::AkismetHelper).to receive_messages(is_spam?: true, spam!: nil) + allow_any_instance_of(Gitlab::AkismetHelper).to receive_messages(is_spam?: true, spam!: true, akismet_enabled?: true) + allow_any_instance_of(Spammable).to receive(:can_be_submitted?).and_return(true) end it { expect(issue.spam_detected?(:mock_env)).to be_truthy } - it { expect(issue.submit_spam).to be_nil } + it { expect(issue.submit_spam).to be_truthy } end end -- cgit v1.2.1 From 96399a81cbb2e8a0f666241eeaff7cc784c26983 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Tue, 2 Aug 2016 16:21:57 -0500 Subject: Allow `Issue` to be submitted as spam - Added controller actions as reusable concerns - Added controller tests --- spec/models/concerns/spammable_spec.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index 7492c42f71e..4e52d05918f 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -14,6 +14,10 @@ describe Issue, 'Spammable' do end describe 'InstanceMethods' do + before do + allow_any_instance_of(Gitlab::AkismetHelper).to receive(:akismet_enabled?).and_return(true) + end + it 'should return the correct creator' do expect(issue.send(:owner).id).to eq(issue.author_id) end @@ -24,14 +28,11 @@ describe Issue, 'Spammable' do end it 'should be submittable' do - create(:user_agent_detail, subject_id: issue.id, subject_type: issue.class.to_s) + create(:user_agent_detail, subject: issue) expect(issue.can_be_submitted?).to be_truthy end describe '#check_for_spam?' do - before do - allow_any_instance_of(Gitlab::AkismetHelper).to receive(:akismet_enabled?).and_return(true) - end it 'returns true for public project' do issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) expect(issue.check_for_spam?).to eq(true) -- cgit v1.2.1 From 43e756d4eafd79f4d2f366b646ebb94af78b5a4c Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Fri, 5 Aug 2016 17:10:08 -0500 Subject: Refactored AkismetHelper into AkismetService and cleaned up `Spammable` - Refactored SpamCheckService into SpamService --- spec/models/concerns/spammable_spec.rb | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) (limited to 'spec/models') diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index 4e52d05918f..7944305e7b3 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -14,25 +14,24 @@ describe Issue, 'Spammable' do end describe 'InstanceMethods' do - before do - allow_any_instance_of(Gitlab::AkismetHelper).to receive(:akismet_enabled?).and_return(true) - end - it 'should return the correct creator' do - expect(issue.send(:owner).id).to eq(issue.author_id) + expect(issue.owner_id).to eq(issue.author_id) end it 'should be invalid if spam' do - issue.spam = true - expect(issue.valid?).to be_truthy + issue = build(:issue, spam: true) + expect(issue.valid?).to be_falsey end - it 'should be submittable' do + it 'should not be submitted' do create(:user_agent_detail, subject: issue) - expect(issue.can_be_submitted?).to be_truthy + expect(issue.submitted?).to be_falsey end describe '#check_for_spam?' do + before do + allow_any_instance_of(ApplicationSetting).to receive(:akismet_enabled).and_return(true) + end it 'returns true for public project' do issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) expect(issue.check_for_spam?).to eq(true) @@ -43,14 +42,4 @@ describe Issue, 'Spammable' do end end end - - describe 'AkismetMethods' do - before do - allow_any_instance_of(Gitlab::AkismetHelper).to receive_messages(is_spam?: true, spam!: true, akismet_enabled?: true) - allow_any_instance_of(Spammable).to receive(:can_be_submitted?).and_return(true) - end - - it { expect(issue.spam_detected?(:mock_env)).to be_truthy } - it { expect(issue.submit_spam).to be_truthy } - end end -- cgit v1.2.1 From e8aab1cd1550dd14408ae4c7b51f45110898b949 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 15 Aug 2016 23:26:40 +0200 Subject: This fixes a long running tests due to changed Sidekiq state --- spec/models/project_services/irker_service_spec.rb | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index b528baaf15c..a8f2d626477 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -52,19 +52,20 @@ describe IrkerService, models: true do let(:colorize_messages) { '1' } before do + @irker_server = TCPServer.new 'localhost', 0 + allow(irker).to receive_messages( - active: true, - project: project, - project_id: project.id, - service_hook: true, - server_host: 'localhost', - server_port: 6659, - default_irc_uri: 'irc://chat.freenode.net/', - recipients: recipients, - colorize_messages: colorize_messages) + active: true, + project: project, + project_id: project.id, + service_hook: true, + server_host: @irker_server.addr[2], + server_port: @irker_server.addr[1], + default_irc_uri: 'irc://chat.freenode.net/', + recipients: recipients, + colorize_messages: colorize_messages) irker.valid? - @irker_server = TCPServer.new 'localhost', 6659 end after do -- cgit v1.2.1 From cd8ba09ee8276b1f2c3ef186156c91ec92cddea3 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 15 Aug 2016 23:33:36 +0200 Subject: Make rubocop happy --- spec/models/project_services/irker_service_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index a8f2d626477..ea718232255 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -55,15 +55,15 @@ describe IrkerService, models: true do @irker_server = TCPServer.new 'localhost', 0 allow(irker).to receive_messages( - active: true, - project: project, - project_id: project.id, - service_hook: true, - server_host: @irker_server.addr[2], - server_port: @irker_server.addr[1], - default_irc_uri: 'irc://chat.freenode.net/', - recipients: recipients, - colorize_messages: colorize_messages) + active: true, + project: project, + project_id: project.id, + service_hook: true, + server_host: @irker_server.addr[2], + server_port: @irker_server.addr[1], + default_irc_uri: 'irc://chat.freenode.net/', + recipients: recipients, + colorize_messages: colorize_messages) irker.valid? end -- cgit v1.2.1 From 5994c11910822463faeabb7b5f11d6529036db9d Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Tue, 9 Aug 2016 12:43:47 -0500 Subject: Further refactor and syntax fixes. --- spec/models/concerns/spammable_spec.rb | 14 +------------- spec/models/user_agent_detail_spec.rb | 22 ++++++++++++++++++---- 2 files changed, 19 insertions(+), 17 deletions(-) (limited to 'spec/models') diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index 7944305e7b3..32935bc0b09 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -9,29 +9,17 @@ describe Issue, 'Spammable' do describe 'ClassMethods' do it 'should return correct attr_spammable' do - expect(issue.send(:spammable_text)).to eq("#{issue.title}\n#{issue.description}") + expect(issue.spammable_text).to eq("#{issue.title}\n#{issue.description}") end end describe 'InstanceMethods' do - it 'should return the correct creator' do - expect(issue.owner_id).to eq(issue.author_id) - end - it 'should be invalid if spam' do issue = build(:issue, spam: true) expect(issue.valid?).to be_falsey end - it 'should not be submitted' do - create(:user_agent_detail, subject: issue) - expect(issue.submitted?).to be_falsey - end - describe '#check_for_spam?' do - before do - allow_any_instance_of(ApplicationSetting).to receive(:akismet_enabled).and_return(true) - end it 'returns true for public project' do issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) expect(issue.check_for_spam?).to eq(true) diff --git a/spec/models/user_agent_detail_spec.rb b/spec/models/user_agent_detail_spec.rb index ba21161fc7f..a8c25766e73 100644 --- a/spec/models/user_agent_detail_spec.rb +++ b/spec/models/user_agent_detail_spec.rb @@ -2,16 +2,30 @@ require 'rails_helper' describe UserAgentDetail, type: :model do describe '.submittable?' do - it 'should be submittable' do - detail = create(:user_agent_detail, :on_issue) + it 'is submittable when not already submitted' do + detail = build(:user_agent_detail) + expect(detail.submittable?).to be_truthy end + + it 'is not submittable when already submitted' do + detail = build(:user_agent_detail, submitted: true) + + expect(detail.submittable?).to be_falsey + end end describe '.valid?' do - it 'should be valid with a subject' do - detail = create(:user_agent_detail, :on_issue) + it 'is valid with a subject' do + detail = build(:user_agent_detail) + expect(detail).to be_valid end + + it 'is invalid without a subject' do + detail = build(:user_agent_detail, subject: nil) + + expect(detail).not_to be_valid + end end end -- cgit v1.2.1