diff options
Diffstat (limited to 'spec')
47 files changed, 910 insertions, 224 deletions
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 92b97bf3d0c..a0870891cf4 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -87,10 +87,10 @@ describe Groups::GroupMembersController do context 'when member is not found' do before { sign_in(user) } - it 'returns 403' do + it 'returns 404' do delete :leave, group_id: group - expect(response).to have_http_status(403) + expect(response).to have_http_status(404) end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 94c9edc91fe..742edd8ba3d 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -644,6 +644,20 @@ describe Projects::MergeRequestsController do end end + context 'POST remove_wip' do + it 'removes the wip status' do + merge_request.title = merge_request.wip_title + merge_request.save + + post :remove_wip, + namespace_id: merge_request.project.namespace.to_param, + project_id: merge_request.project.to_param, + id: merge_request.iid + + expect(merge_request.reload.title).to eq(merge_request.wipless_title) + end + end + context 'POST resolve_conflicts' do let(:json_response) { JSON.parse(response.body) } let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha } diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 5e2a8cf3849..074f85157de 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -135,11 +135,11 @@ describe Projects::ProjectMembersController do context 'when member is not found' do before { sign_in(user) } - it 'returns 403' do + it 'returns 404' do delete :leave, namespace_id: project.namespace, project_id: project - expect(response).to have_http_status(403) + expect(response).to have_http_status(404) end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 8f27e616c3e..48d69377461 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -109,6 +109,44 @@ describe SessionsController do end end + context 'when the user is on their last attempt' do + before do + user.update(failed_attempts: User.maximum_attempts.pred) + end + + context 'when OTP is valid' do + it 'authenticates correctly' do + authenticate_2fa(otp_attempt: user.current_otp) + + expect(subject.current_user).to eq user + end + end + + context 'when OTP is invalid' do + before { authenticate_2fa(otp_attempt: 'invalid') } + + it 'does not authenticate' do + expect(subject.current_user).not_to eq user + end + + it 'warns about invalid login' do + expect(response).to set_flash.now[:alert] + .to /Invalid Login or password/ + end + + it 'locks the user' do + expect(user.reload).to be_access_locked + end + + it 'keeps the user locked on future login attempts' do + post(:create, user: { login: user.username, password: user.password }) + + expect(response) + .to set_flash.now[:alert].to /Invalid Login or password/ + end + end + end + context 'when another user does not have 2FA enabled' do let(:another_user) { create(:user) } diff --git a/spec/features/issues/user_uses_slash_commands_spec.rb b/spec/features/issues/user_uses_slash_commands_spec.rb index bf2b93c92fb..3f2da1c380c 100644 --- a/spec/features/issues/user_uses_slash_commands_spec.rb +++ b/spec/features/issues/user_uses_slash_commands_spec.rb @@ -99,5 +99,15 @@ feature 'Issues > User uses slash commands', feature: true, js: true do end end end + + describe 'toggling the WIP prefix from the title from note' do + let(:issue) { create(:issue, project: project) } + + it 'does not recognize the command nor create a note' do + write_note("/wip") + + expect(page).not_to have_content '/wip' + end + end end end diff --git a/spec/features/merge_requests/user_uses_slash_commands_spec.rb b/spec/features/merge_requests/user_uses_slash_commands_spec.rb index 22d9d1b9fd5..cb3cea3fd51 100644 --- a/spec/features/merge_requests/user_uses_slash_commands_spec.rb +++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb @@ -14,21 +14,66 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do let(:new_url_opts) { { merge_request: { source_branch: 'feature' } } } end - describe 'adding a due date from note' do + describe 'merge-request-only commands' do before do project.team << [user, :master] login_with(user) visit namespace_project_merge_request_path(project.namespace, project, merge_request) end - + after do wait_for_ajax end - it 'does not recognize the command nor create a note' do - write_note("/due 2016-08-28") + describe 'toggling the WIP prefix in the title from note' do + context 'when the current user can toggle the WIP prefix' do + it 'adds the WIP: prefix to the title' do + write_note("/wip") + + expect(page).not_to have_content '/wip' + expect(page).to have_content 'Your commands have been executed!' + + expect(merge_request.reload.work_in_progress?).to eq true + end + + it 'removes the WIP: prefix from the title' do + merge_request.title = merge_request.wip_title + merge_request.save + write_note("/wip") + + expect(page).not_to have_content '/wip' + expect(page).to have_content 'Your commands have been executed!' + + expect(merge_request.reload.work_in_progress?).to eq false + end + end + + context 'when the current user cannot toggle the WIP prefix' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + logout + login_with(guest) + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'does not change the WIP prefix' do + write_note("/wip") + + expect(page).not_to have_content '/wip' + expect(page).not_to have_content 'Your commands have been executed!' + + expect(merge_request.reload.work_in_progress?).to eq false + end + end + end + + describe 'adding a due date from note' do + it 'does not recognize the command nor create a note' do + write_note('/due 2016-08-28') - expect(page).not_to have_content '/due 2016-08-28' + expect(page).not_to have_content '/due 2016-08-28' + end end end end diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index f1c522155d3..5d7247e2a62 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -240,6 +240,18 @@ describe 'Comments', feature: true do is_expected.to have_css('.notes_holder .note', count: 1) is_expected.to have_button('Reply...') end + + it 'adds code to discussion' do + click_button 'Reply...' + + page.within(first('.js-discussion-note-form')) do + fill_in 'note[note]', with: '```{{ test }}```' + + click_button('Comment') + end + + expect(page).to have_content('{{ test }}') + end end end end diff --git a/spec/features/profiles/keys_spec.rb b/spec/features/profiles/keys_spec.rb index 3b20d38c520..eb1050d21c6 100644 --- a/spec/features/profiles/keys_spec.rb +++ b/spec/features/profiles/keys_spec.rb @@ -1,18 +1,57 @@ require 'rails_helper' -describe 'Profile > SSH Keys', feature: true do +feature 'Profile > SSH Keys', feature: true do let(:user) { create(:user) } before do login_as(user) - visit profile_keys_path end - describe 'User adds an SSH key' do - it 'auto-populates the title', js: true do + describe 'User adds a key' do + before do + visit profile_keys_path + end + + scenario 'auto-populates the title', js: true do fill_in('Key', with: attributes_for(:key).fetch(:key)) expect(find_field('Title').value).to eq 'dummy@gitlab.com' end + + scenario 'saves the new key' do + attrs = attributes_for(:key) + + fill_in('Key', with: attrs[:key]) + fill_in('Title', with: attrs[:title]) + click_button('Add key') + + expect(page).to have_content("Title: #{attrs[:title]}") + expect(page).to have_content(attrs[:key]) + end + end + + scenario 'User sees their keys' do + key = create(:key, user: user) + visit profile_keys_path + + expect(page).to have_content(key.title) + end + + scenario 'User removes a key via the key index' do + create(:key, user: user) + visit profile_keys_path + + click_link('Remove') + + expect(page).to have_content('Your SSH keys (0)') + end + + scenario 'User removes a key via its details page' do + key = create(:key, user: user) + visit profile_key_path(key) + + click_link('Remove') + + expect(page).to have_content('Your SSH keys (0)') end end diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 09cd6369881..f32834801a0 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -86,14 +86,14 @@ feature 'Import/Export - project import integration test', feature: true, js: tr login_as(normal_user) end - scenario 'non-admin user is not allowed to import a project' do + scenario 'non-admin user is allowed to import a project' do expect(Project.all.count).to be_zero visit new_project_path fill_in :project_path, with: 'test-project-path', visible: true - expect(page).not_to have_content('GitLab export') + expect(page).to have_content('GitLab export') end end diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb index f76c4fe8b57..cd79c4f512d 100644 --- a/spec/features/projects/issuable_templates_spec.rb +++ b/spec/features/projects/issuable_templates_spec.rb @@ -26,7 +26,7 @@ feature 'issuable templates', feature: true, js: true do scenario 'user selects "bug" template' do select_template 'bug' wait_for_ajax - preview_template + preview_template(template_content) save_changes end @@ -42,6 +42,26 @@ feature 'issuable templates', feature: true, js: true do end end + context 'user creates an issue using templates, with a prior description' do + let(:prior_description) { 'test issue description' } + let(:template_content) { 'this is a test "bug" template' } + let(:issue) { create(:issue, author: user, assignee: user, project: project) } + + background do + project.repository.commit_file(user, '.gitlab/issue_templates/bug.md', template_content, 'added issue template', 'master', false) + visit edit_namespace_project_issue_path project.namespace, project, issue + fill_in :'issue[title]', with: 'test issue title' + fill_in :'issue[description]', with: prior_description + end + + scenario 'user selects "bug" template' do + select_template 'bug' + wait_for_ajax + preview_template("#{prior_description}\n\n#{template_content}") + save_changes + end + end + context 'user creates a merge request using templates' do let(:template_content) { 'this is a test "feature-proposal" template' } let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) } @@ -55,7 +75,7 @@ feature 'issuable templates', feature: true, js: true do scenario 'user selects "feature-proposal" template' do select_template 'feature-proposal' wait_for_ajax - preview_template + preview_template(template_content) save_changes end end @@ -82,16 +102,16 @@ feature 'issuable templates', feature: true, js: true do scenario 'user selects template' do select_template 'feature-proposal' wait_for_ajax - preview_template + preview_template(template_content) save_changes end end end end - def preview_template + def preview_template(expected_content) click_link 'Preview' - expect(page).to have_content template_content + expect(page).to have_content expected_content end def save_changes diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index fc555a74f30..bf93c1d1251 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -4,7 +4,7 @@ describe 'Dashboard Todos', feature: true do let(:user) { create(:user) } let(:author) { create(:user) } let(:project) { create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } - let(:issue) { create(:issue) } + let(:issue) { create(:issue, due_date: Date.today) } describe 'GET /dashboard/todos' do context 'User does not have todos' do @@ -28,6 +28,12 @@ describe 'Dashboard Todos', feature: true do expect(page).to have_selector('.todos-list .todo', count: 1) end + it 'shows due date as today' do + page.within first('.todo') do + expect(page).to have_content 'Due today' + end + end + describe 'deleting the todo' do before do first('.done-todo').click diff --git a/spec/finders/access_requests_finder_spec.rb b/spec/finders/access_requests_finder_spec.rb index 626513200e4..8cfea9659cb 100644 --- a/spec/finders/access_requests_finder_spec.rb +++ b/spec/finders/access_requests_finder_spec.rb @@ -16,7 +16,7 @@ describe AccessRequestsFinder, services: true do access_requesters = described_class.new(source).public_send(method_name, user) expect(access_requesters.size).to eq(1) - expect(access_requesters.first).to be_a "#{source.class.to_s}Member".constantize + expect(access_requesters.first).to be_a "#{source.class}Member".constantize expect(access_requesters.first.user).to eq(access_requester) end end diff --git a/spec/finders/trending_projects_finder_spec.rb b/spec/finders/trending_projects_finder_spec.rb index a49cbfd5160..cfe15b9defa 100644 --- a/spec/finders/trending_projects_finder_spec.rb +++ b/spec/finders/trending_projects_finder_spec.rb @@ -1,39 +1,48 @@ require 'spec_helper' describe TrendingProjectsFinder do - let(:user) { build(:user) } + let(:user) { create(:user) } + let(:public_project1) { create(:empty_project, :public) } + let(:public_project2) { create(:empty_project, :public) } + let(:private_project) { create(:empty_project, :private) } + let(:internal_project) { create(:empty_project, :internal) } + + before do + 3.times do + create(:note_on_commit, project: public_project1) + end - describe '#execute' do - describe 'without an explicit start date' do - subject { described_class.new } + 2.times do + create(:note_on_commit, project: public_project2, created_at: 5.weeks.ago) + end - it 'returns the trending projects' do - relation = double(:ar_relation) + create(:note_on_commit, project: private_project) + create(:note_on_commit, project: internal_project) + end - allow(subject).to receive(:projects_for) - .with(user) - .and_return(relation) + describe '#execute', caching: true do + context 'without an explicit time range' do + it 'returns public trending projects' do + projects = described_class.new.execute - allow(relation).to receive(:trending) - .with(an_instance_of(ActiveSupport::TimeWithZone)) + expect(projects).to eq([public_project1]) end end - describe 'with an explicit start date' do - let(:date) { 2.months.ago } + context 'with an explicit time range' do + it 'returns public trending projects' do + projects = described_class.new.execute(2) - subject { described_class.new } + expect(projects).to eq([public_project1, public_project2]) + end + end - it 'returns the trending projects' do - relation = double(:ar_relation) + it 'caches the list of projects' do + projects = described_class.new - allow(subject).to receive(:projects_for) - .with(user) - .and_return(relation) + expect(Project).to receive(:trending).once - allow(relation).to receive(:trending) - .with(date) - end + 2.times { projects.execute } end end end diff --git a/spec/javascripts/search_autocomplete_spec.js b/spec/javascripts/search_autocomplete_spec.js index 00d9fc1302a..4470fbcb099 100644 --- a/spec/javascripts/search_autocomplete_spec.js +++ b/spec/javascripts/search_autocomplete_spec.js @@ -112,7 +112,7 @@ fixture.preload('search_autocomplete.html'); beforeEach(function() { fixture.load('search_autocomplete.html'); - return widget = new SearchAutocomplete; + return widget = new gl.SearchAutocomplete; }); it('should show Dashboard specific dropdown menu', function() { var list; diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb index b1370bca833..d265d29ee86 100644 --- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb +++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb @@ -6,21 +6,21 @@ describe Banzai::Filter::SyntaxHighlightFilter, lib: true do context "when no language is specified" do it "highlights as plaintext" do result = filter('<pre><code>def fun end</code></pre>') - expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext"><code>def fun end</code></pre>') + expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" v-pre="true"><code>def fun end</code></pre>') end end context "when a valid language is specified" do it "highlights as that language" do result = filter('<pre><code class="ruby">def fun end</code></pre>') - expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight ruby"><code><span class="k">def</span> <span class="nf">fun</span> <span class="k">end</span></code></pre>') + expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight ruby" v-pre="true"><code><span class="k">def</span> <span class="nf">fun</span> <span class="k">end</span></code></pre>') end end context "when an invalid language is specified" do it "highlights as plaintext" do result = filter('<pre><code class="gnuplot">This is a test</code></pre>') - expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext"><code>This is a test</code></pre>') + expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" v-pre="true"><code>This is a test</code></pre>') end end @@ -31,7 +31,7 @@ describe Banzai::Filter::SyntaxHighlightFilter, lib: true do it "highlights as plaintext" do result = filter('<pre><code class="ruby">This is a test</code></pre>') - expect(result.to_html).to eq('<pre class="code highlight"><code>This is a test</code></pre>') + expect(result.to_html).to eq('<pre class="code highlight" v-pre="true"><code>This is a test</code></pre>') end end end diff --git a/spec/lib/banzai/filter/task_list_filter_spec.rb b/spec/lib/banzai/filter/task_list_filter_spec.rb deleted file mode 100644 index 569cbc885c7..00000000000 --- a/spec/lib/banzai/filter/task_list_filter_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'spec_helper' - -describe Banzai::Filter::TaskListFilter, lib: true do - include FilterSpecHelper - - it 'does not apply `task-list` class to non-task lists' do - exp = act = %(<ul><li>Item</li></ul>) - expect(filter(act).to_html).to eq exp - end - - it 'applies `task-list` to single-item task lists' do - act = filter('<ul><li>[ ] Task 1</li></ul>') - - expect(act.to_html).to start_with '<ul class="task-list">' - end -end diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/github_import/project_creator_spec.rb index ab06b7bc5bb..a73b1f4ff5d 100644 --- a/spec/lib/gitlab/github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/github_import/project_creator_spec.rb @@ -33,7 +33,7 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do expect(project.import_data.credentials).to eq(user: 'asdffg', password: nil) end - context 'when Github project is private' do + context 'when GitHub project is private' do it 'sets project visibility to private' do repo.private = true @@ -43,7 +43,7 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do end end - context 'when Github project is public' do + context 'when GitHub project is public' do before do allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL) end @@ -56,5 +56,25 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) end end + + context 'when GitHub project has wiki' do + it 'does not create the wiki repository' do + allow(repo).to receive(:has_wiki?).and_return(true) + + project = service.execute + + expect(project.wiki.repository_exists?).to eq false + end + end + + context 'when GitHub project does not have wiki' do + it 'creates the wiki repository' do + allow(repo).to receive(:has_wiki?).and_return(false) + + project = service.execute + + expect(project.wiki.repository_exists?).to eq true + end + end end end diff --git a/spec/lib/gitlab/identifier_spec.rb b/spec/lib/gitlab/identifier_spec.rb new file mode 100644 index 00000000000..47d6f1007d1 --- /dev/null +++ b/spec/lib/gitlab/identifier_spec.rb @@ -0,0 +1,123 @@ +require 'spec_helper' + +describe Gitlab::Identifier do + let(:identifier) do + Class.new { include Gitlab::Identifier }.new + end + + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + let(:key) { create(:key, user: user) } + + describe '#identify' do + context 'without an identifier' do + it 'identifies the user using a commit' do + expect(identifier).to receive(:identify_using_commit). + with(project, '123') + + identifier.identify('', project, '123') + end + end + + context 'with a user identifier' do + it 'identifies the user using a user ID' do + expect(identifier).to receive(:identify_using_user). + with("user-#{user.id}") + + identifier.identify("user-#{user.id}", project, '123') + end + end + + context 'with an SSH key identifier' do + it 'identifies the user using an SSH key ID' do + expect(identifier).to receive(:identify_using_ssh_key). + with("key-#{key.id}") + + identifier.identify("key-#{key.id}", project, '123') + end + end + end + + describe '#identify_using_commit' do + it "returns the User for an existing commit author's Email address" do + commit = double(:commit, author_email: user.email) + + expect(project).to receive(:commit).with('123').and_return(commit) + + expect(identifier.identify_using_commit(project, '123')).to eq(user) + end + + it 'returns nil when no user could be found' do + allow(project).to receive(:commit).with('123').and_return(nil) + + expect(identifier.identify_using_commit(project, '123')).to be_nil + end + + it 'returns nil when the commit does not have an author Email' do + commit = double(:commit, author_email: nil) + + expect(project).to receive(:commit).with('123').and_return(commit) + + expect(identifier.identify_using_commit(project, '123')).to be_nil + end + + it 'caches the found users per Email' do + commit = double(:commit, author_email: user.email) + + expect(project).to receive(:commit).with('123').twice.and_return(commit) + expect(User).to receive(:find_by).once.and_call_original + + 2.times do + expect(identifier.identify_using_commit(project, '123')).to eq(user) + end + end + end + + describe '#identify_using_user' do + it 'returns the User for an existing ID in the identifier' do + found = identifier.identify_using_user("user-#{user.id}") + + expect(found).to eq(user) + end + + it 'returns nil for a non existing user ID' do + found = identifier.identify_using_user('user--1') + + expect(found).to be_nil + end + + it 'caches the found users per ID' do + expect(User).to receive(:find_by).once.and_call_original + + 2.times do + found = identifier.identify_using_user("user-#{user.id}") + + expect(found).to eq(user) + end + end + end + + describe '#identify_using_ssh_key' do + it 'returns the User for an existing SSH key' do + found = identifier.identify_using_ssh_key("key-#{key.id}") + + expect(found).to eq(user) + end + + it 'returns nil for an invalid SSH key' do + found = identifier.identify_using_ssh_key('key--1') + + expect(found).to be_nil + end + + it 'caches the found users per key' do + expect(User).to receive(:find_by_ssh_key_id).once.and_call_original + + 2.times do + found = identifier.identify_using_ssh_key("key-#{key.id}") + + expect(found).to eq(user) + end + end + end +end diff --git a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb index 2ba344092ce..2e19d590d83 100644 --- a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb @@ -27,7 +27,7 @@ describe 'Import/Export attribute configuration', lib: true do relation_names.each do |relation_name| relation_class = relation_class_for_name(relation_name) - expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class.to_s} to exist in safe_model_attributes" + expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class} to exist in safe_model_attributes" current_attributes = parsed_attributes(relation_name, relation_class.attribute_names) safe_attributes = safe_model_attributes[relation_class.to_s] diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index cd8578b6f49..0e4130e8a3a 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -831,6 +831,7 @@ describe Notify do let(:user) { create(:user, email: 'old-email@mail.com') } before do + stub_config_setting(email_subject_suffix: 'A Nice Suffix') perform_enqueued_jobs do user.email = "new-email@mail.com" user.save @@ -847,7 +848,7 @@ describe Notify do end it 'has the correct subject' do - is_expected.to have_subject "Confirmation instructions" + is_expected.to have_subject /^Confirmation instructions/ end it 'includes a link to the site' do diff --git a/spec/mailers/shared/notify.rb b/spec/mailers/shared/notify.rb index 5c9851f14c7..3956d05060b 100644 --- a/spec/mailers/shared/notify.rb +++ b/spec/mailers/shared/notify.rb @@ -37,6 +37,16 @@ shared_examples 'an email sent from GitLab' do reply_to = subject.header[:reply_to].addresses expect(reply_to).to eq([gitlab_sender_reply_to]) end + + context 'when custom suffix for email subject is set' do + before do + stub_config_setting(email_subject_suffix: 'A Nice Suffix') + end + + it 'ends the subject with the suffix' do + is_expected.to have_subject /\ \| A Nice Suffix$/ + end + end end shared_examples 'an email that contains a header with author username' do diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index e7864b7ad33..ae185de9ca3 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -39,8 +39,8 @@ describe Ci::Build, models: true do end end - describe '#ignored?' do - subject { build.ignored? } + describe '#failed_but_allowed?' do + subject { build.failed_but_allowed? } context 'when build is not allowed to fail' do before do diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 2f1baff5d66..80c2a1bc7a9 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -7,7 +7,11 @@ describe CommitStatus, models: true do create(:ci_pipeline, project: project, sha: project.commit.id) end - let(:commit_status) { create(:commit_status, pipeline: pipeline) } + let(:commit_status) { create_status } + + def create_status(args = {}) + create(:commit_status, args.merge(pipeline: pipeline)) + end it { is_expected.to belong_to(:pipeline) } it { is_expected.to belong_to(:user) } @@ -125,32 +129,53 @@ describe CommitStatus, models: true do describe '.latest' do subject { CommitStatus.latest.order(:id) } - before do - @commit1 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'running' - @commit2 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'cc', ref: 'cc', status: 'pending' - @commit3 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'cc', status: 'success' - @commit4 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'cc', ref: 'bb', status: 'success' - @commit5 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'success' + let(:statuses) do + [create_status(name: 'aa', ref: 'bb', status: 'running'), + create_status(name: 'cc', ref: 'cc', status: 'pending'), + create_status(name: 'aa', ref: 'cc', status: 'success'), + create_status(name: 'cc', ref: 'bb', status: 'success'), + create_status(name: 'aa', ref: 'bb', status: 'success')] end it 'returns unique statuses' do - is_expected.to eq([@commit4, @commit5]) + is_expected.to eq(statuses.values_at(3, 4)) end end describe '.running_or_pending' do subject { CommitStatus.running_or_pending.order(:id) } - before do - @commit1 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'running' - @commit2 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'cc', ref: 'cc', status: 'pending' - @commit3 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: nil, status: 'success' - @commit4 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'dd', ref: nil, status: 'failed' - @commit5 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'ee', ref: nil, status: 'canceled' + let(:statuses) do + [create_status(name: 'aa', ref: 'bb', status: 'running'), + create_status(name: 'cc', ref: 'cc', status: 'pending'), + create_status(name: 'aa', ref: nil, status: 'success'), + create_status(name: 'dd', ref: nil, status: 'failed'), + create_status(name: 'ee', ref: nil, status: 'canceled')] end it 'returns statuses that are running or pending' do - is_expected.to eq([@commit1, @commit2]) + is_expected.to eq(statuses.values_at(0, 1)) + end + end + + describe '.exclude_ignored' do + subject { CommitStatus.exclude_ignored.order(:id) } + + let(:statuses) do + [create_status(when: 'manual', status: 'skipped'), + create_status(when: 'manual', status: 'success'), + create_status(when: 'manual', status: 'failed'), + create_status(when: 'on_failure', status: 'skipped'), + create_status(when: 'on_failure', status: 'success'), + create_status(when: 'on_failure', status: 'failed'), + create_status(allow_failure: true, status: 'success'), + create_status(allow_failure: true, status: 'failed'), + create_status(allow_failure: false, status: 'success'), + create_status(allow_failure: false, status: 'failed')] + end + + it 'returns statuses without what we want to ignore' do + is_expected.to eq(statuses.values_at(1, 2, 4, 5, 6, 8, 9)) end end diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index e118432d098..87bffbdc54e 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -1,26 +1,17 @@ require 'spec_helper' describe HasStatus do - before do - @object = Object.new - @object.extend(HasStatus::ClassMethods) - end - describe '.status' do - before do - allow(@object).to receive(:all).and_return(CommitStatus.where(id: statuses)) - end - - subject { @object.status } + subject { CommitStatus.status } shared_examples 'build status summary' do context 'all successful' do - let(:statuses) { Array.new(2) { create(type, status: :success) } } + let!(:statuses) { Array.new(2) { create(type, status: :success) } } it { is_expected.to eq 'success' } end context 'at least one failed' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :failed)] end @@ -28,7 +19,7 @@ describe HasStatus do end context 'at least one running' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :running)] end @@ -36,7 +27,7 @@ describe HasStatus do end context 'at least one pending' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :pending)] end @@ -44,7 +35,7 @@ describe HasStatus do end context 'success and failed but allowed to fail' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :failed, allow_failure: true)] end @@ -53,12 +44,15 @@ describe HasStatus do end context 'one failed but allowed to fail' do - let(:statuses) { [create(type, status: :failed, allow_failure: true)] } + let!(:statuses) do + [create(type, status: :failed, allow_failure: true)] + end + it { is_expected.to eq 'success' } end context 'success and canceled' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :canceled)] end @@ -66,7 +60,7 @@ describe HasStatus do end context 'one failed and one canceled' do - let(:statuses) do + let!(:statuses) do [create(type, status: :failed), create(type, status: :canceled)] end @@ -74,7 +68,7 @@ describe HasStatus do end context 'one failed but allowed to fail and one canceled' do - let(:statuses) do + let!(:statuses) do [create(type, status: :failed, allow_failure: true), create(type, status: :canceled)] end @@ -83,7 +77,7 @@ describe HasStatus do end context 'one running one canceled' do - let(:statuses) do + let!(:statuses) do [create(type, status: :running), create(type, status: :canceled)] end @@ -91,14 +85,15 @@ describe HasStatus do end context 'all canceled' do - let(:statuses) do + let!(:statuses) do [create(type, status: :canceled), create(type, status: :canceled)] end + it { is_expected.to eq 'canceled' } end context 'success and canceled but allowed to fail' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :canceled, allow_failure: true)] end @@ -107,7 +102,7 @@ describe HasStatus do end context 'one finished and second running but allowed to fail' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :running, allow_failure: true)] end @@ -118,11 +113,13 @@ describe HasStatus do context 'ci build statuses' do let(:type) { :ci_build } + it_behaves_like 'build status summary' end context 'generic commit statuses' do let(:type) { :generic_commit_status } + it_behaves_like 'build status summary' end end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 549b0042038..132858950d5 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -1,18 +1,27 @@ require 'spec_helper' describe Mentionable do - include Mentionable + class Example + include Mentionable - def author - nil + attr_accessor :project, :message + attr_mentionable :message + + def author + nil + end end describe 'references' do let(:project) { create(:project) } + let(:mentionable) { Example.new } it 'excludes JIRA references' do allow(project).to receive_messages(jira_tracker?: true) - expect(referenced_mentionables(project, 'JIRA-123')).to be_empty + + mentionable.project = project + mentionable.message = 'JIRA-123' + expect(mentionable.referenced_mentionables).to be_empty end end end @@ -39,9 +48,8 @@ describe Issue, "Mentionable" do let(:user) { create(:user) } def referenced_issues(current_user) - text = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}" - - issue.referenced_mentionables(current_user, text) + issue.title = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}" + issue.referenced_mentionables(current_user) end context 'when the current user can see the issue' do diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 8600eb4d2c4..af5002487cc 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -173,13 +173,11 @@ describe Event, models: true do it 'updates the project' do project.update(last_activity_at: 1.year.ago) - expect_any_instance_of(Gitlab::ExclusiveLease). - to receive(:try_obtain).and_return(true) + create_event(project, project.owner) - expect(project).to receive(:update_column). - with(:last_activity_at, a_kind_of(Time)) + project.reload - create_event(project, project.owner) + project.last_activity_at <= 1.minute.ago end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 580a3235127..38b6da50168 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -86,6 +86,30 @@ describe MergeRequest, models: true do end end + describe '#cache_merge_request_closes_issues!' do + before do + subject.project.team << [subject.author, :developer] + subject.target_branch = subject.project.default_branch + end + + it 'caches closed issues' do + issue = create :issue, project: subject.project + commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") + allow(subject).to receive(:commits).and_return([commit]) + + expect { subject.cache_merge_request_closes_issues! }.to change(subject.merge_requests_closing_issues, :count).by(1) + end + + it 'does not cache issues from external trackers' do + subject.project.update_attribute(:has_external_issue_tracker, true) + issue = ExternalIssue.new('JIRA-123', subject.project) + commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") + allow(subject).to receive(:commits).and_return([commit]) + + expect { subject.cache_merge_request_closes_issues! }.not_to change(subject.merge_requests_closing_issues, :count) + end + end + describe '#source_branch_sha' do let(:last_branch_commit) { subject.source_project.repository.commit(subject.source_branch) } @@ -287,6 +311,46 @@ describe MergeRequest, models: true do end end + describe "#wipless_title" do + ['WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP [WIP] WIP: WIP '].each do |wip_prefix| + it "removes the '#{wip_prefix}' prefix" do + wipless_title = subject.title + subject.title = "#{wip_prefix}#{subject.title}" + + expect(subject.wipless_title).to eq wipless_title + end + + it "is satisfies the #work_in_progress? method" do + subject.title = "#{wip_prefix}#{subject.title}" + subject.title = subject.wipless_title + + expect(subject.work_in_progress?).to eq false + end + end + end + + describe "#wip_title" do + it "adds the WIP: prefix to the title" do + wip_title = "WIP: #{subject.title}" + + expect(subject.wip_title).to eq wip_title + end + + it "does not add the WIP: prefix multiple times" do + wip_title = "WIP: #{subject.title}" + subject.title = subject.wip_title + subject.title = subject.wip_title + + expect(subject.wip_title).to eq wip_title + end + + it "is satisfies the #work_in_progress? method" do + subject.title = subject.wip_title + + expect(subject.work_in_progress?).to eq true + end + end + describe '#can_remove_source_branch?' do let(:user) { create(:user) } let(:user2) { create(:user) } @@ -482,7 +546,7 @@ describe MergeRequest, models: true do end it_behaves_like 'an editable mentionable' do - subject { create(:merge_request) } + subject { create(:merge_request, :simple) } let(:backref_text) { "merge request #{subject.to_reference}" } let(:set_mentionable_text) { ->(txt){ subject.description = txt } } diff --git a/spec/models/project_services/slack_service/issue_message_spec.rb b/spec/models/project_services/slack_service/issue_message_spec.rb index 0f8889bdf3c..98c36ec088d 100644 --- a/spec/models/project_services/slack_service/issue_message_spec.rb +++ b/spec/models/project_services/slack_service/issue_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::IssueMessage, models: true do { user: { name: 'Test User', - username: 'Test User' + username: 'test.user' }, project_name: 'project_name', project_url: 'somewhere.com', @@ -40,7 +40,7 @@ describe SlackService::IssueMessage, models: true do context 'open' do it 'returns a message regarding opening of issues' do expect(subject.pretext).to eq( - '<somewhere.com|[project_name>] Issue opened by Test User') + '<somewhere.com|[project_name>] Issue opened by test.user') expect(subject.attachments).to eq([ { title: "#100 Issue title", @@ -60,7 +60,7 @@ describe SlackService::IssueMessage, models: true do it 'returns a message regarding closing of issues' do expect(subject.pretext). to eq( - '<somewhere.com|[project_name>] Issue <url|#100 Issue title> closed by Test User') + '<somewhere.com|[project_name>] Issue <url|#100 Issue title> closed by test.user') expect(subject.attachments).to be_empty end end diff --git a/spec/models/project_services/slack_service/merge_message_spec.rb b/spec/models/project_services/slack_service/merge_message_spec.rb index 224c7ceabe8..c5c052d9af1 100644 --- a/spec/models/project_services/slack_service/merge_message_spec.rb +++ b/spec/models/project_services/slack_service/merge_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::MergeMessage, models: true do { user: { name: 'Test User', - username: 'Test User' + username: 'test.user' }, project_name: 'project_name', project_url: 'somewhere.com', @@ -31,7 +31,7 @@ describe SlackService::MergeMessage, models: true do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'Test User opened <somewhere.com/merge_requests/100|merge request !100> '\ + 'test.user opened <somewhere.com/merge_requests/100|merge request !100> '\ 'in <somewhere.com|project_name>: *Issue title*') expect(subject.attachments).to be_empty end @@ -43,7 +43,7 @@ describe SlackService::MergeMessage, models: true do end it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'Test User closed <somewhere.com/merge_requests/100|merge request !100> '\ + 'test.user closed <somewhere.com/merge_requests/100|merge request !100> '\ 'in <somewhere.com|project_name>: *Issue title*') expect(subject.attachments).to be_empty end 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 41b93f08050..38cfe4ad3e3 100644 --- a/spec/models/project_services/slack_service/note_message_spec.rb +++ b/spec/models/project_services/slack_service/note_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::NoteMessage, models: true do @args = { user: { name: 'Test User', - username: 'username', + username: 'test.user', avatar_url: 'http://fakeavatar' }, project_name: 'project_name', @@ -37,7 +37,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on commits' do message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("Test User commented on " \ + expect(message.pretext).to eq("test.user commented on " \ "<url|commit 5f163b2b> in <somewhere.com|project_name>: " \ "*Added a commit message*") expected_attachments = [ @@ -63,7 +63,7 @@ describe SlackService::NoteMessage, models: true do 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 " \ + expect(message.pretext).to eq("test.user commented on " \ "<url|merge request !30> in <somewhere.com|project_name>: " \ "*merge request title*") expected_attachments = [ @@ -90,7 +90,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on an issue' do message = SlackService::NoteMessage.new(@args) expect(message.pretext).to eq( - "Test User commented on " \ + "test.user commented on " \ "<url|issue #20> in <somewhere.com|project_name>: " \ "*issue title*") expected_attachments = [ @@ -115,7 +115,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on a project snippet' do message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("Test User commented on " \ + expect(message.pretext).to eq("test.user commented on " \ "<url|snippet #5> in <somewhere.com|project_name>: " \ "*snippet title*") expected_attachments = [ diff --git a/spec/models/project_services/slack_service/push_message_spec.rb b/spec/models/project_services/slack_service/push_message_spec.rb index cda9ee670b0..17cd05e24f1 100644 --- a/spec/models/project_services/slack_service/push_message_spec.rb +++ b/spec/models/project_services/slack_service/push_message_spec.rb @@ -9,7 +9,7 @@ describe SlackService::PushMessage, models: true do before: 'before', project_name: 'project_name', ref: 'refs/heads/master', - user_name: 'user_name', + user_name: 'test.user', project_url: 'url' } end @@ -26,7 +26,7 @@ describe SlackService::PushMessage, models: true do it 'returns a message regarding pushes' do expect(subject.pretext).to eq( - 'user_name pushed to branch <url/commits/master|master> of '\ + 'test.user pushed to branch <url/commits/master|master> of '\ '<url|project_name> (<url/compare/before...after|Compare changes>)' ) expect(subject.attachments).to eq([ @@ -46,13 +46,13 @@ describe SlackService::PushMessage, models: true do before: Gitlab::Git::BLANK_SHA, project_name: 'project_name', ref: 'refs/tags/new_tag', - user_name: 'user_name', + user_name: 'test.user', project_url: 'url' } end it 'returns a message regarding pushes' do - expect(subject.pretext).to eq('user_name pushed new tag ' \ + expect(subject.pretext).to eq('test.user pushed new tag ' \ '<url/commits/new_tag|new_tag> to ' \ '<url|project_name>') expect(subject.attachments).to be_empty @@ -66,7 +66,7 @@ describe SlackService::PushMessage, models: true do it 'returns a message regarding a new branch' do expect(subject.pretext).to eq( - 'user_name pushed new branch <url/commits/master|master> to '\ + 'test.user pushed new branch <url/commits/master|master> to '\ '<url|project_name>' ) expect(subject.attachments).to be_empty @@ -80,7 +80,7 @@ describe SlackService::PushMessage, models: true do it 'returns a message regarding a removed branch' do expect(subject.pretext).to eq( - 'user_name removed branch master from <url|project_name>' + 'test.user removed branch master from <url|project_name>' ) expect(subject.attachments).to be_empty end 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 13aea0b0600..093911598b0 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 @@ -7,7 +7,7 @@ describe SlackService::WikiPageMessage, models: true do { user: { name: 'Test User', - username: 'Test User' + username: 'test.user' }, project_name: 'project_name', project_url: 'somewhere.com', @@ -25,7 +25,7 @@ describe SlackService::WikiPageMessage, models: true do it 'returns a message that a new wiki page was created' do expect(subject.pretext).to eq( - 'Test User created <url|wiki page> in <somewhere.com|project_name>: '\ + 'test.user created <url|wiki page> in <somewhere.com|project_name>: '\ '*Wiki page title*') end end @@ -35,7 +35,7 @@ describe SlackService::WikiPageMessage, models: true do it 'returns a message that a wiki page was updated' do expect(subject.pretext).to eq( - 'Test User edited <url|wiki page> in <somewhere.com|project_name>: '\ + 'test.user edited <url|wiki page> in <somewhere.com|project_name>: '\ '*Wiki page title*') end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ef854a25321..e52d4aaf884 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -308,8 +308,7 @@ describe Project, models: true do end describe 'last_activity methods' do - let(:timestamp) { Time.now - 2.hours } - let(:project) { create(:project, created_at: timestamp, updated_at: timestamp) } + let(:project) { create(:project, last_activity_at: 2.hours.ago) } describe 'last_activity' do it 'alias last_activity to last_event' do @@ -321,7 +320,6 @@ describe Project, models: true do describe 'last_activity_date' do it 'returns the creation date of the project\'s last event if present' do - expect_any_instance_of(Event).to receive(:try_obtain_lease).and_return(true) new_event = create(:event, project: project, created_at: Time.now) expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i) @@ -826,6 +824,14 @@ describe Project, models: true do expect(subject).to eq([project2, project1]) end end + + it 'does not take system notes into account' do + 10.times do + create(:note_on_commit, project: project2, system: true) + end + + expect(described_class.trending.to_a).to eq([project1, project2]) + end end describe '.visible_to_user' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index db29f4d353b..98c64c079b9 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -320,6 +320,16 @@ describe Repository, models: true do end end + describe '#create_ref' do + it 'redirects the call to fetch_ref' do + ref, ref_path = '1', '2' + + expect(repository).to receive(:fetch_ref).with(repository.path_to_repo, ref, ref_path) + + repository.create_ref(ref, ref_path) + end + end + describe "#changelog" do before do repository.send(:cache).expire(:changelog) diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 05056a4bb47..ed1bc9271ae 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -203,6 +203,23 @@ describe Service, models: true do end end + describe 'initialize service with no properties' do + let(:service) do + GitlabIssueTrackerService.create( + project: create(:project), + title: 'random title' + ) + end + + it 'does not raise error' do + expect { service }.not_to raise_error + end + + it 'creates the properties' do + expect(service.properties).to eq({ "title" => "random title" }) + end + end + describe "callbacks" do let(:project) { create(:project) } let!(:service) do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a1770d96f83..65b2896930a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -610,6 +610,23 @@ describe User, models: true do end end + describe '.find_by_ssh_key_id' do + context 'using an existing SSH key ID' do + let(:user) { create(:user) } + let(:key) { create(:key, user: user) } + + it 'returns the corresponding User' do + expect(described_class.find_by_ssh_key_id(key.id)).to eq(user) + end + end + + context 'using an invalid SSH key ID' do + it 'returns nil' do + expect(described_class.find_by_ssh_key_id(-1)).to be_nil + end + end + end + describe '.by_login' do let(:username) { 'John' } let!(:user) { create(:user, username: username) } diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb index 905a7311372..b467890a403 100644 --- a/spec/requests/api/access_requests_spec.rb +++ b/spec/requests/api/access_requests_spec.rb @@ -195,7 +195,7 @@ describe API::AccessRequests, api: true do end context 'when authenticated as the access requester' do - it 'returns 200' do + it 'deletes the access requester' do expect do delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", access_requester) @@ -205,7 +205,7 @@ describe API::AccessRequests, api: true do end context 'when authenticated as a master/owner' do - it 'returns 200' do + it 'deletes the access requester' do expect do delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", master) @@ -213,6 +213,16 @@ describe API::AccessRequests, api: true do end.to change { source.requesters.count }.by(-1) end + context 'user_id matches a member, not an access requester' do + it 'returns 404' do + expect do + delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{developer.id}", master) + + expect(response).to have_http_status(404) + end.not_to change { source.requesters.count } + end + end + context 'user_id does not match an existing access requester' do it 'returns 404' do expect do diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index df97f1bf7b6..7b7d62feb2c 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -35,18 +35,24 @@ describe Ci::API::API do end end - it "starts a build" do - register_builds info: { platform: :darwin } - - expect(response).to have_http_status(201) - expect(json_response['sha']).to eq(build.sha) - expect(runner.reload.platform).to eq("darwin") - expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] }) - expect(json_response["variables"]).to include( - { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true }, - { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true }, - { "key" => "DB_NAME", "value" => "postgres", "public" => true } - ) + context 'when there is a pending build' do + it 'starts a build' do + register_builds info: { platform: :darwin } + + expect(response).to have_http_status(201) + expect(json_response['sha']).to eq(build.sha) + expect(runner.reload.platform).to eq("darwin") + expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] }) + expect(json_response["variables"]).to include( + { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true }, + { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true }, + { "key" => "DB_NAME", "value" => "postgres", "public" => true } + ) + end + + it 'updates runner info' do + expect { register_builds }.to change { runner.reload.contacted_at } + end end context 'when builds are finished' do @@ -159,13 +165,18 @@ describe Ci::API::API do end context 'when runner is paused' do - let(:inactive_runner) { create(:ci_runner, :inactive, token: "InactiveRunner") } + let(:runner) { create(:ci_runner, :inactive, token: 'InactiveRunner') } - before do - register_builds inactive_runner.token + it 'responds with 404' do + register_builds + + expect(response).to have_http_status 404 end - it { expect(response).to have_http_status 404 } + it 'does not update runner info' do + expect { register_builds } + .not_to change { runner.reload.contacted_at } + end end def register_builds(token = runner.token, **params) diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index 8326e5cd313..ff113efd916 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -18,7 +18,7 @@ describe Ci::ProcessPipelineService, services: true do all_builds.where.not(status: [:created, :skipped]) end - def create_builds + def process_pipeline described_class.new(pipeline.project, user).execute(pipeline) end @@ -36,26 +36,26 @@ describe Ci::ProcessPipelineService, services: true do end it 'processes a pipeline' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy succeed_pending expect(builds.success.count).to eq(2) - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy succeed_pending expect(builds.success.count).to eq(4) - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy succeed_pending expect(builds.success.count).to eq(5) - expect(create_builds).to be_falsey + expect(process_pipeline).to be_falsey end it 'does not process pipeline if existing stage is running' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pending.count).to eq(2) - expect(create_builds).to be_falsey + expect(process_pipeline).to be_falsey expect(builds.pending.count).to eq(2) end end @@ -67,7 +67,7 @@ describe Ci::ProcessPipelineService, services: true do end it 'automatically triggers a next stage when build finishes' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:drop) @@ -88,7 +88,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when builds are successful' do it 'properly creates builds' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build') expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:success) @@ -113,7 +113,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when test job fails' do it 'properly creates builds' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build') expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:success) @@ -138,7 +138,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when test and test_failure jobs fail' do it 'properly creates builds' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build') expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:success) @@ -164,7 +164,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when deploy job fails' do it 'properly creates builds' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build') expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:success) @@ -189,7 +189,7 @@ describe Ci::ProcessPipelineService, services: true do 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(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build') expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:success) @@ -208,7 +208,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when listing manual actions' do it 'returns only for skipped builds' do # currently all builds are created - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(manual_actions).to be_empty # succeed stage build @@ -230,6 +230,69 @@ describe Ci::ProcessPipelineService, services: true do end end + context 'when there are manual/on_failure jobs in earlier stages' do + before do + builds + process_pipeline + builds.each(&:reload) + end + + context 'when first stage has only manual jobs' do + let(:builds) do + [create_build('build', 0, 'manual'), + create_build('check', 1), + create_build('test', 2)] + end + + it 'starts from the second stage' do + expect(builds.map(&:status)).to eq(%w[skipped pending created]) + end + end + + context 'when second stage has only manual jobs' do + let(:builds) do + [create_build('check', 0), + create_build('build', 1, 'manual'), + create_build('test', 2)] + end + + it 'skips second stage and continues on third stage' do + expect(builds.map(&:status)).to eq(%w[pending created created]) + + builds.first.success + builds.each(&:reload) + + expect(builds.map(&:status)).to eq(%w[success skipped pending]) + end + end + + context 'when second stage has only on_failure jobs' do + let(:builds) do + [create_build('check', 0), + create_build('build', 1, 'on_failure'), + create_build('test', 2)] + end + + it 'skips second stage and continues on third stage' do + expect(builds.map(&:status)).to eq(%w[pending created created]) + + builds.first.success + builds.each(&:reload) + + expect(builds.map(&:status)).to eq(%w[success skipped pending]) + end + end + + def create_build(name, stage_idx, when_value = nil) + create(:ci_build, + :created, + pipeline: pipeline, + name: name, + stage_idx: stage_idx, + when: when_value) + end + end + context 'when failed build in the middle stage is retried' do context 'when failed build is the only unsuccessful build in the stage' do before do @@ -242,7 +305,7 @@ describe Ci::ProcessPipelineService, services: true do end it 'does trigger builds in the next stage' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build:1', 'build:2') pipeline.builds.running_or_pending.each(&:success) @@ -297,14 +360,14 @@ describe Ci::ProcessPipelineService, services: true do expect(all_builds.count).to eq(2) # Create builds will mark the created as pending - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.count).to eq(2) expect(all_builds.count).to eq(2) # When we builds succeed we will create a rest of pipeline from .gitlab-ci.yml # We will have 2 succeeded, 2 pending (from stage test), total 5 (one more build from deploy) succeed_pending - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.success.count).to eq(2) expect(builds.pending.count).to eq(2) expect(all_builds.count).to eq(5) @@ -312,14 +375,14 @@ describe Ci::ProcessPipelineService, services: true do # When we succeed the 2 pending from stage test, # We will queue a deploy stage, no new builds will be created succeed_pending - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pending.count).to eq(1) expect(builds.success.count).to eq(4) expect(all_builds.count).to eq(5) # When we succeed last pending build, we will have a total of 5 succeeded builds, no new builds will be created succeed_pending - expect(create_builds).to be_falsey + expect(process_pipeline).to be_falsey expect(builds.success.count).to eq(5) expect(all_builds.count).to eq(5) end diff --git a/spec/services/members/approve_access_request_service_spec.rb b/spec/services/members/approve_access_request_service_spec.rb index 6fca80b5613..03e296259f9 100644 --- a/spec/services/members/approve_access_request_service_spec.rb +++ b/spec/services/members/approve_access_request_service_spec.rb @@ -26,7 +26,7 @@ describe Members::ApproveAccessRequestService, services: true do it 'returns a <Source>Member' do member = described_class.new(source, user, params).execute - expect(member).to be_a "#{source.class.to_s}Member".constantize + expect(member).to be_a "#{source.class}Member".constantize expect(member.requested_at).to be_nil end diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 2395445e7fd..9995f3488af 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -2,70 +2,111 @@ require 'spec_helper' describe Members::DestroyService, services: true do let(:user) { create(:user) } - let(:project) { create(:project) } - let!(:member) { create(:project_member, source: project) } + let(:member_user) { create(:user) } + let(:project) { create(:project, :public) } + let(:group) { create(:group, :public) } - context 'when member is nil' do - before do - project.team << [user, :developer] + shared_examples 'a service raising ActiveRecord::RecordNotFound' do + it 'raises ActiveRecord::RecordNotFound' do + expect { described_class.new(source, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) end + end - it 'does not destroy the member' do - expect { destroy_member(nil, user) }.to raise_error(Gitlab::Access::AccessDeniedError) + shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do + it 'raises Gitlab::Access::AccessDeniedError' do + expect { described_class.new(source, user, params).execute }.to raise_error(Gitlab::Access::AccessDeniedError) end end - context 'when current user cannot destroy the given member' do - before do - project.team << [user, :developer] + shared_examples 'a service destroying a member' do + it 'destroys the member' do + expect { described_class.new(source, user, params).execute }.to change { source.members.count }.by(-1) + end + + context 'when the given member is an access requester' do + before do + source.members.find_by(user_id: member_user).destroy + source.request_access(member_user) + end + let(:access_requester) { source.requesters.find_by(user_id: member_user) } + + it_behaves_like 'a service raising ActiveRecord::RecordNotFound' + + %i[requesters all].each do |scope| + context "and #{scope} scope is passed" do + it 'destroys the access requester' do + expect { described_class.new(source, user, params).execute(scope) }.to change { source.requesters.count }.by(-1) + end + + it 'calls Member#after_decline_request' do + expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(access_requester) + + described_class.new(source, user, params).execute(scope) + end + + context 'when current user is the member' do + it 'does not call Member#after_decline_request' do + expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(access_requester) + + described_class.new(source, member_user, params).execute(scope) + end + end + end + end end + end + + context 'when no member are found' do + let(:params) { { user_id: 42 } } - it 'does not destroy the member' do - expect { destroy_member(member, user) }.to raise_error(Gitlab::Access::AccessDeniedError) + it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do + let(:source) { project } + end + + it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do + let(:source) { group } end end - context 'when current user can destroy the given member' do + context 'when a member is found' do before do - project.team << [user, :master] + project.team << [member_user, :developer] + group.add_developer(member_user) end + let(:params) { { user_id: member_user.id } } - it 'destroys the member' do - destroy_member(member, user) + context 'when current user cannot destroy the given member' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { project } + end - expect(member).to be_destroyed + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { group } + end end - context 'when the given member is a requester' do + context 'when current user can destroy the given member' do before do - member.update_column(:requested_at, Time.now) + project.team << [user, :master] + group.add_owner(user) end - it 'calls Member#after_decline_request' do - expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(member) - - destroy_member(member, user) + it_behaves_like 'a service destroying a member' do + let(:source) { project } end - context 'when current user is the member' do - it 'does not call Member#after_decline_request' do - expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member) - - destroy_member(member, member.user) - end + it_behaves_like 'a service destroying a member' do + let(:source) { group } end - context 'when current user is the member and ' do - it 'does not call Member#after_decline_request' do - expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member) + context 'when given a :id' do + let(:params) { { id: project.members.find_by!(user_id: user.id).id } } - destroy_member(member, member.user) + it 'destroys the member' do + expect { described_class.new(project, user, params).execute }. + to change { project.members.count }.by(-1) end end end end - - def destroy_member(member, user) - Members::DestroyService.new(member, user).execute - end end diff --git a/spec/services/members/request_access_service_spec.rb b/spec/services/members/request_access_service_spec.rb index dff5b4917ae..0d2d5f03199 100644 --- a/spec/services/members/request_access_service_spec.rb +++ b/spec/services/members/request_access_service_spec.rb @@ -19,7 +19,7 @@ describe Members::RequestAccessService, services: true do it 'returns a <Source>Member' do member = described_class.new(source, user).execute - expect(member).to be_a "#{source.class.to_s}Member".constantize + expect(member).to be_a "#{source.class}Member".constantize expect(member.requested_at).to be_present end end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 31167675d07..e49a0d5e553 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -38,6 +38,42 @@ describe MergeRequests::MergeService, services: true do end end + context 'closes related issues' do + let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } + + before do + allow(project).to receive(:default_branch).and_return(merge_request.target_branch) + end + + it 'closes GitLab issue tracker issues' do + issue = create :issue, project: project + commit = double('commit', safe_message: "Fixes #{issue.to_reference}") + allow(merge_request).to receive(:commits).and_return([commit]) + + service.execute(merge_request) + + expect(issue.reload.closed?).to be_truthy + end + + context 'with JIRA integration' do + include JiraServiceHelper + + let(:jira_tracker) { project.create_jira_service } + + before { jira_service_settings } + + it 'closes issues on JIRA issue tracker' do + jira_issue = ExternalIssue.new('JIRA-123', project) + commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}") + allow(merge_request).to receive(:commits).and_return([commit]) + + expect_any_instance_of(JiraService).to receive(:close_issue).with(merge_request, jira_issue).once + + service.execute(merge_request) + end + end + end + context 'closes related todos' do let(:merge_request) { create(:merge_request, assignee: user, author: user) } let(:project) { merge_request.project } diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 29341c5e57e..7dcd03496bb 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -5,6 +5,7 @@ describe Projects::DestroyService, services: true do let!(:project) { create(:project, namespace: user.namespace) } let!(:path) { project.repository.path_to_repo } let!(:remove_path) { path.sub(/\.git\Z/, "+#{project.id}+deleted.git") } + let!(:async) { false } # execute or async_execute context 'Sidekiq inline' do before do @@ -28,6 +29,22 @@ describe Projects::DestroyService, services: true do it { expect(Dir.exist?(remove_path)).to be_truthy } end + context 'async delete of project with private issue visibility' do + let!(:async) { true } + + before do + project.project_feature.update_attribute("issues_access_level", ProjectFeature::PRIVATE) + # Run sidekiq immediately to check that renamed repository will be removed + Sidekiq::Testing.inline! { destroy_project(project, user, {}) } + end + + it 'deletes the project' do + expect(Project.all).not_to include(project) + expect(Dir.exist?(path)).to be_falsey + expect(Dir.exist?(remove_path)).to be_falsey + end + end + context 'container registry' do before do stub_container_registry_config(enabled: true) @@ -52,6 +69,10 @@ describe Projects::DestroyService, services: true do end def destroy_project(project, user, params) - Projects::DestroyService.new(project, user, params).execute + if async + Projects::DestroyService.new(project, user, params).async_execute + else + Projects::DestroyService.new(project, user, params).execute + end end end diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb index 5b1edba87a1..ae4d286d250 100644 --- a/spec/services/slash_commands/interpret_service_spec.rb +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -165,6 +165,23 @@ describe SlashCommands::InterpretService, services: true do end end + shared_examples 'wip command' do + it 'returns wip_event: "wip" if content contains /wip' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(wip_event: 'wip') + end + end + + shared_examples 'unwip command' do + it 'returns wip_event: "unwip" if content contains /wip' do + issuable.update(title: issuable.wip_title) + _, updates = service.execute(content, issuable) + + expect(updates).to eq(wip_event: 'unwip') + end + end + shared_examples 'empty command' do it 'populates {} if content contains an unsupported command' do _, updates = service.execute(content, issuable) @@ -376,6 +393,16 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { issue } end + it_behaves_like 'wip command' do + let(:content) { '/wip' } + let(:issuable) { merge_request } + end + + it_behaves_like 'unwip command' do + let(:content) { '/wip' } + let(:issuable) { merge_request } + end + it_behaves_like 'empty command' do let(:content) { '/remove_due_date' } let(:issuable) { merge_request } diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index e876d44c166..f57c82809a6 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -9,7 +9,7 @@ shared_context 'mentionable context' do let(:author) { subject.author } let(:mentioned_issue) { create(:issue, project: project) } - let!(:mentioned_mr) { create(:merge_request, :simple, source_project: project) } + let!(:mentioned_mr) { create(:merge_request, source_project: project) } let(:mentioned_commit) { project.commit("HEAD~1") } let(:ext_proj) { create(:project, :public) } @@ -100,6 +100,7 @@ shared_examples 'an editable mentionable' do it 'creates new cross-reference notes when the mentionable text is edited' do subject.save + subject.create_cross_references! new_text = <<-MSG.strip_heredoc These references already existed: @@ -131,6 +132,7 @@ shared_examples 'an editable mentionable' do end # These two issues are new and should receive reference notes + # In the case of MergeRequests remember that cannot mention commits included in the MergeRequest new_issues.each do |newref| expect(SystemNoteService).to receive(:cross_reference). with(newref, subject.local_reference, author) diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 1d2cf7acddd..ffeaafe654a 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -79,7 +79,9 @@ describe PostReceive do end it "does not run if the author is not in the project" do - allow(Key).to receive(:find_by).with(hash_including(id: anything())) { nil } + allow_any_instance_of(Gitlab::GitPostReceive). + to receive(:identify_using_ssh_key). + and_return(nil) expect(project).not_to receive(:execute_hooks) |