diff options
Diffstat (limited to 'spec')
103 files changed, 2756 insertions, 420 deletions
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 90419368f22..dbe5ddccbcf 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -55,6 +55,30 @@ describe Projects::IssuesController do end describe 'GET #new' do + context 'internal issue tracker' do + before do + sign_in(user) + project.team << [user, :developer] + end + + it 'builds a new issue' do + get :new, namespace_id: project.namespace.path, project_id: project + + expect(assigns(:issue)).to be_a_new(Issue) + end + + it 'fills in an issue for a merge request' do + project_with_repository = create(:project) + project_with_repository.team << [user, :developer] + mr = create(:merge_request_with_diff_notes, source_project: project_with_repository) + + get :new, namespace_id: project_with_repository.namespace.path, project_id: project_with_repository, merge_request_for_resolving_discussions: mr.iid + + expect(assigns(:issue).title).not_to be_empty + expect(assigns(:issue).description).not_to be_empty + end + end + context 'external issue tracker' do it 'redirects to the external issue tracker' do external = double(new_issue_path: 'https://example.com/issues/new') @@ -272,6 +296,42 @@ describe Projects::IssuesController do end describe 'POST #create' do + context 'resolving discussions in MergeRequest' do + let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first } + let(:merge_request) { discussion.noteable } + let(:project) { merge_request.source_project } + + before do + project.team << [user, :master] + sign_in user + end + + let(:merge_request_params) do + { merge_request_for_resolving_discussions: merge_request.iid } + end + + def post_issue(issue_params) + post :create, namespace_id: project.namespace.to_param, project_id: project.to_param, issue: issue_params, merge_request_for_resolving_discussions: merge_request.iid + end + + it 'creates an issue for the project' do + expect { post_issue({ title: 'Hello' }) }.to change { project.issues.reload.size }.by(1) + end + + it "doesn't overwrite given params" do + post_issue(description: 'Manually entered description') + + expect(assigns(:issue).description).to eq('Manually entered description') + end + + it 'resolves the discussion in the merge_request' do + post_issue(title: 'Hello') + discussion.first_note.reload + + expect(discussion.resolved?).to eq(true) + end + end + context 'Akismet is enabled' do before do allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) diff --git a/spec/controllers/projects/todo_controller_spec.rb b/spec/controllers/projects/todo_controller_spec.rb index 193a3f6b5a3..415c264e0dd 100644 --- a/spec/controllers/projects/todo_controller_spec.rb +++ b/spec/controllers/projects/todo_controller_spec.rb @@ -110,7 +110,7 @@ describe Projects::TodosController do end end - context 'when not authorized' do + context 'when not authorized for project' do it 'does not create todo for merge request user has no access to' do sign_in(user) expect do @@ -128,6 +128,19 @@ describe Projects::TodosController do expect(response).to have_http_status(302) end end + + context 'when not authorized for merge_request' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + sign_in(user) + end + + it "doesn't create todo" do + expect{ go }.not_to change { user.todos.count } + expect(response).to have_http_status(404) + end + end end end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 48d69377461..b56c7880b64 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -22,7 +22,6 @@ describe SessionsController do it 'authenticates user correctly' do post(:create, user: { login: user.username, password: user.password }) - expect(response).to set_flash.to /Signed in successfully/ expect(subject.current_user). to eq user end diff --git a/spec/factories/ci/stages.rb b/spec/factories/ci/stages.rb new file mode 100644 index 00000000000..ee3b17b8bf1 --- /dev/null +++ b/spec/factories/ci/stages.rb @@ -0,0 +1,13 @@ +FactoryGirl.define do + factory :ci_stage, class: Ci::Stage do + transient do + name 'test' + status nil + pipeline factory: :ci_empty_pipeline + end + + initialize_with do + Ci::Stage.new(pipeline, name: name, status: status) + end + end +end diff --git a/spec/features/admin/admin_browses_logs_spec.rb b/spec/features/admin/admin_browses_logs_spec.rb new file mode 100644 index 00000000000..d880f3f07db --- /dev/null +++ b/spec/features/admin/admin_browses_logs_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe 'Admin browses logs' do + before do + login_as :admin + end + + it 'shows available log files' do + visit admin_logs_path + + expect(page).to have_content 'test.log' + expect(page).to have_content 'githost.log' + expect(page).to have_content 'application.log' + end +end diff --git a/spec/features/admin/admin_hooks_spec.rb b/spec/features/admin/admin_hooks_spec.rb index b3ce72b1452..f246997d5a2 100644 --- a/spec/features/admin/admin_hooks_spec.rb +++ b/spec/features/admin/admin_hooks_spec.rb @@ -26,16 +26,17 @@ describe "Admin::Hooks", feature: true do end describe "New Hook" do - before do - @url = FFaker::Internet.uri("http") + let(:url) { FFaker::Internet.uri('http') } + + it 'adds new hook' do visit admin_hooks_path - fill_in "hook_url", with: @url - expect { click_button "Add System Hook" }.to change(SystemHook, :count).by(1) - end + fill_in 'hook_url', with: url + check 'Enable SSL verification' - it "opens new hook popup" do + expect { click_button 'Add System Hook' }.to change(SystemHook, :count).by(1) + expect(page).to have_content 'SSL Verification: enabled' expect(current_path).to eq(admin_hooks_path) - expect(page).to have_content(@url) + expect(page).to have_content(url) end end diff --git a/spec/features/environments_spec.rb b/spec/features/environments_spec.rb index c7fe622c477..e1b97b31e5d 100644 --- a/spec/features/environments_spec.rb +++ b/spec/features/environments_spec.rb @@ -85,14 +85,14 @@ feature 'Environments page', :feature, :js do end scenario 'does show a play button' do - find('.dropdown-play-icon-container').click + find('.js-dropdown-play-icon-container').click expect(page).to have_content(manual.name.humanize) end scenario 'does allow to play manual action', js: true do expect(manual).to be_skipped - find('.dropdown-play-icon-container').click + find('.js-dropdown-play-icon-container').click expect(page).to have_content(manual.name.humanize) expect { click_link(manual.name.humanize) } diff --git a/spec/features/groups/labels/edit_spec.rb b/spec/features/groups/labels/edit_spec.rb new file mode 100644 index 00000000000..69281cecb7b --- /dev/null +++ b/spec/features/groups/labels/edit_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +feature 'Edit group label', feature: true do + given(:user) { create(:user) } + given(:group) { create(:group) } + given(:label) { create(:group_label, group: group) } + + background do + group.add_owner(user) + login_as(user) + visit edit_group_label_path(group, label) + end + + scenario 'update label with new title' do + fill_in 'label_title', with: 'new label name' + click_button 'Save changes' + + expect(current_path).to eq(root_path) + expect(label.reload.title).to eq('new label name') + end +end diff --git a/spec/features/groups/members/last_owner_cannot_leave_group_spec.rb b/spec/features/groups/members/last_owner_cannot_leave_group_spec.rb index 33bf6d3752f..be60b0489c7 100644 --- a/spec/features/groups/members/last_owner_cannot_leave_group_spec.rb +++ b/spec/features/groups/members/last_owner_cannot_leave_group_spec.rb @@ -10,7 +10,7 @@ feature 'Groups > Members > Last owner cannot leave group', feature: true do visit group_path(group) end - scenario 'user does not see a "Leave Group" link' do - expect(page).not_to have_content 'Leave Group' + scenario 'user does not see a "Leave group" link' do + expect(page).not_to have_content 'Leave group' end end diff --git a/spec/features/groups/members/member_leaves_group_spec.rb b/spec/features/groups/members/member_leaves_group_spec.rb index 3185ff924b9..ac4d94658ae 100644 --- a/spec/features/groups/members/member_leaves_group_spec.rb +++ b/spec/features/groups/members/member_leaves_group_spec.rb @@ -13,7 +13,7 @@ feature 'Groups > Members > Member leaves group', feature: true do end scenario 'user leaves group' do - click_link 'Leave Group' + click_link 'Leave group' expect(current_path).to eq(dashboard_groups_path) expect(group.users.exists?(user.id)).to be_falsey diff --git a/spec/features/groups/members/user_requests_access_spec.rb b/spec/features/groups/members/user_requests_access_spec.rb index d8c9c487996..e4b5ea91bd3 100644 --- a/spec/features/groups/members/user_requests_access_spec.rb +++ b/spec/features/groups/members/user_requests_access_spec.rb @@ -29,7 +29,7 @@ feature 'Groups > Members > User requests access', feature: true do expect(page).to have_content 'Your request for access has been queued for review.' expect(page).to have_content 'Withdraw Access Request' - expect(page).not_to have_content 'Leave Group' + expect(page).not_to have_content 'Leave group' end scenario 'user does not see private projects' do diff --git a/spec/features/groups/merge_requests_spec.rb b/spec/features/groups/merge_requests_spec.rb index a2791b57544..30b80aa82b0 100644 --- a/spec/features/groups/merge_requests_spec.rb +++ b/spec/features/groups/merge_requests_spec.rb @@ -2,7 +2,35 @@ require 'spec_helper' feature 'Group merge requests page', feature: true do let(:path) { merge_requests_group_path(group) } - let(:issuable) { create(:merge_request, source_project: project, target_project: project, title: "this is my created issuable")} + let(:issuable) { create(:merge_request, source_project: project, target_project: project, title: 'this is my created issuable') } include_examples 'project features apply to issuables', MergeRequest + + context 'archived issuable' do + let(:project_archived) { create(:project, group: group, merge_requests_access_level: ProjectFeature::ENABLED, archived: true) } + let(:issuable_archived) { create(:merge_request, source_project: project_archived, target_project: project_archived, title: 'issuable of an archived project') } + let(:access_level) { ProjectFeature::ENABLED } + let(:user) { user_in_group } + + before do + issuable_archived + visit path + end + + it 'hides archived merge requests' do + expect(page).to have_content(issuable.title) + expect(page).not_to have_content(issuable_archived.title) + end + + it 'ignores archived merge request count badges in navbar' do + expect( page.find('[title="Merge Requests"] span.badge.count').text).to eq("1") + end + + it 'ignores archived merge request count badges in state-filters' do + expect(page.find('#state-opened span.badge').text).to eq("1") + expect(page.find('#state-merged span.badge').text).to eq("0") + expect(page.find('#state-closed span.badge').text).to eq("0") + expect(page.find('#state-all span.badge').text).to eq("1") + end + end end diff --git a/spec/features/help_pages_spec.rb b/spec/features/help_pages_spec.rb index 4319d6db0d2..40a1fced8d8 100644 --- a/spec/features/help_pages_spec.rb +++ b/spec/features/help_pages_spec.rb @@ -1,16 +1,6 @@ require 'spec_helper' describe 'Help Pages', feature: true do - describe 'Show SSH page' do - before do - login_as :user - end - it 'replaces the variable $your_email with the email of the user' do - visit help_page_path('ssh/README') - expect(page).to have_content("ssh-keygen -t rsa -C \"#{@user.email}\"") - end - end - describe 'Get the main help page' do shared_examples_for 'help page' do |prefix: ''| it 'prefixes links correctly' do diff --git a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb new file mode 100644 index 00000000000..762cab0c0e1 --- /dev/null +++ b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb @@ -0,0 +1,76 @@ +require 'rails_helper' + +feature 'Resolving all open discussions in a merge request from an issue', feature: true do + let(:user) { create(:user) } + let(:project) { create(:project, only_allow_merge_if_all_discussions_are_resolved: true) } + let(:merge_request) { create(:merge_request, source_project: project) } + let!(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request, noteable: merge_request, project: project)]).first } + + before do + project.team << [user, :master] + login_as user + end + + context 'with the internal tracker disabled' do + before do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'does not show a link to create a new issue' do + expect(page).not_to have_link 'open an issue to resolve them later' + end + end + + context 'merge request has discussions that need to be resolved' do + before do + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'shows a warning that the merge request contains unresolved discussions' do + expect(page).to have_content 'This merge request has unresolved discussions' + end + + it 'has a link to resolve all discussions by creating an issue' do + page.within '.mr-widget-body' do + expect(page).to have_link 'open an issue to resolve them later', href: new_namespace_project_issue_path(project.namespace, project, merge_request_for_resolving_discussions: merge_request.iid) + end + end + + context 'creating an issue for discussions' do + before do + page.click_link 'open an issue to resolve them later', href: new_namespace_project_issue_path(project.namespace, project, merge_request_for_resolving_discussions: merge_request.iid) + end + + it 'shows an issue with the title filled in' do + title_field = page.find_field('issue[title]') + + expect(title_field.value).to include(merge_request.title) + end + + it 'has a mention of the discussion in the description' do + description_field = page.find_field('issue[description]') + + expect(description_field.value).to include(discussion.first_note.note) + end + + it 'has a hidden field for the merge request' do + merge_request_field = find('#merge_request_for_resolving_discussions', visible: false) + + expect(merge_request_field.value).to eq(merge_request.iid.to_s) + end + + it 'can create a new issue for the project' do + expect { click_button 'Submit issue' }.to change { project.issues.reload.size }.by(1) + end + + it 'resolves the discussion in the merge request' do + click_button 'Submit issue' + + discussion.first_note.reload + + expect(discussion.resolved?).to eq(true) + end + end + end +end diff --git a/spec/features/merge_requests/deleted_source_branch_spec.rb b/spec/features/merge_requests/deleted_source_branch_spec.rb index 778b3a90cf3..d5c9ed8a3b7 100644 --- a/spec/features/merge_requests/deleted_source_branch_spec.rb +++ b/spec/features/merge_requests/deleted_source_branch_spec.rb @@ -1,5 +1,8 @@ require 'spec_helper' +# This test serves as a regression test for a bug that caused an error +# message to be shown by JavaScript when the source branch was deleted. +# Please do not remove "js: true". describe 'Deleted source branch', feature: true, js: true do let(:user) { create(:user) } let(:merge_request) { create(:merge_request) } diff --git a/spec/features/merge_requests/target_branch_spec.rb b/spec/features/merge_requests/target_branch_spec.rb new file mode 100644 index 00000000000..b6134540273 --- /dev/null +++ b/spec/features/merge_requests/target_branch_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe 'Target branch', feature: true do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + + def path_to_merge_request + namespace_project_merge_request_path( + project.namespace, + project, merge_request + ) + end + + before do + login_as user + project.team << [user, :master] + end + + it 'shows link to target branch' do + visit path_to_merge_request + expect(page).to have_link('feature', href: namespace_project_commits_path(project.namespace, project, merge_request.target_branch)) + end + + context 'when branch was deleted' do + before do + DeleteBranchService.new(project, user).execute('feature') + visit path_to_merge_request + end + + it 'shows a message about missing target branch' do + expect(page).to have_content( + 'Target branch feature does not exist' + ) + end + + it 'does not show link to target branch' do + expect(page).not_to have_link('feature') + end + end +end diff --git a/spec/features/projects/features_visibility_spec.rb b/spec/features/projects/features_visibility_spec.rb index 09aa6758b5c..3bb33394be7 100644 --- a/spec/features/projects/features_visibility_spec.rb +++ b/spec/features/projects/features_visibility_spec.rb @@ -182,6 +182,44 @@ describe 'Edit Project Settings', feature: true do expect(page).not_to have_content("Comments") end end + + # Regression spec for https://gitlab.com/gitlab-org/gitlab-ce/issues/25272 + it "hides comments activity tab only on disabled issues, merge requests and repository" do + select "Disabled", from: "project_project_feature_attributes_issues_access_level" + + save_changes_and_check_activity_tab do + expect(page).to have_content("Comments") + end + + visit edit_namespace_project_path(project.namespace, project) + + select "Disabled", from: "project_project_feature_attributes_merge_requests_access_level" + + save_changes_and_check_activity_tab do + expect(page).to have_content("Comments") + end + + visit edit_namespace_project_path(project.namespace, project) + + select "Disabled", from: "project_project_feature_attributes_repository_access_level" + + save_changes_and_check_activity_tab do + expect(page).not_to have_content("Comments") + end + + visit edit_namespace_project_path(project.namespace, project) + end + + def save_changes_and_check_activity_tab + click_button "Save changes" + wait_for_ajax + + visit activity_namespace_project_path(project.namespace, project) + + page.within(".event-filter") do + yield + end + end end # Regression spec for https://gitlab.com/gitlab-org/gitlab-ce/issues/24056 diff --git a/spec/features/projects/guest_navigation_menu_spec.rb b/spec/features/projects/guest_navigation_menu_spec.rb index c22441f8929..8120a51c515 100644 --- a/spec/features/projects/guest_navigation_menu_spec.rb +++ b/spec/features/projects/guest_navigation_menu_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe "Guest navigation menu" do - let(:project) { create :empty_project, :private } - let(:guest) { create :user } + let(:project) { create(:empty_project, :private, public_builds: false) } + let(:guest) { create(:user) } before do project.team << [guest, :guest] diff --git a/spec/features/projects/members/group_links_spec.rb b/spec/features/projects/members/group_links_spec.rb index cc2f695211c..94995f7cf95 100644 --- a/spec/features/projects/members/group_links_spec.rb +++ b/spec/features/projects/members/group_links_spec.rb @@ -16,12 +16,17 @@ feature 'Projects > Members > Anonymous user sees members', feature: true, js: t end it 'updates group access level' do - select 'Guest', from: "member_access_level_#{group.id}" + click_button @group_link.human_access + + page.within '.dropdown-menu' do + click_link 'Guest' + end + wait_for_ajax visit namespace_project_project_members_path(project.namespace, project) - expect(page).to have_select("member_access_level_#{group.id}", selected: 'Guest') + expect(first('.group_member')).to have_content('Guest') end it 'updates expiry date' do diff --git a/spec/features/projects/members/group_member_cannot_leave_group_project_spec.rb b/spec/features/projects/members/group_member_cannot_leave_group_project_spec.rb index 728c0e16361..b483ba4c54c 100644 --- a/spec/features/projects/members/group_member_cannot_leave_group_project_spec.rb +++ b/spec/features/projects/members/group_member_cannot_leave_group_project_spec.rb @@ -12,6 +12,6 @@ feature 'Projects > Members > Group member cannot leave group project', feature: end scenario 'user does not see a "Leave project" link' do - expect(page).not_to have_content 'Leave Project' + expect(page).not_to have_content 'Leave project' end end diff --git a/spec/features/projects/members/group_members_spec.rb b/spec/features/projects/members/group_members_spec.rb new file mode 100644 index 00000000000..7d0065ee2c4 --- /dev/null +++ b/spec/features/projects/members/group_members_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +feature 'Projects members', feature: true do + let(:user) { create(:user) } + let(:developer) { create(:user) } + let(:group) { create(:group, :public, :access_requestable) } + let(:project) { create(:empty_project, :public, :access_requestable, creator: user, group: group) } + let(:project_invitee) { create(:project_member, project: project, invite_token: '123', invite_email: 'test1@abc.com', user: nil) } + let(:group_invitee) { create(:group_member, group: group, invite_token: '123', invite_email: 'test2@abc.com', user: nil) } + let(:project_requester) { create(:user) } + let(:group_requester) { create(:user) } + + background do + project.team << [developer, :developer] + group.add_owner(user) + login_as(user) + end + + context 'with a group invitee' do + before do + group_invitee + visit namespace_project_project_members_path(project.namespace, project) + end + + scenario 'does not appear in the project members page' do + page.within first('.content-list') do + expect(page).not_to have_content('test2@abc.com') + end + end + end + + context 'with a group and a project invitee' do + before do + group_invitee + project_invitee + visit namespace_project_project_members_path(project.namespace, project) + end + + scenario 'shows the project invitee, the project developer, and the group owner' do + page.within first('.content-list') do + expect(page).to have_content('test1@abc.com') + expect(page).not_to have_content('test2@abc.com') + + # Project developer + expect(page).to have_content(developer.name) + + # Group owner + expect(page).to have_content(user.name) + expect(page).to have_content(group.name) + end + end + end + + context 'with a group requester' do + before do + group.request_access(group_requester) + visit namespace_project_project_members_path(project.namespace, project) + end + + scenario 'does not appear in the project members page' do + page.within first('.content-list') do + expect(page).not_to have_content(group_requester.name) + end + end + end + + context 'with a group and a project requesters' do + before do + group.request_access(group_requester) + project.request_access(project_requester) + visit namespace_project_project_members_path(project.namespace, project) + end + + scenario 'shows the project requester, the project developer, and the group owner' do + page.within first('.content-list') do + expect(page).to have_content(project_requester.name) + expect(page).not_to have_content(group_requester.name) + end + + page.within all('.content-list').last do + # Project developer + expect(page).to have_content(developer.name) + + # Group owner + expect(page).to have_content(user.name) + expect(page).to have_content(group.name) + end + end + end +end diff --git a/spec/features/projects/members/group_requester_cannot_request_access_to_project_spec.rb b/spec/features/projects/members/group_requester_cannot_request_access_to_project_spec.rb index 4973e0aee85..bdeeef57273 100644 --- a/spec/features/projects/members/group_requester_cannot_request_access_to_project_spec.rb +++ b/spec/features/projects/members/group_requester_cannot_request_access_to_project_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'Projects > Members > Group requester cannot request access to project', feature: true do +feature 'Projects > Members > Group requester cannot request access to project', feature: true, js: true do let(:user) { create(:user) } let(:owner) { create(:user) } let(:group) { create(:group, :public, :access_requestable) } diff --git a/spec/features/projects/members/member_leaves_project_spec.rb b/spec/features/projects/members/member_leaves_project_spec.rb index 79dec442818..5daa932e4e6 100644 --- a/spec/features/projects/members/member_leaves_project_spec.rb +++ b/spec/features/projects/members/member_leaves_project_spec.rb @@ -11,7 +11,7 @@ feature 'Projects > Members > Member leaves project', feature: true do end scenario 'user leaves project' do - click_link 'Leave Project' + click_link 'Leave project' expect(current_path).to eq(dashboard_projects_path) expect(project.users.exists?(user.id)).to be_falsey diff --git a/spec/features/projects/members/owner_cannot_leave_project_spec.rb b/spec/features/projects/members/owner_cannot_leave_project_spec.rb index 6e948b7a616..b26d55c5d5d 100644 --- a/spec/features/projects/members/owner_cannot_leave_project_spec.rb +++ b/spec/features/projects/members/owner_cannot_leave_project_spec.rb @@ -8,7 +8,7 @@ feature 'Projects > Members > Owner cannot leave project', feature: true do visit namespace_project_path(project.namespace, project) end - scenario 'user does not see a "Leave Project" link' do - expect(page).not_to have_content 'Leave Project' + scenario 'user does not see a "Leave project" link' do + expect(page).not_to have_content 'Leave project' end end diff --git a/spec/features/projects/settings/merge_requests_settings_spec.rb b/spec/features/projects/settings/merge_requests_settings_spec.rb new file mode 100644 index 00000000000..4bfaa499272 --- /dev/null +++ b/spec/features/projects/settings/merge_requests_settings_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +feature 'Project settings > Merge Requests', feature: true, js: true do + include GitlabRoutingHelper + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + + background do + project.team << [user, :master] + login_as(user) + end + + context 'when Merge Request and Builds are initially enabled' do + before do + project.project_feature.update_attribute('merge_requests_access_level', ProjectFeature::ENABLED) + end + + context 'when Builds are initially enabled' do + before do + project.project_feature.update_attribute('builds_access_level', ProjectFeature::ENABLED) + visit edit_project_path(project) + end + + scenario 'shows the Merge Requests settings' do + expect(page).to have_content('Only allow merge requests to be merged if the build succeeds') + expect(page).to have_content('Only allow merge requests to be merged if all discussions are resolved') + + select 'Disabled', from: "project_project_feature_attributes_merge_requests_access_level" + + expect(page).not_to have_content('Only allow merge requests to be merged if the build succeeds') + expect(page).not_to have_content('Only allow merge requests to be merged if all discussions are resolved') + end + end + + context 'when Builds are initially disabled' do + before do + project.project_feature.update_attribute('builds_access_level', ProjectFeature::DISABLED) + visit edit_project_path(project) + end + + scenario 'shows the Merge Requests settings that do not depend on Builds feature' do + expect(page).not_to have_content('Only allow merge requests to be merged if the build succeeds') + expect(page).to have_content('Only allow merge requests to be merged if all discussions are resolved') + + select 'Everyone with access', from: "project_project_feature_attributes_builds_access_level" + + expect(page).to have_content('Only allow merge requests to be merged if the build succeeds') + expect(page).to have_content('Only allow merge requests to be merged if all discussions are resolved') + end + end + end + + context 'when Merge Request are initially disabled' do + before do + project.project_feature.update_attribute('merge_requests_access_level', ProjectFeature::DISABLED) + visit edit_project_path(project) + end + + scenario 'does not show the Merge Requests settings' do + expect(page).not_to have_content('Only allow merge requests to be merged if the build succeeds') + expect(page).not_to have_content('Only allow merge requests to be merged if all discussions are resolved') + + select 'Everyone with access', from: "project_project_feature_attributes_merge_requests_access_level" + + expect(page).to have_content('Only allow merge requests to be merged if the build succeeds') + expect(page).to have_content('Only allow merge requests to be merged if all discussions are resolved') + end + end +end diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index 290ddb4c6dd..f52e23f9433 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe "Private Project Access", feature: true do include AccessMatchers - let(:project) { create(:project, :private) } + let(:project) { create(:project, :private, public_builds: false) } describe "Project should be private" do describe '#private?' do @@ -260,6 +260,18 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } + + context 'when public builds is enabled' do + before do + project.update(public_builds: true) + end + + it { is_expected.to be_allowed_for(:guest).of(project) } + end + + context 'when public buils are disabled' do + it { is_expected.to be_denied_for(:guest).of(project) } + end end describe "GET /:project_path/pipelines/:id" do @@ -275,6 +287,18 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } + + context 'when public builds is enabled' do + before do + project.update(public_builds: true) + end + + it { is_expected.to be_allowed_for(:guest).of(project) } + end + + context 'when public buils are disabled' do + it { is_expected.to be_denied_for(:guest).of(project) } + end end describe "GET /:project_path/builds" do @@ -289,6 +313,18 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } + + context 'when public builds is enabled' do + before do + project.update(public_builds: true) + end + + it { is_expected.to be_allowed_for(:guest).of(project) } + end + + context 'when public buils are disabled' do + it { is_expected.to be_denied_for(:guest).of(project) } + end end describe "GET /:project_path/builds/:id" do @@ -305,6 +341,23 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } + + context 'when public builds is enabled' do + before do + project.update(public_builds: true) + end + + it { is_expected.to be_allowed_for(:guest).of(project) } + end + + context 'when public buils are disabled' do + before do + project.public_builds = false + project.save + end + + it { is_expected.to be_denied_for(:guest).of(project) } + end end describe "GET /:project_path/environments" do diff --git a/spec/features/u2f_spec.rb b/spec/features/u2f_spec.rb index b750f27ea72..be21b403084 100644 --- a/spec/features/u2f_spec.rb +++ b/spec/features/u2f_spec.rb @@ -163,8 +163,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: click_on "Sign in via U2F device" expect(page.body).to match('We heard back from your U2F device') click_on "Authenticate via U2F Device" - - expect(page.body).to match('Signed in successfully') + expect(page.body).to match('href="/users/sign_out"') end end @@ -178,7 +177,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: expect(page.body).to match('We heard back from your U2F device') click_on "Authenticate via U2F Device" - expect(page.body).to match('Signed in successfully') + expect(page.body).to match('href="/users/sign_out"') end end @@ -234,7 +233,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: expect(page.body).to match('We heard back from your U2F device') click_on "Authenticate via U2F Device" - expect(page.body).to match('Signed in successfully') + expect(page.body).to match('href="/users/sign_out"') end end end @@ -275,7 +274,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: expect(page.body).to match('We heard back from your U2F device') click_on "Authenticate via U2F Device" - expect(page.body).to match('Signed in successfully') + expect(page.body).to match('href="/users/sign_out"') logout end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 535aabfc18d..88361e27102 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -6,14 +6,17 @@ describe MergeRequestsFinder do let(:project1) { create(:project) } let(:project2) { create(:project, forked_from_project: project1) } + let(:project3) { create(:project, forked_from_project: project1, archived: true) } let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) } let!(:merge_request2) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1, state: 'closed') } let!(:merge_request3) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project2) } + let!(:merge_request4) { create(:merge_request, :simple, author: user, source_project: project3, target_project: project3) } before do project1.team << [user, :master] project2.team << [user, :developer] + project3.team << [user, :developer] project2.team << [user2, :developer] end @@ -21,7 +24,7 @@ describe MergeRequestsFinder do it 'filters by scope' do params = { scope: 'authored', state: 'opened' } merge_requests = MergeRequestsFinder.new(user, params).execute - expect(merge_requests.size).to eq(2) + expect(merge_requests.size).to eq(3) end it 'filters by project' do @@ -29,5 +32,11 @@ describe MergeRequestsFinder do merge_requests = MergeRequestsFinder.new(user, params).execute expect(merge_requests.size).to eq(1) end + + it 'filters by non_archived' do + params = { non_archived: true } + merge_requests = MergeRequestsFinder.new(user, params).execute + expect(merge_requests.size).to eq(3) + end end end diff --git a/spec/finders/snippets_finder_spec.rb b/spec/finders/snippets_finder_spec.rb index 28bdc18e840..4427443208a 100644 --- a/spec/finders/snippets_finder_spec.rb +++ b/spec/finders/snippets_finder_spec.rb @@ -9,65 +9,74 @@ describe SnippetsFinder do let(:project2) { create(:empty_project, :private, group: group) } context ':all filter' do - before do - @snippet1 = create(:personal_snippet, :private) - @snippet2 = create(:personal_snippet, :internal) - @snippet3 = create(:personal_snippet, :public) - end + let!(:snippet1) { create(:personal_snippet, :private) } + let!(:snippet2) { create(:personal_snippet, :internal) } + let!(:snippet3) { create(:personal_snippet, :public) } it "returns all private and internal snippets" do snippets = SnippetsFinder.new.execute(user, filter: :all) - expect(snippets).to include(@snippet2, @snippet3) - expect(snippets).not_to include(@snippet1) + expect(snippets).to include(snippet2, snippet3) + expect(snippets).not_to include(snippet1) end it "returns all public snippets" do snippets = SnippetsFinder.new.execute(nil, filter: :all) - expect(snippets).to include(@snippet3) - expect(snippets).not_to include(@snippet1, @snippet2) + expect(snippets).to include(snippet3) + expect(snippets).not_to include(snippet1, snippet2) end end - context ':by_user filter' do - before do - @snippet1 = create(:personal_snippet, :private, author: user) - @snippet2 = create(:personal_snippet, :internal, author: user) - @snippet3 = create(:personal_snippet, :public, author: user) + context ':public filter' do + let!(:snippet1) { create(:personal_snippet, :private) } + let!(:snippet2) { create(:personal_snippet, :internal) } + let!(:snippet3) { create(:personal_snippet, :public) } + + it "returns public public snippets" do + snippets = SnippetsFinder.new.execute(nil, filter: :public) + + expect(snippets).to include(snippet3) + expect(snippets).not_to include(snippet1, snippet2) end + end + + context ':by_user filter' do + let!(:snippet1) { create(:personal_snippet, :private, author: user) } + let!(:snippet2) { create(:personal_snippet, :internal, author: user) } + let!(:snippet3) { create(:personal_snippet, :public, author: user) } it "returns all public and internal snippets" do snippets = SnippetsFinder.new.execute(user1, filter: :by_user, user: user) - expect(snippets).to include(@snippet2, @snippet3) - expect(snippets).not_to include(@snippet1) + expect(snippets).to include(snippet2, snippet3) + expect(snippets).not_to include(snippet1) end it "returns internal snippets" do snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user, scope: "are_internal") - expect(snippets).to include(@snippet2) - expect(snippets).not_to include(@snippet1, @snippet3) + expect(snippets).to include(snippet2) + expect(snippets).not_to include(snippet1, snippet3) end it "returns private snippets" do snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user, scope: "are_private") - expect(snippets).to include(@snippet1) - expect(snippets).not_to include(@snippet2, @snippet3) + expect(snippets).to include(snippet1) + expect(snippets).not_to include(snippet2, snippet3) end it "returns public snippets" do snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user, scope: "are_public") - expect(snippets).to include(@snippet3) - expect(snippets).not_to include(@snippet1, @snippet2) + expect(snippets).to include(snippet3) + expect(snippets).not_to include(snippet1, snippet2) end it "returns all snippets" do snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user) - expect(snippets).to include(@snippet1, @snippet2, @snippet3) + expect(snippets).to include(snippet1, snippet2, snippet3) end it "returns only public snippets if unauthenticated user" do snippets = SnippetsFinder.new.execute(nil, filter: :by_user, user: user) - expect(snippets).to include(@snippet3) - expect(snippets).not_to include(@snippet2, @snippet1) + expect(snippets).to include(snippet3) + expect(snippets).not_to include(snippet2, snippet1) end end diff --git a/spec/fixtures/api/schemas/user/login.json b/spec/fixtures/api/schemas/user/login.json new file mode 100644 index 00000000000..e6c1d9c9d84 --- /dev/null +++ b/spec/fixtures/api/schemas/user/login.json @@ -0,0 +1,37 @@ +{ + "type": "object", + "required": [ + "id", + "username", + "email", + "name", + "state", + "avatar_url", + "web_url", + "created_at", + "is_admin", + "bio", + "location", + "skype", + "linkedin", + "twitter", + "website_url", + "organization", + "last_sign_in_at", + "confirmed_at", + "theme_id", + "color_scheme_id", + "projects_limit", + "current_sign_in_at", + "identities", + "can_create_group", + "can_create_project", + "two_factor_enabled", + "external", + "private_token" + ], + "properties": { + "$ref": "full.json", + "private_token": { "type": "string" } + } +} diff --git a/spec/fixtures/api/schemas/user/public.json b/spec/fixtures/api/schemas/user/public.json new file mode 100644 index 00000000000..dbd5d32e89c --- /dev/null +++ b/spec/fixtures/api/schemas/user/public.json @@ -0,0 +1,79 @@ +{ + "type": "object", + "required": [ + "id", + "username", + "email", + "name", + "state", + "avatar_url", + "web_url", + "created_at", + "is_admin", + "bio", + "location", + "skype", + "linkedin", + "twitter", + "website_url", + "organization", + "last_sign_in_at", + "confirmed_at", + "theme_id", + "color_scheme_id", + "projects_limit", + "current_sign_in_at", + "identities", + "can_create_group", + "can_create_project", + "two_factor_enabled", + "external" + ], + "properties": { + "id": { "type": "integer" }, + "username": { "type": "string" }, + "email": { + "type": "string", + "pattern": "^[^@]+@[^@]+$" + }, + "name": { "type": "string" }, + "state": { + "type": "string", + "enum": ["active", "blocked"] + }, + "avatar_url": { "type": "string" }, + "web_url": { "type": "string" }, + "created_at": { "type": "date" }, + "is_admin": { "type": "boolean" }, + "bio": { "type": ["string", "null"] }, + "location": { "type": ["string", "null"] }, + "skype": { "type": "string" }, + "linkedin": { "type": "string" }, + "twitter": { "type": "string "}, + "website_url": { "type": "string" }, + "organization": { "type": ["string", "null"] }, + "last_sign_in_at": { "type": "date" }, + "confirmed_at": { "type": ["date", "null"] }, + "theme_id": { "type": "integer" }, + "color_scheme_id": { "type": "integer" }, + "projects_limit": { "type": "integer" }, + "current_sign_in_at": { "type": "date" }, + "identities": { + "type": "array", + "items": { + "type": "object", + "properties": { + "provider": { + "type": "string", + "enum": ["github", "bitbucket", "google_oauth2"] + }, + "extern_uid": { "type": ["number", "string"] } + } + } + }, + "can_create_group": { "type": "boolean" }, + "can_create_project": { "type": "boolean" }, + "two_factor_enabled": { "type": "boolean" }, + "external": { "type": "boolean" } + } +} diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 837e7afa7e8..468bcc7badc 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -60,15 +60,58 @@ describe DiffHelper do end describe '#diff_line_content' do - it 'returns non breaking space when line is empty' do - expect(diff_line_content(nil)).to eq(' ') - end - - it 'returns the line itself' do - expect(diff_line_content(diff_file.diff_lines.first.text)). - to eq('@@ -6,12 +6,18 @@ module Popen') - expect(diff_line_content(diff_file.diff_lines.first.type)).to eq('match') - expect(diff_file.diff_lines.first.new_pos).to eq(6) + context 'when the line is empty' do + it 'returns a non breaking space' do + expect(diff_line_content(nil)).to eq(' ') + end + + it 'returns an HTML-safe string' do + expect(diff_line_content(nil)).to be_html_safe + end + end + + context 'when the line is not empty' do + context 'when the line starts with +, -, or a space' do + it 'strips the first character' do + expect(diff_line_content('+new line')).to eq('new line') + expect(diff_line_content('-new line')).to eq('new line') + expect(diff_line_content(' new line')).to eq('new line') + end + + context 'when the line is HTML-safe' do + it 'returns an HTML-safe string' do + expect(diff_line_content('+new line'.html_safe)).to be_html_safe + expect(diff_line_content('-new line'.html_safe)).to be_html_safe + expect(diff_line_content(' new line'.html_safe)).to be_html_safe + end + end + + context 'when the line is not HTML-safe' do + it 'returns a non-HTML-safe string' do + expect(diff_line_content('+new line')).not_to be_html_safe + expect(diff_line_content('-new line')).not_to be_html_safe + expect(diff_line_content(' new line')).not_to be_html_safe + end + end + end + + context 'when the line does not start with a +, -, or a space' do + it 'returns the string' do + expect(diff_line_content('@@ -6,12 +6,18 @@ module Popen')).to eq('@@ -6,12 +6,18 @@ module Popen') + end + + context 'when the line is HTML-safe' do + it 'returns an HTML-safe string' do + expect(diff_line_content('@@ -6,12 +6,18 @@ module Popen'.html_safe)).to be_html_safe + end + end + + context 'when the line is not HTML-safe' do + it 'returns a non-HTML-safe string' do + expect(diff_line_content('@@ -6,12 +6,18 @@ module Popen')).not_to be_html_safe + end + end + end end end diff --git a/spec/javascripts/environments/environment_actions_spec.js.es6 b/spec/javascripts/environments/environment_actions_spec.js.es6 index 76e81233e89..4bae3f30bb5 100644 --- a/spec/javascripts/environments/environment_actions_spec.js.es6 +++ b/spec/javascripts/environments/environment_actions_spec.js.es6 @@ -8,7 +8,7 @@ describe('Actions Component', () => { fixture.load('environments/element.html'); }); - it('Should render a dropdown with the provided actions', () => { + it('should render a dropdown with the provided actions', () => { const actionsMock = [ { name: 'bar', @@ -24,6 +24,7 @@ describe('Actions Component', () => { el: document.querySelector('.test-dom-element'), propsData: { actions: actionsMock, + playIconSvg: '<svg></svg>', }, }); @@ -34,4 +35,33 @@ describe('Actions Component', () => { component.$el.querySelector('.dropdown-menu li a').getAttribute('href'), ).toEqual(actionsMock[0].play_path); }); + + it('should render a dropdown with the provided svg', () => { + const actionsMock = [ + { + name: 'bar', + play_path: 'https://gitlab.com/play', + }, + { + name: 'foo', + play_path: '#', + }, + ]; + + const component = new window.gl.environmentsList.ActionsComponent({ + el: document.querySelector('.test-dom-element'), + propsData: { + actions: actionsMock, + playIconSvg: '<svg></svg>', + }, + }); + + expect( + component.$el.querySelector('.js-dropdown-play-icon-container').children, + ).toContain('svg'); + + expect( + component.$el.querySelector('.js-action-play-icon-container').children, + ).toContain('svg'); + }); }); diff --git a/spec/javascripts/environments/environment_external_url_spec.js.es6 b/spec/javascripts/environments/environment_external_url_spec.js.es6 index 156506ef28f..9f82567c35b 100644 --- a/spec/javascripts/environments/environment_external_url_spec.js.es6 +++ b/spec/javascripts/environments/environment_external_url_spec.js.es6 @@ -7,12 +7,12 @@ describe('External URL Component', () => { fixture.load('environments/element.html'); }); - it('should link to the provided external_url', () => { + it('should link to the provided externalUrl prop', () => { const externalURL = 'https://gitlab.com'; const component = new window.gl.environmentsList.ExternalUrlComponent({ el: document.querySelector('.test-dom-element'), propsData: { - external_url: externalURL, + externalUrl: externalURL, }, }); diff --git a/spec/javascripts/environments/environment_rollback_spec.js.es6 b/spec/javascripts/environments/environment_rollback_spec.js.es6 index 29449bbbd9e..77ba0ab38ec 100644 --- a/spec/javascripts/environments/environment_rollback_spec.js.es6 +++ b/spec/javascripts/environments/environment_rollback_spec.js.es6 @@ -9,24 +9,24 @@ describe('Rollback Component', () => { fixture.load('environments/element.html'); }); - it('Should link to the provided retry_url', () => { + it('Should link to the provided retryUrl', () => { const component = new window.gl.environmentsList.RollbackComponent({ el: document.querySelector('.test-dom-element'), propsData: { - retry_url: retryURL, - is_last_deployment: true, + retryUrl: retryURL, + isLastDeployment: true, }, }); expect(component.$el.getAttribute('href')).toEqual(retryURL); }); - it('Should render Re-deploy label when is_last_deployment is true', () => { + it('Should render Re-deploy label when isLastDeployment is true', () => { const component = new window.gl.environmentsList.RollbackComponent({ el: document.querySelector('.test-dom-element'), propsData: { - retry_url: retryURL, - is_last_deployment: true, + retryUrl: retryURL, + isLastDeployment: true, }, }); @@ -34,12 +34,12 @@ describe('Rollback Component', () => { }); - it('Should render Rollback label when is_last_deployment is false', () => { + it('Should render Rollback label when isLastDeployment is false', () => { const component = new window.gl.environmentsList.RollbackComponent({ el: document.querySelector('.test-dom-element'), propsData: { - retry_url: retryURL, - is_last_deployment: false, + retryUrl: retryURL, + isLastDeployment: false, }, }); diff --git a/spec/javascripts/environments/environment_stop_spec.js.es6 b/spec/javascripts/environments/environment_stop_spec.js.es6 index b842be4da61..84a41b2bf46 100644 --- a/spec/javascripts/environments/environment_stop_spec.js.es6 +++ b/spec/javascripts/environments/environment_stop_spec.js.es6 @@ -13,7 +13,7 @@ describe('Stop Component', () => { component = new window.gl.environmentsList.StopComponent({ el: document.querySelector('.test-dom-element'), propsData: { - stop_url: stopURL, + stopUrl: stopURL, }, }); }); diff --git a/spec/javascripts/fixtures/event_filter.html.haml b/spec/javascripts/fixtures/event_filter.html.haml index 95e248cadf8..5477c6075f0 100644 --- a/spec/javascripts/fixtures/event_filter.html.haml +++ b/spec/javascripts/fixtures/event_filter.html.haml @@ -12,6 +12,10 @@ %span Merge events %li + %a.event-filter-link{ id: "issue_event_filter", title: "Filter by issue events", href: "/dashboard/activity"} + %span + Issue events + %li %a.event-filter-link{ id: "comments_event_filter", title: "Filter by comments", href: "/dashboard/activity"} %span Comments diff --git a/spec/javascripts/fixtures/signin_tabs.html.haml b/spec/javascripts/fixtures/signin_tabs.html.haml new file mode 100644 index 00000000000..12b8d423cbe --- /dev/null +++ b/spec/javascripts/fixtures/signin_tabs.html.haml @@ -0,0 +1,5 @@ +%ul.nav-tabs + %li + %a.active{ id: 'standard', href: '#standard'} Standard + %li + %a{ id: 'ldap', href: '#ldap'} Ldap diff --git a/spec/javascripts/merge_request_widget_spec.js b/spec/javascripts/merge_request_widget_spec.js index 62890f1ca96..6f91529db00 100644 --- a/spec/javascripts/merge_request_widget_spec.js +++ b/spec/javascripts/merge_request_widget_spec.js @@ -106,6 +106,18 @@ }); }); + describe('mergeInProgress', function() { + it('should display error with h4 tag', function() { + spyOn(this.class.$widgetBody, 'html').and.callFake(function(html) { + expect(html).toBe('<h4>Sorry, something went wrong.</h4>'); + }); + spyOn($, 'ajax').and.callFake(function(e) { + e.success({ merge_error: 'Sorry, something went wrong.' }); + }); + this.class.mergeInProgress(null); + }); + }); + return describe('getCIStatus', function() { beforeEach(function() { this.ciStatusData = { diff --git a/spec/javascripts/signin_tabs_memoizer_spec.js.es6 b/spec/javascripts/signin_tabs_memoizer_spec.js.es6 new file mode 100644 index 00000000000..9a9fb22255b --- /dev/null +++ b/spec/javascripts/signin_tabs_memoizer_spec.js.es6 @@ -0,0 +1,53 @@ +/*= require signin_tabs_memoizer */ + +((global) => { + describe('SigninTabsMemoizer', () => { + const fixtureTemplate = 'signin_tabs.html'; + const tabSelector = 'ul.nav-tabs'; + const currentTabKey = 'current_signin_tab'; + let memo; + + function createMemoizer() { + memo = new global.ActiveTabMemoizer({ + currentTabKey, + tabSelector, + }); + return memo; + } + + fixture.preload(fixtureTemplate); + + beforeEach(() => { + fixture.load(fixtureTemplate); + }); + + it('does nothing if no tab was previously selected', () => { + createMemoizer(); + + expect(document.querySelector('li a.active').getAttribute('id')).toEqual('standard'); + }); + + it('shows last selected tab on boot', () => { + createMemoizer().saveData('#ldap'); + const fakeTab = { + click: () => {}, + }; + spyOn(document, 'querySelector').and.returnValue(fakeTab); + spyOn(fakeTab, 'click'); + + memo.bootstrap(); + + // verify that triggers click on the last selected tab + expect(document.querySelector).toHaveBeenCalledWith(`${tabSelector} a[href="#ldap"]`); + expect(fakeTab.click).toHaveBeenCalled(); + }); + + it('saves last selected tab on change', () => { + createMemoizer(); + + document.getElementById('standard').click(); + + expect(memo.readData()).toEqual('#standard'); + }); + }); +})(window); diff --git a/spec/javascripts/smart_interval_spec.js.es6 b/spec/javascripts/smart_interval_spec.js.es6 index ed6166a25a8..1b7ca97cde4 100644 --- a/spec/javascripts/smart_interval_spec.js.es6 +++ b/spec/javascripts/smart_interval_spec.js.es6 @@ -14,8 +14,9 @@ startingInterval: DEFAULT_STARTING_INTERVAL, maxInterval: DEFAULT_MAX_INTERVAL, incrementByFactorOf: DEFAULT_INCREMENT_FACTOR, - delayStartBy: 0, lazyStart: false, + immediateExecution: false, + hiddenInterval: null, }; if (config) { @@ -114,14 +115,31 @@ expect(interval.state.intervalId).toBeTruthy(); // simulates triggering of visibilitychange event - interval.state.pageVisibility = 'hidden'; - interval.handleVisibilityChange(); + interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } }); expect(interval.state.intervalId).toBeUndefined(); done(); }, DEFAULT_SHORT_TIMEOUT); }); + it('should change to the hidden interval when page is not visible', function (done) { + const HIDDEN_INTERVAL = 1500; + const interval = createDefaultSmartInterval({ hiddenInterval: HIDDEN_INTERVAL }); + + setTimeout(() => { + expect(interval.state.intervalId).toBeTruthy(); + expect(interval.getCurrentInterval() >= DEFAULT_STARTING_INTERVAL && + interval.getCurrentInterval() <= DEFAULT_MAX_INTERVAL).toBeTruthy(); + + // simulates triggering of visibilitychange event + interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } }); + + expect(interval.state.intervalId).toBeTruthy(); + expect(interval.getCurrentInterval()).toBe(HIDDEN_INTERVAL); + done(); + }, DEFAULT_SHORT_TIMEOUT); + }); + it('should resume when page is becomes visible at the previous interval', function (done) { const interval = this.smartInterval; @@ -129,14 +147,12 @@ expect(interval.state.intervalId).toBeTruthy(); // simulates triggering of visibilitychange event - interval.state.pageVisibility = 'hidden'; - interval.handleVisibilityChange(); + interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } }); expect(interval.state.intervalId).toBeUndefined(); // simulates triggering of visibilitychange event - interval.state.pageVisibility = 'visible'; - interval.handleVisibilityChange(); + interval.handleVisibilityChange({ target: { visibilityState: 'visible' } }); expect(interval.state.intervalId).toBeTruthy(); @@ -154,6 +170,11 @@ done(); }, DEFAULT_SHORT_TIMEOUT); }); + + it('should execute callback before first interval', function () { + const interval = createDefaultSmartInterval({ immediateExecution: true }); + expect(interval.cfg.immediateExecution).toBeFalsy(); + }); }); }); })(window.gl || (window.gl = {})); diff --git a/spec/javascripts/vue_common_components/commit_spec.js.es6 b/spec/javascripts/vue_common_components/commit_spec.js.es6 index d170517dd9b..26dfdb94aae 100644 --- a/spec/javascripts/vue_common_components/commit_spec.js.es6 +++ b/spec/javascripts/vue_common_components/commit_spec.js.es6 @@ -10,12 +10,12 @@ describe('Commit component', () => { el: document.querySelector('.test-commit-container'), propsData: { tag: false, - commit_ref: { + commitRef: { name: 'master', ref_url: 'http://localhost/namespace2/gitlabhq/tree/master', }, - commit_url: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067', - short_sha: 'b7836edd', + commitUrl: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067', + shortSha: 'b7836edd', title: 'Commit message', author: { avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png', @@ -34,18 +34,19 @@ describe('Commit component', () => { props = { tag: true, - commit_ref: { + commitRef: { name: 'master', ref_url: 'http://localhost/namespace2/gitlabhq/tree/master', }, - commit_url: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067', - short_sha: 'b7836edd', + commitUrl: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067', + shortSha: 'b7836edd', title: 'Commit message', author: { avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png', web_url: 'https://gitlab.com/jschatz1', username: 'jschatz1', }, + commitIconSvg: '<svg></svg>', }; component = new window.gl.CommitComponent({ @@ -59,20 +60,24 @@ describe('Commit component', () => { }); it('should render a link to the ref url', () => { - expect(component.$el.querySelector('.branch-name').getAttribute('href')).toEqual(props.commit_ref.ref_url); + expect(component.$el.querySelector('.branch-name').getAttribute('href')).toEqual(props.commitRef.ref_url); }); it('should render the ref name', () => { - expect(component.$el.querySelector('.branch-name').textContent).toContain(props.commit_ref.name); + expect(component.$el.querySelector('.branch-name').textContent).toContain(props.commitRef.name); }); it('should render the commit short sha with a link to the commit url', () => { - expect(component.$el.querySelector('.commit-id').getAttribute('href')).toEqual(props.commit_url); - expect(component.$el.querySelector('.commit-id').textContent).toContain(props.short_sha); + expect(component.$el.querySelector('.commit-id').getAttribute('href')).toEqual(props.commitUrl); + expect(component.$el.querySelector('.commit-id').textContent).toContain(props.shortSha); + }); + + it('should render the given commitIconSvg', () => { + expect(component.$el.querySelector('.js-commit-icon').children).toContain('svg'); }); describe('Given commit title and author props', () => { - it('Should render a link to the author profile', () => { + it('should render a link to the author profile', () => { expect( component.$el.querySelector('.commit-title .avatar-image-container').getAttribute('href'), ).toEqual(props.author.web_url); @@ -91,7 +96,7 @@ describe('Commit component', () => { it('should render the commit title', () => { expect( component.$el.querySelector('a.commit-row-message').getAttribute('href'), - ).toEqual(props.commit_url); + ).toEqual(props.commitUrl); expect( component.$el.querySelector('a.commit-row-message').textContent, ).toContain(props.title); @@ -99,16 +104,16 @@ describe('Commit component', () => { }); describe('When commit title is not provided', () => { - it('Should render default message', () => { + it('should render default message', () => { fixture.set('<div class="test-commit-container"></div>'); props = { tag: false, - commit_ref: { + commitRef: { name: 'master', ref_url: 'http://localhost/namespace2/gitlabhq/tree/master', }, - commit_url: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067', - short_sha: 'b7836edd', + commitUrl: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067', + shortSha: 'b7836edd', title: null, author: {}, }; diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb index 2bfa51deb20..df2dd173b57 100644 --- a/spec/lib/banzai/filter/relative_link_filter_spec.rb +++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb @@ -175,7 +175,7 @@ describe Banzai::Filter::RelativeLinkFilter, lib: true do allow_any_instance_of(described_class).to receive(:uri_type).and_return(:raw) doc = filter(image(escaped)) - expect(doc.at_css('img')['src']).to match '/raw/' + expect(doc.at_css('img')['src']).to eq "/#{project_path}/raw/#{Addressable::URI.escape(ref)}/#{escaped}" end context 'when requested path is a file in the repo' do diff --git a/spec/lib/constraints/group_url_constrainer_spec.rb b/spec/lib/constraints/group_url_constrainer_spec.rb index 892554f2870..96dacdc5cd2 100644 --- a/spec/lib/constraints/group_url_constrainer_spec.rb +++ b/spec/lib/constraints/group_url_constrainer_spec.rb @@ -10,6 +10,13 @@ describe GroupUrlConstrainer, lib: true do it { expect(subject.matches?(request)).to be_truthy } end + context 'valid request for nested group' do + let!(:nested_group) { create(:group, path: 'nested', parent: group) } + let!(:request) { build_request('gitlab/nested') } + + it { expect(subject.matches?(request)).to be_truthy } + end + context 'invalid request' do let(:request) { build_request('foo') } diff --git a/spec/lib/event_filter_spec.rb b/spec/lib/event_filter_spec.rb index a6d8e6927e0..ec2f66b1136 100644 --- a/spec/lib/event_filter_spec.rb +++ b/spec/lib/event_filter_spec.rb @@ -7,6 +7,10 @@ describe EventFilter, lib: true do let!(:push_event) { create(:event, action: Event::PUSHED, project: public_project, target: public_project, author: source_user) } let!(:merged_event) { create(:event, action: Event::MERGED, project: public_project, target: public_project, author: source_user) } + let!(:created_event) { create(:event, action: Event::CREATED, project: public_project, target: public_project, author: source_user) } + let!(:updated_event) { create(:event, action: Event::UPDATED, project: public_project, target: public_project, author: source_user) } + let!(:closed_event) { create(:event, action: Event::CLOSED, project: public_project, target: public_project, author: source_user) } + let!(:reopened_event) { create(:event, action: Event::REOPENED, project: public_project, target: public_project, author: source_user) } let!(:comments_event) { create(:event, action: Event::COMMENTED, project: public_project, target: public_project, author: source_user) } let!(:joined_event) { create(:event, action: Event::JOINED, project: public_project, target: public_project, author: source_user) } let!(:left_event) { create(:event, action: Event::LEFT, project: public_project, target: public_project, author: source_user) } @@ -21,6 +25,11 @@ describe EventFilter, lib: true do expect(events).to contain_exactly(merged_event) end + it 'applies issue filter' do + events = EventFilter.new(EventFilter.issue).apply_filter(Event.all) + expect(events).to contain_exactly(created_event, updated_event, closed_event, reopened_event) + end + it 'applies comments filter' do events = EventFilter.new(EventFilter.comments).apply_filter(Event.all) expect(events).to contain_exactly(comments_event) @@ -33,17 +42,17 @@ describe EventFilter, lib: true do it 'applies all filter' do events = EventFilter.new(EventFilter.all).apply_filter(Event.all) - expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event) + expect(events).to contain_exactly(push_event, merged_event, created_event, updated_event, closed_event, reopened_event, comments_event, joined_event, left_event) end it 'applies no filter' do events = EventFilter.new(nil).apply_filter(Event.all) - expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event) + expect(events).to contain_exactly(push_event, merged_event, created_event, updated_event, closed_event, reopened_event, comments_event, joined_event, left_event) end it 'applies unknown filter' do events = EventFilter.new('').apply_filter(Event.all) - expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event) + expect(events).to contain_exactly(push_event, merged_event, created_event, updated_event, closed_event, reopened_event, comments_event, joined_event, left_event) end end end diff --git a/spec/lib/gitlab/backup/manager_spec.rb b/spec/lib/gitlab/backup/manager_spec.rb new file mode 100644 index 00000000000..1b749d1bd39 --- /dev/null +++ b/spec/lib/gitlab/backup/manager_spec.rb @@ -0,0 +1,127 @@ +require 'spec_helper' + +describe Backup::Manager, lib: true do + describe '#remove_old' do + let(:progress) { StringIO.new } + + let(:files) do + [ + '1451606400_2016_01_01_gitlab_backup.tar', + '1451520000_2015_12_31_gitlab_backup.tar', + '1450742400_2015_12_22_gitlab_backup.tar', + '1449878400_gitlab_backup.tar', + '1449014400_gitlab_backup.tar', + 'manual_gitlab_backup.tar' + ] + end + + before do + allow(Dir).to receive(:chdir).and_yield + allow(Dir).to receive(:glob).and_return(files) + allow(FileUtils).to receive(:rm) + allow(Time).to receive(:now).and_return(Time.utc(2016)) + + allow(progress).to receive(:puts) + allow(progress).to receive(:print) + + allow_any_instance_of(String).to receive(:color) do |string, _color| + string + end + + @old_progress = $progress # rubocop:disable Style/GlobalVars + $progress = progress # rubocop:disable Style/GlobalVars + end + + after do + $progress = @old_progress # rubocop:disable Style/GlobalVars + end + + context 'when keep_time is zero' do + before do + allow(Gitlab.config.backup).to receive(:keep_time).and_return(0) + + subject.remove_old + end + + it 'removes no files' do + expect(FileUtils).not_to have_received(:rm) + end + + it 'prints a skipped message' do + expect(progress).to have_received(:puts).with('skipping') + end + end + + context 'when there are no files older than keep_time' do + before do + allow(Gitlab.config.backup).to receive(:keep_time).and_return(2592000) + + subject.remove_old + end + + it 'removes no files' do + expect(FileUtils).not_to have_received(:rm) + end + + it 'prints a done message' do + expect(progress).to have_received(:puts).with('done. (0 removed)') + end + end + + context 'when keep_time is set to remove files' do + before do + allow(Gitlab.config.backup).to receive(:keep_time).and_return(1) + + subject.remove_old + end + + it 'removes matching files with a human-readable timestamp' do + expect(FileUtils).to have_received(:rm).with(files[1]) + expect(FileUtils).to have_received(:rm).with(files[2]) + end + + it 'removes matching files without a human-readable timestamp' do + expect(FileUtils).to have_received(:rm).with(files[3]) + expect(FileUtils).to have_received(:rm).with(files[4]) + end + + it 'does not remove files that are not old enough' do + expect(FileUtils).not_to have_received(:rm).with(files[0]) + end + + it 'does not remove non-matching files' do + expect(FileUtils).not_to have_received(:rm).with(files[5]) + end + + it 'prints a done message' do + expect(progress).to have_received(:puts).with('done. (4 removed)') + end + end + + context 'when removing a file fails' do + let(:file) { files[1] } + let(:message) { "Permission denied @ unlink_internal - #{file}" } + + before do + allow(Gitlab.config.backup).to receive(:keep_time).and_return(1) + allow(FileUtils).to receive(:rm).with(file).and_raise(Errno::EACCES, message) + + subject.remove_old + end + + it 'removes the remaining expected files' do + expect(FileUtils).to have_received(:rm).with(files[2]) + expect(FileUtils).to have_received(:rm).with(files[3]) + expect(FileUtils).to have_received(:rm).with(files[4]) + end + + it 'sets the correct removed count' do + expect(progress).to have_received(:puts).with('done. (3 removed)') + end + + it 'prints the error from file that could not be removed' do + expect(progress).to have_received(:puts).with(a_string_matching(message)) + end + end + end +end diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb new file mode 100644 index 00000000000..619ecbcba67 --- /dev/null +++ b/spec/lib/gitlab/ci/status/canceled_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Canceled do + subject { described_class.new(double('subject')) } + + describe '#text' do + it { expect(subject.label).to eq 'canceled' } + end + + describe '#label' do + it { expect(subject.label).to eq 'canceled' } + end + + describe '#icon' do + it { expect(subject.icon).to eq 'icon_status_canceled' } + end + + describe '#title' do + it { expect(subject.title).to eq 'Double: canceled' } + end +end diff --git a/spec/lib/gitlab/ci/status/created_spec.rb b/spec/lib/gitlab/ci/status/created_spec.rb new file mode 100644 index 00000000000..157302c65a8 --- /dev/null +++ b/spec/lib/gitlab/ci/status/created_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Created do + subject { described_class.new(double('subject')) } + + describe '#text' do + it { expect(subject.label).to eq 'created' } + end + + describe '#label' do + it { expect(subject.label).to eq 'created' } + end + + describe '#icon' do + it { expect(subject.icon).to eq 'icon_status_created' } + end + + describe '#title' do + it { expect(subject.title).to eq 'Double: created' } + end +end diff --git a/spec/lib/gitlab/ci/status/extended_spec.rb b/spec/lib/gitlab/ci/status/extended_spec.rb new file mode 100644 index 00000000000..120e121aae5 --- /dev/null +++ b/spec/lib/gitlab/ci/status/extended_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Extended do + subject do + Class.new.extend(described_class) + end + + it 'requires subclass to implement matcher' do + expect { subject.matches?(double) } + .to raise_error(NotImplementedError) + end +end diff --git a/spec/lib/gitlab/ci/status/factory_spec.rb b/spec/lib/gitlab/ci/status/factory_spec.rb new file mode 100644 index 00000000000..d5bd7f7102b --- /dev/null +++ b/spec/lib/gitlab/ci/status/factory_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Factory do + subject do + described_class.new(object) + end + + let(:status) { subject.fabricate! } + + context 'when object has a core status' do + HasStatus::AVAILABLE_STATUSES.each do |core_status| + context "when core status is #{core_status}" do + let(:object) { double(status: core_status) } + + it "fabricates a core status #{core_status}" do + expect(status).to be_a( + Gitlab::Ci::Status.const_get(core_status.capitalize)) + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/status/failed_spec.rb b/spec/lib/gitlab/ci/status/failed_spec.rb new file mode 100644 index 00000000000..0b3cb8168e6 --- /dev/null +++ b/spec/lib/gitlab/ci/status/failed_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Failed do + subject { described_class.new(double('subject')) } + + describe '#text' do + it { expect(subject.label).to eq 'failed' } + end + + describe '#label' do + it { expect(subject.label).to eq 'failed' } + end + + describe '#icon' do + it { expect(subject.icon).to eq 'icon_status_failed' } + end + + describe '#title' do + it { expect(subject.title).to eq 'Double: failed' } + end +end diff --git a/spec/lib/gitlab/ci/status/pending_spec.rb b/spec/lib/gitlab/ci/status/pending_spec.rb new file mode 100644 index 00000000000..57c901c1202 --- /dev/null +++ b/spec/lib/gitlab/ci/status/pending_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Pending do + subject { described_class.new(double('subject')) } + + describe '#text' do + it { expect(subject.label).to eq 'pending' } + end + + describe '#label' do + it { expect(subject.label).to eq 'pending' } + end + + describe '#icon' do + it { expect(subject.icon).to eq 'icon_status_pending' } + end + + describe '#title' do + it { expect(subject.title).to eq 'Double: pending' } + end +end diff --git a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb new file mode 100644 index 00000000000..21adee3f8e7 --- /dev/null +++ b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Pipeline::Common do + let(:pipeline) { create(:ci_pipeline) } + + subject do + Class.new(Gitlab::Ci::Status::Core) + .new(pipeline).extend(described_class) + end + + it 'does not have action' do + expect(subject).not_to have_action + end + + it 'has details' do + expect(subject).to have_details + end + + it 'links to the pipeline details page' do + expect(subject.details_path) + .to include "pipelines/#{pipeline.id}" + end +end diff --git a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb new file mode 100644 index 00000000000..d6243940f2e --- /dev/null +++ b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Pipeline::Factory do + subject do + described_class.new(pipeline) + end + + let(:status) do + subject.fabricate! + end + + context 'when pipeline has a core status' do + HasStatus::AVAILABLE_STATUSES.each do |core_status| + context "when core status is #{core_status}" do + let(:pipeline) do + create(:ci_pipeline, status: core_status) + end + + it "fabricates a core status #{core_status}" do + expect(status).to be_a( + Gitlab::Ci::Status.const_get(core_status.capitalize)) + end + + it 'extends core status with common pipeline methods' do + expect(status).to have_details + expect(status).not_to have_action + expect(status.details_path) + .to include "pipelines/#{pipeline.id}" + end + end + end + end + + context 'when pipeline has warnings' do + let(:pipeline) do + create(:ci_pipeline, status: :success) + end + + before do + create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline) + end + + it 'fabricates extended "success with warnings" status' do + expect(status) + .to be_a Gitlab::Ci::Status::Pipeline::SuccessWithWarnings + end + + it 'extends core status with common pipeline methods' do + expect(status).to have_details + end + end +end diff --git a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb new file mode 100644 index 00000000000..02e526e3de2 --- /dev/null +++ b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb @@ -0,0 +1,65 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Pipeline::SuccessWithWarnings do + subject do + described_class.new(double('status')) + end + + describe '#test' do + it { expect(subject.text).to eq 'passed' } + end + + describe '#label' do + it { expect(subject.label).to eq 'passed with warnings' } + end + + describe '#icon' do + it { expect(subject.icon).to eq 'icon_status_warning' } + end + + describe '.matches?' do + context 'when pipeline is successful' do + let(:pipeline) do + create(:ci_pipeline, status: :success) + end + + context 'when pipeline has warnings' do + before do + allow(pipeline).to receive(:has_warnings?).and_return(true) + end + + it 'is a correct match' do + expect(described_class.matches?(pipeline)).to eq true + end + end + + context 'when pipeline does not have warnings' do + it 'does not match' do + expect(described_class.matches?(pipeline)).to eq false + end + end + end + + context 'when pipeline is not successful' do + let(:pipeline) do + create(:ci_pipeline, status: :skipped) + end + + context 'when pipeline has warnings' do + before do + allow(pipeline).to receive(:has_warnings?).and_return(true) + end + + it 'does not match' do + expect(described_class.matches?(pipeline)).to eq false + end + end + + context 'when pipeline does not have warnings' do + it 'does not match' do + expect(described_class.matches?(pipeline)).to eq false + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/status/running_spec.rb b/spec/lib/gitlab/ci/status/running_spec.rb new file mode 100644 index 00000000000..c023f1872cc --- /dev/null +++ b/spec/lib/gitlab/ci/status/running_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Running do + subject { described_class.new(double('subject')) } + + describe '#text' do + it { expect(subject.label).to eq 'running' } + end + + describe '#label' do + it { expect(subject.label).to eq 'running' } + end + + describe '#icon' do + it { expect(subject.icon).to eq 'icon_status_running' } + end + + describe '#title' do + it { expect(subject.title).to eq 'Double: running' } + end +end diff --git a/spec/lib/gitlab/ci/status/skipped_spec.rb b/spec/lib/gitlab/ci/status/skipped_spec.rb new file mode 100644 index 00000000000..d4f7f4b3b70 --- /dev/null +++ b/spec/lib/gitlab/ci/status/skipped_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Skipped do + subject { described_class.new(double('subject')) } + + describe '#text' do + it { expect(subject.label).to eq 'skipped' } + end + + describe '#label' do + it { expect(subject.label).to eq 'skipped' } + end + + describe '#icon' do + it { expect(subject.icon).to eq 'icon_status_skipped' } + end + + describe '#title' do + it { expect(subject.title).to eq 'Double: skipped' } + end +end diff --git a/spec/lib/gitlab/ci/status/stage/common_spec.rb b/spec/lib/gitlab/ci/status/stage/common_spec.rb new file mode 100644 index 00000000000..f3259c6f23e --- /dev/null +++ b/spec/lib/gitlab/ci/status/stage/common_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Stage::Common do + let(:pipeline) { create(:ci_empty_pipeline) } + let(:stage) { build(:ci_stage, pipeline: pipeline, name: 'test') } + + subject do + Class.new(Gitlab::Ci::Status::Core) + .new(stage).extend(described_class) + end + + it 'does not have action' do + expect(subject).not_to have_action + end + + it 'has details' do + expect(subject).to have_details + end + + it 'links to the pipeline details page' do + expect(subject.details_path) + .to include "pipelines/#{pipeline.id}" + expect(subject.details_path) + .to include "##{stage.name}" + end +end diff --git a/spec/lib/gitlab/ci/status/stage/factory_spec.rb b/spec/lib/gitlab/ci/status/stage/factory_spec.rb new file mode 100644 index 00000000000..17929665c83 --- /dev/null +++ b/spec/lib/gitlab/ci/status/stage/factory_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Stage::Factory do + let(:pipeline) { create(:ci_empty_pipeline) } + let(:stage) { build(:ci_stage, pipeline: pipeline, name: 'test') } + + subject do + described_class.new(stage) + end + + let(:status) do + subject.fabricate! + end + + context 'when stage has a core status' do + HasStatus::AVAILABLE_STATUSES.each do |core_status| + context "when core status is #{core_status}" do + before do + create(:ci_build, pipeline: pipeline, stage: 'test', status: core_status) + create(:commit_status, pipeline: pipeline, stage: 'test', status: core_status) + create(:ci_build, pipeline: pipeline, stage: 'build', status: :failed) + end + + it "fabricates a core status #{core_status}" do + expect(status).to be_a( + Gitlab::Ci::Status.const_get(core_status.capitalize)) + end + + it 'extends core status with common stage methods' do + expect(status).to have_details + expect(status.details_path).to include "pipelines/#{pipeline.id}" + expect(status.details_path).to include "##{stage.name}" + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/status/success_spec.rb b/spec/lib/gitlab/ci/status/success_spec.rb new file mode 100644 index 00000000000..9e261a3aa5f --- /dev/null +++ b/spec/lib/gitlab/ci/status/success_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Success do + subject { described_class.new(double('subject')) } + + describe '#text' do + it { expect(subject.label).to eq 'passed' } + end + + describe '#label' do + it { expect(subject.label).to eq 'passed' } + end + + describe '#icon' do + it { expect(subject.icon).to eq 'icon_status_success' } + end + + describe '#title' do + it { expect(subject.title).to eq 'Double: passed' } + end +end diff --git a/spec/lib/gitlab/cycle_analytics/permissions_spec.rb b/spec/lib/gitlab/cycle_analytics/permissions_spec.rb index dc4f7dc69db..2d85e712db0 100644 --- a/spec/lib/gitlab/cycle_analytics/permissions_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/permissions_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::CycleAnalytics::Permissions do - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, public_builds: false) } let(:user) { create(:user) } subject { described_class.get(user: user, project: project) } diff --git a/spec/lib/gitlab/github_import/importer_spec.rb b/spec/lib/gitlab/github_import/importer_spec.rb index 000b9aa6f83..9e027839f59 100644 --- a/spec/lib/gitlab/github_import/importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer_spec.rb @@ -155,7 +155,7 @@ describe Gitlab::GithubImport::Importer, lib: true do message: 'The remote data could not be fully imported.', errors: [ { type: :label, url: "https://api.github.com/repos/octocat/Hello-World/labels/bug", errors: "Validation failed: Title can't be blank, Title is invalid" }, - { type: :issue, url: "https://api.github.com/repos/octocat/Hello-World/issues/1348", errors: "Validation failed: Title can't be blank, Title is too short (minimum is 0 characters)" }, + { type: :issue, url: "https://api.github.com/repos/octocat/Hello-World/issues/1348", errors: "Validation failed: Title can't be blank" }, { type: :wiki, errors: "Gitlab::Shell::Error" }, { type: :release, url: 'https://api.github.com/repos/octocat/Hello-World/releases/2', errors: "Validation failed: Description can't be blank" } ] diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 7e00e214c6e..8e1a28f2723 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -188,6 +188,7 @@ project: - project_feature - authorized_users - project_authorizations +- route award_emoji: - awardable - user diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index f23e3522625..9614aad3e73 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -40,6 +40,15 @@ describe Gitlab::SearchResults do expect(results.milestones_count).to eq(1) end end + + it 'includes merge requests from source and target projects' do + forked_project = create(:empty_project, forked_from_project: project) + merge_request_2 = create(:merge_request, target_project: project, source_project: forked_project, title: 'foo') + + results = described_class.new(user, Project.where(id: forked_project.id), 'foo') + + expect(results.objects('merge_requests')).to include merge_request_2 + end end it 'does not list issues on private projects' do @@ -152,4 +161,11 @@ describe Gitlab::SearchResults do expect(results.issues_count).to eq 5 end end + + it 'does not list merge requests on projects with limited access' do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + + expect(results.objects('merge_requests')).not_to include merge_request + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 0d2b4920835..8158e71dd55 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -20,8 +20,6 @@ describe Ci::Pipeline, models: true do it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :short_sha } - it { is_expected.to delegate_method(:stages).to(:statuses) } - describe '#valid_commit_sha' do context 'commit.sha can not start with 00000000' do before do @@ -125,16 +123,55 @@ describe Ci::Pipeline, models: true do end describe '#stages' do - let(:pipeline2) { FactoryGirl.create :ci_pipeline, project: project } - subject { CommitStatus.where(pipeline: [pipeline, pipeline2]).stages } - before do - FactoryGirl.create :ci_build, pipeline: pipeline2, stage: 'test', stage_idx: 1 - FactoryGirl.create :ci_build, pipeline: pipeline, stage: 'build', stage_idx: 0 + create(:commit_status, pipeline: pipeline, stage: 'build', name: 'linux', stage_idx: 0, status: 'success') + create(:commit_status, pipeline: pipeline, stage: 'build', name: 'mac', stage_idx: 0, status: 'failed') + create(:commit_status, pipeline: pipeline, stage: 'deploy', name: 'staging', stage_idx: 2, status: 'running') + create(:commit_status, pipeline: pipeline, stage: 'test', name: 'rspec', stage_idx: 1, status: 'success') + end + + subject { pipeline.stages } + + context 'stages list' do + it 'returns ordered list of stages' do + expect(subject.map(&:name)).to eq(%w[build test deploy]) + end + end + + it 'returns a valid number of stages' do + expect(pipeline.stages_count).to eq(3) + end + + it 'returns a valid names of stages' do + expect(pipeline.stages_name).to eq(['build', 'test', 'deploy']) end - it 'return all stages' do - is_expected.to eq(%w(build test)) + context 'stages with statuses' do + let(:statuses) do + subject.map do |stage| + [stage.name, stage.status] + end + end + + it 'returns list of stages with statuses' do + expect(statuses).to eq([['build', 'failed'], + ['test', 'success'], + ['deploy', 'running'] + ]) + end + + context 'when build is retried' do + before do + create(:commit_status, pipeline: pipeline, stage: 'build', name: 'mac', stage_idx: 0, status: 'success') + end + + it 'ignores the previous state' do + expect(statuses).to eq([['build', 'success'], + ['test', 'success'], + ['deploy', 'running'] + ]) + end + end end end @@ -404,6 +441,76 @@ describe Ci::Pipeline, models: true do end end + describe '#detailed_status' do + context 'when pipeline is created' do + let(:pipeline) { create(:ci_pipeline, status: :created) } + + it 'returns detailed status for created pipeline' do + expect(pipeline.detailed_status.text).to eq 'created' + end + end + + context 'when pipeline is pending' do + let(:pipeline) { create(:ci_pipeline, status: :pending) } + + it 'returns detailed status for pending pipeline' do + expect(pipeline.detailed_status.text).to eq 'pending' + end + end + + context 'when pipeline is running' do + let(:pipeline) { create(:ci_pipeline, status: :running) } + + it 'returns detailed status for running pipeline' do + expect(pipeline.detailed_status.text).to eq 'running' + end + end + + context 'when pipeline is successful' do + let(:pipeline) { create(:ci_pipeline, status: :success) } + + it 'returns detailed status for successful pipeline' do + expect(pipeline.detailed_status.text).to eq 'passed' + end + end + + context 'when pipeline is failed' do + let(:pipeline) { create(:ci_pipeline, status: :failed) } + + it 'returns detailed status for failed pipeline' do + expect(pipeline.detailed_status.text).to eq 'failed' + end + end + + context 'when pipeline is canceled' do + let(:pipeline) { create(:ci_pipeline, status: :canceled) } + + it 'returns detailed status for canceled pipeline' do + expect(pipeline.detailed_status.text).to eq 'canceled' + end + end + + context 'when pipeline is skipped' do + let(:pipeline) { create(:ci_pipeline, status: :skipped) } + + it 'returns detailed status for skipped pipeline' do + expect(pipeline.detailed_status.text).to eq 'skipped' + end + end + + context 'when pipeline is successful but with warnings' do + let(:pipeline) { create(:ci_pipeline, status: :success) } + + before do + create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline) + end + + it 'retruns detailed status for successful pipeline with warnings' do + expect(pipeline.detailed_status.label).to eq 'passed with warnings' + end + end + end + describe '#cancelable?' do %i[created running pending].each do |status0| context "when there is a build #{status0}" do diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb new file mode 100644 index 00000000000..f232761dba2 --- /dev/null +++ b/spec/models/ci/stage_spec.rb @@ -0,0 +1,133 @@ +require 'spec_helper' + +describe Ci::Stage, models: true do + let(:stage) { build(:ci_stage) } + let(:pipeline) { stage.pipeline } + let(:stage_name) { stage.name } + + describe '#expectations' do + subject { stage } + + it { is_expected.to include_module(StaticModel) } + + it { is_expected.to respond_to(:pipeline) } + it { is_expected.to respond_to(:name) } + + it { is_expected.to delegate_method(:project).to(:pipeline) } + end + + describe '#statuses' do + let!(:stage_build) { create_job(:ci_build) } + let!(:commit_status) { create_job(:commit_status) } + let!(:other_build) { create_job(:ci_build, stage: 'other stage') } + + subject { stage.statuses } + + it "returns only matching statuses" do + is_expected.to contain_exactly(stage_build, commit_status) + end + end + + describe '#builds' do + let!(:stage_build) { create_job(:ci_build) } + let!(:commit_status) { create_job(:commit_status) } + + subject { stage.builds } + + it "returns only builds" do + is_expected.to contain_exactly(stage_build) + end + end + + describe '#status' do + subject { stage.status } + + context 'if status is already defined' do + let(:stage) { build(:ci_stage, status: 'success') } + + it "returns defined status" do + is_expected.to eq('success') + end + end + + context 'if status has to be calculated' do + let!(:stage_build) { create_job(:ci_build, status: :failed) } + + it "returns status of a build" do + is_expected.to eq('failed') + end + + context 'and builds are retried' do + let!(:new_build) { create_job(:ci_build, status: :success) } + + it "returns status of latest build" do + is_expected.to eq('success') + end + end + end + end + + describe '#detailed_status' do + subject { stage.detailed_status } + + context 'when build is created' do + let!(:stage_build) { create_job(:ci_build, status: :created) } + + it 'returns detailed status for created stage' do + expect(subject.text).to eq 'created' + end + end + + context 'when build is pending' do + let!(:stage_build) { create_job(:ci_build, status: :pending) } + + it 'returns detailed status for pending stage' do + expect(subject.text).to eq 'pending' + end + end + + context 'when build is running' do + let!(:stage_build) { create_job(:ci_build, status: :running) } + + it 'returns detailed status for running stage' do + expect(subject.text).to eq 'running' + end + end + + context 'when build is successful' do + let!(:stage_build) { create_job(:ci_build, status: :success) } + + it 'returns detailed status for successful stage' do + expect(subject.text).to eq 'passed' + end + end + + context 'when build is failed' do + let!(:stage_build) { create_job(:ci_build, status: :failed) } + + it 'returns detailed status for failed stage' do + expect(subject.text).to eq 'failed' + end + end + + context 'when build is canceled' do + let!(:stage_build) { create_job(:ci_build, status: :canceled) } + + it 'returns detailed status for canceled stage' do + expect(subject.text).to eq 'canceled' + end + end + + context 'when build is skipped' do + let!(:stage_build) { create_job(:ci_build, status: :skipped) } + + it 'returns detailed status for skipped stage' do + expect(subject.text).to eq 'skipped' + end + end + end + + def create_job(type, status: 'success', stage: stage_name) + create(type, pipeline: pipeline, stage: stage, status: status) + end +end diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index 4e7833c3162..bee9f714849 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -5,6 +5,13 @@ describe Ci::Variable, models: true do let(:secret_value) { 'secret' } + it { is_expected.to validate_presence_of(:key) } + it { is_expected.to validate_uniqueness_of(:key).scoped_to(:gl_project_id) } + it { is_expected.to validate_length_of(:key).is_at_most(255) } + it { is_expected.to allow_value('foo').for(:key) } + it { is_expected.not_to allow_value('foo bar').for(:key) } + it { is_expected.not_to allow_value('foo/bar').for(:key) } + before :each do subject.value = secret_value end diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb index d89d4342dea..30782ca75a0 100644 --- a/spec/models/commit_range_spec.rb +++ b/spec/models/commit_range_spec.rb @@ -137,26 +137,25 @@ describe CommitRange, models: true do end describe '#has_been_reverted?' do - it 'returns true if the commit has been reverted' do - issue = create(:issue) + let(:issue) { create(:issue) } + let(:user) { issue.author } + it 'returns true if the commit has been reverted' do create(:note_on_issue, noteable: issue, system: true, - note: commit1.revert_description, + note: commit1.revert_description(user), project: issue.project) expect_any_instance_of(Commit).to receive(:reverts_commit?). - with(commit1). + with(commit1, user). and_return(true) - expect(commit1.has_been_reverted?(nil, issue)).to eq(true) + expect(commit1.has_been_reverted?(user, issue)).to eq(true) end it 'returns false a commit has not been reverted' do - issue = create(:issue) - - expect(commit1.has_been_reverted?(nil, issue)).to eq(false) + expect(commit1.has_been_reverted?(user, issue)).to eq(false) end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index eb482c7f913..0935fc0561c 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -179,25 +179,26 @@ eos describe '#reverts_commit?' do let(:another_commit) { double(:commit, revert_description: "This reverts commit #{commit.sha}") } + let(:user) { commit.author } - it { expect(commit.reverts_commit?(another_commit)).to be_falsy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy } context 'commit has no description' do before { allow(commit).to receive(:description?).and_return(false) } - it { expect(commit.reverts_commit?(another_commit)).to be_falsy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy } end context "another_commit's description does not revert commit" do before { allow(commit).to receive(:description).and_return("Foo Bar") } - it { expect(commit.reverts_commit?(another_commit)).to be_falsy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy } end context "another_commit's description reverts commit" do before { allow(commit).to receive(:description).and_return("Foo #{another_commit.revert_description} Bar") } - it { expect(commit.reverts_commit?(another_commit)).to be_truthy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_truthy } end context "another_commit's description reverts merged merge request" do @@ -207,7 +208,7 @@ eos allow(commit).to receive(:description).and_return("Foo #{another_commit.revert_description} Bar") end - it { expect(commit.reverts_commit?(another_commit)).to be_truthy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_truthy } end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 80c2a1bc7a9..1ec08c2a9d0 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -175,7 +175,7 @@ describe CommitStatus, models: true do 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)) + is_expected.to eq(statuses.values_at(0, 1, 2, 3, 4, 5, 6, 8, 9)) end end @@ -200,49 +200,6 @@ describe CommitStatus, models: true do end end - describe '#stages' do - before do - create :commit_status, pipeline: pipeline, stage: 'build', name: 'linux', stage_idx: 0, status: 'success' - create :commit_status, pipeline: pipeline, stage: 'build', name: 'mac', stage_idx: 0, status: 'failed' - create :commit_status, pipeline: pipeline, stage: 'deploy', name: 'staging', stage_idx: 2, status: 'running' - create :commit_status, pipeline: pipeline, stage: 'test', name: 'rspec', stage_idx: 1, status: 'success' - end - - context 'stages list' do - subject { CommitStatus.where(pipeline: pipeline).stages } - - it 'returns ordered list of stages' do - is_expected.to eq(%w[build test deploy]) - end - end - - context 'stages with statuses' do - subject { CommitStatus.where(pipeline: pipeline).latest.stages_status } - - it 'returns list of stages with statuses' do - is_expected.to eq({ - 'build' => 'failed', - 'test' => 'success', - 'deploy' => 'running' - }) - end - - context 'when build is retried' do - before do - create :commit_status, pipeline: pipeline, stage: 'build', name: 'mac', stage_idx: 0, status: 'success' - end - - it 'ignores a previous state' do - is_expected.to eq({ - 'build' => 'success', - 'test' => 'success', - 'deploy' => 'running' - }) - end - end - end - end - describe '#commit' do it 'returns commit pipeline has been created for' do expect(commit_status.commit).to eq project.commit diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index 9defb17dc92..4d0f51fe82a 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -48,7 +48,7 @@ describe HasStatus do [create(type, status: :failed, allow_failure: true)] end - it { is_expected.to eq 'success' } + it { is_expected.to eq 'skipped' } end context 'success and canceled' do diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 6f84bffe046..4fa06a8c60a 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -35,7 +35,7 @@ describe Issue, "Issuable" do it { is_expected.to validate_presence_of(:iid) } it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_length_of(:title).is_at_least(0).is_at_most(255) } + it { is_expected.to validate_length_of(:title).is_at_most(255) } end describe "Scope" do diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb new file mode 100644 index 00000000000..0acefc0c1d5 --- /dev/null +++ b/spec/models/concerns/routable_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe Group, 'Routable' do + let!(:group) { create(:group) } + + describe 'Associations' do + it { is_expected.to have_one(:route).dependent(:destroy) } + end + + describe 'Callbacks' do + it 'creates route record on create' do + expect(group.route.path).to eq(group.path) + end + + it 'updates route record on path change' do + group.update_attributes(path: 'wow') + + expect(group.route.path).to eq('wow') + end + + it 'ensure route path uniqueness across different objects' do + create(:group, parent: group, path: 'xyz') + duplicate = build(:project, namespace: group, path: 'xyz') + + expect { duplicate.save! }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Route path has already been taken, Route is invalid') + end + end + + describe '.find_by_full_path' do + let!(:nested_group) { create(:group, parent: group) } + + it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) } + it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) } + it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) } + it { expect(described_class.find_by_full_path('unknown')).to eq(nil) } + end + + describe '.where_paths_in' do + context 'without any paths' do + it 'returns an empty relation' do + expect(described_class.where_paths_in([])).to eq([]) + end + end + + context 'without any valid paths' do + it 'returns an empty relation' do + expect(described_class.where_paths_in(%w[unknown])).to eq([]) + end + end + + context 'with valid paths' do + let!(:nested_group) { create(:group, parent: group) } + + it 'returns the projects matching the paths' do + result = described_class.where_paths_in([group.to_param, nested_group.to_param]) + + expect(result).to contain_exactly(group, nested_group) + end + + it 'returns projects regardless of the casing of paths' do + result = described_class.where_paths_in([group.to_param.upcase, nested_group.to_param.upcase]) + + expect(result).to contain_exactly(group, nested_group) + end + end + end +end diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb index 2a67c60b978..bc32fadd391 100644 --- a/spec/models/discussion_spec.rb +++ b/spec/models/discussion_spec.rb @@ -521,6 +521,15 @@ describe Discussion, model: true do end end + describe "#first_note_to_resolve" do + it "returns the first not that still needs to be resolved" do + allow(first_note).to receive(:to_be_resolved?).and_return(false) + allow(second_note).to receive(:to_be_resolved?).and_return(true) + + expect(subject.first_note_to_resolve).to eq(second_note) + end + end + describe "#collapsed?" do context "when a diff discussion" do before do diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index d06665197db..c8170164898 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -13,9 +13,9 @@ 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(:name).is_at_most(255) } - it { is_expected.to validate_length_of(:external_url).is_within(0..255) } + it { is_expected.to validate_length_of(:external_url).is_at_most(255) } # To circumvent a not null violation of the name column: # https://github.com/thoughtbot/shoulda-matchers/issues/336 diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index c5a5467ca99..7758b7ffa97 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -9,9 +9,13 @@ describe Key, models: true do describe "Validation" do it { is_expected.to validate_presence_of(:title) } + it { is_expected.to validate_length_of(:title).is_at_most(255) } + it { is_expected.to validate_presence_of(:key) } - it { is_expected.to validate_length_of(:title).is_within(0..255) } - it { is_expected.to validate_length_of(:key).is_within(0..5000) } + it { is_expected.to validate_length_of(:key).is_at_most(5000) } + it { is_expected.to allow_value('ssh-foo').for(:key) } + it { is_expected.to allow_value('ecdsa-foo').for(:key) } + it { is_expected.not_to allow_value('foo-bar').for(:key) } end describe "Methods" do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index af524497d1e..8b730be91fd 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1127,6 +1127,46 @@ describe MergeRequest, models: true do allow(subject).to receive(:diff_discussions).and_return([first_discussion, second_discussion, third_discussion]) end + describe '#resolvable_discussions' do + before do + allow(first_discussion).to receive(:to_be_resolved?).and_return(true) + allow(second_discussion).to receive(:to_be_resolved?).and_return(false) + allow(third_discussion).to receive(:to_be_resolved?).and_return(false) + end + + it 'includes only discussions that need to be resolved' do + expect(subject.resolvable_discussions).to eq([first_discussion]) + end + end + + describe '#discussions_can_be_resolved_by? user' do + let(:user) { build(:user) } + + context 'all discussions can be resolved by the user' do + before do + allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(third_discussion).to receive(:can_resolve?).with(user).and_return(true) + end + + it 'allows a user to resolve the discussions' do + expect(subject.discussions_can_be_resolved_by?(user)).to be(true) + end + end + + context 'one discussion cannot be resolved by the user' do + before do + allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(third_discussion).to receive(:can_resolve?).with(user).and_return(false) + end + + it 'allows a user to resolve the discussions' do + expect(subject.discussions_can_be_resolved_by?(user)).to be(false) + end + end + end + describe "#discussions_resolvable?" do context "when all discussions are unresolvable" do before do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 431b3e4435f..7f82e85563b 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -4,11 +4,18 @@ describe Namespace, models: true do let!(:namespace) { create(:namespace) } it { is_expected.to have_many :projects } - it { is_expected.to validate_presence_of :name } + + it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name) } - it { is_expected.to validate_presence_of :path } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + + it { is_expected.to validate_length_of(:description).is_at_most(255) } + + it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_uniqueness_of(:path) } - it { is_expected.to validate_presence_of :owner } + it { is_expected.to validate_length_of(:path).is_at_most(255) } + + it { is_expected.to validate_presence_of(:owner) } describe "Mass assignment" do end @@ -117,4 +124,12 @@ describe Namespace, models: true do expect(Namespace.clean_path("--%+--valid_*&%name=.git.%.atom.atom.@email.com")).to eq("valid_name") end end + + describe '#full_path' do + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + + it { expect(group.full_path).to eq(group.path) } + it { expect(nested_group.full_path).to eq("#{group.path}/#{nested_group.path}") } + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 587ca1936a3..21ff238841e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -131,14 +131,18 @@ describe Project, models: true do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:namespace_id) } - it { is_expected.to validate_length_of(:name).is_within(0..255) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_uniqueness_of(:path).scoped_to(:namespace_id) } - it { is_expected.to validate_length_of(:path).is_within(0..255) } - it { is_expected.to validate_length_of(:description).is_within(0..2000) } + it { is_expected.to validate_length_of(:path).is_at_most(255) } + + it { is_expected.to validate_length_of(:description).is_at_most(2000) } + it { is_expected.to validate_presence_of(:creator) } + it { is_expected.to validate_presence_of(:namespace) } + it { is_expected.to validate_presence_of(:repository_storage) } it 'does not allow new projects beyond user limits' do @@ -474,35 +478,6 @@ describe Project, models: true do end end - describe '.find_with_namespace' do - context 'with namespace' do - before do - @group = create :group, name: 'gitlab' - @project = create(:project, name: 'gitlabhq', namespace: @group) - end - - it { expect(Project.find_with_namespace('gitlab/gitlabhq')).to eq(@project) } - it { expect(Project.find_with_namespace('GitLab/GitlabHQ')).to eq(@project) } - it { expect(Project.find_with_namespace('gitlab-ci')).to be_nil } - end - - context 'when multiple projects using a similar name exist' do - let(:group) { create(:group, name: 'gitlab') } - - let!(:project1) do - create(:empty_project, name: 'gitlab1', path: 'gitlab', namespace: group) - end - - let!(:project2) do - create(:empty_project, name: 'gitlab2', path: 'GITLAB', namespace: group) - end - - it 'returns the row where the path matches literally' do - expect(Project.find_with_namespace('gitlab/GITLAB')).to eq(project2) - end - end - end - describe '#to_param' do context 'with namespace' do before do @@ -1544,39 +1519,6 @@ describe Project, models: true do end end - describe '.where_paths_in' do - context 'without any paths' do - it 'returns an empty relation' do - expect(Project.where_paths_in([])).to eq([]) - end - end - - context 'without any valid paths' do - it 'returns an empty relation' do - expect(Project.where_paths_in(%w[foo])).to eq([]) - end - end - - context 'with valid paths' do - let!(:project1) { create(:project) } - let!(:project2) { create(:project) } - - it 'returns the projects matching the paths' do - projects = Project.where_paths_in([project1.path_with_namespace, - project2.path_with_namespace]) - - expect(projects).to contain_exactly(project1, project2) - end - - it 'returns projects regardless of the casing of paths' do - projects = Project.where_paths_in([project1.path_with_namespace.upcase, - project2.path_with_namespace.upcase]) - - expect(projects).to contain_exactly(project1, project2) - end - end - end - describe 'change_head' do let(:project) { create(:project) } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index b797d19161d..b5a42edd192 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -768,7 +768,6 @@ describe Repository, models: true do 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) repository.update_branch_with_hooks(user, 'new-feature') { new_rev } end @@ -786,7 +785,6 @@ describe Repository, models: true do 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) empty_repository.commit_file(user, 'CHANGELOG', 'Changelog!', 'Updates file content', 'master', false) @@ -829,15 +827,6 @@ describe Repository, models: true do expect(subject).to eq(true) end - - it 'caches the output' do - expect(repository).to receive(:branch_count). - once. - and_return(3) - - repository.has_visible_content? - repository.has_visible_content? - end end end @@ -918,20 +907,6 @@ describe Repository, models: true do end end - describe '#expire_has_visible_content_cache' do - it 'expires the visible content cache' do - repository.has_visible_content? - - expect(repository).to receive(:branch_count). - once. - and_return(0) - - repository.expire_has_visible_content_cache - - expect(repository.has_visible_content?).to eq(false) - end - end - describe '#expire_branch_cache' do # This method is private but we need it for testing purposes. Sadly there's # no other proper way of testing caching operations. @@ -967,7 +942,6 @@ describe Repository, models: true do allow(repository).to receive(:empty?).and_return(true) expect(cache).to receive(:expire).with(:empty?) - expect(repository).to receive(:expire_has_visible_content_cache) repository.expire_emptiness_caches end @@ -976,7 +950,6 @@ describe Repository, models: true do allow(repository).to receive(:empty?).and_return(false) expect(cache).not_to receive(:expire).with(:empty?) - expect(repository).not_to receive(:expire_has_visible_content_cache) repository.expire_emptiness_caches end @@ -1203,16 +1176,16 @@ describe Repository, models: true do end describe '#after_create_branch' do - it 'flushes the visible content cache' do - expect(repository).to receive(:expire_has_visible_content_cache) + it 'expires the branch caches' do + expect(repository).to receive(:expire_branches_cache) repository.after_create_branch end end describe '#after_remove_branch' do - it 'flushes the visible content cache' do - expect(repository).to receive(:expire_has_visible_content_cache) + it 'expires the branch caches' do + expect(repository).to receive(:expire_branches_cache) repository.after_remove_branch end diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb new file mode 100644 index 00000000000..6f491fdf9a0 --- /dev/null +++ b/spec/models/route_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Route, models: true do + let!(:group) { create(:group) } + let!(:route) { group.route } + + describe 'relationships' do + it { is_expected.to belong_to(:source) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:source) } + it { is_expected.to validate_presence_of(:path) } + it { is_expected.to validate_uniqueness_of(:path) } + end + + describe '#rename_children' do + let!(:nested_group) { create(:group, path: "test", parent: group) } + let!(:deep_nested_group) { create(:group, path: "foo", parent: nested_group) } + + it "updates children routes with new path" do + route.update_attributes(path: 'bar') + + expect(described_class.exists?(path: 'bar')).to be_truthy + expect(described_class.exists?(path: 'bar/test')).to be_truthy + expect(described_class.exists?(path: 'bar/test/foo')).to be_truthy + end + end +end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 79d2843e122..7425a897769 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -23,9 +23,9 @@ describe Snippet, models: true do it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_length_of(:title).is_within(0..255) } + it { is_expected.to validate_length_of(:title).is_at_most(255) } - it { is_expected.to validate_length_of(:file_name).is_within(0..255) } + it { is_expected.to validate_length_of(:file_name).is_at_most(255) } it { is_expected.to validate_presence_of(:content) } @@ -61,6 +61,26 @@ describe Snippet, models: true do end end + describe '#file_name' do + let(:project) { create(:empty_project) } + + context 'file_name is nil' do + let(:snippet) { create(:snippet, project: project, file_name: nil) } + + it 'returns an empty string' do + expect(snippet.file_name).to eq '' + end + end + + context 'file_name is not nil' do + let(:snippet) { create(:snippet, project: project, file_name: 'foo.txt') } + + it 'returns the file_name' do + expect(snippet.file_name).to eq 'foo.txt' + end + end + end + describe "#content_html_invalidated?" do let(:snippet) { create(:snippet, content: "md", content_html: "html", file_name: "foo.md") } it "invalidates the HTML cache of content when the filename changes" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2244803f90c..bad6ed9e146 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -79,7 +79,7 @@ describe User, models: true do it { is_expected.to allow_value(0).for(:projects_limit) } it { is_expected.not_to allow_value(-1).for(:projects_limit) } - it { is_expected.to validate_length_of(:bio).is_within(0..255) } + it { is_expected.to validate_length_of(:bio).is_at_most(255) } it_behaves_like 'an object with email-formated attributes', :email do subject { build(:user) } diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index b49e4f3a8bc..eeab9827d99 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -111,14 +111,36 @@ describe ProjectPolicy, models: true do context 'guests' do let(:current_user) { guest } + let(:reporter_public_build_permissions) do + reporter_permissions - [:read_build, :read_pipeline] + end + it do is_expected.to include(*guest_permissions) - is_expected.not_to include(*reporter_permissions) + is_expected.not_to include(*reporter_public_build_permissions) is_expected.not_to include(*team_member_reporter_permissions) is_expected.not_to include(*developer_permissions) is_expected.not_to include(*master_permissions) is_expected.not_to include(*owner_permissions) end + + context 'public builds enabled' do + it do + is_expected.to include(*guest_permissions) + is_expected.to include(:read_build, :read_pipeline) + end + end + + context 'public builds disabled' do + before do + project.update(public_builds: false) + end + + it do + is_expected.to include(*guest_permissions) + is_expected.not_to include(:read_build, :read_pipeline) + end + end end context 'reporter' do diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index 36517ad0f8c..3f34309f419 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -153,85 +153,144 @@ describe API::Helpers, api: true do end end - it "changes current user to sudo when admin" do - set_env(admin, user.id) - expect(current_user).to eq(user) - set_param(admin, user.id) - expect(current_user).to eq(user) - set_env(admin, user.username) - expect(current_user).to eq(user) - set_param(admin, user.username) - expect(current_user).to eq(user) - end + context 'sudo usage' do + context 'with admin' do + context 'with header' do + context 'with id' do + it 'changes current_user to sudo' do + set_env(admin, user.id) - it "throws an error when the current user is not an admin and attempting to sudo" do - set_env(user, admin.id) - expect { current_user }.to raise_error(Exception) - set_param(user, admin.id) - expect { current_user }.to raise_error(Exception) - set_env(user, admin.username) - expect { current_user }.to raise_error(Exception) - set_param(user, admin.username) - expect { current_user }.to raise_error(Exception) - end + expect(current_user).to eq(user) + end - it "throws an error when the user cannot be found for a given id" do - id = user.id + admin.id - expect(user.id).not_to eq(id) - expect(admin.id).not_to eq(id) - set_env(admin, id) - expect { current_user }.to raise_error(Exception) + it 'handles sudo to oneself' do + set_env(admin, admin.id) - set_param(admin, id) - expect { current_user }.to raise_error(Exception) - end + expect(current_user).to eq(admin) + end - it "throws an error when the user cannot be found for a given username" do - username = "#{user.username}#{admin.username}" - expect(user.username).not_to eq(username) - expect(admin.username).not_to eq(username) - set_env(admin, username) - expect { current_user }.to raise_error(Exception) + it 'throws an error when user cannot be found' do + id = user.id + admin.id + expect(user.id).not_to eq(id) + expect(admin.id).not_to eq(id) - set_param(admin, username) - expect { current_user }.to raise_error(Exception) - end + set_env(admin, id) - it "handles sudo's to oneself" do - set_env(admin, admin.id) - expect(current_user).to eq(admin) - set_param(admin, admin.id) - expect(current_user).to eq(admin) - set_env(admin, admin.username) - expect(current_user).to eq(admin) - set_param(admin, admin.username) - expect(current_user).to eq(admin) - end + expect { current_user }.to raise_error(Exception) + end + end - it "handles multiple sudo's to oneself" do - set_env(admin, user.id) - expect(current_user).to eq(user) - expect(current_user).to eq(user) - set_env(admin, user.username) - expect(current_user).to eq(user) - expect(current_user).to eq(user) - - set_param(admin, user.id) - expect(current_user).to eq(user) - expect(current_user).to eq(user) - set_param(admin, user.username) - expect(current_user).to eq(user) - expect(current_user).to eq(user) - end + context 'with username' do + it 'changes current_user to sudo' do + set_env(admin, user.username) + + expect(current_user).to eq(user) + end + + it 'handles sudo to oneself' do + set_env(admin, admin.username) + + expect(current_user).to eq(admin) + end + + it "throws an error when the user cannot be found for a given username" do + username = "#{user.username}#{admin.username}" + expect(user.username).not_to eq(username) + expect(admin.username).not_to eq(username) + + set_env(admin, username) + + expect { current_user }.to raise_error(Exception) + end + end + end + + context 'with param' do + context 'with id' do + it 'changes current_user to sudo' do + set_param(admin, user.id) + + expect(current_user).to eq(user) + end + + it 'handles sudo to oneself' do + set_param(admin, admin.id) + + expect(current_user).to eq(admin) + end + + it 'handles sudo to oneself using string' do + set_env(admin, user.id.to_s) + + expect(current_user).to eq(user) + end + + it 'throws an error when user cannot be found' do + id = user.id + admin.id + expect(user.id).not_to eq(id) + expect(admin.id).not_to eq(id) - it "handles multiple sudo's to oneself using string ids" do - set_env(admin, user.id.to_s) - expect(current_user).to eq(user) - expect(current_user).to eq(user) + set_param(admin, id) - set_param(admin, user.id.to_s) - expect(current_user).to eq(user) - expect(current_user).to eq(user) + expect { current_user }.to raise_error(Exception) + end + end + + context 'with username' do + it 'changes current_user to sudo' do + set_param(admin, user.username) + + expect(current_user).to eq(user) + end + + it 'handles sudo to oneself' do + set_param(admin, admin.username) + + expect(current_user).to eq(admin) + end + + it "throws an error when the user cannot be found for a given username" do + username = "#{user.username}#{admin.username}" + expect(user.username).not_to eq(username) + expect(admin.username).not_to eq(username) + + set_param(admin, username) + + expect { current_user }.to raise_error(Exception) + end + end + end + end + + context 'with regular user' do + context 'with env' do + it 'changes current_user to sudo when admin and user id' do + set_env(user, admin.id) + + expect { current_user }.to raise_error(Exception) + end + + it 'changes current_user to sudo when admin and user username' do + set_env(user, admin.username) + + expect { current_user }.to raise_error(Exception) + end + end + + context 'with params' do + it 'changes current_user to sudo when admin and user id' do + set_param(user, admin.id) + + expect { current_user }.to raise_error(Exception) + end + + it 'changes current_user to sudo when admin and user username' do + set_param(user, admin.username) + + expect { current_user }.to raise_error(Exception) + end + end + end end end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 55b8c8c0c69..2878e0cb59b 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -11,6 +11,7 @@ describe API::Branches, api: true do let!(:guest) { create(:project_member, :guest, user: user2, project: project) } let!(:branch_name) { 'feature' } let!(:branch_sha) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } + let!(:branch_with_dot) { CreateBranchService.new(project, user).execute("with.1.2.3", "master") } describe "GET /projects/:id/repository/branches" do it "returns an array of project branches" do @@ -37,6 +38,13 @@ describe API::Branches, api: true do expect(json_response['developers_can_merge']).to eq(false) end + it "returns the branch information for a single branch with dots in the name" do + get api("/projects/#{project.id}/repository/branches/with.1.2.3", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq("with.1.2.3") + end + context 'on a merged branch' do it "returns the branch information for a single branch" do get api("/projects/#{project.id}/repository/branches/merge-test", user) @@ -71,6 +79,14 @@ describe API::Branches, api: true do expect(json_response['developers_can_merge']).to eq(false) end + it "protects a single branch with dots in the name" do + put api("/projects/#{project.id}/repository/branches/with.1.2.3/protect", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq("with.1.2.3") + expect(json_response['protected']).to eq(true) + end + it 'protects a single branch and developers can push' do put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_push: true @@ -220,6 +236,14 @@ describe API::Branches, api: true do expect(json_response['protected']).to eq(false) end + it "update branches with dots in branch name" do + put api("/projects/#{project.id}/repository/branches/with.1.2.3/unprotect", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq("with.1.2.3") + expect(json_response['protected']).to eq(false) + end + it "returns success when unprotect branch" do put api("/projects/#{project.id}/repository/branches/unknown/unprotect", user) expect(response).to have_http_status(404) @@ -292,6 +316,13 @@ describe API::Branches, api: true do expect(json_response['branch_name']).to eq(branch_name) end + it "removes a branch with dots in the branch name" do + delete api("/projects/#{project.id}/repository/branches/with.1.2.3", user) + + expect(response).to have_http_status(200) + expect(json_response['branch_name']).to eq("with.1.2.3") + end + it 'returns 404 if branch not exists' do delete api("/projects/#{project.id}/repository/branches/foobar", user) expect(response).to have_http_status(404) diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 0ea991b18b8..7be7acebb19 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -5,7 +5,7 @@ describe API::Builds, api: true do let(:user) { create(:user) } let(:api_user) { user } - let!(:project) { create(:project, creator_id: user.id) } + let!(:project) { create(:project, creator_id: user.id, public_builds: false) } let!(:developer) { create(:project_member, :developer, user: user, project: project) } let(:reporter) { create(:project_member, :reporter, project: project) } let(:guest) { create(:project_member, :guest, project: project) } diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index 5fa7299044e..aabab8e6ae6 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -75,7 +75,6 @@ describe API::DeployKeys, api: true do expect(response).to have_http_status(400) expect(json_response['message']['key']).to eq([ 'can\'t be blank', - 'is too short (minimum is 0 characters)', 'is invalid' ]) end @@ -85,8 +84,7 @@ describe API::DeployKeys, api: true do expect(response).to have_http_status(400) expect(json_response['message']['title']).to eq([ - 'can\'t be blank', - 'is too short (minimum is 0 characters)' + 'can\'t be blank' ]) end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 548ed8e1892..15647b262b6 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -245,6 +245,17 @@ describe API::Groups, api: true do expect(project_names).to match_array([project1.name, project3.name]) end + it 'filters the groups projects' do + public_projet = create(:project, :public, path: 'test1', group: group1) + + get api("/groups/#{group1.id}/projects", user1), visibility: 'public' + + expect(response).to have_http_status(200) + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(1) + expect(json_response.first['name']).to eq(public_projet.name) + end + it "does not return a non existing group" do get api("/groups/1328/projects", user1) expect(response).to have_http_status(404) diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 5700f800c2e..5c80dd98dc7 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -72,13 +72,6 @@ describe API::Issues, api: true do expect(json_response.last).to have_key('web_url') end - it "adds pagination headers and keep query params" do - get api("/issues?state=closed&per_page=3", user) - expect(response.headers['Link']).to eq( - '<http://www.example.com/api/v3/issues?page=1&per_page=3&private_token=%s&state=closed>; rel="first", <http://www.example.com/api/v3/issues?page=1&per_page=3&private_token=%s&state=closed>; rel="last"' % [user.private_token, user.private_token] - ) - end - it 'returns an array of closed issues' do get api('/issues?state=closed', user) expect(response).to have_http_status(200) @@ -649,9 +642,8 @@ describe API::Issues, api: true do post api("/projects/#{project.id}/issues", user), title: 'new issue', confidential: 'foo' - expect(response).to have_http_status(201) - expect(json_response['title']).to eq('new issue') - expect(json_response['confidential']).to be_falsy + expect(response).to have_http_status(400) + expect(json_response['error']).to eq('confidential is invalid') end it "sends notifications for subscribers of newly added labels" do @@ -692,6 +684,32 @@ describe API::Issues, api: true do ]) end + context 'resolving issues in a merge request' do + let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first } + let(:merge_request) { discussion.noteable } + let(:project) { merge_request.source_project } + before do + project.team << [user, :master] + post api("/projects/#{project.id}/issues", user), + title: 'New Issue', + merge_request_for_resolving_discussions: merge_request.iid + end + + it 'creates a new project issue' do + expect(response).to have_http_status(:created) + end + + it 'resolves the discussions in a merge request' do + discussion.first_note.reload + + expect(discussion.resolved?).to be(true) + end + + it 'assigns a description to the issue mentioning the merge request' do + expect(json_response['description']).to include(merge_request.to_reference) + end + end + context 'with due date' do it 'creates a new project issue' do due_date = 2.weeks.from_now.strftime('%Y-%m-%d') @@ -836,8 +854,8 @@ describe API::Issues, api: true do put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user), confidential: 'foo' - expect(response).to have_http_status(200) - expect(json_response['confidential']).to be_truthy + expect(response).to have_http_status(400) + expect(json_response['error']).to eq('confidential is invalid') end end end @@ -959,6 +977,14 @@ describe API::Issues, api: true do expect(json_response['state']).to eq 'opened' end end + + context 'when issue does not exist' do + it 'returns 404 when trying to move an issue' do + delete api("/projects/#{project.id}/issues/123", user) + + expect(response).to have_http_status(404) + end + end end describe '/projects/:id/issues/:issue_id/move' do @@ -1007,6 +1033,7 @@ describe API::Issues, api: true do to_project_id: target_project.id expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Issue Not Found') end end @@ -1016,6 +1043,7 @@ describe API::Issues, api: true do to_project_id: target_project.id expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Project Not Found') end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 0b89ac7960e..75b270aa93c 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -234,11 +234,14 @@ describe API::MergeRequests, api: true do target_branch: 'master', author: user, labels: 'label, label2', - milestone_id: milestone.id + milestone_id: milestone.id, + remove_source_branch: true + expect(response).to have_http_status(201) expect(json_response['title']).to eq('Test merge_request') expect(json_response['labels']).to eq(['label', 'label2']) expect(json_response['milestone']['id']).to eq(milestone.id) + expect(json_response['force_remove_source_branch']).to be_truthy end it "returns 422 when source_branch equals target_branch" do @@ -511,6 +514,13 @@ describe API::MergeRequests, api: true do expect(json_response['target_branch']).to eq('wiki') end + it "returns merge_request that removes the source branch" do + put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), remove_source_branch: true + + expect(response).to have_http_status(200) + expect(json_response['force_remove_source_branch']).to be_truthy + end + it 'allows special label names' do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), title: 'new issue', diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb new file mode 100644 index 00000000000..f6fb6ea5506 --- /dev/null +++ b/spec/requests/api/snippets_spec.rb @@ -0,0 +1,157 @@ +require 'rails_helper' + +describe API::Snippets, api: true do + include ApiHelpers + let!(:user) { create(:user) } + + describe 'GET /snippets/' do + it 'returns snippets available' do + public_snippet = create(:personal_snippet, :public, author: user) + private_snippet = create(:personal_snippet, :private, author: user) + internal_snippet = create(:personal_snippet, :internal, author: user) + + get api("/snippets/", user) + + expect(response).to have_http_status(200) + expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( + public_snippet.id, + internal_snippet.id, + private_snippet.id) + expect(json_response.last).to have_key('web_url') + expect(json_response.last).to have_key('raw_url') + end + + it 'hides private snippets from regular user' do + create(:personal_snippet, :private) + + get api("/snippets/", user) + expect(response).to have_http_status(200) + expect(json_response.size).to eq(0) + end + end + + describe 'GET /snippets/public' do + let!(:other_user) { create(:user) } + let!(:public_snippet) { create(:personal_snippet, :public, author: user) } + let!(:private_snippet) { create(:personal_snippet, :private, author: user) } + let!(:internal_snippet) { create(:personal_snippet, :internal, author: user) } + let!(:public_snippet_other) { create(:personal_snippet, :public, author: other_user) } + let!(:private_snippet_other) { create(:personal_snippet, :private, author: other_user) } + let!(:internal_snippet_other) { create(:personal_snippet, :internal, author: other_user) } + + it 'returns all snippets with public visibility from all users' do + get api("/snippets/public", user) + + expect(response).to have_http_status(200) + expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( + public_snippet.id, + public_snippet_other.id) + expect(json_response.map{ |snippet| snippet['web_url']} ).to include( + "http://localhost/snippets/#{public_snippet.id}", + "http://localhost/snippets/#{public_snippet_other.id}") + expect(json_response.map{ |snippet| snippet['raw_url']} ).to include( + "http://localhost/snippets/#{public_snippet.id}/raw", + "http://localhost/snippets/#{public_snippet_other.id}/raw") + end + end + + describe 'GET /snippets/:id/raw' do + let(:snippet) { create(:personal_snippet, author: user) } + + it 'returns raw text' do + get api("/snippets/#{snippet.id}/raw", user) + + expect(response).to have_http_status(200) + expect(response.content_type).to eq 'text/plain' + expect(response.body).to eq(snippet.content) + end + + it 'returns 404 for invalid snippet id' do + delete api("/snippets/1234", user) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Snippet Not Found') + end + end + + describe 'POST /snippets/' do + let(:params) do + { + title: 'Test Title', + file_name: 'test.rb', + content: 'puts "hello world"', + visibility_level: Gitlab::VisibilityLevel::PUBLIC + } + end + + it 'creates a new snippet' do + expect do + post api("/snippets/", user), params + end.to change { PersonalSnippet.count }.by(1) + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq(params[:title]) + expect(json_response['file_name']).to eq(params[:file_name]) + end + + it 'returns 400 for missing parameters' do + params.delete(:title) + + post api("/snippets/", user), params + + expect(response).to have_http_status(400) + end + end + + describe 'PUT /snippets/:id' do + let(:other_user) { create(:user) } + let(:public_snippet) { create(:personal_snippet, :public, author: user) } + it 'updates snippet' do + new_content = 'New content' + + put api("/snippets/#{public_snippet.id}", user), content: new_content + + expect(response).to have_http_status(200) + public_snippet.reload + expect(public_snippet.content).to eq(new_content) + end + + it 'returns 404 for invalid snippet id' do + put api("/snippets/1234", user), title: 'foo' + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Snippet Not Found') + end + + it "returns 404 for another user's snippet" do + put api("/snippets/#{public_snippet.id}", other_user), title: 'fubar' + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Snippet Not Found') + end + + it 'returns 400 for missing parameters' do + put api("/snippets/1234", user) + + expect(response).to have_http_status(400) + end + end + + describe 'DELETE /snippets/:id' do + let!(:public_snippet) { create(:personal_snippet, :public, author: user) } + it 'deletes snippet' do + expect do + delete api("/snippets/#{public_snippet.id}", user) + + expect(response).to have_http_status(204) + end.to change { PersonalSnippet.count }.by(-1) + end + + it 'returns 404 for invalid snippet id' do + delete api("/snippets/1234", user) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Snippet Not Found') + end + end +end diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index 06fa94fae87..a1c32ae65ba 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -15,6 +15,31 @@ describe API::Tags, api: true do let(:tag_name) { project.repository.tag_names.sort.reverse.first } let(:description) { 'Awesome release!' } + shared_examples_for 'repository tags' do + it 'returns the repository tags' do + get api("/projects/#{project.id}/repository/tags", current_user) + + expect(response).to have_http_status(200) + + first_tag = json_response.first + + expect(first_tag['name']).to eq(tag_name) + end + end + + context 'when unauthenticated' do + it_behaves_like 'repository tags' do + let(:project) { create(:project, :public) } + let(:current_user) { nil } + end + end + + context 'when authenticated' do + it_behaves_like 'repository tags' do + let(:current_user) { user } + end + end + context 'without releases' do it "returns an array of project tags" do get api("/projects/#{project.id}/repository/tags", user) @@ -45,17 +70,33 @@ describe API::Tags, api: true do describe 'GET /projects/:id/repository/tags/:tag_name' do let(:tag_name) { project.repository.tag_names.sort.reverse.first } - it 'returns a specific tag' do - get api("/projects/#{project.id}/repository/tags/#{tag_name}", user) + shared_examples_for 'repository tag' do + it 'returns the repository tag' do + get api("/projects/#{project.id}/repository/tags/#{tag_name}", current_user) + + expect(response).to have_http_status(200) + + expect(json_response['name']).to eq(tag_name) + end + + it 'returns 404 for an invalid tag name' do + get api("/projects/#{project.id}/repository/tags/foobar", current_user) - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(tag_name) + expect(response).to have_http_status(404) + end end - it 'returns 404 for an invalid tag name' do - get api("/projects/#{project.id}/repository/tags/foobar", user) + context 'when unauthenticated' do + it_behaves_like 'repository tag' do + let(:project) { create(:project, :public) } + let(:current_user) { nil } + end + end - expect(response).to have_http_status(404) + context 'when authenticated' do + it_behaves_like 'repository tag' do + let(:current_user) { user } + end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index f82f52e7399..c37dbfa0a33 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -651,20 +651,75 @@ describe API::Users, api: true do end describe "GET /user" do - it "returns current user" do - get api("/user", user) - expect(response).to have_http_status(200) - expect(json_response['email']).to eq(user.email) - expect(json_response['is_admin']).to eq(user.is_admin?) - expect(json_response['can_create_project']).to eq(user.can_create_project?) - expect(json_response['can_create_group']).to eq(user.can_create_group?) - expect(json_response['projects_limit']).to eq(user.projects_limit) - expect(json_response['private_token']).to be_blank + let(:personal_access_token) { create(:personal_access_token, user: user) } + let(:private_token) { user.private_token } + + context 'with regular user' do + context 'with personal access token' do + it 'returns 403 without private token when sudo is defined' do + get api("/user?private_token=#{personal_access_token.token}&sudo=#{user.id}") + + expect(response).to have_http_status(403) + end + end + + context 'with private token' do + it 'returns 403 without private token when sudo defined' do + get api("/user?private_token=#{private_token}&sudo=#{user.id}") + + expect(response).to have_http_status(403) + end + end + + it 'returns current user without private token when sudo not defined' do + get api("/user", user) + + expect(response).to have_http_status(200) + expect(response).to match_response_schema('user/public') + end end - it "returns 401 error if user is unauthenticated" do - get api("/user") - expect(response).to have_http_status(401) + context 'with admin' do + let(:user) { create(:admin) } + + context 'with personal access token' do + it 'returns 403 without private token when sudo defined' do + get api("/user?private_token=#{personal_access_token.token}&sudo=#{user.id}") + + expect(response).to have_http_status(403) + end + + it 'returns current user without private token when sudo not defined' do + get api("/user?private_token=#{personal_access_token.token}") + + expect(response).to have_http_status(200) + expect(response).to match_response_schema('user/public') + end + end + + context 'with private token' do + it 'returns current user with private token when sudo defined' do + get api("/user?private_token=#{private_token}&sudo=#{user.id}") + + expect(response).to have_http_status(200) + expect(response).to match_response_schema('user/login') + end + + it 'returns current user without private token when sudo not defined' do + get api("/user?private_token=#{private_token}") + + expect(response).to have_http_status(200) + expect(response).to match_response_schema('user/public') + end + end + end + + context 'with unauthenticated user' do + it "returns 401 error if user is unauthenticated" do + get api("/user") + + expect(response).to have_http_status(401) + end end end diff --git a/spec/requests/projects/cycle_analytics_events_spec.rb b/spec/requests/projects/cycle_analytics_events_spec.rb index f5e0fdcda2d..e0368e6001f 100644 --- a/spec/requests/projects/cycle_analytics_events_spec.rb +++ b/spec/requests/projects/cycle_analytics_events_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe 'cycle analytics events' do let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, public_builds: false) } let(:issue) { create(:issue, project: project, created_at: 2.days.ago) } describe 'GET /:namespace/:project/cycle_analytics/events/issues' do diff --git a/spec/services/discussions/resolve_service_spec.rb b/spec/services/discussions/resolve_service_spec.rb new file mode 100644 index 00000000000..12c3cdf28c6 --- /dev/null +++ b/spec/services/discussions/resolve_service_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +describe Discussions::ResolveService do + describe '#execute' do + let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first } + let(:project) { merge_request.project } + let(:merge_request) { discussion.noteable } + let(:user) { create(:user) } + let(:service) { described_class.new(discussion.noteable.project, user, merge_request: merge_request) } + + before do + project.team << [user, :master] + end + + it "doesn't resolve discussions the user can't resolve" do + expect(discussion).to receive(:can_resolve?).with(user).and_return(false) + + service.execute(discussion) + + expect(discussion.resolved?).to be(false) + end + + it 'resolves the discussion' do + service.execute(discussion) + + expect(discussion.resolved?).to be(true) + end + + it 'executes the notification service' do + expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(discussion.noteable) + + service.execute(discussion) + end + + it 'adds a system note to the discussion' do + issue = create(:issue, project: project) + + expect(SystemNoteService).to receive(:discussion_continued_in_issue).with(discussion, project, user, issue) + service = described_class.new(project, user, merge_request: merge_request, follow_up_issue: issue) + service.execute(discussion) + end + + it 'can resolve multiple discussions at once' do + other_discussion = Discussion.for_diff_notes([create(:diff_note_on_merge_request, noteable: discussion.noteable, project: discussion.noteable.source_project)]).first + + service.execute([discussion, other_discussion]) + + expect(discussion.resolved?).to be(true) + expect(other_discussion.resolved?).to be(true) + end + end +end diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb new file mode 100644 index 00000000000..4cfba35c830 --- /dev/null +++ b/spec/services/issues/build_service_spec.rb @@ -0,0 +1,130 @@ +require 'spec_helper.rb' + +describe Issues::BuildService, services: true do + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + project.team << [user, :developer] + end + + context 'for discussions in a merge request' do + let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) } + let(:issue) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request).execute } + + def position_on_line(line_number) + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: line_number, + diff_refs: merge_request.diff_refs + ) + end + + describe '#items_for_discussions' do + it 'has an item for each discussion' do + create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.source_project, position: position_on_line(13)) + service = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request) + + service.execute + + expect(service.items_for_discussions.size).to eq(2) + end + end + + describe '#item_for_discussion' do + let(:service) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request) } + + it 'mentions the author of the note' do + discussion = Discussion.new([create(:diff_note_on_merge_request, author: create(:user, username: 'author'))]) + expect(service.item_for_discussion(discussion)).to include('@author') + end + + it 'wraps the note in a blockquote' do + note_text = "This is a string\n"\ + ">>>\n"\ + "with a blockquote\n"\ + "> That has a quote\n"\ + ">>>\n" + note_result = "This is a string\n"\ + "> with a blockquote\n"\ + "> > That has a quote\n" + discussion = Discussion.new([create(:diff_note_on_merge_request, note: note_text)]) + expect(service.item_for_discussion(discussion)).to include(">>>\n#{note_result}\n>>>") + end + end + + describe '#execute' do + it 'has the merge request reference in the title' do + expect(issue.title).to include(merge_request.title) + end + + it 'has the reference of the merge request in the description' do + expect(issue.description).to include(merge_request.to_reference) + end + + it 'does not assign title when a title was given' do + issue = described_class.new(project, user, + merge_request_for_resolving_discussions: merge_request, + title: 'What an issue').execute + + expect(issue.title).to eq('What an issue') + end + + it 'does not assign description when a description was given' do + issue = described_class.new(project, user, + merge_request_for_resolving_discussions: merge_request, + description: 'Fix at your earliest conveignance').execute + + expect(issue.description).to eq('Fix at your earliest conveignance') + end + + describe 'with multiple discussions' do + before do + create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.target_project, position: position_on_line(15)) + end + + it 'mentions all the authors in the description' do + authors = merge_request.diff_discussions.map(&:author) + + expect(issue.description).to include(*authors.map(&:to_reference)) + end + + it 'has a link for each unresolved discussion in the description' do + notes = merge_request.diff_discussions.map(&:first_note) + links = notes.map { |note| Gitlab::UrlBuilder.build(note) } + + expect(issue.description).to include(*links) + end + + it 'mentions additional notes' do + create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, position: position_on_line(15)) + + expect(issue.description).to include('(+2 comments)') + end + end + end + end + + context 'For a merge request without discussions' do + let(:merge_request) { create(:merge_request, source_project: project) } + + describe '#execute' do + it 'mentions the merge request in the description' do + issue = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request).execute + + expect(issue.description).to include("Review the conversation in #{merge_request.to_reference}") + end + end + end + + describe '#execute' do + it 'builds a new issues with given params' do + issue = described_class.new(project, user, title: 'Issue #1', description: 'Issue description').execute + + expect(issue.title).to eq('Issue #1') + expect(issue.description).to eq('Issue description') + end + end +end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 5c0331ebe66..8bde61ee336 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -136,5 +136,48 @@ describe Issues::CreateService, services: true do end it_behaves_like 'new issuable record that supports slash commands' + + context 'for a merge request' do + let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first } + let(:merge_request) { discussion.noteable } + let(:project) { merge_request.source_project } + let(:opts) { { merge_request_for_resolving_discussions: merge_request } } + + before do + project.team << [user, :master] + end + + it 'resolves the discussion for the merge request' do + described_class.new(project, user, opts).execute + discussion.first_note.reload + + expect(discussion.resolved?).to be(true) + end + + it 'added a system note to the discussion' do + described_class.new(project, user, opts).execute + + reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first + + expect(reloaded_discussion.last_note.system).to eq(true) + end + + it 'assigns the title and description for the issue' do + issue = described_class.new(project, user, opts).execute + + expect(issue.title).not_to be_nil + expect(issue.description).not_to be_nil + end + + it 'can set nil explicityly to the title and description' do + issue = described_class.new(project, user, + merge_request_for_resolving_discussions: merge_request, + description: nil, + title: nil).execute + + expect(issue.description).to be_nil + expect(issue.title).to be_nil + end + end end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index bc340ff9d3c..7e3705983fb 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -126,17 +126,27 @@ describe MergeRequests::RefreshService, services: true do end context 'push to fork repo target branch' do - before do - service.new(@fork_project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') - reload_mrs + describe 'changes to merge requests' do + before do + service.new(@fork_project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + reload_mrs + end + + it { expect(@merge_request.notes).to be_empty } + it { expect(@merge_request).to be_open } + it { expect(@fork_merge_request.notes).to be_empty } + it { expect(@fork_merge_request).to be_open } + it { expect(@build_failed_todo).to be_pending } + it { expect(@fork_build_failed_todo).to be_pending } end - it { expect(@merge_request.notes).to be_empty } - it { expect(@merge_request).to be_open } - it { expect(@fork_merge_request.notes).to be_empty } - it { expect(@fork_merge_request).to be_open } - it { expect(@build_failed_todo).to be_pending } - it { expect(@fork_build_failed_todo).to be_pending } + describe 'merge request diff' do + it 'does not reload the diff of the merge request made from fork' do + expect do + service.new(@fork_project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + end.not_to change { @fork_merge_request.reload.merge_request_diff } + end + end end context 'push to origin repo target branch after fork project was removed' do diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 4bffe622d72..90b7e62bc6f 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -712,4 +712,32 @@ describe SystemNoteService, services: true do end end end + + describe '.discussion_continued_in_issue' do + let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first } + let(:merge_request) { discussion.noteable } + let(:project) { merge_request.source_project } + let(:issue) { create(:issue, project: project) } + let(:user) { create(:user) } + + def reloaded_merge_request + MergeRequest.find(merge_request.id) + end + + before do + project.team << [user, :developer] + end + + it 'creates a new note in the discussion' do + # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded. + expect { SystemNoteService.discussion_continued_in_issue(discussion, project, user, issue) }. + to change { reloaded_merge_request.discussions.first.notes.size }.by(1) + end + + it 'mentions the created issue in the system note' do + note = SystemNoteService.discussion_continued_in_issue(discussion, project, user, issue) + + expect(note.note).to include(issue.to_reference) + end + end end diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb index c0b3e83244d..ad1eed5b369 100644 --- a/spec/support/login_helpers.rb +++ b/spec/support/login_helpers.rb @@ -75,7 +75,8 @@ module LoginHelpers def logout find(".header-user-dropdown-toggle").click click_link "Sign out" - expect(page).to have_content('Signed out successfully') + # check the sign_in button + expect(page).to have_button('Sign in') end # Logout without JavaScript driver diff --git a/spec/support/matchers/is_within.rb b/spec/support/matchers/is_within.rb deleted file mode 100644 index 0c35fc7e899..00000000000 --- a/spec/support/matchers/is_within.rb +++ /dev/null @@ -1,9 +0,0 @@ -# Extend shoulda-matchers -module Shoulda::Matchers::ActiveModel - class ValidateLengthOfMatcher - # Shortcut for is_at_least and is_at_most - def is_within(range) - is_at_least(range.min) && is_at_most(range.max) - end - end -end diff --git a/spec/workers/pipeline_notification_worker_spec.rb b/spec/workers/pipeline_notification_worker_spec.rb index 739f9b63967..603ae52ed1e 100644 --- a/spec/workers/pipeline_notification_worker_spec.rb +++ b/spec/workers/pipeline_notification_worker_spec.rb @@ -11,7 +11,7 @@ describe PipelineNotificationWorker do status: status) end - let(:project) { create(:project) } + let(:project) { create(:project, public_builds: false) } let(:user) { create(:user) } let(:pusher) { user } let(:watcher) { pusher } |