diff options
Diffstat (limited to 'spec')
132 files changed, 3841 insertions, 897 deletions
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index c34475976c6..92b97bf3d0c 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -2,9 +2,10 @@ require 'spec_helper' describe Groups::GroupMembersController do let(:user) { create(:user) } - let(:group) { create(:group) } describe '#index' do + let(:group) { create(:group) } + before do group.add_owner(user) stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) diff --git a/spec/controllers/projects/boards/lists_controller_spec.rb b/spec/controllers/projects/boards/lists_controller_spec.rb index d687dea3c3b..709006a3601 100644 --- a/spec/controllers/projects/boards/lists_controller_spec.rb +++ b/spec/controllers/projects/boards/lists_controller_spec.rb @@ -20,10 +20,7 @@ describe Projects::Boards::ListsController do end it 'returns a list of board lists' do - board = project.create_board - create(:backlog_list, board: board) create(:list, board: board) - create(:done_list, board: board) read_board_list user: user diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 94c9edc91fe..742edd8ba3d 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -644,6 +644,20 @@ describe Projects::MergeRequestsController do end end + context 'POST remove_wip' do + it 'removes the wip status' do + merge_request.title = merge_request.wip_title + merge_request.save + + post :remove_wip, + namespace_id: merge_request.project.namespace.to_param, + project_id: merge_request.project.to_param, + id: merge_request.iid + + expect(merge_request.reload.title).to eq(merge_request.wipless_title) + end + end + context 'POST resolve_conflicts' do let(:json_response) { JSON.parse(response.body) } let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha } diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index 2fe3c263524..38e02a46626 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -8,7 +8,7 @@ describe Projects::RepositoriesController do it 'responds with redirect in correct format' do get :archive, namespace_id: project.namespace.path, project_id: project.path, format: "zip" - expect(response.content_type).to start_with 'text/html' + expect(response.header["Content-Type"]).to start_with('text/html') expect(response).to be_redirect end end diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb index 7b3a26d7ca7..19a152bcb05 100644 --- a/spec/controllers/projects/templates_controller_spec.rb +++ b/spec/controllers/projects/templates_controller_spec.rb @@ -13,7 +13,7 @@ describe Projects::TemplatesController do end before do - project.team.add_user(user, Gitlab::Access::MASTER) + project.add_user(user, Gitlab::Access::MASTER) project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false) end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index b0f740f48f7..da0fdce39db 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -63,6 +63,28 @@ describe ProjectsController do end end + context "project with broken repo" do + let(:empty_project) { create(:project_broken_repo, :public) } + + before { sign_in(user) } + + User.project_views.keys.each do |project_view| + context "with #{project_view} view set" do + before do + user.update_attributes(project_view: project_view) + + get :show, namespace_id: empty_project.namespace.path, id: empty_project.path + end + + it "renders the empty project view" do + allow(Project).to receive(:repo).and_raise(Gitlab::Git::Repository::NoRepository) + + expect(response).to render_template('projects/no_repo') + end + end + end + end + context "rendering default project view" do render_views diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 2a89159c070..41d263a46a4 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' describe SnippetsController do - describe 'GET #show' do - let(:user) { create(:user) } + let(:user) { create(:user) } + describe 'GET #show' do context 'when the personal snippet is private' do let(:personal_snippet) { create(:personal_snippet, :private, author: user) } @@ -230,4 +230,33 @@ describe SnippetsController do end end end + + context 'award emoji on snippets' do + let(:personal_snippet) { create(:personal_snippet, :public, author: user) } + let(:another_user) { create(:user) } + + before do + sign_in(another_user) + end + + describe 'POST #toggle_award_emoji' do + it "toggles the award emoji" do + expect do + post(:toggle_award_emoji, id: personal_snippet.to_param, name: "thumbsup") + end.to change { personal_snippet.award_emoji.count }.from(0).to(1) + + expect(response.status).to eq(200) + end + + it "removes the already awarded emoji" do + post(:toggle_award_emoji, id: personal_snippet.to_param, name: "thumbsup") + + expect do + post(:toggle_award_emoji, id: personal_snippet.to_param, name: "thumbsup") + end.to change { personal_snippet.award_emoji.count }.from(1).to(0) + + expect(response.status).to eq(200) + end + end + end end diff --git a/spec/factories/group_members.rb b/spec/factories/group_members.rb index 2044ebec09a..795df5dfda9 100644 --- a/spec/factories/group_members.rb +++ b/spec/factories/group_members.rb @@ -3,5 +3,11 @@ FactoryGirl.define do access_level { GroupMember::OWNER } group user + + trait(:guest) { access_level GroupMember::GUEST } + trait(:reporter) { access_level GroupMember::REPORTER } + trait(:developer) { access_level GroupMember::DEVELOPER } + trait(:master) { access_level GroupMember::MASTER } + trait(:owner) { access_level GroupMember::OWNER } end end diff --git a/spec/factories/milestones.rb b/spec/factories/milestones.rb index e9e85962fe4..84da71ed6dc 100644 --- a/spec/factories/milestones.rb +++ b/spec/factories/milestones.rb @@ -3,10 +3,15 @@ FactoryGirl.define do title project + trait :active do + state "active" + end + trait :closed do - state :closed + state "closed" end + factory :active_milestone, traits: [:active] factory :closed_milestone, traits: [:closed] end end diff --git a/spec/factories/project_members.rb b/spec/factories/project_members.rb index cf3659ba275..1ddb305a8af 100644 --- a/spec/factories/project_members.rb +++ b/spec/factories/project_members.rb @@ -4,24 +4,9 @@ FactoryGirl.define do project master - trait :guest do - access_level ProjectMember::GUEST - end - - trait :reporter do - access_level ProjectMember::REPORTER - end - - trait :developer do - access_level ProjectMember::DEVELOPER - end - - trait :master do - access_level ProjectMember::MASTER - end - - trait :owner do - access_level ProjectMember::OWNER - end + trait(:guest) { access_level ProjectMember::GUEST } + trait(:reporter) { access_level ProjectMember::REPORTER } + trait(:developer) { access_level ProjectMember::DEVELOPER } + trait(:master) { access_level ProjectMember::MASTER } end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index fb84ba07d25..873d3fcb5af 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -27,6 +27,14 @@ FactoryGirl.define do end end + trait :broken_repo do + after(:create) do |project| + project.create_repository + + FileUtils.rm_r(File.join(project.repository_storage_path, "#{project.path_with_namespace}.git", 'refs')) + end + end + # Nest Project Feature attributes transient do wiki_access_level ProjectFeature::ENABLED @@ -56,6 +64,13 @@ FactoryGirl.define do empty_repo end + # Project with broken repository + # + # Project with an invalid repository state + factory :project_broken_repo, parent: :empty_project do + broken_repo + end + # Project with test repository # # Test repository source can be found at @@ -106,6 +121,8 @@ FactoryGirl.define do factory :project_with_board, parent: :empty_project do after(:create) do |project| project.create_board + project.board.lists.create(list_type: :backlog) + project.board.lists.create(list_type: :done) end end end diff --git a/spec/features/boards/boards_spec.rb b/spec/features/boards/boards_spec.rb index 19941978c5f..26ea06e002b 100644 --- a/spec/features/boards/boards_spec.rb +++ b/spec/features/boards/boards_spec.rb @@ -4,15 +4,11 @@ describe 'Issue Boards', feature: true, js: true do include WaitForAjax include WaitForVueResource - let(:project) { create(:empty_project, :public) } + let(:project) { create(:project_with_board, :public) } let(:user) { create(:user) } let!(:user2) { create(:user) } before do - project.create_board - project.board.lists.create(list_type: :backlog) - project.board.lists.create(list_type: :done) - project.team << [user, :master] project.team << [user2, :master] @@ -62,6 +58,7 @@ describe 'Issue Boards', feature: true, js: true do let(:bug) { create(:label, project: project, name: 'Bug') } let!(:backlog) { create(:label, project: project, name: 'Backlog') } let!(:done) { create(:label, project: project, name: 'Done') } + let!(:accepting) { create(:label, project: project, name: 'Accepting Merge Requests') } let!(:list1) { create(:list, board: project.board, label: planning, position: 0) } let!(:list2) { create(:list, board: project.board, label: development, position: 1) } @@ -75,7 +72,7 @@ describe 'Issue Boards', feature: true, js: true do let!(:issue6) { create(:labeled_issue, project: project, labels: [planning, development]) } let!(:issue7) { create(:labeled_issue, project: project, labels: [development]) } let!(:issue8) { create(:closed_issue, project: project) } - let!(:issue9) { create(:labeled_issue, project: project, labels: [testing, bug]) } + let!(:issue9) { create(:labeled_issue, project: project, labels: [testing, bug, accepting]) } before do visit namespace_project_board_path(project.namespace, project) @@ -441,6 +438,57 @@ describe 'Issue Boards', feature: true, js: true do wait_for_empty_boards((2..4)) end + it 'filters by label with space after reload' do + page.within '.issues-filters' do + click_button('Label') + wait_for_ajax + + page.within '.dropdown-menu-labels' do + click_link(accepting.title) + wait_for_vue_resource(spinner: false) + find('.dropdown-menu-close').click + end + end + + # Test after reload + page.evaluate_script 'window.location.reload()' + + wait_for_vue_resource + + page.within(find('.board', match: :first)) do + expect(page.find('.board-header')).to have_content('1') + expect(page).to have_selector('.card', count: 1) + end + + page.within(find('.board:nth-child(2)')) do + expect(page.find('.board-header')).to have_content('0') + expect(page).to have_selector('.card', count: 0) + end + end + + it 'removes filtered labels' do + wait_for_vue_resource + + page.within '.labels-filter' do + click_button('Label') + wait_for_ajax + + page.within '.dropdown-menu-labels' do + click_link(testing.title) + wait_for_vue_resource(spinner: false) + end + + expect(page).to have_css('input[name="label_name[]"]', visible: false) + + page.within '.dropdown-menu-labels' do + click_link(testing.title) + wait_for_vue_resource(spinner: false) + end + + expect(page).not_to have_css('input[name="label_name[]"]', visible: false) + end + end + it 'infinite scrolls list with label filter' do 50.times do create(:labeled_issue, project: project, labels: [testing]) diff --git a/spec/features/calendar_spec.rb b/spec/features/calendar_spec.rb index fd5fbaf2af4..7fa0c95cae2 100644 --- a/spec/features/calendar_spec.rb +++ b/spec/features/calendar_spec.rb @@ -5,13 +5,43 @@ feature 'Contributions Calendar', js: true, feature: true do let(:contributed_project) { create(:project, :public) } - before do - login_as :user + # Ex/ Sunday Jan 1, 2016 + date_format = '%A %b %-d, %Y' + + issue_title = 'Bug in old browser' + issue_params = { title: issue_title } + + def get_cell_color_selector(contributions) + contribution_cell = '.user-contrib-cell' + activity_colors = Array['#ededed', '#acd5f2', '#7fa8c9', '#527ba0', '#254e77'] + activity_colors_index = 0 + + if contributions > 0 && contributions < 10 + activity_colors_index = 1 + elsif contributions >= 10 && contributions < 20 + activity_colors_index = 2 + elsif contributions >= 20 && contributions < 30 + activity_colors_index = 3 + elsif contributions >= 30 + activity_colors_index = 4 + end + + "#{contribution_cell}[fill='#{activity_colors[activity_colors_index]}']" + end - issue_params = { title: 'Bug in old browser' } - Issues::CreateService.new(contributed_project, @user, issue_params).execute + def get_cell_date_selector(contributions, date) + contribution_text = 'No contributions' - # Push code contribution + if contributions === 1 + contribution_text = '1 contribution' + elsif contributions > 1 + contribution_text = "#{contributions} contributions" + end + + "#{get_cell_color_selector(contributions)}[data-original-title='#{contribution_text}<br />#{date}']" + end + + def push_code_contribution push_params = { project: contributed_project, action: Event::PUSHED, @@ -20,7 +50,10 @@ feature 'Contributions Calendar', js: true, feature: true do } Event.create(push_params) + end + before do + login_as :user visit @user.username wait_for_ajax end @@ -29,11 +62,71 @@ feature 'Contributions Calendar', js: true, feature: true do expect(page).to have_css('.js-contrib-calendar') end - it 'displays calendar activity log', js: true do - expect(find('.content_list .event-note')).to have_content "Bug in old browser" + describe '1 calendar activity' do + before do + Issues::CreateService.new(contributed_project, @user, issue_params).execute + visit @user.username + wait_for_ajax + end + + it 'displays calendar activity log', js: true do + expect(find('.content_list .event-note')).to have_content issue_title + end + + it 'displays calendar activity square color for 1 contribution', js: true do + expect(page).to have_selector(get_cell_color_selector(1), count: 1) + end + + it 'displays calendar activity square on the correct date', js: true do + today = Date.today.strftime(date_format) + expect(page).to have_selector(get_cell_date_selector(1, today), count: 1) + end end - it 'displays calendar activity square color', js: true do - expect(page).to have_selector('.user-contrib-cell[fill=\'#acd5f2\']', count: 1) + describe '10 calendar activities' do + before do + (0..9).each do |i| + push_code_contribution() + end + + visit @user.username + wait_for_ajax + end + + it 'displays calendar activity square color for 10 contributions', js: true do + expect(page).to have_selector(get_cell_color_selector(10), count: 1) + end + + it 'displays calendar activity square on the correct date', js: true do + today = Date.today.strftime(date_format) + expect(page).to have_selector(get_cell_date_selector(10, today), count: 1) + end + end + + describe 'calendar activity on two days' do + before do + push_code_contribution() + + Timecop.freeze(Date.yesterday) + Issues::CreateService.new(contributed_project, @user, issue_params).execute + Timecop.return + + visit @user.username + wait_for_ajax + end + + it 'displays calendar activity squares for both days', js: true do + expect(page).to have_selector(get_cell_color_selector(1), count: 2) + end + + it 'displays calendar activity square for yesterday', js: true do + yesterday = Date.yesterday.strftime(date_format) + expect(page).to have_selector(get_cell_date_selector(1, yesterday), count: 1) + end + + it 'displays calendar activity square for today', js: true do + today = Date.today.strftime(date_format) + expect(page).to have_selector(get_cell_date_selector(1, today), count: 1) + end end end diff --git a/spec/features/compare_spec.rb b/spec/features/compare_spec.rb index ca7f73e24cc..33dfd0d5b62 100644 --- a/spec/features/compare_spec.rb +++ b/spec/features/compare_spec.rb @@ -12,15 +12,16 @@ describe "Compare", js: true do describe "branches" do it "pre-populates fields" do - expect(page.find_field("from").value).to eq("master") + expect(find(".js-compare-from-dropdown .dropdown-toggle-text")).to have_content("master") + expect(find(".js-compare-to-dropdown .dropdown-toggle-text")).to have_content("master") end it "compares branches" do - fill_in "from", with: "fea" - find("#from").click + select_using_dropdown "from", "feature" + expect(find(".js-compare-from-dropdown .dropdown-toggle-text")).to have_content("feature") - click_link "feature" - expect(page.find_field("from").value).to eq("feature") + select_using_dropdown "to", "binary-encoding" + expect(find(".js-compare-to-dropdown .dropdown-toggle-text")).to have_content("binary-encoding") click_button "Compare" expect(page).to have_content "Commits" @@ -29,14 +30,21 @@ describe "Compare", js: true do describe "tags" do it "compares tags" do - fill_in "from", with: "v1.0" - find("#from").click + select_using_dropdown "from", "v1.0.0" + expect(find(".js-compare-from-dropdown .dropdown-toggle-text")).to have_content("v1.0.0") - click_link "v1.0.0" - expect(page.find_field("from").value).to eq("v1.0.0") + select_using_dropdown "to", "v1.1.0" + expect(find(".js-compare-to-dropdown .dropdown-toggle-text")).to have_content("v1.1.0") click_button "Compare" expect(page).to have_content "Commits" end end + + def select_using_dropdown(dropdown_type, selection) + dropdown = find(".js-compare-#{dropdown_type}-dropdown") + dropdown.find(".compare-dropdown-toggle").click + dropdown.fill_in("Filter by branch/tag", with: selection) + click_link selection + end end diff --git a/spec/features/dashboard/snippets_spec.rb b/spec/features/dashboard/snippets_spec.rb new file mode 100644 index 00000000000..62937688c22 --- /dev/null +++ b/spec/features/dashboard/snippets_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe 'Dashboard snippets', feature: true do + context 'when the project has snippets' do + let(:project) { create(:empty_project, :public) } + let!(:snippets) { create_list(:project_snippet, 2, :public, author: project.owner, project: project) } + before do + allow(Snippet).to receive(:default_per_page).and_return(1) + login_as(project.owner) + visit dashboard_snippets_path + end + + it_behaves_like 'paginated snippets' + end +end diff --git a/spec/features/dashboard_issues_spec.rb b/spec/features/dashboard_issues_spec.rb index fc914022a59..9b54b5301e5 100644 --- a/spec/features/dashboard_issues_spec.rb +++ b/spec/features/dashboard_issues_spec.rb @@ -21,9 +21,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do click_link 'No Milestone' - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '1') - end + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) expect(page).to have_selector('.issue', count: 1) end @@ -32,9 +30,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do click_link 'Any Milestone' - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '2') - end + expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2) expect(page).to have_selector('.issue', count: 2) end @@ -45,9 +41,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do click_link milestone.title end - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '1') - end + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) expect(page).to have_selector('.issue', count: 1) end end diff --git a/spec/features/expand_collapse_diffs_spec.rb b/spec/features/expand_collapse_diffs_spec.rb index 8863554ee91..6c938bdead8 100644 --- a/spec/features/expand_collapse_diffs_spec.rb +++ b/spec/features/expand_collapse_diffs_spec.rb @@ -68,7 +68,7 @@ feature 'Expand and collapse diffs', js: true, feature: true do context 'expanding a diff for a renamed file' do before do - large_diff_renamed.find('.nothing-here-block').click + large_diff_renamed.find('.click-to-expand').click wait_for_ajax end @@ -87,7 +87,10 @@ feature 'Expand and collapse diffs', js: true, feature: true do context 'expanding a large diff' do before do - click_link('large_diff.md') + # Wait for diffs + find('.file-title', match: :first) + # Click `large_diff.md` title + all('.file-title')[1].click wait_for_ajax end @@ -128,7 +131,10 @@ feature 'Expand and collapse diffs', js: true, feature: true do context 'expanding the diff' do before do - click_link('large_diff.md') + # Wait for diffs + find('.file-title', match: :first) + # Click `large_diff.md` title + all('.file-title')[1].click wait_for_ajax end @@ -146,7 +152,12 @@ feature 'Expand and collapse diffs', js: true, feature: true do end context 'collapsing an expanded diff' do - before { click_link('small_diff.md') } + before do + # Wait for diffs + find('.file-title', match: :first) + # Click `small_diff.md` title + all('.file-title')[3].click + end it 'hides the diff content' do expect(small_diff).not_to have_selector('.code') @@ -154,7 +165,12 @@ feature 'Expand and collapse diffs', js: true, feature: true do end context 're-expanding the same diff' do - before { click_link('small_diff.md') } + before do + # Wait for diffs + find('.file-title', match: :first) + # Click `small_diff.md` title + all('.file-title')[3].click + end it 'shows the diff content' do expect(small_diff).to have_selector('.code') @@ -231,7 +247,12 @@ feature 'Expand and collapse diffs', js: true, feature: true do end context 'collapsing an expanded diff' do - before { click_link('small_diff.md') } + before do + # Wait for diffs + find('.file-title', match: :first) + # Click `small_diff.md` title + all('.file-title')[3].click + end it 'hides the diff content' do expect(small_diff).not_to have_selector('.code') @@ -239,7 +260,12 @@ feature 'Expand and collapse diffs', js: true, feature: true do end context 're-expanding the same diff' do - before { click_link('small_diff.md') } + before do + # Wait for diffs + find('.file-title', match: :first) + # Click `small_diff.md` title + all('.file-title')[3].click + end it 'shows the diff content' do expect(small_diff).to have_selector('.code') diff --git a/spec/features/issues/filter_by_labels_spec.rb b/spec/features/issues/filter_by_labels_spec.rb index 908b18e5339..0253629f753 100644 --- a/spec/features/issues/filter_by_labels_spec.rb +++ b/spec/features/issues/filter_by_labels_spec.rb @@ -1,10 +1,10 @@ require 'rails_helper' -feature 'Issue filtering by Labels', feature: true do +feature 'Issue filtering by Labels', feature: true, js: true do include WaitForAjax let(:project) { create(:project, :public) } - let!(:user) { create(:user)} + let!(:user) { create(:user) } let!(:label) { create(:label, project: project) } before do @@ -28,156 +28,81 @@ feature 'Issue filtering by Labels', feature: true do visit namespace_project_issues_path(project.namespace, project) end - context 'filter by label bug', js: true do + context 'filter by label bug' do before do - page.find('.js-label-select').click - wait_for_ajax - execute_script("$('.dropdown-menu-labels li:contains(\"bug\") a').click()") - page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click - wait_for_ajax + select_labels('bug') end - it 'shows issue "Bugfix1" and "Bugfix2" in issues list' do + it 'apply the filter' do expect(page).to have_content "Bugfix1" expect(page).to have_content "Bugfix2" - end - - it 'does not show "Feature1" in issues list' do expect(page).not_to have_content "Feature1" - end - - it 'shows label "bug" in filtered-labels' do expect(find('.filtered-labels')).to have_content "bug" - end - - it 'does not show label "feature" and "enhancement" in filtered-labels' do expect(find('.filtered-labels')).not_to have_content "feature" expect(find('.filtered-labels')).not_to have_content "enhancement" - end - it 'removes label "bug"' do find('.js-label-filter-remove').click wait_for_ajax expect(find('.filtered-labels', visible: false)).to have_no_content "bug" end end - context 'filter by label feature', js: true do + context 'filter by label feature' do before do - page.find('.js-label-select').click - wait_for_ajax - execute_script("$('.dropdown-menu-labels li:contains(\"feature\") a').click()") - page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click - wait_for_ajax + select_labels('feature') end - it 'shows issue "Feature1" in issues list' do + it 'applies the filter' do expect(page).to have_content "Feature1" - end - - it 'does not show "Bugfix1" and "Bugfix2" in issues list' do expect(page).not_to have_content "Bugfix2" expect(page).not_to have_content "Bugfix1" - end - - it 'shows label "feature" in filtered-labels' do expect(find('.filtered-labels')).to have_content "feature" - end - - it 'does not show label "bug" and "enhancement" in filtered-labels' do expect(find('.filtered-labels')).not_to have_content "bug" expect(find('.filtered-labels')).not_to have_content "enhancement" end end - context 'filter by label enhancement', js: true do + context 'filter by label enhancement' do before do - page.find('.js-label-select').click - wait_for_ajax - execute_script("$('.dropdown-menu-labels li:contains(\"enhancement\") a').click()") - page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click - wait_for_ajax + select_labels('enhancement') end - it 'shows issue "Bugfix2" in issues list' do + it 'applies the filter' do expect(page).to have_content "Bugfix2" - end - - it 'does not show "Feature1" and "Bugfix1" in issues list' do expect(page).not_to have_content "Feature1" expect(page).not_to have_content "Bugfix1" - end - - it 'shows label "enhancement" in filtered-labels' do expect(find('.filtered-labels')).to have_content "enhancement" - end - - it 'does not show label "feature" and "bug" in filtered-labels' do expect(find('.filtered-labels')).not_to have_content "bug" expect(find('.filtered-labels')).not_to have_content "feature" end end - context 'filter by label enhancement or feature', js: true do + context 'filter by label enhancement and bug in issues list' do before do - page.find('.js-label-select').click - wait_for_ajax - execute_script("$('.dropdown-menu-labels li:contains(\"enhancement\") a').click()") - execute_script("$('.dropdown-menu-labels li:contains(\"feature\") a').click()") - page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click - wait_for_ajax + select_labels('bug', 'enhancement') end - it 'does not show "Bugfix1" or "Feature1" in issues list' do - expect(page).not_to have_content "Bugfix1" + it 'applies the filters' do + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).to have_content "Bugfix2" expect(page).not_to have_content "Feature1" - end - - it 'shows label "enhancement" and "feature" in filtered-labels' do + expect(find('.filtered-labels')).to have_content "bug" expect(find('.filtered-labels')).to have_content "enhancement" - expect(find('.filtered-labels')).to have_content "feature" - end - - it 'does not show label "bug" in filtered-labels' do - expect(find('.filtered-labels')).not_to have_content "bug" - end + expect(find('.filtered-labels')).not_to have_content "feature" - it 'removes label "enhancement"' do find('.js-label-filter-remove', match: :first).click wait_for_ajax - expect(find('.filtered-labels')).to have_no_content "enhancement" - end - end - - context 'filter by label enhancement and bug in issues list', js: true do - before do - page.find('.js-label-select').click - wait_for_ajax - execute_script("$('.dropdown-menu-labels li:contains(\"enhancement\") a').click()") - execute_script("$('.dropdown-menu-labels li:contains(\"bug\") a').click()") - page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click - wait_for_ajax - end - it 'shows issue "Bugfix2" in issues list' do expect(page).to have_content "Bugfix2" - end - - it 'does not show "Feature1"' do expect(page).not_to have_content "Feature1" - end - - it 'shows label "bug" and "enhancement" in filtered-labels' do - expect(find('.filtered-labels')).to have_content "bug" + expect(page).not_to have_content "Bugfix1" + expect(find('.filtered-labels')).not_to have_content "bug" expect(find('.filtered-labels')).to have_content "enhancement" - end - - it 'does not show label "feature" in filtered-labels' do expect(find('.filtered-labels')).not_to have_content "feature" end end - context 'remove filtered labels', js: true do + context 'remove filtered labels' do before do page.within '.labels-filter' do click_button 'Label' @@ -200,7 +125,7 @@ feature 'Issue filtering by Labels', feature: true do end end - context 'dropdown filtering', js: true do + context 'dropdown filtering' do it 'filters by label name' do page.within '.labels-filter' do click_button 'Label' @@ -214,4 +139,14 @@ feature 'Issue filtering by Labels', feature: true do end end end + + def select_labels(*labels) + page.find('.js-label-select').click + wait_for_ajax + labels.each do |label| + execute_script("$('.dropdown-menu-labels li:contains(\"#{label}\") a').click()") + end + page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click + wait_for_ajax + end end diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index 72f39e2fbca..8d19198efd3 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -7,15 +7,15 @@ describe 'Filter issues', feature: true do let!(:user) { create(:user)} let!(:milestone) { create(:milestone, project: project) } let!(:label) { create(:label, project: project) } - let!(:issue1) { create(:issue, project: project) } let!(:wontfix) { create(:label, project: project, title: "Won't fix") } before do project.team << [user, :master] login_as(user) + create(:issue, project: project) end - describe 'Filter issues for assignee from issues#index' do + describe 'for assignee from issues#index' do before do visit namespace_project_issues_path(project.namespace, project) @@ -45,7 +45,7 @@ describe 'Filter issues', feature: true do end end - describe 'Filter issues for milestone from issues#index' do + describe 'for milestone from issues#index' do before do visit namespace_project_issues_path(project.namespace, project) @@ -75,7 +75,7 @@ describe 'Filter issues', feature: true do end end - describe 'Filter issues for label from issues#index', js: true do + describe 'for label from issues#index', js: true do before do visit namespace_project_issues_path(project.namespace, project) find('.js-label-select').click @@ -115,6 +115,7 @@ describe 'Filter issues', feature: true do expect(page).to have_content wontfix.title click_link wontfix.title end + expect(find('.js-label-select .dropdown-toggle-text')).to have_content(wontfix.title) end @@ -146,7 +147,7 @@ describe 'Filter issues', feature: true do end end - describe 'Filter issues for assignee and label from issues#index' do + describe 'for assignee and label from issues#index' do before do visit namespace_project_issues_path(project.namespace, project) @@ -226,102 +227,78 @@ describe 'Filter issues', feature: true do it 'filters by text and label' do fill_in 'issuable_search', with: 'Bug' + expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2) page.within '.issues-list' do expect(page).to have_selector('.issue', count: 2) end - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '2') - end - click_button 'Label' page.within '.labels-filter' do click_link 'bug' end find('.dropdown-menu-close-icon').click + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) page.within '.issues-list' do expect(page).to have_selector('.issue', count: 1) end - - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '1') - end end it 'filters by text and milestone' do fill_in 'issuable_search', with: 'Bug' + expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2) page.within '.issues-list' do expect(page).to have_selector('.issue', count: 2) end - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '2') - end - click_button 'Milestone' page.within '.milestone-filter' do click_link '8' end + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) page.within '.issues-list' do expect(page).to have_selector('.issue', count: 1) end - - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '1') - end end it 'filters by text and assignee' do fill_in 'issuable_search', with: 'Bug' + expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2) page.within '.issues-list' do expect(page).to have_selector('.issue', count: 2) end - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '2') - end - click_button 'Assignee' page.within '.dropdown-menu-assignee' do click_link user.name end + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) page.within '.issues-list' do expect(page).to have_selector('.issue', count: 1) end - - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '1') - end end it 'filters by text and author' do fill_in 'issuable_search', with: 'Bug' + expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2) page.within '.issues-list' do expect(page).to have_selector('.issue', count: 2) end - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '2') - end - click_button 'Author' page.within '.dropdown-menu-author' do click_link user.name end + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) page.within '.issues-list' do expect(page).to have_selector('.issue', count: 1) end - - page.within '.issues-state-filters' do - expect(page).to have_selector('.active .badge', text: '1') - end end end end @@ -347,6 +324,7 @@ describe 'Filter issues', feature: true do find('.dropdown-menu-close-icon').click wait_for_ajax + expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2) page.within '.issues-list' do expect(page).to have_selector('.issue', count: 2) end diff --git a/spec/features/issues/user_uses_slash_commands_spec.rb b/spec/features/issues/user_uses_slash_commands_spec.rb index 105629c485a..3f2da1c380c 100644 --- a/spec/features/issues/user_uses_slash_commands_spec.rb +++ b/spec/features/issues/user_uses_slash_commands_spec.rb @@ -25,32 +25,88 @@ feature 'Issues > User uses slash commands', feature: true, js: true do describe 'adding a due date from note' do let(:issue) { create(:issue, project: project) } - it 'does not create a note, and sets the due date accordingly' do - write_note("/due 2016-08-28") + context 'when the current user can update the due date' do + it 'does not create a note, and sets the due date accordingly' do + write_note("/due 2016-08-28") - expect(page).not_to have_content '/due 2016-08-28' - expect(page).to have_content 'Your commands have been executed!' + expect(page).not_to have_content '/due 2016-08-28' + expect(page).to have_content 'Your commands have been executed!' - issue.reload + issue.reload - expect(issue.due_date).to eq Date.new(2016, 8, 28) + expect(issue.due_date).to eq Date.new(2016, 8, 28) + end + end + + context 'when the current user cannot update the due date' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + logout + login_with(guest) + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it 'does not create a note, and sets the due date accordingly' do + write_note("/due 2016-08-28") + + expect(page).to have_content '/due 2016-08-28' + expect(page).not_to have_content 'Your commands have been executed!' + + issue.reload + + expect(issue.due_date).to be_nil + end end end describe 'removing a due date from note' do let(:issue) { create(:issue, project: project, due_date: Date.new(2016, 8, 28)) } - it 'does not create a note, and removes the due date accordingly' do - expect(issue.due_date).to eq Date.new(2016, 8, 28) + context 'when the current user can update the due date' do + it 'does not create a note, and removes the due date accordingly' do + expect(issue.due_date).to eq Date.new(2016, 8, 28) + + write_note("/remove_due_date") + + expect(page).not_to have_content '/remove_due_date' + expect(page).to have_content 'Your commands have been executed!' + + issue.reload - write_note("/remove_due_date") + expect(issue.due_date).to be_nil + end + end + + context 'when the current user cannot update the due date' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + logout + login_with(guest) + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it 'does not create a note, and sets the due date accordingly' do + write_note("/remove_due_date") + + expect(page).to have_content '/remove_due_date' + expect(page).not_to have_content 'Your commands have been executed!' + + issue.reload - expect(page).not_to have_content '/remove_due_date' - expect(page).to have_content 'Your commands have been executed!' + expect(issue.due_date).to eq Date.new(2016, 8, 28) + end + end + end + + describe 'toggling the WIP prefix from the title from note' do + let(:issue) { create(:issue, project: project) } - issue.reload + it 'does not recognize the command nor create a note' do + write_note("/wip") - expect(issue.due_date).to be_nil + expect(page).not_to have_content '/wip' end end end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 22359c8f938..9fe40ea0892 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -369,6 +369,24 @@ describe 'Issues', feature: true do end end + describe 'update labels from issue#show', js: true do + let(:issue) { create(:issue, project: project, author: @user, assignee: @user) } + let!(:label) { create(:label, project: project) } + + before do + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it 'will not send ajax request when no data is changed' do + page.within '.labels' do + click_link 'Edit' + first('.dropdown-menu-close').click + + expect(page).not_to have_selector('.block-loading') + end + end + end + describe 'update assignee from issue#show' do let(:issue) { create(:issue, project: project, author: @user, assignee: @user) } diff --git a/spec/features/merge_requests/filter_by_milestone_spec.rb b/spec/features/merge_requests/filter_by_milestone_spec.rb index bb0bb590a46..d917d5950ec 100644 --- a/spec/features/merge_requests/filter_by_milestone_spec.rb +++ b/spec/features/merge_requests/filter_by_milestone_spec.rb @@ -17,6 +17,7 @@ feature 'Merge Request filtering by Milestone', feature: true do visit_merge_requests(project) filter_by_milestone(Milestone::None.title) + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) expect(page).to have_css('.merge-request', count: 1) end @@ -39,6 +40,7 @@ feature 'Merge Request filtering by Milestone', feature: true do visit_merge_requests(project) filter_by_milestone(Milestone::Upcoming.title) + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) expect(page).to have_css('.merge-request', count: 1) end @@ -61,6 +63,7 @@ feature 'Merge Request filtering by Milestone', feature: true do visit_merge_requests(project) filter_by_milestone(milestone.title) + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) expect(page).to have_css('.merge-request', count: 1) end diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb index 22d9e42119d..23cee891bac 100644 --- a/spec/features/merge_requests/merge_request_versions_spec.rb +++ b/spec/features/merge_requests/merge_request_versions_spec.rb @@ -1,12 +1,13 @@ require 'spec_helper' feature 'Merge Request versions', js: true, feature: true do + let(:merge_request) { create(:merge_request, importing: true) } + let(:project) { merge_request.source_project } + before do login_as :admin - merge_request = create(:merge_request, importing: true) merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') - project = merge_request.source_project visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) end @@ -47,6 +48,16 @@ feature 'Merge Request versions', js: true, feature: true do end end + it 'has a path with comparison context' do + expect(page).to have_current_path diffs_namespace_project_merge_request_path( + project.namespace, + project, + merge_request.iid, + diff_id: 2, + start_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' + ) + end + it 'should have correct value in the compare dropdown' do page.within '.mr-version-compare-dropdown' do expect(page).to have_content 'version 1' @@ -61,10 +72,6 @@ feature 'Merge Request versions', js: true, feature: true do expect(page).to have_content '4 changed files with 15 additions and 6 deletions' end - it 'show diff between new and old version' do - expect(page).to have_content '4 changed files with 15 additions and 6 deletions' - end - it 'should return to latest version when "Show latest version" button is clicked' do click_link 'Show latest version' page.within '.mr-version-dropdown' do diff --git a/spec/features/merge_requests/user_uses_slash_commands_spec.rb b/spec/features/merge_requests/user_uses_slash_commands_spec.rb index 22d9d1b9fd5..cb3cea3fd51 100644 --- a/spec/features/merge_requests/user_uses_slash_commands_spec.rb +++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb @@ -14,21 +14,66 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do let(:new_url_opts) { { merge_request: { source_branch: 'feature' } } } end - describe 'adding a due date from note' do + describe 'merge-request-only commands' do before do project.team << [user, :master] login_with(user) visit namespace_project_merge_request_path(project.namespace, project, merge_request) end - + after do wait_for_ajax end - it 'does not recognize the command nor create a note' do - write_note("/due 2016-08-28") + describe 'toggling the WIP prefix in the title from note' do + context 'when the current user can toggle the WIP prefix' do + it 'adds the WIP: prefix to the title' do + write_note("/wip") + + expect(page).not_to have_content '/wip' + expect(page).to have_content 'Your commands have been executed!' + + expect(merge_request.reload.work_in_progress?).to eq true + end + + it 'removes the WIP: prefix from the title' do + merge_request.title = merge_request.wip_title + merge_request.save + write_note("/wip") + + expect(page).not_to have_content '/wip' + expect(page).to have_content 'Your commands have been executed!' + + expect(merge_request.reload.work_in_progress?).to eq false + end + end + + context 'when the current user cannot toggle the WIP prefix' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + logout + login_with(guest) + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'does not change the WIP prefix' do + write_note("/wip") + + expect(page).not_to have_content '/wip' + expect(page).not_to have_content 'Your commands have been executed!' + + expect(merge_request.reload.work_in_progress?).to eq false + end + end + end + + describe 'adding a due date from note' do + it 'does not recognize the command nor create a note' do + write_note('/due 2016-08-28') - expect(page).not_to have_content '/due 2016-08-28' + expect(page).not_to have_content '/due 2016-08-28' + end end end end diff --git a/spec/features/profiles/keys_spec.rb b/spec/features/profiles/keys_spec.rb index 3b20d38c520..eb1050d21c6 100644 --- a/spec/features/profiles/keys_spec.rb +++ b/spec/features/profiles/keys_spec.rb @@ -1,18 +1,57 @@ require 'rails_helper' -describe 'Profile > SSH Keys', feature: true do +feature 'Profile > SSH Keys', feature: true do let(:user) { create(:user) } before do login_as(user) - visit profile_keys_path end - describe 'User adds an SSH key' do - it 'auto-populates the title', js: true do + describe 'User adds a key' do + before do + visit profile_keys_path + end + + scenario 'auto-populates the title', js: true do fill_in('Key', with: attributes_for(:key).fetch(:key)) expect(find_field('Title').value).to eq 'dummy@gitlab.com' end + + scenario 'saves the new key' do + attrs = attributes_for(:key) + + fill_in('Key', with: attrs[:key]) + fill_in('Title', with: attrs[:title]) + click_button('Add key') + + expect(page).to have_content("Title: #{attrs[:title]}") + expect(page).to have_content(attrs[:key]) + end + end + + scenario 'User sees their keys' do + key = create(:key, user: user) + visit profile_keys_path + + expect(page).to have_content(key.title) + end + + scenario 'User removes a key via the key index' do + create(:key, user: user) + visit profile_keys_path + + click_link('Remove') + + expect(page).to have_content('Your SSH keys (0)') + end + + scenario 'User removes a key via its details page' do + key = create(:key, user: user) + visit profile_key_path(key) + + click_link('Remove') + + expect(page).to have_content('Your SSH keys (0)') end end diff --git a/spec/features/projects/files/edit_file_soft_wrap_spec.rb b/spec/features/projects/files/edit_file_soft_wrap_spec.rb new file mode 100644 index 00000000000..012befa7990 --- /dev/null +++ b/spec/features/projects/files/edit_file_soft_wrap_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +feature 'User uses soft wrap whilst editing file', feature: true, js: true do + before do + user = create(:user) + project = create(:project) + project.team << [user, :master] + login_as user + visit namespace_project_new_blob_path(project.namespace, project, 'master', file_name: 'test_file-name') + editor = find('.file-editor.code') + editor.click + editor.send_keys 'Touch water with paw then recoil in horror chase dog then + run away chase the pig around the house eat owner\'s food, and knock + dish off table head butt cant eat out of my own dish. Cat is love, cat + is life rub face on everything poop on grasses so meow. Playing with + balls of wool flee in terror at cucumber discovered on floor run in + circles tuxedo cats always looking dapper, but attack dog, run away + and pretend to be victim so all of a sudden cat goes crazy, yet chase + laser. Make muffins sit in window and stare ooo, a bird! yum lick yarn + hanging out of own butt jump off balcony, onto stranger\'s head yet + chase laser. Purr for no reason stare at ceiling hola te quiero.'.squish + end + + let(:toggle_button) { find('.soft-wrap-toggle') } + + scenario 'user clicks the "Soft wrap" button and then "No wrap" button' do + wrapped_content_width = get_content_width + toggle_button.click + expect(toggle_button).to have_content 'No wrap' + unwrapped_content_width = get_content_width + expect(unwrapped_content_width).to be < wrapped_content_width + + toggle_button.click + expect(toggle_button).to have_content 'Soft wrap' + expect(get_content_width).to be > unwrapped_content_width + end + + def get_content_width + find('.ace_content')[:style].slice!(/width: \d+/).slice!(/\d+/) + end +end diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb new file mode 100644 index 00000000000..27c986c5187 --- /dev/null +++ b/spec/features/projects/import_export/export_file_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +# Integration test that exports a file using the Import/Export feature +# It looks up for any sensitive word inside the JSON, so if a sensitive word is found +# we''l have to either include it adding the model that includes it to the +safe_list+ +# or make sure the attribute is blacklisted in the +import_export.yml+ configuration +feature 'Import/Export - project export integration test', feature: true, js: true do + include Select2Helper + include ExportFileHelper + + let(:user) { create(:admin) } + let(:export_path) { "#{Dir::tmpdir}/import_file_spec" } + let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys } + + let(:sensitive_words) { %w[pass secret token key] } + let(:safe_list) do + { + token: [ProjectHook, Ci::Trigger, CommitStatus], + key: [Project, Ci::Variable, :yaml_variables] + } + end + let(:safe_hashes) { { yaml_variables: %w[key value public] } } + + let(:project) { setup_project } + + background do + allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + end + + after do + FileUtils.rm_rf(export_path, secure: true) + end + + context 'admin user' do + before do + login_as(user) + end + + scenario 'exports a project successfully' do + visit edit_namespace_project_path(project.namespace, project) + + expect(page).to have_content('Export project') + + click_link 'Export project' + + visit edit_namespace_project_path(project.namespace, project) + + expect(page).to have_content('Download export') + + in_directory_with_expanded_export(project) do |exit_status, tmpdir| + expect(exit_status).to eq(0) + + project_json_path = File.join(tmpdir, 'project.json') + expect(File).to exist(project_json_path) + + project_hash = JSON.parse(IO.read(project_json_path)) + + sensitive_words.each do |sensitive_word| + found = find_sensitive_attributes(sensitive_word, project_hash) + + expect(found).to be_nil, failure_message(found.try(:key_found), found.try(:parent), sensitive_word) + end + end + end + + def failure_message(key_found, parent, sensitive_word) + <<-MSG + Found a new sensitive word <#{key_found}>, which is part of the hash #{parent.inspect} + + If you think this information shouldn't get exported, please exclude the model or attribute in IMPORT_EXPORT_CONFIG. + + Otherwise, please add the exception to +safe_list+ in CURRENT_SPEC using #{sensitive_word} as the key and the + correspondent hash or model as the value. + + IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file} + CURRENT_SPEC: #{__FILE__} + MSG + end + end +end diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index f707ccf4e93..f32834801a0 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'project import', feature: true, js: true do +feature 'Import/Export - project import integration test', feature: true, js: true do include Select2Helper let(:admin) { create(:admin) } @@ -86,14 +86,14 @@ feature 'project import', feature: true, js: true do login_as(normal_user) end - scenario 'non-admin user is not allowed to import a project' do + scenario 'non-admin user is allowed to import a project' do expect(Project.all.count).to be_zero visit new_project_path fill_in :project_path, with: 'test-project-path', visible: true - expect(page).not_to have_content('GitLab export') + expect(page).to have_content('GitLab export') end end diff --git a/spec/features/projects/members/owner_cannot_leave_project_spec.rb b/spec/features/projects/members/owner_cannot_leave_project_spec.rb index 67811b1048e..6e948b7a616 100644 --- a/spec/features/projects/members/owner_cannot_leave_project_spec.rb +++ b/spec/features/projects/members/owner_cannot_leave_project_spec.rb @@ -1,12 +1,10 @@ require 'spec_helper' feature 'Projects > Members > Owner cannot leave project', feature: true do - let(:owner) { create(:user) } let(:project) { create(:project) } background do - project.team << [owner, :owner] - login_as(owner) + login_as(project.owner) visit namespace_project_path(project.namespace, project) end diff --git a/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb b/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb index 0e54c4fdf20..4ca9272b9c1 100644 --- a/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb +++ b/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb @@ -1,12 +1,10 @@ require 'spec_helper' feature 'Projects > Members > Owner cannot request access to his project', feature: true do - let(:owner) { create(:user) } let(:project) { create(:project) } background do - project.team << [owner, :owner] - login_as(owner) + login_as(project.owner) visit namespace_project_path(project.namespace, project) end diff --git a/spec/features/projects/snippets_spec.rb b/spec/features/projects/snippets_spec.rb new file mode 100644 index 00000000000..d37e8ed4699 --- /dev/null +++ b/spec/features/projects/snippets_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe 'Project snippets', feature: true do + context 'when the project has snippets' do + let(:project) { create(:empty_project, :public) } + let!(:snippets) { create_list(:project_snippet, 2, :public, author: project.owner, project: project) } + before do + allow(Snippet).to receive(:default_per_page).and_return(1) + visit namespace_project_snippets_path(project.namespace, project) + end + + it_behaves_like 'paginated snippets' + end +end diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index 2242cb6236a..c30d38b6508 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -82,7 +82,7 @@ feature 'Project', feature: true do before do login_with(user) - project.team.add_user(user, Gitlab::Access::MASTER) + project.add_user(user, Gitlab::Access::MASTER) visit namespace_project_path(project.namespace, project) end @@ -101,8 +101,8 @@ feature 'Project', feature: true do context 'on issues page', js: true do before do login_with(user) - project.team.add_user(user, Gitlab::Access::MASTER) - project2.team.add_user(user, Gitlab::Access::MASTER) + project.add_user(user, Gitlab::Access::MASTER) + project2.add_user(user, Gitlab::Access::MASTER) visit namespace_project_issue_path(project.namespace, project, issue) end diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index a5ed3595b0a..0e1cc9a0f73 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -60,7 +60,7 @@ describe "Runners" do it "removes specific runner for project if this is last project for that runners" do within ".activated-specific-runners" do - click_on "Remove runner" + click_on "Remove Runner" end expect(Ci::Runner.exists?(id: @specific_runner)).to be_falsey @@ -75,7 +75,7 @@ describe "Runners" do end it "enables shared runners" do - click_on "Enable shared runners" + click_on "Enable shared Runners" expect(@project.reload.shared_runners_enabled).to be_truthy end end diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index dcd3a2f17b0..1806200c82c 100644 --- a/spec/features/search_spec.rb +++ b/spec/features/search_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe "Search", feature: true do + include WaitForAjax + let(:user) { create(:user) } let(:project) { create(:project, namespace: user.namespace) } let!(:issue) { create(:issue, project: project, assignee: user) } @@ -16,6 +18,36 @@ describe "Search", feature: true do expect(page).not_to have_selector('.search') end + context 'search filters', js: true do + let(:group) { create(:group) } + + before do + group.add_owner(user) + end + + it 'shows group name after filtering' do + find('.js-search-group-dropdown').click + wait_for_ajax + + page.within '.search-holder' do + click_link group.name + end + + expect(find('.js-search-group-dropdown')).to have_content(group.name) + end + + it 'shows project name after filtering' do + page.within('.project-filter') do + find('.js-search-project-dropdown').click + wait_for_ajax + + click_link project.name_with_namespace + end + + expect(find('.js-search-project-dropdown')).to have_content(project.name_with_namespace) + end + end + describe 'searching for Projects' do it 'finds a project' do page.within '.search-holder' do diff --git a/spec/features/snippets_spec.rb b/spec/features/snippets_spec.rb new file mode 100644 index 00000000000..70b16bfc810 --- /dev/null +++ b/spec/features/snippets_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe 'Snippets', feature: true do + context 'when the project has snippets' do + let(:project) { create(:empty_project, :public) } + let!(:snippets) { create_list(:project_snippet, 2, :public, author: project.owner, project: project) } + before do + allow(Snippet).to receive(:default_per_page).and_return(1) + visit snippets_path(username: project.owner.username) + end + + it_behaves_like 'paginated snippets' + end +end diff --git a/spec/features/unsubscribe_links_spec.rb b/spec/features/unsubscribe_links_spec.rb index cc40671787c..33b52d1547e 100644 --- a/spec/features/unsubscribe_links_spec.rb +++ b/spec/features/unsubscribe_links_spec.rb @@ -11,7 +11,7 @@ describe 'Unsubscribe links', feature: true do let(:mail) { ActionMailer::Base.deliveries.last } let(:body) { Capybara::Node::Simple.new(mail.default_part_body.to_s) } - let(:header_link) { mail.header['List-Unsubscribe'] } + let(:header_link) { mail.header['List-Unsubscribe'].to_s[1..-2] } # Strip angle brackets let(:body_link) { body.find_link('unsubscribe')['href'] } before do diff --git a/spec/features/users/snippets_spec.rb b/spec/features/users/snippets_spec.rb index f00abd82fea..ce7e809ec76 100644 --- a/spec/features/users/snippets_spec.rb +++ b/spec/features/users/snippets_spec.rb @@ -3,30 +3,16 @@ require 'spec_helper' describe 'Snippets tab on a user profile', feature: true, js: true do include WaitForAjax - let(:user) { create(:user) } - context 'when the user has snippets' do + let(:user) { create(:user) } + let!(:snippets) { create_list(:snippet, 2, :public, author: user) } before do - create_list(:snippet, 25, :public, author: user) - + allow(Snippet).to receive(:default_per_page).and_return(1) visit user_path(user) page.within('.user-profile-nav') { click_link 'Snippets' } wait_for_ajax end - it 'is limited to 20 items per page' do - expect(page.all('.snippets-list-holder .snippet-row').count).to eq(20) - end - - context 'clicking on the link to the second page' do - before do - click_link('2') - wait_for_ajax - end - - it 'shows the remaining snippets' do - expect(page.all('.snippets-list-holder .snippet-row').count).to eq(5) - end - end + it_behaves_like 'paginated snippets', remote: true end end diff --git a/spec/finders/access_requests_finder_spec.rb b/spec/finders/access_requests_finder_spec.rb new file mode 100644 index 00000000000..8cfea9659cb --- /dev/null +++ b/spec/finders/access_requests_finder_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +describe AccessRequestsFinder, services: true do + let(:user) { create(:user) } + let(:access_requester) { create(:user) } + let(:project) { create(:project, :public) } + let(:group) { create(:group, :public) } + + before do + project.request_access(access_requester) + group.request_access(access_requester) + end + + shared_examples 'a finder returning access requesters' do |method_name| + it 'returns access requesters' do + access_requesters = described_class.new(source).public_send(method_name, user) + + expect(access_requesters.size).to eq(1) + expect(access_requesters.first).to be_a "#{source.class}Member".constantize + expect(access_requesters.first.user).to eq(access_requester) + end + end + + shared_examples 'a finder returning no results' do |method_name| + it 'raises Gitlab::Access::AccessDeniedError' do + expect(described_class.new(source).public_send(method_name, user)).to be_empty + end + end + + shared_examples 'a finder raising Gitlab::Access::AccessDeniedError' do |method_name| + it 'raises Gitlab::Access::AccessDeniedError' do + expect { described_class.new(source).public_send(method_name, user) }.to raise_error(Gitlab::Access::AccessDeniedError) + end + end + + describe '#execute' do + context 'when current user cannot see project access requests' do + it_behaves_like 'a finder returning no results', :execute do + let(:source) { project } + end + + it_behaves_like 'a finder returning no results', :execute do + let(:source) { group } + end + end + + context 'when current user can see access requests' do + before do + project.team << [user, :master] + group.add_owner(user) + end + + it_behaves_like 'a finder returning access requesters', :execute do + let(:source) { project } + end + + it_behaves_like 'a finder returning access requesters', :execute do + let(:source) { group } + end + end + end + + describe '#execute!' do + context 'when current user cannot see access requests' do + it_behaves_like 'a finder raising Gitlab::Access::AccessDeniedError', :execute! do + let(:source) { project } + end + + it_behaves_like 'a finder raising Gitlab::Access::AccessDeniedError', :execute! do + let(:source) { group } + end + end + + context 'when current user can see access requests' do + before do + project.team << [user, :master] + group.add_owner(user) + end + + it_behaves_like 'a finder returning access requesters', :execute! do + let(:source) { project } + end + + it_behaves_like 'a finder returning access requesters', :execute! do + let(:source) { group } + end + end + end +end diff --git a/spec/finders/joined_groups_finder_spec.rb b/spec/finders/joined_groups_finder_spec.rb index f90a8e007c8..29a47e005a6 100644 --- a/spec/finders/joined_groups_finder_spec.rb +++ b/spec/finders/joined_groups_finder_spec.rb @@ -43,7 +43,7 @@ describe JoinedGroupsFinder do context 'if profile visitor is in one of the private group projects' do before do project = create(:project, :private, group: private_group, name: 'B', path: 'B') - project.team.add_user(profile_visitor, Gitlab::Access::DEVELOPER) + project.add_user(profile_visitor, Gitlab::Access::DEVELOPER) end it 'shows group' do diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 7a3a74335e8..13bda5f7c5a 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -38,7 +38,7 @@ describe ProjectsFinder do describe 'with private projects' do before do - private_project.team.add_user(user, Gitlab::Access::MASTER) + private_project.add_user(user, Gitlab::Access::MASTER) end it do diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 2dd2eab0524..62cc10f579a 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' -describe IssuablesHelper do +describe IssuablesHelper do let(:label) { build_stubbed(:label) } let(:label2) { build_stubbed(:label) } - context 'label tooltip' do + describe '#issuable_labels_tooltip' do it 'returns label text' do expect(issuable_labels_tooltip([label])).to eq(label.title) end @@ -13,4 +13,105 @@ describe IssuablesHelper do expect(issuable_labels_tooltip([label, label2], limit: 1)).to eq("#{label.title}, and 1 more") end end + + describe '#issuables_state_counter_text' do + let(:user) { create(:user) } + + describe 'state text' do + before do + allow(helper).to receive(:issuables_count_for_state).and_return(42) + end + + it 'returns "Open" when state is :opened' do + expect(helper.issuables_state_counter_text(:issues, :opened)). + to eq('<span>Open</span> <span class="badge">42</span>') + end + + it 'returns "Closed" when state is :closed' do + expect(helper.issuables_state_counter_text(:issues, :closed)). + to eq('<span>Closed</span> <span class="badge">42</span>') + end + + it 'returns "Merged" when state is :merged' do + expect(helper.issuables_state_counter_text(:merge_requests, :merged)). + to eq('<span>Merged</span> <span class="badge">42</span>') + end + + it 'returns "All" when state is :all' do + expect(helper.issuables_state_counter_text(:merge_requests, :all)). + to eq('<span>All</span> <span class="badge">42</span>') + end + end + + describe 'counter caching based on issuable type and params', :caching do + let(:params) do + { + scope: 'created-by-me', + state: 'opened', + utf8: '✓', + author_id: '11', + assignee_id: '18', + label_name: ['bug', 'discussion', 'documentation'], + milestone_title: 'v4.0', + sort: 'due_date_asc', + namespace_id: 'gitlab-org', + project_id: 'gitlab-ce', + page: 2 + }.with_indifferent_access + end + + it 'returns the cached value when called for the same issuable type & with the same params' do + expect(helper).to receive(:params).twice.and_return(params) + expect(helper).to receive(:issuables_count_for_state).with(:issues, :opened).and_return(42) + + expect(helper.issuables_state_counter_text(:issues, :opened)). + to eq('<span>Open</span> <span class="badge">42</span>') + + expect(helper).not_to receive(:issuables_count_for_state) + + expect(helper.issuables_state_counter_text(:issues, :opened)). + to eq('<span>Open</span> <span class="badge">42</span>') + end + + it 'does not take some keys into account in the cache key' do + expect(helper).to receive(:params).and_return({ + author_id: '11', + state: 'foo', + sort: 'foo', + utf8: 'foo', + page: 'foo' + }.with_indifferent_access) + expect(helper).to receive(:issuables_count_for_state).with(:issues, :opened).and_return(42) + + expect(helper.issuables_state_counter_text(:issues, :opened)). + to eq('<span>Open</span> <span class="badge">42</span>') + + expect(helper).to receive(:params).and_return({ + author_id: '11', + state: 'bar', + sort: 'bar', + utf8: 'bar', + page: 'bar' + }.with_indifferent_access) + expect(helper).not_to receive(:issuables_count_for_state) + + expect(helper.issuables_state_counter_text(:issues, :opened)). + to eq('<span>Open</span> <span class="badge">42</span>') + end + + it 'does not take params order into account in the cache key' do + expect(helper).to receive(:params).and_return('author_id' => '11', 'state' => 'opened') + expect(helper).to receive(:issuables_count_for_state).with(:issues, :opened).and_return(42) + + expect(helper.issuables_state_counter_text(:issues, :opened)). + to eq('<span>Open</span> <span class="badge">42</span>') + + expect(helper).to receive(:params).and_return('state' => 'opened', 'author_id' => '11') + expect(helper).not_to receive(:issuables_count_for_state) + + expect(helper.issuables_state_counter_text(:issues, :opened)). + to eq('<span>Open</span> <span class="badge">42</span>') + end + end + end end diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb index 7998209b7b0..6703d88e357 100644 --- a/spec/helpers/members_helper_spec.rb +++ b/spec/helpers/members_helper_spec.rb @@ -11,7 +11,7 @@ describe MembersHelper do describe '#remove_member_message' do let(:requester) { build(:user) } - let(:project) { create(:project) } + let(:project) { create(:empty_project, :public) } let(:project_member) { build(:project_member, project: project) } let(:project_member_invite) { build(:project_member, project: project).tap { |m| m.generate_invite_token! } } let(:project_member_request) { project.request_access(requester) } @@ -32,7 +32,7 @@ describe MembersHelper do describe '#remove_member_title' do let(:requester) { build(:user) } - let(:project) { create(:project) } + let(:project) { create(:empty_project, :public) } let(:project_member) { build(:project_member, project: project) } let(:project_member_request) { project.request_access(requester) } let(:group) { create(:group) } diff --git a/spec/helpers/milestones_helper_spec.rb b/spec/helpers/milestones_helper_spec.rb new file mode 100644 index 00000000000..28c2268f8d0 --- /dev/null +++ b/spec/helpers/milestones_helper_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe MilestonesHelper do + describe '#milestone_counts' do + let(:project) { FactoryGirl.create(:project) } + let(:counts) { helper.milestone_counts(project.milestones) } + + context 'when there are milestones' do + let!(:milestone_1) { FactoryGirl.create(:active_milestone, project: project) } + let!(:milestone_2) { FactoryGirl.create(:active_milestone, project: project) } + let!(:milestone_3) { FactoryGirl.create(:closed_milestone, project: project) } + + it 'returns the correct counts' do + expect(counts).to eq(opened: 2, closed: 1, all: 3) + end + end + + context 'when there are only milestones of one type' do + let!(:milestone_1) { FactoryGirl.create(:active_milestone, project: project) } + let!(:milestone_2) { FactoryGirl.create(:active_milestone, project: project) } + + it 'returns the correct counts' do + expect(counts).to eq(opened: 2, closed: 0, all: 2) + end + end + + context 'when there are no milestones' do + it 'returns the correct counts' do + expect(counts).to eq(opened: 0, closed: 0, all: 0) + end + end + end +end diff --git a/spec/javascripts/labels_issue_sidebar_spec.js.es6 b/spec/javascripts/labels_issue_sidebar_spec.js.es6 index 840c7b6d015..1ad6f612210 100644 --- a/spec/javascripts/labels_issue_sidebar_spec.js.es6 +++ b/spec/javascripts/labels_issue_sidebar_spec.js.es6 @@ -48,9 +48,9 @@ setTimeout(() => { expect($('.dropdown-content a').length).toBe(10); - $('.dropdow-content a').each((i, $link) => { - if (i < 5) { - $link.get(0).click(); + $('.dropdown-content a').each(function (i) { + if (i < saveLabelCount) { + $(this).get(0).click(); } }); @@ -70,9 +70,9 @@ setTimeout(() => { expect($('.dropdown-content a').length).toBe(10); - $('.dropdow-content a').each((i, $link) => { - if (i < 5) { - $link.get(0).click(); + $('.dropdown-content a').each(function (i) { + if (i < saveLabelCount) { + $(this).get(0).click(); } }); @@ -86,4 +86,3 @@ }); }); })(); - diff --git a/spec/lib/banzai/filter/task_list_filter_spec.rb b/spec/lib/banzai/filter/task_list_filter_spec.rb deleted file mode 100644 index 569cbc885c7..00000000000 --- a/spec/lib/banzai/filter/task_list_filter_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'spec_helper' - -describe Banzai::Filter::TaskListFilter, lib: true do - include FilterSpecHelper - - it 'does not apply `task-list` class to non-task lists' do - exp = act = %(<ul><li>Item</li></ul>) - expect(filter(act).to_html).to eq exp - end - - it 'applies `task-list` to single-item task lists' do - act = filter('<ul><li>[ ] Task 1</li></ul>') - - expect(act.to_html).to start_with '<ul class="task-list">' - end -end diff --git a/spec/lib/ci/mask_secret_spec.rb b/spec/lib/ci/mask_secret_spec.rb index 518de76911c..3101bed20fb 100644 --- a/spec/lib/ci/mask_secret_spec.rb +++ b/spec/lib/ci/mask_secret_spec.rb @@ -5,15 +5,23 @@ describe Ci::MaskSecret, lib: true do describe '#mask' do it 'masks exact number of characters' do - expect(subject.mask('token', 'oke')).to eq('txxxn') + expect(mask('token', 'oke')).to eq('txxxn') end it 'masks multiple occurrences' do - expect(subject.mask('token token token', 'oke')).to eq('txxxn txxxn txxxn') + expect(mask('token token token', 'oke')).to eq('txxxn txxxn txxxn') end it 'does not mask if not found' do - expect(subject.mask('token', 'not')).to eq('token') + expect(mask('token', 'not')).to eq('token') + end + + it 'does support null token' do + expect(mask('token', nil)).to eq('token') + end + + def mask(value, token) + subject.mask!(value.dup, token) end end end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 745fbc0df45..c9d64e99f88 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -64,7 +64,7 @@ describe Gitlab::Auth, lib: true do it 'recognizes user lfs tokens' do user = create(:user) ip = 'ip' - token = Gitlab::LfsToken.new(user).generate + token = Gitlab::LfsToken.new(user).token expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username) expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, full_authentication_abilities)) @@ -73,7 +73,7 @@ describe Gitlab::Auth, lib: true do it 'recognizes deploy key lfs tokens' do key = create(:deploy_key) ip = 'ip' - token = Gitlab::LfsToken.new(key).generate + token = Gitlab::LfsToken.new(key).token expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs+deploy-key-#{key.id}") expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities)) diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index ed43646330f..de68e32e5b4 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -343,7 +343,7 @@ describe Gitlab::GitAccess, lib: true do end context 'to private project' do - let(:project) { create(:project, :internal) } + let(:project) { create(:project) } it { expect(subject).not_to be_allowed } end diff --git a/spec/lib/gitlab/git_spec.rb b/spec/lib/gitlab/git_spec.rb new file mode 100644 index 00000000000..219198eff60 --- /dev/null +++ b/spec/lib/gitlab/git_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' + +describe Gitlab::Git, lib: true do + let(:committer_email) { FFaker::Internet.email } + + # I have to remove periods from the end of the name + # This happened when the user's name had a suffix (i.e. "Sr.") + # This seems to be what git does under the hood. For example, this commit: + # + # $ git commit --author='Foo Sr. <foo@example.com>' -m 'Where's my trailing period?' + # + # results in this: + # + # $ git show --pretty + # ... + # Author: Foo Sr <foo@example.com> + # ... + let(:committer_name) { FFaker::Name.name.chomp("\.") } + + describe 'committer_hash' do + it "returns a hash containing the given email and name" do + committer_hash = Gitlab::Git::committer_hash(email: committer_email, name: committer_name) + + expect(committer_hash[:email]).to eq(committer_email) + expect(committer_hash[:name]).to eq(committer_name) + expect(committer_hash[:time]).to be_a(Time) + end + + context 'when email is nil' do + it "returns nil" do + committer_hash = Gitlab::Git::committer_hash(email: nil, name: committer_name) + + expect(committer_hash).to be_nil + end + end + + context 'when name is nil' do + it "returns nil" do + committer_hash = Gitlab::Git::committer_hash(email: committer_email, name: nil) + + expect(committer_hash).to be_nil + end + end + end +end diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb index 613c47d55f1..e829b936343 100644 --- a/spec/lib/gitlab/github_import/client_spec.rb +++ b/spec/lib/gitlab/github_import/client_spec.rb @@ -66,6 +66,6 @@ describe Gitlab::GithubImport::Client, lib: true do stub_request(:get, /api.github.com/) allow(client.api).to receive(:rate_limit!).and_raise(Octokit::NotFound) - expect { client.issues }.not_to raise_error + expect { client.issues {} }.not_to raise_error end end diff --git a/spec/lib/gitlab/github_import/importer_spec.rb b/spec/lib/gitlab/github_import/importer_spec.rb index 553c849c9b4..8854c8431b5 100644 --- a/spec/lib/gitlab/github_import/importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer_spec.rb @@ -57,7 +57,8 @@ describe Gitlab::GithubImport::Importer, lib: true do created_at: created_at, updated_at: updated_at, closed_at: nil, - url: 'https://api.github.com/repos/octocat/Hello-World/issues/1347' + url: 'https://api.github.com/repos/octocat/Hello-World/issues/1347', + labels: [double(name: 'Label #1')], ) end @@ -75,7 +76,8 @@ describe Gitlab::GithubImport::Importer, lib: true do created_at: created_at, updated_at: updated_at, closed_at: nil, - url: 'https://api.github.com/repos/octocat/Hello-World/issues/1348' + url: 'https://api.github.com/repos/octocat/Hello-World/issues/1348', + labels: [double(name: 'Label #2')], ) end @@ -94,7 +96,8 @@ describe Gitlab::GithubImport::Importer, lib: true do updated_at: updated_at, closed_at: nil, merged_at: nil, - url: 'https://api.github.com/repos/octocat/Hello-World/pulls/1347' + url: 'https://api.github.com/repos/octocat/Hello-World/pulls/1347', + labels: [double(name: 'Label #3')], ) end @@ -129,6 +132,8 @@ describe Gitlab::GithubImport::Importer, lib: true do allow_any_instance_of(Octokit::Client).to receive(:milestones).and_return([milestone, milestone]) allow_any_instance_of(Octokit::Client).to receive(:issues).and_return([issue1, issue2]) allow_any_instance_of(Octokit::Client).to receive(:pull_requests).and_return([pull_request, pull_request]) + allow_any_instance_of(Octokit::Client).to receive(:issues_comments).and_return([]) + allow_any_instance_of(Octokit::Client).to receive(:pull_requests_comments).and_return([]) allow_any_instance_of(Octokit::Client).to receive(:last_response).and_return(double(rels: { next: nil })) allow_any_instance_of(Octokit::Client).to receive(:releases).and_return([release1, release2]) allow_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_raise(Gitlab::Shell::Error) @@ -148,9 +153,7 @@ describe Gitlab::GithubImport::Importer, lib: true do 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: :milestone, url: "https://api.github.com/repos/octocat/Hello-World/milestones/1", errors: "Validation failed: Title has already been taken" }, - { type: :issue, url: "https://api.github.com/repos/octocat/Hello-World/issues/1347", errors: "Invalid Repository. Use user/repo format." }, { 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: :pull_request, url: "https://api.github.com/repos/octocat/Hello-World/pulls/1347", errors: "Invalid Repository. Use user/repo format." }, { type: :pull_request, url: "https://api.github.com/repos/octocat/Hello-World/pulls/1347", errors: "Validation failed: Validate branches Cannot Create: This merge request already exists: [\"New feature\"]" }, { 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 new file mode 100644 index 00000000000..006569254a6 --- /dev/null +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -0,0 +1,187 @@ +--- +issues: +- subscriptions +- award_emoji +- author +- assignee +- updated_by +- milestone +- notes +- label_links +- labels +- todos +- user_agent_detail +- moved_to +- events +- merge_requests_closing_issues +- metrics +events: +- author +- project +- target +notes: +- award_emoji +- project +- noteable +- author +- updated_by +- resolved_by +- todos +- events +label_links: +- target +- label +label: +- subscriptions +- project +- lists +- label_links +- issues +- merge_requests +milestone: +- project +- issues +- labels +- merge_requests +- participants +- events +snippets: +- author +- project +- notes +- award_emoji +releases: +- project +project_members: +- created_by +- user +- source +- project +merge_requests: +- subscriptions +- award_emoji +- author +- assignee +- updated_by +- milestone +- notes +- label_links +- labels +- todos +- target_project +- source_project +- merge_user +- merge_request_diffs +- merge_request_diff +- events +- merge_requests_closing_issues +- metrics +merge_request_diff: +- merge_request +pipelines: +- project +- user +- statuses +- builds +- trigger_requests +statuses: +- project +- pipeline +- user +variables: +- project +triggers: +- project +- trigger_requests +deploy_keys: +- user +- deploy_keys_projects +- projects +services: +- project +- service_hook +hooks: +- project +protected_branches: +- project +- merge_access_levels +- push_access_levels +merge_access_levels: +- protected_branch +push_access_levels: +- protected_branch +project: +- taggings +- base_tags +- tag_taggings +- tags +- creator +- group +- namespace +- board +- last_event +- services +- campfire_service +- drone_ci_service +- emails_on_push_service +- builds_email_service +- irker_service +- pivotaltracker_service +- hipchat_service +- flowdock_service +- assembla_service +- asana_service +- gemnasium_service +- slack_service +- buildkite_service +- bamboo_service +- teamcity_service +- pushover_service +- jira_service +- redmine_service +- custom_issue_tracker_service +- bugzilla_service +- gitlab_issue_tracker_service +- external_wiki_service +- forked_project_link +- forked_from_project +- forked_project_links +- forks +- merge_requests +- fork_merge_requests +- issues +- labels +- events +- milestones +- notes +- snippets +- hooks +- protected_branches +- project_members +- users +- requesters +- deploy_keys_projects +- deploy_keys +- users_star_projects +- starrers +- releases +- lfs_objects_projects +- lfs_objects +- project_group_links +- invited_groups +- todos +- notification_settings +- import_data +- commit_statuses +- pipelines +- builds +- runner_projects +- runners +- variables +- triggers +- environments +- deployments +- project_feature +award_emoji: +- awardable +- user
\ No newline at end of file diff --git a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb new file mode 100644 index 00000000000..2e19d590d83 --- /dev/null +++ b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +# Part of the test security suite for the Import/Export feature +# Checks whether there are new attributes in models that are currently being exported as part of the +# project Import/Export feature. +# If there are new attributes, these will have to either be added to this spec in case we want them +# to be included as part of the export, or blacklist them using the import_export.yml configuration file. +# Likewise, new models added to import_export.yml, will need to be added with their correspondent attributes +# to this spec. +describe 'Import/Export attribute configuration', lib: true do + include ConfigurationHelper + + let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys } + let(:relation_names) do + names = names_from_tree(config_hash['project_tree']) + + # Remove duplicated or add missing models + # - project is not part of the tree, so it has to be added manually. + # - milestone, labels have both singular and plural versions in the tree, so remove the duplicates. + names.flatten.uniq - ['milestones', 'labels'] + ['project'] + end + + let(:safe_attributes_file) { 'spec/lib/gitlab/import_export/safe_model_attributes.yml' } + let(:safe_model_attributes) { YAML.load_file(safe_attributes_file) } + + it 'has no new columns' do + relation_names.each do |relation_name| + relation_class = relation_class_for_name(relation_name) + + expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class} to exist in safe_model_attributes" + + current_attributes = parsed_attributes(relation_name, relation_class.attribute_names) + safe_attributes = safe_model_attributes[relation_class.to_s] + new_attributes = current_attributes - safe_attributes + + expect(new_attributes).to be_empty, failure_message(relation_class.to_s, new_attributes) + end + end + + def failure_message(relation_class, new_attributes) + <<-MSG + It looks like #{relation_class}, which is exported using the project Import/Export, has new attributes: #{new_attributes.join(',')} + + Please add the attribute(s) to SAFE_MODEL_ATTRIBUTES if you consider this can be exported. + Otherwise, please blacklist the attribute(s) in IMPORT_EXPORT_CONFIG by adding it to its correspondent + model in the +excluded_attributes+ section. + + SAFE_MODEL_ATTRIBUTES: #{File.expand_path(safe_attributes_file)} + IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file} + MSG + end + + class Author < User + end +end diff --git a/spec/lib/gitlab/import_export/model_configuration_spec.rb b/spec/lib/gitlab/import_export/model_configuration_spec.rb new file mode 100644 index 00000000000..9b492d1b9c7 --- /dev/null +++ b/spec/lib/gitlab/import_export/model_configuration_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +# Part of the test security suite for the Import/Export feature +# Finds if a new model has been added that can potentially be part of the Import/Export +# If it finds a new model, it will show a +failure_message+ with the options available. +describe 'Import/Export model configuration', lib: true do + include ConfigurationHelper + + let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys } + let(:model_names) do + names = names_from_tree(config_hash['project_tree']) + + # Remove duplicated or add missing models + # - project is not part of the tree, so it has to be added manually. + # - milestone, labels have both singular and plural versions in the tree, so remove the duplicates. + # - User, Author... Models we do not care about for checking models + names.flatten.uniq - ['milestones', 'labels', 'user', 'author'] + ['project'] + end + + let(:all_models_yml) { 'spec/lib/gitlab/import_export/all_models.yml' } + let(:all_models) { YAML.load_file(all_models_yml) } + let(:current_models) { setup_models } + + it 'has no new models' do + model_names.each do |model_name| + new_models = Array(current_models[model_name]) - Array(all_models[model_name]) + expect(new_models).to be_empty, failure_message(model_name.classify, new_models) + end + end + + # List of current models between models, in the format of + # {model: [model_2, model3], ...} + def setup_models + all_models_hash = {} + + model_names.each do |model_name| + model_class = relation_class_for_name(model_name) + + all_models_hash[model_name] = associations_for(model_class) - ['project'] + end + + all_models_hash + end + + def failure_message(parent_model_name, new_models) + <<-MSG + New model(s) <#{new_models.join(',')}> have been added, related to #{parent_model_name}, which is exported by + the Import/Export feature. + + If you think this model should be included in the export, please add it to IMPORT_EXPORT_CONFIG. + Definitely add it to MODELS_JSON to signal that you've handled this error and to prevent it from showing up in the future. + + MODELS_JSON: #{File.expand_path(all_models_yml)} + IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file} + MSG + end +end diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 281f6cf1177..98323fe6be4 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -2232,6 +2232,31 @@ ], "milestones": [ { + "id": 1, + "title": "test milestone", + "project_id": 8, + "description": "test milestone", + "due_date": null, + "created_at": "2016-06-14T15:02:04.415Z", + "updated_at": "2016-06-14T15:02:04.415Z", + "state": "active", + "iid": 1, + "events": [ + { + "id": 487, + "target_type": "Milestone", + "target_id": 1, + "title": null, + "data": null, + "project_id": 46, + "created_at": "2016-06-14T15:02:04.418Z", + "updated_at": "2016-06-14T15:02:04.418Z", + "action": 1, + "author_id": 18 + } + ] + }, + { "id": 20, "title": "v4.0", "project_id": 5, @@ -6918,6 +6943,7 @@ "note_events": true, "build_events": true, "category": "issue_tracker", + "type": "CustomIssueTrackerService", "default": true, "wiki_page_events": true }, @@ -7372,5 +7398,16 @@ } ] } - ] + ], + "project_feature": { + "builds_access_level": 0, + "created_at": "2014-12-26T09:26:45.000Z", + "id": 2, + "issues_access_level": 0, + "merge_requests_access_level": 20, + "project_id": 4, + "snippets_access_level": 20, + "updated_at": "2016-09-23T11:58:28.000Z", + "wiki_access_level": 20 + } }
\ No newline at end of file diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index feacb295231..7582a732cdf 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -107,6 +107,18 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do expect(Label.first.label_links.first.target).not_to be_nil end + it 'has a project feature' do + restored_project_json + + expect(project.project_feature).not_to be_nil + end + + it 'restores the correct service' do + restored_project_json + + expect(CustomIssueTrackerService.first).not_to be_nil + end + context 'Merge requests' do before do restored_project_json diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index d891c2d0cc6..cf8f2200c57 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -111,6 +111,10 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do expect(saved_project_json['issues'].first['label_links'].first['label']).not_to be_empty end + it 'saves the correct service type' do + expect(saved_project_json['services'].first['type']).to eq('CustomIssueTrackerService') + end + it 'has project feature' do project_feature = saved_project_json['project_feature'] expect(project_feature).not_to be_empty @@ -161,6 +165,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do commit_id: ci_pipeline.sha) create(:event, target: milestone, project: project, action: Event::CREATED, author: user) + create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker') project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::ENABLED) diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml new file mode 100644 index 00000000000..8bccd313d6c --- /dev/null +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -0,0 +1,330 @@ +--- +Issue: +- id +- title +- assignee_id +- author_id +- project_id +- created_at +- updated_at +- position +- branch_name +- description +- state +- iid +- updated_by_id +- confidential +- deleted_at +- due_date +- moved_to_id +- lock_version +- milestone_id +- weight +Event: +- id +- target_type +- target_id +- title +- data +- project_id +- created_at +- updated_at +- action +- author_id +Note: +- id +- note +- noteable_type +- author_id +- created_at +- updated_at +- project_id +- attachment +- line_code +- commit_id +- noteable_id +- system +- st_diff +- updated_by_id +- type +- position +- original_position +- resolved_at +- resolved_by_id +- discussion_id +- original_discussion_id +LabelLink: +- id +- label_id +- target_id +- target_type +- created_at +- updated_at +Label: +- id +- title +- color +- project_id +- created_at +- updated_at +- template +- description +- priority +Milestone: +- id +- title +- project_id +- description +- due_date +- created_at +- updated_at +- state +- iid +ProjectSnippet: +- id +- title +- content +- author_id +- project_id +- created_at +- updated_at +- file_name +- type +- visibility_level +Release: +- id +- tag +- description +- project_id +- created_at +- updated_at +ProjectMember: +- id +- access_level +- source_id +- source_type +- user_id +- notification_level +- type +- created_at +- updated_at +- created_by_id +- invite_email +- invite_token +- invite_accepted_at +- requested_at +- expires_at +User: +- id +- username +- email +MergeRequest: +- id +- target_branch +- source_branch +- source_project_id +- author_id +- assignee_id +- title +- created_at +- updated_at +- state +- merge_status +- target_project_id +- iid +- description +- position +- locked_at +- updated_by_id +- merge_error +- merge_params +- merge_when_build_succeeds +- merge_user_id +- merge_commit_sha +- deleted_at +- in_progress_merge_commit_sha +- lock_version +- milestone_id +- approvals_before_merge +- rebase_commit_sha +MergeRequestDiff: +- id +- state +- st_commits +- merge_request_id +- created_at +- updated_at +- base_commit_sha +- real_size +- head_commit_sha +- start_commit_sha +Ci::Pipeline: +- id +- project_id +- ref +- sha +- before_sha +- push_data +- created_at +- updated_at +- tag +- yaml_errors +- committed_at +- gl_project_id +- status +- started_at +- finished_at +- duration +- user_id +CommitStatus: +- id +- project_id +- status +- finished_at +- trace +- created_at +- updated_at +- started_at +- runner_id +- coverage +- commit_id +- commands +- job_id +- name +- deploy +- options +- allow_failure +- stage +- trigger_request_id +- stage_idx +- tag +- ref +- user_id +- type +- target_url +- description +- artifacts_file +- gl_project_id +- artifacts_metadata +- erased_by_id +- erased_at +- artifacts_expire_at +- environment +- artifacts_size +- when +- yaml_variables +- queued_at +- token +Ci::Variable: +- id +- project_id +- key +- value +- encrypted_value +- encrypted_value_salt +- encrypted_value_iv +- gl_project_id +Ci::Trigger: +- id +- token +- project_id +- deleted_at +- created_at +- updated_at +- gl_project_id +DeployKey: +- id +- user_id +- created_at +- updated_at +- key +- title +- type +- fingerprint +- public +Service: +- id +- type +- title +- project_id +- created_at +- updated_at +- active +- properties +- template +- push_events +- issues_events +- merge_requests_events +- tag_push_events +- note_events +- pipeline_events +- build_events +- category +- default +- wiki_page_events +- confidential_issues_events +ProjectHook: +- id +- url +- project_id +- created_at +- updated_at +- type +- service_id +- push_events +- issues_events +- merge_requests_events +- tag_push_events +- note_events +- pipeline_events +- enable_ssl_verification +- build_events +- wiki_page_events +- token +- group_id +- confidential_issues_events +ProtectedBranch: +- id +- project_id +- name +- created_at +- updated_at +Project: +- description +- issues_enabled +- merge_requests_enabled +- wiki_enabled +- snippets_enabled +- visibility_level +- archived +Author: +- name +ProjectFeature: +- id +- project_id +- merge_requests_access_level +- issues_access_level +- wiki_access_level +- snippets_access_level +- builds_access_level +- created_at +- updated_at +ProtectedBranch::MergeAccessLevel: +- id +- protected_branch_id +- access_level +- created_at +- updated_at +ProtectedBranch::PushAccessLevel: +- id +- protected_branch_id +- access_level +- created_at +- updated_at +AwardEmoji: +- id +- user_id +- name +- awardable_type +- created_at +- updated_at diff --git a/spec/lib/gitlab/ldap/adapter_spec.rb b/spec/lib/gitlab/ldap/adapter_spec.rb index 0600893f4cf..563c074017a 100644 --- a/spec/lib/gitlab/ldap/adapter_spec.rb +++ b/spec/lib/gitlab/ldap/adapter_spec.rb @@ -73,17 +73,33 @@ describe Gitlab::LDAP::Adapter, lib: true do describe '#dn_matches_filter?' do subject { adapter.dn_matches_filter?(:dn, :filter) } + context "when the search result is non-empty" do + before { allow(adapter).to receive(:ldap_search).and_return([:foo]) } + + it { is_expected.to be_truthy } + end + + context "when the search result is empty" do + before { allow(adapter).to receive(:ldap_search).and_return([]) } + + it { is_expected.to be_falsey } + end + end + + describe '#ldap_search' do + subject { adapter.ldap_search(base: :dn, filter: :filter) } + context "when the search is successful" do context "and the result is non-empty" do before { allow(ldap).to receive(:search).and_return([:foo]) } - it { is_expected.to be_truthy } + it { is_expected.to eq [:foo] } end context "and the result is empty" do before { allow(ldap).to receive(:search).and_return([]) } - it { is_expected.to be_falsey } + it { is_expected.to eq [] } end end @@ -95,7 +111,22 @@ describe Gitlab::LDAP::Adapter, lib: true do ) end - it { is_expected.to be_falsey } + it { is_expected.to eq [] } + end + + context "when the search raises an LDAP exception" do + before do + allow(ldap).to receive(:search) { raise Net::LDAP::Error, "some error" } + allow(Rails.logger).to receive(:warn) + end + + it { is_expected.to eq [] } + + it 'logs the error' do + subject + expect(Rails.logger).to have_received(:warn).with( + "LDAP search raised exception Net::LDAP::Error: some error") + end end end end diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb index 9f04f67e0a8..e9c1163e22a 100644 --- a/spec/lib/gitlab/lfs_token_spec.rb +++ b/spec/lib/gitlab/lfs_token_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' describe Gitlab::LfsToken, lib: true do - describe '#generate and #value' do + describe '#token' do shared_examples 'an LFS token generator' do it 'returns a randomly generated token' do - token = handler.generate + token = handler.token expect(token).not_to be_nil expect(token).to be_a String @@ -12,9 +12,9 @@ describe Gitlab::LfsToken, lib: true do end it 'returns the correct token based on the key' do - token = handler.generate + token = handler.token - expect(handler.value).to eq(token) + expect(handler.token).to eq(token) end end diff --git a/spec/lib/gitlab/redis_spec.rb b/spec/lib/gitlab/redis_spec.rb index e54f5ffb312..cb54c020b31 100644 --- a/spec/lib/gitlab/redis_spec.rb +++ b/spec/lib/gitlab/redis_spec.rb @@ -3,19 +3,27 @@ require 'spec_helper' describe Gitlab::Redis do let(:redis_config) { Rails.root.join('config', 'resque.yml').to_s } - before(:each) { described_class.reset_params! } - after(:each) { described_class.reset_params! } + before(:each) { clear_raw_config } + after(:each) { clear_raw_config } describe '.params' do subject { described_class.params } + it 'withstands mutation' do + params1 = described_class.params + params2 = described_class.params + params1[:foo] = :bar + + expect(params2).not_to have_key(:foo) + end + context 'when url contains unix socket reference' do let(:config_old) { Rails.root.join('spec/fixtures/config/redis_old_format_socket.yml').to_s } let(:config_new) { Rails.root.join('spec/fixtures/config/redis_new_format_socket.yml').to_s } context 'with old format' do it 'returns path key instead' do - expect_any_instance_of(described_class).to receive(:config_file) { config_old } + stub_const("#{described_class}::CONFIG_FILE", config_old) is_expected.to include(path: '/path/to/old/redis.sock') is_expected.not_to have_key(:url) @@ -24,7 +32,7 @@ describe Gitlab::Redis do context 'with new format' do it 'returns path key instead' do - expect_any_instance_of(described_class).to receive(:config_file) { config_new } + stub_const("#{described_class}::CONFIG_FILE", config_new) is_expected.to include(path: '/path/to/redis.sock') is_expected.not_to have_key(:url) @@ -38,7 +46,7 @@ describe Gitlab::Redis do context 'with old format' do it 'returns hash with host, port, db, and password' do - expect_any_instance_of(described_class).to receive(:config_file) { config_old } + stub_const("#{described_class}::CONFIG_FILE", config_old) is_expected.to include(host: 'localhost', password: 'mypassword', port: 6379, db: 99) is_expected.not_to have_key(:url) @@ -47,7 +55,7 @@ describe Gitlab::Redis do context 'with new format' do it 'returns hash with host, port, db, and password' do - expect_any_instance_of(described_class).to receive(:config_file) { config_new } + stub_const("#{described_class}::CONFIG_FILE", config_new) is_expected.to include(host: 'localhost', password: 'mynewpassword', port: 6379, db: 99) is_expected.not_to have_key(:url) @@ -56,6 +64,30 @@ describe Gitlab::Redis do end end + describe '.url' do + it 'withstands mutation' do + url1 = described_class.url + url2 = described_class.url + url1 << 'foobar' + + expect(url2).not_to end_with('foobar') + end + end + + describe '._raw_config' do + subject { described_class._raw_config } + + it 'should be frozen' do + expect(subject).to be_frozen + end + + it 'returns false when the file does not exist' do + stub_const("#{described_class}::CONFIG_FILE", '/var/empty/doesnotexist') + + expect(subject).to eq(false) + end + end + describe '#raw_config_hash' do it 'returns default redis url when no config file is present' do expect(subject).to receive(:fetch_config) { false } @@ -71,9 +103,15 @@ describe Gitlab::Redis do describe '#fetch_config' do it 'returns false when no config file is present' do - allow(File).to receive(:exist?).with(redis_config) { false } + allow(described_class).to receive(:_raw_config) { false } expect(subject.send(:fetch_config)).to be_falsey end end + + def clear_raw_config + described_class.remove_instance_variable(:@_raw_config) + rescue NameError + # raised if @_raw_config was not set; ignore + end end diff --git a/spec/lib/gitlab/template/issue_template_spec.rb b/spec/lib/gitlab/template/issue_template_spec.rb index f770857e958..d2d334e6413 100644 --- a/spec/lib/gitlab/template/issue_template_spec.rb +++ b/spec/lib/gitlab/template/issue_template_spec.rb @@ -10,7 +10,7 @@ describe Gitlab::Template::IssueTemplate do let(:file_path_3) { '.gitlab/issue_templates/feature_proposal.md' } before do - project.team.add_user(user, Gitlab::Access::MASTER) + project.add_user(user, Gitlab::Access::MASTER) project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false) project.repository.commit_file(user, file_path_2, "template_test", "test 1", "master", false) project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false) @@ -53,7 +53,7 @@ describe Gitlab::Template::IssueTemplate do context 'when repo is bare or empty' do let(:empty_project) { create(:empty_project) } - before { empty_project.team.add_user(user, Gitlab::Access::MASTER) } + before { empty_project.add_user(user, Gitlab::Access::MASTER) } it "returns empty array" do templates = subject.by_category('', empty_project) @@ -78,7 +78,7 @@ describe Gitlab::Template::IssueTemplate do context "when repo is empty" do let(:empty_project) { create(:empty_project) } - before { empty_project.team.add_user(user, Gitlab::Access::MASTER) } + before { empty_project.add_user(user, Gitlab::Access::MASTER) } it "raises file not found" do issue_template = subject.new('.gitlab/issue_templates/not_existent.md', empty_project) diff --git a/spec/lib/gitlab/template/merge_request_template_spec.rb b/spec/lib/gitlab/template/merge_request_template_spec.rb index bb0f68043fa..ddf68c4cf78 100644 --- a/spec/lib/gitlab/template/merge_request_template_spec.rb +++ b/spec/lib/gitlab/template/merge_request_template_spec.rb @@ -10,7 +10,7 @@ describe Gitlab::Template::MergeRequestTemplate do let(:file_path_3) { '.gitlab/merge_request_templates/feature_proposal.md' } before do - project.team.add_user(user, Gitlab::Access::MASTER) + project.add_user(user, Gitlab::Access::MASTER) project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false) project.repository.commit_file(user, file_path_2, "template_test", "test 1", "master", false) project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false) @@ -53,7 +53,7 @@ describe Gitlab::Template::MergeRequestTemplate do context 'when repo is bare or empty' do let(:empty_project) { create(:empty_project) } - before { empty_project.team.add_user(user, Gitlab::Access::MASTER) } + before { empty_project.add_user(user, Gitlab::Access::MASTER) } it "returns empty array" do templates = subject.by_category('', empty_project) @@ -78,7 +78,7 @@ describe Gitlab::Template::MergeRequestTemplate do context "when repo is empty" do let(:empty_project) { create(:empty_project) } - before { empty_project.team.add_user(user, Gitlab::Access::MASTER) } + before { empty_project.add_user(user, Gitlab::Access::MASTER) } it "raises file not found" do issue_template = subject.new('.gitlab/merge_request_templates/not_existent.md', empty_project) diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 6c7fa7e7c15..b5b685da904 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -1,8 +1,16 @@ require 'spec_helper' describe Gitlab::Workhorse, lib: true do - let(:project) { create(:project) } - let(:subject) { Gitlab::Workhorse } + let(:project) { create(:project) } + let(:repository) { project.repository } + + def decode_workhorse_header(array) + key, value = array + command, encoded_params = value.split(":") + params = JSON.parse(Base64.urlsafe_decode64(encoded_params)) + + [key, command, params] + end describe ".send_git_archive" do context "when the repository doesn't have an archive file path" do @@ -11,11 +19,37 @@ describe Gitlab::Workhorse, lib: true do end it "raises an error" do - expect { subject.send_git_archive(project.repository, ref: "master", format: "zip") }.to raise_error(RuntimeError) + expect { described_class.send_git_archive(project.repository, ref: "master", format: "zip") }.to raise_error(RuntimeError) end end end + describe '.send_git_patch' do + let(:diff_refs) { double(base_sha: "base", head_sha: "head") } + subject { described_class.send_git_patch(repository, diff_refs) } + + it 'sets the header correctly' do + key, command, params = decode_workhorse_header(subject) + + expect(key).to eq("Gitlab-Workhorse-Send-Data") + expect(command).to eq("git-format-patch") + expect(params).to eq("RepoPath" => repository.path_to_repo, "ShaFrom" => "base", "ShaTo" => "head") + end + end + + describe '.send_git_diff' do + let(:diff_refs) { double(base_sha: "base", head_sha: "head") } + subject { described_class.send_git_patch(repository, diff_refs) } + + it 'sets the header correctly' do + key, command, params = decode_workhorse_header(subject) + + expect(key).to eq("Gitlab-Workhorse-Send-Data") + expect(command).to eq("git-format-patch") + expect(params).to eq("RepoPath" => repository.path_to_repo, "ShaFrom" => "base", "ShaTo" => "head") + end + end + describe ".secret" do subject { described_class.secret } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 0363bc74939..0e4130e8a3a 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -402,7 +402,7 @@ describe Notify do describe 'project access requested' do context 'for a project in a user namespace' do - let(:project) { create(:project).tap { |p| p.team << [p.owner, :master, p.owner] } } + let(:project) { create(:project, :public).tap { |p| p.team << [p.owner, :master, p.owner] } } let(:user) { create(:user) } let(:project_member) do project.request_access(user) @@ -429,7 +429,7 @@ describe Notify do context 'for a project in a group' do let(:group_owner) { create(:user) } let(:group) { create(:group).tap { |g| g.add_owner(group_owner) } } - let(:project) { create(:project, namespace: group) } + let(:project) { create(:project, :public, namespace: group) } let(:user) { create(:user) } let(:project_member) do project.request_access(user) @@ -492,21 +492,22 @@ describe Notify do end end - def invite_to_project(project:, email:, inviter:) - Member.add_user( - project.project_members, - 'toto@example.com', - Gitlab::Access::DEVELOPER, - current_user: inviter + def invite_to_project(project, inviter:) + create( + :project_member, + :developer, + project: project, + invite_token: '1234', + invite_email: 'toto@example.com', + user: nil, + created_by: inviter ) - - project.project_members.invite.last end describe 'project invitation' do let(:project) { create(:project) } let(:master) { create(:user).tap { |u| project.team << [u, :master] } } - let(:project_member) { invite_to_project(project: project, email: 'toto@example.com', inviter: master) } + let(:project_member) { invite_to_project(project, inviter: master) } subject { Notify.member_invited_email('project', project_member.id, project_member.invite_token) } @@ -525,10 +526,10 @@ describe Notify do describe 'project invitation accepted' do let(:project) { create(:project) } - let(:invited_user) { create(:user) } + let(:invited_user) { create(:user, name: 'invited user') } let(:master) { create(:user).tap { |u| project.team << [u, :master] } } let(:project_member) do - invitee = invite_to_project(project: project, email: 'toto@example.com', inviter: master) + invitee = invite_to_project(project, inviter: master) invitee.accept_invite!(invited_user) invitee end @@ -552,7 +553,7 @@ describe Notify do let(:project) { create(:project) } let(:master) { create(:user).tap { |u| project.team << [u, :master] } } let(:project_member) do - invitee = invite_to_project(project: project, email: 'toto@example.com', inviter: master) + invitee = invite_to_project(project, inviter: master) invitee.decline_invite! invitee end @@ -744,21 +745,22 @@ describe Notify do end end - def invite_to_group(group:, email:, inviter:) - Member.add_user( - group.group_members, - 'toto@example.com', - Gitlab::Access::DEVELOPER, - current_user: inviter + def invite_to_group(group, inviter:) + create( + :group_member, + :developer, + group: group, + invite_token: '1234', + invite_email: 'toto@example.com', + user: nil, + created_by: inviter ) - - group.group_members.invite.last end describe 'group invitation' do let(:group) { create(:group) } let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } - let(:group_member) { invite_to_group(group: group, email: 'toto@example.com', inviter: owner) } + let(:group_member) { invite_to_group(group, inviter: owner) } subject { Notify.member_invited_email('group', group_member.id, group_member.invite_token) } @@ -777,10 +779,10 @@ describe Notify do describe 'group invitation accepted' do let(:group) { create(:group) } - let(:invited_user) { create(:user) } + let(:invited_user) { create(:user, name: 'invited user') } let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } let(:group_member) do - invitee = invite_to_group(group: group, email: 'toto@example.com', inviter: owner) + invitee = invite_to_group(group, inviter: owner) invitee.accept_invite!(invited_user) invitee end @@ -804,7 +806,7 @@ describe Notify do let(:group) { create(:group) } let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } let(:group_member) do - invitee = invite_to_group(group: group, email: 'toto@example.com', inviter: owner) + invitee = invite_to_group(group, inviter: owner) invitee.decline_invite! invitee end @@ -829,6 +831,7 @@ describe Notify do let(:user) { create(:user, email: 'old-email@mail.com') } before do + stub_config_setting(email_subject_suffix: 'A Nice Suffix') perform_enqueued_jobs do user.email = "new-email@mail.com" user.save @@ -845,7 +848,7 @@ describe Notify do end it 'has the correct subject' do - is_expected.to have_subject "Confirmation instructions" + is_expected.to have_subject /^Confirmation instructions/ end it 'includes a link to the site' do diff --git a/spec/mailers/shared/notify.rb b/spec/mailers/shared/notify.rb index 56872da9a8f..3956d05060b 100644 --- a/spec/mailers/shared/notify.rb +++ b/spec/mailers/shared/notify.rb @@ -37,6 +37,16 @@ shared_examples 'an email sent from GitLab' do reply_to = subject.header[:reply_to].addresses expect(reply_to).to eq([gitlab_sender_reply_to]) end + + context 'when custom suffix for email subject is set' do + before do + stub_config_setting(email_subject_suffix: 'A Nice Suffix') + end + + it 'ends the subject with the suffix' do + is_expected.to have_subject /\ \| A Nice Suffix$/ + end + end end shared_examples 'an email that contains a header with author username' do @@ -169,8 +179,9 @@ shared_examples 'it should show Gmail Actions View Commit link' do end shared_examples 'an unsubscribeable thread' do - it 'has a List-Unsubscribe header' do + it 'has a List-Unsubscribe header in the correct format' do is_expected.to have_header 'List-Unsubscribe', /unsubscribe/ + is_expected.to have_header 'List-Unsubscribe', /^<.+>$/ end it { is_expected.to have_body_text /unsubscribe/ } diff --git a/spec/models/cycle_analytics/summary_spec.rb b/spec/models/cycle_analytics/summary_spec.rb index 743bc2da33f..9d67bc82cba 100644 --- a/spec/models/cycle_analytics/summary_spec.rb +++ b/spec/models/cycle_analytics/summary_spec.rb @@ -34,6 +34,12 @@ describe CycleAnalytics::Summary, models: true do expect(subject.commits).to eq(0) end + + it "finds a large (> 100) snumber of commits if present" do + Timecop.freeze(5.days.from_now) { create_commit("Test message", project, user, 'master', count: 100) } + + expect(subject.commits).to eq(100) + end end describe "#deploys" do diff --git a/spec/models/global_milestone_spec.rb b/spec/models/global_milestone_spec.rb index 92e0f7f27ce..dd033480527 100644 --- a/spec/models/global_milestone_spec.rb +++ b/spec/models/global_milestone_spec.rb @@ -50,8 +50,9 @@ describe GlobalMilestone, models: true do milestone1_project2, milestone1_project3, ] + milestones_relation = Milestone.where(id: milestones.map(&:id)) - @global_milestone = GlobalMilestone.new(milestone1_project1.title, milestones) + @global_milestone = GlobalMilestone.new(milestone1_project1.title, milestones_relation) end it 'has exactly one group milestone' do @@ -67,7 +68,7 @@ describe GlobalMilestone, models: true do let(:milestone) { create(:milestone, title: "git / test", project: project1) } it 'strips out slashes and spaces' do - global_milestone = GlobalMilestone.new(milestone.title, [milestone]) + global_milestone = GlobalMilestone.new(milestone.title, Milestone.where(id: milestone.id)) expect(global_milestone.safe_title).to eq('git-test') end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 3259f795296..3b8b743af2d 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -494,7 +494,7 @@ describe Issue, models: true do context 'with an admin user' do let(:project) { create(:empty_project) } - let(:user) { create(:user, admin: true) } + let(:user) { create(:admin) } it 'returns true for a regular issue' do issue = build(:issue, project: project) diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 0b1634f654a..485121701af 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -57,7 +57,7 @@ describe Member, models: true do describe 'Scopes & finders' do before do - project = create(:empty_project) + project = create(:empty_project, :public) group = create(:group) @owner_user = create(:user).tap { |u| group.add_owner(u) } @owner = group.members.find_by(user_id: @owner_user.id) @@ -74,22 +74,17 @@ describe Member, models: true do @blocked_master = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::MASTER) @blocked_developer = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::DEVELOPER) - Member.add_user( - project.members, - 'toto1@example.com', - Gitlab::Access::DEVELOPER, - current_user: @master_user - ) - @invited_member = project.members.invite.find_by_invite_email('toto1@example.com') + @invited_member = create(:project_member, :developer, + project: project, + invite_token: '1234', + invite_email: 'toto1@example.com') accepted_invite_user = build(:user, state: :active) - Member.add_user( - project.members, - 'toto2@example.com', - Gitlab::Access::DEVELOPER, - current_user: @master_user - ) - @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) } + @accepted_invite_member = create(:project_member, :developer, + project: project, + invite_token: '1234', + invite_email: 'toto2@example.com'). + tap { |u| u.accept_invite!(accepted_invite_user) } requested_user = create(:user).tap { |u| project.request_access(u) } @requested_member = project.requesters.find_by(user_id: requested_user.id) @@ -176,39 +171,209 @@ describe Member, models: true do it { is_expected.to respond_to(:user_email) } end - describe ".add_user" do - let!(:user) { create(:user) } - let(:project) { create(:project) } + describe '.add_user' do + %w[project group].each do |source_type| + context "when source is a #{source_type}" do + let!(:source) { create(source_type, :public) } + let!(:user) { create(:user) } + let!(:admin) { create(:admin) } - context "when called with a user id" do - it "adds the user as a member" do - Member.add_user(project.project_members, user.id, ProjectMember::MASTER) + it 'returns a <Source>Member object' do + member = described_class.add_user(source, user, :master) - expect(project.users).to include(user) - end - end + expect(member).to be_a "#{source_type.classify}Member".constantize + expect(member).to be_persisted + end - context "when called with a user object" do - it "adds the user as a member" do - Member.add_user(project.project_members, user, ProjectMember::MASTER) + it 'sets members.created_by to the given current_user' do + member = described_class.add_user(source, user, :master, current_user: admin) - expect(project.users).to include(user) - end - end + expect(member.created_by).to eq(admin) + end - context "when called with a known user email" do - it "adds the user as a member" do - Member.add_user(project.project_members, user.email, ProjectMember::MASTER) + it 'sets members.expires_at to the given expires_at' do + member = described_class.add_user(source, user, :master, expires_at: Date.new(2016, 9, 22)) - expect(project.users).to include(user) - end - end + expect(member.expires_at).to eq(Date.new(2016, 9, 22)) + end + + described_class.access_levels.each do |sym_key, int_access_level| + it "accepts the :#{sym_key} symbol as access level" do + expect(source.users).not_to include(user) + + member = described_class.add_user(source, user.id, sym_key) + + expect(member.access_level).to eq(int_access_level) + expect(source.users.reload).to include(user) + end + + it "accepts the #{int_access_level} integer as access level" do + expect(source.users).not_to include(user) + + member = described_class.add_user(source, user.id, int_access_level) + + expect(member.access_level).to eq(int_access_level) + expect(source.users.reload).to include(user) + end + end + + context 'with no current_user' do + context 'when called with a known user id' do + it 'adds the user as a member' do + expect(source.users).not_to include(user) + + described_class.add_user(source, user.id, :master) + + expect(source.users.reload).to include(user) + end + end + + context 'when called with an unknown user id' do + it 'adds the user as a member' do + expect(source.users).not_to include(user) + + described_class.add_user(source, 42, :master) + + expect(source.users.reload).not_to include(user) + end + end + + context 'when called with a user object' do + it 'adds the user as a member' do + expect(source.users).not_to include(user) + + described_class.add_user(source, user, :master) + + expect(source.users.reload).to include(user) + end + end + + context 'when called with a requester user object' do + before do + source.request_access(user) + end + + it 'adds the requester as a member' do + expect(source.users).not_to include(user) + expect(source.requesters.exists?(user_id: user)).to be_truthy + + expect { described_class.add_user(source, user, :master) }. + to raise_error(Gitlab::Access::AccessDeniedError) + + expect(source.users.reload).not_to include(user) + expect(source.requesters.reload.exists?(user_id: user)).to be_truthy + end + end + + context 'when called with a known user email' do + it 'adds the user as a member' do + expect(source.users).not_to include(user) + + described_class.add_user(source, user.email, :master) + + expect(source.users.reload).to include(user) + end + end + + context 'when called with an unknown user email' do + it 'creates an invited member' do + expect(source.users).not_to include(user) + + described_class.add_user(source, 'user@example.com', :master) + + expect(source.members.invite.pluck(:invite_email)).to include('user@example.com') + end + end + end + + context 'when current_user can update member' do + it 'creates the member' do + expect(source.users).not_to include(user) + + described_class.add_user(source, user, :master, current_user: admin) + + expect(source.users.reload).to include(user) + end + + context 'when called with a requester user object' do + before do + source.request_access(user) + end + + it 'adds the requester as a member' do + expect(source.users).not_to include(user) + expect(source.requesters.exists?(user_id: user)).to be_truthy + + described_class.add_user(source, user, :master, current_user: admin) + + expect(source.users.reload).to include(user) + expect(source.requesters.reload.exists?(user_id: user)).to be_falsy + end + end + end + + context 'when current_user cannot update member' do + it 'does not create the member' do + expect(source.users).not_to include(user) + + member = described_class.add_user(source, user, :master, current_user: user) + + expect(source.users.reload).not_to include(user) + expect(member).not_to be_persisted + end + + context 'when called with a requester user object' do + before do + source.request_access(user) + end + + it 'does not destroy the requester' do + expect(source.users).not_to include(user) + expect(source.requesters.exists?(user_id: user)).to be_truthy + + described_class.add_user(source, user, :master, current_user: user) + + expect(source.users.reload).not_to include(user) + expect(source.requesters.exists?(user_id: user)).to be_truthy + end + end + end + + context 'when member already exists' do + before do + source.add_user(user, :developer) + end + + context 'with no current_user' do + it 'updates the member' do + expect(source.users).to include(user) + + described_class.add_user(source, user, :master) + + expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MASTER) + end + end + + context 'when current_user can update member' do + it 'updates the member' do + expect(source.users).to include(user) + + described_class.add_user(source, user, :master, current_user: admin) + + expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MASTER) + end + end + + context 'when current_user cannot update member' do + it 'does not update the member' do + expect(source.users).to include(user) - context "when called with an unknown user email" do - it "adds a member invite" do - Member.add_user(project.project_members, "user@example.com", ProjectMember::MASTER) + described_class.add_user(source, user, :master, current_user: user) - expect(project.project_members.invite.pluck(:invite_email)).to include("user@example.com") + expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::DEVELOPER) + end + end + end end end end diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 56fa7fa6134..370aeb9e0a9 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -1,6 +1,33 @@ require 'spec_helper' describe GroupMember, models: true do + describe '.access_level_roles' do + it 'returns Gitlab::Access.options_with_owner' do + expect(described_class.access_level_roles).to eq(Gitlab::Access.options_with_owner) + end + end + + describe '.access_levels' do + it 'returns Gitlab::Access.options_with_owner' do + expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner) + end + end + + describe '.add_users_to_group' do + it 'adds the given users to the given group' do + group = create(:group) + users = create_list(:user, 2) + + described_class.add_users_to_group( + group, + [users.first.id, users.second], + described_class::MASTER + ) + + expect(group.users).to include(users.first, users.second) + end + end + describe 'notifications' do describe "#after_create" do it "sends email to user" do diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 805c15a4e5e..d85a1c1e3b2 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -15,6 +15,26 @@ describe ProjectMember, models: true do it { is_expected.to include_module(Gitlab::ShellAdapter) } end + describe '.access_level_roles' do + it 'returns Gitlab::Access.options' do + expect(described_class.access_level_roles).to eq(Gitlab::Access.options) + end + end + + describe '.add_user' do + context 'when called with the project owner' do + it 'adds the user as a member' do + project = create(:empty_project) + + expect(project.users).not_to include(project.owner) + + described_class.add_user(project, project.owner, :master, current_user: project.owner) + + expect(project.users.reload).to include(project.owner) + end + end + end + describe '#real_source_type' do subject { create(:project_member).real_source_type } @@ -50,7 +70,7 @@ describe ProjectMember, models: true do end end - describe :import_team do + describe '.import_team' do before do @project_1 = create :project @project_2 = create :project @@ -81,25 +101,21 @@ describe ProjectMember, models: true do end describe '.add_users_to_projects' do - before do - @project_1 = create :project - @project_2 = create :project + it 'adds the given users to the given projects' do + projects = create_list(:empty_project, 2) + users = create_list(:user, 2) - @user_1 = create :user - @user_2 = create :user - - ProjectMember.add_users_to_projects( - [@project_1.id, @project_2.id], - [@user_1.id, @user_2.id], - ProjectMember::MASTER - ) - end + described_class.add_users_to_projects( + [projects.first.id, projects.second], + [users.first.id, users.second], + described_class::MASTER) - it { expect(@project_1.users).to include(@user_1) } - it { expect(@project_1.users).to include(@user_2) } + expect(projects.first.users).to include(users.first) + expect(projects.first.users).to include(users.second) - it { expect(@project_2.users).to include(@user_1) } - it { expect(@project_2.users).to include(@user_2) } + expect(projects.second.users).to include(users.first) + expect(projects.second.users).to include(users.second) + end end describe '.truncate_teams' do diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index e5b185dc3f6..530a7def553 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -64,5 +64,27 @@ describe MergeRequestDiff, models: true do end end end + + describe '#commits_sha' do + shared_examples 'returning all commits SHA' do + it 'returns all commits SHA' do + commits_sha = subject.commits_sha + + expect(commits_sha).to eq(subject.commits.map(&:sha)) + end + end + + context 'when commits were loaded' do + before do + subject.commits + end + + it_behaves_like 'returning all commits SHA' + end + + context 'when commits were not loaded' do + it_behaves_like 'returning all commits SHA' + end + end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 06feeb1bbba..9d7be2429ed 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -287,6 +287,46 @@ describe MergeRequest, models: true do end end + describe "#wipless_title" do + ['WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP [WIP] WIP: WIP '].each do |wip_prefix| + it "removes the '#{wip_prefix}' prefix" do + wipless_title = subject.title + subject.title = "#{wip_prefix}#{subject.title}" + + expect(subject.wipless_title).to eq wipless_title + end + + it "is satisfies the #work_in_progress? method" do + subject.title = "#{wip_prefix}#{subject.title}" + subject.title = subject.wipless_title + + expect(subject.work_in_progress?).to eq false + end + end + end + + describe "#wip_title" do + it "adds the WIP: prefix to the title" do + wip_title = "WIP: #{subject.title}" + + expect(subject.wip_title).to eq wip_title + end + + it "does not add the WIP: prefix multiple times" do + wip_title = "WIP: #{subject.title}" + subject.title = subject.wip_title + subject.title = subject.wip_title + + expect(subject.wip_title).to eq wip_title + end + + it "is satisfies the #work_in_progress? method" do + subject.title = subject.wip_title + + expect(subject.work_in_progress?).to eq true + end + end + describe '#can_remove_source_branch?' do let(:user) { create(:user) } let(:user2) { create(:user) } @@ -328,6 +368,42 @@ describe MergeRequest, models: true do end end + describe '#merge_commit_message' do + it 'includes merge information as the title' do + request = build(:merge_request, source_branch: 'source', target_branch: 'target') + + expect(request.merge_commit_message) + .to match("Merge branch 'source' into 'target'\n\n") + end + + it 'includes its title in the body' do + request = build(:merge_request, title: 'Remove all technical debt') + + expect(request.merge_commit_message) + .to match("Remove all technical debt\n\n") + end + + it 'includes its description in the body' do + request = build(:merge_request, description: 'By removing all code') + + expect(request.merge_commit_message) + .to match("By removing all code\n\n") + end + + it 'includes its reference in the body' do + request = build_stubbed(:merge_request) + + expect(request.merge_commit_message) + .to match("See merge request #{request.to_reference}") + end + + it 'excludes multiple linebreak runs when description is blank' do + request = build(:merge_request, title: 'Title', description: nil) + + expect(request.merge_commit_message).not_to match("Title\n\n\n\n") + end + end + describe "#reset_merge_when_build_succeeds" do let(:merge_if_green) do create :merge_request, merge_when_build_succeeds: true, merge_user: create(:user), @@ -495,15 +571,77 @@ describe MergeRequest, models: true do end describe '#all_pipelines' do - let!(:pipelines) do - subject.merge_request_diff.commits.map do |commit| - create(:ci_empty_pipeline, project: subject.source_project, sha: commit.id, ref: subject.source_branch) + shared_examples 'returning pipelines with proper ordering' do + let!(:all_pipelines) do + subject.all_commits_sha.map do |sha| + create(:ci_empty_pipeline, + project: subject.source_project, + sha: sha, + ref: subject.source_branch) + end + end + + it 'returns all pipelines' do + expect(subject.all_pipelines).not_to be_empty + expect(subject.all_pipelines).to eq(all_pipelines.reverse) end end - it 'returns a pipelines from source projects with proper ordering' do - expect(subject.all_pipelines).not_to be_empty - expect(subject.all_pipelines).to eq(pipelines.reverse) + context 'with single merge_request_diffs' do + it_behaves_like 'returning pipelines with proper ordering' + end + + context 'with multiple irrelevant merge_request_diffs' do + before do + subject.update(target_branch: 'v1.0.0') + end + + it_behaves_like 'returning pipelines with proper ordering' + end + + context 'with unsaved merge request' do + subject { build(:merge_request) } + + let!(:pipeline) do + create(:ci_empty_pipeline, + project: subject.project, + sha: subject.diff_head_sha, + ref: subject.source_branch) + end + + it 'returns pipelines from diff_head_sha' do + expect(subject.all_pipelines).to contain_exactly(pipeline) + end + end + end + + describe '#all_commits_sha' do + let(:all_commits_sha) do + subject.merge_request_diffs.flat_map(&:commits).map(&:sha).uniq + end + + shared_examples 'returning all SHA' do + it 'returns all SHA from all merge_request_diffs' do + expect(subject.merge_request_diffs.size).to eq(2) + expect(subject.all_commits_sha).to eq(all_commits_sha) + end + end + + context 'with a completely different branch' do + before do + subject.update(target_branch: 'v1.0.0') + end + + it_behaves_like 'returning all SHA' + end + + context 'with a branch having no difference' do + before do + subject.update(target_branch: 'v1.1.0') + subject.reload # make sure commits were not cached + end + + it_behaves_like 'returning all SHA' end end @@ -701,16 +839,57 @@ describe MergeRequest, models: true do end end - describe "#environments" do + describe '#environments' do let(:project) { create(:project) } let(:merge_request) { create(:merge_request, source_project: project) } - it 'selects deployed environments' do - environments = create_list(:environment, 3, project: project) - create(:deployment, environment: environments.first, sha: project.commit('master').id) - create(:deployment, environment: environments.second, sha: project.commit('feature').id) + context 'with multiple environments' do + let(:environments) { create_list(:environment, 3, project: project) } + + before do + create(:deployment, environment: environments.first, ref: 'master', sha: project.commit('master').id) + create(:deployment, environment: environments.second, ref: 'feature', sha: project.commit('feature').id) + end - expect(merge_request.environments).to eq [environments.first] + it 'selects deployed environments' do + expect(merge_request.environments).to contain_exactly(environments.first) + end + end + + context 'with environments on source project' do + let(:source_project) do + create(:project) do |fork_project| + fork_project.create_forked_project_link(forked_to_project_id: fork_project.id, forked_from_project_id: project.id) + end + end + + let(:merge_request) do + create(:merge_request, + source_project: source_project, source_branch: 'feature', + target_project: project) + end + + let(:source_environment) { create(:environment, project: source_project) } + + before do + create(:deployment, environment: source_environment, ref: 'feature', sha: merge_request.diff_head_sha) + end + + it 'selects deployed environments' do + expect(merge_request.environments).to contain_exactly(source_environment) + end + + context 'with environments on target project' do + let(:target_environment) { create(:environment, project: project) } + + before do + create(:deployment, environment: target_environment, tag: true, sha: merge_request.diff_head_sha) + end + + it 'selects deployed environments' do + expect(merge_request.environments).to contain_exactly(source_environment, target_environment) + end + end end context 'without a diff_head_commit' do diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index d64d6cde2b5..33fe22dd98c 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -20,10 +20,10 @@ describe Milestone, models: true do let(:user) { create(:user) } describe "#title" do - let(:milestone) { create(:milestone, title: "<b>test</b>") } + let(:milestone) { create(:milestone, title: "<b>foo & bar -> 2.2</b>") } it "sanitizes title" do - expect(milestone.title).to eq("test") + expect(milestone.title).to eq("foo & bar -> 2.2") end end diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/project_services/custom_issue_tracker_service_spec.rb index 7020667ea58..63320931e76 100644 --- a/spec/models/project_services/custom_issue_tracker_service_spec.rb +++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb @@ -25,5 +25,21 @@ describe CustomIssueTrackerService, models: true do it { is_expected.not_to validate_presence_of(:issues_url) } it { is_expected.not_to validate_presence_of(:new_issue_url) } end + + context 'title' do + let(:issue_tracker) { described_class.new(properties: {}) } + + it 'sets a default title' do + issue_tracker.title = nil + + expect(issue_tracker.title).to eq('Custom Issue Tracker') + end + + it 'sets the custom title' do + issue_tracker.title = 'test title' + + expect(issue_tracker.title).to eq('test title') + end + end end end diff --git a/spec/models/project_services/slack_service/issue_message_spec.rb b/spec/models/project_services/slack_service/issue_message_spec.rb index 0f8889bdf3c..98c36ec088d 100644 --- a/spec/models/project_services/slack_service/issue_message_spec.rb +++ b/spec/models/project_services/slack_service/issue_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::IssueMessage, models: true do { user: { name: 'Test User', - username: 'Test User' + username: 'test.user' }, project_name: 'project_name', project_url: 'somewhere.com', @@ -40,7 +40,7 @@ describe SlackService::IssueMessage, models: true do context 'open' do it 'returns a message regarding opening of issues' do expect(subject.pretext).to eq( - '<somewhere.com|[project_name>] Issue opened by Test User') + '<somewhere.com|[project_name>] Issue opened by test.user') expect(subject.attachments).to eq([ { title: "#100 Issue title", @@ -60,7 +60,7 @@ describe SlackService::IssueMessage, models: true do it 'returns a message regarding closing of issues' do expect(subject.pretext). to eq( - '<somewhere.com|[project_name>] Issue <url|#100 Issue title> closed by Test User') + '<somewhere.com|[project_name>] Issue <url|#100 Issue title> closed by test.user') expect(subject.attachments).to be_empty end end diff --git a/spec/models/project_services/slack_service/merge_message_spec.rb b/spec/models/project_services/slack_service/merge_message_spec.rb index 224c7ceabe8..c5c052d9af1 100644 --- a/spec/models/project_services/slack_service/merge_message_spec.rb +++ b/spec/models/project_services/slack_service/merge_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::MergeMessage, models: true do { user: { name: 'Test User', - username: 'Test User' + username: 'test.user' }, project_name: 'project_name', project_url: 'somewhere.com', @@ -31,7 +31,7 @@ describe SlackService::MergeMessage, models: true do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'Test User opened <somewhere.com/merge_requests/100|merge request !100> '\ + 'test.user opened <somewhere.com/merge_requests/100|merge request !100> '\ 'in <somewhere.com|project_name>: *Issue title*') expect(subject.attachments).to be_empty end @@ -43,7 +43,7 @@ describe SlackService::MergeMessage, models: true do end it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'Test User closed <somewhere.com/merge_requests/100|merge request !100> '\ + 'test.user closed <somewhere.com/merge_requests/100|merge request !100> '\ 'in <somewhere.com|project_name>: *Issue title*') expect(subject.attachments).to be_empty end diff --git a/spec/models/project_services/slack_service/note_message_spec.rb b/spec/models/project_services/slack_service/note_message_spec.rb index 41b93f08050..38cfe4ad3e3 100644 --- a/spec/models/project_services/slack_service/note_message_spec.rb +++ b/spec/models/project_services/slack_service/note_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::NoteMessage, models: true do @args = { user: { name: 'Test User', - username: 'username', + username: 'test.user', avatar_url: 'http://fakeavatar' }, project_name: 'project_name', @@ -37,7 +37,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on commits' do message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("Test User commented on " \ + expect(message.pretext).to eq("test.user commented on " \ "<url|commit 5f163b2b> in <somewhere.com|project_name>: " \ "*Added a commit message*") expected_attachments = [ @@ -63,7 +63,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on a merge request' do message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("Test User commented on " \ + expect(message.pretext).to eq("test.user commented on " \ "<url|merge request !30> in <somewhere.com|project_name>: " \ "*merge request title*") expected_attachments = [ @@ -90,7 +90,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on an issue' do message = SlackService::NoteMessage.new(@args) expect(message.pretext).to eq( - "Test User commented on " \ + "test.user commented on " \ "<url|issue #20> in <somewhere.com|project_name>: " \ "*issue title*") expected_attachments = [ @@ -115,7 +115,7 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on a project snippet' do message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("Test User commented on " \ + expect(message.pretext).to eq("test.user commented on " \ "<url|snippet #5> in <somewhere.com|project_name>: " \ "*snippet title*") expected_attachments = [ diff --git a/spec/models/project_services/slack_service/push_message_spec.rb b/spec/models/project_services/slack_service/push_message_spec.rb index cda9ee670b0..17cd05e24f1 100644 --- a/spec/models/project_services/slack_service/push_message_spec.rb +++ b/spec/models/project_services/slack_service/push_message_spec.rb @@ -9,7 +9,7 @@ describe SlackService::PushMessage, models: true do before: 'before', project_name: 'project_name', ref: 'refs/heads/master', - user_name: 'user_name', + user_name: 'test.user', project_url: 'url' } end @@ -26,7 +26,7 @@ describe SlackService::PushMessage, models: true do it 'returns a message regarding pushes' do expect(subject.pretext).to eq( - 'user_name pushed to branch <url/commits/master|master> of '\ + 'test.user pushed to branch <url/commits/master|master> of '\ '<url|project_name> (<url/compare/before...after|Compare changes>)' ) expect(subject.attachments).to eq([ @@ -46,13 +46,13 @@ describe SlackService::PushMessage, models: true do before: Gitlab::Git::BLANK_SHA, project_name: 'project_name', ref: 'refs/tags/new_tag', - user_name: 'user_name', + user_name: 'test.user', project_url: 'url' } end it 'returns a message regarding pushes' do - expect(subject.pretext).to eq('user_name pushed new tag ' \ + expect(subject.pretext).to eq('test.user pushed new tag ' \ '<url/commits/new_tag|new_tag> to ' \ '<url|project_name>') expect(subject.attachments).to be_empty @@ -66,7 +66,7 @@ describe SlackService::PushMessage, models: true do it 'returns a message regarding a new branch' do expect(subject.pretext).to eq( - 'user_name pushed new branch <url/commits/master|master> to '\ + 'test.user pushed new branch <url/commits/master|master> to '\ '<url|project_name>' ) expect(subject.attachments).to be_empty @@ -80,7 +80,7 @@ describe SlackService::PushMessage, models: true do it 'returns a message regarding a removed branch' do expect(subject.pretext).to eq( - 'user_name removed branch master from <url|project_name>' + 'test.user removed branch master from <url|project_name>' ) expect(subject.attachments).to be_empty end diff --git a/spec/models/project_services/slack_service/wiki_page_message_spec.rb b/spec/models/project_services/slack_service/wiki_page_message_spec.rb index 13aea0b0600..093911598b0 100644 --- a/spec/models/project_services/slack_service/wiki_page_message_spec.rb +++ b/spec/models/project_services/slack_service/wiki_page_message_spec.rb @@ -7,7 +7,7 @@ describe SlackService::WikiPageMessage, models: true do { user: { name: 'Test User', - username: 'Test User' + username: 'test.user' }, project_name: 'project_name', project_url: 'somewhere.com', @@ -25,7 +25,7 @@ describe SlackService::WikiPageMessage, models: true do it 'returns a message that a new wiki page was created' do expect(subject.pretext).to eq( - 'Test User created <url|wiki page> in <somewhere.com|project_name>: '\ + 'test.user created <url|wiki page> in <somewhere.com|project_name>: '\ '*Wiki page title*') end end @@ -35,7 +35,7 @@ describe SlackService::WikiPageMessage, models: true do it 'returns a message that a wiki page was updated' do expect(subject.pretext).to eq( - 'Test User edited <url|wiki page> in <somewhere.com|project_name>: '\ + 'test.user edited <url|wiki page> in <somewhere.com|project_name>: '\ '*Wiki page title*') end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a388ff703a6..ef854a25321 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -68,7 +68,7 @@ describe Project, models: true do it { is_expected.to have_many(:forks).through(:forked_project_links) } describe '#members & #requesters' do - let(:project) { create(:project) } + let(:project) { create(:project, :public) } let(:requester) { create(:user) } let(:developer) { create(:user) } before do @@ -836,7 +836,7 @@ describe Project, models: true do describe 'when a user has access to a project' do before do - project.team.add_user(user, Gitlab::Access::MASTER) + project.add_user(user, Gitlab::Access::MASTER) end it { is_expected.to eq([project]) } @@ -1647,6 +1647,47 @@ describe Project, models: true do end end + describe '#environments_for' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + + context 'tagged deployment' do + before do + create(:deployment, environment: environment, ref: '1.0', tag: true, sha: project.commit.id) + end + + it 'returns environment when with_tags is set' do + expect(project.environments_for('master', project.commit, with_tags: true)).to contain_exactly(environment) + end + + it 'does not return environment when no with_tags is set' do + expect(project.environments_for('master', project.commit)).to be_empty + end + + it 'does not return environment when commit is not part of deployment' do + expect(project.environments_for('master', project.commit('feature'))).to be_empty + end + end + + context 'branch deployment' do + before do + create(:deployment, environment: environment, ref: 'master', sha: project.commit.id) + end + + it 'returns environment when ref is set' do + expect(project.environments_for('master', project.commit)).to contain_exactly(environment) + end + + it 'does not environment when ref is different' do + expect(project.environments_for('feature', project.commit)).to be_empty + end + + it 'does not return environment when commit is not part of deployment' do + expect(project.environments_for('master', project.commit('feature'))).to be_empty + end + end + end + def enable_lfs allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 5eaf0d3b7a6..e0f2dadf189 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -73,9 +73,71 @@ describe ProjectTeam, models: true do end end - describe '#find_member' do + describe '#fetch_members' do context 'personal project' do let(:project) { create(:empty_project) } + + it 'returns project members' do + user = create(:user) + project.team << [user, :guest] + + expect(project.team.members).to contain_exactly(user) + end + + it 'returns project members of a specified level' do + user = create(:user) + project.team << [user, :reporter] + + expect(project.team.guests).to be_empty + expect(project.team.reporters).to contain_exactly(user) + end + + it 'returns invited members of a group' do + group_member = create(:group_member) + + project.project_group_links.create!( + group: group_member.group, + group_access: Gitlab::Access::GUEST + ) + + expect(project.team.members).to contain_exactly(group_member.user) + end + + it 'returns invited members of a group of a specified level' do + group_member = create(:group_member) + + project.project_group_links.create!( + group: group_member.group, + group_access: Gitlab::Access::REPORTER + ) + + expect(project.team.guests).to be_empty + expect(project.team.reporters).to contain_exactly(group_member.user) + end + end + + context 'group project' do + let(:group) { create(:group) } + let(:project) { create(:empty_project, group: group) } + + it 'returns project members' do + group_member = create(:group_member, group: group) + + expect(project.team.members).to contain_exactly(group_member.user) + end + + it 'returns project members of a specified level' do + group_member = create(:group_member, :reporter, group: group) + + expect(project.team.guests).to be_empty + expect(project.team.reporters).to contain_exactly(group_member.user) + end + end + end + + describe '#find_member' do + context 'personal project' do + let(:project) { create(:empty_project, :public) } let(:requester) { create(:user) } before do @@ -138,7 +200,7 @@ describe ProjectTeam, models: true do let(:requester) { create(:user) } context 'personal project' do - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :public) } context 'when project is not shared with group' do before do diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 0621c6a06ce..e6bc5296398 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -9,12 +9,14 @@ describe Snippet, models: true do it { is_expected.to include_module(Participable) } it { is_expected.to include_module(Referable) } it { is_expected.to include_module(Sortable) } + it { is_expected.to include_module(Awardable) } end describe 'associations' do it { is_expected.to belong_to(:author).class_name('User') } it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:notes).dependent(:destroy) } + it { is_expected.to have_many(:award_emoji).dependent(:destroy) } end describe 'validation' do diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb index d78494b76fa..905a7311372 100644 --- a/spec/requests/api/access_requests_spec.rb +++ b/spec/requests/api/access_requests_spec.rb @@ -64,12 +64,12 @@ describe API::AccessRequests, api: true do context 'when authenticated as a member' do %i[developer master].each do |type| context "as a #{type}" do - it 'returns 400' do + it 'returns 403' do expect do user = public_send(type) post api("/#{source_type.pluralize}/#{source.id}/access_requests", user) - expect(response).to have_http_status(400) + expect(response).to have_http_status(403) end.not_to change { source.requesters.count } end end @@ -87,6 +87,20 @@ describe API::AccessRequests, api: true do end context 'when authenticated as a stranger' do + context "when access request is disabled for the #{source_type}" do + before do + source.update(request_access_enabled: false) + end + + it 'returns 403' do + expect do + post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger) + + expect(response).to have_http_status(403) + end.not_to change { source.requesters.count } + end + end + it 'returns 201' do expect do post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger) diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index bbdf8f03c2b..e66faeed705 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -36,11 +36,36 @@ describe API::Helpers, api: true do params.delete(API::Helpers::SUDO_PARAM) end + def warden_authenticate_returns(value) + warden = double("warden", authenticate: value) + env['warden'] = warden + end + + def doorkeeper_guard_returns(value) + allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ value } + end + def error!(message, status) raise Exception end describe ".current_user" do + subject { current_user } + + describe "when authenticating via Warden" do + before { doorkeeper_guard_returns false } + + context "fails" do + it { is_expected.to be_nil } + end + + context "succeeds" do + before { warden_authenticate_returns user } + + it { is_expected.to eq(user) } + end + end + describe "when authenticating using a user's private token" do it "returns nil for an invalid token" do env[API::Helpers::PRIVATE_TOKEN_HEADER] = 'invalid token' diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb index 981a6791881..5ad4fc4865a 100644 --- a/spec/requests/api/award_emoji_spec.rb +++ b/spec/requests/api/award_emoji_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe API::API, api: true do include ApiHelpers let(:user) { create(:user) } - let!(:project) { create(:project) } + let!(:project) { create(:empty_project) } let(:issue) { create(:issue, project: project) } let!(:award_emoji) { create(:award_emoji, awardable: issue, user: user) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } @@ -39,6 +39,19 @@ describe API::API, api: true do end end + context 'on a snippet' do + let(:snippet) { create(:project_snippet, :public, project: project) } + let!(:award) { create(:award_emoji, awardable: snippet) } + + it 'returns the awarded emoji' do + get api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(award.name) + end + end + context 'when the user has no access' do it 'returns a status code 404' do user1 = create(:user) @@ -91,6 +104,20 @@ describe API::API, api: true do end end + context 'on a snippet' do + let(:snippet) { create(:project_snippet, :public, project: project) } + let!(:award) { create(:award_emoji, awardable: snippet) } + + it 'returns the awarded emoji' do + get api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji/#{award.id}", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(award.name) + expect(json_response['awardable_id']).to eq(snippet.id) + expect(json_response['awardable_type']).to eq("Snippet") + end + end + context 'when the user has no access' do it 'returns a status code 404' do user1 = create(:user) @@ -160,6 +187,18 @@ describe API::API, api: true do end end end + + context 'on a snippet' do + it 'creates a new award emoji' do + snippet = create(:project_snippet, :public, project: project) + + post api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji", user), name: 'blowfish' + + expect(response).to have_http_status(201) + expect(json_response['name']).to eq('blowfish') + expect(json_response['user']['username']).to eq(user.username) + end + end end describe "POST /projects/:id/awardable/:awardable_id/notes/:note_id/award_emoji" do @@ -229,6 +268,19 @@ describe API::API, api: true do expect(response).to have_http_status(404) end end + + context 'when the awardable is a Snippet' do + let(:snippet) { create(:project_snippet, :public, project: project) } + let!(:award) { create(:award_emoji, awardable: snippet, user: user) } + + it 'deletes the award' do + expect do + delete api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji/#{award.id}", user) + end.to change { snippet.award_emoji.count }.from(1).to(0) + + expect(response).to have_http_status(200) + end + end end describe 'DELETE /projects/:id/awardable/:awardable_id/award_emoji/:award_emoji_id' do diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index ee0b61e2ca4..95c7bbf99c9 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -30,6 +30,15 @@ describe API::API, api: true do expect(json_response.first['commit']['id']).to eq project.commit.id end + it 'returns pipeline data' do + json_build = json_response.first + expect(json_build['pipeline']).not_to be_empty + expect(json_build['pipeline']['id']).to eq build.pipeline.id + expect(json_build['pipeline']['ref']).to eq build.pipeline.ref + expect(json_build['pipeline']['sha']).to eq build.pipeline.sha + expect(json_build['pipeline']['status']).to eq build.pipeline.status + end + context 'filter project with one scope element' do let(:query) { 'scope=pending' } @@ -91,6 +100,15 @@ describe API::API, api: true do expect(json_response).to be_an Array expect(json_response.size).to eq 2 end + + it 'returns pipeline data' do + json_build = json_response.first + expect(json_build['pipeline']).not_to be_empty + expect(json_build['pipeline']['id']).to eq build.pipeline.id + expect(json_build['pipeline']['ref']).to eq build.pipeline.ref + expect(json_build['pipeline']['sha']).to eq build.pipeline.sha + expect(json_build['pipeline']['status']).to eq build.pipeline.status + end end context 'when pipeline has no builds' do @@ -133,6 +151,15 @@ describe API::API, api: true do expect(response).to have_http_status(200) expect(json_response['name']).to eq('test') end + + it 'returns pipeline data' do + json_build = json_response + expect(json_build['pipeline']).not_to be_empty + expect(json_build['pipeline']['id']).to eq build.pipeline.id + expect(json_build['pipeline']['ref']).to eq build.pipeline.ref + expect(json_build['pipeline']['sha']).to eq build.pipeline.sha + expect(json_build['pipeline']['status']).to eq build.pipeline.status + end end context 'unauthorized user' do diff --git a/spec/requests/api/fork_spec.rb b/spec/requests/api/fork_spec.rb index 06e3a2183c0..34f84f78952 100644 --- a/spec/requests/api/fork_spec.rb +++ b/spec/requests/api/fork_spec.rb @@ -94,7 +94,7 @@ describe API::API, api: true do it 'fails if trying to fork to another user when not admin' do post api("/projects/fork/#{project.id}", user2), namespace: admin.namespace.id - expect(response).to have_http_status(409) + expect(response).to have_http_status(404) end it 'fails if trying to fork to non-existent namespace' do @@ -114,7 +114,7 @@ describe API::API, api: true do it 'fails to fork to not owned group' do post api("/projects/fork/#{project.id}", user2), namespace: group.name - expect(response).to have_http_status(409) + expect(response).to have_http_status(404) end it 'forks to not owned group when admin' do diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 46e8e6f1169..f0f590b0331 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -111,7 +111,7 @@ describe API::API, api: true do expect(response).to have_http_status(200) expect(json_response['username']).to eq(user.username) - expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value) + expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).token) expect(json_response['repository_http_path']).to eq(project.http_url_to_repo) end @@ -131,7 +131,7 @@ describe API::API, api: true do expect(response).to have_http_status(200) expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}") - expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value) + expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).token) expect(json_response['repository_http_path']).to eq(project.http_url_to_repo) end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index a7930c59df9..b813ee967f8 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -15,7 +15,7 @@ describe API::API, api: true do let(:milestone) { create(:milestone, title: '1.0.0', project: project) } before do - project.team << [user, :reporters] + project.team << [user, :reporter] end describe "GET /projects/:id/merge_requests" do @@ -299,7 +299,7 @@ describe API::API, api: true do let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } before :each do |each| - fork_project.team << [user2, :reporters] + fork_project.team << [user2, :reporter] end it "returns merge_request" do diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb index b89dac01040..dd192bea432 100644 --- a/spec/requests/api/milestones_spec.rb +++ b/spec/requests/api/milestones_spec.rb @@ -104,6 +104,14 @@ describe API::API, api: true do expect(response).to have_http_status(400) end + + it 'creates a new project with reserved html characters' do + post api("/projects/#{project.id}/milestones", user), title: 'foo & bar 1.1 -> 2.2' + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq('foo & bar 1.1 -> 2.2') + expect(json_response['description']).to be_nil + end end describe 'PUT /projects/:id/milestones/:milestone_id' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 192c7d14c13..4a0d727faea 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -761,13 +761,16 @@ describe API::API, api: true do let(:group) { create(:group) } it "shares project with group" do + expires_at = 10.days.from_now.to_date + expect do - post api("/projects/#{project.id}/share", user), group_id: group.id, group_access: Gitlab::Access::DEVELOPER + post api("/projects/#{project.id}/share", user), group_id: group.id, group_access: Gitlab::Access::DEVELOPER, expires_at: expires_at end.to change { ProjectGroupLink.count }.by(1) expect(response.status).to eq 201 - expect(json_response['group_id']).to eq group.id - expect(json_response['group_access']).to eq Gitlab::Access::DEVELOPER + expect(json_response['group_id']).to eq(group.id) + expect(json_response['group_access']).to eq(Gitlab::Access::DEVELOPER) + expect(json_response['expires_at']).to eq(expires_at.to_s) end it "returns a 400 error when group id is not given" do diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 54d096e8b7f..f4903d8e0be 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -14,22 +14,38 @@ describe API::API, 'Settings', api: true do expect(json_response['default_projects_limit']).to eq(42) expect(json_response['signin_enabled']).to be_truthy expect(json_response['repository_storage']).to eq('default') + expect(json_response['koding_enabled']).to be_falsey + expect(json_response['koding_url']).to be_nil end end describe "PUT /application/settings" do - before do - storages = { 'custom' => 'tmp/tests/custom_repositories' } - allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) + context "custom repository storage type set in the config" do + before do + storages = { 'custom' => 'tmp/tests/custom_repositories' } + allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) + end + + it "updates application settings" do + put api("/application/settings", admin), + default_projects_limit: 3, signin_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com' + expect(response).to have_http_status(200) + expect(json_response['default_projects_limit']).to eq(3) + expect(json_response['signin_enabled']).to be_falsey + expect(json_response['repository_storage']).to eq('custom') + expect(json_response['koding_enabled']).to be_truthy + expect(json_response['koding_url']).to eq('http://koding.example.com') + end end - it "updates application settings" do - put api("/application/settings", admin), - default_projects_limit: 3, signin_enabled: false, repository_storage: 'custom' - expect(response).to have_http_status(200) - expect(json_response['default_projects_limit']).to eq(3) - expect(json_response['signin_enabled']).to be_falsey - expect(json_response['repository_storage']).to eq('custom') + context "missing koding_url value when koding_enabled is true" do + it "returns a blank parameter error message" do + put api("/application/settings", admin), koding_enabled: true + + expect(response).to have_http_status(400) + expect(json_response['message']).to have_key('koding_url') + expect(json_response['message']['koding_url']).to include "can't be blank" + end end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index ef73778efa9..f4ea3bebb4c 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -62,6 +62,7 @@ describe API::API, api: true do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.first.keys).to include 'email' + expect(json_response.first.keys).to include 'organization' expect(json_response.first.keys).to include 'identities' expect(json_response.first.keys).to include 'can_create_project' expect(json_response.first.keys).to include 'two_factor_enabled' @@ -265,6 +266,14 @@ describe API::API, api: true do expect(user.reload.bio).to eq('new test bio') end + it "updates user with organization" do + put api("/users/#{user.id}", admin), { organization: 'GitLab' } + + expect(response).to have_http_status(200) + expect(json_response['organization']).to eq('GitLab') + expect(user.reload.organization).to eq('GitLab') + end + it 'updates user with his own email' do put api("/users/#{user.id}", admin), email: user.email expect(response).to have_http_status(200) diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index e3922bec689..c0c1e62e910 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -1,508 +1,517 @@ require "spec_helper" describe 'Git HTTP requests', lib: true do + include GitHttpHelpers include WorkhorseHelpers - let(:user) { create(:user) } - let(:project) { create(:project, path: 'project.git-project') } - it "gives WWW-Authenticate hints" do clone_get('doesnt/exist.git') expect(response.header['WWW-Authenticate']).to start_with('Basic ') end - context "when the project doesn't exist" do - context "when no authentication is provided" do - it "responds with status 401 (no project existence information leak)" do - download('doesnt/exist.git') do |response| - expect(response).to have_http_status(401) - end - end - end + describe "User with no identities" do + let(:user) { create(:user) } + let(:project) { create(:project, path: 'project.git-project') } - context "when username and password are provided" do - context "when authentication fails" do - it "responds with status 401" do - download('doesnt/exist.git', user: user.username, password: "nope") do |response| + context "when the project doesn't exist" do + context "when no authentication is provided" do + it "responds with status 401 (no project existence information leak)" do + download('doesnt/exist.git') do |response| expect(response).to have_http_status(401) end end end - context "when authentication succeeds" do - it "responds with status 404" do - download('/doesnt/exist.git', user: user.username, password: user.password) do |response| - expect(response).to have_http_status(404) + context "when username and password are provided" do + context "when authentication fails" do + it "responds with status 401" do + download('doesnt/exist.git', user: user.username, password: "nope") do |response| + expect(response).to have_http_status(401) + end end end - end - end - end - - context "when the Wiki for a project exists" do - it "responds with the right project" do - wiki = ProjectWiki.new(project) - project.update_attribute(:visibility_level, Project::PUBLIC) - download("/#{wiki.repository.path_with_namespace}.git") do |response| - json_body = ActiveSupport::JSON.decode(response.body) - - expect(response).to have_http_status(200) - expect(json_body['RepoPath']).to include(wiki.repository.path_with_namespace) - expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + context "when authentication succeeds" do + it "responds with status 404" do + download('/doesnt/exist.git', user: user.username, password: user.password) do |response| + expect(response).to have_http_status(404) + end + end + end end end - end - context "when the project exists" do - let(:path) { "#{project.path_with_namespace}.git" } - - context "when the project is public" do - before do + context "when the Wiki for a project exists" do + it "responds with the right project" do + wiki = ProjectWiki.new(project) project.update_attribute(:visibility_level, Project::PUBLIC) - end - it "downloads get status 200" do - download(path, {}) do |response| + download("/#{wiki.repository.path_with_namespace}.git") do |response| + json_body = ActiveSupport::JSON.decode(response.body) + expect(response).to have_http_status(200) + expect(json_body['RepoPath']).to include(wiki.repository.path_with_namespace) expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) end end + end + + context "when the project exists" do + let(:path) { "#{project.path_with_namespace}.git" } - it "uploads get status 401" do - upload(path, {}) do |response| - expect(response).to have_http_status(401) + context "when the project is public" do + before do + project.update_attribute(:visibility_level, Project::PUBLIC) end - end - context "with correct credentials" do - let(:env) { { user: user.username, password: user.password } } + it "downloads get status 200" do + download(path, {}) do |response| + expect(response).to have_http_status(200) + expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + end + end - it "uploads get status 403" do - upload(path, env) do |response| - expect(response).to have_http_status(403) + it "uploads get status 401" do + upload(path, {}) do |response| + expect(response).to have_http_status(401) end end - context 'but git-receive-pack is disabled' do - it "responds with status 404" do - allow(Gitlab.config.gitlab_shell).to receive(:receive_pack).and_return(false) + context "with correct credentials" do + let(:env) { { user: user.username, password: user.password } } + it "uploads get status 403" do upload(path, env) do |response| expect(response).to have_http_status(403) end end - end - end - context 'but git-upload-pack is disabled' do - it "responds with status 404" do - allow(Gitlab.config.gitlab_shell).to receive(:upload_pack).and_return(false) + context 'but git-receive-pack is disabled' do + it "responds with status 404" do + allow(Gitlab.config.gitlab_shell).to receive(:receive_pack).and_return(false) - download(path, {}) do |response| - expect(response).to have_http_status(404) + upload(path, env) do |response| + expect(response).to have_http_status(403) + end + end end end - end - - context 'when the request is not from gitlab-workhorse' do - it 'raises an exception' do - expect do - get("/#{project.path_with_namespace}.git/info/refs?service=git-upload-pack") - end.to raise_error(JWT::DecodeError) - end - end - end - context "when the project is private" do - before do - project.update_attribute(:visibility_level, Project::PRIVATE) - end + context 'but git-upload-pack is disabled' do + it "responds with status 404" do + allow(Gitlab.config.gitlab_shell).to receive(:upload_pack).and_return(false) - context "when no authentication is provided" do - it "responds with status 401 to downloads" do - download(path, {}) do |response| - expect(response).to have_http_status(401) + download(path, {}) do |response| + expect(response).to have_http_status(404) + end end end - it "responds with status 401 to uploads" do - upload(path, {}) do |response| - expect(response).to have_http_status(401) + context 'when the request is not from gitlab-workhorse' do + it 'raises an exception' do + expect do + get("/#{project.path_with_namespace}.git/info/refs?service=git-upload-pack") + end.to raise_error(JWT::DecodeError) end end end - context "when username and password are provided" do - let(:env) { { user: user.username, password: 'nope' } } + context "when the project is private" do + before do + project.update_attribute(:visibility_level, Project::PRIVATE) + end - context "when authentication fails" do - it "responds with status 401" do - download(path, env) do |response| + context "when no authentication is provided" do + it "responds with status 401 to downloads" do + download(path, {}) do |response| expect(response).to have_http_status(401) end end - context "when the user is IP banned" do - it "responds with status 401" do - expect(Rack::Attack::Allow2Ban).to receive(:filter).and_return(true) - allow_any_instance_of(Rack::Request).to receive(:ip).and_return('1.2.3.4') - - clone_get(path, env) - + it "responds with status 401 to uploads" do + upload(path, {}) do |response| expect(response).to have_http_status(401) end end end - context "when authentication succeeds" do - let(:env) { { user: user.username, password: user.password } } + context "when username and password are provided" do + let(:env) { { user: user.username, password: 'nope' } } - context "when the user has access to the project" do - before do - project.team << [user, :master] + context "when authentication fails" do + it "responds with status 401" do + download(path, env) do |response| + expect(response).to have_http_status(401) + end end - context "when the user is blocked" do - it "responds with status 404" do - user.block - project.team << [user, :master] + context "when the user is IP banned" do + it "responds with status 401" do + expect(Rack::Attack::Allow2Ban).to receive(:filter).and_return(true) + allow_any_instance_of(Rack::Request).to receive(:ip).and_return('1.2.3.4') - download(path, env) do |response| - expect(response).to have_http_status(404) - end + clone_get(path, env) + + expect(response).to have_http_status(401) end end + end - context "when the user isn't blocked" do - it "downloads get status 200" do - expect(Rack::Attack::Allow2Ban).to receive(:reset) + context "when authentication succeeds" do + let(:env) { { user: user.username, password: user.password } } - clone_get(path, env) + context "when the user has access to the project" do + before do + project.team << [user, :master] + end - expect(response).to have_http_status(200) - expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + context "when the user is blocked" do + it "responds with status 404" do + user.block + project.team << [user, :master] + + download(path, env) do |response| + expect(response).to have_http_status(404) + end + end end - it "uploads get status 200" do - upload(path, env) do |response| + context "when the user isn't blocked" do + it "downloads get status 200" do + expect(Rack::Attack::Allow2Ban).to receive(:reset) + + clone_get(path, env) + expect(response).to have_http_status(200) expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) end - end - end - context "when an oauth token is provided" do - before do - application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) - @token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id) + it "uploads get status 200" do + upload(path, env) do |response| + expect(response).to have_http_status(200) + expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + end + end end - it "downloads get status 200" do - clone_get "#{project.path_with_namespace}.git", user: 'oauth2', password: @token.token + context "when an oauth token is provided" do + before do + application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) + @token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id) + end - expect(response).to have_http_status(200) - expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) - end + it "downloads get status 200" do + clone_get "#{project.path_with_namespace}.git", user: 'oauth2', password: @token.token - it "uploads get status 401 (no project existence information leak)" do - push_get "#{project.path_with_namespace}.git", user: 'oauth2', password: @token.token + expect(response).to have_http_status(200) + expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + end - expect(response).to have_http_status(401) + it "uploads get status 401 (no project existence information leak)" do + push_get "#{project.path_with_namespace}.git", user: 'oauth2', password: @token.token + + expect(response).to have_http_status(401) + end end - end - context 'when user has 2FA enabled' do - let(:user) { create(:user, :two_factor) } - let(:access_token) { create(:personal_access_token, user: user) } + context 'when user has 2FA enabled' do + let(:user) { create(:user, :two_factor) } + let(:access_token) { create(:personal_access_token, user: user) } - before do - project.team << [user, :master] - end + before do + project.team << [user, :master] + end - context 'when username and password are provided' do - it 'rejects the clone attempt' do - download("#{project.path_with_namespace}.git", user: user.username, password: user.password) do |response| - expect(response).to have_http_status(401) - expect(response.body).to include('You have 2FA enabled, please use a personal access token for Git over HTTP') + context 'when username and password are provided' do + it 'rejects the clone attempt' do + download("#{project.path_with_namespace}.git", user: user.username, password: user.password) do |response| + expect(response).to have_http_status(401) + expect(response.body).to include('You have 2FA enabled, please use a personal access token for Git over HTTP') + end end - end - it 'rejects the push attempt' do - upload("#{project.path_with_namespace}.git", user: user.username, password: user.password) do |response| - expect(response).to have_http_status(401) - expect(response.body).to include('You have 2FA enabled, please use a personal access token for Git over HTTP') + it 'rejects the push attempt' do + upload("#{project.path_with_namespace}.git", user: user.username, password: user.password) do |response| + expect(response).to have_http_status(401) + expect(response.body).to include('You have 2FA enabled, please use a personal access token for Git over HTTP') + end end end - end - context 'when username and personal access token are provided' do - it 'allows clones' do - download("#{project.path_with_namespace}.git", user: user.username, password: access_token.token) do |response| - expect(response).to have_http_status(200) + context 'when username and personal access token are provided' do + it 'allows clones' do + download("#{project.path_with_namespace}.git", user: user.username, password: access_token.token) do |response| + expect(response).to have_http_status(200) + end end - end - it 'allows pushes' do - upload("#{project.path_with_namespace}.git", user: user.username, password: access_token.token) do |response| - expect(response).to have_http_status(200) + it 'allows pushes' do + upload("#{project.path_with_namespace}.git", user: user.username, password: access_token.token) do |response| + expect(response).to have_http_status(200) + end end end end - end - context "when blank password attempts follow a valid login" do - def attempt_login(include_password) - password = include_password ? user.password : "" - clone_get path, user: user.username, password: password - response.status - end + context "when blank password attempts follow a valid login" do + def attempt_login(include_password) + password = include_password ? user.password : "" + clone_get path, user: user.username, password: password + response.status + end - it "repeated attempts followed by successful attempt" do - options = Gitlab.config.rack_attack.git_basic_auth - maxretry = options[:maxretry] - 1 - ip = '1.2.3.4' + it "repeated attempts followed by successful attempt" do + options = Gitlab.config.rack_attack.git_basic_auth + maxretry = options[:maxretry] - 1 + ip = '1.2.3.4' - allow_any_instance_of(Rack::Request).to receive(:ip).and_return(ip) - Rack::Attack::Allow2Ban.reset(ip, options) + allow_any_instance_of(Rack::Request).to receive(:ip).and_return(ip) + Rack::Attack::Allow2Ban.reset(ip, options) - maxretry.times.each do - expect(attempt_login(false)).to eq(401) - end + maxretry.times.each do + expect(attempt_login(false)).to eq(401) + end - expect(attempt_login(true)).to eq(200) - expect(Rack::Attack::Allow2Ban.banned?(ip)).to be_falsey + expect(attempt_login(true)).to eq(200) + expect(Rack::Attack::Allow2Ban.banned?(ip)).to be_falsey - maxretry.times.each do - expect(attempt_login(false)).to eq(401) - end + maxretry.times.each do + expect(attempt_login(false)).to eq(401) + end - Rack::Attack::Allow2Ban.reset(ip, options) + Rack::Attack::Allow2Ban.reset(ip, options) + end end end - end - context "when the user doesn't have access to the project" do - it "downloads get status 404" do - download(path, user: user.username, password: user.password) do |response| - expect(response).to have_http_status(404) + context "when the user doesn't have access to the project" do + it "downloads get status 404" do + download(path, user: user.username, password: user.password) do |response| + expect(response).to have_http_status(404) + end end - end - it "uploads get status 404" do - upload(path, user: user.username, password: user.password) do |response| - expect(response).to have_http_status(404) + it "uploads get status 404" do + upload(path, user: user.username, password: user.password) do |response| + expect(response).to have_http_status(404) + end end end end end - end - - context "when a gitlab ci token is provided" do - let(:build) { create(:ci_build, :running) } - let(:project) { build.project } - let(:other_project) { create(:empty_project) } - - before do - project.project_feature.update_attributes(builds_access_level: ProjectFeature::ENABLED) - end - - context 'when build created by system is authenticated' do - it "downloads get status 200" do - clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token - - expect(response).to have_http_status(200) - expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) - end - - it "uploads get status 401 (no project existence information leak)" do - push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token - - expect(response).to have_http_status(401) - end - - it "downloads from other project get status 404" do - clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token - expect(response).to have_http_status(404) - end - end + context "when a gitlab ci token is provided" do + let(:build) { create(:ci_build, :running) } + let(:project) { build.project } + let(:other_project) { create(:empty_project) } - context 'and build created by' do before do - build.update(user: user) - project.team << [user, :reporter] + project.project_feature.update_attributes(builds_access_level: ProjectFeature::ENABLED) end - shared_examples 'can download code only from own projects' do - it 'downloads get status 200' do + context 'when build created by system is authenticated' do + it "downloads get status 200" do clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token expect(response).to have_http_status(200) expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) end - it 'uploads get status 403' do + it "uploads get status 401 (no project existence information leak)" do push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token expect(response).to have_http_status(401) end + + it "downloads from other project get status 404" do + clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token + + expect(response).to have_http_status(404) + end end - context 'administrator' do - let(:user) { create(:admin) } + context 'and build created by' do + before do + build.update(user: user) + project.team << [user, :reporter] + end - it_behaves_like 'can download code only from own projects' + shared_examples 'can download code only' do + it 'downloads get status 200' do + clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token - it 'downloads from other project get status 403' do - clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token + expect(response).to have_http_status(200) + expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + end - expect(response).to have_http_status(403) + it 'uploads get status 403' do + push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token + + expect(response).to have_http_status(401) + end end - end - context 'regular user' do - let(:user) { create(:user) } + context 'administrator' do + let(:user) { create(:admin) } - it_behaves_like 'can download code only from own projects' + it_behaves_like 'can download code only' - it 'downloads from other project get status 404' do - clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token + it 'downloads from other project get status 403' do + clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token - expect(response).to have_http_status(404) + expect(response).to have_http_status(403) + end + end + + context 'regular user' do + let(:user) { create(:user) } + + it_behaves_like 'can download code only' + + it 'downloads from other project get status 404' do + clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token + + expect(response).to have_http_status(404) + end end end end end end - end - context "when the project path doesn't end in .git" do - context "GET info/refs" do - let(:path) { "/#{project.path_with_namespace}/info/refs" } + context "when the project path doesn't end in .git" do + context "GET info/refs" do + let(:path) { "/#{project.path_with_namespace}/info/refs" } - context "when no params are added" do - before { get path } + context "when no params are added" do + before { get path } - it "redirects to the .git suffix version" do - expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs") + it "redirects to the .git suffix version" do + expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs") + end end - end - context "when the upload-pack service is requested" do - let(:params) { { service: 'git-upload-pack' } } - before { get path, params } + context "when the upload-pack service is requested" do + let(:params) { { service: 'git-upload-pack' } } + before { get path, params } - it "redirects to the .git suffix version" do - expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs?service=#{params[:service]}") + it "redirects to the .git suffix version" do + expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs?service=#{params[:service]}") + end end - end - context "when the receive-pack service is requested" do - let(:params) { { service: 'git-receive-pack' } } - before { get path, params } + context "when the receive-pack service is requested" do + let(:params) { { service: 'git-receive-pack' } } + before { get path, params } - it "redirects to the .git suffix version" do - expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs?service=#{params[:service]}") + it "redirects to the .git suffix version" do + expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs?service=#{params[:service]}") + end end - end - context "when the params are anything else" do - let(:params) { { service: 'git-implode-pack' } } - before { get path, params } + context "when the params are anything else" do + let(:params) { { service: 'git-implode-pack' } } + before { get path, params } - it "redirects to the sign-in page" do - expect(response).to redirect_to(new_user_session_path) + it "redirects to the sign-in page" do + expect(response).to redirect_to(new_user_session_path) + end end end - end - context "POST git-upload-pack" do - it "fails to find a route" do - expect { clone_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError) + context "POST git-upload-pack" do + it "fails to find a route" do + expect { clone_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError) + end end - end - context "POST git-receive-pack" do - it "failes to find a route" do - expect { push_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError) + context "POST git-receive-pack" do + it "failes to find a route" do + expect { push_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError) + end end end - end - context "retrieving an info/refs file" do - before { project.update_attribute(:visibility_level, Project::PUBLIC) } + context "retrieving an info/refs file" do + before { project.update_attribute(:visibility_level, Project::PUBLIC) } - context "when the file exists" do - before do - # Provide a dummy file in its place - allow_any_instance_of(Repository).to receive(:blob_at).and_call_original - allow_any_instance_of(Repository).to receive(:blob_at).with('5937ac0a7beb003549fc5fd26fc247adbce4a52e', 'info/refs') do - Gitlab::Git::Blob.find(project.repository, 'master', '.gitignore') - end + context "when the file exists" do + before do + # Provide a dummy file in its place + allow_any_instance_of(Repository).to receive(:blob_at).and_call_original + allow_any_instance_of(Repository).to receive(:blob_at).with('5937ac0a7beb003549fc5fd26fc247adbce4a52e', 'info/refs') do + Gitlab::Git::Blob.find(project.repository, 'master', '.gitignore') + end - get "/#{project.path_with_namespace}/blob/master/info/refs" - end + get "/#{project.path_with_namespace}/blob/master/info/refs" + end - it "returns the file" do - expect(response).to have_http_status(200) + it "returns the file" do + expect(response).to have_http_status(200) + end end - end - context "when the file does not exist" do - before { get "/#{project.path_with_namespace}/blob/master/info/refs" } + context "when the file does not exist" do + before { get "/#{project.path_with_namespace}/blob/master/info/refs" } - it "returns not found" do - expect(response).to have_http_status(404) + it "returns not found" do + expect(response).to have_http_status(404) + end end end end - def clone_get(project, options = {}) - get "/#{project}/info/refs", { service: 'git-upload-pack' }, auth_env(*options.values_at(:user, :password, :spnego_request_token)) - end - - def clone_post(project, options = {}) - post "/#{project}/git-upload-pack", {}, auth_env(*options.values_at(:user, :password, :spnego_request_token)) - end - - def push_get(project, options = {}) - get "/#{project}/info/refs", { service: 'git-receive-pack' }, auth_env(*options.values_at(:user, :password, :spnego_request_token)) - end - - def push_post(project, options = {}) - post "/#{project}/git-receive-pack", {}, auth_env(*options.values_at(:user, :password, :spnego_request_token)) - end + describe "User with LDAP identity" do + let(:user) { create(:omniauth_user, extern_uid: dn) } + let(:dn) { 'uid=john,ou=people,dc=example,dc=com' } - def download(project, user: nil, password: nil, spnego_request_token: nil) - args = [project, { user: user, password: password, spnego_request_token: spnego_request_token }] + before do + allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) + allow(Gitlab::LDAP::Authentication).to receive(:login).and_return(nil) + allow(Gitlab::LDAP::Authentication).to receive(:login).with(user.username, user.password).and_return(user) + end - clone_get(*args) - yield response + context "when authentication fails" do + context "when no authentication is provided" do + it "responds with status 401" do + download('doesnt/exist.git') do |response| + expect(response).to have_http_status(401) + end + end + end - clone_post(*args) - yield response - end + context "when username and invalid password are provided" do + it "responds with status 401" do + download('doesnt/exist.git', user: user.username, password: "nope") do |response| + expect(response).to have_http_status(401) + end + end + end + end - def upload(project, user: nil, password: nil, spnego_request_token: nil) - args = [project, { user: user, password: password, spnego_request_token: spnego_request_token }] + context "when authentication succeeds" do + context "when the project doesn't exist" do + it "responds with status 404" do + download('/doesnt/exist.git', user: user.username, password: user.password) do |response| + expect(response).to have_http_status(404) + end + end + end - push_get(*args) - yield response + context "when the project exists" do + let(:project) { create(:project, path: 'project.git-project') } - push_post(*args) - yield response - end + before do + project.team << [user, :master] + end - def auth_env(user, password, spnego_request_token) - env = workhorse_internal_api_request_header - if user && password - env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(user, password) - elsif spnego_request_token - env['HTTP_AUTHORIZATION'] = "Negotiate #{::Base64.strict_encode64('opaque_request_token')}" + it "responds with status 200" do + clone_get(path, user: user.username, password: user.password) do |response| + expect(response).to have_http_status(200) + end + end + end end - - env end end diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 6b956e63004..f0ef155bd7b 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -39,7 +39,7 @@ describe JwtController do subject! { get '/jwt/auth', parameters, headers } - it { expect(response).to have_http_status(403) } + it { expect(response).to have_http_status(401) } end end @@ -77,7 +77,7 @@ describe JwtController do subject! { get '/jwt/auth', parameters, headers } - it { expect(response).to have_http_status(403) } + it { expect(response).to have_http_status(401) } end end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 09e4e265dd1..dbdf83a0dff 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -257,6 +257,29 @@ describe 'Git LFS API and storage' do it_behaves_like 'responds with a file' end + describe 'when using a user key' do + let(:authorization) { authorize_user_key } + + context 'when user allowed' do + let(:update_permissions) do + project.team << [user, :master] + project.lfs_objects << lfs_object + end + + it_behaves_like 'responds with a file' + end + + context 'when user not allowed' do + let(:update_permissions) do + project.lfs_objects << lfs_object + end + + it 'responds with status 404' do + expect(response).to have_http_status(404) + end + end + end + context 'when build is authorized as' do let(:authorization) { authorize_ci_project } @@ -1110,7 +1133,11 @@ describe 'Git LFS API and storage' do end def authorize_deploy_key - ActionController::HttpAuthentication::Basic.encode_credentials("lfs+deploy-key-#{key.id}", Gitlab::LfsToken.new(key).generate) + ActionController::HttpAuthentication::Basic.encode_credentials("lfs+deploy-key-#{key.id}", Gitlab::LfsToken.new(key).token) + end + + def authorize_user_key + ActionController::HttpAuthentication::Basic.encode_credentials(user.username, Gitlab::LfsToken.new(user).token) end def fork_project(project, user, object = nil) diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb index cf4c5f13635..e65da15aca8 100644 --- a/spec/services/boards/issues/list_service_spec.rb +++ b/spec/services/boards/issues/list_service_spec.rb @@ -13,10 +13,10 @@ describe Boards::Issues::ListService, services: true do let(:p2) { create(:label, title: 'P2', project: project, priority: 2) } let(:p3) { create(:label, title: 'P3', project: project, priority: 3) } - let!(:backlog) { create(:backlog_list, board: board) } + let!(:backlog) { project.board.backlog_list } let!(:list1) { create(:list, board: board, label: development, position: 0) } let!(:list2) { create(:list, board: board, label: testing, position: 1) } - let!(:done) { create(:done_list, board: board) } + let!(:done) { project.board.done_list } let!(:opened_issue1) { create(:labeled_issue, project: project, labels: [bug]) } let!(:opened_issue2) { create(:labeled_issue, project: project, labels: [p2]) } diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb index 0122159cab8..180f1b08631 100644 --- a/spec/services/boards/issues/move_service_spec.rb +++ b/spec/services/boards/issues/move_service_spec.rb @@ -10,10 +10,10 @@ describe Boards::Issues::MoveService, services: true do let(:development) { create(:label, project: project, name: 'Development') } let(:testing) { create(:label, project: project, name: 'Testing') } - let!(:backlog) { create(:backlog_list, board: board) } + let!(:backlog) { project.board.backlog_list } let!(:list1) { create(:list, board: board, label: development, position: 0) } let!(:list2) { create(:list, board: board, label: testing, position: 1) } - let!(:done) { create(:done_list, board: board) } + let!(:done) { project.board.done_list } before do project.team << [user, :developer] diff --git a/spec/services/boards/lists/create_service_spec.rb b/spec/services/boards/lists/create_service_spec.rb index 90764b86b16..bff9c1fd1fe 100644 --- a/spec/services/boards/lists/create_service_spec.rb +++ b/spec/services/boards/lists/create_service_spec.rb @@ -17,17 +17,15 @@ describe Boards::Lists::CreateService, services: true do end end - context 'when board lists has only a backlog list' do + context 'when board lists has backlog, and done lists' do it 'creates a new list at beginning of the list' do - create(:backlog_list, board: board) - list = service.execute expect(list.position).to eq 0 end end - context 'when board lists has only labels lists' do + context 'when board lists has labels lists' do it 'creates a new list at end of the lists' do create(:list, board: board, position: 0) create(:list, board: board, position: 1) @@ -40,8 +38,6 @@ describe Boards::Lists::CreateService, services: true do context 'when board lists has backlog, label and done lists' do it 'creates a new list at end of the label lists' do - create(:backlog_list, board: board) - create(:done_list, board: board) list1 = create(:list, board: board, position: 0) list2 = service.execute diff --git a/spec/services/boards/lists/destroy_service_spec.rb b/spec/services/boards/lists/destroy_service_spec.rb index 6eff445feee..474c4512471 100644 --- a/spec/services/boards/lists/destroy_service_spec.rb +++ b/spec/services/boards/lists/destroy_service_spec.rb @@ -15,11 +15,11 @@ describe Boards::Lists::DestroyService, services: true do end it 'decrements position of higher lists' do - backlog = create(:backlog_list, board: board) + backlog = project.board.backlog_list development = create(:list, board: board, position: 0) review = create(:list, board: board, position: 1) staging = create(:list, board: board, position: 2) - done = create(:done_list, board: board) + done = project.board.done_list described_class.new(project, user).execute(development) @@ -31,14 +31,14 @@ describe Boards::Lists::DestroyService, services: true do end it 'does not remove list from board when list type is backlog' do - list = create(:backlog_list, board: board) + list = project.board.backlog_list service = described_class.new(project, user) expect { service.execute(list) }.not_to change(board.lists, :count) end it 'does not remove list from board when list type is done' do - list = create(:done_list, board: board) + list = project.board.done_list service = described_class.new(project, user) expect { service.execute(list) }.not_to change(board.lists, :count) diff --git a/spec/services/boards/lists/move_service_spec.rb b/spec/services/boards/lists/move_service_spec.rb index 3e9b7d07fc6..102ed67449d 100644 --- a/spec/services/boards/lists/move_service_spec.rb +++ b/spec/services/boards/lists/move_service_spec.rb @@ -6,12 +6,12 @@ describe Boards::Lists::MoveService, services: true do let(:board) { project.board } let(:user) { create(:user) } - let!(:backlog) { create(:backlog_list, board: board) } + let!(:backlog) { project.board.backlog_list } let!(:planning) { create(:list, board: board, position: 0) } let!(:development) { create(:list, board: board, position: 1) } let!(:review) { create(:list, board: board, position: 2) } let!(:staging) { create(:list, board: board, position: 3) } - let!(:done) { create(:done_list, board: board) } + let!(:done) { project.board.done_list } context 'when list type is set to label' do it 'keeps position of lists when new position is nil' do diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 58569ba96c3..1050502fa19 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -20,16 +20,38 @@ describe Issues::CreateService, services: true do let(:opts) do { title: 'Awesome issue', description: 'please fix', - assignee: assignee, + assignee_id: assignee.id, label_ids: labels.map(&:id), - milestone_id: milestone.id } + milestone_id: milestone.id, + due_date: Date.tomorrow } end - it { expect(issue).to be_valid } - it { expect(issue.title).to eq('Awesome issue') } - it { expect(issue.assignee).to eq assignee } - it { expect(issue.labels).to match_array labels } - it { expect(issue.milestone).to eq milestone } + it 'creates the issue with the given params' do + expect(issue).to be_persisted + expect(issue.title).to eq('Awesome issue') + expect(issue.assignee).to eq assignee + expect(issue.labels).to match_array labels + expect(issue.milestone).to eq milestone + expect(issue.due_date).to eq Date.tomorrow + end + + context 'when current user cannot admin issues in the project' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + end + + it 'filters out params that cannot be set without the :admin_issue permission' do + issue = described_class.new(project, guest, opts).execute + + expect(issue).to be_persisted + expect(issue.title).to eq('Awesome issue') + expect(issue.assignee).to be_nil + expect(issue.labels).to be_empty + expect(issue.milestone).to be_nil + expect(issue.due_date).to be_nil + end + end it 'creates a pending todo for new assignee' do attributes = { diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 4f5375a3583..1638a46ed51 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -32,55 +32,84 @@ describe Issues::UpdateService, services: true do described_class.new(project, user, opts).execute(issue) end - context "valid params" do - before do - opts = { + context 'valid params' do + let(:opts) do + { title: 'New title', description: 'Also please fix', assignee_id: user2.id, state_event: 'close', - label_ids: [label.id] + label_ids: [label.id], + due_date: Date.tomorrow } - - perform_enqueued_jobs do - update_issue(opts) - end end - it { expect(issue).to be_valid } - it { expect(issue.title).to eq('New title') } - it { expect(issue.assignee).to eq(user2) } - it { expect(issue).to be_closed } - it { expect(issue.labels.count).to eq(1) } - it { expect(issue.labels.first.title).to eq(label.name) } - - it 'sends email to user2 about assign of new issue and email to user3 about issue unassignment' do - deliveries = ActionMailer::Base.deliveries - email = deliveries.last - recipients = deliveries.last(2).map(&:to).flatten - expect(recipients).to include(user2.email, user3.email) - expect(email.subject).to include(issue.title) + it 'updates the issue with the given params' do + update_issue(opts) + + expect(issue).to be_valid + expect(issue.title).to eq 'New title' + expect(issue.description).to eq 'Also please fix' + expect(issue.assignee).to eq user2 + expect(issue).to be_closed + expect(issue.labels).to match_array [label] + expect(issue.due_date).to eq Date.tomorrow end - it 'creates system note about issue reassign' do - note = find_note('Reassigned to') + context 'when current user cannot admin issues in the project' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + end - expect(note).not_to be_nil - expect(note.note).to include "Reassigned to \@#{user2.username}" + it 'filters out params that cannot be set without the :admin_issue permission' do + described_class.new(project, guest, opts).execute(issue) + + expect(issue).to be_valid + expect(issue.title).to eq 'New title' + expect(issue.description).to eq 'Also please fix' + expect(issue.assignee).to eq user3 + expect(issue.labels).to be_empty + expect(issue.milestone).to be_nil + expect(issue.due_date).to be_nil + end end - it 'creates system note about issue label edit' do - note = find_note('Added ~') + context 'with background jobs processed' do + before do + perform_enqueued_jobs do + update_issue(opts) + end + end + + it 'sends email to user2 about assign of new issue and email to user3 about issue unassignment' do + deliveries = ActionMailer::Base.deliveries + email = deliveries.last + recipients = deliveries.last(2).map(&:to).flatten + expect(recipients).to include(user2.email, user3.email) + expect(email.subject).to include(issue.title) + end - expect(note).not_to be_nil - expect(note.note).to include "Added ~#{label.id} label" - end + it 'creates system note about issue reassign' do + note = find_note('Reassigned to') - it 'creates system note about title change' do - note = find_note('Changed title:') + expect(note).not_to be_nil + expect(note.note).to include "Reassigned to \@#{user2.username}" + end - expect(note).not_to be_nil - expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**' + it 'creates system note about issue label edit' do + note = find_note('Added ~') + + expect(note).not_to be_nil + expect(note.note).to include "Added ~#{label.id} label" + end + + it 'creates system note about title change' do + note = find_note('Changed title:') + + expect(note).not_to be_nil + expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**' + end end end diff --git a/spec/services/members/approve_access_request_service_spec.rb b/spec/services/members/approve_access_request_service_spec.rb new file mode 100644 index 00000000000..03e296259f9 --- /dev/null +++ b/spec/services/members/approve_access_request_service_spec.rb @@ -0,0 +1,96 @@ +require 'spec_helper' + +describe Members::ApproveAccessRequestService, services: true do + let(:user) { create(:user) } + let(:access_requester) { create(:user) } + let(:project) { create(:project, :public) } + let(:group) { create(:group, :public) } + + shared_examples 'a service raising ActiveRecord::RecordNotFound' do + it 'raises ActiveRecord::RecordNotFound' do + expect { described_class.new(source, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do + it 'raises Gitlab::Access::AccessDeniedError' do + expect { described_class.new(source, user, params).execute }.to raise_error(Gitlab::Access::AccessDeniedError) + end + end + + shared_examples 'a service approving an access request' do + it 'succeeds' do + expect { described_class.new(source, user, params).execute }.to change { source.requesters.count }.by(-1) + end + + it 'returns a <Source>Member' do + member = described_class.new(source, user, params).execute + + expect(member).to be_a "#{source.class}Member".constantize + expect(member.requested_at).to be_nil + end + + context 'with a custom access level' do + let(:params) { { user_id: access_requester.id, access_level: Gitlab::Access::MASTER } } + + it 'returns a ProjectMember with the custom access level' do + member = described_class.new(source, user, params).execute + + expect(member.access_level).to eq Gitlab::Access::MASTER + end + end + end + + context 'when no access requester are found' do + let(:params) { { user_id: 42 } } + + it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do + let(:source) { project } + end + + it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do + let(:source) { group } + end + end + + context 'when an access requester is found' do + before do + project.request_access(access_requester) + group.request_access(access_requester) + end + let(:params) { { user_id: access_requester.id } } + + context 'when current user cannot approve access request to the project' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { project } + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { group } + end + end + + context 'when current user can approve access request to the project' do + before do + project.team << [user, :master] + group.add_owner(user) + end + + it_behaves_like 'a service approving an access request' do + let(:source) { project } + end + + it_behaves_like 'a service approving an access request' do + let(:source) { group } + end + + context 'when given a :id' do + let(:params) { { id: project.requesters.find_by!(user_id: access_requester.id).id } } + + it_behaves_like 'a service approving an access request' do + let(:source) { project } + end + end + end + end +end diff --git a/spec/services/members/request_access_service_spec.rb b/spec/services/members/request_access_service_spec.rb new file mode 100644 index 00000000000..0d2d5f03199 --- /dev/null +++ b/spec/services/members/request_access_service_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe Members::RequestAccessService, services: true do + let(:user) { create(:user) } + let(:project) { create(:project, :private) } + let(:group) { create(:group, :private) } + + shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do + it 'raises Gitlab::Access::AccessDeniedError' do + expect { described_class.new(source, user).execute }.to raise_error(Gitlab::Access::AccessDeniedError) + end + end + + shared_examples 'a service creating a access request' do + it 'succeeds' do + expect { described_class.new(source, user).execute }.to change { source.requesters.count }.by(1) + end + + it 'returns a <Source>Member' do + member = described_class.new(source, user).execute + + expect(member).to be_a "#{source.class}Member".constantize + expect(member.requested_at).to be_present + end + end + + context 'when source is nil' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { nil } + end + end + + context 'when current user cannot request access to the project' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { project } + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { group } + end + end + + context 'when current user can request access to the project' do + before do + project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + group.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it_behaves_like 'a service creating a access request' do + let(:source) { project } + end + + it_behaves_like 'a service creating a access request' do + let(:source) { group } + end + end +end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 159f6817e8d..31167675d07 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -38,6 +38,30 @@ describe MergeRequests::MergeService, services: true do end end + context 'closes related todos' do + let(:merge_request) { create(:merge_request, assignee: user, author: user) } + let(:project) { merge_request.project } + let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') } + let!(:todo) do + create(:todo, :assigned, + project: project, + author: user, + user: user, + target: merge_request) + end + + before do + allow(service).to receive(:execute_hooks) + + perform_enqueued_jobs do + service.execute(merge_request) + todo.reload + end + end + + it { expect(todo).to be_done } + end + context 'remove source branch by author' do let(:service) do merge_request.merge_params['force_remove_source_branch'] = '1' diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index a162df5fc34..59d3912018a 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -79,8 +79,8 @@ describe MergeRequests::RefreshService, services: true do it { expect(@merge_request).to be_merged } it { expect(@fork_merge_request).to be_merged } it { expect(@fork_merge_request.notes.last.note).to include('changed to merged') } - it { expect(@build_failed_todo).to be_pending } - it { expect(@fork_build_failed_todo).to be_pending } + it { expect(@build_failed_todo).to be_done } + it { expect(@fork_build_failed_todo).to be_done } end context 'manual merge of source branch' do @@ -99,8 +99,8 @@ describe MergeRequests::RefreshService, services: true do it { expect(@merge_request.diffs.size).to be > 0 } it { expect(@fork_merge_request).to be_merged } it { expect(@fork_merge_request.notes.last.note).to include('changed to merged') } - it { expect(@build_failed_todo).to be_pending } - it { expect(@fork_build_failed_todo).to be_pending } + it { expect(@build_failed_todo).to be_done } + it { expect(@fork_build_failed_todo).to be_done } end context 'push to fork repo source branch' do @@ -149,8 +149,8 @@ describe MergeRequests::RefreshService, services: true do it { expect(@merge_request).to be_merged } it { expect(@fork_merge_request).to be_open } it { expect(@fork_merge_request.notes).to be_empty } - it { expect(@build_failed_todo).to be_pending } - it { expect(@fork_build_failed_todo).to be_pending } + it { expect(@build_failed_todo).to be_done } + it { expect(@fork_build_failed_todo).to be_done } end context 'push new branch that exists in a merge request' do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 0d152534c38..d820646ebdf 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -962,6 +962,20 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end + it "notifies the merger when merge_when_build_succeeds is true" do + merge_request.merge_when_build_succeeds = true + notification.merge_mr(merge_request, @u_watcher) + + should_email(@u_watcher) + end + + it "does not notify the merger when merge_when_build_succeeds is false" do + merge_request.merge_when_build_succeeds = false + notification.merge_mr(merge_request, @u_watcher) + + should_not_email(@u_watcher) + end + context 'participating' do context 'by assignee' do before do diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index d5d4d7c56ef..ed1384798ab 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -108,6 +108,16 @@ describe Projects::ImportService, services: true do expect(result[:status]).to eq :error expect(result[:message]).to eq 'Github: failed to connect API' end + + it 'expires existence cache after error' do + allow_any_instance_of(Project).to receive(:repository_exists?).and_return(true) + + expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).with(project.repository_storage_path, project.path_with_namespace, project.import_url).and_raise(Gitlab::Shell::Error.new('Failed to import the repository')) + expect_any_instance_of(Repository).to receive(:expire_emptiness_caches).and_call_original + expect_any_instance_of(Repository).to receive(:expire_exists_cache).and_call_original + + subject.execute + end end def stub_github_omniauth_provider diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb index a616275e883..ae4d286d250 100644 --- a/spec/services/slash_commands/interpret_service_spec.rb +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -1,19 +1,19 @@ require 'spec_helper' describe SlashCommands::InterpretService, services: true do - let(:project) { create(:project) } - let(:user) { create(:user) } + let(:project) { create(:empty_project, :public) } + let(:developer) { create(:user) } let(:issue) { create(:issue, project: project) } let(:milestone) { create(:milestone, project: project, title: '9.10') } let(:inprogress) { create(:label, project: project, title: 'In Progress') } let(:bug) { create(:label, project: project, title: 'Bug') } before do - project.team << [user, :developer] + project.team << [developer, :developer] end describe '#execute' do - let(:service) { described_class.new(project, user) } + let(:service) { described_class.new(project, developer) } let(:merge_request) { create(:merge_request, source_project: project) } shared_examples 'reopen command' do @@ -45,13 +45,13 @@ describe SlashCommands::InterpretService, services: true do it 'fetches assignee and populates assignee_id if content contains /assign' do _, updates = service.execute(content, issuable) - expect(updates).to eq(assignee_id: user.id) + expect(updates).to eq(assignee_id: developer.id) end end shared_examples 'unassign command' do it 'populates assignee_id: nil if content contains /unassign' do - issuable.update(assignee_id: user.id) + issuable.update(assignee_id: developer.id) _, updates = service.execute(content, issuable) expect(updates).to eq(assignee_id: nil) @@ -124,7 +124,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'done command' do it 'populates todo_event: "done" if content contains /done' do - TodoService.new.mark_todo(issuable, user) + TodoService.new.mark_todo(issuable, developer) _, updates = service.execute(content, issuable) expect(updates).to eq(todo_event: 'done') @@ -141,7 +141,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'unsubscribe command' do it 'populates subscription_event: "unsubscribe" if content contains /unsubscribe' do - issuable.subscribe(user) + issuable.subscribe(developer) _, updates = service.execute(content, issuable) expect(updates).to eq(subscription_event: 'unsubscribe') @@ -165,6 +165,23 @@ describe SlashCommands::InterpretService, services: true do end end + shared_examples 'wip command' do + it 'returns wip_event: "wip" if content contains /wip' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(wip_event: 'wip') + end + end + + shared_examples 'unwip command' do + it 'returns wip_event: "unwip" if content contains /wip' do + issuable.update(title: issuable.wip_title) + _, updates = service.execute(content, issuable) + + expect(updates).to eq(wip_event: 'unwip') + end + end + shared_examples 'empty command' do it 'populates {} if content contains an unsupported command' do _, updates = service.execute(content, issuable) @@ -209,12 +226,12 @@ describe SlashCommands::InterpretService, services: true do end it_behaves_like 'assign command' do - let(:content) { "/assign @#{user.username}" } + let(:content) { "/assign @#{developer.username}" } let(:issuable) { issue } end it_behaves_like 'assign command' do - let(:content) { "/assign @#{user.username}" } + let(:content) { "/assign @#{developer.username}" } let(:issuable) { merge_request } end @@ -376,9 +393,70 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { issue } end + it_behaves_like 'wip command' do + let(:content) { '/wip' } + let(:issuable) { merge_request } + end + + it_behaves_like 'unwip command' do + let(:content) { '/wip' } + let(:issuable) { merge_request } + end + it_behaves_like 'empty command' do let(:content) { '/remove_due_date' } let(:issuable) { merge_request } end + + context 'when current_user cannot :admin_issue' do + let(:visitor) { create(:user) } + let(:issue) { create(:issue, project: project, author: visitor) } + let(:service) { described_class.new(project, visitor) } + + it_behaves_like 'empty command' do + let(:content) { "/assign @#{developer.username}" } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/unassign' } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { "/milestone %#{milestone.title}" } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/remove_milestone' } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { %(/label ~"#{inprogress.title}" ~#{bug.title} ~unknown) } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { %(/unlabel ~"#{inprogress.title}") } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { %(/relabel ~"#{inprogress.title}") } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/due tomorrow' } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/remove_due_date' } + let(:issuable) { issue } + end + end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 3d854a959f3..b16840a1238 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -40,6 +40,12 @@ describe SystemNoteService, services: true do describe 'note body' do let(:note_lines) { subject.note.split("\n").reject(&:blank?) } + describe 'comparison diff link line' do + it 'adds the comparison text' do + expect(note_lines[2]).to match "[Compare with previous version]" + end + end + context 'without existing commits' do it 'adds a message header' do expect(note_lines[0]).to eq "Added #{new_commits.size} commits:" diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 02b2b3ca101..b19f5824236 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -26,7 +26,7 @@ RSpec.configure do |config| config.verbose_retry = true config.display_try_failure_messages = true - config.include Devise::TestHelpers, type: :controller + config.include Devise::Test::ControllerHelpers, type: :controller config.include Warden::Test::Helpers, type: :request config.include LoginHelpers, type: :feature config.include StubConfiguration diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb index e8e760a6187..62a5b46d47b 100644 --- a/spec/support/cycle_analytics_helpers.rb +++ b/spec/support/cycle_analytics_helpers.rb @@ -4,24 +4,28 @@ module CycleAnalyticsHelpers create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) end - def create_commit(message, project, user, branch_name) - filename = random_git_name + def create_commit(message, project, user, branch_name, count: 1) oldrev = project.repository.commit(branch_name).sha + commit_shas = Array.new(count) do |index| + filename = random_git_name - options = { - committer: project.repository.user_to_committer(user), - author: project.repository.user_to_committer(user), - commit: { message: message, branch: branch_name, update_ref: true }, - file: { content: "content", path: filename, update: false } - } + options = { + committer: project.repository.user_to_committer(user), + author: project.repository.user_to_committer(user), + commit: { message: message, branch: branch_name, update_ref: true }, + file: { content: "content", path: filename, update: false } + } + + commit_sha = Gitlab::Git::Blob.commit(project.repository, options) + project.repository.commit(commit_sha) - commit_sha = Gitlab::Git::Blob.commit(project.repository, options) - project.repository.commit(commit_sha) + commit_sha + end GitPushService.new(project, user, oldrev: oldrev, - newrev: commit_sha, + newrev: commit_shas.last, ref: 'refs/heads/master').execute end diff --git a/spec/support/issuable_slash_commands_shared_examples.rb b/spec/support/features/issuable_slash_commands_shared_examples.rb index 5e3b8f2b23e..5e3b8f2b23e 100644 --- a/spec/support/issuable_slash_commands_shared_examples.rb +++ b/spec/support/features/issuable_slash_commands_shared_examples.rb diff --git a/spec/support/git_http_helpers.rb b/spec/support/git_http_helpers.rb new file mode 100644 index 00000000000..46b686fce94 --- /dev/null +++ b/spec/support/git_http_helpers.rb @@ -0,0 +1,48 @@ +module GitHttpHelpers + def clone_get(project, options = {}) + get "/#{project}/info/refs", { service: 'git-upload-pack' }, auth_env(*options.values_at(:user, :password, :spnego_request_token)) + end + + def clone_post(project, options = {}) + post "/#{project}/git-upload-pack", {}, auth_env(*options.values_at(:user, :password, :spnego_request_token)) + end + + def push_get(project, options = {}) + get "/#{project}/info/refs", { service: 'git-receive-pack' }, auth_env(*options.values_at(:user, :password, :spnego_request_token)) + end + + def push_post(project, options = {}) + post "/#{project}/git-receive-pack", {}, auth_env(*options.values_at(:user, :password, :spnego_request_token)) + end + + def download(project, user: nil, password: nil, spnego_request_token: nil) + args = [project, { user: user, password: password, spnego_request_token: spnego_request_token }] + + clone_get(*args) + yield response + + clone_post(*args) + yield response + end + + def upload(project, user: nil, password: nil, spnego_request_token: nil) + args = [project, { user: user, password: password, spnego_request_token: spnego_request_token }] + + push_get(*args) + yield response + + push_post(*args) + yield response + end + + def auth_env(user, password, spnego_request_token) + env = workhorse_internal_api_request_header + if user && password + env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(user, password) + elsif spnego_request_token + env['HTTP_AUTHORIZATION'] = "Negotiate #{::Base64.strict_encode64('opaque_request_token')}" + end + + env + end +end diff --git a/spec/support/import_export/configuration_helper.rb b/spec/support/import_export/configuration_helper.rb new file mode 100644 index 00000000000..f752508d48c --- /dev/null +++ b/spec/support/import_export/configuration_helper.rb @@ -0,0 +1,29 @@ +module ConfigurationHelper + # Returns a list of models from hashes/arrays contained in +project_tree+ + def names_from_tree(project_tree) + project_tree.map do |branch_or_model| + branch_or_model = branch_or_model.to_s if branch_or_model.is_a?(Symbol) + + branch_or_model.is_a?(String) ? branch_or_model : names_from_tree(branch_or_model) + end + end + + def relation_class_for_name(relation_name) + relation_name = Gitlab::ImportExport::RelationFactory::OVERRIDES[relation_name.to_sym] || relation_name + relation_name.to_s.classify.constantize + end + + def parsed_attributes(relation_name, attributes) + excluded_attributes = config_hash['excluded_attributes'][relation_name] + included_attributes = config_hash['included_attributes'][relation_name] + + attributes = attributes - JSON[excluded_attributes.to_json] if excluded_attributes + attributes = attributes & JSON[included_attributes.to_json] if included_attributes + + attributes + end + + def associations_for(safe_model) + safe_model.reflect_on_all_associations.map { |assoc| assoc.name.to_s } + end +end diff --git a/spec/support/import_export/export_file_helper.rb b/spec/support/import_export/export_file_helper.rb new file mode 100644 index 00000000000..be0772d6a4a --- /dev/null +++ b/spec/support/import_export/export_file_helper.rb @@ -0,0 +1,133 @@ +require './spec/support/import_export/configuration_helper' + +module ExportFileHelper + include ConfigurationHelper + + ObjectWithParent = Struct.new(:object, :parent, :key_found) + + def setup_project + project = create(:project, :public) + + create(:release, project: project) + + issue = create(:issue, assignee: user, project: project) + snippet = create(:project_snippet, project: project) + label = create(:label, project: project) + milestone = create(:milestone, project: project) + merge_request = create(:merge_request, source_project: project, milestone: milestone) + commit_status = create(:commit_status, project: project) + + create(:label_link, label: label, target: issue) + + ci_pipeline = create(:ci_pipeline, + project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + statuses: [commit_status]) + + create(:ci_build, pipeline: ci_pipeline, project: project) + create(:milestone, project: project) + create(:note, noteable: issue, project: project) + create(:note, noteable: merge_request, project: project) + create(:note, noteable: snippet, project: project) + create(:note_on_commit, + author: user, + project: project, + commit_id: ci_pipeline.sha) + + create(:event, target: milestone, project: project, action: Event::CREATED, author: user) + create(:project_member, :master, user: user, project: project) + create(:ci_variable, project: project) + create(:ci_trigger, project: project) + key = create(:deploy_key) + key.projects << project + create(:service, project: project) + create(:project_hook, project: project, token: 'token') + create(:protected_branch, project: project) + + project + end + + # Expands the compressed file for an exported project into +tmpdir+ + def in_directory_with_expanded_export(project) + Dir.mktmpdir do |tmpdir| + export_file = project.export_project_path + _output, exit_status = Gitlab::Popen.popen(%W{tar -zxf #{export_file} -C #{tmpdir}}) + + yield(exit_status, tmpdir) + end + end + + # Recursively finds key/values including +key+ as part of the key, inside a nested hash + def deep_find_with_parent(sensitive_key_word, object, found = nil) + sensitive_key_found = object_contains_key?(object, sensitive_key_word) + + # Returns the parent object and the object found containing a sensitive word as part of the key + if sensitive_key_found && object[sensitive_key_found] + ObjectWithParent.new(object[sensitive_key_found], object, sensitive_key_found) + elsif object.is_a?(Enumerable) + # Recursively lookup for keys containing sensitive words in a Hash or Array + object_with_parent = nil + + object.find do |*hash_or_array| + object_with_parent = deep_find_with_parent(sensitive_key_word, hash_or_array.last, found) + end + + object_with_parent + end + end + + # Return true if the hash has a key containing a sensitive word + def object_contains_key?(object, sensitive_key_word) + return false unless object.is_a?(Hash) + + object.keys.find { |key| key.include?(sensitive_key_word) } + end + + # Returns the offended ObjectWithParent object if a sensitive word is found inside a hash, + # excluding the whitelisted safe hashes. + def find_sensitive_attributes(sensitive_word, project_hash) + loop do + object_with_parent = deep_find_with_parent(sensitive_word, project_hash) + + return nil unless object_with_parent && object_with_parent.object + + if is_safe_hash?(object_with_parent.parent, sensitive_word) + # It's in the safe list, remove hash and keep looking + object_with_parent.parent.delete(object_with_parent.key_found) + else + return object_with_parent + end + + nil + end + end + + # Returns true if it's one of the excluded models in +safe_list+ + def is_safe_hash?(parent, sensitive_word) + return false unless parent && safe_list[sensitive_word.to_sym] + + # Extra attributes that appear in a model but not in the exported hash. + excluded_attributes = ['type'] + + safe_list[sensitive_word.to_sym].each do |model| + # Check whether this is a hash attribute inside a model + if model.is_a?(Symbol) + return true if (safe_hashes[model] - parent.keys).empty? + else + return true if safe_model?(model, excluded_attributes, parent) + end + end + + false + end + + # Compares model attributes with those those found in the hash + # and returns true if there is a match, ignoring some excluded attributes. + def safe_model?(model, excluded_attributes, parent) + excluded_attributes += associations_for(model) + parsed_model_attributes = parsed_attributes(model.name.underscore, model.attribute_names) + + (parsed_model_attributes - parent.keys - excluded_attributes).empty? + end +end diff --git a/spec/support/matchers/have_issuable_counts.rb b/spec/support/matchers/have_issuable_counts.rb new file mode 100644 index 00000000000..02605d6b70e --- /dev/null +++ b/spec/support/matchers/have_issuable_counts.rb @@ -0,0 +1,21 @@ +RSpec::Matchers.define :have_issuable_counts do |opts| + match do |actual| + expected_counts = opts.map do |state, count| + "#{state.to_s.humanize} #{count}" + end + + actual.within '.issues-state-filters' do + expected_counts.each do |expected_count| + expect(actual).to have_content(expected_count) + end + end + end + + description do + "displays the following issuable counts: #{expected_counts.inspect}" + end + + failure_message do + "expected the following issuable counts: #{expected_counts.inspect} to be displayed" + end +end diff --git a/spec/support/issuable_create_service_slash_commands_shared_examples.rb b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb index 5f9645ed44f..5f9645ed44f 100644 --- a/spec/support/issuable_create_service_slash_commands_shared_examples.rb +++ b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb diff --git a/spec/support/snippets_shared_examples.rb b/spec/support/snippets_shared_examples.rb new file mode 100644 index 00000000000..57dfff3471f --- /dev/null +++ b/spec/support/snippets_shared_examples.rb @@ -0,0 +1,18 @@ +# These shared examples expect a `snippets` array of snippets +RSpec.shared_examples 'paginated snippets' do |remote: false| + it "is limited to #{Snippet.default_per_page} items per page" do + expect(page.all('.snippets-list-holder .snippet-row').count).to eq(Snippet.default_per_page) + end + + context 'clicking on the link to the second page' do + before do + click_link('2') + wait_for_ajax if remote + end + + it 'shows the remaining snippets' do + remaining_snippets_count = [snippets.size - Snippet.default_per_page, Snippet.default_per_page].min + expect(page).to have_selector('.snippets-list-holder .snippet-row', count: remaining_snippets_count) + end + end +end diff --git a/spec/views/admin/dashboard/index.html.haml_spec.rb b/spec/views/admin/dashboard/index.html.haml_spec.rb index dae858a52f6..68d2d72876e 100644 --- a/spec/views/admin/dashboard/index.html.haml_spec.rb +++ b/spec/views/admin/dashboard/index.html.haml_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'admin/dashboard/index.html.haml' do - include Devise::TestHelpers + include Devise::Test::ControllerHelpers before do assign(:projects, create_list(:empty_project, 1)) diff --git a/spec/views/ci/lints/show.html.haml_spec.rb b/spec/views/ci/lints/show.html.haml_spec.rb new file mode 100644 index 00000000000..793b747e7eb --- /dev/null +++ b/spec/views/ci/lints/show.html.haml_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe 'ci/lints/show' do + let(:content) do + { + build_template: { + script: './build.sh', + tags: ['dotnet'], + only: ['test@dude/repo'], + except: ['deploy'], + environment: 'testing' + } + } + end + + let(:config_processor) { Ci::GitlabCiYamlProcessor.new(YAML.dump(content)) } + + context 'when the content is valid' do + before do + assign(:status, true) + assign(:builds, config_processor.builds) + assign(:stages, config_processor.stages) + assign(:jobs, config_processor.jobs) + end + + it 'shows the correct values' do + render + + expect(rendered).to have_content('Tag list: dotnet') + expect(rendered).to have_content('Refs only: test@dude/repo') + expect(rendered).to have_content('Refs except: deploy') + expect(rendered).to have_content('Environment: testing') + expect(rendered).to have_content('When: on_success') + end + end + + context 'when the content is invalid' do + before do + assign(:status, false) + assign(:error, 'Undefined error') + end + + it 'shows error message' do + render + + expect(rendered).to have_content('Status: syntax is incorrect') + expect(rendered).to have_content('Error: Undefined error') + expect(rendered).not_to have_content('Tag list:') + end + end +end diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index 446ba3bfa14..da43622d3f9 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'projects/builds/show' do - include Devise::TestHelpers + include Devise::Test::ControllerHelpers let(:project) { create(:project) } let(:pipeline) do diff --git a/spec/views/projects/issues/_related_branches.html.haml_spec.rb b/spec/views/projects/issues/_related_branches.html.haml_spec.rb index 78af61f15a7..c8a3d02d8fd 100644 --- a/spec/views/projects/issues/_related_branches.html.haml_spec.rb +++ b/spec/views/projects/issues/_related_branches.html.haml_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'projects/issues/_related_branches' do - include Devise::TestHelpers + include Devise::Test::ControllerHelpers let(:project) { create(:project) } let(:branch) { project.repository.find_branch('feature') } diff --git a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb index 733b2dfa7ff..86980f59cd8 100644 --- a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'projects/merge_requests/widget/_heading' do - include Devise::TestHelpers + include Devise::Test::ControllerHelpers context 'when released to an environment' do let(:project) { merge_request.target_project } @@ -15,6 +15,8 @@ describe 'projects/merge_requests/widget/_heading' do assign(:merge_request, merge_request) assign(:project, project) + allow(view).to receive(:can?).and_return(true) + render end diff --git a/spec/views/projects/merge_requests/edit.html.haml_spec.rb b/spec/views/projects/merge_requests/edit.html.haml_spec.rb index 31bbb150698..26ea252fecb 100644 --- a/spec/views/projects/merge_requests/edit.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/edit.html.haml_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'projects/merge_requests/edit.html.haml' do - include Devise::TestHelpers + include Devise::Test::ControllerHelpers let(:user) { create(:user) } let(:project) { create(:project) } diff --git a/spec/views/projects/merge_requests/show.html.haml_spec.rb b/spec/views/projects/merge_requests/show.html.haml_spec.rb index fe0780e72df..33cabd14913 100644 --- a/spec/views/projects/merge_requests/show.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/show.html.haml_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'projects/merge_requests/show.html.haml' do - include Devise::TestHelpers + include Devise::Test::ControllerHelpers let(:user) { create(:user) } let(:project) { create(:project) } @@ -41,4 +41,17 @@ describe 'projects/merge_requests/show.html.haml' do expect(rendered).to have_css('a', visible: false, text: 'Close') end end + + context 'when the merge request is open' do + it 'closes the merge request if the source project does not exist' do + closed_merge_request.update_attributes(state: 'open') + fork_project.destroy + + render + + expect(closed_merge_request.reload.state).to eq('closed') + expect(rendered).to have_css('a', visible: false, text: 'Reopen') + expect(rendered).to have_css('a', visible: false, text: 'Close') + end + end end diff --git a/spec/views/projects/notes/_form.html.haml_spec.rb b/spec/views/projects/notes/_form.html.haml_spec.rb index 932d6756ad2..b14b1ece2d0 100644 --- a/spec/views/projects/notes/_form.html.haml_spec.rb +++ b/spec/views/projects/notes/_form.html.haml_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'projects/notes/_form' do - include Devise::TestHelpers + include Devise::Test::ControllerHelpers let(:user) { create(:user) } let(:project) { create(:empty_project) } diff --git a/spec/views/projects/pipelines/show.html.haml_spec.rb b/spec/views/projects/pipelines/show.html.haml_spec.rb index ac7f3ffb157..bf027499c94 100644 --- a/spec/views/projects/pipelines/show.html.haml_spec.rb +++ b/spec/views/projects/pipelines/show.html.haml_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'projects/pipelines/show' do - include Devise::TestHelpers + include Devise::Test::ControllerHelpers let(:project) { create(:project) } let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id) } diff --git a/spec/views/projects/tree/show.html.haml_spec.rb b/spec/views/projects/tree/show.html.haml_spec.rb index 0f3fc1ee1ac..c381b1a86df 100644 --- a/spec/views/projects/tree/show.html.haml_spec.rb +++ b/spec/views/projects/tree/show.html.haml_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'projects/tree/show' do - include Devise::TestHelpers + include Devise::Test::ControllerHelpers let(:project) { create(:project) } let(:repository) { project.repository } |